Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
Dec 7, 2022
Merged

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

merged 4 commits into from
Dec 7, 2022

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

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
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

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

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 <carl@oddbird.net>
@bedevere-bot
Copy link

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

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 <carl@oddbird.net>
@bedevere-bot
Copy link

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

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 <carl@oddbird.net>
@JelleZijlstra
Copy link
Member

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 <carl@oddbird.net>
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 <carl@oddbird.net>
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.

None yet
5 participants
@carljm @JelleZijlstra @miss-islington @bedevere-bot @AlexWaygood