From 2a912cbbea2856d5e8bf3e21ae512534dc4d6d5c Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 23 Sep 2026 10:11:00 +0100 Subject: [PATCH 1/8] feat(cfg): assert the Python grammar vocabulary before building against it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M4 Step 0. ADR-0002 names node kinds and field names in prose, and prose is not a measurement. This probe parses one corpus exercising every construct in the ADR's table and asserts the kinds and fields the CFG builder will depend on, so a grammar bump that renames or removes one fails here, naming the thing that moved, rather than silently turning a CFG edge into a no-op. Running it overturned two claims the builder would otherwise have been written against. Both came from reading the grammar's generated `node-types.json` during planning rather than from parsing: * `except*` does NOT produce an error node. tree-sitter-python 0.25.0 accepts PEP 654 syntax and demotes the `*` to an anonymous child, leaving a named tree shape identical to plain `except`. `has_error()` is false, so the planned OPAQUE trigger does not cover it — and misreading a group handler as an ordinary one is worse than refusing the function, since the two have genuinely different control flow. Hence `is_except_group`, a structural detector, with both arms asserted and two mutants killed against it. * The `except E as e` binding is NOT `except_clause.alias`, despite `node-types.json` advertising exactly that field. The parser emits `value: (as_pattern (identifier) alias: (as_pattern_target ...))`, one level down. A builder written from the schema would have got `None`, never bound `e`, and turned every aliased handler into a `read-before-init` false positive on `e`. Also asserted, each with a paired control so no leg can pass vacuously: try/with/delete expose no fields the builder could use, so their parts must be matched by kind; `while/else` exists though the ADR table omits it; a bare annotation `x: int` carries `type` but no `right` and so binds nothing. No builder code yet beyond the one detector the measurements forced. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/cfg/grammar_probe.rs | 551 ++++++++++++++++++++++ crates/pons-core/src/cfg/mod.rs | 50 ++ crates/pons-core/src/lib.rs | 1 + 3 files changed, 602 insertions(+) create mode 100644 crates/pons-core/src/cfg/grammar_probe.rs create mode 100644 crates/pons-core/src/cfg/mod.rs diff --git a/crates/pons-core/src/cfg/grammar_probe.rs b/crates/pons-core/src/cfg/grammar_probe.rs new file mode 100644 index 0000000..46c3f11 --- /dev/null +++ b/crates/pons-core/src/cfg/grammar_probe.rs @@ -0,0 +1,551 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! What the pinned Python grammar actually produces. +//! +//! ADR-0002 is the design authority for the CFG, and it names node kinds and +//! field names in prose. Prose is not a measurement. This probe parses one +//! corpus exercising every construct in the ADR's table and asserts the kinds +//! and fields the builder will depend on, so that a grammar bump which renames +//! or removes one of them fails *here*, naming the thing that moved, rather +//! than silently turning a CFG edge into a no-op. +//! +//! Several assertions below record facts ADR-0002 does **not** state, each of +//! which changes what the builder must do. They are called out in the test +//! names rather than buried in comments. Two of them contradict the plan +//! outright and were found by running this probe: +//! +//! * `except*` does **not** produce an error node (see +//! [`except_star_parses_clean_so_has_error_cannot_be_its_opaque_trigger`]); +//! * the `except … as e` binding is **not** on `except_clause.alias` (see +//! [`the_except_as_binding_is_nested_in_an_as_pattern_not_a_field_of_the_clause`]), +//! even though the grammar's own `node-types.json` advertises such a field — +//! a reminder that the generated schema is not what the parser emits. +//! +//! The corpus is an embedded raw string rather than a `.py` file on purpose: +//! Python is banned outside `fixtures/` estate-wide, and a probe corpus is not +//! a fixture. + +use std::collections::BTreeSet; + +use tree_sitter::{Node, Parser, Tree}; + +use super::is_except_group; +use crate::lang::Lang; + +/// Every construct in ADR-0002's table, in one parse-clean module. +const CORPUS: &str = r#" +import os +from sys import path as syspath + +GLOBAL_NAME = 0 + + +@decorator +class Widget: + kind: str = "widget" + + def method(self, a, b=1, *args, **kwargs): + return a + b + + +def plain(a, b): + total: int = 0 + annotated_only: int + total += a + + if a > b: + total = a + elif a == b: + total = b + else: + total = 0 + + while total > 0: + total -= 1 + if total == 3: + break + continue + else: + total = -1 + + for item in range(a): + total += item + else: + total += 100 + + try: + risky = compute(a) + except ValueError as exc: + total = len(str(exc)) + except (TypeError, KeyError): + total = 0 + else: + total += risky + finally: + cleanup() + + with open("f") as handle: + data = handle.read() + + del data + + match total: + case 0: + label = "zero" + case [first, *rest] if first > 0: + label = "positive-head" + case {"key": value}: + label = value + case Widget(kind=k): + label = k + case _: + label = "other" + + squares = [n * n for n in range(a) if n] + gen = (n for n in range(b)) + pairs = {k: v for k, v in items} + uniq = {n for n in range(a)} + + if (found := lookup(a)) is not None: + total += found + + fn = lambda x: x + 1 + assert total >= 0, "non-negative" + raise SystemExit(total) + + +async def fetch(url): + async with session() as s: + async for chunk in s.stream(url): + yield chunk + result = await s.get(url) + return result + + +def generator(): + yield 1 + received = yield + return received + + +def scopes(): + global GLOBAL_NAME + captured = 1 + + def inner(): + nonlocal captured + captured = 2 + + if GLOBAL_NAME: + pass + + inner() + return captured +"#; + +fn parse_python(text: &str) -> Tree { + let mut parser = Parser::new(); + parser + .set_language(&Lang::Python.tree_sitter_language()) + .expect("the pinned tree-sitter-python grammar loads"); + parser + .parse(text, None) + .expect("tree-sitter produced a tree") +} + +/// Every *named* kind appearing anywhere in the tree. +fn kinds(tree: &Tree) -> BTreeSet { + let mut found = BTreeSet::new(); + let mut cursor = tree.walk(); + let mut stack = vec![tree.root_node()]; + while let Some(node) = stack.pop() { + if node.is_named() { + found.insert(node.kind().to_string()); + } + for child in node.children(&mut cursor) { + stack.push(child); + } + } + found +} + +/// Depth-first search for the first node of `kind`. +fn find<'t>(root: Node<'t>, kind: &str) -> Option> { + find_where(root, &|n| n.kind() == kind) +} + +fn find_where<'t>(root: Node<'t>, pred: &dyn Fn(Node<'t>) -> bool) -> Option> { + if pred(root) { + return Some(root); + } + let mut cursor = root.walk(); + for child in root.children(&mut cursor) { + if let Some(hit) = find_where(child, pred) { + return Some(hit); + } + } + None +} + +/// Which of `candidates` this node actually exposes, in the order given. +fn field_names(node: Node<'_>, candidates: &[&str]) -> Vec { + candidates + .iter() + .filter(|name| node.child_by_field_name(*name).is_some()) + .map(|name| (*name).to_string()) + .collect() +} + +fn handlers_of_the_corpus_try(tree: &Tree) -> Vec> { + let try_stmt = find(tree.root_node(), "try_statement").expect("corpus has a try"); + let mut cursor = try_stmt.walk(); + try_stmt + .children(&mut cursor) + .filter(|c| c.kind() == "except_clause") + .collect() +} + +#[test] +fn the_probe_corpus_parses_without_error() { + // The positive control for every other test in this file. If the corpus + // itself is malformed, "kind absent" below would mean "I wrote bad Python", + // not "the grammar changed". + let tree = parse_python(CORPUS); + assert!( + !tree.root_node().has_error(), + "the probe corpus must parse clean, or every assertion below is measuring my typo" + ); +} + +#[test] +fn every_node_kind_the_builder_depends_on_is_produced_by_this_grammar() { + let tree = parse_python(CORPUS); + let present = kinds(&tree); + + let expected = [ + // units + "module", + "function_definition", + "lambda", + "class_definition", + "block", + "parameters", + "decorated_definition", + // branches + "if_statement", + "elif_clause", + "else_clause", + // loops + "while_statement", + "for_statement", + // exceptions + "try_statement", + "except_clause", + "finally_clause", + "raise_statement", + // the `as` binding, shared by `except` and `with` + "as_pattern", + "as_pattern_target", + // context managers + "with_statement", + "with_clause", + "with_item", + // binding and unbinding + "assignment", + "augmented_assignment", + "delete_statement", + "named_expression", + "global_statement", + "nonlocal_statement", + // structural pattern matching + "match_statement", + "case_clause", + // comprehensions + "list_comprehension", + "generator_expression", + "dictionary_comprehension", + "set_comprehension", + "for_in_clause", + // jumps + "return_statement", + "break_statement", + "continue_statement", + "pass_statement", + // suspensions + "yield", + "await", + // misc the builder walks through + "expression_statement", + "call", + "identifier", + "attribute", + "assert_statement", + "import_statement", + "import_from_statement", + ]; + + let missing: Vec<&str> = expected + .iter() + .copied() + .filter(|k| !present.contains(*k)) + .collect(); + + assert!( + missing.is_empty(), + "the pinned grammar no longer produces these kinds, which the CFG builder depends on: {missing:?}" + ); +} + +#[test] +fn try_with_and_delete_hide_their_parts_from_child_by_field_name() { + // ⚠ ADR-0002 does not state this, and it decides how the builder is written. + // `try_statement` exposes only `body` as a named field; `with_statement` + // only `body`; `delete_statement` exposes nothing at all. So handlers, + // `else`, `finally`, the `with` items and the `del` targets must be found + // by iterating children and matching on *kind*. A builder reaching for + // `child_by_field_name("handlers")` gets `None` and silently drops every + // exception edge — a whole analysis quietly turning into a no-op. + let tree = parse_python(CORPUS); + let root = tree.root_node(); + + let try_stmt = find(root, "try_statement").expect("corpus has a try"); + assert_eq!( + field_names( + try_stmt, + &[ + "body", + "handlers", + "alternative", + "finalizer", + "else", + "finally" + ] + ), + vec!["body".to_string()], + "try_statement exposes a field the builder could have used" + ); + // ...but the parts are all reachable by kind. + assert!(find(try_stmt, "except_clause").is_some()); + assert!(find(try_stmt, "finally_clause").is_some()); + assert!(find(try_stmt, "else_clause").is_some()); + + let with_stmt = find(root, "with_statement").expect("corpus has a with"); + assert_eq!( + field_names(with_stmt, &["body", "items", "clause", "alias"]), + vec!["body".to_string()], + "with_statement exposes a field the builder could have used" + ); + assert!(find(with_stmt, "with_item").is_some()); + + let del = find(root, "delete_statement").expect("corpus has a del"); + assert_eq!( + field_names(del, &["target", "targets", "left", "value", "body"]), + Vec::::new(), + "delete_statement exposes a field the builder could have used" + ); + assert!( + del.named_child_count() >= 1, + "the del target must still be reachable positionally" + ); +} + +#[test] +fn while_else_exists_even_though_the_adr_table_omits_it() { + // ⚠ Gap-fill. ADR-0002's table gives `for/else` and is silent on + // `while/else`. The grammar has it, so the builder must mirror the + // `for/else` edge shape or the else-block becomes unreachable in the CFG + // and every store in it reads as dead. + let tree = parse_python(CORPUS); + let while_stmt = find(tree.root_node(), "while_statement").expect("corpus has a while/else"); + + assert!( + while_stmt.child_by_field_name("alternative").is_some(), + "while_statement lost its `alternative` field; while/else edges need rewriting" + ); + assert_eq!( + field_names(while_stmt, &["condition", "body", "alternative"]), + vec![ + "condition".to_string(), + "body".to_string(), + "alternative".to_string() + ] + ); +} + +#[test] +fn a_bare_annotation_is_an_assignment_that_binds_nothing() { + // ⚠ Gap-fill, and a false-negative source if missed. `x: int` parses as an + // `assignment` carrying `left` and `type` but **no** `right`. It declares a + // type; it does not bind a value. Counting it as a definition would make + // `read-before-init` silently miss every read of an annotated-but-unassigned + // local — the exact bug the rule exists to find. + let tree = parse_python(CORPUS); + let root = tree.root_node(); + + let bare = find_where(root, &|n: Node<'_>| { + n.kind() == "assignment" + && n.child_by_field_name("left") + .and_then(|l| l.utf8_text(CORPUS.as_bytes()).ok()) + == Some("annotated_only") + }) + .expect("corpus has a bare annotation"); + + assert!( + bare.child_by_field_name("type").is_some(), + "the bare annotation lost its `type` field" + ); + assert!( + bare.child_by_field_name("right").is_none(), + "a bare annotation must not carry `right`, or it would read as a binding" + ); + + // Paired control: the annotated *assignment* two lines above does bind, and + // carries all three. Without this leg the assertion above would pass just as + // happily on a grammar that had dropped `right` everywhere. + let bound = find_where(root, &|n: Node<'_>| { + n.kind() == "assignment" + && n.child_by_field_name("left") + .and_then(|l| l.utf8_text(CORPUS.as_bytes()).ok()) + == Some("total") + && n.child_by_field_name("type").is_some() + }) + .expect("corpus has an annotated assignment that binds"); + + assert!( + bound.child_by_field_name("right").is_some(), + "`total: int = 0` must carry `right` — it binds" + ); +} + +#[test] +fn except_star_parses_clean_so_has_error_cannot_be_its_opaque_trigger() { + // ⚠⚠ MEASURED, AND IT CONTRADICTS THE PLAN. ADR-0002's implementation plan + // assumed PEP 654 `except*` is unrepresentable in the pinned grammar and so + // would land in an ERROR node, caught by the `has_error()` OPAQUE trigger. + // It does not. `tree-sitter-python` 0.25.0 accepts `except*` and demotes the + // `*` to an anonymous child, leaving a *named* tree shape indistinguishable + // from plain `except`. `has_error()` is false. + // + // That makes this the dangerous case rather than the safe one: a group + // handler has genuinely different control flow from an ordinary one, so a + // builder that cannot tell them apart emits a confident, plausible, wrong + // CFG instead of refusing the function. Hence the dedicated structural + // detector, asserted here against both arms. + let starred = parse_python("try:\n pass\nexcept* ValueError:\n pass\n"); + assert!( + !starred.root_node().has_error(), + "`except*` now produces an error node — the has_error() trigger would cover it, \ + and is_except_group() may be redundant; re-check before deleting it" + ); + assert!( + !kinds(&starred).contains("except_group_clause"), + "the grammar gained a dedicated `except_group_clause`; the detector should use it" + ); + + let starred_clause = + find(starred.root_node(), "except_clause").expect("parsed as except_clause"); + assert!( + is_except_group(starred_clause), + "the `*` must remain detectable as an anonymous child, or `except*` becomes \ + invisible and gets analysed as an ordinary handler" + ); + + // Paired control: an ordinary handler must NOT trip the detector, or every + // try/except in the corpus would be forced opaque and both T1 rules would + // go vacuously silent while still passing their own tests. + let plain = parse_python("try:\n pass\nexcept ValueError:\n pass\n"); + let plain_clause = find(plain.root_node(), "except_clause").expect("parsed as except_clause"); + assert!( + !is_except_group(plain_clause), + "an ordinary `except` must not read as a group handler" + ); + for handler in handlers_of_the_corpus_try(&parse_python(CORPUS)) { + assert!( + !is_except_group(handler), + "no handler in the probe corpus is a group handler" + ); + } +} + +#[test] +fn the_except_as_binding_is_nested_in_an_as_pattern_not_a_field_of_the_clause() { + // ⚠⚠ MEASURED, AND `node-types.json` IS MISLEADING HERE. The generated + // schema advertises an `alias` field on `except_clause`, so a builder + // written from the schema would call + // `except_clause.child_by_field_name("alias")`, get `None`, and silently + // never bind `e` at handler entry — turning every `except E as e` into a + // `read-before-init` false positive on `e`. + // + // What the parser actually emits is + // (except_clause value: (as_pattern (identifier) + // alias: (as_pattern_target (identifier)))) + // so the binding is one level down, inside `value`. + let tree = parse_python(CORPUS); + let handlers = handlers_of_the_corpus_try(&tree); + assert_eq!( + handlers.len(), + 2, + "corpus has one aliased and one bare handler" + ); + + let aliased = handlers[0]; + assert!( + aliased.child_by_field_name("alias").is_none(), + "the schema's `alias` field is not what the parser emits here; if this starts \ + passing, simplify the binding lookup below" + ); + + let value = aliased + .child_by_field_name("value") + .expect("an aliased handler exposes `value`"); + assert_eq!( + value.kind(), + "as_pattern", + "`except E as e` wraps its exception type in an as_pattern" + ); + let target = value + .child_by_field_name("alias") + .expect("the as_pattern carries the binding in `alias`"); + assert_eq!(target.kind(), "as_pattern_target"); + assert_eq!(target.utf8_text(CORPUS.as_bytes()).unwrap(), "exc"); + + // Paired control: a bare handler binds nothing, and its `value` is the + // exception tuple itself rather than an as_pattern. + let bare = handlers[1]; + let bare_value = bare + .child_by_field_name("value") + .expect("a bare handler still exposes `value`"); + assert_ne!( + bare_value.kind(), + "as_pattern", + "`except (TypeError, KeyError):` binds no name" + ); +} + +#[test] +fn the_fields_the_builder_will_use_are_all_present() { + let tree = parse_python(CORPUS); + let root = tree.root_node(); + + let cases: &[(&str, &[&str])] = &[ + ("for_statement", &["left", "right", "body", "alternative"]), + ("if_statement", &["condition", "consequence", "alternative"]), + ("function_definition", &["name", "parameters", "body"]), + ("lambda", &["parameters", "body"]), + ("named_expression", &["name", "value"]), + ("for_in_clause", &["left", "right"]), + ("case_clause", &["consequence"]), + ("match_statement", &["subject", "body"]), + ("augmented_assignment", &["left", "operator", "right"]), + ]; + + for (kind, fields) in cases { + let node = find(root, kind).unwrap_or_else(|| panic!("corpus has no {kind}")); + let present = field_names(node, fields); + assert_eq!( + present.len(), + fields.len(), + "{kind} is missing fields the builder uses: expected {fields:?}, found {present:?}" + ); + } +} diff --git a/crates/pons-core/src/cfg/mod.rs b/crates/pons-core/src/cfg/mod.rs new file mode 100644 index 0000000..d3c8792 --- /dev/null +++ b/crates/pons-core/src/cfg/mod.rs @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! Control-flow graph construction for the T1 dataflow analyses (ADR-0002). +//! +//! Deliberately almost empty of builder code at this commit. The first thing +//! this module owes the rest of the crate is proof that the grammar vocabulary +//! it is about to be written against is the vocabulary the *pinned parser* +//! actually produces — see [`grammar_probe`]. Writing a CFG builder against +//! guessed node names is the same class of mistake as reading a config key with +//! `grep` and assuming the parser agrees. +//! +//! That probe has already overturned two claims this module would otherwise +//! have been built on; [`is_except_group`] exists because of one of them. + +use tree_sitter::Node; + +#[cfg(test)] +mod grammar_probe; + +/// Is this `except_clause` a PEP 654 exception *group* handler (`except*`)? +/// +/// **This cannot be left to the `has_error()` OPAQUE trigger.** ADR-0002's +/// implementation plan assumed `except*` is unparseable by the pinned grammar +/// and so would be caught as an error node. Measured, it is not: +/// `tree-sitter-python` 0.25.0 parses `except* ValueError:` to an ordinary +/// `except_clause` with the `*` demoted to an *anonymous* child, producing a +/// named-tree shape **identical** to plain `except ValueError:`. +/// `has_error()` returns `false`. +/// +/// Silently misreading a group handler as an ordinary one is worse than +/// refusing to analyse it: the two have genuinely different control flow (a +/// group handler runs for the matching *part* of a raised group, and several +/// sibling handlers can each run for a single raise), so the builder would emit +/// a confident, plausible, wrong CFG and both T1 rules would report against it. +/// +/// The `*` token does survive as an anonymous child, so the discrimination is +/// structural rather than textual. +// Consumed by the CFG builder in Step 2 (the OPAQUE hatch); until that lands the +// grammar probe is its only caller, which reads as dead code in a non-test build. +#[allow(dead_code)] +pub(crate) fn is_except_group(except_clause: Node<'_>) -> bool { + debug_assert_eq!(except_clause.kind(), "except_clause"); + let mut cursor = except_clause.walk(); + for child in except_clause.children(&mut cursor) { + if !child.is_named() && child.kind() == "*" { + return true; + } + } + false +} diff --git a/crates/pons-core/src/lib.rs b/crates/pons-core/src/lib.rs index c33b5f4..380f88c 100644 --- a/crates/pons-core/src/lib.rs +++ b/crates/pons-core/src/lib.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 +pub mod cfg; pub mod engine; pub mod finding; pub mod lang; From 1feb17e0beb0bb294529c786e920c1ae8f521a00 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:00:58 +0100 Subject: [PATCH 2/8] refactor(engine): construct RuleCtx through RuleCtx::new No behaviour change. All nine struct-literal sites (the engine, the falsifier, and seven T0 test helpers) now go through one constructor. A later commit can then add a private, lazily-built per-file CFG shared by every T1 rule without touching each site again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/engine.rs | 21 +++++++++++++------ .../pons-rules/src/t0/constant_condition.rs | 7 +------ crates/pons-rules/src/t0/empty_effect_loop.rs | 7 +------ crates/pons-rules/src/t0/self_assignment.rs | 7 +------ .../src/t0/string_concat_in_loop.rs | 7 +------ crates/pons-rules/src/t0/swallowed_error.rs | 7 +------ .../src/t0/unreachable_after_jump.rs | 7 +------ .../pons-rules/src/t0/while_true_no_break.rs | 7 +------ crates/pons-rules/tests/falsifier.rs | 7 +------ 9 files changed, 23 insertions(+), 54 deletions(-) diff --git a/crates/pons-core/src/engine.rs b/crates/pons-core/src/engine.rs index d8c7a66..47e8f41 100644 --- a/crates/pons-core/src/engine.rs +++ b/crates/pons-core/src/engine.rs @@ -18,6 +18,20 @@ pub struct RuleCtx<'a> { pub tree: &'a Tree, } +impl<'a> RuleCtx<'a> { + /// The only way to build a context. Construction goes through here so + /// that per-file analyses shared between rules (the M4 CFG) can be added + /// as private, lazily-built fields without touching every call site again. + pub fn new(path: &'a Path, lang: Lang, text: &'a str, tree: &'a Tree) -> Self { + Self { + path, + lang, + text, + tree, + } + } +} + /// A single check. Implementors live in `pons-rules`; `pons-core` stays /// rule-agnostic. `check` returns [`RawFinding`], not [`Finding`] — a rule /// has no way to set its own `rule_id`; only [`Engine::scan`] can, from @@ -153,12 +167,7 @@ impl Engine { files_scanned += 1; languages.insert(src.lang); - let ctx = RuleCtx { - path: &src.path, - lang: src.lang, - text: &src.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&src.path, src.lang, &src.text, &tree); for rule in &self.rules { if rule.languages().contains(&src.lang) { diff --git a/crates/pons-rules/src/t0/constant_condition.rs b/crates/pons-rules/src/t0/constant_condition.rs index 399c43b..f7b212d 100644 --- a/crates/pons-rules/src/t0/constant_condition.rs +++ b/crates/pons-rules/src/t0/constant_condition.rs @@ -132,12 +132,7 @@ mod tests { text: text.to_string(), }; let tree = parse::parse(&source).unwrap(); - let ctx = RuleCtx { - path: &source.path, - lang: source.lang, - text: &source.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); rule.check(&ctx) } diff --git a/crates/pons-rules/src/t0/empty_effect_loop.rs b/crates/pons-rules/src/t0/empty_effect_loop.rs index c63f75d..e037f00 100644 --- a/crates/pons-rules/src/t0/empty_effect_loop.rs +++ b/crates/pons-rules/src/t0/empty_effect_loop.rs @@ -144,12 +144,7 @@ mod tests { text: text.to_string(), }; let tree = parse::parse(&source).unwrap(); - let ctx = RuleCtx { - path: &source.path, - lang: source.lang, - text: &source.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); rule.check(&ctx) } diff --git a/crates/pons-rules/src/t0/self_assignment.rs b/crates/pons-rules/src/t0/self_assignment.rs index 6dc2848..26d5bd0 100644 --- a/crates/pons-rules/src/t0/self_assignment.rs +++ b/crates/pons-rules/src/t0/self_assignment.rs @@ -136,12 +136,7 @@ mod tests { text: text.to_string(), }; let tree = parse::parse(&source).unwrap(); - let ctx = RuleCtx { - path: &source.path, - lang: source.lang, - text: &source.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); rule.check(&ctx) } diff --git a/crates/pons-rules/src/t0/string_concat_in_loop.rs b/crates/pons-rules/src/t0/string_concat_in_loop.rs index ae8f70c..6393599 100644 --- a/crates/pons-rules/src/t0/string_concat_in_loop.rs +++ b/crates/pons-rules/src/t0/string_concat_in_loop.rs @@ -369,12 +369,7 @@ mod tests { text: text.to_string(), }; let tree = parse::parse(&source).unwrap(); - let ctx = RuleCtx { - path: &source.path, - lang: source.lang, - text: &source.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); rule.check(&ctx) } diff --git a/crates/pons-rules/src/t0/swallowed_error.rs b/crates/pons-rules/src/t0/swallowed_error.rs index e4acfe6..14339e6 100644 --- a/crates/pons-rules/src/t0/swallowed_error.rs +++ b/crates/pons-rules/src/t0/swallowed_error.rs @@ -152,12 +152,7 @@ mod tests { text: text.to_string(), }; let tree = parse::parse(&source).unwrap(); - let ctx = RuleCtx { - path: &source.path, - lang: source.lang, - text: &source.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); rule.check(&ctx) } diff --git a/crates/pons-rules/src/t0/unreachable_after_jump.rs b/crates/pons-rules/src/t0/unreachable_after_jump.rs index 63c8e2c..4a0d036 100644 --- a/crates/pons-rules/src/t0/unreachable_after_jump.rs +++ b/crates/pons-rules/src/t0/unreachable_after_jump.rs @@ -138,12 +138,7 @@ mod tests { text: text.to_string(), }; let tree = parse::parse(&source).unwrap(); - let ctx = RuleCtx { - path: &source.path, - lang: source.lang, - text: &source.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); rule.check(&ctx) } diff --git a/crates/pons-rules/src/t0/while_true_no_break.rs b/crates/pons-rules/src/t0/while_true_no_break.rs index 487472f..5e76b2f 100644 --- a/crates/pons-rules/src/t0/while_true_no_break.rs +++ b/crates/pons-rules/src/t0/while_true_no_break.rs @@ -191,12 +191,7 @@ mod tests { text: text.to_string(), }; let tree = parse::parse(&source).unwrap(); - let ctx = RuleCtx { - path: &source.path, - lang: source.lang, - text: &source.text, - tree: &tree, - }; + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); rule.check(&ctx) } diff --git a/crates/pons-rules/tests/falsifier.rs b/crates/pons-rules/tests/falsifier.rs index 2dec9cc..2e32738 100644 --- a/crates/pons-rules/tests/falsifier.rs +++ b/crates/pons-rules/tests/falsifier.rs @@ -66,12 +66,7 @@ fn check_fixture(rule: &dyn Rule, fixture: &Fixture) -> Vec Date: Wed, 30 Sep 2026 11:22:44 +0100 Subject: [PATCH 3/8] =?UTF-8?q?feat(cfg):=20M4=20step=202=20=E2=80=94=20Py?= =?UTF-8?q?thon=20CFG=20builder,=20locals=20pre-pass,=20OPAQUE=20hatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lowers every function_definition to a FunctionUnit, per ADR-0002: - Analysable(FunctionCfg) with private fields, or Opaque{reason}; a T1 rule cannot obtain a CFG for a function it must not analyse. Triggers: parse error, exec/eval/locals/globals/vars in the own body, except*, module wildcard import, frame introspection, >4096 locals. - Locals pre-pass (scope.rs): global/nonlocal excluded, bare annotations local, comprehension targets not leaked but walrus targets are, match captures precise (class names and keyword attribute names are not locals). - try: a try_entry block and per-statement blocks give both handler edges ADR-0002 needs; else/handler tails raise to the exceptional finally; finally is copied per exit (normal, exceptional, each jump) — gap-fill. - while/else, while True with no false edge, match arms — gap-fills. - RuleCtx::units() builds once per file, lazily, Python only. Six CFG snapshots (three ADR-mandated, three gap-fills), each read before blessing; a mutant dropping the pre-statement raise edge is killed by the direct try-edge test. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/cfg/builder.rs | 921 ++++++++++++++++++ crates/pons-core/src/cfg/mod.rs | 363 ++++++- crates/pons-core/src/cfg/scope.rs | 342 +++++++ crates/pons-core/src/cfg/tests.rs | 362 +++++++ crates/pons-core/src/engine.rs | 16 + .../tests/golden/cfg_comprehension_walrus.txt | 9 + .../tests/golden/cfg_finally_jumps.txt | 33 + .../pons-core/tests/golden/cfg_for_else.txt | 21 + crates/pons-core/tests/golden/cfg_match.txt | 18 + .../tests/golden/cfg_try_except_finally.txt | 23 + .../pons-core/tests/golden/cfg_while_true.txt | 19 + 11 files changed, 2114 insertions(+), 13 deletions(-) create mode 100644 crates/pons-core/src/cfg/builder.rs create mode 100644 crates/pons-core/src/cfg/scope.rs create mode 100644 crates/pons-core/src/cfg/tests.rs create mode 100644 crates/pons-core/tests/golden/cfg_comprehension_walrus.txt create mode 100644 crates/pons-core/tests/golden/cfg_finally_jumps.txt create mode 100644 crates/pons-core/tests/golden/cfg_for_else.txt create mode 100644 crates/pons-core/tests/golden/cfg_match.txt create mode 100644 crates/pons-core/tests/golden/cfg_try_except_finally.txt create mode 100644 crates/pons-core/tests/golden/cfg_while_true.txt diff --git a/crates/pons-core/src/cfg/builder.rs b/crates/pons-core/src/cfg/builder.rs new file mode 100644 index 0000000..46e0d10 --- /dev/null +++ b/crates/pons-core/src/cfg/builder.rs @@ -0,0 +1,921 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! Lowers one `function_definition` body to a [`FunctionCfg`]. +//! +//! ADR-0002 is the authority. Where it is silent the choice made here is +//! marked **gap-fill** and pinned by a snapshot test in `cfg::tests`. + +use tree_sitter::Node; + +use super::scope::{self, Scope}; +use super::{BasicBlock, BlockId, DefReport, Event, EventKind, FunctionCfg, LocalId, Stmt}; +use crate::finding::Location; + +/// Something a `break`, `continue` or `return` has to pass on its way out. +#[derive(Clone)] +enum Frame<'t> { + Loop { + brk: BlockId, + cont: BlockId, + }, + /// A `finally` still to run. `outer_raise` is where an exception goes once + /// it is outside this `try`. + Finally { + body: Node<'t>, + outer_raise: Vec, + }, +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum Jump { + Return, + Break, + Continue, +} + +pub(super) struct Builder<'t> { + text: &'t str, + file: String, + scope: &'t Scope, + blocks: Vec, + cur: BlockId, + frames: Vec>, + /// Where an exception raised at the current point goes: the handlers of + /// the enclosing `try`, the exceptional copy of its `finally`, or `EXIT`. + raise_to: Vec, + /// A comprehension's own iteration variables, innermost last. They + /// shadow a same-named local inside the comprehension. + shadow: Vec>, +} + +impl<'t> Builder<'t> { + pub(super) fn build( + func: Node<'t>, + name: String, + span: Location, + text: &'t str, + file: String, + scope: &'t Scope, + ) -> FunctionCfg { + let mut b = Builder { + text, + file, + scope, + blocks: vec![BasicBlock::default(), BasicBlock::default()], + cur: FunctionCfg::ENTRY, + frames: Vec::new(), + raise_to: vec![FunctionCfg::EXIT], + shadow: Vec::new(), + }; + let body = func + .child_by_field_name("body") + .expect("function_definition always has a body"); + b.block(body); + b.edge(b.cur, FunctionCfg::EXIT); + FunctionCfg { + name, + span, + locals: scope.locals.clone(), + params: scope.params.clone(), + blocks: b.blocks, + } + } + + // ---- graph plumbing ------------------------------------------------- + + fn new_block(&mut self) -> BlockId { + self.blocks.push(BasicBlock::default()); + self.blocks.len() - 1 + } + + fn edge(&mut self, from: BlockId, to: BlockId) { + let succs = &mut self.blocks[from].succs; + if !succs.contains(&to) { + succs.push(to); + } + } + + fn edges_to_raise(&mut self, from: BlockId) { + for t in self.raise_to.clone() { + self.edge(from, t); + } + } + + /// Inside any `try` region an exception can leave between two statements, + /// so each statement gets its own block (see [`Builder::stmt`]). + fn in_try_region(&self) -> bool { + self.raise_to != [FunctionCfg::EXIT] + } + + fn tail_raise(&mut self) { + if self.in_try_region() { + self.edges_to_raise(self.cur); + } + } + + fn emit(&mut self, start: usize, end: usize, events: Vec) { + self.blocks[self.cur].stmts.push(Stmt { + byte_start: start, + byte_end: end, + events, + }); + } + + fn emit_node(&mut self, node: Node<'_>, events: Vec) { + self.emit(node.start_byte(), node.end_byte(), events); + } + + // ---- events --------------------------------------------------------- + + fn lookup(&self, ident: Node<'_>) -> Option { + let name = &self.text[ident.byte_range()]; + if self.shadow.iter().any(|s| s.iter().any(|n| n == name)) { + return None; + } + self.scope.index.get(name).copied() + } + + fn event(&self, ident: Node<'_>, kind: EventKind, ev: &mut Vec) { + if let Some(local) = self.lookup(ident) { + ev.push(Event { + local, + kind, + loc: Location::from_node(self.file.clone(), &ident, self.text), + }); + } + } + + fn use_(&self, ident: Node<'_>, checkable: bool, ev: &mut Vec) { + self.event(ident, EventKind::Use { checkable }, ev); + } + + /// A strong definition. `report` is dropped for a captured local: a + /// closure reads it when *called*, so the store is never provably dead + /// (the late-binding closure false positive). + fn def(&self, ident: Node<'_>, report: Option, ev: &mut Vec) { + let report = report.filter(|_| { + self.lookup(ident) + .is_some_and(|id| !self.scope.captured.contains(&id)) + }); + self.event(ident, EventKind::Def { report }, ev); + } + + // ---- expressions ---------------------------------------------------- + + fn expr(&mut self, node: Node<'t>, ev: &mut Vec, checkable: bool) { + match node.kind() { + "identifier" => self.use_(node, checkable, ev), + // `obj.name`: only `obj` is a read of a variable. + "attribute" => { + if let Some(obj) = node.child_by_field_name("object") { + self.expr(obj, ev, checkable); + } + } + // `f(name=value)`: `name` is a parameter name, not a variable. + "keyword_argument" => { + if let Some(value) = node.child_by_field_name("value") { + self.expr(value, ev, checkable); + } + } + // Annotations are not evaluated as reads of locals. + "type" => {} + "lambda" => self.capture_uses(node, ev), + kind if scope::is_comprehension(kind) => self.comprehension(node, ev, checkable), + // PEP 572: the value first, then the name. A walrus is a *weak* + // def: it binds, so it clears may-unbound, but it never kills + // liveness and is never reported as a dead store. It usually sits + // inside a condition whose branches this CFG does not split. + "named_expression" => { + if let Some(value) = node.child_by_field_name("value") { + self.expr(value, ev, checkable); + } + if let Some(name) = node.child_by_field_name("name") { + self.event(name, EventKind::WeakDef, ev); + } + } + "assignment" => self.assignment(node, ev), + "augmented_assignment" => self.augmented(node, ev), + _ => { + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + for child in children { + self.expr(child, ev, checkable); + } + } + } + } + + /// Every outer local a nested `def`, `lambda` or `class` mentions is read, + /// uncheckably, at the point the nested scope is created. + fn capture_uses(&self, node: Node<'_>, ev: &mut Vec) { + let mut idents = Vec::new(); + scope::identifiers_in(node, &mut idents); + for ident in idents { + self.use_(ident, false, ev); + } + } + + /// Comprehensions are evaluated inline. The first iterable runs in this + /// scope; the rest runs in the comprehension's own, where its `for` + /// targets shadow ours. A generator's body runs later, lazily, so its + /// reads cannot be checked for may-unbound here. + fn comprehension(&mut self, node: Node<'t>, ev: &mut Vec, checkable: bool) { + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + let clauses: Vec<_> = children + .iter() + .copied() + .filter(|c| c.kind() == "for_in_clause") + .collect(); + + let mut own = Vec::new(); + for clause in &clauses { + if let Some(left) = clause.child_by_field_name("left") { + let mut ids = Vec::new(); + scope::target_identifiers(left, &mut ids); + own.extend(ids.iter().map(|i| self.text[i.byte_range()].to_string())); + } + } + + if let Some(right) = clauses.first().and_then(|c| c.child_by_field_name("right")) { + self.expr(right, ev, checkable); + } + let inner = checkable && node.kind() != "generator_expression"; + self.shadow.push(own); + for child in children { + if child.kind() == "for_in_clause" { + if Some(child) != clauses.first().copied() { + if let Some(right) = child.child_by_field_name("right") { + self.expr(right, ev, inner); + } + } + } else { + self.expr(child, ev, inner); + } + } + self.shadow.pop(); + } + + fn has_effect(node: Node<'_>) -> bool { + match node.kind() { + "call" | "await" | "yield" | "named_expression" => true, + "lambda" => false, + _ => { + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + children.into_iter().any(Self::has_effect) + } + } + } + + /// Binds a target. Only a bare identifier target of a plain or augmented + /// assignment carries a `report`; everything else is a def that the + /// dead-store rule does not speak about. + fn bind_target(&mut self, target: Node<'t>, report: Option, ev: &mut Vec) { + match target.kind() { + "identifier" => self.def(target, report, ev), + "pattern_list" + | "tuple_pattern" + | "list_pattern" + | "list_splat_pattern" + | "parenthesized_expression" + | "expression_list" + | "tuple" + | "list" => { + let mut cursor = target.walk(); + let children: Vec<_> = target.named_children(&mut cursor).collect(); + for child in children { + self.bind_target(child, None, ev); + } + } + // `obj.attr = v`, `xs[i] = v`: reads of `obj`, `xs`, `i`. + _ => self.expr(target, ev, true), + } + } + + fn assignment(&mut self, node: Node<'t>, ev: &mut Vec) { + // `a = b = rhs` nests: assignment{left: a, right: assignment{left: b, + // right: rhs}}. Collect the chain; evaluate `rhs` once; then bind. + let mut targets = Vec::new(); + let mut cur = node; + let rhs = loop { + if let Some(left) = cur.child_by_field_name("left") { + targets.push(left); + } + match cur.child_by_field_name("right") { + Some(r) if r.kind() == "assignment" => cur = r, + other => break other, + } + }; + // A bare annotation `x: int` evaluates nothing and binds nothing. + let Some(rhs) = rhs else { return }; + let report = Some(DefReport { + rhs_has_effect: Self::has_effect(rhs), + }); + self.expr(rhs, ev, true); + for target in targets { + self.bind_target(target, report, ev); + } + } + + fn augmented(&mut self, node: Node<'t>, ev: &mut Vec) { + let (Some(left), Some(right)) = ( + node.child_by_field_name("left"), + node.child_by_field_name("right"), + ) else { + return; + }; + self.expr(right, ev, true); + if left.kind() == "identifier" { + self.use_(left, true, ev); + let report = Some(DefReport { + rhs_has_effect: Self::has_effect(right), + }); + self.def(left, report, ev); + } else { + self.expr(left, ev, true); + } + } + + // ---- statements ----------------------------------------------------- + + fn block(&mut self, block: Node<'t>) { + let mut cursor = block.walk(); + let stmts: Vec<_> = block.named_children(&mut cursor).collect(); + for stmt in stmts { + self.stmt(stmt); + } + } + + fn stmt(&mut self, node: Node<'t>) { + if node.kind() == "comment" { + return; + } + if self.in_try_region() { + // The block *before* each statement can raise to the handlers. + // For the first statement of a `try` body that block is + // `try_entry`, which is what delivers the pre-try state to + // may-unbound (ADR-0002). + let prev = self.cur; + self.edges_to_raise(prev); + let next = self.new_block(); + self.edge(prev, next); + self.cur = next; + } + let mut ev = Vec::new(); + match node.kind() { + "return_statement" => { + self.walk_children(node, &mut ev); + self.emit_node(node, ev); + self.jump(Jump::Return); + } + "break_statement" => { + self.emit_node(node, ev); + self.jump(Jump::Break); + } + "continue_statement" => { + self.emit_node(node, ev); + self.jump(Jump::Continue); + } + "raise_statement" => { + self.walk_children(node, &mut ev); + self.emit_node(node, ev); + let from = self.cur; + self.edges_to_raise(from); + self.cur = self.new_block(); + } + "pass_statement" + | "global_statement" + | "nonlocal_statement" + | "future_import_statement" => self.emit_node(node, ev), + "import_statement" | "import_from_statement" => { + for bound in scope::import_bindings(node) { + self.def(bound, None, &mut ev); + } + self.emit_node(node, ev); + } + "delete_statement" => { + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + for child in children { + self.del_target(child, &mut ev); + } + self.emit_node(node, ev); + } + "if_statement" => self.if_(node), + "while_statement" => self.while_(node), + "for_statement" => self.for_(node), + "try_statement" => self.try_(node), + "with_statement" => self.with_(node), + "match_statement" => self.match_(node), + "function_definition" | "class_definition" => { + self.nested_def(node, &mut ev); + self.emit_node(node, ev); + } + "decorated_definition" => { + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + for child in children { + if child.kind() == "decorator" { + self.expr(child, &mut ev, true); + } + } + if let Some(def) = node.child_by_field_name("definition") { + self.nested_def(def, &mut ev); + } + self.emit_node(node, ev); + } + // expression_statement (which carries assignments), assert, and + // anything this grammar adds later: read every name in it. + _ => { + self.walk_children(node, &mut ev); + self.emit_node(node, ev); + } + } + } + + fn walk_children(&mut self, node: Node<'t>, ev: &mut Vec) { + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + for child in children { + self.expr(child, ev, true); + } + } + + fn del_target(&mut self, target: Node<'t>, ev: &mut Vec) { + match target.kind() { + "identifier" => self.event(target, EventKind::Kill, ev), + "expression_list" | "tuple" | "list" | "parenthesized_expression" => { + let mut cursor = target.walk(); + let children: Vec<_> = target.named_children(&mut cursor).collect(); + for child in children { + self.del_target(child, ev); + } + } + _ => self.expr(target, ev, true), + } + } + + /// `def` and `class` inside the unit: defaults, decorators and bases are + /// read now; the body's mentions of our locals are captures; then the + /// name is bound. + fn nested_def(&mut self, node: Node<'t>, ev: &mut Vec) { + if node.kind() == "function_definition" { + if let Some(params) = node.child_by_field_name("parameters") { + let mut cursor = params.walk(); + let ps: Vec<_> = params.named_children(&mut cursor).collect(); + for p in ps { + if let Some(v) = p.child_by_field_name("value") { + self.expr(v, ev, true); + } + } + } + if let Some(body) = node.child_by_field_name("body") { + self.capture_uses(body, ev); + } + } else { + if let Some(bases) = node.child_by_field_name("superclasses") { + self.expr(bases, ev, true); + } + if let Some(body) = node.child_by_field_name("body") { + self.capture_uses(body, ev); + } + } + if let Some(name) = node.child_by_field_name("name") { + self.def(name, None, ev); + } + } + + /// `return` / `break` / `continue`: run every `finally` between here and + /// the destination, innermost first, each as its own copy of the + /// `finally` body (**gap-fill**, see [`Builder::try_`]). + fn jump(&mut self, kind: Jump) { + let mut i = self.frames.len(); + let dest = loop { + if i == 0 { + // `break` outside a loop is a SyntaxError; send it to EXIT. + break FunctionCfg::EXIT; + } + i -= 1; + match self.frames[i].clone() { + Frame::Loop { brk, cont } if kind != Jump::Return => { + break if kind == Jump::Break { brk } else { cont }; + } + Frame::Loop { .. } => {} + Frame::Finally { body, outer_raise } => { + let tail = self.frames.split_off(i); + let saved = std::mem::replace(&mut self.raise_to, outer_raise); + let copy = self.new_block(); + self.edge(self.cur, copy); + self.cur = copy; + self.block(body); + self.raise_to = saved; + self.frames.extend(tail); + } + } + }; + self.edge(self.cur, dest); + // Anything after a jump is unreachable; it still gets a block. + self.cur = self.new_block(); + } + + fn if_(&mut self, node: Node<'t>) { + let after = self.new_block(); + let mut fork = self.cur; + if let Some(cond) = node.child_by_field_name("condition") { + let mut ev = Vec::new(); + self.expr(cond, &mut ev, true); + self.emit_node(cond, ev); + } + if let Some(cons) = node.child_by_field_name("consequence") { + let b = self.new_block(); + self.edge(fork, b); + self.cur = b; + self.block(cons); + self.edge(self.cur, after); + } + let mut cursor = node.walk(); + let alts: Vec<_> = node + .children_by_field_name("alternative", &mut cursor) + .collect(); + let mut has_else = false; + for alt in alts { + let head = self.new_block(); + self.edge(fork, head); + self.cur = head; + if alt.kind() == "elif_clause" { + if let Some(cond) = alt.child_by_field_name("condition") { + let mut ev = Vec::new(); + self.expr(cond, &mut ev, true); + self.emit_node(cond, ev); + } + let b = self.new_block(); + self.edge(head, b); + self.cur = b; + if let Some(cons) = alt.child_by_field_name("consequence") { + self.block(cons); + } + self.edge(self.cur, after); + fork = head; + } else { + if let Some(body) = alt.child_by_field_name("body") { + self.block(body); + } + self.edge(self.cur, after); + has_else = true; + } + } + if !has_else { + self.edge(fork, after); + } + self.cur = after; + } + + /// `while`, including `while … else` (**gap-fill**: the grammar has the + /// field, ADR-0002's table does not). A literal `while True:` has no false + /// edge (**gap-fill**): without this, code after an infinite loop that is + /// left only by `break` would look reachable with the loop's pre-state. + fn while_(&mut self, node: Node<'t>) { + let header = self.new_block(); + self.edge(self.cur, header); + self.cur = header; + let cond = node.child_by_field_name("condition"); + if let Some(cond) = cond { + let mut ev = Vec::new(); + self.expr(cond, &mut ev, true); + self.emit_node(cond, ev); + } + let after = self.new_block(); + self.loop_body(node, header, after); + if cond.is_some_and(|c| c.kind() == "true") { + if node.child_by_field_name("alternative").is_some() { + // `while True: … else:` — the else can never run. + let dead = self.new_block(); + self.cur = dead; + self.else_body(node, after); + } + } else { + let e = self.new_block(); + self.edge(header, e); + self.cur = e; + self.else_body(node, after); + } + self.cur = after; + } + + /// `for`: the iterable is evaluated once, before the loop. The target is + /// defined on the iterate edge, in a synthetic block of its own, so that a + /// read of the loop variable after a loop that never ran is may-unbound. + fn for_(&mut self, node: Node<'t>) { + if let Some(right) = node.child_by_field_name("right") { + let mut ev = Vec::new(); + self.expr(right, &mut ev, true); + self.emit_node(right, ev); + } + let header = self.new_block(); + self.edge(self.cur, header); + let iterate = self.new_block(); + self.edge(header, iterate); + self.cur = iterate; + if let Some(left) = node.child_by_field_name("left") { + let mut ev = Vec::new(); + self.bind_target(left, None, &mut ev); + self.emit_node(left, ev); + } + let after = self.new_block(); + self.loop_body(node, header, after); + let exhausted = self.new_block(); + self.edge(header, exhausted); + self.cur = exhausted; + self.else_body(node, after); + self.cur = after; + } + + fn loop_body(&mut self, node: Node<'t>, header: BlockId, after: BlockId) { + let body_entry = self.new_block(); + self.edge(self.cur, body_entry); + self.cur = body_entry; + self.frames.push(Frame::Loop { + brk: after, + cont: header, + }); + if let Some(body) = node.child_by_field_name("body") { + self.block(body); + } + self.frames.pop(); + self.edge(self.cur, header); + } + + /// The `else` of a loop runs when the loop ends without `break`. + fn else_body(&mut self, node: Node<'t>, after: BlockId) { + if let Some(alt) = node.child_by_field_name("alternative") { + if let Some(body) = alt.child_by_field_name("body") { + self.block(body); + } + } + self.edge(self.cur, after); + } + + fn with_(&mut self, node: Node<'t>) { + let mut ev = Vec::new(); + let mut header_end = node.start_byte(); + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + for clause in children.iter().filter(|c| c.kind() == "with_clause") { + header_end = clause.end_byte(); + let mut cc = clause.walk(); + let items: Vec<_> = clause.named_children(&mut cc).collect(); + for item in items.into_iter().filter(|i| i.kind() == "with_item") { + let Some(value) = item.child_by_field_name("value") else { + continue; + }; + if value.kind() == "as_pattern" { + if let Some(expr) = value.named_child(0) { + self.expr(expr, &mut ev, true); + } + if let Some(alias) = value.child_by_field_name("alias") { + let mut ac = alias.walk(); + let targets: Vec<_> = alias.named_children(&mut ac).collect(); + for t in targets { + self.bind_target(t, None, &mut ev); + } + } + } else { + self.expr(value, &mut ev, true); + } + } + } + self.emit(node.start_byte(), header_end, ev); + if let Some(body) = node.child_by_field_name("body") { + self.block(body); + } + } + + /// `match`: the subject is read, then each arm is a successor of the + /// header. Every identifier in a pattern is first an uncheckable read + /// (value patterns read names), then each capture is a weak def. Unless + /// the last arm is irrefutable (`case _`, or a bare capture, with no + /// guard), the header also falls through to `after`. + fn match_(&mut self, node: Node<'t>) { + if let Some(subject) = node.child_by_field_name("subject") { + let mut ev = Vec::new(); + self.expr(subject, &mut ev, true); + self.emit_node(subject, ev); + } + let header = self.cur; + let after = self.new_block(); + let mut cases = Vec::new(); + if let Some(body) = node.child_by_field_name("body") { + let mut cursor = body.walk(); + cases.extend( + body.named_children(&mut cursor) + .filter(|c| c.kind() == "case_clause"), + ); + } + let irrefutable = cases.last().is_some_and(|c| Self::irrefutable(*c)); + for case in cases { + let arm = self.new_block(); + self.edge(header, arm); + self.cur = arm; + let mut ev = Vec::new(); + let mut cursor = case.walk(); + let patterns: Vec<_> = case + .named_children(&mut cursor) + .filter(|c| c.kind() == "case_pattern") + .collect(); + let mut idents = Vec::new(); + for p in &patterns { + scope::identifiers_in(*p, &mut idents); + } + idents.retain(|i| !scope::is_keyword_pattern_name(*i)); + for i in &idents { + self.use_(*i, false, &mut ev); + } + for i in idents.iter().filter(|i| scope::is_capture(**i)) { + self.event(*i, EventKind::WeakDef, &mut ev); + } + if let Some(guard) = case.child_by_field_name("guard") { + self.expr(guard, &mut ev, true); + } + let header_end = case + .child_by_field_name("consequence") + .map_or(case.end_byte(), |c| c.start_byte()); + self.emit(case.start_byte(), header_end, ev); + if let Some(cons) = case.child_by_field_name("consequence") { + self.block(cons); + } + self.edge(self.cur, after); + } + if !irrefutable { + self.edge(header, after); + } + self.cur = after; + } + + fn irrefutable(case: Node<'_>) -> bool { + if case.child_by_field_name("guard").is_some() { + return false; + } + let mut cursor = case.walk(); + let patterns: Vec<_> = case + .named_children(&mut cursor) + .filter(|c| c.kind() == "case_pattern") + .collect(); + let [pattern] = patterns.as_slice() else { + return false; + }; + match pattern.named_child_count() { + 0 => true, // `case _` + 1 => { + let only = pattern.named_child(0).expect("count is 1"); + only.kind() == "dotted_name" && only.named_child_count() == 1 + } + _ => false, + } + } + + /// `try`. Two load-bearing raise edges per handler (ADR-0002): one from + /// the block before each statement of the body (the first of which is the + /// empty `try_entry`, carrying the pre-try state), and one from the body's + /// tail. Unhandled exceptions also leave to the enclosing raise targets. + /// + /// **Gap-fill — `finally` is built once per exit destination**, not once + /// as a join: a normal copy (to `after`), an exceptional copy (re-raise to + /// the enclosing targets), and one per `return`/`break`/`continue` that + /// crosses it (see [`Builder::jump`]). A single shared `finally` would + /// merge those paths, and a read in `finally` of a name bound only on the + /// normal path would look may-unbound on every path. The rules must + /// therefore combine per source location: a store is dead only if it is + /// dead in every copy; a read is may-unbound if it is in any copy. + fn try_(&mut self, node: Node<'t>) { + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + let handlers: Vec<_> = children + .iter() + .copied() + .filter(|c| c.kind() == "except_clause") + .collect(); + let else_clause = children.iter().copied().find(|c| c.kind() == "else_clause"); + let finally_body = children + .iter() + .copied() + .find(|c| c.kind() == "finally_clause") + .and_then(Self::clause_block); + + let outer_raise = self.raise_to.clone(); + let exc_finally = finally_body.map(|_| self.new_block()); + let past_try: Vec = match exc_finally { + Some(f) => vec![f], + None => outer_raise.clone(), + }; + let handler_entries: Vec = handlers.iter().map(|_| self.new_block()).collect(); + + if let Some(body) = finally_body { + self.frames.push(Frame::Finally { + body, + outer_raise: outer_raise.clone(), + }); + } + + // Body. + self.raise_to = handler_entries + .iter() + .copied() + .chain(past_try.clone()) + .collect(); + let try_entry = self.new_block(); + self.edge(self.cur, try_entry); + self.cur = try_entry; + if let Some(body) = node.child_by_field_name("body") { + self.block(body); + } + let tail = self.cur; + self.edges_to_raise(tail); + + // `else` runs after a body that raised nothing; handlers don't cover it. + self.raise_to = past_try.clone(); + if let Some(else_body) = else_clause.and_then(|e| e.child_by_field_name("body")) { + self.block(else_body); + } + // `stmt` adds a raise edge *before* each statement; the state after the + // last one needs its own, or the final statement cannot raise. + self.tail_raise(); + let mut normal_ends = vec![self.cur]; + + for (clause, entry) in handlers.iter().zip(handler_entries) { + self.cur = entry; + let mut ev = Vec::new(); + let mut alias_targets = Vec::new(); + let value = clause.child_by_field_name("value"); + if let Some(value) = value { + if value.kind() == "as_pattern" { + if let Some(ty) = value.named_child(0) { + self.expr(ty, &mut ev, true); + } + if let Some(alias) = value.child_by_field_name("alias") { + let mut ac = alias.walk(); + let ts: Vec<_> = alias.named_children(&mut ac).collect(); + for t in ts { + scope::target_identifiers(t, &mut alias_targets); + } + } + for t in &alias_targets { + self.def(*t, None, &mut ev); + } + } else { + self.expr(value, &mut ev, true); + } + } + let block = Self::clause_block(*clause); + let header_end = block.map_or(clause.end_byte(), |b| b.start_byte()); + self.emit(clause.start_byte(), header_end, ev); + if let Some(block) = block { + self.block(block); + } + // Python deletes the `as` name when the handler exits. + if !alias_targets.is_empty() { + let mut ev = Vec::new(); + for t in &alias_targets { + self.event(*t, EventKind::Kill, &mut ev); + } + let (s, e) = (alias_targets[0].start_byte(), alias_targets[0].end_byte()); + self.emit(s, e, ev); + } + // After the kill: the implicit `del` runs on the exceptional exit + // too. + self.tail_raise(); + normal_ends.push(self.cur); + } + + if finally_body.is_some() { + self.frames.pop(); + } + self.raise_to = outer_raise.clone(); + + let join = self.new_block(); + for end in normal_ends { + self.edge(end, join); + } + self.cur = join; + if let Some(body) = finally_body { + self.block(body); + let after_normal = self.cur; + + self.cur = exc_finally.expect("allocated with finally_body"); + self.block(body); + let from = self.cur; + self.edges_to_raise(from); + + self.cur = after_normal; + } + } + + /// The unfielded `block` child of `except_clause` / `finally_clause`. + fn clause_block(clause: Node<'t>) -> Option> { + let mut cursor = clause.walk(); + let found = clause + .named_children(&mut cursor) + .find(|c| c.kind() == "block"); + found + } +} diff --git a/crates/pons-core/src/cfg/mod.rs b/crates/pons-core/src/cfg/mod.rs index d3c8792..915eae0 100644 --- a/crates/pons-core/src/cfg/mod.rs +++ b/crates/pons-core/src/cfg/mod.rs @@ -1,21 +1,361 @@ // SPDX-License-Identifier: MPL-2.0 -//! Control-flow graph construction for the T1 dataflow analyses (ADR-0002). +//! Control-flow graphs for the T1 dataflow analyses (ADR-0002). //! -//! Deliberately almost empty of builder code at this commit. The first thing -//! this module owes the rest of the crate is proof that the grammar vocabulary -//! it is about to be written against is the vocabulary the *pinned parser* -//! actually produces — see [`grammar_probe`]. Writing a CFG builder against -//! guessed node names is the same class of mistake as reading a config key with -//! `grep` and assuming the parser agrees. +//! The analysis unit is a `function_definition` body, at any depth. Module +//! top level and class bodies are **not** units: a module-level name can be +//! imported by another module, so a store to it is never provably dead. +//! Lambdas are not units either; they contain no statements. //! -//! That probe has already overturned two claims this module would otherwise -//! have been built on; [`is_except_group`] exists because of one of them. +//! A unit is either [`FunctionUnit::Analysable`] or [`FunctionUnit::Opaque`]. +//! A T1 rule cannot get a CFG for an opaque function, so it cannot forget to +//! check whether it should have asked. That mirrors the way `finding.rs` +//! enforces the evidence invariant in the type system rather than in review. +//! +//! This module also holds the grammar probe, which proves that the vocabulary +//! the builder is written against is the vocabulary the *pinned parser* +//! produces — see [`grammar_probe`]. That probe overturned two claims this +//! module would otherwise have been built on; [`is_except_group`] exists +//! because of one of them. + +use std::fmt; +use std::path::Path; -use tree_sitter::Node; +use tree_sitter::{Node, Tree}; +use crate::finding::Location; + +mod builder; #[cfg(test)] mod grammar_probe; +mod scope; +#[cfg(test)] +mod tests; + +pub type BlockId = usize; +/// Index into [`FunctionCfg::locals`]. +pub type LocalId = usize; + +/// ADR-0002's runaway guard: a function with more locals than this is +/// generated code, and is reported opaque rather than analysed. +pub const MAX_LOCALS: usize = 4096; + +/// What one statement does to one local. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EventKind { + /// A read. `checkable` is false for reads the may-unbound analysis cannot + /// judge from here: a closure capture (read when the closure runs), a + /// name in a `match` pattern, a lazily-run generator body. + Use { checkable: bool }, + /// A binding that overwrites the old value. `report` is `Some` only for a + /// store the dead-store rule may speak about: a bare-name target of a + /// plain or augmented assignment, and not captured by a nested scope. + Def { report: Option }, + /// A binding that clears may-unbound but does not end a live range: a + /// walrus target or a `match` capture. Both usually sit on a branch this + /// CFG does not split, so treating them as a kill would invent dead stores. + WeakDef, + /// `del x`, and the `except … as e` name at the end of its handler. + Kill, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct DefReport { + /// The right-hand side contains a call, `await`, `yield` or walrus. + pub rhs_has_effect: bool, +} + +#[derive(Debug, Clone)] +pub struct Event { + pub local: LocalId, + pub kind: EventKind, + /// The identifier the event is about. + pub loc: Location, +} + +/// One statement, or one statement's header (`if` condition, `for` target, +/// `except` clause). The byte range is kept, not just the events, because +/// M5's typestate walks this same CFG and needs to see the calls in it. +#[derive(Debug, Clone)] +pub struct Stmt { + pub byte_start: usize, + pub byte_end: usize, + /// In evaluation order. + pub events: Vec, +} + +#[derive(Debug, Clone, Default)] +pub struct BasicBlock { + pub stmts: Vec, + pub succs: Vec, +} + +/// The CFG of one analysable function. Only the builder can make one. +#[derive(Debug)] +pub struct FunctionCfg { + name: String, + span: Location, + locals: Vec, + params: Vec, + blocks: Vec, +} + +impl FunctionCfg { + pub const ENTRY: BlockId = 0; + pub const EXIT: BlockId = 1; + + pub fn name(&self) -> &str { + &self.name + } + pub fn span(&self) -> &Location { + &self.span + } + pub fn locals(&self) -> &[String] { + &self.locals + } + pub fn params(&self) -> &[LocalId] { + &self.params + } + pub fn blocks(&self) -> &[BasicBlock] { + &self.blocks + } + + pub fn preds(&self) -> Vec> { + let mut preds = vec![Vec::new(); self.blocks.len()]; + for (b, block) in self.blocks.iter().enumerate() { + for &s in &block.succs { + preds[s].push(b); + } + } + preds + } + + /// Blocks reachable from [`FunctionCfg::ENTRY`]. Code after a `return` + /// still gets blocks; findings in it belong to `unreachable-after-jump`, + /// not to the dataflow rules. + pub fn reachable(&self) -> Vec { + let mut seen = vec![false; self.blocks.len()]; + let mut stack = vec![Self::ENTRY]; + while let Some(b) = stack.pop() { + if std::mem::replace(&mut seen[b], true) { + continue; + } + stack.extend(&self.blocks[b].succs); + } + seen + } + + /// A stable, human-diffable rendering for the snapshot tests. + pub fn render(&self, text: &str) -> String { + use std::fmt::Write; + let mut out = String::new(); + let names = |ids: &[LocalId]| { + ids.iter() + .map(|&i| self.locals[i].as_str()) + .collect::>() + .join(" ") + }; + let all: Vec = (0..self.locals.len()).collect(); + writeln!(out, "fn {}", self.name).unwrap(); + writeln!(out, "locals: {}", names(&all)).unwrap(); + writeln!(out, "params: {}", names(&self.params)).unwrap(); + let reachable = self.reachable(); + for (b, block) in self.blocks.iter().enumerate() { + let label = match b { + Self::ENTRY => " (entry)", + Self::EXIT => " (exit)", + _ if !reachable[b] => " (unreachable)", + _ => "", + }; + let succs: Vec = block.succs.iter().map(|s| format!("b{s}")).collect(); + writeln!(out, "b{b}{label} -> [{}]", succs.join(" ")).unwrap(); + for stmt in &block.stmts { + let line = text[..stmt.byte_start].matches('\n').count() + 1; + let src = text[stmt.byte_start..stmt.byte_end] + .lines() + .next() + .unwrap_or("") + .trim(); + let events: Vec = stmt + .events + .iter() + .map(|e| { + let n = &self.locals[e.local]; + match e.kind { + EventKind::Use { checkable: true } => format!("use {n}"), + EventKind::Use { checkable: false } => format!("use~ {n}"), + EventKind::Def { report: None } => format!("def {n}"), + EventKind::Def { + report: + Some(DefReport { + rhs_has_effect: false, + }), + } => format!("def! {n}"), + EventKind::Def { + report: + Some(DefReport { + rhs_has_effect: true, + }), + } => format!("def!* {n}"), + EventKind::WeakDef => format!("weak {n}"), + EventKind::Kill => format!("kill {n}"), + } + }) + .collect(); + writeln!(out, " L{line} `{src}` {{{}}}", events.join(", ")).unwrap(); + } + } + out + } +} + +/// Why a function was not analysed. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OpaqueReason { + /// A call to `exec`, `eval`, `locals`, `globals` or `vars`: the set of + /// names, or their values, is no longer visible in the source. + DynamicScope(&'static str), + /// `from m import *` anywhere in the module. + WildcardImport, + /// `sys._getframe` / `inspect.currentframe`. + FrameIntrospection, + /// A PEP 654 `except*` handler (see [`is_except_group`]). + ExceptGroup, + /// The function's subtree contains a parse error. + ParseError, + /// More than [`MAX_LOCALS`] locals. + TooManyLocals(usize), +} + +impl fmt::Display for OpaqueReason { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::DynamicScope(name) => write!(f, "calls `{name}`"), + Self::WildcardImport => f.write_str("the module has a wildcard import"), + Self::FrameIntrospection => f.write_str("inspects stack frames"), + Self::ExceptGroup => f.write_str("has an `except*` handler"), + Self::ParseError => f.write_str("does not parse cleanly"), + Self::TooManyLocals(n) => write!(f, "has {n} locals (limit {MAX_LOCALS})"), + } + } +} + +#[derive(Debug)] +pub enum FunctionUnit { + Analysable(FunctionCfg), + Opaque { + name: String, + reason: OpaqueReason, + span: Location, + }, +} + +/// Every function unit in a Python module, in source order. +pub fn build_units(path: &Path, text: &str, tree: &Tree) -> Vec { + let file = path.display().to_string(); + let root = tree.root_node(); + let wildcard = contains_kind(root, "wildcard_import"); + let mut funcs = Vec::new(); + collect_functions(root, &mut funcs); + funcs + .into_iter() + .map(|func| { + let name = func + .child_by_field_name("name") + .map_or("", |n| &text[n.byte_range()]) + .to_string(); + let span = Location::from_node(file.clone(), &func, text); + let opaque = |reason| FunctionUnit::Opaque { + name: name.clone(), + reason, + span: span.clone(), + }; + if let Some(reason) = opaque_reason(func, text, wildcard) { + return opaque(reason); + } + let scope = scope::analyse(func, text); + if scope.locals.len() > MAX_LOCALS { + return opaque(OpaqueReason::TooManyLocals(scope.locals.len())); + } + FunctionUnit::Analysable(builder::Builder::build( + func, + name.clone(), + span.clone(), + text, + file.clone(), + &scope, + )) + }) + .collect() +} + +fn collect_functions<'t>(node: Node<'t>, out: &mut Vec>) { + if node.kind() == "function_definition" { + out.push(node); + } + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + collect_functions(child, out); + } +} + +fn contains_kind(node: Node<'_>, kind: &str) -> bool { + if node.kind() == kind { + return true; + } + let mut cursor = node.walk(); + let found = node + .named_children(&mut cursor) + .any(|c| contains_kind(c, kind)); + found +} + +const DYNAMIC_SCOPE: &[&str] = &["exec", "eval", "locals", "globals", "vars"]; + +fn opaque_reason(func: Node<'_>, text: &str, module_has_wildcard: bool) -> Option { + if func.has_error() { + return Some(OpaqueReason::ParseError); + } + let body = func.child_by_field_name("body")?; + if let Some(reason) = own_body_trigger(body, text) { + return Some(reason); + } + if module_has_wildcard { + return Some(OpaqueReason::WildcardImport); + } + // Textual, and deliberately broad: `from sys import _getframe` then a bare + // `_getframe()` is caught too. A false OPAQUE costs findings, never + // correctness. + let src = &text[body.byte_range()]; + if src.contains("_getframe") || src.contains("currentframe") { + return Some(OpaqueReason::FrameIntrospection); + } + None +} + +/// Triggers that belong to this function's own code, not to a nested `def` +/// or `lambda` (whose `locals()` is its own). +fn own_body_trigger(node: Node<'_>, text: &str) -> Option { + match node.kind() { + "function_definition" | "lambda" => return None, + "exec_statement" => return Some(OpaqueReason::DynamicScope("exec")), + "except_clause" if is_except_group(node) => return Some(OpaqueReason::ExceptGroup), + "call" => { + if let Some(f) = node.child_by_field_name("function") { + if f.kind() == "identifier" { + let name = &text[f.byte_range()]; + if let Some(hit) = DYNAMIC_SCOPE.iter().find(|d| **d == name) { + return Some(OpaqueReason::DynamicScope(hit)); + } + } + } + } + _ => {} + } + let mut cursor = node.walk(); + let children: Vec<_> = node.named_children(&mut cursor).collect(); + children.into_iter().find_map(|c| own_body_trigger(c, text)) +} /// Is this `except_clause` a PEP 654 exception *group* handler (`except*`)? /// @@ -35,9 +375,6 @@ mod grammar_probe; /// /// The `*` token does survive as an anonymous child, so the discrimination is /// structural rather than textual. -// Consumed by the CFG builder in Step 2 (the OPAQUE hatch); until that lands the -// grammar probe is its only caller, which reads as dead code in a non-test build. -#[allow(dead_code)] pub(crate) fn is_except_group(except_clause: Node<'_>) -> bool { debug_assert_eq!(except_clause.kind(), "except_clause"); let mut cursor = except_clause.walk(); diff --git a/crates/pons-core/src/cfg/scope.rs b/crates/pons-core/src/cfg/scope.rs new file mode 100644 index 0000000..d0c0d32 --- /dev/null +++ b/crates/pons-core/src/cfg/scope.rs @@ -0,0 +1,342 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! The locals pre-pass: which names a function binds, before any edge exists. +//! +//! Python decides scope statically. A name is local to a function if the +//! function binds it *anywhere* in its body, whatever the order. So the set of +//! locals has to be complete before the builder emits its first event. The +//! builder only records events for names in this set. Everything else is a +//! global or a builtin, and neither T1 rule has anything to say about those. + +use std::collections::{HashMap, HashSet}; + +use tree_sitter::Node; + +use super::LocalId; + +pub(super) struct Scope { + pub locals: Vec, + pub index: HashMap, + pub params: Vec, + /// Locals that some nested `def`, `lambda` or `class` mentions. A closure + /// reads a variable when it is *called*, not when it is defined, so no + /// store to one of these can be shown dead. See `Builder::def`. + pub captured: HashSet, +} + +/// Node kinds that open a new scope whose body is not this unit's code. +pub(super) fn is_nested_scope(kind: &str) -> bool { + matches!(kind, "function_definition" | "class_definition" | "lambda") +} + +pub(super) fn is_comprehension(kind: &str) -> bool { + matches!( + kind, + "list_comprehension" + | "set_comprehension" + | "dictionary_comprehension" + | "generator_expression" + ) +} + +pub(super) fn analyse(func: Node<'_>, text: &str) -> Scope { + let body = func + .child_by_field_name("body") + .expect("function_definition always has a body"); + + let mut declared = HashSet::new(); + collect_declared(body, text, &mut declared); + + let mut names = Vec::new(); + if let Some(params) = func.child_by_field_name("parameters") { + collect_params(params, text, &mut names); + } + let n_params = names.len(); + collect_bound(body, text, &mut names); + + let mut locals = Vec::new(); + let mut index = HashMap::new(); + let mut params = Vec::new(); + for (i, name) in names.into_iter().enumerate() { + // `global x` / `nonlocal x` make `x` a name this function writes + // *through*, not one it owns. + if declared.contains(&name) { + continue; + } + let id = *index.entry(name.clone()).or_insert_with(|| { + locals.push(name); + locals.len() - 1 + }); + if i < n_params && !params.contains(&id) { + params.push(id); + } + } + + let mut mentioned = HashSet::new(); + collect_nested_mentions(body, text, &mut mentioned); + let captured = mentioned + .iter() + .filter_map(|name| index.get(name.as_str()).copied()) + .collect(); + + Scope { + locals, + index, + params, + captured, + } +} + +fn text_of<'t>(node: Node<'_>, text: &'t str) -> &'t str { + &text[node.byte_range()] +} + +fn named_children<'t>(node: Node<'t>) -> Vec> { + let mut cursor = node.walk(); + node.named_children(&mut cursor).collect() +} + +fn collect_declared(node: Node<'_>, text: &str, out: &mut HashSet) { + match node.kind() { + "global_statement" | "nonlocal_statement" => { + for child in named_children(node) { + if child.kind() == "identifier" { + out.insert(text_of(child, text).to_string()); + } + } + } + kind if is_nested_scope(kind) => {} + _ => { + for child in named_children(node) { + collect_declared(child, text, out); + } + } + } +} + +/// Every name a `parameters` node binds: plain, defaulted, typed, and the +/// identifiers inside `*args` / `**kwargs`. Separators bind nothing. +fn collect_params(params: Node<'_>, text: &str, out: &mut Vec) { + for p in named_children(params) { + let name = match p.kind() { + "identifier" => Some(p), + "default_parameter" | "typed_default_parameter" => p.child_by_field_name("name"), + "typed_parameter" | "list_splat_pattern" | "dictionary_splat_pattern" => { + first_identifier(p) + } + _ => None, + }; + if let Some(name) = name { + if name.kind() == "identifier" { + out.push(text_of(name, text).to_string()); + } else if let Some(inner) = first_identifier(name) { + // `typed_parameter` wrapping a splat: `*args: int`. + out.push(text_of(inner, text).to_string()); + } + } + } +} + +fn first_identifier(node: Node<'_>) -> Option> { + for child in named_children(node) { + if child.kind() == "identifier" { + return Some(child); + } + if matches!( + child.kind(), + "list_splat_pattern" | "dictionary_splat_pattern" + ) { + return first_identifier(child); + } + } + None +} + +/// The names a binding target binds. Attribute and subscript targets bind +/// nothing; they *read* their object. +pub(super) fn target_identifiers<'t>(target: Node<'t>, out: &mut Vec>) { + match target.kind() { + "identifier" => out.push(target), + "pattern_list" + | "tuple_pattern" + | "list_pattern" + | "list_splat_pattern" + | "parenthesized_expression" + | "expression_list" + | "tuple" + | "list" => { + for child in named_children(target) { + target_identifiers(child, out); + } + } + _ => {} + } +} + +/// The name an import binds: `import a.b` binds `a`; `import a.b as c` and +/// `from m import a as c` bind `c`; `from m import a` binds `a`. +pub(super) fn import_bindings<'t>(stmt: Node<'t>) -> Vec> { + let mut cursor = stmt.walk(); + let mut out = Vec::new(); + for name in stmt.children_by_field_name("name", &mut cursor) { + let bound = match name.kind() { + "aliased_import" => name.child_by_field_name("alias"), + "dotted_name" => name.named_child(0), + _ => None, + }; + if let Some(bound) = bound.filter(|b| b.kind() == "identifier") { + out.push(bound); + } + } + out +} + +/// Is this identifier, found inside a `case_pattern`, a *capture*? +/// +/// Measured against the pinned grammar: a bare name is a `dotted_name` with a +/// single identifier; `Color.RED` is a `dotted_name` with two and is a value, +/// not a capture; the class in `Point(x=0)` is the `dotted_name` directly under +/// `class_pattern`; `*rest`, `**kw` and `as p` put the identifier directly +/// under `splat_pattern` / `as_pattern`. The `x` in `Point(x=0)` is an +/// attribute name under `keyword_pattern` and binds nothing. +pub(super) fn is_capture(ident: Node<'_>) -> bool { + let Some(parent) = ident.parent() else { + return false; + }; + match parent.kind() { + "splat_pattern" => true, + "as_pattern" => parent.named_child(0) != Some(ident), + "dotted_name" => { + parent.named_child_count() == 1 + && parent.parent().is_some_and(|g| g.kind() != "class_pattern") + } + _ => false, + } +} + +/// Is this identifier the attribute name in `Point(x=0)`? It is neither a +/// read nor a binding. +pub(super) fn is_keyword_pattern_name(ident: Node<'_>) -> bool { + ident + .parent() + .is_some_and(|p| p.kind() == "keyword_pattern" && p.named_child(0) == Some(ident)) +} + +pub(super) fn identifiers_in<'t>(node: Node<'t>, out: &mut Vec>) { + if node.kind() == "identifier" { + out.push(node); + return; + } + for child in named_children(node) { + identifiers_in(child, out); + } +} + +fn push_all(nodes: &[Node<'_>], text: &str, out: &mut Vec) { + out.extend(nodes.iter().map(|n| text_of(*n, text).to_string())); +} + +fn collect_bound(node: Node<'_>, text: &str, out: &mut Vec) { + let mut targets = Vec::new(); + match node.kind() { + "function_definition" | "class_definition" => { + if let Some(name) = node.child_by_field_name("name") { + out.push(text_of(name, text).to_string()); + } + // Default values and decorators run in *this* scope, and a walrus + // there would bind here. Both are vanishingly rare; the body is + // another scope entirely and is not entered. + return; + } + "lambda" => return, + "global_statement" | "nonlocal_statement" => return, + kind if is_comprehension(kind) => { + // A comprehension's own `for` targets do not leak (PEP 709 kept + // that), but a walrus inside it binds in *this* scope (PEP 572). + collect_walrus(node, text, out); + return; + } + "assignment" | "augmented_assignment" => { + if let Some(left) = node.child_by_field_name("left") { + target_identifiers(left, &mut targets); + } + // A bare annotation `x: int` has no `right`. It still makes `x` + // local (reading it before assignment is UnboundLocalError), so it + // is collected here. It binds nothing; the builder emits no def. + if let Some(right) = node.child_by_field_name("right") { + collect_bound(right, text, out); + } + } + "named_expression" => { + if let Some(name) = node.child_by_field_name("name") { + targets.push(name); + } + if let Some(value) = node.child_by_field_name("value") { + collect_bound(value, text, out); + } + } + "for_statement" => { + if let Some(left) = node.child_by_field_name("left") { + target_identifiers(left, &mut targets); + } + for child in named_children(node) { + if Some(child) != node.child_by_field_name("left") { + collect_bound(child, text, out); + } + } + } + "as_pattern_target" => { + for child in named_children(node) { + target_identifiers(child, &mut targets); + } + } + "import_statement" | "import_from_statement" => targets = import_bindings(node), + "delete_statement" => { + for child in named_children(node) { + target_identifiers(child, &mut targets); + } + } + "case_pattern" => { + let mut idents = Vec::new(); + identifiers_in(node, &mut idents); + targets.extend(idents.into_iter().filter(|i| is_capture(*i))); + } + _ => { + for child in named_children(node) { + collect_bound(child, text, out); + } + } + } + push_all(&targets, text, out); +} + +fn collect_walrus(node: Node<'_>, text: &str, out: &mut Vec) { + match node.kind() { + kind if is_nested_scope(kind) => {} + "named_expression" => { + if let Some(name) = node.child_by_field_name("name") { + out.push(text_of(name, text).to_string()); + } + if let Some(value) = node.child_by_field_name("value") { + collect_walrus(value, text, out); + } + } + _ => { + for child in named_children(node) { + collect_walrus(child, text, out); + } + } + } +} + +fn collect_nested_mentions(node: Node<'_>, text: &str, out: &mut HashSet) { + if is_nested_scope(node.kind()) { + let mut idents = Vec::new(); + identifiers_in(node, &mut idents); + out.extend(idents.iter().map(|i| text_of(*i, text).to_string())); + return; + } + for child in named_children(node) { + collect_nested_mentions(child, text, out); + } +} diff --git a/crates/pons-core/src/cfg/tests.rs b/crates/pons-core/src/cfg/tests.rs new file mode 100644 index 0000000..b946c86 --- /dev/null +++ b/crates/pons-core/src/cfg/tests.rs @@ -0,0 +1,362 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! CFG snapshots and OPAQUE-hatch controls. +//! +//! Inputs are embedded raw strings: Python is banned outside `fixtures/`. +//! Snapshots use `report::assert_golden`; bless with `PONS_BLESS=1`, and only +//! after reading the diff. + +use std::path::Path; + +use tree_sitter::Parser; + +use super::{build_units, EventKind, FunctionCfg, FunctionUnit, OpaqueReason}; +use crate::report::assert_golden; + +fn units(src: &str) -> Vec { + let mut parser = Parser::new(); + parser + .set_language(&tree_sitter_python::LANGUAGE.into()) + .unwrap(); + let tree = parser.parse(src, None).unwrap(); + build_units(Path::new("t.py"), src, &tree) +} + +fn only_cfg(src: &str) -> FunctionCfg { + let mut us = units(src); + assert_eq!(us.len(), 1, "expected exactly one unit"); + match us.remove(0) { + FunctionUnit::Analysable(cfg) => cfg, + FunctionUnit::Opaque { reason, .. } => panic!("unexpectedly opaque: {reason}"), + } +} + +fn opaque_reason(src: &str) -> Option { + match units(src).remove(0) { + FunctionUnit::Analysable(_) => None, + FunctionUnit::Opaque { reason, .. } => Some(reason), + } +} + +fn snapshot(name: &str, src: &str, expected: &str) { + let cfg = only_cfg(src); + assert_golden(name, &cfg.render(src), expected); +} + +// ---- snapshots ADR-0002 requires ------------------------------------------ + +#[test] +fn cfg_for_else() { + snapshot( + "cfg_for_else.txt", + r#"def f(xs): + for x in xs: + if x: + break + y = x + else: + y = 0 + return x +"#, + include_str!("../../tests/golden/cfg_for_else.txt"), + ); +} + +#[test] +fn cfg_try_except_finally() { + snapshot( + "cfg_try_except_finally.txt", + r#"def f(): + try: + a = g() + b = a + except E as e: + print(e, a) + else: + c = 1 + finally: + cleanup(a) + return a +"#, + include_str!("../../tests/golden/cfg_try_except_finally.txt"), + ); +} + +#[test] +fn cfg_comprehension_walrus() { + snapshot( + "cfg_comprehension_walrus.txt", + r#"def f(xs): + x = 1 + ys = [x for x in xs if (n := x)] + return n, x, ys +"#, + include_str!("../../tests/golden/cfg_comprehension_walrus.txt"), + ); +} + +// ---- gap-fills -------------------------------------------------------------- + +#[test] +fn cfg_while_true_has_no_false_edge() { + snapshot( + "cfg_while_true.txt", + r#"def f(): + while True: + v = g() + if v: + break + else: + v = 0 + return v +"#, + include_str!("../../tests/golden/cfg_while_true.txt"), + ); +} + +#[test] +fn cfg_jumps_run_a_copy_of_finally() { + snapshot( + "cfg_finally_jumps.txt", + r#"def f(xs): + for x in xs: + try: + if x: + continue + return x + finally: + done(x) + return None +"#, + include_str!("../../tests/golden/cfg_finally_jumps.txt"), + ); +} + +#[test] +fn cfg_match_arms_and_captures() { + snapshot( + "cfg_match.txt", + r#"def f(s): + match s: + case Point(x=0, y=b) as p: + r = b + case [a, *rest] if a: + r = a + case _: + r = None + return r +"#, + include_str!("../../tests/golden/cfg_match.txt"), + ); +} + +// ---- the try edges, asserted directly -------------------------------------- + +/// Both raise edges into a handler must exist: from `try_entry` (the pre-try +/// state, for may-unbound) and from the body's tail (so a store in the body +/// stays live if the handler reads it). Either alone breaks an ADR-0002 +/// corpus case, so each is asserted here as well as in the snapshot. +#[test] +fn a_handler_is_reached_from_try_entry_and_from_the_body_tail() { + let src = r#"def f(): + try: + a = 1 + except E: + use(a) +"#; + let cfg = only_cfg(src); + let preds = cfg.preds(); + let handler = cfg + .blocks() + .iter() + .position(|b| { + b.stmts + .first() + .is_some_and(|s| src[s.byte_start..].starts_with("except")) + }) + .expect("a block starting with the except clause"); + let def_block = cfg + .blocks() + .iter() + .position(|b| { + b.stmts.iter().any(|s| { + s.events + .iter() + .any(|e| matches!(e.kind, EventKind::Def { .. })) + }) + }) + .expect("the block holding `a = 1`"); + // The body tail (the block that defines `a`) reaches the handler... + assert!(preds[handler].contains(&def_block), "no body-tail edge"); + // ...and so does a block where `a` is not yet defined. + assert!( + preds[handler].iter().any(|&p| p != def_block), + "no try_entry edge" + ); +} + +// ---- scope ------------------------------------------------------------------- + +fn locals_of(src: &str) -> Vec { + only_cfg(src).locals().to_vec() +} + +#[test] +fn a_class_pattern_name_and_a_keyword_name_are_not_locals() { + let locals = locals_of( + r#"def f(s): + match s: + case Point(x=0, y=b) as p: + pass + case [a, *rest] | {"k": a, **rest}: + pass + case Color.RED: + pass +"#, + ); + assert_eq!(locals, ["s", "b", "p", "a", "rest"]); +} + +#[test] +fn a_bare_annotation_is_local_and_global_names_are_not() { + let locals = locals_of( + r#"def f(a, b=1, *args, c: int = 2, **kw): + global g + x: int + g = 1 + import os.path, sys as system + from m import y, z as w + with open() as (h, i): + pass + for j, *k in xs: + pass + del q +"#, + ); + assert_eq!( + locals, + ["a", "b", "args", "c", "kw", "x", "os", "system", "y", "w", "h", "i", "j", "k", "q"] + ); +} + +#[test] +fn a_comprehension_target_does_not_leak_but_its_walrus_does() { + let locals = locals_of("def f(xs):\n return [v for v in xs if (n := v)]\n"); + assert_eq!(locals, ["xs", "n"]); +} + +#[test] +fn a_nested_def_and_class_bind_their_names_but_not_their_bodies() { + let src = r#"def f(): + def g(p): + inner = p + class C: + attr = 1 + lambda q: q +"#; + let us = units(src); + assert_eq!( + us.len(), + 2, + "f and g are units; C's body and the lambda are not" + ); + let FunctionUnit::Analysable(f) = &us[0] else { + panic!("f opaque") + }; + assert_eq!(f.locals(), ["g", "C"]); +} + +// ---- the OPAQUE hatch ------------------------------------------------------ +// +// Every trigger has a known-answer control: the same function without the +// trigger must be analysable. Without the twin, "opaque" could be the builder +// failing for some unrelated reason. + +#[test] +fn each_dynamic_scope_builtin_makes_a_function_opaque() { + for name in ["exec", "eval", "locals", "globals", "vars"] { + let src = format!("def f():\n x = 1\n {name}('x')\n"); + assert_eq!( + opaque_reason(&src), + Some(OpaqueReason::DynamicScope(match name { + "exec" => "exec", + "eval" => "eval", + "locals" => "locals", + "globals" => "globals", + _ => "vars", + })), + "{name}" + ); + } + assert_eq!(opaque_reason("def f():\n x = 1\n run('x')\n"), None); +} + +#[test] +fn a_dynamic_scope_call_in_a_nested_def_does_not_make_the_outer_opaque() { + let us = units("def f():\n def g():\n return locals()\n return g\n"); + assert!(matches!(us[0], FunctionUnit::Analysable(_)), "f"); + assert!(matches!(us[1], FunctionUnit::Opaque { .. }), "g"); +} + +#[test] +fn a_wildcard_import_anywhere_in_the_module_makes_every_function_opaque() { + assert_eq!( + opaque_reason("def f():\n x = 1\n return x\nfrom m import *\n"), + Some(OpaqueReason::WildcardImport) + ); + assert_eq!( + opaque_reason("def f():\n x = 1\n return x\nfrom m import n\n"), + None + ); +} + +#[test] +fn frame_introspection_makes_a_function_opaque() { + assert_eq!( + opaque_reason("def f():\n return sys._getframe(1)\n"), + Some(OpaqueReason::FrameIntrospection) + ); + assert_eq!( + opaque_reason("def f():\n return inspect.currentframe()\n"), + Some(OpaqueReason::FrameIntrospection) + ); + assert_eq!(opaque_reason("def f():\n return sys.argv\n"), None); +} + +#[test] +fn an_except_star_handler_makes_a_function_opaque() { + assert_eq!( + opaque_reason("def f():\n try:\n g()\n except* ValueError:\n pass\n"), + Some(OpaqueReason::ExceptGroup) + ); + assert_eq!( + opaque_reason("def f():\n try:\n g()\n except ValueError:\n pass\n"), + None + ); +} + +/// A unit test, not a fixture: the falsifier panics on a fixture that does +/// not parse, by design. +#[test] +fn a_parse_error_in_the_function_makes_it_opaque() { + assert_eq!( + opaque_reason("def f():\n x = = 1\n"), + Some(OpaqueReason::ParseError) + ); + assert_eq!(opaque_reason("def f():\n x = 1\n"), None); +} + +#[test] +fn more_than_max_locals_makes_a_function_opaque() { + let body: String = (0..=super::MAX_LOCALS) + .map(|i| format!(" v{i} = 0\n")) + .collect(); + assert_eq!( + opaque_reason(&format!("def f():\n{body}")), + Some(OpaqueReason::TooManyLocals(super::MAX_LOCALS + 1)) + ); + let body: String = (0..super::MAX_LOCALS) + .map(|i| format!(" v{i} = 0\n")) + .collect(); + assert_eq!(opaque_reason(&format!("def f():\n{body}")), None); +} diff --git a/crates/pons-core/src/engine.rs b/crates/pons-core/src/engine.rs index 47e8f41..9de4c54 100644 --- a/crates/pons-core/src/engine.rs +++ b/crates/pons-core/src/engine.rs @@ -1,10 +1,12 @@ // SPDX-License-Identifier: MPL-2.0 +use std::cell::OnceCell; use std::collections::BTreeSet; use std::path::Path; use tree_sitter::Tree; +use crate::cfg::{self, FunctionUnit}; use crate::finding::{Finding, RawFinding}; use crate::lang::Lang; use crate::source::DiscoveredFile; @@ -16,6 +18,10 @@ pub struct RuleCtx<'a> { pub lang: Lang, pub text: &'a str, pub tree: &'a Tree, + /// Built at most once per file, on first request, and shared by every T1 + /// (and, from M5, T2) rule. Rebuilding it per rule would be exactly the + /// wasted work this tool exists to flag. + units: OnceCell>, } impl<'a> RuleCtx<'a> { @@ -28,8 +34,18 @@ impl<'a> RuleCtx<'a> { lang, text, tree, + units: OnceCell::new(), } } + + /// Every function unit in this file (ADR-0002). Empty for anything but + /// Python, so a JS/TS/Rust file never pays for a Python CFG. + pub fn units(&self) -> &[FunctionUnit] { + self.units.get_or_init(|| match self.lang { + Lang::Python => cfg::build_units(self.path, self.text, self.tree), + _ => Vec::new(), + }) + } } /// A single check. Implementors live in `pons-rules`; `pons-core` stays diff --git a/crates/pons-core/tests/golden/cfg_comprehension_walrus.txt b/crates/pons-core/tests/golden/cfg_comprehension_walrus.txt new file mode 100644 index 0000000..61faa54 --- /dev/null +++ b/crates/pons-core/tests/golden/cfg_comprehension_walrus.txt @@ -0,0 +1,9 @@ +fn f +locals: xs x n ys +params: xs +b0 (entry) -> [b1] + L2 `x = 1` {def! x} + L3 `ys = [x for x in xs if (n := x)]` {use xs, weak n, def!* ys} + L4 `return n, x, ys` {use n, use x, use ys} +b1 (exit) -> [] +b2 (unreachable) -> [b1] diff --git a/crates/pons-core/tests/golden/cfg_finally_jumps.txt b/crates/pons-core/tests/golden/cfg_finally_jumps.txt new file mode 100644 index 0000000..15e8947 --- /dev/null +++ b/crates/pons-core/tests/golden/cfg_finally_jumps.txt @@ -0,0 +1,33 @@ +fn f +locals: xs x +params: xs +b0 (entry) -> [b2] + L2 `xs` {use xs} +b1 (exit) -> [] +b2 -> [b3 b18] +b3 -> [b5] + L2 `x` {def x} +b4 -> [b1] + L9 `return None` {} +b5 -> [b7] +b6 -> [b1] + L8 `done(x)` {use x} +b7 -> [b6 b8] +b8 -> [b10 b9] + L4 `x` {use x} +b9 -> [b6 b14] +b10 -> [b6 b11] +b11 -> [b12] + L5 `continue` {} +b12 -> [b2] + L8 `done(x)` {use x} +b13 (unreachable) -> [b9] +b14 -> [b15] + L6 `return x` {use x} +b15 -> [b1] + L8 `done(x)` {use x} +b16 (unreachable) -> [b6 b17] +b17 (unreachable) -> [b2] + L8 `done(x)` {use x} +b18 -> [b4] +b19 (unreachable) -> [b1] diff --git a/crates/pons-core/tests/golden/cfg_for_else.txt b/crates/pons-core/tests/golden/cfg_for_else.txt new file mode 100644 index 0000000..fbbe7b0 --- /dev/null +++ b/crates/pons-core/tests/golden/cfg_for_else.txt @@ -0,0 +1,21 @@ +fn f +locals: xs y x +params: xs +b0 (entry) -> [b2] + L2 `xs` {use xs} +b1 (exit) -> [] +b2 -> [b3 b9] +b3 -> [b5] + L2 `x` {def x} +b4 -> [b1] + L8 `return x` {use x} +b5 -> [b7 b6] + L3 `x` {use x} +b6 -> [b2] + L5 `y = x` {use x, def! y} +b7 -> [b4] + L4 `break` {} +b8 (unreachable) -> [b6] +b9 -> [b4] + L7 `y = 0` {def! y} +b10 (unreachable) -> [b1] diff --git a/crates/pons-core/tests/golden/cfg_match.txt b/crates/pons-core/tests/golden/cfg_match.txt new file mode 100644 index 0000000..c41ce7a --- /dev/null +++ b/crates/pons-core/tests/golden/cfg_match.txt @@ -0,0 +1,18 @@ +fn f +locals: s b p r a rest +params: s +b0 (entry) -> [b3 b4 b5] + L2 `s` {use s} +b1 (exit) -> [] +b2 -> [b1] + L9 `return r` {use r} +b3 -> [b2] + L3 `case Point(x=0, y=b) as p:` {use~ b, use~ p, weak b, weak p} + L4 `r = b` {use b, def! r} +b4 -> [b2] + L5 `case [a, *rest] if a:` {use~ a, use~ rest, weak a, weak rest, use a} + L6 `r = a` {use a, def! r} +b5 -> [b2] + L7 `case _:` {} + L8 `r = None` {def! r} +b6 (unreachable) -> [b1] diff --git a/crates/pons-core/tests/golden/cfg_try_except_finally.txt b/crates/pons-core/tests/golden/cfg_try_except_finally.txt new file mode 100644 index 0000000..57e2a9b --- /dev/null +++ b/crates/pons-core/tests/golden/cfg_try_except_finally.txt @@ -0,0 +1,23 @@ +fn f +locals: a b e c +params: +b0 (entry) -> [b4] +b1 (exit) -> [] +b2 -> [b1] + L10 `cleanup(a)` {use a} +b3 -> [b2 b8] + L5 `except E as e:` {def e} +b4 -> [b3 b2 b5] +b5 -> [b3 b2 b6] + L3 `a = g()` {def!* a} +b6 -> [b3 b2 b7] + L4 `b = a` {use a, def! b} +b7 -> [b2 b9] + L8 `c = 1` {def! c} +b8 -> [b2 b9] + L6 `print(e, a)` {use e, use a} + L5 `e` {kill e} +b9 -> [b1] + L10 `cleanup(a)` {use a} + L11 `return a` {use a} +b10 (unreachable) -> [b1] diff --git a/crates/pons-core/tests/golden/cfg_while_true.txt b/crates/pons-core/tests/golden/cfg_while_true.txt new file mode 100644 index 0000000..d05a24b --- /dev/null +++ b/crates/pons-core/tests/golden/cfg_while_true.txt @@ -0,0 +1,19 @@ +fn f +locals: v +params: +b0 (entry) -> [b2] +b1 (exit) -> [] +b2 -> [b4] + L2 `True` {} +b3 -> [b1] + L8 `return v` {use v} +b4 -> [b6 b5] + L3 `v = g()` {def!* v} + L4 `v` {use v} +b5 -> [b2] +b6 -> [b3] + L5 `break` {} +b7 (unreachable) -> [b5] +b8 (unreachable) -> [b3] + L7 `v = 0` {def! v} +b9 (unreachable) -> [b1] From 48a908a37b3079daff0ccb97dc564edc22fdf0aa Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:26:09 +0100 Subject: [PATCH 4/8] =?UTF-8?q?feat(dataflow):=20M4=20step=203=20=E2=80=94?= =?UTF-8?q?=20liveness=20and=20may-unbound=20over=20the=20CFG?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One worklist solver over a union meet, generic in direction, and the two ADR-0002 problems transcribed from its equations. Block-level solutions are replayed per event; dead_stores / unbound_reads combine the per-exit copies of a finally per source location (dead only if dead in every copy; may-unbound if in any), skipping unreachable blocks. 18 corpus cases, each "no finding" beside one that reports. Mutants killed: intersection meet (5 tests red), dropped try-body-tail edge (3 red). A walrus inside a comprehension clearing may-unbound is a chosen, documented false negative. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/dataflow.rs | 282 +++++++++++++++++++++++++ crates/pons-core/src/dataflow/tests.rs | 204 ++++++++++++++++++ crates/pons-core/src/lib.rs | 1 + 3 files changed, 487 insertions(+) create mode 100644 crates/pons-core/src/dataflow.rs create mode 100644 crates/pons-core/src/dataflow/tests.rs diff --git a/crates/pons-core/src/dataflow.rs b/crates/pons-core/src/dataflow.rs new file mode 100644 index 0000000..1ad97ee --- /dev/null +++ b/crates/pons-core/src/dataflow.rs @@ -0,0 +1,282 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! The two T1 analyses over a [`FunctionCfg`], transcribed from ADR-0002: +//! +//! ```text +//! LiveOut(B) = ⋃ { LiveIn(S) : S ∈ succ(B) } +//! LiveIn(B) = use(B) ∪ ( LiveOut(B) \ def(B) ) +//! +//! Unbound_in(entry) = Locals \ Params +//! Unbound_in(B) = ⋃ { Unbound_out(P) : P ∈ pred(B) } +//! ``` +//! +//! Both are powersets of a finite set of locals with union as the meet, so +//! the worklist terminates without a widening operator. Do not add one. +//! +//! Facts are solved per block, then replayed per statement to recover the fact +//! at each event. +//! +//! One source location can appear in several blocks, because the builder +//! copies each `finally` once per way out. The queries at the bottom combine +//! those copies: a store is dead only if it is dead in *every* reachable copy, +//! and a read is may-unbound if it is in *any*. + +use std::collections::{BTreeMap, VecDeque}; + +use crate::cfg::{BlockId, DefReport, Event, EventKind, FunctionCfg, LocalId}; + +/// A set of locals, as a bitset. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LocalSet { + words: Vec, +} + +impl LocalSet { + pub fn empty(n: usize) -> Self { + Self { + words: vec![0; n.div_ceil(64)], + } + } + + pub fn full(n: usize) -> Self { + let mut s = Self::empty(n); + for id in 0..n { + s.insert(id); + } + s + } + + pub fn contains(&self, id: LocalId) -> bool { + self.words[id / 64] & (1 << (id % 64)) != 0 + } + + pub fn insert(&mut self, id: LocalId) { + self.words[id / 64] |= 1 << (id % 64); + } + + pub fn remove(&mut self, id: LocalId) { + self.words[id / 64] &= !(1 << (id % 64)); + } + + /// `self ∪= other`; true if `self` grew. + pub fn union_with(&mut self, other: &LocalSet) -> bool { + let mut changed = false; + for (w, o) in self.words.iter_mut().zip(&other.words) { + let merged = *w | o; + changed |= merged != *w; + *w = merged; + } + changed + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Direction { + Forward, + Backward, +} + +/// A gen/kill problem over [`LocalSet`] with union meet. +pub trait Problem { + const DIRECTION: Direction; + /// The fact at `ENTRY`'s start (forward) or `EXIT`'s end (backward). + fn boundary(&self, cfg: &FunctionCfg) -> LocalSet; + fn transfer(&self, event: &Event, state: &mut LocalSet); +} + +/// The fact at the start and at the end of every block, in program order +/// whatever the direction of the analysis. +#[derive(Debug)] +pub struct Solution { + pub at_start: Vec, + pub at_end: Vec, +} + +/// Every event of `block`, in the analysis's direction. +fn events_of(cfg: &FunctionCfg, block: BlockId) -> Vec<&Event> { + let events = cfg.blocks()[block].stmts.iter().flat_map(|s| &s.events); + match P::DIRECTION { + Direction::Forward => events.collect(), + Direction::Backward => { + let mut v: Vec<_> = events.collect(); + v.reverse(); + v + } + } +} + +pub fn solve(cfg: &FunctionCfg, problem: &P) -> Solution { + let n_locals = cfg.locals().len(); + let n_blocks = cfg.blocks().len(); + let preds = cfg.preds(); + // `into[b]` is where facts enter `b`; `from[b]` where they leave it. + let (into_of, flows_to): (Vec>, Vec>) = match P::DIRECTION { + Direction::Forward => ( + preds, + cfg.blocks().iter().map(|b| b.succs.clone()).collect(), + ), + Direction::Backward => ( + cfg.blocks().iter().map(|b| b.succs.clone()).collect(), + preds, + ), + }; + let boundary_block = match P::DIRECTION { + Direction::Forward => FunctionCfg::ENTRY, + Direction::Backward => FunctionCfg::EXIT, + }; + + let mut into = vec![LocalSet::empty(n_locals); n_blocks]; + let mut from = vec![LocalSet::empty(n_locals); n_blocks]; + let boundary = problem.boundary(cfg); + + let mut queue: VecDeque = (0..n_blocks).collect(); + let mut queued = vec![true; n_blocks]; + while let Some(b) = queue.pop_front() { + queued[b] = false; + let mut state = if b == boundary_block { + boundary.clone() + } else { + LocalSet::empty(n_locals) + }; + // The meet. Both ADR-0002 analyses are may-analyses: union. + for &p in &into_of[b] { + state.union_with(&from[p]); + } + into[b] = state.clone(); + for event in events_of::

(cfg, b) { + problem.transfer(event, &mut state); + } + if state != from[b] { + from[b] = state; + for &s in &flows_to[b] { + if !queued[s] { + queued[s] = true; + queue.push_back(s); + } + } + } + } + + match P::DIRECTION { + Direction::Forward => Solution { + at_start: into, + at_end: from, + }, + Direction::Backward => Solution { + at_start: from, + at_end: into, + }, + } +} + +/// Walks `block` in the analysis's direction, calling `visit` with each event +/// and the fact *before that event's transfer* — so, in program order, the +/// fact just before the event (forward) or just after it (backward). +pub fn replay<'c, P: Problem>( + cfg: &'c FunctionCfg, + problem: &P, + solution: &Solution, + block: BlockId, + mut visit: impl FnMut(&'c Event, &LocalSet), +) { + let mut state = match P::DIRECTION { + Direction::Forward => solution.at_start[block].clone(), + Direction::Backward => solution.at_end[block].clone(), + }; + for event in events_of::

(cfg, block) { + visit(event, &state); + problem.transfer(event, &mut state); + } +} + +/// Backward liveness. +pub struct Liveness; + +impl Problem for Liveness { + const DIRECTION: Direction = Direction::Backward; + + fn boundary(&self, cfg: &FunctionCfg) -> LocalSet { + LocalSet::empty(cfg.locals().len()) + } + + fn transfer(&self, event: &Event, live: &mut LocalSet) { + match event.kind { + EventKind::Use { .. } => live.insert(event.local), + EventKind::Def { .. } | EventKind::Kill => live.remove(event.local), + // A walrus or a match capture may not have run; ending the live + // range there would invent dead stores. + EventKind::WeakDef => {} + } + } +} + +/// Forward may-unbound. +pub struct MayUnbound; + +impl Problem for MayUnbound { + const DIRECTION: Direction = Direction::Forward; + + fn boundary(&self, cfg: &FunctionCfg) -> LocalSet { + let mut unbound = LocalSet::full(cfg.locals().len()); + for &p in cfg.params() { + unbound.remove(p); + } + unbound + } + + fn transfer(&self, event: &Event, unbound: &mut LocalSet) { + match event.kind { + EventKind::Def { .. } | EventKind::WeakDef => unbound.remove(event.local), + EventKind::Kill => unbound.insert(event.local), + EventKind::Use { .. } => {} + } + } +} + +/// Every reportable store whose value no path reads, one per source location. +/// A store in a copied `finally` counts only if it is dead in every copy. +pub fn dead_stores(cfg: &FunctionCfg) -> Vec<(&Event, DefReport)> { + let solution = solve(cfg, &Liveness); + let reachable = cfg.reachable(); + // byte_start → (event, report, dead in every copy so far) + let mut by_loc: BTreeMap = BTreeMap::new(); + for (b, _) in reachable.iter().enumerate().filter(|(_, r)| **r) { + replay(cfg, &Liveness, &solution, b, |event, live_after| { + if let EventKind::Def { + report: Some(report), + } = event.kind + { + let dead = !live_after.contains(event.local); + by_loc + .entry(event.loc.byte_start) + .and_modify(|e| e.2 &= dead) + .or_insert((event, report, dead)); + } + }); + } + // `replay` hands out borrows of `cfg`'s events, so these outlive it. + by_loc + .into_values() + .filter(|(_, _, dead)| *dead) + .map(|(event, report, _)| (event, report)) + .collect() +} + +/// Every checkable read some path reaches with the name unbound, one per +/// source location. +pub fn unbound_reads(cfg: &FunctionCfg) -> Vec<&Event> { + let solution = solve(cfg, &MayUnbound); + let reachable = cfg.reachable(); + let mut by_loc: BTreeMap = BTreeMap::new(); + for (b, _) in reachable.iter().enumerate().filter(|(_, r)| **r) { + replay(cfg, &MayUnbound, &solution, b, |event, unbound| { + if event.kind == (EventKind::Use { checkable: true }) && unbound.contains(event.local) { + by_loc.entry(event.loc.byte_start).or_insert(event); + } + }); + } + by_loc.into_values().collect() +} + +#[cfg(test)] +mod tests; diff --git a/crates/pons-core/src/dataflow/tests.rs b/crates/pons-core/src/dataflow/tests.rs new file mode 100644 index 0000000..269f410 --- /dev/null +++ b/crates/pons-core/src/dataflow/tests.rs @@ -0,0 +1,204 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! Each ADR-0002 corpus case, asserted on the analysis directly. Every "no +//! finding" case sits next to a case that does report, so a solver that +//! reports nothing cannot pass. + +use std::path::Path; + +use tree_sitter::Parser; + +use super::{dead_stores, unbound_reads}; +use crate::cfg::{build_units, FunctionCfg, FunctionUnit}; + +fn cfg(src: &str) -> FunctionCfg { + let mut parser = Parser::new(); + parser + .set_language(&tree_sitter_python::LANGUAGE.into()) + .unwrap(); + let tree = parser.parse(src, None).unwrap(); + match build_units(Path::new("t.py"), src, &tree).remove(0) { + FunctionUnit::Analysable(cfg) => cfg, + FunctionUnit::Opaque { reason, .. } => panic!("opaque: {reason}"), + } +} + +/// `name@line` for each dead store. +fn dead(src: &str) -> Vec { + let cfg = cfg(src); + dead_stores(&cfg) + .iter() + .map(|(e, _)| format!("{}@{}", cfg.locals()[e.local], e.loc.line_start)) + .collect() +} + +fn unbound(src: &str) -> Vec { + let cfg = cfg(src); + unbound_reads(&cfg) + .iter() + .map(|e| format!("{}@{}", cfg.locals()[e.local], e.loc.line_start)) + .collect() +} + +// ---- dead-store --------------------------------------------------------------- + +#[test] +fn an_overwritten_store_is_dead_and_the_read_one_is_not() { + assert_eq!( + dead("def f():\n x = 1\n x = 2\n return x\n"), + ["x@2"] + ); +} + +#[test] +fn a_store_never_read_is_dead() { + assert_eq!(dead("def f():\n x = 1\n"), ["x@2"]); +} + +#[test] +fn a_store_read_round_a_loop_back_edge_is_live() { + assert_eq!( + dead("def f(c):\n n = 0\n while c():\n n = n + 1\n return n\n"), + Vec::::new() + ); + assert_eq!( + dead("def f(c):\n n = 0\n while c():\n n = 1\n return 0\n"), + ["n@2", "n@4"] + ); +} + +#[test] +fn a_store_in_try_read_only_by_the_handler_is_live() { + // The try-body-tail → handler edge. + let src = "def f():\n s = 'start'\n try:\n s = 'mid'\n g()\n except E:\n log(s)\n"; + assert_eq!(dead(src), Vec::::new()); +} + +/// ADR-0002's body-tail → handler edge, pinned. Every statement in a `try` +/// already raises from its own block, so this edge matters only for the state +/// after the *last* statement, where it is conservative: the handler cannot +/// in fact see `s = 'end'`. The ADR mandates the edge; this keeps the store +/// silent rather than reported, which is the direction T1 errs in. +#[test] +fn the_last_store_in_try_is_live_if_a_handler_reads_it() { + let src = "def f():\n try:\n g()\n s = 'end'\n except E:\n log(s)\n"; + assert_eq!(dead(src), Vec::::new()); + assert_eq!(unbound(src), ["s@6"]); +} + +#[test] +fn a_store_read_only_in_finally_on_the_return_path_is_live() { + // The gap-fill: `return` runs the finally, which reads `y`. + let src = "def f():\n try:\n y = 1\n return 0\n finally:\n log(y)\n"; + assert_eq!(dead(src), Vec::::new()); +} + +#[test] +fn a_store_in_finally_is_dead_only_if_dead_in_every_copy() { + // Dead in the exceptional copy (it re-raises), live in the normal one. + let src = "def f():\n try:\n g()\n finally:\n y = 1\n return y\n"; + assert_eq!(dead(src), Vec::::new()); + let src = "def f():\n try:\n g()\n finally:\n y = 1\n return 0\n"; + assert_eq!(dead(src), ["y@5"]); +} + +#[test] +fn a_captured_local_is_never_a_dead_store() { + // The late-binding closure: `h` reads `x` when called, after the rebind. + let src = "def f():\n x = 1\n h = lambda: x\n x = 2\n return h\n"; + assert_eq!(dead(src), Vec::::new()); +} + +#[test] +fn a_store_in_unreachable_code_is_not_reported() { + assert_eq!( + dead("def f():\n return 0\n x = 1\n"), + Vec::::new() + ); +} + +#[test] +fn a_walrus_does_not_end_a_live_range() { + let src = "def f(c):\n n = 0\n if c and (n := g()):\n pass\n return n\n"; + assert_eq!(dead(src), Vec::::new()); +} + +// ---- read-before-init ------------------------------------------------------------ + +#[test] +fn a_read_after_a_one_armed_if_is_may_unbound() { + assert_eq!( + unbound("def f(c):\n if c:\n y = 1\n return y\n"), + ["y@4"] + ); + assert_eq!( + unbound("def f(c):\n if c:\n y = 1\n else:\n y = 2\n return y\n"), + Vec::::new() + ); +} + +#[test] +fn a_parameter_is_bound_on_entry() { + assert_eq!( + unbound("def f(a, *b, c=1, **d):\n return a, b, c, d\n"), + Vec::::new() + ); +} + +#[test] +fn a_handler_read_of_a_name_bound_in_try_is_may_unbound() { + // The try_entry → handler edge. + let src = "def f():\n try:\n a = g()\n except E:\n log(a)\n"; + assert_eq!(unbound(src), ["a@5"]); +} + +#[test] +fn del_makes_a_name_unbound_again_and_reaches_the_handler() { + assert_eq!( + unbound("def f():\n x = 1\n del x\n return x\n"), + ["x@4"] + ); + let src = "def f():\n x = 1\n try:\n del x\n g()\n except E:\n log(x)\n"; + assert_eq!(unbound(src), ["x@7"]); +} + +#[test] +fn the_except_alias_is_unbound_after_its_handler() { + let src = "def f():\n try:\n g()\n except E as e:\n pass\n return e\n"; + assert_eq!(unbound(src), ["e@6"]); +} + +#[test] +fn the_loop_variable_after_a_loop_that_may_not_run_is_may_unbound() { + assert_eq!( + unbound("def f(xs):\n for x in xs:\n pass\n return x\n"), + ["x@4"] + ); +} + +#[test] +fn a_while_true_loop_exits_only_through_break() { + let src = "def f():\n while True:\n v = g()\n if v:\n break\n return v\n"; + assert_eq!(unbound(src), Vec::::new()); +} + +#[test] +fn a_walrus_binds_for_may_unbound() { + assert_eq!( + unbound("def f():\n if (n := g()):\n pass\n return n\n"), + Vec::::new() + ); + // A known false negative, chosen: a walrus inside a comprehension runs + // only if the iterable is non-empty, so `n` really may be unbound here. + // Reporting it would also fire on `if any((m := p(s)) for s in xs): + // use(m)`, where `any()` being true guarantees the binding — a false + // positive on a common idiom. T1 errs silent. + let src = "def f(xs):\n if any((n := x) for x in xs):\n pass\n return n\n"; + assert_eq!(unbound(src), Vec::::new()); +} + +#[test] +fn a_read_in_a_nested_function_is_not_judged_here() { + let src = "def f():\n def g():\n return y\n y = 1\n return g\n"; + assert_eq!(unbound(src), Vec::::new()); +} diff --git a/crates/pons-core/src/lib.rs b/crates/pons-core/src/lib.rs index 380f88c..d0613d3 100644 --- a/crates/pons-core/src/lib.rs +++ b/crates/pons-core/src/lib.rs @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 pub mod cfg; +pub mod dataflow; pub mod engine; pub mod finding; pub mod lang; From c75c68922b8a3b42c120fd5c51c3c8189f5e79ec Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:32:21 +0100 Subject: [PATCH 5/8] =?UTF-8?q?feat(rules):=20M4=20steps=204=E2=80=935=20?= =?UTF-8?q?=E2=80=94=20T1=20rules=20dead-store=20and=20read-before-init?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule 9 `dead-store` and rule 10 `read-before-init`, transcribed from ADR-0002 over `RuleCtx::units()` and `pons_core::dataflow`. Both are Tier::T1 (DATAFLOW, derived from the tier), Python only, and never see an OPAQUE function: `FunctionUnit::Opaque` carries no CFG. dead-store: `_`/`_`-prefixed targets suppressed; an RHS with a call, await, yield or walrus reports at INFO with the ADR's counter-condition; otherwise WARN. Never ERROR. read-before-init: WARN uniformly. Registered after the T0 catalogue; the ordered-id test is widened and renamed. RawFinding gains read-only getters so rule tests can pin a severity and counter-condition, not only a count. No setters. Fixtures carry every ADR-0002 corpus obligation, one case per file, plus an `exec(` negative with a known-answer positive twin per rule, and a module-level negative. Mutants killed: dropping `exec` from the OPAQUE triggers turns dead-store/negative/opaque_exec.py red; deleting the `_` suppression turns dead-store/negative/underscore_target.py red. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/finding.rs | 18 +++ crates/pons-rules/src/lib.rs | 1 + crates/pons-rules/src/registry.rs | 10 +- crates/pons-rules/src/t1/dead_store.rs | 129 ++++++++++++++++++ crates/pons-rules/src/t1/mod.rs | 35 +++++ crates/pons-rules/src/t1/read_before_init.rs | 110 +++++++++++++++ fixtures/dead-store/README.adoc | 27 ++++ .../dead-store/negative/conditional_use.py | 5 + .../negative/loop_var_after_loop.py | 5 + fixtures/dead-store/negative/module_level.py | 3 + fixtures/dead-store/negative/opaque_exec.py | 6 + .../negative/read_only_in_except.py | 7 + .../dead-store/negative/underscore_target.py | 4 + .../dead-store/positive/opaque_control.py | 7 + fixtures/dead-store/positive/plain.py | 4 + .../positive/redefined_before_use.py | 4 + fixtures/dead-store/positive/rhs_call.py | 4 + fixtures/read-before-init/README.adoc | 23 ++++ .../negative/bound_on_every_path.py | 6 + .../negative/comprehension_var_shadow.py | 4 + .../read-before-init/negative/global_name.py | 4 + .../read-before-init/negative/module_level.py | 3 + .../read-before-init/negative/opaque_exec.py | 6 + .../positive/aug_assign_unbound.py | 3 + .../read-before-init/positive/else_less_if.py | 4 + .../positive/for_target_after_loop.py | 4 + .../positive/handler_reads_try_name.py | 5 + .../positive/opaque_control.py | 7 + 28 files changed, 446 insertions(+), 2 deletions(-) create mode 100644 crates/pons-rules/src/t1/dead_store.rs create mode 100644 crates/pons-rules/src/t1/mod.rs create mode 100644 crates/pons-rules/src/t1/read_before_init.rs create mode 100644 fixtures/dead-store/README.adoc create mode 100644 fixtures/dead-store/negative/conditional_use.py create mode 100644 fixtures/dead-store/negative/loop_var_after_loop.py create mode 100644 fixtures/dead-store/negative/module_level.py create mode 100644 fixtures/dead-store/negative/opaque_exec.py create mode 100644 fixtures/dead-store/negative/read_only_in_except.py create mode 100644 fixtures/dead-store/negative/underscore_target.py create mode 100644 fixtures/dead-store/positive/opaque_control.py create mode 100644 fixtures/dead-store/positive/plain.py create mode 100644 fixtures/dead-store/positive/redefined_before_use.py create mode 100644 fixtures/dead-store/positive/rhs_call.py create mode 100644 fixtures/read-before-init/README.adoc create mode 100644 fixtures/read-before-init/negative/bound_on_every_path.py create mode 100644 fixtures/read-before-init/negative/comprehension_var_shadow.py create mode 100644 fixtures/read-before-init/negative/global_name.py create mode 100644 fixtures/read-before-init/negative/module_level.py create mode 100644 fixtures/read-before-init/negative/opaque_exec.py create mode 100644 fixtures/read-before-init/positive/aug_assign_unbound.py create mode 100644 fixtures/read-before-init/positive/else_less_if.py create mode 100644 fixtures/read-before-init/positive/for_target_after_loop.py create mode 100644 fixtures/read-before-init/positive/handler_reads_try_name.py create mode 100644 fixtures/read-before-init/positive/opaque_control.py diff --git a/crates/pons-core/src/finding.rs b/crates/pons-core/src/finding.rs index b5fe0d5..72eb264 100644 --- a/crates/pons-core/src/finding.rs +++ b/crates/pons-core/src/finding.rs @@ -245,6 +245,24 @@ impl RawFinding { } } + // Read-only views, for rule tests that must pin a severity or a + // counter-condition rather than only a count. There are no setters. + pub fn tier(&self) -> Tier { + self.tier + } + + pub fn severity(&self) -> Severity { + self.severity + } + + pub fn location(&self) -> &Location { + &self.location + } + + pub fn counter_condition(&self) -> Option<&str> { + self.counter_condition.as_deref() + } + pub(crate) fn into_finding(self, rule_id: impl Into) -> Finding { Finding::new( rule_id, diff --git a/crates/pons-rules/src/lib.rs b/crates/pons-rules/src/lib.rs index 2d92958..56c8413 100644 --- a/crates/pons-rules/src/lib.rs +++ b/crates/pons-rules/src/lib.rs @@ -2,3 +2,4 @@ pub mod registry; pub mod t0; +pub mod t1; diff --git a/crates/pons-rules/src/registry.rs b/crates/pons-rules/src/registry.rs index 225e031..e5d80a5 100644 --- a/crates/pons-rules/src/registry.rs +++ b/crates/pons-rules/src/registry.rs @@ -10,6 +10,8 @@ use crate::t0::string_concat_in_loop::StringConcatInLoop; use crate::t0::swallowed_error::SwallowedError; use crate::t0::unreachable_after_jump::UnreachableAfterJump; use crate::t0::while_true_no_break::WhileTrueNoBreak; +use crate::t1::dead_store::DeadStore; +use crate::t1::read_before_init::ReadBeforeInit; /// Builds the full set of registered rules — the single source of truth /// both `pons-cli` and the falsifier gate (`tests/falsifier.rs`) read from, @@ -28,6 +30,8 @@ impl RuleRegistry { Box::new(EmptyEffectLoop::new()), Box::new(UnreachableAfterJump::new()), Box::new(StringConcatInLoop::new()), + Box::new(DeadStore::new()), + Box::new(ReadBeforeInit::new()), ] } } @@ -37,7 +41,7 @@ mod tests { use super::*; #[test] - fn registry_registers_the_t0_catalogue_as_it_lands() { + fn registry_registers_the_catalogue_in_order_as_it_lands() { let ids: Vec<&str> = RuleRegistry::all().iter().map(|r| r.id()).collect(); assert_eq!( ids, @@ -49,7 +53,9 @@ mod tests { "while-true-no-break", "empty-effect-loop", "unreachable-after-jump", - "string-concat-in-loop" + "string-concat-in-loop", + "dead-store", + "read-before-init" ] ); } diff --git a/crates/pons-rules/src/t1/dead_store.rs b/crates/pons-rules/src/t1/dead_store.rs new file mode 100644 index 0000000..6ab1f1b --- /dev/null +++ b/crates/pons-rules/src/t1/dead_store.rs @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: MPL-2.0 + +use pons_core::cfg::FunctionUnit; +use pons_core::dataflow; +use pons_core::engine::{Rule, RuleCtx}; +use pons_core::finding::{RawFinding, Severity, Tier}; +use pons_core::lang::Lang; + +const LANGUAGES: &[Lang] = &[Lang::Python]; + +const EFFECT_COUNTER_CONDITION: &str = "the binding is dead but its right-hand side may have \ + side effects worth keeping; drop the binding, not the call."; + +/// Rule 9: a store to a local that no path reads before it is redefined or the +/// function ends. Counter-conditions in ADR-0002's order: a `_`-prefixed +/// target is a declared throwaway and is not reported; a right-hand side with +/// a call, `await`, `yield` or walrus is reported at `INFO`; anything else is +/// `WARN`. Never `ERROR` — a program can be correct with a dead store. +pub struct DeadStore; + +impl DeadStore { + pub fn new() -> Self { + Self + } +} + +impl Default for DeadStore { + fn default() -> Self { + Self::new() + } +} + +impl Rule for DeadStore { + fn id(&self) -> &'static str { + "dead-store" + } + + fn description(&self) -> &'static str { + "a value stored and then overwritten before any read" + } + + fn languages(&self) -> &'static [Lang] { + LANGUAGES + } + + fn check(&self, ctx: &RuleCtx) -> Vec { + let mut findings = Vec::new(); + for unit in ctx.units() { + let FunctionUnit::Analysable(cfg) = unit else { + continue; + }; + for (event, report) in dataflow::dead_stores(cfg) { + let name = &cfg.locals()[event.local]; + if name.starts_with('_') { + continue; + } + let (severity, counter_condition) = if report.rhs_has_effect { + (Severity::Info, Some(EFFECT_COUNTER_CONDITION.to_string())) + } else { + (Severity::Warn, None) + }; + findings.push(RawFinding::new( + Tier::T1, + severity, + event.loc.clone(), + "value stored but never read", + format!( + "`{name}` is assigned here, and every path from this point in `{}` \ + reassigns it or returns before reading it", + cfg.name() + ), + counter_condition, + )); + } + } + findings + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::t1::test_util::findings_for; + + fn check(text: &str) -> Vec { + findings_for(&DeadStore::new(), text) + } + + #[test] + fn a_plain_dead_store_is_warn_with_no_counter_condition() { + let f = check("def f():\n x = 1\n x = 2\n return x\n"); + assert_eq!(f.len(), 1); + assert_eq!(f[0].severity(), Severity::Warn); + assert_eq!(f[0].tier(), Tier::T1); + assert_eq!(f[0].counter_condition(), None); + assert_eq!(f[0].location().line_start, 2); + } + + #[test] + fn a_dead_store_of_a_call_is_info_with_the_adr_counter_condition() { + let f = check("def f():\n x = g()\n x = 2\n return x\n"); + assert_eq!(f.len(), 1); + assert_eq!(f[0].severity(), Severity::Info); + assert_eq!(f[0].counter_condition(), Some(EFFECT_COUNTER_CONDITION)); + } + + #[test] + fn an_underscore_target_is_suppressed_and_its_twin_is_not() { + assert!(check("def f():\n _ = g()\n _tmp = 1\n return 0\n").is_empty()); + assert_eq!(check("def f():\n tmp = 1\n return 0\n").len(), 1); + } + + #[test] + fn a_function_calling_exec_produces_no_finding_and_its_twin_does() { + // PLAN.adoc M4 exit gate. + assert!( + check("def f():\n x = 1\n exec('x = 2')\n x = 3\n return x\n").is_empty() + ); + assert_eq!( + check("def f():\n x = 1\n run('x = 2')\n x = 3\n return x\n").len(), + 1 + ); + } + + #[test] + fn module_level_code_is_not_a_unit() { + assert!(check("x = 1\nx = 2\n").is_empty()); + } +} diff --git a/crates/pons-rules/src/t1/mod.rs b/crates/pons-rules/src/t1/mod.rs new file mode 100644 index 0000000..3fdf243 --- /dev/null +++ b/crates/pons-rules/src/t1/mod.rs @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! T1: intraprocedural dataflow over the Python CFG (ADR-0002). +//! +//! These rules depart from the `.scm` house pattern: a CFG needs structural +//! recursion, not pattern matching. They read [`RuleCtx::units`], which is +//! built once per file and shared, and they never see an opaque function — +//! [`FunctionUnit::Opaque`] carries no CFG to analyse. +//! +//! [`RuleCtx::units`]: pons_core::engine::RuleCtx::units +//! [`FunctionUnit::Opaque`]: pons_core::cfg::FunctionUnit::Opaque + +pub mod dead_store; +pub mod read_before_init; + +#[cfg(test)] +pub(crate) mod test_util { + use pons_core::engine::{Rule, RuleCtx}; + use pons_core::finding::RawFinding; + use pons_core::lang::Lang; + use pons_core::parse; + use pons_core::source::SourceFile; + use std::path::PathBuf; + + pub fn findings_for(rule: &dyn Rule, text: &str) -> Vec { + let source = SourceFile { + path: PathBuf::from("test.py"), + lang: Lang::Python, + text: text.to_string(), + }; + let tree = parse::parse(&source).unwrap(); + let ctx = RuleCtx::new(&source.path, source.lang, &source.text, &tree); + rule.check(&ctx) + } +} diff --git a/crates/pons-rules/src/t1/read_before_init.rs b/crates/pons-rules/src/t1/read_before_init.rs new file mode 100644 index 0000000..aaf5641 --- /dev/null +++ b/crates/pons-rules/src/t1/read_before_init.rs @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: MPL-2.0 + +use pons_core::cfg::FunctionUnit; +use pons_core::dataflow; +use pons_core::engine::{Rule, RuleCtx}; +use pons_core::finding::{RawFinding, Severity, Tier}; +use pons_core::lang::Lang; + +const LANGUAGES: &[Lang] = &[Lang::Python]; + +/// Rule 10: a read of a local that some path reaches before the name is +/// bound — Python raises `UnboundLocalError`. `WARN` uniformly in v0.1.0: +/// raising the every-path case to `ERROR` would need a must-analysis, which +/// ADR-0002 declines. Python has no default initialisation, so the rule has +/// no counter-condition; opacity and scope, its only false-positive sources, +/// are handled by the OPAQUE hatch and the locals pre-pass. +pub struct ReadBeforeInit; + +impl ReadBeforeInit { + pub fn new() -> Self { + Self + } +} + +impl Default for ReadBeforeInit { + fn default() -> Self { + Self::new() + } +} + +impl Rule for ReadBeforeInit { + fn id(&self) -> &'static str { + "read-before-init" + } + + fn description(&self) -> &'static str { + "a local read on a path where it may not yet be bound" + } + + fn languages(&self) -> &'static [Lang] { + LANGUAGES + } + + fn check(&self, ctx: &RuleCtx) -> Vec { + let mut findings = Vec::new(); + for unit in ctx.units() { + let FunctionUnit::Analysable(cfg) = unit else { + continue; + }; + for event in dataflow::unbound_reads(cfg) { + let name = &cfg.locals()[event.local]; + findings.push(RawFinding::new( + Tier::T1, + Severity::Warn, + event.loc.clone(), + "local may be read before it is assigned", + format!( + "on at least one path through `{}`, `{name}` is read here before it \ + is assigned — Python raises `UnboundLocalError`", + cfg.name() + ), + None, + )); + } + } + findings + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::t1::test_util::findings_for; + + fn check(text: &str) -> Vec { + findings_for(&ReadBeforeInit::new(), text) + } + + #[test] + fn a_read_after_an_else_less_if_is_warn_and_the_two_armed_twin_is_clean() { + let f = check("def f(c):\n if c:\n y = 1\n return y\n"); + assert_eq!(f.len(), 1); + assert_eq!(f[0].severity(), Severity::Warn); + assert_eq!(f[0].tier(), Tier::T1); + assert_eq!(f[0].location().line_start, 4); + assert!(check( + "def f(c):\n if c:\n y = 1\n else:\n y = 2\n return y\n" + ) + .is_empty()); + } + + #[test] + fn augmented_assignment_with_no_binding_is_a_genuine_read() { + assert_eq!(check("def f():\n n += 1\n return n\n").len(), 1); + } + + #[test] + fn a_global_name_is_never_checked() { + assert!(check("def f():\n global n\n n += 1\n return n\n").is_empty()); + } + + #[test] + fn a_function_calling_exec_produces_no_finding_and_its_twin_does() { + assert!(check("def f():\n exec('y = 1')\n return y\n y = 2\n").is_empty()); + assert_eq!( + check("def f():\n run('y = 1')\n return y\n y = 2\n").len(), + 1 + ); + } +} diff --git a/fixtures/dead-store/README.adoc b/fixtures/dead-store/README.adoc new file mode 100644 index 0000000..e6dca3e --- /dev/null +++ b/fixtures/dead-store/README.adoc @@ -0,0 +1,27 @@ += Fixtures: dead-store + +T1 (DATAFLOW), Python only. The corpus is ADR-0002's, one case per file, +because the falsifier asks each positive file for at least one finding and +a file holding several cases would prove only one of them. + +Positive fixtures: a plain overwritten store (`plain.py`, `WARN`); a dead +store whose right-hand side is a call (`rhs_call.py`, reported at `INFO` +with the "drop the binding, not the call" counter-condition); a +redefinition before any use (`redefined_before_use.py`); and +`opaque_control.py`, the known-answer twin of `negative/opaque_exec.py`. + +Negative fixtures, and why each is a legitimate non-finding: + +* `underscore_target.py` — `_` and `_`-prefixed names are declared + throwaways; suppressing them is ADR-0002's first counter-condition. +* `read_only_in_except.py` — both stores are read by the handler, which a + raise in the `try` body can reach from either point. +* `loop_var_after_loop.py` — the loop may not run, so `return last` can read + the initial `None`. +* `conditional_use.py` — read on one branch is read; liveness is a + may-analysis. +* `opaque_exec.py` — `exec` can read any local by name, so the function is + OPAQUE and no T1 rule sees it. Silent *because of* `exec`: its twin in + `positive/` differs only in the call and does report. +* `module_level.py` — module-level names are importable from elsewhere, so a + store to one is never provably dead; only function bodies are units. diff --git a/fixtures/dead-store/negative/conditional_use.py b/fixtures/dead-store/negative/conditional_use.py new file mode 100644 index 0000000..e2a69bc --- /dev/null +++ b/fixtures/dead-store/negative/conditional_use.py @@ -0,0 +1,5 @@ +def f(c): + x = 1 + if c: + return x + return 0 diff --git a/fixtures/dead-store/negative/loop_var_after_loop.py b/fixtures/dead-store/negative/loop_var_after_loop.py new file mode 100644 index 0000000..b953588 --- /dev/null +++ b/fixtures/dead-store/negative/loop_var_after_loop.py @@ -0,0 +1,5 @@ +def last_of(xs): + last = None + for last in xs: + pass + return last diff --git a/fixtures/dead-store/negative/module_level.py b/fixtures/dead-store/negative/module_level.py new file mode 100644 index 0000000..3851a62 --- /dev/null +++ b/fixtures/dead-store/negative/module_level.py @@ -0,0 +1,3 @@ +# Module-level names are importable: a store to one is never provably dead. +x = 1 +x = 2 diff --git a/fixtures/dead-store/negative/opaque_exec.py b/fixtures/dead-store/negative/opaque_exec.py new file mode 100644 index 0000000..3366401 --- /dev/null +++ b/fixtures/dead-store/negative/opaque_exec.py @@ -0,0 +1,6 @@ +# `exec` can read any local by name, so no store here is provably dead. +def f(): + x = 1 + exec("print(x)") + x = 2 + return x diff --git a/fixtures/dead-store/negative/read_only_in_except.py b/fixtures/dead-store/negative/read_only_in_except.py new file mode 100644 index 0000000..21f2bf6 --- /dev/null +++ b/fixtures/dead-store/negative/read_only_in_except.py @@ -0,0 +1,7 @@ +def f(): + state = "start" + try: + state = "mid" + step() + except ValueError: + log(state) diff --git a/fixtures/dead-store/negative/underscore_target.py b/fixtures/dead-store/negative/underscore_target.py new file mode 100644 index 0000000..8b9d1ec --- /dev/null +++ b/fixtures/dead-store/negative/underscore_target.py @@ -0,0 +1,4 @@ +def f(): + _ = g() + _unused = 1 + return 0 diff --git a/fixtures/dead-store/positive/opaque_control.py b/fixtures/dead-store/positive/opaque_control.py new file mode 100644 index 0000000..b3fa454 --- /dev/null +++ b/fixtures/dead-store/positive/opaque_control.py @@ -0,0 +1,7 @@ +# Known-answer twin of negative/opaque_exec.py: identical but for the +# call, so the negative is silent because of `exec`, not by accident. +def f(): + x = 1 + run("print(x)") + x = 2 + return x diff --git a/fixtures/dead-store/positive/plain.py b/fixtures/dead-store/positive/plain.py new file mode 100644 index 0000000..e3adbe5 --- /dev/null +++ b/fixtures/dead-store/positive/plain.py @@ -0,0 +1,4 @@ +def area(w, h): + result = 0 + result = w * h + return result diff --git a/fixtures/dead-store/positive/redefined_before_use.py b/fixtures/dead-store/positive/redefined_before_use.py new file mode 100644 index 0000000..7a9d490 --- /dev/null +++ b/fixtures/dead-store/positive/redefined_before_use.py @@ -0,0 +1,4 @@ +def load(path): + config = {} + config = parse(path) + return config diff --git a/fixtures/dead-store/positive/rhs_call.py b/fixtures/dead-store/positive/rhs_call.py new file mode 100644 index 0000000..cdb743c --- /dev/null +++ b/fixtures/dead-store/positive/rhs_call.py @@ -0,0 +1,4 @@ +def reload(cache): + entry = cache.pop("k") + entry = None + return entry diff --git a/fixtures/read-before-init/README.adoc b/fixtures/read-before-init/README.adoc new file mode 100644 index 0000000..8f330f9 --- /dev/null +++ b/fixtures/read-before-init/README.adoc @@ -0,0 +1,23 @@ += Fixtures: read-before-init + +T1 (DATAFLOW), Python only, `WARN` uniformly: a may-analysis, so a finding +means "on at least one path", never "on every path". One case per file, as +in `dead-store/`. + +Positive fixtures: a read after an `else`-less `if` (`else_less_if.py`); a +`for` target read after a loop that may not run +(`for_target_after_loop.py`); `count += 1` with no prior binding, which +Python compiles to a read (`aug_assign_unbound.py`); a handler reading a +name the `try` body may not have bound yet (`handler_reads_try_name.py`); +and `opaque_control.py`, the known-answer twin of `negative/opaque_exec.py`. + +Negative fixtures, and why each is a legitimate non-finding: + +* `global_name.py` — a `global` name is not a local, so it is never + checked. +* `bound_on_every_path.py` — both arms bind `y` before the join. +* `comprehension_var_shadow.py` — a comprehension's `for` target lives in + its own scope; the outer `x` bound afterwards is a different name. +* `opaque_exec.py` — `exec` can bind `y`, so the function is OPAQUE. Its twin + in `positive/` differs only in the call and does report. +* `module_level.py` — module-level code is not an analysis unit in v0.1.0. diff --git a/fixtures/read-before-init/negative/bound_on_every_path.py b/fixtures/read-before-init/negative/bound_on_every_path.py new file mode 100644 index 0000000..f04c4de --- /dev/null +++ b/fixtures/read-before-init/negative/bound_on_every_path.py @@ -0,0 +1,6 @@ +def f(c): + if c: + y = 1 + else: + y = 2 + return y diff --git a/fixtures/read-before-init/negative/comprehension_var_shadow.py b/fixtures/read-before-init/negative/comprehension_var_shadow.py new file mode 100644 index 0000000..8f73115 --- /dev/null +++ b/fixtures/read-before-init/negative/comprehension_var_shadow.py @@ -0,0 +1,4 @@ +def f(xs): + ys = [x for x in xs] + x = 1 + return ys, x diff --git a/fixtures/read-before-init/negative/global_name.py b/fixtures/read-before-init/negative/global_name.py new file mode 100644 index 0000000..368a61a --- /dev/null +++ b/fixtures/read-before-init/negative/global_name.py @@ -0,0 +1,4 @@ +def bump(): + global count + count += 1 + return count diff --git a/fixtures/read-before-init/negative/module_level.py b/fixtures/read-before-init/negative/module_level.py new file mode 100644 index 0000000..bc8d5f6 --- /dev/null +++ b/fixtures/read-before-init/negative/module_level.py @@ -0,0 +1,3 @@ +# Module-level code is not an analysis unit in v0.1.0. +print(y) +y = 1 diff --git a/fixtures/read-before-init/negative/opaque_exec.py b/fixtures/read-before-init/negative/opaque_exec.py new file mode 100644 index 0000000..f427119 --- /dev/null +++ b/fixtures/read-before-init/negative/opaque_exec.py @@ -0,0 +1,6 @@ +# `exec` can bind `y`, so its read is not judged. +def f(c): + exec("y = 1") + if c: + y = 2 + return y diff --git a/fixtures/read-before-init/positive/aug_assign_unbound.py b/fixtures/read-before-init/positive/aug_assign_unbound.py new file mode 100644 index 0000000..4791e23 --- /dev/null +++ b/fixtures/read-before-init/positive/aug_assign_unbound.py @@ -0,0 +1,3 @@ +def bump(): + count += 1 + return count diff --git a/fixtures/read-before-init/positive/else_less_if.py b/fixtures/read-before-init/positive/else_less_if.py new file mode 100644 index 0000000..e3cf532 --- /dev/null +++ b/fixtures/read-before-init/positive/else_less_if.py @@ -0,0 +1,4 @@ +def f(c): + if c: + y = 1 + return y diff --git a/fixtures/read-before-init/positive/for_target_after_loop.py b/fixtures/read-before-init/positive/for_target_after_loop.py new file mode 100644 index 0000000..86a4d92 --- /dev/null +++ b/fixtures/read-before-init/positive/for_target_after_loop.py @@ -0,0 +1,4 @@ +def last_of(xs): + for item in xs: + pass + return item diff --git a/fixtures/read-before-init/positive/handler_reads_try_name.py b/fixtures/read-before-init/positive/handler_reads_try_name.py new file mode 100644 index 0000000..d63d029 --- /dev/null +++ b/fixtures/read-before-init/positive/handler_reads_try_name.py @@ -0,0 +1,5 @@ +def f(): + try: + conn = connect() + except OSError: + conn.close() diff --git a/fixtures/read-before-init/positive/opaque_control.py b/fixtures/read-before-init/positive/opaque_control.py new file mode 100644 index 0000000..1777906 --- /dev/null +++ b/fixtures/read-before-init/positive/opaque_control.py @@ -0,0 +1,7 @@ +# Known-answer twin of negative/opaque_exec.py: identical but for the +# call, so the negative is silent because of `exec`, not by accident. +def f(c): + run("y = 1") + if c: + y = 2 + return y From caee724394e0fd8b341756128beebbf719dbb5a6 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:49:24 +0100 Subject: [PATCH 6/8] =?UTF-8?q?fix(t1):=20address=20#46=20review=20?= =?UTF-8?q?=E2=80=94=20probe=20coverage,=20honest=20rule=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - grammar probe: assert the 10 further fields the builder reads (elif/else/with_item/decorated_definition/attribute/keyword_argument/ default_parameter, plus guard/superclasses/with-as alias found by shape); a mutant renaming `value` and `guard` turned both tests red. - dead-store description now transcribes ADR-0002's firing condition ("never read before it is redefined or the function ends"); the old string described only the overwrite case. - read-before-init doc no longer claims opacity and scope are the only false-positive sources — the infeasible-path classes are #47. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/cfg/grammar_probe.rs | 48 +++++++++++++++++++- crates/pons-rules/src/t1/dead_store.rs | 2 +- crates/pons-rules/src/t1/read_before_init.rs | 6 ++- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/crates/pons-core/src/cfg/grammar_probe.rs b/crates/pons-core/src/cfg/grammar_probe.rs index 46c3f11..75acd8e 100644 --- a/crates/pons-core/src/cfg/grammar_probe.rs +++ b/crates/pons-core/src/cfg/grammar_probe.rs @@ -48,6 +48,10 @@ class Widget: return a + b +class Sub(Widget): + pass + + def plain(a, b): total: int = 0 annotated_only: int @@ -82,7 +86,7 @@ def plain(a, b): else: total += risky finally: - cleanup() + cleanup(force=True) with open("f") as handle: data = handle.read() @@ -537,6 +541,13 @@ fn the_fields_the_builder_will_use_are_all_present() { ("case_clause", &["consequence"]), ("match_statement", &["subject", "body"]), ("augmented_assignment", &["left", "operator", "right"]), + ("elif_clause", &["condition", "consequence"]), + ("else_clause", &["body"]), + ("with_item", &["value"]), + ("decorated_definition", &["definition"]), + ("attribute", &["object"]), + ("keyword_argument", &["value"]), + ("default_parameter", &["value"]), ]; for (kind, fields) in cases { @@ -549,3 +560,38 @@ fn the_fields_the_builder_will_use_are_all_present() { ); } } + +/// The fields above sit on the first node of their kind. These three sit only +/// on a later one — a guarded `case`, a subclass, the `with … as` binding — +/// so each is found by shape rather than by first occurrence. +/// What is sought, the field it must expose, and how to recognise it. +type ShapedCase<'a> = (&'a str, &'a str, &'a dyn Fn(Node<'_>) -> bool); + +#[test] +fn the_fields_only_some_nodes_of_a_kind_carry_are_present() { + let tree = parse_python(CORPUS); + let root = tree.root_node(); + + let cases: &[ShapedCase] = &[ + ("guarded case_clause", "guard", &|n| { + n.kind() == "case_clause" && n.utf8_text(CORPUS.as_bytes()).unwrap().contains(" if ") + }), + ("subclass class_definition", "superclasses", &|n| { + n.kind() == "class_definition" + && n.utf8_text(CORPUS.as_bytes()) + .unwrap() + .starts_with("class Sub") + }), + ("with_item as_pattern", "alias", &|n| { + n.kind() == "as_pattern" && n.parent().is_some_and(|p| p.kind() == "with_item") + }), + ]; + + for (what, field, pred) in cases { + let node = find_where(root, *pred).unwrap_or_else(|| panic!("corpus has no {what}")); + assert!( + node.child_by_field_name(field).is_some(), + "{what} does not expose `{field}`, which the builder reads" + ); + } +} diff --git a/crates/pons-rules/src/t1/dead_store.rs b/crates/pons-rules/src/t1/dead_store.rs index 6ab1f1b..a121b63 100644 --- a/crates/pons-rules/src/t1/dead_store.rs +++ b/crates/pons-rules/src/t1/dead_store.rs @@ -36,7 +36,7 @@ impl Rule for DeadStore { } fn description(&self) -> &'static str { - "a value stored and then overwritten before any read" + "a value stored and never read before it is redefined or the function ends" } fn languages(&self) -> &'static [Lang] { diff --git a/crates/pons-rules/src/t1/read_before_init.rs b/crates/pons-rules/src/t1/read_before_init.rs index aaf5641..09de3f8 100644 --- a/crates/pons-rules/src/t1/read_before_init.rs +++ b/crates/pons-rules/src/t1/read_before_init.rs @@ -12,8 +12,10 @@ const LANGUAGES: &[Lang] = &[Lang::Python]; /// bound — Python raises `UnboundLocalError`. `WARN` uniformly in v0.1.0: /// raising the every-path case to `ERROR` would need a must-analysis, which /// ADR-0002 declines. Python has no default initialisation, so the rule has -/// no counter-condition; opacity and scope, its only false-positive sources, -/// are handled by the OPAQUE hatch and the locals pre-pass. +/// no counter-condition. Opacity and scope are handled by the OPAQUE hatch and +/// the locals pre-pass; the analysis is also path-insensitive, so correlated +/// guards and loops over non-empty literals yield infeasible-path false +/// positives that ADR-0002 does not yet name (#47). pub struct ReadBeforeInit; impl ReadBeforeInit { From 288582e9ffe80b477aaf7be5f63d3433313c06fd Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:54:27 +0000 Subject: [PATCH 7/8] docs(analysis): document CFG construction, dataflow contracts, and T1 rule behavior Re-signed by the maintainer from unsigned 2ed317d (PR #46) so the branch satisfies main's required_signatures rule; content reviewed and unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/cfg/builder.rs | 33 +++++++++++++++--- crates/pons-core/src/cfg/mod.rs | 27 +++++++++++++++ crates/pons-core/src/cfg/scope.rs | 17 ++++++++++ crates/pons-core/src/dataflow.rs | 35 ++++++++++++++++++-- crates/pons-core/src/engine.rs | 3 ++ crates/pons-rules/src/registry.rs | 1 + crates/pons-rules/src/t1/dead_store.rs | 4 +++ crates/pons-rules/src/t1/read_before_init.rs | 2 ++ 8 files changed, 116 insertions(+), 6 deletions(-) diff --git a/crates/pons-core/src/cfg/builder.rs b/crates/pons-core/src/cfg/builder.rs index 46e0d10..0fd0239 100644 --- a/crates/pons-core/src/cfg/builder.rs +++ b/crates/pons-core/src/cfg/builder.rs @@ -49,6 +49,12 @@ pub(super) struct Builder<'t> { } impl<'t> Builder<'t> { + /// Builds a CFG using the scope pre-pass for `func`. The node, source text + /// and scope must describe the same function; opacity is checked by the caller. + /// + /// # Panics + /// Panics if `func` has no body or a node's byte range cannot be sliced + /// from `text`. pub(super) fn build( func: Node<'t>, name: String, @@ -127,6 +133,7 @@ impl<'t> Builder<'t> { // ---- events --------------------------------------------------------- + /// Resolves a local unless a comprehension iteration variable shadows it. fn lookup(&self, ident: Node<'_>) -> Option { let name = &self.text[ident.byte_range()]; if self.shadow.iter().any(|s| s.iter().any(|n| n == name)) { @@ -135,6 +142,7 @@ impl<'t> Builder<'t> { self.scope.index.get(name).copied() } + /// Appends an event only for a local visible in the current scope. fn event(&self, ident: Node<'_>, kind: EventKind, ev: &mut Vec) { if let Some(local) = self.lookup(ident) { ev.push(Event { @@ -162,6 +170,8 @@ impl<'t> Builder<'t> { // ---- expressions ---------------------------------------------------- + /// Appends local events for an expression. `checkable` controls whether + /// ordinary reads may be reported as unbound; captures remain uncheckable. fn expr(&mut self, node: Node<'t>, ev: &mut Vec, checkable: bool) { match node.kind() { "identifier" => self.use_(node, checkable, ev), @@ -256,6 +266,8 @@ impl<'t> Builder<'t> { self.shadow.pop(); } + /// Detects a call, `await`, `yield` or walrus outside lambdas for dead-store + /// severity selection; a false result does not prove purity. fn has_effect(node: Node<'_>) -> bool { match node.kind() { "call" | "await" | "yield" | "named_expression" => true, @@ -293,6 +305,8 @@ impl<'t> Builder<'t> { } } + /// Records the RHS reads and target bindings of a possibly chained assignment. + /// A bare annotation emits no events. fn assignment(&mut self, node: Node<'t>, ev: &mut Vec) { // `a = b = rhs` nests: assignment{left: a, right: assignment{left: b, // right: rhs}}. Collect the chain; evaluate `rhs` once; then bind. @@ -318,6 +332,8 @@ impl<'t> Builder<'t> { } } + /// Records RHS reads and, for a bare local target, a read followed by a store. + /// Attribute and subscript targets contribute reads only. fn augmented(&mut self, node: Node<'t>, ev: &mut Vec) { let (Some(left), Some(right)) = ( node.child_by_field_name("left"), @@ -347,6 +363,8 @@ impl<'t> Builder<'t> { } } + /// Lowers a statement at the current block, adding exceptional edges where + /// required. Comments are ignored; code after jumps still receives blocks. fn stmt(&mut self, node: Node<'t>) { if node.kind() == "comment" { return; @@ -442,6 +460,7 @@ impl<'t> Builder<'t> { } } + /// Records unbinding for local deletion targets and reads for object targets. fn del_target(&mut self, target: Node<'t>, ev: &mut Vec) { match target.kind() { "identifier" => self.event(target, EventKind::Kill, ev), @@ -456,9 +475,9 @@ impl<'t> Builder<'t> { } } - /// `def` and `class` inside the unit: defaults, decorators and bases are + /// `def` and `class` inside the unit: defaults and bases are /// read now; the body's mentions of our locals are captures; then the - /// name is bound. + /// name is bound. Decorators are handled by the caller. fn nested_def(&mut self, node: Node<'t>, ev: &mut Vec) { if node.kind() == "function_definition" { if let Some(params) = node.child_by_field_name("parameters") { @@ -519,6 +538,7 @@ impl<'t> Builder<'t> { self.cur = self.new_block(); } + /// Builds conditional branches, retaining fallthrough when there is no `else`. fn if_(&mut self, node: Node<'t>) { let after = self.new_block(); let mut fork = self.cur; @@ -631,6 +651,8 @@ impl<'t> Builder<'t> { self.cur = after; } + /// Lowers a loop body with `continue` targeting `header` and `break` + /// targeting `after`; normal completion returns to `header`. fn loop_body(&mut self, node: Node<'t>, header: BlockId, after: BlockId) { let body_entry = self.new_block(); self.edge(self.cur, body_entry); @@ -656,6 +678,8 @@ impl<'t> Builder<'t> { self.edge(self.cur, after); } + /// Records context expressions and alias bindings before the body. + /// Context-manager exit calls and exception suppression are not modelled. fn with_(&mut self, node: Node<'t>) { let mut ev = Vec::new(); let mut header_end = node.start_byte(); @@ -692,8 +716,8 @@ impl<'t> Builder<'t> { } /// `match`: the subject is read, then each arm is a successor of the - /// header. Every identifier in a pattern is first an uncheckable read - /// (value patterns read names), then each capture is a weak def. Unless + /// header. Pattern identifiers, excluding keyword attribute names, are + /// first uncheckable reads, then each capture is a weak def. Unless /// the last arm is irrefutable (`case _`, or a bare capture, with no /// guard), the header also falls through to `after`. fn match_(&mut self, node: Node<'t>) { @@ -752,6 +776,7 @@ impl<'t> Builder<'t> { self.cur = after; } + /// Recognises an unguarded wildcard or bare capture as an exhaustive arm. fn irrefutable(case: Node<'_>) -> bool { if case.child_by_field_name("guard").is_some() { return false; diff --git a/crates/pons-core/src/cfg/mod.rs b/crates/pons-core/src/cfg/mod.rs index 915eae0..cf1787b 100644 --- a/crates/pons-core/src/cfg/mod.rs +++ b/crates/pons-core/src/cfg/mod.rs @@ -107,19 +107,25 @@ impl FunctionCfg { pub fn name(&self) -> &str { &self.name } + /// The source span of the whole function definition, including its header. pub fn span(&self) -> &Location { &self.span } + /// Local names indexed by [`LocalId`], including parameters but excluding + /// names declared `global` or `nonlocal`. pub fn locals(&self) -> &[String] { &self.locals } + /// Local IDs bound by the function's parameters on entry. pub fn params(&self) -> &[LocalId] { &self.params } + /// Blocks indexed by [`BlockId`], including entry, exit and unreachable code. pub fn blocks(&self) -> &[BasicBlock] { &self.blocks } + /// Predecessors indexed by destination block ID, including unreachable edges. pub fn preds(&self) -> Vec> { let mut preds = vec![Vec::new(); self.blocks.len()]; for (b, block) in self.blocks.iter().enumerate() { @@ -146,6 +152,11 @@ impl FunctionCfg { } /// A stable, human-diffable rendering for the snapshot tests. + /// `text` must be the complete source used to build this CFG. + /// + /// # Panics + /// Panics if a stored statement byte range is out of bounds or does not + /// lie on UTF-8 boundaries in `text`. pub fn render(&self, text: &str) -> String { use std::fmt::Write; let mut out = String::new(); @@ -251,6 +262,16 @@ pub enum FunctionUnit { } /// Every function unit in a Python module, in source order. +/// Includes nested functions and methods; module and class bodies and lambdas +/// are not units. Unsupported functions are returned as opaque units with a +/// reason, including functions with parse errors or more than [`MAX_LOCALS`]. +/// +/// `tree` must be the Python parse of `text`. `path` labels source locations; +/// no file is read. +/// +/// # Panics +/// Panics if a node's byte range cannot be sliced from `text`, for example +/// because the tree and source do not match. pub fn build_units(path: &Path, text: &str, tree: &Tree) -> Vec { let file = path.display().to_string(); let root = tree.root_node(); @@ -289,6 +310,7 @@ pub fn build_units(path: &Path, text: &str, tree: &Tree) -> Vec { .collect() } +/// Appends function definitions in source order, descending into nested scopes. fn collect_functions<'t>(node: Node<'t>, out: &mut Vec>) { if node.kind() == "function_definition" { out.push(node); @@ -312,6 +334,8 @@ fn contains_kind(node: Node<'_>, kind: &str) -> bool { const DYNAMIC_SCOPE: &[&str] = &["exec", "eval", "locals", "globals", "vars"]; +/// Returns the first opacity trigger, before the separate local-count check. +/// Frame-introspection names are matched textually anywhere in the body. fn opaque_reason(func: Node<'_>, text: &str, module_has_wildcard: bool) -> Option { if func.has_error() { return Some(OpaqueReason::ParseError); @@ -375,6 +399,9 @@ fn own_body_trigger(node: Node<'_>, text: &str) -> Option { /// /// The `*` token does survive as an anonymous child, so the discrimination is /// structural rather than textual. +/// +/// # Panics +/// Panics in debug builds if the node is not an `except_clause`. pub(crate) fn is_except_group(except_clause: Node<'_>) -> bool { debug_assert_eq!(except_clause.kind(), "except_clause"); let mut cursor = except_clause.walk(); diff --git a/crates/pons-core/src/cfg/scope.rs b/crates/pons-core/src/cfg/scope.rs index d0c0d32..434d8b7 100644 --- a/crates/pons-core/src/cfg/scope.rs +++ b/crates/pons-core/src/cfg/scope.rs @@ -29,6 +29,7 @@ pub(super) fn is_nested_scope(kind: &str) -> bool { matches!(kind, "function_definition" | "class_definition" | "lambda") } +/// Recognises comprehension scopes, including lazy generator expressions. pub(super) fn is_comprehension(kind: &str) -> bool { matches!( kind, @@ -39,6 +40,13 @@ pub(super) fn is_comprehension(kind: &str) -> bool { ) } +/// Collects parameters, local bindings and conservative nested-scope captures. +/// Global and nonlocal declarations exclude names throughout the function. +/// `text` must be the source from which `func` was parsed. +/// +/// # Panics +/// Panics if `func` has no body or a node's byte range cannot be sliced from +/// `text`. pub(super) fn analyse(func: Node<'_>, text: &str) -> Scope { let body = func .child_by_field_name("body") @@ -96,6 +104,7 @@ fn named_children<'t>(node: Node<'t>) -> Vec> { node.named_children(&mut cursor).collect() } +/// Adds global and nonlocal declarations without entering nested scopes. fn collect_declared(node: Node<'_>, text: &str, out: &mut HashSet) { match node.kind() { "global_statement" | "nonlocal_statement" => { @@ -137,6 +146,7 @@ fn collect_params(params: Node<'_>, text: &str, out: &mut Vec) { } } +/// Finds a parameter identifier, looking through `*args` and `**kwargs` wrappers. fn first_identifier(node: Node<'_>) -> Option> { for child in named_children(node) { if child.kind() == "identifier" { @@ -222,6 +232,7 @@ pub(super) fn is_keyword_pattern_name(ident: Node<'_>) -> bool { .is_some_and(|p| p.kind() == "keyword_pattern" && p.named_child(0) == Some(ident)) } +/// Appends all identifier nodes in source order, including nested scopes. pub(super) fn identifiers_in<'t>(node: Node<'t>, out: &mut Vec>) { if node.kind() == "identifier" { out.push(node); @@ -236,6 +247,9 @@ fn push_all(nodes: &[Node<'_>], text: &str, out: &mut Vec) { out.extend(nodes.iter().map(|n| text_of(*n, text).to_string())); } +/// Appends names that establish local scope, including annotations and `del`. +/// Nested definitions contribute their names; comprehensions contribute only +/// walrus bindings. Repeated names are retained for the caller to deduplicate. fn collect_bound(node: Node<'_>, text: &str, out: &mut Vec) { let mut targets = Vec::new(); match node.kind() { @@ -310,6 +324,7 @@ fn collect_bound(node: Node<'_>, text: &str, out: &mut Vec) { push_all(&targets, text, out); } +/// Appends walrus targets, descending into comprehensions but not nested scopes. fn collect_walrus(node: Node<'_>, text: &str, out: &mut Vec) { match node.kind() { kind if is_nested_scope(kind) => {} @@ -329,6 +344,8 @@ fn collect_walrus(node: Node<'_>, text: &str, out: &mut Vec) { } } +/// Adds every name mentioned inside a nested scope as a possible capture, +/// without resolving whether that scope shadows it. fn collect_nested_mentions(node: Node<'_>, text: &str, out: &mut HashSet) { if is_nested_scope(node.kind()) { let mut idents = Vec::new(); diff --git a/crates/pons-core/src/dataflow.rs b/crates/pons-core/src/dataflow.rs index 1ad97ee..31d41a1 100644 --- a/crates/pons-core/src/dataflow.rs +++ b/crates/pons-core/src/dataflow.rs @@ -32,12 +32,14 @@ pub struct LocalSet { } impl LocalSet { + /// An empty set for `n` locals, with capacity rounded up to 64-bit words. pub fn empty(n: usize) -> Self { Self { words: vec![0; n.div_ceil(64)], } } + /// Contains exactly the local IDs in `0..n`; any padding bits stay clear. pub fn full(n: usize) -> Self { let mut s = Self::empty(n); for id in 0..n { @@ -46,19 +48,33 @@ impl LocalSet { s } + /// Tests membership, including IDs in the allocated padding. + /// + /// # Panics + /// Panics if `id / 64` is outside the allocated words. pub fn contains(&self, id: LocalId) -> bool { self.words[id / 64] & (1 << (id % 64)) != 0 } + /// Adds a local without growing the set's capacity. + /// + /// # Panics + /// Panics if `id / 64` is outside the allocated words. pub fn insert(&mut self, id: LocalId) { self.words[id / 64] |= 1 << (id % 64); } + /// Clears a local's membership; an absent local is unchanged. + /// + /// # Panics + /// Panics if `id / 64` is outside the allocated words. pub fn remove(&mut self, id: LocalId) { self.words[id / 64] &= !(1 << (id % 64)); } /// `self ∪= other`; true if `self` grew. + /// Only overlapping words are merged if capacities differ; `self` is + /// never resized and its remaining words are unchanged. pub fn union_with(&mut self, other: &LocalSet) -> bool { let mut changed = false; for (w, o) in self.words.iter_mut().zip(&other.words) { @@ -81,6 +97,7 @@ pub trait Problem { const DIRECTION: Direction; /// The fact at `ENTRY`'s start (forward) or `EXIT`'s end (backward). fn boundary(&self, cfg: &FunctionCfg) -> LocalSet; + /// Updates the fact across one event in the analysis's direction. fn transfer(&self, event: &Event, state: &mut LocalSet); } @@ -105,6 +122,12 @@ fn events_of(cfg: &FunctionCfg, block: BlockId) -> Vec<&Event> { } } +/// Solves a gen/kill problem using union at joins, returning facts indexed by +/// block ID in program order. All blocks participate, including unreachable +/// ones; callers must filter reachability when reporting findings. +/// +/// The problem must use sets sized for `cfg.locals()` and monotone transfers +/// for convergence. Panics from the problem's callbacks propagate to the caller. pub fn solve(cfg: &FunctionCfg, problem: &P) -> Solution { let n_locals = cfg.locals().len(); let n_blocks = cfg.blocks().len(); @@ -172,6 +195,12 @@ pub fn solve(cfg: &FunctionCfg, problem: &P) -> Solution { /// Walks `block` in the analysis's direction, calling `visit` with each event /// and the fact *before that event's transfer* — so, in program order, the /// fact just before the event (forward) or just after it (backward). +/// `solution` must come from the same CFG and problem; unreachable blocks are +/// replayed too if requested. +/// +/// # Panics +/// Panics if `block` is outside the CFG or the selected solution vector. +/// Panics from `visit` or the transfer function propagate to the caller. pub fn replay<'c, P: Problem>( cfg: &'c FunctionCfg, problem: &P, @@ -234,7 +263,8 @@ impl Problem for MayUnbound { } /// Every reportable store whose value no path reads, one per source location. -/// A store in a copied `finally` counts only if it is dead in every copy. +/// A store in a copied `finally` counts only if it is dead in every reachable +/// copy. Unreachable stores are omitted; results are ordered by byte offset. pub fn dead_stores(cfg: &FunctionCfg) -> Vec<(&Event, DefReport)> { let solution = solve(cfg, &Liveness); let reachable = cfg.reachable(); @@ -263,7 +293,8 @@ pub fn dead_stores(cfg: &FunctionCfg) -> Vec<(&Event, DefReport)> { } /// Every checkable read some path reaches with the name unbound, one per -/// source location. +/// source location, ordered by byte offset. Unreachable reads are omitted; +/// a read in any reachable `finally` copy is enough to include the location. pub fn unbound_reads(cfg: &FunctionCfg) -> Vec<&Event> { let solution = solve(cfg, &MayUnbound); let reachable = cfg.reachable(); diff --git a/crates/pons-core/src/engine.rs b/crates/pons-core/src/engine.rs index 9de4c54..14c6fc4 100644 --- a/crates/pons-core/src/engine.rs +++ b/crates/pons-core/src/engine.rs @@ -28,6 +28,7 @@ impl<'a> RuleCtx<'a> { /// The only way to build a context. Construction goes through here so /// that per-file analyses shared between rules (the M4 CFG) can be added /// as private, lazily-built fields without touching every call site again. + /// `tree` must be the parse of `text` for `lang`; `path` labels findings. pub fn new(path: &'a Path, lang: Lang, text: &'a str, tree: &'a Tree) -> Self { Self { path, @@ -40,6 +41,8 @@ impl<'a> RuleCtx<'a> { /// Every function unit in this file (ADR-0002). Empty for anything but /// Python, so a JS/TS/Rust file never pays for a Python CFG. + /// Builds and caches the units on first access, including opaque units. + /// Panics from [`cfg::build_units`] propagate to the caller. pub fn units(&self) -> &[FunctionUnit] { self.units.get_or_init(|| match self.lang { Lang::Python => cfg::build_units(self.path, self.text, self.tree), diff --git a/crates/pons-rules/src/registry.rs b/crates/pons-rules/src/registry.rs index e5d80a5..71c72cf 100644 --- a/crates/pons-rules/src/registry.rs +++ b/crates/pons-rules/src/registry.rs @@ -20,6 +20,7 @@ use crate::t1::read_before_init::ReadBeforeInit; pub struct RuleRegistry; impl RuleRegistry { + /// Creates the complete rule set in catalogue order, including both T1 rules. pub fn all() -> Vec> { vec![ Box::new(DivByLiteralZero::new()), diff --git a/crates/pons-rules/src/t1/dead_store.rs b/crates/pons-rules/src/t1/dead_store.rs index a121b63..a68bbef 100644 --- a/crates/pons-rules/src/t1/dead_store.rs +++ b/crates/pons-rules/src/t1/dead_store.rs @@ -43,6 +43,10 @@ impl Rule for DeadStore { LANGUAGES } + /// Reports dead stores in analysable functions, suppressing `_`-prefixed + /// locals. A call, `await`, `yield` or walrus outside lambdas in the RHS + /// yields `INFO` with a counter-condition; other stores yield `WARN`. + /// Opaque functions produce no findings. fn check(&self, ctx: &RuleCtx) -> Vec { let mut findings = Vec::new(); for unit in ctx.units() { diff --git a/crates/pons-rules/src/t1/read_before_init.rs b/crates/pons-rules/src/t1/read_before_init.rs index 09de3f8..bebd5d5 100644 --- a/crates/pons-rules/src/t1/read_before_init.rs +++ b/crates/pons-rules/src/t1/read_before_init.rs @@ -43,6 +43,8 @@ impl Rule for ReadBeforeInit { LANGUAGES } + /// Reports possible unbound local reads in analysable functions as `WARN`, + /// without counter-conditions. Opaque functions produce no findings. fn check(&self, ctx: &RuleCtx) -> Vec { let mut findings = Vec::new(); for unit in ctx.units() { From e0f5f8d33a8cfd73d1c11c31978da20bbead4903 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:58:35 +0100 Subject: [PATCH 8/8] test(cfg): probe the import and scope fields cfg/ reads, not only builder.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit (#46) correctly noted my d172272 reply was wrong: scope.rs and mod.rs read further fields — import_statement/import_from_statement.name, aliased_import.alias, (typed_)default_parameter.name, class_definition.name, assignment.left/right, call.function. All now asserted; a mutant renaming aliased_import.alias turned the probe red. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- crates/pons-core/src/cfg/grammar_probe.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/pons-core/src/cfg/grammar_probe.rs b/crates/pons-core/src/cfg/grammar_probe.rs index 75acd8e..1c9dd00 100644 --- a/crates/pons-core/src/cfg/grammar_probe.rs +++ b/crates/pons-core/src/cfg/grammar_probe.rs @@ -126,6 +126,10 @@ async def fetch(url): return result +def typed(n: int = 0): + return n + + def generator(): yield 1 received = yield @@ -526,6 +530,8 @@ fn the_except_as_binding_is_nested_in_an_as_pattern_not_a_field_of_the_clause() ); } +/// Covers every field `cfg/` reads — `builder.rs`, `scope.rs` and `mod.rs` — +/// not only the builder. #[test] fn the_fields_the_builder_will_use_are_all_present() { let tree = parse_python(CORPUS); @@ -547,7 +553,14 @@ fn the_fields_the_builder_will_use_are_all_present() { ("decorated_definition", &["definition"]), ("attribute", &["object"]), ("keyword_argument", &["value"]), - ("default_parameter", &["value"]), + ("default_parameter", &["name", "value"]), + ("typed_default_parameter", &["name", "value"]), + ("class_definition", &["name", "body"]), + ("assignment", &["left", "right"]), + ("call", &["function", "arguments"]), + ("import_statement", &["name"]), + ("import_from_statement", &["module_name", "name"]), + ("aliased_import", &["name", "alias"]), ]; for (kind, fields) in cases { @@ -556,7 +569,7 @@ fn the_fields_the_builder_will_use_are_all_present() { assert_eq!( present.len(), fields.len(), - "{kind} is missing fields the builder uses: expected {fields:?}, found {present:?}" + "{kind} is missing fields cfg/ reads: expected {fields:?}, found {present:?}" ); } } @@ -591,7 +604,7 @@ fn the_fields_only_some_nodes_of_a_kind_carry_are_present() { let node = find_where(root, *pred).unwrap_or_else(|| panic!("corpus has no {what}")); assert!( node.child_by_field_name(field).is_some(), - "{what} does not expose `{field}`, which the builder reads" + "{what} does not expose `{field}`, which cfg/ reads" ); } }