From baa9472a674ec50f8cbd11c2aa6d97790ff90e76 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 01/24] test(modules): cover the names and imports that must survive #264 Pin what naming modules after their packages must keep: a module in packages is named after them, one in no package from its own directory, and no directory above the project root names a module; a script reaches the module beside it by its short name, a module beside it named like a library leaves the library modelled, and each importer reaches the helper of its own directory. --- tests/test_namespace_module_imports.py | 91 ++++++++++++++++++++++++++ tests/test_namespace_module_names.py | 55 ++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 tests/test_namespace_module_imports.py create mode 100644 tests/test_namespace_module_names.py diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py new file mode 100644 index 0000000..5eeb52c --- /dev/null +++ b/tests/test_namespace_module_imports.py @@ -0,0 +1,91 @@ +"""Acceptance tests for issue #264: imports of and from modules in directories without +``__init__.py`` inside a package. + +Such a module has one identity, its name after its package, and may be imported by two +names: that one, and, as a script run from its directory imports it (``sys.path[0]``), +the name it has from there. Its relative imports resolve against its package; an +absolute import from it prefers what its own directory holds, as a script finds it, +then what the package's import root holds. Of two files with one such name, each +importer reaches the one in its own directory. Whatever the names a module is reached +by, its summaries are computed once, and the result does not depend on the cache or on +the number of processes. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python import engine + +PLUGINS = [engine.BUNDLED_PLUGINS] + + +def project(tmp_path: Path, files: dict[str, str], name: str = "project") -> Path: + root = tmp_path / name + for relative, text in files.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return root + + +def located(analysis: engine.ProjectAnalysis, root: Path, *rules: str) -> list[tuple[str, str, int]]: + return sorted( + (f.rule_id, Path(str(f.span.source_id)).relative_to(root.resolve()).as_posix(), f.span.start_line) + for f in analysis.findings + if not rules or f.rule_id in rules + ) + + +SCRIPTS = { + "pkg/__init__.py": "", + "pkg/tools/run.py": 'import helper\n\n\ndef main():\n helper.execute(input())\n\n\nif __name__ == "__main__":\n main()\n', + "pkg/tools/helper.py": "import os\n\n\ndef execute(command):\n os.system(command)\n", +} + + +def test_a_script_reaches_the_module_beside_it_by_its_short_name(tmp_path: Path) -> None: + root = project(tmp_path, SCRIPTS) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root) == [("command-injection", "pkg/tools/run.py", 5)] + + +def test_a_module_beside_a_script_named_like_a_library_leaves_the_library_modelled(tmp_path: Path) -> None: + # The script's os is the library: the os.py beside it defines no system. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/tools/run.py": "import os\n\n\ndef main():\n os.system(input())\n", + "pkg/tools/os.py": "def unrelated():\n return 1\n", + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root) == [("command-injection", "pkg/tools/run.py", 5)] + + +@pytest.mark.parametrize("dangerous", ["a", "b"]) +def test_each_importer_reaches_the_helper_of_its_own_directory(tmp_path: Path, dangerous: str) -> None: + run = "import helper\n\n\ndef main():\n helper.execute(input())\n" + sink = "import os\n\n\ndef execute(command):\n os.system(command)\n" + harmless = "def execute(command):\n return command\n" + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/a/run.py": run, + "pkg/a/helper.py": sink if dangerous == "a" else harmless, + "pkg/b/run.py": run, + "pkg/b/helper.py": sink if dangerous == "b" else harmless, + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root, "command-injection") == [("command-injection", f"pkg/{dangerous}/run.py", 5)] diff --git a/tests/test_namespace_module_names.py b/tests/test_namespace_module_names.py new file mode 100644 index 0000000..b8a48e7 --- /dev/null +++ b/tests/test_namespace_module_names.py @@ -0,0 +1,55 @@ +"""Acceptance tests for issue #264: the name of a module follows the packages it is in. + +Python imports a directory without ``__init__.py`` inside a regular package as a portion +of that package (PEP 420): ``pkg/sub/app.py`` is ``pkg.sub.app`` once the directory above +``pkg`` is on the import path, and so is ``pkg/a/b/deep.py`` through two such directories. +A project's module is named that way: from the parent of the outermost directory holding +an ``__init__.py``, searched up through directories without one as far as the project +root. A module in no package is named from its own directory, as before, and a package +climbs above the root only while each directory holds an ``__init__.py``. +""" + +from __future__ import annotations + +from pathlib import Path + +from coretrace_python import engine + + +def project(tmp_path: Path, files: dict[str, str]) -> Path: + root = tmp_path / "project" + for relative, text in files.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return root + + +def module_names(root: Path) -> dict[str, str]: + """Each analysed file of the project, by its path from the root, with its module name.""" + + graph = engine.analyze_project(root, [engine.BUNDLED_PLUGINS]).graph + return { + Path(str(graph.source(name).source_id)).relative_to(root.resolve()).as_posix(): name + for name in graph.modules + } + + +def test_a_module_in_packages_is_named_after_them(tmp_path: Path) -> None: + root = project(tmp_path, {"pkg/__init__.py": "", "pkg/sub/__init__.py": "", "pkg/sub/mod.py": ""}) + + assert module_names(root) == {"pkg/__init__.py": "pkg", "pkg/sub/__init__.py": "pkg.sub", "pkg/sub/mod.py": "pkg.sub.mod"} + + +def test_a_module_in_no_package_is_named_from_its_own_directory(tmp_path: Path) -> None: + root = project(tmp_path, {"scripts/tool.py": "", "setup.py": ""}) + + assert module_names(root) == {"scripts/tool.py": "tool", "setup.py": "setup"} + + +def test_no_directory_above_the_project_root_names_a_module(tmp_path: Path) -> None: + # Outside the project, an __init__.py names nothing in it: ``app`` stays ``app``. + (tmp_path / "__init__.py").write_text("", encoding="utf-8") + root = project(tmp_path, {"tools/app.py": ""}) + + assert module_names(root) == {"tools/app.py": "app"} From 8541ff6fe1d938765cee33323335e6dffa0b2c95 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 02/24] refactor(source): name a file's module and its import root in one place The module name of a file and the directory an import of it starts from were both found by walking up the packages above it, in two places. A file loaded from disk now carries the names it can be imported by, each with that directory, and the module graph reads its import root from them. Behaviour is unchanged. --- .../interprocedural/modulegraph.py | 14 ++++------- src/coretrace_python/source/__init__.py | 10 +++++++- src/coretrace_python/source/manager.py | 23 +++++++++++++------ src/coretrace_python/source/model.py | 13 ++++++++++- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/coretrace_python/interprocedural/modulegraph.py b/src/coretrace_python/interprocedural/modulegraph.py index b540563..1cdadb6 100644 --- a/src/coretrace_python/interprocedural/modulegraph.py +++ b/src/coretrace_python/interprocedural/modulegraph.py @@ -121,16 +121,10 @@ def _dotted(path: Path) -> str: def _import_root(source: SourceFile) -> Path | None: - """The directory an import of ``source`` by its module name starts from: the parent - of its top-level package, or its own directory for a module outside any package, - where ``module_name_for`` stopped. ``None`` for a source not on disk.""" - - if source.path is None: - return None - directory = source.path.parent - while (directory / "__init__.py").is_file(): - directory = directory.parent - return directory + """The directory an import of ``source`` by its module name starts from + (``SourceFile.import_names``); ``None`` for a source not on disk.""" + + return source.import_names[0].root if source.import_names else None def project_symbol(module_name: str, qualified_name: str) -> SymbolId: diff --git a/src/coretrace_python/source/__init__.py b/src/coretrace_python/source/__init__.py index c2a75a2..aa2756b 100644 --- a/src/coretrace_python/source/__init__.py +++ b/src/coretrace_python/source/__init__.py @@ -1,11 +1,19 @@ """Source storage and location primitives.""" from coretrace_python.source.manager import LINE_BREAK, SourceManager, decode_text, lines_of -from coretrace_python.source.model import FileLocation, Location, SourceFile, SourceId, SourceSpan +from coretrace_python.source.model import ( + FileLocation, + ImportName, + Location, + SourceFile, + SourceId, + SourceSpan, +) __all__ = [ "LINE_BREAK", "FileLocation", + "ImportName", "Location", "SourceFile", "SourceId", diff --git a/src/coretrace_python/source/manager.py b/src/coretrace_python/source/manager.py index 4eba81f..c00a2b2 100644 --- a/src/coretrace_python/source/manager.py +++ b/src/coretrace_python/source/manager.py @@ -5,20 +5,27 @@ import re from pathlib import Path -from coretrace_python.source.model import SourceFile, SourceId +from coretrace_python.source.model import ImportName, SourceFile, SourceId -def module_name_for(path: Path) -> str: - """Dotted module name of ``path``, walking up through ``__init__.py`` packages. - - A package's ``__init__.py`` is named after the package itself.""" +def import_names_for(path: Path) -> tuple[ImportName, ...]: + """The names the file ``path`` can be imported by, its module name first, each with + the directory an import of it starts from: walking up through ``__init__.py`` + packages, named from the first directory without one. A package's ``__init__.py`` + is named after the package itself.""" parts = [] if path.name == "__init__.py" else [path.stem] directory = path.parent while (directory / "__init__.py").is_file(): parts.append(directory.name) directory = directory.parent - return ".".join(reversed(parts)) or path.stem + return (ImportName(".".join(reversed(parts)) or path.stem, directory),) + + +def module_name_for(path: Path) -> str: + """Dotted module name of ``path`` (``import_names_for``).""" + + return import_names_for(path)[0].name def decode_text(data: bytes) -> str: @@ -76,12 +83,14 @@ def load_file(self, path: str | Path) -> SourceFile: return existing text = decode_text(resolved_path.read_bytes()) + names = import_names_for(resolved_path) source = SourceFile( source_id=source_id, text=text, - module_name=module_name_for(resolved_path), + module_name=names[0].name, path=resolved_path, is_package=resolved_path.name == "__init__.py", + import_names=names, ) self._sources[source_id] = source return source diff --git a/src/coretrace_python/source/model.py b/src/coretrace_python/source/model.py index da39927..9d9c49b 100644 --- a/src/coretrace_python/source/model.py +++ b/src/coretrace_python/source/model.py @@ -65,13 +65,24 @@ def display(self) -> str: Location = SourceSpan | FileLocation +@dataclass(frozen=True) +class ImportName: + """A name a module can be imported by, with the directory an import of that name + starts from: the entry of the import path under which the module has that name.""" + + name: str + root: Path + + @dataclass(frozen=True) class SourceFile: - """Decoded source text, its identity and its dotted module name.""" + """Decoded source text, its identity and its dotted module name; for a file on disk, + the names it can be imported by (``import_names``), its module name first.""" source_id: SourceId text: str module_name: str path: Path | None = None is_package: bool = False + import_names: tuple[ImportName, ...] = () From 224d1dd6a6e6e4aa73829dcd0257405311b9ffa4 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 03/24] refactor(modulegraph): resolve an import by any name of a module, from the importer's roots in order A module may be imported by more than one name, each from its own directory. The graph indexes every name a module has, and resolves a name several files share from the first of the importer's roots that holds exactly one, its last name's root first, as a script finds what lies beside it before what its package's root holds. It also records which module each import name of an importer reached. With one name per module, behaviour is unchanged. --- .../interprocedural/modulegraph.py | 63 +++++++++++++------ 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/src/coretrace_python/interprocedural/modulegraph.py b/src/coretrace_python/interprocedural/modulegraph.py index 1cdadb6..bd6fc5f 100644 --- a/src/coretrace_python/interprocedural/modulegraph.py +++ b/src/coretrace_python/interprocedural/modulegraph.py @@ -120,11 +120,12 @@ def _dotted(path: Path) -> str: return ".".join(parts) or path.as_posix() -def _import_root(source: SourceFile) -> Path | None: - """The directory an import of ``source`` by its module name starts from - (``SourceFile.import_names``); ``None`` for a source not on disk.""" +def import_names(source: SourceFile) -> tuple[tuple[str, Path | None], ...]: + """The names ``source`` can be imported by, its module name first, each with the + directory an import of it starts from (``SourceFile.import_names``); for a source + not on disk, its module name from nowhere in particular.""" - return source.import_names[0].root if source.import_names else None + return tuple((n.name, n.root) for n in source.import_names) or ((source.module_name, None),) def project_symbol(module_name: str, qualified_name: str) -> SymbolId: @@ -151,6 +152,7 @@ class ModuleGraph: _sources: Mapping[str, SourceFile] _imports: Mapping[str, frozenset[str]] _unresolved: Mapping[str, frozenset[str]] = field(default_factory=dict) + _resolved: Mapping[str, Mapping[str, str]] = field(default_factory=dict) @property def modules(self) -> tuple[str, ...]: @@ -171,6 +173,12 @@ def unresolved(self, name: str) -> frozenset[str]: return self._unresolved.get(name, frozenset()) + def resolved(self, name: str) -> Mapping[str, str]: + """The project modules ``name`` imports, each by the import name its code gives + it, which may be another than the module's name in the graph.""" + + return self._resolved.get(name, MappingProxyType({})) + def schedule(self) -> tuple[tuple[frozenset[str], ...], ...]: """Strongly connected components in waves: every component of a wave imports, outside itself, only components of earlier waves, so one wave can be analysed @@ -235,16 +243,20 @@ def build_module_graph( imports: Mapping[str, ImportTable], ) -> ModuleGraph: """Edges from each module to the project modules it imports, in any scope. An import - names a module by its import name; when several files have that name, it resolves to - the one in the importer's own root and, without exactly one there, stays unresolved: - no edge, and the name is recorded on the graph.""" + names a module by one of its import names; when several files have that name, it + resolves to the one from the first of the importer's own roots that has exactly one, + and otherwise stays unresolved: no edge, and the name is recorded on the graph. The + importer's roots are those of its import names, the last first: as a script run from + its directory finds what lies beside it before what its package's root holds.""" - by_name: dict[str, list[str]] = {} + by_name: dict[str, list[tuple[str, Path | None]]] = {} for name, source in sources.items(): - by_name.setdefault(source.module_name, []).append(name) - roots = {name: _import_root(source) for name, source in sources.items()} + for import_name, root in import_names(source): + by_name.setdefault(import_name, []).append((name, root)) + roots = {name: tuple(root for _, root in reversed(import_names(source))) for name, source in sources.items()} edges: dict[str, frozenset[str]] = {} unresolved: dict[str, frozenset[str]] = {} + resolved: dict[str, Mapping[str, str]] = {} for name, module in modules.items(): candidates: set[str] = set() dotted_tops: set[str] = set() @@ -261,24 +273,39 @@ def build_module_graph( candidates.add(path) # The most specific project module each import names; ``app.helpers`` implies # the ``app`` package, which is not an edge worth recording. - found: set[str] = set() + found: dict[str, str] = {} missing: set[str] = set() for candidate in candidates: for prefix in _prefixes(candidate): matching = by_name.get(prefix) if matching is None: continue - if len(matching) > 1: - matching = [m for m in matching if roots[m] == roots[name]] - if len(matching) == 1: - found.add(matching[0]) - else: + chosen = _chosen(matching, roots[name]) + if chosen is None: missing.add(prefix) + elif chosen != name: + found[prefix] = chosen break - edges[name] = frozenset(found - {name}) + edges[name] = frozenset(found.values()) + resolved[name] = MappingProxyType(found) if missing: unresolved[name] = frozenset(missing) - return ModuleGraph(MappingProxyType(dict(sources)), MappingProxyType(edges), MappingProxyType(unresolved)) + return ModuleGraph( + MappingProxyType(dict(sources)), MappingProxyType(edges), MappingProxyType(unresolved), MappingProxyType(resolved) + ) + + +def _chosen(matching: list[tuple[str, Path | None]], roots: tuple[Path | None, ...]) -> str | None: + """The module an import name names among the files that have it, from the first of + the importer's ``roots`` holding exactly one; ``None`` when none does.""" + + if len({module for module, _ in matching}) == 1: + return matching[0][0] + for root in roots: + there = {module for module, named_from in matching if named_from == root} + if there: + return there.pop() if len(there) == 1 else None + return None def _prefixes(dotted: str) -> list[str]: From a67e0664c475231b7cca039206154d1e2a8560a1 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 04/24] refactor(engine): register a module's summaries under every name it is reached by A component's members refer to the modules they import by the names they resolved them by, which may differ from a module's name in the graph. Their summaries go under each of those names, a member's own resolution first, and a member registers its own functions under each of its import names that no other member shares; worker processes receive the names with each module. A module is still analysed once, whatever the number of names. With one name per module, behaviour is unchanged. --- src/coretrace_python/engine.py | 102 ++++++++++++------ .../interprocedural/__init__.py | 2 + .../interprocedural/modulegraph.py | 1 + 3 files changed, 72 insertions(+), 33 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 911606d..6fc6584 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -95,6 +95,7 @@ TemplateCallsAnalysis, build_module_graph, discover_sources, + import_names, name_modules, project_symbol, starting_summaries, @@ -596,7 +597,13 @@ def analyze_project( batch = {name: analysable[name] for name in sorted(component)} computed.update( _analyse_managers( - batch, _seed(results, graph, component), module_plugins, affected, suffixes, root + batch, + _seed(results, graph, component), + module_plugins, + affected, + suffixes, + root, + {name: _names_of(files[name]) for name in batch}, ) ) for manager in batch.values(): @@ -608,8 +615,8 @@ def analyze_project( _Batch( {name: _path_of(files[name]) for name in sorted(component)}, { - name: (import_name, keys[name]) - for name, import_name in _imported_names(graph, component, results).items() + name: (names, keys[name]) + for name, names in _imported_names(graph, component, results).items() }, ), ) @@ -628,7 +635,7 @@ def analyze_project( if spooled is not None: spooled.cleanup() - index = _index(results, {name: name for name in results}) + index = _index(results, {name: (name,) for name in results}) call_graphs: dict[str, CallGraph] = {} functions: dict[str, tuple[ModuleFunction, ...]] = {} for name in sorted(analysable): @@ -823,13 +830,14 @@ def _seed(results: Mapping[str, CachedModule], graph: ModuleGraph, component: fr def _imported_names( graph: ModuleGraph, component: frozenset[str], results: Mapping[str, CachedModule] -) -> dict[str, str]: - """The modules whose summaries a component starts from, each with the import name the - component's code refers to it by: every module it imports, transitively, all final - by the time its wave runs. Of several files with one import name, only the one a - member imports directly, resolved in its own root, goes under that name: the others - are reached through summaries that inline them already, and a name no member's root - resolved is followed into neither file.""" +) -> dict[str, tuple[str, ...]]: + """The modules whose summaries a component starts from, each with the import names + the component's code refers to it by: every module it imports, transitively, all + final by the time its wave runs. A member's own import goes under the name it + resolved by, unless another member resolved that name to another file. Of several + files with one import name, only the one a member imports directly goes under that + name: the others are reached through summaries that inline them already, and a name + no member's root resolved is followed into neither file.""" wanted: set[str] = set() pending = list(component) @@ -838,31 +846,48 @@ def _imported_names( if imported in results and imported not in wanted: wanted.add(imported) pending.append(imported) + chosen: dict[str, str | None] = {} + for member in sorted(component): + for import_name, module in graph.resolved(member).items(): + if module in results and chosen.setdefault(import_name, module) != module: + chosen[import_name] = None direct = {imported for member in component for imported in graph.imports(member)} by_import: dict[str, list[str]] = {} for name in wanted: - by_import.setdefault(graph.source(name).module_name, []).append(name) - names: dict[str, str] = {} + for import_name, _ in import_names(graph.source(name)): + by_import.setdefault(import_name, []).append(name) for import_name, modules in by_import.items(): - chosen = modules if len(modules) == 1 else [name for name in modules if name in direct] - if len(chosen) == 1: - names[chosen[0]] = import_name - return names + picked = modules if len(modules) == 1 else [name for name in modules if name in direct] + if import_name not in chosen and len(picked) == 1: + chosen[import_name] = picked[0] + names: dict[str, list[str]] = {} + for import_name, reached in sorted(chosen.items()): + if reached is not None: + names.setdefault(reached, []).append(import_name) + return {module: tuple(module_names) for module, module_names in names.items()} -def _index(results: Mapping[str, CachedModule], names: Mapping[str, str]) -> SummaryIndex: +def _index(results: Mapping[str, CachedModule], names: Mapping[str, Iterable[str]]) -> SummaryIndex: """The summaries of the modules in ``names``, each function under the project symbol - of the module name given for its module.""" + of every module name given for its module: one module, reached by several names, + has its summaries computed once.""" return SummaryIndex( { project_symbol(module, function): summary - for name, module in sorted(names.items()) + for name, module_names in sorted(names.items()) + for module in module_names for function, summary in results[name].summaries.items() } ) +def _names_of(source: SourceFile) -> tuple[str, ...]: + """The import names of ``source`` (``import_names``), without their roots.""" + + return tuple(name for name, _ in import_names(source)) + + def _analyse_managers( managers: Mapping[str, AnalysisManager], seed: SummaryIndex, @@ -870,14 +895,15 @@ def _analyse_managers( affected: Affected, suffixes: SuffixIndex, root: Path, + member_names: Mapping[str, tuple[str, ...]], ) -> dict[str, CachedModule]: """Analyse one component: iterate its summaries to a fixpoint over ``seed`` (§21), then extract what the rest of the run needs from each module. Each member registers - its functions under its import name, what the others' code refers to it by; a name - two members share, a cycle across roots, is registered by neither.""" + its functions under its import names (``member_names``), what the others' code refers + to it by; a name two members share, a cycle across roots, is registered by neither.""" - counts = Counter(manager.module.name for manager in managers.values()) - names = {name: manager.module.name for name, manager in managers.items() if counts[manager.module.name] == 1} + counts = Counter(module for name in managers for module in member_names[name]) + names = {name: tuple(module for module in member_names[name] if counts[module] == 1) for name in managers} # The members' functions start from the summaries ``SummaryAnalysis`` starts from. # Left out, a call from one member to another is a call to an unknown symbol on the # first iteration and a project call on the next: around a cycle of modules, the @@ -887,7 +913,8 @@ def _analyse_managers( **seed.summaries, **{ project_symbol(module, function): summary - for name, module in names.items() + for name, modules in names.items() + for module in modules for function, summary in starting_summaries(managers[name].get(CallGraphAnalysis)).items() }, } @@ -910,7 +937,8 @@ def _analyse_managers( **seed.summaries, **{ project_symbol(module, function): summary - for name, module in names.items() + for name, modules in names.items() + for module in modules for function, summary in _summaries_of(managers[name]).items() }, } @@ -947,11 +975,11 @@ class _Project: @dataclass(frozen=True) class _Batch: """One component handed to a worker process: its files under the engine's module - names, and the modules its summaries start from, each with its import name and the + names, and the modules its summaries start from, each with its import names and the key of its entry in the spool. Only these travel, not the summaries themselves.""" paths: Mapping[str, Path] - imported: Mapping[str, tuple[str, str]] + imported: Mapping[str, tuple[tuple[str, ...], str]] @dataclass @@ -967,7 +995,7 @@ class _Worker: root: Path read: dict[str, CachedModule] = field(default_factory=dict) - def entries(self, imported: Mapping[str, tuple[str, str]]) -> dict[str, CachedModule]: + def entries(self, imported: Mapping[str, tuple[tuple[str, ...], str]]) -> dict[str, CachedModule]: found: dict[str, CachedModule] = {} for name, (_, key) in imported.items(): entry = self.read.get(key) @@ -1033,12 +1061,20 @@ def managed(manager: AnalysisManager) -> None: def _analyse_batch(batch: _Batch) -> dict[str, dict[str, Any]]: worker = _WORKER assert worker is not None, "the pool starts every worker with _start_worker" - sources = SourceManager() - managers = {name: _register_all(build_hir(sources.load_file(path))) for name, path in batch.paths.items()} + sources = {name: SourceManager().load_file(path) for name, path in batch.paths.items()} + managers = {name: _register_all(build_hir(source)) for name, source in sources.items()} for manager in managers.values(): worker.managed(manager) - seed = _index(worker.entries(batch.imported), {name: import_name for name, (import_name, _) in batch.imported.items()}) - results = _analyse_managers(managers, seed, worker.plugins, worker.affected, worker.suffixes, worker.root) + seed = _index(worker.entries(batch.imported), {name: names for name, (names, _) in batch.imported.items()}) + results = _analyse_managers( + managers, + seed, + worker.plugins, + worker.affected, + worker.suffixes, + worker.root, + {name: _names_of(source) for name, source in sources.items()}, + ) return {name: encode(entry) for name, entry in results.items()} diff --git a/src/coretrace_python/interprocedural/__init__.py b/src/coretrace_python/interprocedural/__init__.py index e27a800..e695be4 100644 --- a/src/coretrace_python/interprocedural/__init__.py +++ b/src/coretrace_python/interprocedural/__init__.py @@ -20,6 +20,7 @@ build_module_graph, discover_files, discover_sources, + import_names, name_modules, project_symbol, ) @@ -77,6 +78,7 @@ "cleared_by", "discover_files", "discover_sources", + "import_names", "name_modules", "project_symbol", "starting_summaries", diff --git a/src/coretrace_python/interprocedural/modulegraph.py b/src/coretrace_python/interprocedural/modulegraph.py index bd6fc5f..a20500c 100644 --- a/src/coretrace_python/interprocedural/modulegraph.py +++ b/src/coretrace_python/interprocedural/modulegraph.py @@ -331,6 +331,7 @@ def _statements(body: Iterable[nodes.Statement]) -> Iterable[nodes.Statement]: "SummaryIndex", "build_module_graph", "discover_sources", + "import_names", "name_modules", "project_symbol", ] From 25051d920bb391b78bdf9fb8f34660ec7a9848fb Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 05/24] fix(cache): key a module by the names it is imported by Under another name, what the modules importing a module reach, and the symbols of its own functions, change, so a module's names are part of its key. CACHE_FORMAT goes to 19: caches built before miss once. --- src/coretrace_python/cache.py | 5 +++-- src/coretrace_python/engine.py | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/coretrace_python/cache.py b/src/coretrace_python/cache.py index 879daaf..6b73db3 100644 --- a/src/coretrace_python/cache.py +++ b/src/coretrace_python/cache.py @@ -44,8 +44,9 @@ # Cached summaries from earlier versions preserve unstable external-call # insertion order. They must be recomputed with canonical ordering (#261). -# Findings carry their related locations from format 18 on (#271). -CACHE_FORMAT = 18 +# Findings carry their related locations from format 18 on (#271). A module's key holds +# the names it is imported by from format 19 on (#264). +CACHE_FORMAT = 19 @dataclass(frozen=True) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 6fc6584..0e69eae 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -548,9 +548,14 @@ def analyze_project( configuration = _configuration_key( components, plugins, models, advisories, dependencies, routes, escaped, template_calls ) + # A module's names are part of its key: under another name, what the modules importing + # it reach, and the symbols of its own functions, change. keys = module_keys( graph, - {name: fingerprint(configuration, str(files[name].source_id), name, files[name].text) for name in analysable}, + { + name: fingerprint(configuration, str(files[name].source_id), name, *_names_of(files[name]), files[name].text) + for name in analysable + }, ) results: dict[str, CachedModule] = {} if cache is not None: From 6538946a5a963ff4061b384983942fb145a3172d Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 06/24] test(engine): cover the note on an import no root decides A script at the project root that imports a name two directories hold, neither beside it, is noted and not followed: pin the note before the names of those modules change. --- tests/test_namespace_module_imports.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py index 5eeb52c..86df906 100644 --- a/tests/test_namespace_module_imports.py +++ b/tests/test_namespace_module_imports.py @@ -89,3 +89,22 @@ def test_each_importer_reaches_the_helper_of_its_own_directory(tmp_path: Path, d analysis = engine.analyze_project(root, PLUGINS) assert located(analysis, root, "command-injection") == [("command-injection", f"pkg/{dangerous}/run.py", 5)] + + +def test_an_import_no_root_decides_is_noted_and_not_followed(tmp_path: Path) -> None: + # Run from the root, tool.py finds no helper beside it: neither file is the one. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/a/helper.py": "import os\n\n\ndef execute(command):\n os.system(command)\n", + "pkg/b/helper.py": "def execute(command):\n return command\n", + "tool.py": "import helper\n\n\ndef main():\n helper.execute(input())\n", + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + (note,) = [f for f in analysis.findings if Path(str(f.span.source_id)).name == "tool.py"] + assert note.rule_id == "ambiguous-module" + assert note.message == "'helper' imported here is pkg/a/helper.py or pkg/b/helper.py: calls into it are not followed" From 589471c2931ad8c9fe73e0019f4c4a7acb5aef41 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 07/24] fix(source): name a module in a directory without __init__.py after its package Python takes a directory without __init__.py inside a regular package for a portion of that package (PEP 420), but such a module was named from that directory: its relative imports were rejected as beyond the top-level package, and the file was not analysed (flask's sansio, pygoat's playground). A module of a project is now named from the parent of the outermost directory holding an __init__.py, searched up through directories without one as far as the project root. The name a script run from the module's directory gives it stays a second name: an import from such a script still reaches the module beside it first. Files sharing only that second name are told apart by the importer's root and no longer noted as ambiguous; an import no root decides is still noted. --- src/coretrace_python/engine.py | 12 +++- .../interprocedural/modulegraph.py | 2 +- src/coretrace_python/source/manager.py | 55 ++++++++++++++++--- tests/test_namespace_module_imports.py | 55 +++++++++++++++++++ tests/test_namespace_module_names.py | 35 ++++++++++++ 5 files changed, 146 insertions(+), 13 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 0e69eae..5ae105a 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -793,7 +793,9 @@ def _advisory_paths(root: Path, advisory_files: Sequence[Path]) -> tuple[Path, . def _shared_names(graph: ModuleGraph) -> dict[str, list[str]]: - """The import names several analysable files have, each with those files' modules.""" + """The module names several analysable files have, each with those files' modules. + A name a module also has from its own directory (``import_names``) is not among + them: the importer's root tells such files apart.""" by_name: dict[str, list[str]] = {} for name in graph.modules: @@ -809,6 +811,10 @@ def _ambiguity_notes(graph: ModuleGraph, shared: Mapping[str, list[str]], root: def located(name: str) -> str: return _located(_path_of(graph.source(name)), root) + having: dict[str, list[str]] = {} + for module in graph.modules: + for import_name, _ in import_names(graph.source(module)): + having.setdefault(import_name, []).append(module) notes: list[Finding] = [] for import_name, names in sorted(shared.items()): for name in names: @@ -821,7 +827,7 @@ def located(name: str) -> str: notes.append(_note("ambiguous-module", message, graph.source(name), 1)) for name in graph.modules: for import_name in sorted(graph.unresolved(name)): - candidates = " or ".join(located(other) for other in shared[import_name]) + candidates = " or ".join(located(other) for other in having[import_name]) message = f"'{import_name}' imported here is {candidates}: calls into it are not followed" notes.append(_note("ambiguous-module", message, graph.source(name), 1)) return notes @@ -1066,7 +1072,7 @@ def managed(manager: AnalysisManager) -> None: def _analyse_batch(batch: _Batch) -> dict[str, dict[str, Any]]: worker = _WORKER assert worker is not None, "the pool starts every worker with _start_worker" - sources = {name: SourceManager().load_file(path) for name, path in batch.paths.items()} + sources = {name: SourceManager().load_file(path, worker.root) for name, path in batch.paths.items()} managers = {name: _register_all(build_hir(source)) for name, source in sources.items()} for manager in managers.values(): worker.managed(manager) diff --git a/src/coretrace_python/interprocedural/modulegraph.py b/src/coretrace_python/interprocedural/modulegraph.py index a20500c..d27d97b 100644 --- a/src/coretrace_python/interprocedural/modulegraph.py +++ b/src/coretrace_python/interprocedural/modulegraph.py @@ -59,7 +59,7 @@ def discover_sources( found: list[SourceFile] = [] for path in discover_files(root, "*.py"): try: - found.append(manager.load_file(path)) + found.append(manager.load_file(path, root)) except (OSError, UnicodeDecodeError) as error: if errors is None: raise diff --git a/src/coretrace_python/source/manager.py b/src/coretrace_python/source/manager.py index c00a2b2..248c2f9 100644 --- a/src/coretrace_python/source/manager.py +++ b/src/coretrace_python/source/manager.py @@ -8,24 +8,57 @@ from coretrace_python.source.model import ImportName, SourceFile, SourceId -def import_names_for(path: Path) -> tuple[ImportName, ...]: +def import_names_for(path: Path, root: Path | None = None) -> tuple[ImportName, ...]: """The names the file ``path`` can be imported by, its module name first, each with - the directory an import of it starts from: walking up through ``__init__.py`` - packages, named from the first directory without one. A package's ``__init__.py`` - is named after the package itself.""" + the directory an import of it starts from. A package's ``__init__.py`` is named after + the package itself. + + The module name follows the packages the file is in, as Python takes a directory + without ``__init__.py`` inside a regular package for a portion of it (PEP 420): it + runs from the parent of the outermost directory holding an ``__init__.py``, searched + up through directories without one as far as ``root``, the project's, and above it + while each directory holds one (#264). In no package, or without a root, it runs from + the first directory without an ``__init__.py`` above the file. That second way is how + a script run from that directory imports the file (``sys.path[0]``): when it gives + another name, the file has that name too.""" parts = [] if path.name == "__init__.py" else [path.stem] directory = path.parent while (directory / "__init__.py").is_file(): parts.append(directory.name) directory = directory.parent - return (ImportName(".".join(reversed(parts)) or path.stem, directory),) + script = ImportName(".".join(reversed(parts)) or path.stem, directory) + top = None if root is None else _outermost_package(path.parent, root) + if top is None or top.parent == script.root: + return (script,) + named = path.relative_to(top.parent).with_suffix("").parts + return (ImportName(".".join(named[:-1] if named[-1] == "__init__" else named), top.parent), script) + + +def _outermost_package(directory: Path, root: Path) -> Path | None: + """The outermost directory holding an ``__init__.py`` among ``directory`` and those + above it up to ``root``, and above ``root`` while each holds one; ``None`` when none + does, or ``directory`` is not under ``root``.""" + + if directory != root and root not in directory.parents: + return None + top = None + while True: + if (directory / "__init__.py").is_file(): + top = directory + if directory == root: + break + directory = directory.parent + if top == root: + while (top.parent / "__init__.py").is_file(): + top = top.parent + return top -def module_name_for(path: Path) -> str: +def module_name_for(path: Path, root: Path | None = None) -> str: """Dotted module name of ``path`` (``import_names_for``).""" - return import_names_for(path)[0].name + return import_names_for(path, root)[0].name def decode_text(data: bytes) -> str: @@ -75,7 +108,11 @@ def add_source(self, name: str, text: str, module_name: str | None = None) -> So self._sources[source_id] = source return source - def load_file(self, path: str | Path) -> SourceFile: + def load_file(self, path: str | Path, root: Path | None = None) -> SourceFile: + """The file at ``path``, named as a module of the project under ``root`` + (``import_names_for``); without a root, from the first directory without an + ``__init__.py`` above it.""" + resolved_path = Path(path).resolve() source_id = SourceId(str(resolved_path)) existing = self._sources.get(source_id) @@ -83,7 +120,7 @@ def load_file(self, path: str | Path) -> SourceFile: return existing text = decode_text(resolved_path.read_bytes()) - names = import_names_for(resolved_path) + names = import_names_for(resolved_path, None if root is None else root.resolve()) source = SourceFile( source_id=source_id, text=text, diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py index 86df906..e72a20e 100644 --- a/tests/test_namespace_module_imports.py +++ b/tests/test_namespace_module_imports.py @@ -39,6 +39,61 @@ def located(analysis: engine.ProjectAnalysis, root: Path, *rules: str) -> list[t ) +NAMESPACE = { + "pkg/__init__.py": "", + "pkg/config.py": "import os\n\n\ndef run(command):\n os.system(command)\n", + "pkg/sub/app.py": "from ..config import run\nfrom .helper import read\n\n\ndef view():\n run(read())\n", + "pkg/sub/helper.py": "def read():\n return input()\n", + "pkg/a/b/deep.py": "from ...config import run\n\n\ndef view():\n run(input())\n", +} + + +def test_relative_imports_from_a_directory_without_init_resolve_against_its_package(tmp_path: Path) -> None: + root = project(tmp_path, NAMESPACE) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root) == [("command-injection", "pkg/a/b/deep.py", 5), ("command-injection", "pkg/sub/app.py", 6)] + + +def test_a_core_module_in_such_a_directory_is_followed_from_its_package(tmp_path: Path) -> None: + # flask 3.1.0: src/flask/sansio has no __init__.py, and flask/app.py imports from it. + root = project( + tmp_path, + { + "src/flask/__init__.py": "", + "src/flask/helpers.py": "import os\n\n\ndef spawn(command):\n os.system(command)\n", + "src/flask/sansio/scaffold.py": "from ..helpers import spawn\n\n\ndef launch(command):\n spawn(command)\n", + "src/flask/app.py": "from .sansio.scaffold import launch\n\n\ndef serve():\n launch(input())\n", + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root) == [("command-injection", "src/flask/app.py", 5)] + + +def test_a_sibling_imported_relatively_is_found_beside_a_module_of_the_same_short_name(tmp_path: Path) -> None: + # pygoat: introduction/playground/A9/api.py imports .main, and another main.py lies + # elsewhere, outside any package. + root = project( + tmp_path, + { + "introduction/__init__.py": "", + "introduction/playground/__init__.py": "", + "introduction/playground/A9/main.py": "import os\n\n\nclass Log:\n def write(self, line):\n os.system(line)\n", + "introduction/playground/A9/api.py": "from .main import Log\n\n\ndef view():\n Log().write(input())\n", + "labs/insec/main.py": "def unrelated():\n return 1\n", + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root, "command-injection", "syntax-error") == [ + ("command-injection", "introduction/playground/A9/api.py", 5) + ] + + SCRIPTS = { "pkg/__init__.py": "", "pkg/tools/run.py": 'import helper\n\n\ndef main():\n helper.execute(input())\n\n\nif __name__ == "__main__":\n main()\n', diff --git a/tests/test_namespace_module_names.py b/tests/test_namespace_module_names.py index b8a48e7..d5f737b 100644 --- a/tests/test_namespace_module_names.py +++ b/tests/test_namespace_module_names.py @@ -11,10 +11,14 @@ from __future__ import annotations +import subprocess +import sys from pathlib import Path from coretrace_python import engine +TIMEOUT = 60 + def project(tmp_path: Path, files: dict[str, str]) -> Path: root = tmp_path / "project" @@ -53,3 +57,34 @@ def test_no_directory_above_the_project_root_names_a_module(tmp_path: Path) -> N root = project(tmp_path, {"tools/app.py": ""}) assert module_names(root) == {"tools/app.py": "app"} + + +def test_a_directory_without_init_inside_a_package_is_a_portion_of_it(tmp_path: Path) -> None: + root = project(tmp_path, {"pkg/__init__.py": "", "pkg/sub/app.py": "", "pkg/a/b/deep.py": ""}) + + assert module_names(root) == {"pkg/__init__.py": "pkg", "pkg/sub/app.py": "pkg.sub.app", "pkg/a/b/deep.py": "pkg.a.b.deep"} + + +def test_python_imports_every_module_by_its_name(tmp_path: Path) -> None: + root = project( + tmp_path, + { + "src/flask/__init__.py": "", + "src/flask/helpers.py": "VALUE = 1\n", + "src/flask/sansio/app.py": "from ..helpers import VALUE\n", + "src/flask/sansio/deep/more.py": "from ...helpers import VALUE\n", + }, + ) + names = module_names(root) + check = "import importlib, sys\nsys.path.insert(0, sys.argv[1])\nfor name in sys.argv[2:]:\n importlib.import_module(name)\n" + + completed = subprocess.run( + [sys.executable, "-I", "-c", check, str(root / "src"), *names.values()], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + + assert completed.returncode == 0, completed.stderr + assert names["src/flask/sansio/deep/more.py"] == "flask.sansio.deep.more" From 261a1f6b3d63dcf431e670a72903865a1bce0720 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 08/24] test(modules): a module reached by both its names keeps one identity A module beside a script is imported by its short name from the script and by its package name from the rest of the package: it stays one module of the graph, covered once, and both importers reach its summaries. --- tests/test_namespace_module_imports.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py index e72a20e..2416152 100644 --- a/tests/test_namespace_module_imports.py +++ b/tests/test_namespace_module_imports.py @@ -146,6 +146,23 @@ def test_each_importer_reaches_the_helper_of_its_own_directory(tmp_path: Path, d assert located(analysis, root, "command-injection") == [("command-injection", f"pkg/{dangerous}/run.py", 5)] +def test_a_module_reached_by_both_its_names_keeps_one_identity(tmp_path: Path) -> None: + root = project( + tmp_path, + { + **SCRIPTS, + "pkg/other.py": "from .tools import helper\n\n\ndef handle():\n helper.execute(input())\n", + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + helpers = [n for n in analysis.graph.modules if str(analysis.graph.source(n).source_id).endswith("tools/helper.py")] + assert helpers == ["pkg.tools.helper"] + assert [d.path for d in analysis.coverage.details].count(str((root / "pkg/tools/helper.py").resolve())) == 1 + assert located(analysis, root) == [("command-injection", "pkg/other.py", 5), ("command-injection", "pkg/tools/run.py", 5)] + + def test_an_import_no_root_decides_is_noted_and_not_followed(tmp_path: Path) -> None: # Run from the root, tool.py finds no helper beside it: neither file is the one. root = project( From b267329490114fc12c96f20baf9b0e589b86fe36 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 09/24] test(modules): the result does not depend on the cache or the processes A project mixing relative imports from directories without __init__.py, scripts beside their helpers, a module reached by both its names and two helpers of one short name gives the same findings and coverage without a cache, with a cold and a warm one, and in two processes, with or without a cache. --- tests/test_namespace_module_imports.py | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py index 2416152..ef7ffdc 100644 --- a/tests/test_namespace_module_imports.py +++ b/tests/test_namespace_module_imports.py @@ -18,6 +18,7 @@ import pytest from coretrace_python import engine +from coretrace_python.cache import ProjectCache PLUGINS = [engine.BUNDLED_PLUGINS] @@ -163,6 +164,38 @@ def test_a_module_reached_by_both_its_names_keeps_one_identity(tmp_path: Path) - assert located(analysis, root) == [("command-injection", "pkg/other.py", 5), ("command-injection", "pkg/tools/run.py", 5)] +EVERYTHING = { + **NAMESPACE, + "pkg/tools/run.py": SCRIPTS["pkg/tools/run.py"], + "pkg/tools/helper.py": SCRIPTS["pkg/tools/helper.py"], + "pkg/other.py": "from .tools import helper\n\n\ndef handle():\n helper.execute(input())\n", + "pkg/b/run.py": "import helper\n\n\ndef main():\n helper.execute(input())\n", + "pkg/b/helper.py": "def execute(command):\n return command\n", +} + + +def test_the_result_does_not_depend_on_the_cache_or_the_processes(tmp_path: Path) -> None: + root = project(tmp_path, EVERYTHING) + + def outcome(**options: object) -> tuple[list[tuple[str, str, int]], list[tuple[str, str]]]: + analysis = engine.analyze_project(root, PLUGINS, **options) # type: ignore[arg-type] + return located(analysis, root), sorted((d.path, d.status) for d in analysis.coverage.details) + + expected = outcome() + cache = ProjectCache(tmp_path / "cache") + + assert expected[0] == [ + ("command-injection", "pkg/a/b/deep.py", 5), + ("command-injection", "pkg/other.py", 5), + ("command-injection", "pkg/sub/app.py", 6), + ("command-injection", "pkg/tools/run.py", 5), + ] + assert outcome(cache=cache) == expected + assert outcome(cache=cache) == expected + assert outcome(jobs=2) == expected + assert outcome(jobs=2, cache=ProjectCache(tmp_path / "parallel-cache")) == expected + + def test_an_import_no_root_decides_is_noted_and_not_followed(tmp_path: Path) -> None: # Run from the root, tool.py finds no helper beside it: neither file is the one. root = project( From 607b8999b57566949aa173bf5fd6b99eddabdc1f Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 10/24] test(modules): a script's helper comes first, under the name it resolved A script finds the helper beside it before a top-level module of the same name, as Python's import path puts its directory first, and keeps the helper it resolved when another directory's helper, imported under its package name, also has that short name. --- tests/test_namespace_module_imports.py | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py index ef7ffdc..caed284 100644 --- a/tests/test_namespace_module_imports.py +++ b/tests/test_namespace_module_imports.py @@ -147,6 +147,38 @@ def test_each_importer_reaches_the_helper_of_its_own_directory(tmp_path: Path, d assert located(analysis, root, "command-injection") == [("command-injection", f"pkg/{dangerous}/run.py", 5)] +def test_a_script_reaches_the_helper_beside_it_before_one_at_the_project_root(tmp_path: Path) -> None: + # Run from its directory, the script finds the helper beside it first (sys.path[0]). + root = project( + tmp_path, + { + **SCRIPTS, + "helper.py": "def execute(command):\n return command\n", + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root, "command-injection") == [("command-injection", "pkg/tools/run.py", 5)] + + +def test_a_script_keeps_the_helper_it_resolved_beside_another_of_that_short_name(tmp_path: Path) -> None: + # pkg/b/helper.py is also ``helper`` from pkg/b: the script's ``helper`` is its own. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/a/run.py": "import helper\nfrom pkg.b import helper as other\n\n\ndef main():\n other.execute(input())\n helper.execute(input())\n", + "pkg/a/helper.py": "import os\n\n\ndef execute(command):\n os.system(command)\n", + "pkg/b/helper.py": "def execute(command):\n return command\n", + }, + ) + + analysis = engine.analyze_project(root, PLUGINS) + + assert located(analysis, root, "command-injection") == [("command-injection", "pkg/a/run.py", 7)] + + def test_a_module_reached_by_both_its_names_keeps_one_identity(tmp_path: Path) -> None: root = project( tmp_path, From c447e425889fe313114cf733ce32b962b1b79af1 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 11/24] docs: state how a module in a directory without __init__.py is named --- docs/usage.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index e9fdb28..77211ba 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -67,9 +67,14 @@ coretrace-python-analyzer [--check | --emit-ir [--ssa]] [options] [path] ## What is analysed Given a directory, every `.py` file below it is a module of one project, named after -its package (`app/views.py` is `app.views`). Two files that would share a name, `a/app.py` -and `z/app.py` outside any package, are told apart by their path from the root, `a.app` -and `z.app`, and reported: each carries an `ambiguous-module` note, since a symbol +its package (`app/views.py` is `app.views`). A directory without `__init__.py` inside a +package is a portion of it, as Python reads one: `pkg/sub/app.py` is `pkg.sub.app`, and +its relative imports resolve against `pkg`. A module there can also be imported by the +name a script run from its directory gives it, `app`: an absolute import from that +directory reaches what lies beside it first, then what the package's root holds, and +files sharing only such a name are told apart by the importer's directory. Two files +that would share a module name, `a/app.py` and `z/app.py` outside any package, are told +apart by their path from the root, `a.app` and `z.app`, and reported: each carries an `ambiguous-module` note, since a symbol `python.app.main` may name a function of either and an import of `app` resolves only from the importer's own directory, and each is covered as `ambiguous` rather than analysed. An import of such a name that no file of the importer's own directory answers From 16877d99c6b29020969143377da8ad2830586942 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:50:21 +0200 Subject: [PATCH 12/24] docs(changelog): announce the names of modules in directories without __init__.py --- CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 948e889..bc4fdd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,15 @@ ### Compatibility and migration -- The cache format changes (18): caches built by an earlier version miss once. +- The cache format changes (19): caches built by an earlier version miss once. +- A module in a directory without `__init__.py` inside a package is named after that + package, as Python names it (#264): `pkg/sub/app.py` is `pkg.sub.app`. Its relative + imports now resolve, and a file that was a `syntax-error` (`relative import beyond + top-level package`) is analysed, flask's `sansio` for one. The project symbols of its + functions change with its name (`python.pkg.sub.app.view`), which a model declared on + the former symbol must follow; baseline keys do not change, since they hold no module + name. A script beside it still imports it by its short name. Files that share only + that short name are no longer noted as `ambiguous-module`. - SARIF: each rule carries a title (`shortDescription`) and a description (`fullDescription`) instead of repeating its id (#270), and each result carries its confidence as the `confidence` property (#271). A flow reaching its sink inside a project function points at From 7d710cbd7bdd288222540731527c9746a928337c Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 18:54:07 +0200 Subject: [PATCH 13/24] test(regression): record pygoat with its playground modules analysed introduction/playground/A9/api.py and archive.py import .main relatively: named after their package, they are analysed (two syntax-error notes gone, four files and eight functions more), and the two main.py files no longer share a module name (two ambiguous-module notes gone). No other finding changes. --- tests/regression/expected/pygoat.json | 30 +++------------------------ 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/tests/regression/expected/pygoat.json b/tests/regression/expected/pygoat.json index 83c6394..e2f34eb 100644 --- a/tests/regression/expected/pygoat.json +++ b/tests/regression/expected/pygoat.json @@ -1,9 +1,9 @@ { "coverage": { "files": 80, - "files_analysed": 74, - "functions": 295, - "functions_analysed": 295 + "files_analysed": 78, + "functions": 303, + "functions_analysed": 303 }, "findings": [ { @@ -50,12 +50,6 @@ "rule": "debug-enabled", "function": null }, - { - "file": "dockerized_labs/insec_des_lab/main.py", - "line": 1, - "rule": "ambiguous-module", - "function": null - }, { "file": "dockerized_labs/insec_des_lab/main.py", "line": 36, @@ -156,24 +150,6 @@ "rule": "missing-timeout", "function": "check_vuln" }, - { - "file": "introduction/playground/A9/api.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, - { - "file": "introduction/playground/A9/archive.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, - { - "file": "introduction/playground/A9/main.py", - "line": 1, - "rule": "ambiguous-module", - "function": null - }, { "file": "introduction/views.py", "line": 162, From 368d3f2e2623ba6639c7cfb5f54acec2fe340430 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 20:09:00 +0200 Subject: [PATCH 14/24] fix(modulegraph): answer a module's short name only from its own directory A module in a directory without __init__.py is also named, as a script run there names it, from that directory. Unique, that name answered an import from anywhere: pkg/sub/os.py took main.py's "import os" at the project root, and its summary hid the model of os.system. A name a module has only from its own directory now answers only an importer with that directory among its roots; an import that only such names match names no module of the project, as Python finds none there (ModuleNotFoundError), and is no longer noted as ambiguous. The test of that note now pins that the import is neither followed nor noted. --- .../interprocedural/modulegraph.py | 16 +++-- tests/test_namespace_module_imports.py | 12 ++-- tests/test_namespace_module_visibility.py | 65 +++++++++++++++++++ 3 files changed, 82 insertions(+), 11 deletions(-) create mode 100644 tests/test_namespace_module_visibility.py diff --git a/src/coretrace_python/interprocedural/modulegraph.py b/src/coretrace_python/interprocedural/modulegraph.py index d27d97b..b903228 100644 --- a/src/coretrace_python/interprocedural/modulegraph.py +++ b/src/coretrace_python/interprocedural/modulegraph.py @@ -247,12 +247,16 @@ def build_module_graph( resolves to the one from the first of the importer's own roots that has exactly one, and otherwise stays unresolved: no edge, and the name is recorded on the graph. The importer's roots are those of its import names, the last first: as a script run from - its directory finds what lies beside it before what its package's root holds.""" + its directory finds what lies beside it before what its package's root holds. A name + a module has only from its own directory, not its module name, answers only an + importer with that directory among its roots, however few files have it: from + elsewhere, ``import os`` is the library even when ``pkg/sub/os.py`` is ``os`` to a + script beside it.""" - by_name: dict[str, list[tuple[str, Path | None]]] = {} + by_name: dict[str, list[tuple[str, Path | None, bool]]] = {} for name, source in sources.items(): - for import_name, root in import_names(source): - by_name.setdefault(import_name, []).append((name, root)) + for index, (import_name, root) in enumerate(import_names(source)): + by_name.setdefault(import_name, []).append((name, root, index == 0)) roots = {name: tuple(root for _, root in reversed(import_names(source))) for name, source in sources.items()} edges: dict[str, frozenset[str]] = {} unresolved: dict[str, frozenset[str]] = {} @@ -277,8 +281,8 @@ def build_module_graph( missing: set[str] = set() for candidate in candidates: for prefix in _prefixes(candidate): - matching = by_name.get(prefix) - if matching is None: + matching = [(m, r) for m, r, primary in by_name.get(prefix, ()) if primary or r in roots[name]] + if not matching: continue chosen = _chosen(matching, roots[name]) if chosen is None: diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py index caed284..4bf5d5c 100644 --- a/tests/test_namespace_module_imports.py +++ b/tests/test_namespace_module_imports.py @@ -228,8 +228,9 @@ def outcome(**options: object) -> tuple[list[tuple[str, str, int]], list[tuple[s assert outcome(jobs=2, cache=ProjectCache(tmp_path / "parallel-cache")) == expected -def test_an_import_no_root_decides_is_noted_and_not_followed(tmp_path: Path) -> None: - # Run from the root, tool.py finds no helper beside it: neither file is the one. +def test_a_short_name_no_directory_of_the_importer_answers_is_no_module_of_the_project(tmp_path: Path) -> None: + # Run from the root, tool.py finds no helper: pkg/a/helper.py and pkg/b/helper.py + # are ``helper`` only to a script beside them, and Python raises ModuleNotFoundError. root = project( tmp_path, { @@ -242,6 +243,7 @@ def test_an_import_no_root_decides_is_noted_and_not_followed(tmp_path: Path) -> analysis = engine.analyze_project(root, PLUGINS) - (note,) = [f for f in analysis.findings if Path(str(f.span.source_id)).name == "tool.py"] - assert note.rule_id == "ambiguous-module" - assert note.message == "'helper' imported here is pkg/a/helper.py or pkg/b/helper.py: calls into it are not followed" + (tool,) = [n for n in analysis.graph.modules if Path(str(analysis.graph.source(n).source_id)).name == "tool.py"] + assert analysis.graph.imports(tool) == frozenset() + assert analysis.graph.unresolved(tool) == frozenset() + assert [f for f in analysis.findings if Path(str(f.span.source_id)).name == "tool.py"] == [] diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py new file mode 100644 index 0000000..374a7a9 --- /dev/null +++ b/tests/test_namespace_module_visibility.py @@ -0,0 +1,65 @@ +"""Acceptance tests for the review of #264: a name a module has only from its own +directory is seen only from there, up to the summaries. + +``pkg/sub/os.py`` is ``os`` to a script run from ``pkg/sub`` (``sys.path[0]``), and +``pkg.sub.os`` to the rest of the project. From the project root, ``import os`` is the +library: the short name answers no import from another directory, even when no other +file has it. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.cache import ProjectCache + +PLUGINS = [engine.BUNDLED_PLUGINS] + + +def project(tmp_path: Path, files: dict[str, str]) -> Path: + root = tmp_path / "project" + for relative, text in files.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return root + + +def injections(tmp_path: Path, root: Path, run: str) -> list[tuple[str, int]]: + options: dict[str, object] = { + "plain": {}, + "cold-cache": {"cache": ProjectCache(tmp_path / "cache")}, + "warm-cache": {"cache": ProjectCache(tmp_path / "cache")}, + "processes": {"jobs": 2}, + }[run] + if run == "warm-cache": + engine.analyze_project(root, PLUGINS, **options) # type: ignore[arg-type] + analysis = engine.analyze_project(root, PLUGINS, **options) # type: ignore[arg-type] + return sorted( + (Path(str(f.span.source_id)).relative_to(root.resolve()).as_posix(), f.span.start_line) + for f in analysis.findings + if f.rule_id == "command-injection" + ) + + +RUNS = ["plain", "cold-cache", "warm-cache", "processes"] +SHADOWED_OS = "def system(command):\n return None\n" + + +@pytest.mark.parametrize("run", RUNS) +@pytest.mark.parametrize("relative", [True, False], ids=["with-a-relative-import", "without"]) +def test_a_short_name_from_another_directory_leaves_the_library(tmp_path: Path, run: str, relative: bool) -> None: + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/config.py": "VALUE = 1\n", + "pkg/sub/os.py": ("from ..config import VALUE\n\n\n" if relative else "") + SHADOWED_OS, + "main.py": "import os\n\n\ndef run():\n os.system(input())\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 5)] From 77d695f98626d4864ffd8b0c8a4055f5a3a5aae5 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 20:09:00 +0200 Subject: [PATCH 15/24] fix(engine): register an imported module only under the names the graph resolved A component's summaries took every name of every module it reaches: a module reached by its full name was also put under its short name, though the graph had left that name unresolved for the importer, and the importer's calls through it read that module's summary, a constant that hid an injection. A module now goes under the names the graph resolved to it, a member's own resolutions first, then those of the modules the component reaches; a name two of them resolved to different files goes under neither, and a name a member left unresolved under none. --- src/coretrace_python/engine.py | 32 ++++++------- tests/test_namespace_module_visibility.py | 58 ++++++++++++++++++++++- 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 5ae105a..3cb6234 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -844,11 +844,11 @@ def _imported_names( ) -> dict[str, tuple[str, ...]]: """The modules whose summaries a component starts from, each with the import names the component's code refers to it by: every module it imports, transitively, all - final by the time its wave runs. A member's own import goes under the name it - resolved by, unless another member resolved that name to another file. Of several - files with one import name, only the one a member imports directly goes under that - name: the others are reached through summaries that inline them already, and a name - no member's root resolved is followed into neither file.""" + final by the time its wave runs. A module goes under the names the graph resolved to + it, and only those: a member's own resolutions first, then those of the modules the + component reaches. A name two of them resolved to different files goes under + neither, and a name a member left unresolved under none, whatever file reached by + another name also has it.""" wanted: set[str] = set() pending = list(component) @@ -857,20 +857,16 @@ def _imported_names( if imported in results and imported not in wanted: wanted.add(imported) pending.append(imported) + unresolved = {import_name for member in component for import_name in graph.unresolved(member)} chosen: dict[str, str | None] = {} - for member in sorted(component): - for import_name, module in graph.resolved(member).items(): - if module in results and chosen.setdefault(import_name, module) != module: - chosen[import_name] = None - direct = {imported for member in component for imported in graph.imports(member)} - by_import: dict[str, list[str]] = {} - for name in wanted: - for import_name, _ in import_names(graph.source(name)): - by_import.setdefault(import_name, []).append(name) - for import_name, modules in by_import.items(): - picked = modules if len(modules) == 1 else [name for name in modules if name in direct] - if import_name not in chosen and len(picked) == 1: - chosen[import_name] = picked[0] + for importers in (sorted(component), sorted(wanted)): + settled = set(chosen) + for importer in importers: + for import_name, module in graph.resolved(importer).items(): + if module not in results or import_name in unresolved or import_name in settled: + continue + if chosen.setdefault(import_name, module) != module: + chosen[import_name] = None names: dict[str, list[str]] = {} for import_name, reached in sorted(chosen.items()): if reached is not None: diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py index 374a7a9..115c025 100644 --- a/tests/test_namespace_module_visibility.py +++ b/tests/test_namespace_module_visibility.py @@ -4,7 +4,9 @@ ``pkg/sub/os.py`` is ``os`` to a script run from ``pkg/sub`` (``sys.path[0]``), and ``pkg.sub.os`` to the rest of the project. From the project root, ``import os`` is the library: the short name answers no import from another directory, even when no other -file has it. +file has it. And a name the module graph leaves unresolved is registered for no file: +reached by its full name, a module is not also put under a short name the importer +could not resolve. """ from __future__ import annotations @@ -63,3 +65,57 @@ def test_a_short_name_from_another_directory_leaves_the_library(tmp_path: Path, ) assert injections(tmp_path, root, run) == [("main.py", 5)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_short_name_the_graph_leaves_unresolved_is_registered_for_no_file(tmp_path: Path, run: str) -> None: + # pkg.a.helper is reached by its full name only: its execute, which returns a + # constant, is not the helper.execute main.py calls. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/a/helper.py": 'def execute(command):\n return "constant"\n\n\ndef noop():\n return None\n', + "pkg/b/helper.py": "def execute(command):\n return command\n", + "main.py": ( + "import os\nimport helper\nfrom pkg.a.helper import noop\n\n\n" + "def run():\n noop()\n os.system(helper.execute(input()))\n" + ), + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 8)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_name_a_member_leaves_unresolved_is_not_taken_from_a_module_it_imports(tmp_path: Path, run: str) -> None: + # a/user.py resolves helper to a/helper.py, from a; main.py, at the root, cannot + # tell a/helper.py from z/helper.py, and its helper.execute is neither. + root = project( + tmp_path, + { + "a/helper.py": 'def execute(command):\n return "constant"\n', + "a/user.py": 'import helper\n\n\ndef use():\n return helper.execute("x")\n', + "z/helper.py": "def execute(command):\n return command\n", + "main.py": "import os\nimport helper\nimport user\n\n\ndef run():\n user.use()\n os.system(helper.execute(input()))\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 8)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_member_keeps_the_module_it_resolved_over_one_an_import_resolved(tmp_path: Path, run: str) -> None: + # pkg/b/tool.py's helper is pkg/b/helper.py; pkg/a/run.py's is its own. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/a/run.py": "import helper\nfrom pkg.b import tool\n\n\ndef main():\n tool.use()\n helper.execute(input())\n", + "pkg/a/helper.py": "import os\n\n\ndef execute(command):\n os.system(command)\n", + "pkg/b/tool.py": 'import helper\n\n\ndef use():\n return helper.execute("x")\n', + "pkg/b/helper.py": "def execute(command):\n return command\n", + }, + ) + + assert injections(tmp_path, root, run) == [("pkg/a/run.py", 7)] From 2972cc35ccacdff75d9273e55e8cdcc6418029f2 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 20:09:00 +0200 Subject: [PATCH 16/24] docs: state that a module's short name answers only its own directory --- CHANGELOG.md | 7 +++++-- docs/usage.md | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc4fdd8..9a8c9b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,11 @@ top-level package`) is analysed, flask's `sansio` for one. The project symbols of its functions change with its name (`python.pkg.sub.app.view`), which a model declared on the former symbol must follow; baseline keys do not change, since they hold no module - name. A script beside it still imports it by its short name. Files that share only - that short name are no longer noted as `ambiguous-module`. + name. A script beside it still imports it by its short name, which answers no import + from another directory: at the project root, `import os` is the library even beside a + `pkg/sub/os.py`, which a top-level module of a directory took before. Files that share + only that short name are no longer noted as `ambiguous-module`, nor is an import that + only such names from other directories match. - SARIF: each rule carries a title (`shortDescription`) and a description (`fullDescription`) instead of repeating its id (#270), and each result carries its confidence as the `confidence` property (#271). A flow reaching its sink inside a project function points at diff --git a/docs/usage.md b/docs/usage.md index 77211ba..ee1bf1b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -72,7 +72,9 @@ package is a portion of it, as Python reads one: `pkg/sub/app.py` is `pkg.sub.ap its relative imports resolve against `pkg`. A module there can also be imported by the name a script run from its directory gives it, `app`: an absolute import from that directory reaches what lies beside it first, then what the package's root holds, and -files sharing only such a name are told apart by the importer's directory. Two files +files sharing only such a name are told apart by the importer's directory. From another +directory that name answers nothing, as Python finds no such module there: at the +project root, `import os` is the library even beside a `pkg/sub/os.py`. Two files that would share a module name, `a/app.py` and `z/app.py` outside any package, are told apart by their path from the root, `a.app` and `z.app`, and reported: each carries an `ambiguous-module` note, since a symbol `python.app.main` may name a function of either and an import of `app` resolves only From 731910d50c1ab1435b162871ed4729b3853f0aa2 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 20:54:23 +0200 Subject: [PATCH 17/24] fix(engine): register a module reached through another under its module name only A component's summaries took, for a module reached through another, the names that other module resolved: pkg/sub/loader.py's "import os", which from pkg/sub reaches pkg/sub/os.py, put that file under "os" for main.py too, whose "import os" is the library, and the summary of its system hid the injection. A module a member imports goes under the name that member resolved it by; a module reached through another now goes under its module name only, the name the whole project knows it by, as pkg.helpers is reached through "import pkg". What a callee resolved no longer changes what its caller's names mean. --- src/coretrace_python/engine.py | 33 +++++++++++-------- tests/test_namespace_module_visibility.py | 39 +++++++++++++++++++++-- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 3cb6234..46932f2 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -844,11 +844,14 @@ def _imported_names( ) -> dict[str, tuple[str, ...]]: """The modules whose summaries a component starts from, each with the import names the component's code refers to it by: every module it imports, transitively, all - final by the time its wave runs. A module goes under the names the graph resolved to - it, and only those: a member's own resolutions first, then those of the modules the - component reaches. A name two of them resolved to different files goes under - neither, and a name a member left unresolved under none, whatever file reached by - another name also has it.""" + final by the time its wave runs. A module a member imports goes under the name the + member resolved it by; two members resolving one name to different files put it + under neither. A module reached through another goes under its module name only, + the name the whole project knows it by (``pkg.helpers``, reached through + ``import pkg``), never under a name a module in between resolved from its own + directory: what a callee resolved does not change what its caller's names mean. Of + several such modules with one module name, only the one a member imports directly + goes under it, and a name a member left unresolved goes under none.""" wanted: set[str] = set() pending = list(component) @@ -859,14 +862,18 @@ def _imported_names( pending.append(imported) unresolved = {import_name for member in component for import_name in graph.unresolved(member)} chosen: dict[str, str | None] = {} - for importers in (sorted(component), sorted(wanted)): - settled = set(chosen) - for importer in importers: - for import_name, module in graph.resolved(importer).items(): - if module not in results or import_name in unresolved or import_name in settled: - continue - if chosen.setdefault(import_name, module) != module: - chosen[import_name] = None + for member in sorted(component): + for import_name, module in graph.resolved(member).items(): + if module in results and import_name not in unresolved and chosen.setdefault(import_name, module) != module: + chosen[import_name] = None + direct = {imported for member in component for imported in graph.imports(member)} + by_module_name: dict[str, list[str]] = {} + for name in sorted(wanted): + by_module_name.setdefault(graph.source(name).module_name, []).append(name) + for module_name, modules in by_module_name.items(): + picked = modules if len(modules) == 1 else [name for name in modules if name in direct] + if module_name not in chosen and module_name not in unresolved and len(picked) == 1: + chosen[module_name] = picked[0] names: dict[str, list[str]] = {} for import_name, reached in sorted(chosen.items()): if reached is not None: diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py index 115c025..1db458e 100644 --- a/tests/test_namespace_module_visibility.py +++ b/tests/test_namespace_module_visibility.py @@ -4,9 +4,10 @@ ``pkg/sub/os.py`` is ``os`` to a script run from ``pkg/sub`` (``sys.path[0]``), and ``pkg.sub.os`` to the rest of the project. From the project root, ``import os`` is the library: the short name answers no import from another directory, even when no other -file has it. And a name the module graph leaves unresolved is registered for no file: -reached by its full name, a module is not also put under a short name the importer -could not resolve. +file has it. Up to the summaries: a module is registered for an importer only under +the names that importer resolved, or, reached through another module, under its module +name. A name the module graph leaves unresolved is registered for no file, and what a +module in between resolved from its own directory changes nothing for its importer. """ from __future__ import annotations @@ -119,3 +120,35 @@ def test_a_member_keeps_the_module_it_resolved_over_one_an_import_resolved(tmp_p ) assert injections(tmp_path, root, run) == [("pkg/a/run.py", 7)] + + +@pytest.mark.parametrize("run", RUNS) +def test_what_a_module_in_between_resolved_does_not_change_the_importer(tmp_path: Path, run: str) -> None: + # loader.py, in pkg/sub, resolves os to pkg/sub/os.py; main.py's os is the library. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/config.py": "VALUE = 1\n", + "pkg/sub/os.py": "from ..config import VALUE\n\n\n" + SHADOWED_OS, + "pkg/sub/loader.py": "import os\n\n\ndef noop():\n return None\n", + "main.py": "import os\nfrom pkg.sub.loader import noop\n\n\ndef run():\n noop()\n os.system(input())\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 7)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_module_reached_through_its_package_is_followed_by_its_module_name(tmp_path: Path, run: str) -> None: + # pkg.helpers is reached through ``import pkg``, whose __init__.py imports it. + root = project( + tmp_path, + { + "pkg/__init__.py": "from . import helpers\n", + "pkg/helpers.py": "import os\n\n\ndef run(command):\n os.system(command)\n", + "main.py": "import pkg\n\n\ndef go():\n pkg.helpers.run(input())\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 5)] From cc9a43a77fa0d455b019598cf1dedbe65c9766de Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 20:54:23 +0200 Subject: [PATCH 18/24] test(modules): a name one member of a cycle resolved does not change another main.py and x/a.py import each other. From x, x/a.py's helper is x/helper.py; main.py, at the project root, cannot tell x/helper.py from y/helper.py and leaves helper unresolved. The two files are one component, and main.py's helper.execute must still read neither. --- tests/test_namespace_module_visibility.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py index 1db458e..e9d7e40 100644 --- a/tests/test_namespace_module_visibility.py +++ b/tests/test_namespace_module_visibility.py @@ -152,3 +152,20 @@ def test_a_module_reached_through_its_package_is_followed_by_its_module_name(tmp ) assert injections(tmp_path, root, run) == [("main.py", 5)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_name_one_member_of_a_cycle_resolved_does_not_change_another(tmp_path: Path, run: str) -> None: + # main.py and x/a.py import each other; x/a.py's helper is x/helper.py, while main.py, + # at the root, cannot tell x/helper.py from y/helper.py. + root = project( + tmp_path, + { + "x/helper.py": 'def execute(command):\n return "constant"\n', + "x/a.py": 'import helper\nimport main\n\n\ndef use():\n return helper.execute("x")\n', + "y/helper.py": "def execute(command):\n return command\n", + "main.py": "import os\nimport helper\nimport a\n\n\ndef run():\n a.use()\n os.system(helper.execute(input()))\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 8)] From 4113e2f29c2b9f4297e18f26327050b89ca6a3f4 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 23:42:14 +0200 Subject: [PATCH 19/24] refactor(engine): decide in one place the names a component reads each module by The names a component read a module by were decided in two places: those of the modules it imports in _imported_names, those of its own members in _analyse_managers. _imported_names now gives both, and _analyse_managers only builds the index from them. A worker receives them, with the spool key of each module the component imports, instead of naming the members itself. Behaviour is unchanged. --- src/coretrace_python/engine.py | 90 +++++++++++++++++----------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 46932f2..e19860a 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -603,30 +603,30 @@ def analyze_project( computed.update( _analyse_managers( batch, - _seed(results, graph, component), + results, + _imported_names(graph, component, results), module_plugins, affected, suffixes, root, - {name: _names_of(files[name]) for name in batch}, ) ) for manager in batch.values(): manager.run(ResultsEvicted) else: - futures = [ - pool.submit( - _analyse_batch, - _Batch( - {name: _path_of(files[name]) for name in sorted(component)}, - { - name: (names, keys[name]) - for name, names in _imported_names(graph, component, results).items() - }, - ), + futures = [] + for component in pending: + names = _imported_names(graph, component, results) + futures.append( + pool.submit( + _analyse_batch, + _Batch( + {name: _path_of(files[name]) for name in sorted(component)}, + {name: keys[name] for name in names if name not in component}, + names, + ), + ) ) - for component in pending - ] for future in futures: computed.update({name: decode(data) for name, data in future.result().items()}) store = cache if pool is None else spool @@ -833,21 +833,17 @@ def located(name: str) -> str: return notes -def _seed(results: Mapping[str, CachedModule], graph: ModuleGraph, component: frozenset[str]) -> SummaryIndex: - """The summaries a component starts from (``_imported_names``).""" - - return _index(results, _imported_names(graph, component, results)) - - def _imported_names( graph: ModuleGraph, component: frozenset[str], results: Mapping[str, CachedModule] ) -> dict[str, tuple[str, ...]]: - """The modules whose summaries a component starts from, each with the import names - the component's code refers to it by: every module it imports, transitively, all - final by the time its wave runs. A module a member imports goes under the name the - member resolved it by; two members resolving one name to different files put it - under neither. A module reached through another goes under its module name only, - the name the whole project knows it by (``pkg.helpers``, reached through + """The modules whose summaries a component reads, each with the import names the + component's code refers to it by: its members, and every module they import, + transitively, all final by the time its wave runs. Each member goes under its + import names, what the others' code refers to it by; a name two members share, a + cycle across roots, goes under neither. A module a member imports goes under the + name the member resolved it by; two members resolving one name to different files + put it under neither. A module reached through another goes under its module name + only, the name the whole project knows it by (``pkg.helpers``, reached through ``import pkg``), never under a name a module in between resolved from its own directory: what a callee resolved does not change what its caller's names mean. Of several such modules with one module name, only the one a member imports directly @@ -878,6 +874,9 @@ def _imported_names( for import_name, reached in sorted(chosen.items()): if reached is not None: names.setdefault(reached, []).append(import_name) + counts = Counter(import_name for member in component for import_name in _names_of(graph.source(member))) + for member in component: + names[member] = [import_name for import_name in _names_of(graph.source(member)) if counts[import_name] == 1] return {module: tuple(module_names) for module, module_names in names.items()} @@ -904,20 +903,19 @@ def _names_of(source: SourceFile) -> tuple[str, ...]: def _analyse_managers( managers: Mapping[str, AnalysisManager], - seed: SummaryIndex, + imported: Mapping[str, CachedModule], + names: Mapping[str, tuple[str, ...]], plugins: tuple[Plugin, ...], affected: Affected, suffixes: SuffixIndex, root: Path, - member_names: Mapping[str, tuple[str, ...]], ) -> dict[str, CachedModule]: - """Analyse one component: iterate its summaries to a fixpoint over ``seed`` (§21), - then extract what the rest of the run needs from each module. Each member registers - its functions under its import names (``member_names``), what the others' code refers - to it by; a name two members share, a cycle across roots, is registered by neither.""" + """Analyse one component: iterate its summaries to a fixpoint (§21), then extract + what the rest of the run needs from each module. The component reads each module of + ``names`` under the names given (``_imported_names``): its members' summaries as they + grow, the others' as ``imported`` holds them, final.""" - counts = Counter(module for name in managers for module in member_names[name]) - names = {name: tuple(module for module in member_names[name] if counts[module] == 1) for name in managers} + seed = _index(imported, {name: module_names for name, module_names in names.items() if name not in managers}) # The members' functions start from the summaries ``SummaryAnalysis`` starts from. # Left out, a call from one member to another is a call to an unknown symbol on the # first iteration and a project call on the next: around a cycle of modules, the @@ -927,8 +925,8 @@ def _analyse_managers( **seed.summaries, **{ project_symbol(module, function): summary - for name, modules in names.items() - for module in modules + for name in managers + for module in names[name] for function, summary in starting_summaries(managers[name].get(CallGraphAnalysis)).items() }, } @@ -951,8 +949,8 @@ def _analyse_managers( **seed.summaries, **{ project_symbol(module, function): summary - for name, modules in names.items() - for module in modules + for name in managers + for module in names[name] for function, summary in _summaries_of(managers[name]).items() }, } @@ -989,11 +987,12 @@ class _Project: @dataclass(frozen=True) class _Batch: """One component handed to a worker process: its files under the engine's module - names, and the modules its summaries start from, each with its import names and the - key of its entry in the spool. Only these travel, not the summaries themselves.""" + names, the key in the spool of each module it imports, and the names it reads each + module by (``_imported_names``). Only these travel, not the summaries themselves.""" paths: Mapping[str, Path] - imported: Mapping[str, tuple[tuple[str, ...], str]] + keys: Mapping[str, str] + names: Mapping[str, tuple[str, ...]] @dataclass @@ -1009,9 +1008,9 @@ class _Worker: root: Path read: dict[str, CachedModule] = field(default_factory=dict) - def entries(self, imported: Mapping[str, tuple[tuple[str, ...], str]]) -> dict[str, CachedModule]: + def entries(self, keys: Mapping[str, str]) -> dict[str, CachedModule]: found: dict[str, CachedModule] = {} - for name, (_, key) in imported.items(): + for name, key in keys.items(): entry = self.read.get(key) if entry is None: entry = self.spool.load(key) @@ -1079,15 +1078,14 @@ def _analyse_batch(batch: _Batch) -> dict[str, dict[str, Any]]: managers = {name: _register_all(build_hir(source)) for name, source in sources.items()} for manager in managers.values(): worker.managed(manager) - seed = _index(worker.entries(batch.imported), {name: names for name, (names, _) in batch.imported.items()}) results = _analyse_managers( managers, - seed, + worker.entries(batch.keys), + batch.names, worker.plugins, worker.affected, worker.suffixes, worker.root, - {name: _names_of(source) for name, source in sources.items()}, ) return {name: encode(entry) for name, entry in results.items()} From 2a478d79e11d50c306f3eb6211a24178f26e1c9d Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 23:42:14 +0200 Subject: [PATCH 20/24] fix(engine): give each member of a cycle the names it resolved The members of an import cycle shared one summary index: each member's functions went under all its names, and each module the cycle imports under the names any member had resolved. A name one member resolved from its own directory thus changed what another member's code meant. With main.py and pkg/sub/os.py importing each other, pkg/sub/os.py's system took the place of the library's os.system in main.py and hid the injection; pkg/sub/loader.py's "import os" did the same from such a cycle. Each member now reads the modules of its component, members included, in an index of its own: itself under its own names, which name it from its own roots, another module under the names it resolved it by, and otherwise under its module name only. --- src/coretrace_python/engine.py | 165 +++++++++++----------- tests/test_namespace_module_visibility.py | 53 +++++++ 2 files changed, 135 insertions(+), 83 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index e19860a..e593981 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -11,7 +11,6 @@ import multiprocessing import sys import tempfile -from collections import Counter from collections.abc import Callable, Iterable, Mapping, Sequence from dataclasses import dataclass, field, replace from pathlib import Path @@ -622,7 +621,7 @@ def analyze_project( _analyse_batch, _Batch( {name: _path_of(files[name]) for name in sorted(component)}, - {name: keys[name] for name in names if name not in component}, + {name: keys[name] for read in names.values() for name in read if name not in component}, names, ), ) @@ -835,49 +834,46 @@ def located(name: str) -> str: def _imported_names( graph: ModuleGraph, component: frozenset[str], results: Mapping[str, CachedModule] -) -> dict[str, tuple[str, ...]]: - """The modules whose summaries a component reads, each with the import names the - component's code refers to it by: its members, and every module they import, - transitively, all final by the time its wave runs. Each member goes under its - import names, what the others' code refers to it by; a name two members share, a - cycle across roots, goes under neither. A module a member imports goes under the - name the member resolved it by; two members resolving one name to different files - put it under neither. A module reached through another goes under its module name - only, the name the whole project knows it by (``pkg.helpers``, reached through - ``import pkg``), never under a name a module in between resolved from its own - directory: what a callee resolved does not change what its caller's names mean. Of - several such modules with one module name, only the one a member imports directly - goes under it, and a name a member left unresolved goes under none.""" - - wanted: set[str] = set() +) -> dict[str, dict[str, tuple[str, ...]]]: + """For each member of a component, the modules whose summaries it reads, each with + the import names the member's own code refers to it by: the members, and every + module they import, transitively, all final by the time the component's wave runs. + The member goes under its own names, each of which names it from its own roots. + Another module goes under the names the member resolved it by, and otherwise under + its module name only, the name the whole project knows it by (``pkg.helpers``, + reached through ``import pkg``), never under a name another module resolved from + its own directory: what a callee, or another member of a cycle, resolved does not + change what the member's names mean. Of several such modules with one module name, + only one the member imports directly goes under it, and a name the member left + unresolved goes under none.""" + + reached = set(component) pending = list(component) while pending: for imported in graph.imports(pending.pop()): - if imported in results and imported not in wanted: - wanted.add(imported) + if imported in results and imported not in reached: + reached.add(imported) pending.append(imported) - unresolved = {import_name for member in component for import_name in graph.unresolved(member)} - chosen: dict[str, str | None] = {} - for member in sorted(component): - for import_name, module in graph.resolved(member).items(): - if module in results and import_name not in unresolved and chosen.setdefault(import_name, module) != module: - chosen[import_name] = None - direct = {imported for member in component for imported in graph.imports(member)} by_module_name: dict[str, list[str]] = {} - for name in sorted(wanted): + for name in sorted(reached): by_module_name.setdefault(graph.source(name).module_name, []).append(name) - for module_name, modules in by_module_name.items(): - picked = modules if len(modules) == 1 else [name for name in modules if name in direct] - if module_name not in chosen and module_name not in unresolved and len(picked) == 1: - chosen[module_name] = picked[0] - names: dict[str, list[str]] = {} - for import_name, reached in sorted(chosen.items()): - if reached is not None: - names.setdefault(reached, []).append(import_name) - counts = Counter(import_name for member in component for import_name in _names_of(graph.source(member))) - for member in component: - names[member] = [import_name for import_name in _names_of(graph.source(member)) if counts[import_name] == 1] - return {module: tuple(module_names) for module, module_names in names.items()} + read: dict[str, dict[str, tuple[str, ...]]] = {} + for member in sorted(component): + # The graph records no import of a module by itself. + chosen = { + **dict.fromkeys(_names_of(graph.source(member)), member), + **{import_name: module for import_name, module in graph.resolved(member).items() if module in reached}, + } + direct = graph.imports(member) + for module_name, modules in by_module_name.items(): + picked = modules if len(modules) == 1 else [name for name in modules if name in direct] + if module_name not in chosen and module_name not in graph.unresolved(member) and len(picked) == 1: + chosen[module_name] = picked[0] + names: dict[str, list[str]] = {} + for import_name, module in sorted(chosen.items()): + names.setdefault(module, []).append(import_name) + read[member] = {module: tuple(module_names) for module, module_names in names.items()} + return read def _index(results: Mapping[str, CachedModule], names: Mapping[str, Iterable[str]]) -> SummaryIndex: @@ -904,63 +900,65 @@ def _names_of(source: SourceFile) -> tuple[str, ...]: def _analyse_managers( managers: Mapping[str, AnalysisManager], imported: Mapping[str, CachedModule], - names: Mapping[str, tuple[str, ...]], + names: Mapping[str, Mapping[str, tuple[str, ...]]], plugins: tuple[Plugin, ...], affected: Affected, suffixes: SuffixIndex, root: Path, ) -> dict[str, CachedModule]: """Analyse one component: iterate its summaries to a fixpoint (§21), then extract - what the rest of the run needs from each module. The component reads each module of - ``names`` under the names given (``_imported_names``): its members' summaries as they - grow, the others' as ``imported`` holds them, final.""" + what the rest of the run needs from each module. Each member reads, in an index of + its own, the modules ``names`` gives it under the names given (``_imported_names``): + the members' summaries as they grow, the others' as ``imported`` holds them, final.""" + + final = { + member: _index(imported, {name: module_names for name, module_names in names[member].items() if name not in managers}) + for member in managers + } + + def indexes(summaries: Mapping[str, Mapping[str, FunctionSummary]]) -> dict[str, SummaryIndex]: + return { + member: SummaryIndex( + { + **final[member].summaries, + **{ + project_symbol(module, function): summary + for name, module_names in names[member].items() + if name in managers + for module in module_names + for function, summary in summaries[name].items() + }, + } + ) + for member in managers + } - seed = _index(imported, {name: module_names for name, module_names in names.items() if name not in managers}) # The members' functions start from the summaries ``SummaryAnalysis`` starts from. # Left out, a call from one member to another is a call to an unknown symbol on the # first iteration and a project call on the next: around a cycle of modules, the # iteration then shifts by one module each time and never settles (#261). - index = SummaryIndex( - { - **seed.summaries, - **{ - project_symbol(module, function): summary - for name in managers - for module in names[name] - for function, summary in starting_summaries(managers[name].get(CallGraphAnalysis)).items() - }, - } - ) - for manager in managers.values(): - manager.provide(ProjectSummaries, index) - # Iterated until the index settles, with no bound: stopped earlier, summaries that - # started empty would miss what later iterations add, a dependency still on its way - # reading as no taint. It settles because each iteration can only add: the index holds - # the same symbols from the start, and in every round of a module's fixpoint the rules - # that read the index join what they read (a call's dependencies and records, heap - # stores, merges), while those that do not join (a nonlocal record not summarized yet, - # ``kept``, reachability, ``unsupported``, ``static``) depend on the code and the round - # only. Each round's table thus grows with the index, the fixpoint too, in a finite - # domain: the parameters, symbols and call records of the code. A rule reading the - # index without joining would break this, and the loop's termination with it. + index = indexes({name: starting_summaries(manager.get(CallGraphAnalysis)) for name, manager in managers.items()}) + for name, manager in managers.items(): + manager.provide(ProjectSummaries, index[name]) + # Iterated until every member's index settles, with no bound: stopped earlier, + # summaries that started empty would miss what later iterations add, a dependency + # still on its way reading as no taint. It settles because each iteration can only + # add: each index holds the same symbols from the start, and in every round of a + # module's fixpoint the rules that read the index join what they read (a call's + # dependencies and records, heap stores, merges), while those that do not join (a + # nonlocal record not summarized yet, ``kept``, reachability, ``unsupported``, + # ``static``) depend on the code and the round only. Each round's table thus grows + # with the index, the fixpoint too, in a finite domain: the parameters, symbols and + # call records of the code. A rule reading the index without joining would break + # this, and the loop's termination with it. while True: - updated = SummaryIndex( - { - **seed.summaries, - **{ - project_symbol(module, function): summary - for name in managers - for module in names[name] - for function, summary in _summaries_of(managers[name]).items() - }, - } - ) + updated = indexes({name: _summaries_of(manager) for name, manager in managers.items()}) if updated == index: break index = updated - for manager in managers.values(): + for name, manager in managers.items(): manager.run(ProjectSummariesUpdated) - manager.provide(ProjectSummaries, index) + manager.provide(ProjectSummaries, index[name]) return {name: _analyse_module(manager, plugins, affected, suffixes, root) for name, manager in managers.items()} @@ -987,12 +985,13 @@ class _Project: @dataclass(frozen=True) class _Batch: """One component handed to a worker process: its files under the engine's module - names, the key in the spool of each module it imports, and the names it reads each - module by (``_imported_names``). Only these travel, not the summaries themselves.""" + names, the key in the spool of each module it imports, and the names each member + reads each module by (``_imported_names``). Only these travel, not the summaries + themselves.""" paths: Mapping[str, Path] keys: Mapping[str, str] - names: Mapping[str, tuple[str, ...]] + names: Mapping[str, Mapping[str, tuple[str, ...]]] @dataclass diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py index e9d7e40..b941c82 100644 --- a/tests/test_namespace_module_visibility.py +++ b/tests/test_namespace_module_visibility.py @@ -8,6 +8,8 @@ the names that importer resolved, or, reached through another module, under its module name. A name the module graph leaves unresolved is registered for no file, and what a module in between resolved from its own directory changes nothing for its importer. +Within an import cycle too, each member reads the others under the names it resolved +itself, and itself under its own names. """ from __future__ import annotations @@ -169,3 +171,54 @@ def test_a_name_one_member_of_a_cycle_resolved_does_not_change_another(tmp_path: ) assert injections(tmp_path, root, run) == [("main.py", 8)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_member_of_a_cycle_is_not_read_under_a_short_name_another_did_not_resolve(tmp_path: Path, run: str) -> None: + # main.py and pkg/sub/os.py import each other; main.py's os is the library. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/config.py": "VALUE = 1\n", + "pkg/sub/os.py": "from ..config import VALUE\nimport main\n\n\ndef noop():\n return None\n\n\n" + SHADOWED_OS, + "main.py": "import os\nfrom pkg.sub.os import noop\n\n\ndef run():\n noop()\n os.system(input())\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 7)] + + +@pytest.mark.parametrize("run", RUNS) +def test_what_a_member_of_a_cycle_resolved_does_not_change_another(tmp_path: Path, run: str) -> None: + # main.py and pkg/sub/loader.py import each other; loader.py's os is pkg/sub/os.py, + # main.py's the library. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/config.py": "VALUE = 1\n", + "pkg/sub/os.py": "from ..config import VALUE\n\n\n" + SHADOWED_OS, + "pkg/sub/loader.py": "import os\nimport main\n\n\ndef noop():\n return None\n", + "main.py": "import os\nfrom pkg.sub.loader import noop\n\n\ndef run():\n noop()\n os.system(input())\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 7)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_module_that_imports_itself_by_its_short_name_reads_its_own_functions(tmp_path: Path, run: str) -> None: + # Run from pkg/tools, helper.py's ``import helper`` is helper.py itself. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/tools/helper.py": ( + "import os\nimport helper\n\n\ndef execute(command):\n os.system(command)\n\n\n" + "def run():\n helper.execute(input())\n" + ), + }, + ) + + assert injections(tmp_path, root, run) == [("pkg/tools/helper.py", 10)] From f00f8f84456ba431bed1beea06c5ee5344e4497c Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 23:42:14 +0200 Subject: [PATCH 21/24] test(modules): each member of a cycle reads the helper it resolved pkg/a/run.py and pkg/b/tool.py import each other, and each the helper.py of its own directory. The shared index put helper under neither; with an index of its own, run.py's helper.execute reaches the sink. --- tests/test_namespace_module_visibility.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py index b941c82..eb334f5 100644 --- a/tests/test_namespace_module_visibility.py +++ b/tests/test_namespace_module_visibility.py @@ -222,3 +222,20 @@ def test_a_module_that_imports_itself_by_its_short_name_reads_its_own_functions( ) assert injections(tmp_path, root, run) == [("pkg/tools/helper.py", 10)] + + +@pytest.mark.parametrize("run", RUNS) +def test_each_member_of_a_cycle_reads_the_module_it_resolved(tmp_path: Path, run: str) -> None: + # pkg/a/run.py and pkg/b/tool.py import each other, and each its own helper. + root = project( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/a/run.py": "import helper\nfrom pkg.b import tool\n\n\ndef main():\n tool.use()\n helper.execute(input())\n", + "pkg/a/helper.py": "import os\n\n\ndef execute(command):\n os.system(command)\n", + "pkg/b/tool.py": 'import helper\nfrom pkg.a import run\n\n\ndef use():\n return helper.execute("x")\n', + "pkg/b/helper.py": "def execute(command):\n return command\n", + }, + ) + + assert injections(tmp_path, root, run) == [("pkg/a/run.py", 7)] From f0e21048057063a527cc5784ac653d2e5b5484ec Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 23:42:14 +0200 Subject: [PATCH 22/24] test(modules): follow a member of a cycle reached through its package by its module name main.py, pkg/__init__.py and pkg/helpers.py import one another in turn. main.py's pkg.helpers.run is that of pkg/helpers.py, a member it does not import by that name. --- tests/test_namespace_module_visibility.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py index eb334f5..c7bf9bc 100644 --- a/tests/test_namespace_module_visibility.py +++ b/tests/test_namespace_module_visibility.py @@ -239,3 +239,18 @@ def test_each_member_of_a_cycle_reads_the_module_it_resolved(tmp_path: Path, run ) assert injections(tmp_path, root, run) == [("pkg/a/run.py", 7)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_member_of_a_cycle_reached_through_its_package_is_followed_by_its_module_name(tmp_path: Path, run: str) -> None: + # main.py, pkg/__init__.py and pkg/helpers.py import one another in turn. + root = project( + tmp_path, + { + "pkg/__init__.py": "from . import helpers\n", + "pkg/helpers.py": "import os\nimport main\n\n\ndef run(command):\n os.system(command)\n", + "main.py": "import pkg\n\n\ndef go():\n pkg.helpers.run(input())\n", + }, + ) + + assert injections(tmp_path, root, run) == [("main.py", 5)] From 1ce0a4d0ab029a662e5ed4eaf8f1fae25206b2f9 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 23:42:14 +0200 Subject: [PATCH 23/24] test(modules): take a module name two reached files share for neither main.py, run from src, reaches src/pkg/helpers.py through its pkg, and lib/pkg/helpers.py through lib/tool.py. Its pkg.helpers is src's, harmless; lib's, which reaches a sink, must not take its place. --- tests/test_namespace_module_visibility.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py index c7bf9bc..685300a 100644 --- a/tests/test_namespace_module_visibility.py +++ b/tests/test_namespace_module_visibility.py @@ -254,3 +254,22 @@ def test_a_member_of_a_cycle_reached_through_its_package_is_followed_by_its_modu ) assert injections(tmp_path, root, run) == [("main.py", 5)] + + +@pytest.mark.parametrize("run", RUNS) +def test_a_module_name_two_reached_files_share_is_taken_for_neither(tmp_path: Path, run: str) -> None: + # main.py, run from src, reaches src/pkg/helpers.py through its pkg, and + # lib/pkg/helpers.py through lib/tool.py; its pkg.helpers is src's, harmless. + root = project( + tmp_path, + { + "src/pkg/__init__.py": "from . import helpers\n", + "src/pkg/helpers.py": "def run(command):\n return command\n", + "lib/pkg/__init__.py": "from . import helpers\n", + "lib/pkg/helpers.py": "import os\n\n\ndef run(command):\n os.system(command)\n", + "lib/tool.py": "import pkg\n\n\ndef noop():\n return None\n", + "src/main.py": "import pkg\nimport tool\n\n\ndef go():\n tool.noop()\n pkg.helpers.run(input())\n", + }, + ) + + assert injections(tmp_path, root, run) == [] From 7bf608fe815b75fc8a192cb00d89c5d438dcc44b Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 22 Oct 2026 23:42:14 +0200 Subject: [PATCH 24/24] perf(engine): share one summary index among the members of a cycle that read alike With an index of its own for each member, built and compared at every round of the fixpoint, Django 5.1.4, whose largest cycle holds 144 modules, took 185 s to analyse instead of 176 s. In a regular package every member reads every module under the same names: the members that do now share one index, and the analysis takes 175 s again, with the same findings. --- src/coretrace_python/engine.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index e593981..e29a52c 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -909,28 +909,33 @@ def _analyse_managers( """Analyse one component: iterate its summaries to a fixpoint (§21), then extract what the rest of the run needs from each module. Each member reads, in an index of its own, the modules ``names`` gives it under the names given (``_imported_names``): - the members' summaries as they grow, the others' as ``imported`` holds them, final.""" + the members' summaries as they grow, the others' as ``imported`` holds them, final. + Members that read every module under the same names, as in a regular package, share + one index.""" + views = {member: tuple(sorted(names[member].items())) for member in managers} final = { - member: _index(imported, {name: module_names for name, module_names in names[member].items() if name not in managers}) - for member in managers + view: _index(imported, {name: module_names for name, module_names in view if name not in managers}) + for view in dict.fromkeys(views.values()) } - def indexes(summaries: Mapping[str, Mapping[str, FunctionSummary]]) -> dict[str, SummaryIndex]: + def indexes( + summaries: Mapping[str, Mapping[str, FunctionSummary]], + ) -> dict[tuple[tuple[str, tuple[str, ...]], ...], SummaryIndex]: return { - member: SummaryIndex( + view: SummaryIndex( { - **final[member].summaries, + **seed.summaries, **{ project_symbol(module, function): summary - for name, module_names in names[member].items() + for name, module_names in view if name in managers for module in module_names for function, summary in summaries[name].items() }, } ) - for member in managers + for view, seed in final.items() } # The members' functions start from the summaries ``SummaryAnalysis`` starts from. @@ -939,7 +944,7 @@ def indexes(summaries: Mapping[str, Mapping[str, FunctionSummary]]) -> dict[str, # iteration then shifts by one module each time and never settles (#261). index = indexes({name: starting_summaries(manager.get(CallGraphAnalysis)) for name, manager in managers.items()}) for name, manager in managers.items(): - manager.provide(ProjectSummaries, index[name]) + manager.provide(ProjectSummaries, index[views[name]]) # Iterated until every member's index settles, with no bound: stopped earlier, # summaries that started empty would miss what later iterations add, a dependency # still on its way reading as no taint. It settles because each iteration can only @@ -958,7 +963,7 @@ def indexes(summaries: Mapping[str, Mapping[str, FunctionSummary]]) -> dict[str, index = updated for name, manager in managers.items(): manager.run(ProjectSummariesUpdated) - manager.provide(ProjectSummaries, index[name]) + manager.provide(ProjectSummaries, index[views[name]]) return {name: _analyse_module(manager, plugins, affected, suffixes, root) for name, manager in managers.items()}