Conversation
There was a problem hiding this comment.
Pull request overview
Updates the AvoidGlobalVariableInFunctionCheck to avoid flagging Python typing constructs (e.g., TypeVar) as “global variables”, aligning the rule behavior with common typing patterns.
Changes:
- Adjusts global-variable collection to skip module-level assignments that are
TypeVar/ParamSpec/NewType/TypeVarTupleconstructions. - Adds a compliant test case verifying that
TypeVarusage inside a function does not raise an issue. - Updates the JUnit test to validate both a noncompliant and a compliant sample file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/main/java/org/greencodeinitiative/creedengo/python/checks/AvoidGlobalVariableInFunctionCheck.java |
Reworks module-global detection and adds typing-construct exclusion logic. |
src/test/java/org/greencodeinitiative/creedengo/python/checks/AvoidGlobalVariableInFunctionCheckTest.java |
Updates verification to include a “no issue” compliant case (and adjusts visibility). |
src/test/resources/checks/avoidGlobalVariableInFunctionCompliant.py |
Makes the TypeVar sample compliant by removing the “Noncompliant” marker. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public void visitFileInput(SubscriptionContext ctx) { | ||
| FileInput fileInput = (FileInput) ctx.syntaxNode(); | ||
| fileInput.globalVariables().stream().filter(v -> v.is(Symbol.Kind.OTHER)).forEach(v -> this.globalVariables.add(v.name())); | ||
|
|
||
| // Add all module-level assignments except TypeVar and similar typing constructs | ||
| StatementList statements = fileInput.statements(); | ||
| if (statements != null) { | ||
| statements.statements().forEach(this::extractGlobalVariablesFromStatement); | ||
| } | ||
| } |
There was a problem hiding this comment.
visitFileInput only inspects the direct children of fileInput.statements() and extractGlobalVariablesFromStatement only handles ASSIGNMENT_STMT / ANNOTATED_ASSIGNMENT. Module-scope assignments inside compound statements (e.g., if, try, for, with, etc.) will be ignored, so globals defined there won’t be tracked and their usage inside functions will no longer be reported (regression vs the previous symbol-based approach). Consider collecting globals via a recursive traversal/visitor over the full module AST (while still excluding TypeVar-like constructs), or reusing fileInput.globalVariables() and filtering out typing-construct definitions more precisely.
.../org/greencodeinitiative/creedengo/python/checks/AvoidGlobalVariableInFunctionCheckTest.java
Show resolved
Hide resolved
|
1 participant
No description provided.