fix(python): only create subscope for FunctionDefinition#103
fix(python): only create subscope for FunctionDefinition#103Arielwyy merged 1 commit intoantgroup:mainfrom
Conversation
CrackTC
requested review from
AntJiuFo,
Arielwyy and
alipaydeshui
as code owners
Summary of ChangesHello @CrackTC, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Python analyzer's scope creation mechanism to accurately reflect Python's variable scoping rules. Previously, subscopes were indiscriminately created for various block statements, leading to incorrect scope management. The updated logic ensures that subscopes are only generated for blocks that are children of Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a bug in the Python analyzer where sub-scopes were being created for block statements that do not introduce new variable scopes in Python, such as if, for, and while. The fix correctly limits the creation of new scopes to ScopedStatement nodes within a FunctionDefinition, which aligns with Python's scoping rules. The implementation is correct. I've added one minor suggestion to improve code conciseness.
| let block_scope = scope | ||
| if (node.parent?.type === 'FunctionDefinition') { | ||
| // 只对函数体内的块语句创建子作用域，python的其他块语句不创建子作用域 | ||
| block_scope = Scope.createSubScope(scopeName, scope, 'scope') | ||
| } |
There was a problem hiding this comment.
This logic can be made more concise by using a ternary operator. This also allows block_scope to be declared with const, which is generally preferred for variables that are not reassigned.
| let block_scope = scope | |
| if (node.parent?.type === 'FunctionDefinition') { | |
| // 只对函数体内的块语句创建子作用域，python的其他块语句不创建子作用域 | |
| block_scope = Scope.createSubScope(scopeName, scope, 'scope') | |
| } | |
| // 只对函数体内的块语句创建子作用域，python的其他块语句不创建子作用域 | |
| const block_scope = | |
| node.parent?.type === 'FunctionDefinition' | |
| ? Scope.createSubScope(scopeName, scope, 'scope') | |
| : scope |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if (node.parent?.type === 'FunctionDefinition') { | ||
| // 只对函数体内的块语句创建子作用域，python的其他块语句不创建子作用域 | ||
| block_scope = Scope.createSubScope(scopeName, scope, 'scope') | ||
| } |
There was a problem hiding this comment.
List comprehensions may lose scope isolation in Python 3
Medium Severity
The change only creates subscopes for ScopedStatement nodes whose parent is FunctionDefinition. The PR description claims list comprehensions don't define scopes in Python, but this is incorrect for Python 3 where list comprehensions DO have their own scope (the loop variable doesn't leak to the enclosing scope). If the YASA-UAST parser doesn't wrap list comprehensions in FunctionDefinition nodes (as it does for lambdas and class bodies), comprehension variables would incorrectly be analyzed as belonging to the outer scope, potentially causing incorrect analysis results.
2 participants
After investigating YASA-UAST python parser, I found that make subscope only when parent is
FunctionDefinitionis enough for fixing this bug.There are 9 situations that generates
ScopedStatementWe check
FunctionDefinitionso 1 is OKFor 2 and 6, the parser generates
FunctionDefinitionas parent ofScopedStatement, so we covered these situations:https://github.com/antgroup/YASA-UAST/blob/3bc130288bbf8135fe3ba8f945e225ea861d386f/parser-Python/uast/visitor.py#L352-L359
https://github.com/antgroup/YASA-UAST/blob/3bc130288bbf8135fe3ba8f945e225ea861d386f/parser-Python/uast/visitor.py#L813-L816
For 3, 4, 5, 7, 8 and 9, they aren't defining variable scopes in python syntax, so this is the bug per se and we shouldn't make subscope for them.
Note
Adjusts Python block scoping to align with Python semantics.
python-analyzer.tsprocessScopedStatement, only create a newscopefor block statements whose parent isFunctionDefinition; otherwise reuse the currentscopeneedCompileFirstvs non-first) andTryStatementhandling unchangedWritten by Cursor Bugbot for commit 90ff6bb. This will update automatically on new commits. Configure here.