Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/update_lib/cmd_todo.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def compute_test_todo_list(
test_order = lib_test_order[lib_name].index(test_name)
else:
# Extract lib name from test name (test_foo -> foo)
lib_name = test_name.removeprefix("test_")
lib_name = test_name.removeprefix("test_").removeprefix("_test")
test_order = 0 # Default order for tests not in DEPENDENCIES

# Check if corresponding lib is up-to-date
Expand Down
138 changes: 92 additions & 46 deletions scripts/update_lib/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import difflib
import functools
import pathlib
import re
import shelve
import subprocess

Expand All @@ -32,62 +31,112 @@
# === Import parsing utilities ===


def _extract_top_level_code(content: str) -> str:
"""Extract only top-level code from Python content for faster parsing."""
def_idx = content.find("\ndef ")
class_idx = content.find("\nclass ")
class ImportVisitor(ast.NodeVisitor):
def __init__(self) -> None:
self.__imports = set()

indices = [i for i in (def_idx, class_idx) if i != -1]
if indices:
content = content[: min(indices)]
return content.rstrip("\n")
@property
def test_imports(self) -> frozenset[str]:
imports = set()
for module in self.__imports:
if not module.startswith("test."):
continue
name = module.removeprefix("test.")

if name == "support" or name.startswith("support."):
continue

_FROM_TEST_IMPORT_RE = re.compile(r"^from test import (.+)", re.MULTILINE)
_FROM_TEST_DOT_RE = re.compile(r"^from test\.(\w+)", re.MULTILINE)
_IMPORT_TEST_DOT_RE = re.compile(r"^import test\.(\w+)", re.MULTILINE)
imports.add(name)

return frozenset(imports)

def parse_test_imports(content: str) -> set[str]:
"""Parse test file content and extract test package dependencies."""
content = _extract_top_level_code(content)
imports = set()
@property
def lib_imports(self) -> frozenset[str]:
return frozenset(
# module.split(".", 1)[0]
module
for module in self.__imports
if not module.startswith("test.")
)

for match in _FROM_TEST_IMPORT_RE.finditer(content):
import_list = match.group(1)
for part in import_list.split(","):
name = part.split()[0].strip()
if name and name not in ("support", "__init__"):
imports.add(name)
def visit_Import(self, node):
for alias in node.names:
self.__imports.add(alias.name)

for match in _FROM_TEST_DOT_RE.finditer(content):
dep = match.group(1)
if dep not in ("support", "__init__"):
imports.add(dep)
def visit_ImportFrom(self, node):
try:
module = node.module
except AttributeError:
# Ignore `from . import my_internal_module`
return

for match in _IMPORT_TEST_DOT_RE.finditer(content):
dep = match.group(1)
if dep not in ("support", "__init__"):
imports.add(dep)
if module is None: # Ignore `from . import my_internal_module`
return

return imports
for alias in node.names:
# We only care about what we import if it was from the "test" module
if module == "test":
name = f"{module}.{alias.name}"
else:
name = module

self.__imports.add(name)

_IMPORT_RE = re.compile(r"^import\s+(\w[\w.]*)", re.MULTILINE)
_FROM_IMPORT_RE = re.compile(r"^from\s+(\w[\w.]*)\s+import", re.MULTILINE)
def visit_Call(self, node) -> None:
"""
In test files, there's sometimes use of:

```python
import test.support
from test.support import script_helper

def parse_lib_imports(content: str) -> set[str]:
"""Parse library file and extract all imported module names."""
imports = set()
script = support.findfile("_test_atexit.py")
script_helper.run_test_script(script)
```

This imports "_test_atexit.py" but does not show as an import node.
"""
func = node.func
if not isinstance(func, ast.Attribute):
return

value = func.value
if not isinstance(value, ast.Name):
return

if (value.id != "support") or (func.attr != "findfile"):
return

arg = node.args[0]
if not isinstance(arg, ast.Constant):
return
Comment on lines +110 to +112
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing bounds check for node.args could raise IndexError.

If a malformed call like support.findfile() (no arguments) exists, accessing node.args[0] will raise an IndexError, potentially crashing the dependency scan for that file.

🛡️ Add 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 an argument exists, which can raise IndexError for
calls like support.findfile(); add a defensive bounds check before accessing
node.args[0] (e.g. if not node.args: return or if len(node.args) == 0: return)
so the existing isinstance(arg, ast.Constant) check remains safe; update the
block that currently reads "arg = node.args[0]" to first verify node.args is
non-empty.


for match in _IMPORT_RE.finditer(content):
imports.add(match.group(1))
target = arg.value
if not target.endswith(".py"):
return

for match in _FROM_IMPORT_RE.finditer(content):
imports.add(match.group(1))
target = target.removesuffix(".py")
self.__imports.add(f"test.{target}")

return imports

def parse_test_imports(content: str) -> set[str]:
"""Parse test file content and extract test package dependencies."""
if not (tree := safe_parse_ast(content)):
return set()

visitor = ImportVisitor()
visitor.visit(tree)
return visitor.test_imports


def parse_lib_imports(content: str) -> set[str]:
"""Parse library file and extract all imported module names."""
if not (tree := safe_parse_ast(content)):
return set()

visitor = ImportVisitor()
visitor.visit(tree)
return visitor.lib_imports
Comment on lines +122 to +139
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Return type annotations don't match actual return types.

Both functions declare -> set[str] but return frozenset[str] from the visitor properties. This mismatch could cause type checker warnings and is misleading to callers.

🔧 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 declare -> set[str] but actually return
visitor.test_imports and visitor.lib_imports (which are frozenset[str]); update
the return type annotations to -> frozenset[str] for both functions to match the
actual return values (or alternatively convert the frozensets to set(...) if you
prefer a mutable collection), and ensure the references to visitor.test_imports
and visitor.lib_imports remain unchanged.



# === TODO marker utilities ===
Expand All @@ -104,7 +153,7 @@ def filter_rustpython_todo(content: str) -> str:

def count_rustpython_todo(content: str) -> int:
"""Count lines containing RustPython TODO markers."""
return sum(1 for line in content.splitlines() if TODO_MARKER in line)
return content.count(TODO_MARKER)


def count_todo_in_path(path: pathlib.Path) -> int:
Expand All @@ -113,10 +162,7 @@ def count_todo_in_path(path: pathlib.Path) -> int:
content = safe_read_text(path)
return count_rustpython_todo(content) if content else 0

total = 0
for _, content in read_python_files(path):
total += count_rustpython_todo(content)
return total
return sum(count_rustpython_todo(content) for _, content in read_python_files(path))


# === Test utilities ===
Expand Down