-
Notifications
You must be signed in to change notification settings - Fork 66
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.
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
Do not enter local scopes in python #1982
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (50.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoop Whoop. I adjusted the "failing" tests, they still were under the assumption that the local scope exists. This was bugging me for so long and we could finally got rid of the local scopes - also thanks to the EOG rework of @konradweiss
I will leave this unmerged until @maximiliankaul has a chance to look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have identified some issues not related to this PR, which will be handled in a separate issue.
...language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt
Show resolved
Hide resolved
...nguage-python/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/python/PythonFrontendTest.kt
Show resolved
Hide resolved
...language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt
Show resolved
Hide resolved
* Do not enter local scopes in python * Revert wronge change * Fixed testGlobal * Fixed testVarsAndFields * Fixed testNonLocal --------- Co-authored-by: Christian Banse <[email protected]>
Python function declarations somehow enter a
LocalScope
which doesn't make sense. This feature is no longer needed by the graph and was there for legacy reasons. Also lead to incorrect lookups of variables. After this PR, python does no longer enter theseLocalScope
s . Since the feature was error-prone and only used in a single place (no not at all any more), it's removed completely.Fixes #1978