Reapply "`ast.NodeVisitor` for import tracking" by ShaharNaveh · Pull Request #7241 · RustPython/RustPython
53-60: Remove commented-out code.
Line 56 contains a commented-out expression # module.split(".", 1)[0] that appears to be a debug artifact or deferred decision.
♻️ Clean up
`@property`
def lib_imports(self) -> frozenset[str]:
return frozenset(
- # module.split(".", 1)[0]
module
for module in self.__imports
if not module.startswith("test.")
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/update_lib/deps.py` around lines 53 - 60, Remove the leftover
commented-out expression from the lib_imports property: delete the line
containing "# module.split(\".\", 1)[0]" inside the lib_imports getter so the
generator comprehension only yields module from self.__imports when not starting
with "test."; ensure no other logic is altered in the lib_imports property or
references to __imports.
110-112: Potential IndexError if support.findfile() is called without arguments.
Line 110 accesses node.args[0] without checking if args is non-empty. While unlikely in practice (a call to findfile() without arguments would be a bug in the test code), defensive coding would prevent a crash in the import parser.
🛡️ Proposed defensive check
if (value.id != "support") or (func.attr != "findfile"):
return
+ if not node.args:
+ return
+
arg = node.args[0]
if not isinstance(arg, ast.Constant):
return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/update_lib/deps.py` around lines 110 - 112, The code accesses
node.args[0] without ensuring node.args is non-empty which can raise IndexError
when parsing a call like support.findfile() with no arguments; update the logic
around the node handling (where node.args is read) to first check that node.args
is truthy/has length > 0 before accessing node.args[0], and early-return (as
done for the ast.Constant check) if there are no args so support.findfile()
calls without args are safely ignored.
122-139: Return type annotation mismatch.
parse_test_imports and parse_lib_imports are annotated to return set[str], but they return visitor.test_imports and visitor.lib_imports which are frozenset[str]. This works at runtime (frozenset is compatible), but the type hints are inaccurate.
♻️ Fix type annotations
-def parse_test_imports(content: str) -> set[str]: +def parse_test_imports(content: str) -> frozenset[str]: """Parse test file content and extract test package dependencies.""" if not (tree := safe_parse_ast(content)): - return set() + return frozenset() visitor = ImportVisitor() visitor.visit(tree) return visitor.test_imports -def parse_lib_imports(content: str) -> set[str]: +def parse_lib_imports(content: str) -> frozenset[str]: """Parse library file and extract all imported module names.""" if not (tree := safe_parse_ast(content)): - return set() + return frozenset() visitor = ImportVisitor() visitor.visit(tree) return visitor.lib_imports
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/update_lib/deps.py` around lines 122 - 139, The functions
parse_test_imports and parse_lib_imports are annotated to return set[str] but
actually return frozenset values (visitor.test_imports and visitor.lib_imports);
either change the return type annotations to frozenset[str] or explicitly
convert the returned frozenset to a set() before returning. Update the
signatures of parse_test_imports and parse_lib_imports to return frozenset[str]
(or wrap visitor.test_imports/visitor.lib_imports with set(...) if you prefer a
mutable set) so the type annotations match the actual return values from
ImportVisitor.
67-74: Unnecessary try/except block: ast.ImportFrom.module never raises AttributeError.
The module attribute always exists on ast.ImportFrom nodes—it's None for relative imports like from . import x, not missing. The except AttributeError branch is unreachable. The subsequent if module is None check at line 73 correctly handles this case.
♻️ Simplified version
def visit_ImportFrom(self, node): - try: - module = node.module - except AttributeError: - # Ignore `from . import my_internal_module` - return - - if module is None: # Ignore `from . import my_internal_module` + module = node.module + if module is None: # Relative import: `from . import my_internal_module` return
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/update_lib/deps.py` around lines 67 - 74, Remove the unreachable
try/except around accessing node.module: replace the try block that sets "module
= node.module" and the except AttributeError branch with a direct assignment
(module = node.module) and keep the existing None check (if module is None:
return); remove the except AttributeError branch and any comment about ignoring
AttributeError so the code simply reads module = node.module followed by the
None-check.