From 67eac98c875d0db9db816bb194a0a46dcd72a860 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 5 Sep 2026 12:47:54 +0200 Subject: [PATCH 1/2] test: define assignment expressions, yield from and starred targets --- tests/test_frontend_gaps.py | 112 ++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 tests/test_frontend_gaps.py diff --git a/tests/test_frontend_gaps.py b/tests/test_frontend_gaps.py new file mode 100644 index 0000000..ca32550 --- /dev/null +++ b/tests/test_frontend_gaps.py @@ -0,0 +1,112 @@ +"""Acceptance tests for the syntax gaps found on large projects (issue #71). + +Assignment expressions (``(y := f(x))``), ``yield from`` and starred assignment targets +(``first, *rest = items``) used to reject the whole file as a ``syntax-error``. They are +now part of the supported subset: a walrus is laid out by the CFG builder as an +assignment before the statement that reads it, recomputed every iteration in a ``while`` +condition; ``yield from`` is a delegating yield; a starred target receives the slice of +the unpacked value that the other targets leave. + +Expected to remain red until the HIR has ``NamedExpr`` and the adapter accepts the three forms. +""" + +from __future__ import annotations + +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.hir import nodes +from coretrace_python.source import SourceManager + +MISSING = None if hasattr(nodes, "NamedExpr") else "nodes.NamedExpr is missing" + + +@pytest.fixture(autouse=True) +def require_gaps() -> None: + if MISSING is not None: + pytest.fail(f"the frontend gaps are not closed yet: {MISSING}") + + +REPO = Path(__file__).resolve().parent.parent +PLUGINS = REPO / "src" / "coretrace_python" / "bundled" + + +def hir(text: str) -> nodes.Module: + return build_hir(SourceManager().add_source("m.py", text)) + + +def check(text: str) -> tuple[Finding, ...]: + return engine.check(SourceManager().add_source("m.py", text), [PLUGINS]) + + +def rules(findings: tuple[Finding, ...]) -> list[str]: + return sorted(f.rule_id for f in findings) + + +# --------------------------------------------------------------------------- HIR + + +def test_the_three_forms_build_a_hir() -> None: + module = hir("def f(items, g):\n first, *rest = items\n total = yield from g()\n if (n := len(rest)):\n return n\n") + function = module.body[0] + assert isinstance(function, nodes.Function) + unpack, delegate, branch = function.body + + assert isinstance(unpack, nodes.Assign) and isinstance(unpack.target, nodes.Tuple) + assert isinstance(unpack.target.elements[1], nodes.Starred) + assert isinstance(unpack.target.elements[1].value, nodes.Name) + assert isinstance(delegate, nodes.Assign) and isinstance(delegate.value, nodes.Yield) + assert delegate.value.delegate is True + assert isinstance(branch, nodes.If) and isinstance(branch.condition, nodes.NamedExpr) + assert branch.condition.target.identifier == "n" + + +# --------------------------------------------------------------------------- analysis + + +def test_walrus_in_a_condition_is_analysed_and_carries_taint() -> None: + findings = check("import os\n\ndef run():\n if (cmd := input()):\n os.system(cmd)\n") + + assert rules(findings) == ["command-injection"] + + +def test_walrus_in_a_while_condition_is_recomputed_each_iteration() -> None: + findings = check("import os\n\ndef run():\n while (line := input()):\n os.system(line)\n") + + assert rules(findings) == ["command-injection"] + + +def test_walrus_inside_a_comprehension_binds_in_the_function() -> None: + findings = check("import os\n\ndef run(xs):\n ys = [y for x in xs if (y := input())]\n os.system(y)\n") + + assert rules(findings) == ["command-injection"] + + +def test_yield_from_is_a_delegating_yield() -> None: + findings = check("import os\n\ndef run(gen):\n data = yield from gen\n os.system(input())\n") + + assert rules(findings) == ["command-injection"] + + +def test_starred_targets_receive_the_remaining_elements() -> None: + findings = check( + "import os\n\ndef run():\n first, *rest, last = [input(), 'x', 'y']\n os.system(rest[0])\n os.system(first)\n" + ) + + assert rules(findings) == ["command-injection", "command-injection"] + + +def test_the_forms_leave_coverage_complete() -> None: + analysis = engine.analyze_file( + SourceManager().add_source( + "m.py", "def a(items):\n x, *y = items\n\ndef b(g):\n yield from g\n\ndef c(v):\n return (w := v)\n" + ), + [PLUGINS], + ) + + assert analysis.coverage.summary() == "coverage: 1/1 files, 3/3 functions" + assert analysis.findings == () From 3c6ae23b413662494faae26a71d228430fcedaf9 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sat, 5 Sep 2026 12:47:54 +0200 Subject: [PATCH 2/2] feat: support assignment expressions, yield from and starred targets --- docs/usage.md | 6 +- src/coretrace_python/cfg/builder.py | 9 +- src/coretrace_python/frontend/ast_adapter.py | 13 +- src/coretrace_python/hir/nodes.py | 14 ++ src/coretrace_python/ir/lowering.py | 30 +++- src/coretrace_python/semantic/scopes.py | 9 + tests/regression/expected/healthchecks.json | 172 +++++++++---------- tests/regression/expected/yt-fts.json | 12 +- tests/test_django_http_clients.py | 2 +- tests/test_hir_builder.py | 6 +- 10 files changed, 156 insertions(+), 117 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 19a3c2d..b6f01a5 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -363,8 +363,10 @@ assignment, list, tuple, set and dict literals with unpacking, f-strings, slices boolean operators, chained comparisons, keyword and starred arguments, `with`, `assert`, `try`/`except`/`else`/`finally`, `await`, `yield`, `if`, `while` and `for` with their `else` clauses, `break`, `continue`, `raise` with `from`, conditional expressions, -comprehensions, lambdas, nested functions and classes, `global`, `nonlocal`, `del` and -`match` with every pattern kind. `finally` is modelled on the normal path only and a +comprehensions, lambdas, nested functions and classes, `global`, `nonlocal`, `del`, +assignment expressions (`(y := f(x))`, laid out as an assignment before the statement), +`yield from`, starred assignment targets (`first, *rest = items`) and `match` with every +pattern kind. `finally` is modelled on the normal path only and a `nonlocal` write stays inside the nested function. Methods of module-level classes are analysed like functions. diff --git a/src/coretrace_python/cfg/builder.py b/src/coretrace_python/cfg/builder.py index f0c46a3..7a16566 100644 --- a/src/coretrace_python/cfg/builder.py +++ b/src/coretrace_python/cfg/builder.py @@ -129,6 +129,9 @@ def hoist(self, node: Any, pending: list[nodes.Statement]) -> Any: return result if isinstance(node, nodes.Comprehension): return self.comprehension(node, pending) + if isinstance(node, nodes.NamedExpr): + pending.append(nodes.Assign(node.target, self.hoist(node.value, pending), node.span)) + return node.target if isinstance(node, nodes.Lambda): defaults = tuple( replace(p, default=self.hoist(p.default, pending)) if p.default is not None else p @@ -481,7 +484,7 @@ def _may_hold_expressions(value: object) -> bool: def _has_control_flow(node: object) -> bool: - if isinstance(node, nodes.Conditional | nodes.Comprehension): + if isinstance(node, nodes.Conditional | nodes.Comprehension | nodes.NamedExpr): return True if isinstance(node, nodes.Lambda): return False @@ -492,9 +495,11 @@ def _has_control_flow(node: object) -> bool: return False -def _bound_names(target: nodes.Target) -> list[str]: +def _bound_names(target: nodes.Target | nodes.Starred) -> list[str]: if isinstance(target, nodes.Name): return [target.identifier] + if isinstance(target, nodes.Starred): + return _bound_names(target.value) # type: ignore[arg-type] if isinstance(target, nodes.Tuple): return [name for element in target.elements for name in _bound_names(element)] # type: ignore[arg-type] return [] diff --git a/src/coretrace_python/frontend/ast_adapter.py b/src/coretrace_python/frontend/ast_adapter.py index 91860c2..6f4b6eb 100644 --- a/src/coretrace_python/frontend/ast_adapter.py +++ b/src/coretrace_python/frontend/ast_adapter.py @@ -146,7 +146,11 @@ def expression(self, node: ast.expr) -> nodes.Expression: yielded = self.expression(node.value) if node.value is not None else None return nodes.Yield(yielded, span) if isinstance(node, ast.YieldFrom): - self.fail(node, "yield from is not supported yet") + return nodes.Yield(self.expression(node.value), span, True) + if isinstance(node, ast.NamedExpr): + assert isinstance(node.target, ast.Name) + target = nodes.Name(node.target.id, self.span(node.target)) + return nodes.NamedExpr(target, self.expression(node.value), span) if isinstance(node, (ast.ListComp, ast.SetComp, ast.GeneratorExp)): generators = tuple(self.generator(generator) for generator in node.generators) return nodes.Comprehension( @@ -178,10 +182,13 @@ def target(self, node: ast.expr) -> nodes.Target: assert isinstance(target, (nodes.Attribute, nodes.Subscript)) return target if isinstance(node, (ast.Tuple, ast.List)): + elements: list[nodes.Expression] = [] for element in node.elts: if isinstance(element, ast.Starred): - self.fail(element, "starred assignment targets are not supported yet") - return nodes.Tuple(tuple(self.target(element) for element in node.elts), self.span(node)) + elements.append(nodes.Starred(self.target(element.value), self.span(element))) + else: + elements.append(self.target(element)) + return nodes.Tuple(tuple(elements), self.span(node)) self.fail(node, f"unsupported assignment target: {type(node).__name__}") def generator(self, node: ast.comprehension) -> nodes.ComprehensionGenerator: diff --git a/src/coretrace_python/hir/nodes.py b/src/coretrace_python/hir/nodes.py index cc36934..35c418e 100644 --- a/src/coretrace_python/hir/nodes.py +++ b/src/coretrace_python/hir/nodes.py @@ -163,8 +163,21 @@ class Await: @dataclass(frozen=True, slots=True) class Yield: + """``yield value``; with ``delegate``, ``yield from value``.""" + value: Expression | None span: SourceSpan + delegate: bool = False + + +@dataclass(frozen=True, slots=True) +class NamedExpr: + """``(target := value)``: binds ``target`` in the enclosing function and evaluates to + the value. The CFG builder lays it out as an assignment before its statement.""" + + target: Name + value: Expression + span: SourceSpan @dataclass(frozen=True, slots=True) @@ -210,6 +223,7 @@ class Comprehension: | Lambda | Await | Yield + | NamedExpr | Comprehension ) diff --git a/src/coretrace_python/ir/lowering.py b/src/coretrace_python/ir/lowering.py index 1b2722d..dcfee6d 100644 --- a/src/coretrace_python/ir/lowering.py +++ b/src/coretrace_python/ir/lowering.py @@ -240,11 +240,29 @@ def store(self, target: nodes.Target, value: Value) -> None: key = self.expression(target.key) self.emit_effect(SetItem(None, target.span, object_value, key, value)) else: - for index, element in enumerate(target.elements): - position = self.emit(Constant(self.new_value(), element.span, index)) + self.unpack(target, value) + + def unpack(self, target: nodes.Tuple, value: Value) -> None: + """Store each element of ``value`` into the targets; a starred target receives + the slice the others leave, and the targets after it index from the end.""" + + count = len(target.elements) + starred = next((i for i, e in enumerate(target.elements) if isinstance(e, nodes.Starred)), None) + for index, element in enumerate(target.elements): + if isinstance(element, nodes.Starred): + lower = self.emit(Constant(self.new_value(), element.span, index)) + upper = None + if index < count - 1: + upper = self.emit(Constant(self.new_value(), element.span, index - count + 1)) + bounds = self.emit(BuildSlice(self.new_value(), element.span, lower, upper, None)) + item = self.emit(GetItem(self.new_value(), element.span, value, bounds)) + element = element.value + else: + offset = index if starred is None or index < starred else index - count + position = self.emit(Constant(self.new_value(), element.span, offset)) item = self.emit(GetItem(self.new_value(), element.span, value, position)) - assert isinstance(element, nodes.Name | nodes.Attribute | nodes.Subscript | nodes.Tuple) - self.store(element, item) + assert isinstance(element, nodes.Name | nodes.Attribute | nodes.Subscript | nodes.Tuple) + self.store(element, item) def statement(self, node: nodes.Statement) -> None: if isinstance(node, nodes.Assign): @@ -466,9 +484,11 @@ def _reassigned_parameters(function: nodes.Function) -> frozenset[str]: assigned: set[str] = set() - def names(target: nodes.Target | None) -> None: + def names(target: nodes.Target | nodes.Starred | None) -> None: if isinstance(target, nodes.Name): assigned.add(target.identifier) + elif isinstance(target, nodes.Starred): + names(target.value) # type: ignore[arg-type] elif isinstance(target, nodes.Tuple): for element in target.elements: names(element) # type: ignore[arg-type] diff --git a/src/coretrace_python/semantic/scopes.py b/src/coretrace_python/semantic/scopes.py index 6a66fab..483ae87 100644 --- a/src/coretrace_python/semantic/scopes.py +++ b/src/coretrace_python/semantic/scopes.py @@ -351,6 +351,13 @@ def expression(self, node: nodes.Expression, scope: _ScopeBuilder) -> None: elif isinstance(node, nodes.Yield): if node.value is not None: self.expression(node.value, scope) + elif isinstance(node, nodes.NamedExpr): + self.expression(node.value, scope) + # PEP 572: a walrus inside a comprehension binds in the containing scope. + 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) elif isinstance(node, nodes.Tuple | nodes.List): for element in node.elements: self.expression(element, scope) @@ -419,6 +426,8 @@ def target(self, node: nodes.Target, scope: _ScopeBuilder) -> None: scope.bind(node.identifier, BindingKind.LOCAL, node.span) elif isinstance(node, nodes.Tuple): for element in node.elements: + if isinstance(element, nodes.Starred): + element = element.value assert isinstance(element, nodes.Name | nodes.Attribute | nodes.Subscript | nodes.Tuple) self.target(element, scope) else: diff --git a/tests/regression/expected/healthchecks.json b/tests/regression/expected/healthchecks.json index 8cb76fa..4f9f090 100644 --- a/tests/regression/expected/healthchecks.json +++ b/tests/regression/expected/healthchecks.json @@ -1,9 +1,9 @@ { "coverage": { "files": 653, - "files_analysed": 636, - "functions": 2566, - "functions_analysed": 2566 + "files_analysed": 653, + "functions": 2746, + "functions_analysed": 2746 }, "findings": [ { @@ -24,24 +24,6 @@ "rule": "open-redirect", "function": "wrapper" }, - { - "file": "hc/accounts/middleware.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, - { - "file": "hc/accounts/migrations/0049_convert_email_lowercase.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, - { - "file": "hc/accounts/models.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "hc/accounts/tests/test_createsuperuser.py", "line": 40, @@ -86,15 +68,69 @@ }, { "file": "hc/accounts/views.py", - "line": 1, - "rule": "syntax-error", - "function": null + "line": 115, + "rule": "open-redirect", + "function": "_redirect_after_login" }, { - "file": "hc/api/management/commands/sendalerts.py", - "line": 1, - "rule": "syntax-error", - "function": null + "file": "hc/accounts/views.py", + "line": 120, + "rule": "open-redirect", + "function": "_redirect_after_login" + }, + { + "file": "hc/accounts/views.py", + "line": 142, + "rule": "open-redirect", + "function": "_check_2fa" + }, + { + "file": "hc/accounts/views.py", + "line": 170, + "rule": "open-redirect", + "function": "login" + }, + { + "file": "hc/accounts/views.py", + "line": 214, + "rule": "xss", + "function": "signup_csrf" + }, + { + "file": "hc/accounts/views.py", + "line": 281, + "rule": "open-redirect", + "function": "check_token" + }, + { + "file": "hc/accounts/views.py", + "line": 281, + "rule": "open-redirect", + "function": "check_token" + }, + { + "file": "hc/accounts/views.py", + "line": 314, + "rule": "hardcoded-credential", + "function": "profile" + }, + { + "file": "hc/accounts/views.py", + "line": 358, + "rule": "open-redirect", + "function": "add_project" + }, + { + "file": "hc/accounts/views.py", + "line": 637, + "rule": "open-redirect", + "function": "change_email_verify" + }, + { + "file": "hc/accounts/views.py", + "line": 637, + "rule": "open-redirect", + "function": "change_email_verify" }, { "file": "hc/api/models.py", @@ -151,16 +187,22 @@ "function": null }, { - "file": "hc/api/transports.py", - "line": 1, - "rule": "syntax-error", - "function": null + "file": "hc/api/views.py", + "line": 645, + "rule": "xss", + "function": "ping_body" }, { "file": "hc/api/views.py", - "line": 1, - "rule": "syntax-error", - "function": null + "line": 778, + "rule": "xss", + "function": "badge" + }, + { + "file": "hc/api/views.py", + "line": 811, + "rule": "xss", + "function": "check_badge" }, { "file": "hc/front/tests/test_webhook_validator.py", @@ -468,18 +510,6 @@ "rule": "open-redirect", "function": "save" }, - { - "file": "hc/integrations/googlechat/tests/test_notify.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, - { - "file": "hc/integrations/googlechat/transport.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "hc/integrations/googlechat/views.py", "line": 33, @@ -552,12 +582,6 @@ "rule": "open-redirect", "function": "add" }, - { - "file": "hc/integrations/msteamsw/transport.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "hc/integrations/msteamsw/views.py", "line": 29, @@ -618,12 +642,6 @@ "rule": "open-redirect", "function": "add" }, - { - "file": "hc/integrations/opsgenie/transport.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "hc/integrations/opsgenie/views.py", "line": 31, @@ -636,12 +654,6 @@ "rule": "open-redirect", "function": "add" }, - { - "file": "hc/integrations/pd/transport.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "hc/integrations/pd/views.py", "line": 52, @@ -708,12 +720,6 @@ "rule": "open-redirect", "function": "add_complete" }, - { - "file": "hc/integrations/rocketchat/transport.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "hc/integrations/rocketchat/views.py", "line": 34, @@ -756,12 +762,6 @@ "rule": "open-redirect", "function": "add_signal" }, - { - "file": "hc/integrations/slack/transport.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "hc/integrations/slack/views.py", "line": 37, @@ -812,9 +812,9 @@ }, { "file": "hc/integrations/telegram/views.py", - "line": 1, - "rule": "syntax-error", - "function": null + "line": 124, + "rule": "open-redirect", + "function": "add" }, { "file": "hc/integrations/trello/tests/test_notify.py", @@ -893,18 +893,6 @@ "line": 29, "rule": "open-redirect", "function": "add" - }, - { - "file": "hc/settings.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, - { - "file": "hc/urls.py", - "line": 1, - "rule": "syntax-error", - "function": null } ] } diff --git a/tests/regression/expected/yt-fts.json b/tests/regression/expected/yt-fts.json index d7408a2..f331bfc 100644 --- a/tests/regression/expected/yt-fts.json +++ b/tests/regression/expected/yt-fts.json @@ -1,17 +1,11 @@ { "coverage": { "files": 20, - "files_analysed": 19, - "functions": 141, - "functions_analysed": 141 + "files_analysed": 20, + "functions": 147, + "functions_analysed": 147 }, "findings": [ - { - "file": "src/yt_fts/llm/get_embeddings.py", - "line": 1, - "rule": "syntax-error", - "function": null - }, { "file": "tests/test_export.py", "line": 48, diff --git a/tests/test_django_http_clients.py b/tests/test_django_http_clients.py index 97d2725..86fee8e 100644 --- a/tests/test_django_http_clients.py +++ b/tests/test_django_http_clients.py @@ -91,7 +91,7 @@ def test_parameter_annotations_are_kept() -> None: def test_unsupported_annotations_do_not_break_the_build() -> None: - module = module_for("def f(x: (n := 1), y: 'Request'):\n return x\n") + module = module_for("async def f(x: [a async for a in b], y: 'Request'):\n return x\n") function = module.body[0] assert isinstance(function, nodes.Function) assert function.parameters[0].annotation is None diff --git a/tests/test_hir_builder.py b/tests/test_hir_builder.py index 8e0ed77..77a3b8b 100644 --- a/tests/test_hir_builder.py +++ b/tests/test_hir_builder.py @@ -209,10 +209,10 @@ def test_represents_raise_with_and_without_exception() -> None: @pytest.mark.parametrize( "source_text, message", [ - ("def f(x):\n yield from x\n", "yield from"), - ("a, *rest = items\n", "starred"), + ("async def f(x):\n return [a async for a in x]\n", "async comprehensions"), + ("try:\n pass\nexcept* ValueError:\n pass\n", "except\\*"), ], - ids=["yield-from", "starred-target"], + ids=["async-comprehension", "exception-group"], ) def test_unsupported_control_flow_forms_are_reported(source_text: str, message: str) -> None: from coretrace_python.frontend import HIRBuildError