Skip to content

gh-83035: handle decorator with nested parens in inspect.getsource#99654

Merged
JelleZijlstra merged 4 commits intopython:mainfrom
carljm:fixinspect
Dec 7, 2022
Merged

gh-83035: handle decorator with nested parens in inspect.getsource#99654
JelleZijlstra merged 4 commits intopython:mainfrom
carljm:fixinspect

Conversation

@carljm
Copy link
Member

@carljm carljm commented Nov 21, 2022 •
edited by bedevere-bot
Loading

@bedevere-bot bedevere-bot mentioned this pull request Nov 21, 2022
Closed
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Nov 21, 2022
@facebook-github-bot
backport inspect.getsource fix for nested parens in decorators
1e69b5f 
Summary:
Backport python/cpython#99654 to fix
`inspect.getsource` handling of decorator calls with nested parens.

Reviewed By: itamaro

Differential Revision: D41438553

fbshipit-source-id: 420a90d
@JelleZijlstra
Copy link
Member

JelleZijlstra commented Nov 25, 2022

I think there's a simpler solution: remove the special cases for parentheses completely. That code is unnecessary because we don't emit NEWLINE tokens for newlines within parentheses. Instead we emit NL:

In [46]: def t(s):
    ...:     return list(tokenize.tokenize(io.BytesIO(s.encode()).readline))

In [48]: t("""(x
    ...: )""")
Out[48]: 
[TokenInfo(type=63 (ENCODING), string='utf-8', start=(0, 0), end=(0, 0), line=''),
 TokenInfo(type=54 (OP), string='(', start=(1, 0), end=(1, 1), line='(x\n'),
 TokenInfo(type=1 (NAME), string='x', start=(1, 1), end=(1, 2), line='(x\n'),
 TokenInfo(type=62 (NL), string='\n', start=(1, 2), end=(1, 3), line='(x\n'),
 TokenInfo(type=54 (OP), string=')', start=(2, 0), end=(2, 1), line=')'),
 TokenInfo(type=4 (NEWLINE), string='', start=(2, 1), end=(2, 2), line=''),
 TokenInfo(type=0 (ENDMARKER), string='', start=(3, 0), end=(3, 0), line='')]

If the parens-handling code was necessary, we would see a similar bug with [] but we don't.

@carljm
Copy link
Member Author

carljm commented Dec 1, 2022 •
edited
Loading

I think there's a simpler solution

Good call! I wondered about that, but decided to just stick with the less-invasive fix. Thanks for the nudge, I'll try just removing the paren-handling instead.

carljm added 2 commits December 1, 2022 07:00
@carljm
Merge branch 'main' into fixinspect
12f7659 
* main: (112 commits)
  pythongh-99894: Ensure the local names don't collide with the test file in traceback suggestion error checking (python#99895)
  pythongh-99612: Fix PyUnicode_DecodeUTF8Stateful() for ASCII-only data (pythonGH-99613)
  Doc: Add summary line to isolation_level & autocommit sqlite3.connect params (python#99917)
  pythonGH-98906 ```re``` module: ```search() vs. match()``` section should mention ```fullmatch()``` (pythonGH-98916)
  pythongh-89189: More compact range iterator (pythonGH-27986)
  bpo-47220: Document the optional callback parameter of weakref.WeakMethod (pythonGH-25491)
  pythonGH-99905: Fix output of misses in summarize_stats.py execution counts (pythonGH-99906)
  pythongh-99845: PEP 670: Convert PyObject macros to functions (python#99850)
  pythongh-99845: Use size_t type in __sizeof__() methods (python#99846)
  pythonGH-99877)
  Fix typo in exception message in `multiprocessing.pool` (python#99900)
  pythongh-87092: move all localsplus preparation into separate function called from assembler stage (pythonGH-99869)
  pythongh-99891: Fix infinite recursion in the tokenizer when showing warnings (pythonGH-99893)
  pythongh-99824: Document that sqlite3.connect implicitly open a transaction if autocommit=False (python#99825)
  pythonGH-81057: remove static state from suggestions.c (python#99411)
  Improve zip64 limit error message (python#95892)
  pythongh-98253: Break potential reference cycles in external code worsened by typing.py lru_cache (python#98591)
  pythongh-99127: Allow some features of syslog to the main interpreter only (pythongh-99128)
  pythongh-82836: fix private network check (python#97733)
  Docs: improve accuracy of socketserver reference (python#24767)
  ...
@carljm
just remove unnecessary paren handling
6045852
@carljm
Copy link
Member Author

carljm commented Dec 1, 2022

Updated.

I also wondered looking at this code whether we couldn't just use the AST lineno information for all objects (as is already done for classes, since #35113) and eliminate the need for BlockFinder altogether. But that's definitely a more involved change.

JelleZijlstra
JelleZijlstra approved these changes Dec 7, 2022
View reviewed changes
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good!

I feel this should be safe to backport to the bugfix branches, do you agree?

@JelleZijlstra JelleZijlstra self-assigned this Dec 7, 2022
@carljm
Copy link
Member Author

carljm commented Dec 7, 2022

@JelleZijlstra Yes, I think this should be safe to backport.

@JelleZijlstra JelleZijlstra added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Dec 7, 2022
@JelleZijlstra JelleZijlstra merged commit 68e4129 into python:main Dec 7, 2022
@miss-islington
Copy link
Contributor

miss-islington commented Dec 7, 2022

Thanks @carljm for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Dec 7, 2022

Sorry @carljm and @JelleZijlstra, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 68e41295b8611a990de68f15c89f1eb3dea51867 3.11

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 7, 2022
@carljm @miss-islington
pythongh-83035: handle decorator with nested parens in inspect.getsou…
15592a7 
…rce (pythonGH-99654)

(cherry picked from commit 68e4129)

Co-authored-by: Carl Meyer <[email protected]>
@bedevere-bot
Copy link

bedevere-bot commented Dec 7, 2022

GH-100080 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Dec 7, 2022
@AlexWaygood AlexWaygood added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Dec 7, 2022
@miss-islington
Copy link
Contributor

miss-islington commented Dec 7, 2022

Thanks @carljm for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@JelleZijlstra JelleZijlstra removed the needs backport to 3.11 only security fixes label Dec 7, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 7, 2022
@carljm @miss-islington
pythongh-83035: handle decorator with nested parens in inspect.getsou…
efdc98f 
…rce (pythonGH-99654)

(cherry picked from commit 68e4129)

Co-authored-by: Carl Meyer <[email protected]>
@bedevere-bot
Copy link

bedevere-bot commented Dec 7, 2022

GH-100081 is a backport of this pull request to the 3.11 branch.

@JelleZijlstra JelleZijlstra added the needs backport to 3.11 only security fixes label Dec 7, 2022
@miss-islington
Copy link
Contributor

miss-islington commented Dec 7, 2022

Thanks @carljm for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 7, 2022
@carljm @miss-islington
pythongh-83035: handle decorator with nested parens in inspect.getsou…
9f52249 
…rce (pythonGH-99654)

(cherry picked from commit 68e4129)

Co-authored-by: Carl Meyer <[email protected]>
@JelleZijlstra
Copy link
Member

JelleZijlstra commented Dec 7, 2022

oops now we'll have two backports

@carljm carljm deleted the fixinspect branch December 7, 2022 16:57
@AlexWaygood
Copy link
Member

AlexWaygood commented Dec 7, 2022 •
edited
Loading

oops now we'll have two backports

No, she force-pushes to the existing backport branch if this happens (idk if it's an undocumented feature or a bug, it's a bit strange)

@AlexWaygood AlexWaygood removed the needs backport to 3.11 only security fixes label Dec 7, 2022
@AlexWaygood AlexWaygood mentioned this pull request Dec 7, 2022
Closed
3 tasks
miss-islington added a commit that referenced this pull request Dec 7, 2022
@miss-islington @carljm
gh-83035: handle decorator with nested parens in inspect.getsource (G…
fe7c309 
…H-99654)

(cherry picked from commit 68e4129)

Co-authored-by: Carl Meyer <[email protected]>
miss-islington added a commit that referenced this pull request Dec 7, 2022
@miss-islington @carljm
gh-83035: handle decorator with nested parens in inspect.getsource (G…
2997f39 
…H-99654)

(cherry picked from commit 68e4129)

Co-authored-by: Carl Meyer <[email protected]>
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Dec 8, 2022
@itamaro @facebook-github-bot
Backport gh-83035: handle decorator with nested parens in inspect.get…
6e1ebd5 
…source (#99654)

Summary: Backport the upstream fix from python/cpython#99654

Reviewed By: carljm

Differential Revision: D41824891

fbshipit-source-id: 903f676
@carljm carljm mentioned this pull request Mar 13, 2023
Closed
@iclementine iclementine mentioned this pull request Jul 10, 2024
Closed
@StrongSpoon StrongSpoon mentioned this pull request Jul 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood

Assignees

@JelleZijlstra JelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@carljm @JelleZijlstra @miss-islington @bedevere-bot @AlexWaygood