Reapply "ast.NodeVisitor for import tracking" (#7241)

ShaharNaveh · web-flow · commit e0eb6d5e8e87 · 2026-02-28T10:37:00.000+09:00
* Reapply "`ast.NodeVisitor` for import tracking (#7229)" (#7230) This reverts commit a47572c.
diff --git a/scripts/update_lib/cmd_todo.py b/scripts/update_lib/cmd_todo.py
@@ -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
diff --git a/scripts/update_lib/deps.py b/scripts/update_lib/deps.py
@@ -11,7 +11,6 @@
 import difflib
 import functools
 import pathlib
-import re
 import shelve
 import subprocess
 
@@ -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
 
-    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
 
 
 # === TODO marker utilities ===
@@ -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:
@@ -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 ===
