◐ Shell
clean mode source ↗

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.