diff --git a/docs/usage.md b/docs/usage.md index 5ea6847..e3dd65c 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -914,7 +914,11 @@ comprehensions, lambdas, nested functions and classes, `global`, `nonlocal`, `de assignment expressions (`(y := f(x))`, laid out as an assignment before the statement), `yield from`, starred assignment targets (`first, *rest = items`), class keyword arguments such as `metaclass=`, `with` targets of any assignable form and `match` with every -pattern kind. `finally` is modelled on the normal path only. A +pattern kind. An assignment expression in an annotation binds its name as Python 3.11 to +3.13 do, where the annotation stands: in a function, the name is a local whether the +annotation is evaluated or not; at module level, it exists only once the annotation is +evaluated, so under `from __future__ import annotations` a builtin it names stays the +builtin. `finally` is modelled on the normal path only. A `nonlocal` write made by a nested function flows back to the enclosing variable when the enclosing function calls that nested function directly by name; a nested function passed around and called elsewhere keeps its writes to itself. Where the nested function may diff --git a/src/coretrace_python/semantic/scopes.py b/src/coretrace_python/semantic/scopes.py index 4120871..779bd42 100644 --- a/src/coretrace_python/semantic/scopes.py +++ b/src/coretrace_python/semantic/scopes.py @@ -221,6 +221,10 @@ def __init__(self, module: nodes.Module) -> None: # The module scope is found by the module's span, as the synthetic ```` # function carries it. self.spans: dict[SourceSpan, ScopeId] = {module.span: self.module.id} + self.postponed = _postpones_annotations(module) + # The module or class an annotation that is never evaluated stands in, while it is + # visited. + self.unevaluated: _ScopeBuilder | None = None self.body(module.body, self.module) def body(self, statements: tuple[nodes.Statement, ...], scope: _ScopeBuilder) -> None: @@ -281,6 +285,8 @@ def statement(self, node: nodes.Statement, scope: _ScopeBuilder) -> None: for decorator in node.decorators: self.expression(decorator, scope) for parameter in node.parameters: + if parameter.annotation is not None: + self.annotation(parameter.annotation, scope, evaluated=not self.postponed) if parameter.default is not None: self.expression(parameter.default, scope) scope.bind(node.name, BindingKind.FUNCTION, node.span) @@ -323,6 +329,10 @@ def statement(self, node: nodes.Statement, scope: _ScopeBuilder) -> None: self.target(target, scope) elif isinstance(node, nodes.Declaration): scope.bind(node.name, BindingKind.LOCAL, node.span) + if node.annotation is not None: + # PEP 526: the annotation of a local variable is never evaluated. + evaluated = not self.postponed and scope.kind is not ScopeKind.FUNCTION + self.annotation(node.annotation, scope, evaluated) elif not isinstance(node, nodes.Pass | nodes.Break | nodes.Continue): raise TypeError(f"unknown statement: {node!r}") @@ -361,7 +371,8 @@ def expression(self, node: nodes.Expression, scope: _ScopeBuilder) -> None: owner = scope while owner.kind is ScopeKind.COMPREHENSION and owner.parent is not None: owner = owner.parent - owner.bind(node.target.identifier, BindingKind.LOCAL, node.target.span) + if owner is not self.unevaluated: + owner.bind(node.target.identifier, BindingKind.LOCAL, node.target.span) elif isinstance(node, nodes.Tuple | nodes.List): for element in node.elements: self.expression(element, scope) @@ -397,6 +408,22 @@ def expression(self, node: nodes.Expression, scope: _ScopeBuilder) -> None: else: raise TypeError(f"unknown expression: {node!r}") + def annotation(self, node: nodes.Expression, scope: _ScopeBuilder, evaluated: bool) -> None: + """Visit an annotation standing in ``scope``. Python's compiler declares the + targets of its assignment expressions in ``scope`` whether it is evaluated or not, + so in a function they are locals all the same, which a nested function may declare + ``nonlocal``. A module's or a class's names exist only once assigned: there, one + that is never evaluated binds nothing, and a builtin it names stays the builtin. + Its lambdas and comprehensions introduce their scopes either way, which later + passes look up.""" + + if evaluated or scope.kind is ScopeKind.FUNCTION: + self.expression(node, scope) + return + self.unevaluated = scope + self.expression(node, scope) + self.unevaluated = None + def pattern(self, node: nodes.Pattern, scope: _ScopeBuilder) -> None: if isinstance(node, nodes.ValuePattern): self.expression(node.value, scope) @@ -461,6 +488,20 @@ def open( return builder +def _postpones_annotations(module: nodes.Module) -> bool: + """Whether ``module`` imports ``annotations`` from ``__future__`` (PEP 563): its + annotations are then kept as strings, never evaluated. Python accepts a future + import only at the top of the module, so the module's statements are enough.""" + + return any( + isinstance(statement, nodes.ImportFrom) + and statement.module == "__future__" + and statement.level == 0 + and any(alias.name == "annotations" for alias in statement.names) + for statement in module.body + ) + + def analyze_scopes(module: nodes.Module) -> ScopeTable: """Build the scope tree of ``module`` and validate its declarations.""" diff --git a/tests/test_annotation_bindings.py b/tests/test_annotation_bindings.py new file mode 100644 index 0000000..2731189 --- /dev/null +++ b/tests/test_annotation_bindings.py @@ -0,0 +1,154 @@ +"""What an annotation binds in the scope it stands in. + +Python's compiler declares the target of an assignment expression in an annotation +where the annotation stands, whether it is evaluated or not: in a function, the name is +a local of the function. A module's names exist only once assigned, so an annotation +that is never evaluated there, under ``from __future__ import annotations`` (PEP 563), +binds nothing: a builtin it names, such as ``eval``, stays the builtin. Either way, its +lambdas and comprehensions introduce scopes of their own, which keep their bindings. +""" + +from __future__ import annotations + +from collections.abc import Iterable +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.findings import Finding +from coretrace_python.frontend import build_hir +from coretrace_python.semantic.scopes import BindingKind, Scope, ScopeTable, analyze_scopes +from coretrace_python.source import SourceManager + +POSTPONED = "from __future__ import annotations\n\n" +PLUGINS = [engine.BUNDLED_PLUGINS] +# The comprehension never runs, and its annotation is a string: ``eval`` stays the builtin. +SHADOWING = POSTPONED + "def f(x: [(eval := i) for i in ()]):\n pass\n\n\ndef run():\n eval(input())\n" +EVAL_FINDINGS = {("code-injection", 8), ("dangerous-eval", 8)} +# The annotation never runs, yet ``cmd`` is a local of ``run``: the ``nonlocal`` is valid +# and the input reaches the command. +NONLOCAL = ( + "from __future__ import annotations\nimport os\n\ndef run():\n" + " def declared(x: [(cmd := i) for i in ()]):\n pass\n" + " def set_cmd():\n nonlocal cmd\n cmd = input()\n" + " set_cmd()\n os.system(cmd)\n" +) +NONLOCAL_FINDINGS = {("command-injection", 11)} + + +def analyze(text: str) -> ScopeTable: + return analyze_scopes(build_hir(SourceManager().add_source("app.py", text))) + + +def child(scopes: ScopeTable, parent: Scope, name: str) -> Scope: + (found,) = (scope for scope in scopes.children(parent.id) if scope.name == name) + return found + + +def located(findings: Iterable[Finding]) -> set[tuple[str, int]]: + return {(finding.rule_id, finding.span.start_line) for finding in findings} + + +def test_an_evaluated_parameter_annotation_binds_where_the_function_is_defined() -> None: + scopes = analyze("def f(x: [(found := i) for i in (1,)]):\n pass\n") + + assert scopes.module_scope.bindings["found"].kind is BindingKind.LOCAL + + +def test_an_evaluated_parameter_annotation_binds_in_the_enclosing_function() -> None: + scopes = analyze("def outer():\n def inner(x: [(found := i) for i in (1,)]):\n pass\n") + + assert "found" in child(scopes, scopes.module_scope, "outer").bindings + assert "found" not in scopes.module_scope.bindings + + +def test_a_lambda_in_a_postponed_annotation_keeps_its_own_bindings() -> None: + scopes = analyze(POSTPONED + "def f(x: (lambda: (inner := 1))):\n pass\n") + + assert "inner" in child(scopes, scopes.module_scope, "").bindings + assert "inner" not in scopes.module_scope.bindings + + +def test_a_postponed_parameter_annotation_binds_nothing_where_it_stands() -> None: + scopes = analyze(POSTPONED + "def f(x: [(found := i) for i in (1,)]):\n pass\n") + + assert "found" not in scopes.module_scope.bindings + assert child(scopes, scopes.module_scope, "").parent == scopes.module_scope.id + + +@pytest.mark.parametrize( + "header", + ["from .__future__ import annotations\n", "from __future__ import division\n", "import __future__\n"], +) +def test_only_the_future_statement_postpones_annotations(header: str) -> None: + scopes = analyze(header + "def f(x: [(found := i) for i in (1,)]):\n pass\n") + + assert "found" in scopes.module_scope.bindings + + +def test_assignment_expressions_after_a_postponed_annotation_still_bind() -> None: + scopes = analyze(POSTPONED + "def f(x: (lambda: 1)):\n pass\n\n\n[(found := i) for i in (1,)]\n") + + assert "found" in scopes.module_scope.bindings + + +def test_a_postponed_annotation_in_a_function_still_declares_there() -> None: + scopes = analyze(POSTPONED + "def outer():\n def inner(x: [(found := i) for i in (1,)]):\n pass\n") + + assert "found" in child(scopes, scopes.module_scope, "outer").bindings + assert "found" not in scopes.module_scope.bindings + + +def test_a_nonlocal_of_a_name_a_postponed_annotation_declares_is_followed_in_a_file() -> None: + findings = engine.check(SourceManager().add_source("app.py", NONLOCAL), PLUGINS) + + assert located(findings) == NONLOCAL_FINDINGS + + +def test_a_nonlocal_of_a_name_a_postponed_annotation_declares_is_followed_in_a_project(tmp_path: Path) -> None: + (tmp_path / "app.py").write_text(NONLOCAL, encoding="utf-8") + + assert located(engine.analyze_project(tmp_path, PLUGINS).findings) == NONLOCAL_FINDINGS + + +def test_a_builtin_named_in_a_postponed_annotation_stays_the_builtin_in_a_file() -> None: + findings = engine.check(SourceManager().add_source("app.py", SHADOWING), PLUGINS) + + assert located(findings) == EVAL_FINDINGS + + +def test_a_builtin_named_in_a_postponed_annotation_stays_the_builtin_in_a_project(tmp_path: Path) -> None: + (tmp_path / "app.py").write_text(SHADOWING, encoding="utf-8") + + assert located(engine.analyze_project(tmp_path, PLUGINS).findings) == EVAL_FINDINGS + + +def test_a_lambda_or_comprehension_in_a_variable_annotation_leaves_a_project_analysable(tmp_path: Path) -> None: + (tmp_path / "app.py").write_text( + "settings: (lambda: 1)\n\n\ndef run():\n pending: [v for v in ()]\n eval(input())\n", encoding="utf-8" + ) + + assert located(engine.analyze_project(tmp_path, PLUGINS).findings) == {("code-injection", 6), ("dangerous-eval", 6)} + + +def test_a_module_variable_annotation_binds_where_it_stands() -> None: + scopes = analyze("settings: [(found := i) for i in (1,)]\n") + + assert "found" in scopes.module_scope.bindings + assert child(scopes, scopes.module_scope, "").parent == scopes.module_scope.id + + +@pytest.mark.parametrize("header", ["", POSTPONED]) +def test_a_local_variable_annotation_declares_in_its_function(header: str) -> None: + scopes = analyze(header + "def run():\n pending: [(found := i) for i in (1,)]\n") + run = child(scopes, scopes.module_scope, "run") + + assert "found" in run.bindings + assert child(scopes, run, "").parent == run.id + + +def test_a_postponed_module_variable_annotation_binds_nothing() -> None: + scopes = analyze(POSTPONED + "settings: [(found := i) for i in (1,)]\n") + + assert "found" not in scopes.module_scope.bindings diff --git a/tests/test_annotation_expression_scopes.py b/tests/test_annotation_expression_scopes.py new file mode 100644 index 0000000..c85dce9 --- /dev/null +++ b/tests/test_annotation_expression_scopes.py @@ -0,0 +1,93 @@ +"""Regression coverage for lexical scopes created inside parameter annotations.""" + +from __future__ import annotations + +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +from coretrace_python.frontend import build_hir +from coretrace_python.semantic.scopes import ResolutionKind, ScopeKind, analyze_scopes +from coretrace_python.source import SourceManager + + +class AnnotationExpressionScopes(unittest.TestCase): + def _check_directory(self, contents: str) -> None: + with tempfile.TemporaryDirectory() as temporary: + project = Path(temporary) / "project" + project.mkdir() + (project / "app.py").write_text(contents, encoding="utf-8") + env = os.environ.copy() + result = subprocess.run( + [sys.executable, "-m", "coretrace_python", "--check", str(project)], + capture_output=True, + text=True, + timeout=15, + check=False, + env=env, + ) + self.assertEqual(result.returncode, 0, result.stderr[-1000:]) + self.assertNotIn("KeyError", result.stderr) + + def test_keyword_only_parameter_lambda_annotation_does_not_crash_project(self): + self._check_directory("def f(*, status: (lambda value: value) = None):\n return status\n") + + def test_positional_parameter_lambda_annotation_does_not_crash_project(self): + self._check_directory("def f(status: (lambda value: value) = None):\n return status\n") + + def test_parameter_comprehension_annotation_does_not_crash_project(self): + self._check_directory("entries = (1, 2)\ndef f(status: [v for v in entries]):\n return status\n") + + def test_annotation_scopes_belong_to_the_enclosing_module(self): + source = SourceManager().add_source( + "annotation_scopes.py", + "entries = (1, 2)\ndef f(first: (lambda x: x), second: [v for v in entries]):\n pass\n", + ) + hir = build_hir(source) + scopes = analyze_scopes(hir) + function = hir.body[1] + first, second = function.parameters + lambda_scope = scopes.scope_for(first.annotation) + comprehension_scope = scopes.scope_for(second.annotation) + self.assertEqual(lambda_scope.kind, ScopeKind.FUNCTION) + self.assertEqual(comprehension_scope.kind, ScopeKind.COMPREHENSION) + self.assertEqual(lambda_scope.parent, scopes.module_scope.id) + self.assertEqual(comprehension_scope.parent, scopes.module_scope.id) + + def test_varargs_annotations_create_scopes_in_the_enclosing_module(self): + source = SourceManager().add_source( + "varargs_scopes.py", + "def f(*args: (lambda x: x), **kwargs: [v for v in (1, 2)]):\n pass\n", + ) + hir = build_hir(source) + scopes = analyze_scopes(hir) + first, second = hir.body[0].parameters + self.assertEqual(scopes.scope_for(first.annotation).parent, scopes.module_scope.id) + self.assertEqual(scopes.scope_for(second.annotation).parent, scopes.module_scope.id) + + def test_nested_parameter_annotation_resolves_enclosing_local(self): + source = SourceManager().add_source( + "nested_annotations.py", + "def outer():\n" + " secret = 1\n" + " def inner(value: (lambda: secret)):\n" + " return value\n" + " return inner\n", + ) + hir = build_hir(source) + scopes = analyze_scopes(hir) + outer = hir.body[0] + inner = outer.body[1] + annotation_scope = scopes.scope_for(inner.parameters[0].annotation) + outer_scope = scopes.scope_for(outer) + self.assertEqual(annotation_scope.parent, outer_scope.id) + resolution = scopes.resolve(annotation_scope.id, "secret") + self.assertEqual(resolution.kind, ResolutionKind.FREE) + self.assertEqual(resolution.scope, outer_scope.id) + + +if __name__ == "__main__": + unittest.main()