diff --git a/CHANGELOG.md b/CHANGELOG.md index 948e889..9a8c9b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,18 @@ ### 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, 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 e9fdb28..ee1bf1b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -67,9 +67,16 @@ 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. 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 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 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 911606d..e29a52c 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 @@ -95,6 +94,7 @@ TemplateCallsAnalysis, build_module_graph, discover_sources, + import_names, name_modules, project_symbol, starting_summaries, @@ -547,9 +547,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: @@ -596,25 +601,31 @@ 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, + results, + _imported_names(graph, component, results), + module_plugins, + affected, + suffixes, + root, ) ) 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: (import_name, keys[name]) - for name, import_name 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 read in names.values() for name in read 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 @@ -628,7 +639,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): @@ -781,7 +792,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: @@ -797,6 +810,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: @@ -809,118 +826,144 @@ 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 -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, 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.""" - - 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) - 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, 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 - - -def _index(results: Mapping[str, CachedModule], names: Mapping[str, str]) -> SummaryIndex: + by_module_name: dict[str, list[str]] = {} + for name in sorted(reached): + by_module_name.setdefault(graph.source(name).module_name, []).append(name) + 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: """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, + imported: Mapping[str, CachedModule], + 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 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.""" + """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. + 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 = { + 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[tuple[tuple[str, tuple[str, ...]], ...], SummaryIndex]: + return { + view: SummaryIndex( + { + **seed.summaries, + **{ + project_symbol(module, function): summary + for name, module_names in view + if name in managers + for module in module_names + for function, summary in summaries[name].items() + }, + } + ) + for view, seed in final.items() + } - 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} # 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, module in names.items() - 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[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 + # 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, module in names.items() - 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[views[name]]) return {name: _analyse_module(manager, plugins, affected, suffixes, root) for name, manager in managers.items()} @@ -947,11 +990,13 @@ 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 - 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 each member + reads each module by (``_imported_names``). Only these travel, not the summaries + themselves.""" paths: Mapping[str, Path] - imported: Mapping[str, tuple[str, str]] + keys: Mapping[str, str] + names: Mapping[str, Mapping[str, tuple[str, ...]]] @dataclass @@ -967,9 +1012,9 @@ 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, 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) @@ -1033,12 +1078,19 @@ 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, 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) - 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) + results = _analyse_managers( + managers, + worker.entries(batch.keys), + batch.names, + worker.plugins, + worker.affected, + worker.suffixes, + worker.root, + ) 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 b540563..b903228 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 @@ -120,17 +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: 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.""" +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.""" - if source.path is None: - return None - directory = source.path.parent - while (directory / "__init__.py").is_file(): - directory = directory.parent - return directory + 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: @@ -157,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, ...]: @@ -177,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 @@ -241,16 +243,24 @@ 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.""" - - by_name: dict[str, list[str]] = {} + 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. 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, bool]]] = {} 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 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]] = {} + resolved: dict[str, Mapping[str, str]] = {} for name, module in modules.items(): candidates: set[str] = set() dotted_tops: set[str] = set() @@ -267,24 +277,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: + matching = [(m, r) for m, r, primary in by_name.get(prefix, ()) if primary or r in roots[name]] + if not matching: 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]: @@ -310,6 +335,7 @@ def _statements(body: Iterable[nodes.Statement]) -> Iterable[nodes.Statement]: "SummaryIndex", "build_module_graph", "discover_sources", + "import_names", "name_modules", "project_symbol", ] 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..248c2f9 100644 --- a/src/coretrace_python/source/manager.py +++ b/src/coretrace_python/source/manager.py @@ -5,20 +5,60 @@ 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. +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. A package's ``__init__.py`` is named after + the package itself. - 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 ".".join(reversed(parts)) or path.stem + 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, root: Path | None = None) -> str: + """Dotted module name of ``path`` (``import_names_for``).""" + + return import_names_for(path, root)[0].name def decode_text(data: bytes) -> str: @@ -68,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) @@ -76,12 +120,14 @@ def load_file(self, path: str | Path) -> SourceFile: return existing text = decode_text(resolved_path.read_bytes()) + names = import_names_for(resolved_path, None if root is None else root.resolve()) 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, ...] = () 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, diff --git a/tests/test_namespace_module_imports.py b/tests/test_namespace_module_imports.py new file mode 100644 index 0000000..4bf5d5c --- /dev/null +++ b/tests/test_namespace_module_imports.py @@ -0,0 +1,249 @@ +"""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 +from coretrace_python.cache import ProjectCache + +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 + ) + + +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', + "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)] + + +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, + { + **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)] + + +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_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, + { + "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) + + (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_names.py b/tests/test_namespace_module_names.py new file mode 100644 index 0000000..d5f737b --- /dev/null +++ b/tests/test_namespace_module_names.py @@ -0,0 +1,90 @@ +"""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 + +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" + 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"} + + +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" diff --git a/tests/test_namespace_module_visibility.py b/tests/test_namespace_module_visibility.py new file mode 100644 index 0000000..685300a --- /dev/null +++ b/tests/test_namespace_module_visibility.py @@ -0,0 +1,275 @@ +"""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. 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. +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 + +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)] + + +@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)] + + +@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)] + + +@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)] + + +@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)] + + +@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)] + + +@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)] + + +@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) == []