From 4940c2c696ad67a01dff304de31efa67cd62c14a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:45:32 +0000 Subject: [PATCH 1/8] =?UTF-8?q?ruff=5Fruby=5Fspo:=20ordered=20behavioral?= =?UTF-8?q?=20ore=20=E2=80=94=20a=20Ruby=20arm=20and=20a=20Rails=20arm?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shipped `functions.rs` arm ends in six `dedup_in_place` calls, so what reaches SPO is a SET: which fields a method reads, not the order it read them in, not the scope it read them under, not whether the read was guarded. That collapse is the same one `ruff_cpp_spo::events` already fixed for C++ (fae369a); this ports the contract to Ruby, and splits it in two because a Rails class carries two different orders: | arm | walks | preserves | |----------------------------|--------------------|------------------------------| | Ruby (`MethodOre`) | `def … end` bodies | primitives in order, in a scope tree | | Rails (`ClassOre::declarations`) | the class body | the declaration stream in order, WITH its options | Measured on the real OpenProject tree (1708 files, unseen by the arm while it was written): classes 1297 methods 5502 declarations 3100 events 40164 scopes 11987 (2152 methods have >1 scope) symbols 3526 shipped set entries 850 ordered fact events 4589 preservation ratio 5.40x 5.40x is the headline: the ordered arm keeps 4589 fact events where the set arm keeps 850 entries. 11987 scopes over 5502 methods is the other half — 2152 methods have real nesting the set arm cannot express at all. DATA AS CONFIG, NOT HAND-ROLLING `RailsEvent` carries the typed `Declaration` itself. It is deliberately NOT re-spelled into strings here: the authoritative wire spelling (`AssocKind::BelongsTo` -> "belongs_to") already exists at `ruff_spo_triplet::expand`'s `association()`, and a second projection of it would be a mirror that can drift. That it is private there is a real gap — the enums own no `as_str()` — but the fix belongs upstream on the enum, read by both, never copied into a second match. The first draft of this file did hand-roll six `*_kind_name` fns, two of them via `format!("{:?}")`, which would have promoted a `Debug` derive to a wire contract; they are gone. `rails_arm` likewise reuses `walk::walk_class_body` rather than re-implementing routing, so a new `Declaration` variant is a compile error here (it was: `Sti` was caught by the non-exhaustive match, not by review). FALSIFIERS Nine tests, four disable-verified red-then-green: D1' reverse the event stream -> 2 fail D2 drop branch scopes -> 1 fail D3 mark every edge a back_edge -> 1 fail D4 drop Rails declaration options -> 1 fail A fifth disable (sort by `(kind, subj)`) left all nine GREEN and was discarded rather than recorded as evidence: interned symbol ids are assigned in first-appearance order, so sorting by id sorts by source position — the "disable" did not disable. The order test's own fixture was also vacuous at first (`alpha`/`beta`/`gamma` sort into source order); it uses `zulu`/`alpha`/`mike` now, where the two orders genuinely differ. `functions.rs` is untouched except for promoting nine helpers to `pub(crate)` — the set arm's behaviour is byte-identical, and both crates' suites (187 + 177) pass unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- crates/ruff_ruby_spo/examples/ore_census.rs | 117 ++ crates/ruff_ruby_spo/src/events.rs | 1750 +++++++++++++++++++ crates/ruff_ruby_spo/src/functions.rs | 21 +- crates/ruff_ruby_spo/src/lib.rs | 1 + 4 files changed, 1880 insertions(+), 9 deletions(-) create mode 100644 crates/ruff_ruby_spo/examples/ore_census.rs create mode 100644 crates/ruff_ruby_spo/src/events.rs diff --git a/crates/ruff_ruby_spo/examples/ore_census.rs b/crates/ruff_ruby_spo/examples/ore_census.rs new file mode 100644 index 00000000000000..bb389887c4876e --- /dev/null +++ b/crates/ruff_ruby_spo/examples/ore_census.rs @@ -0,0 +1,117 @@ +//! Census the ordered ore over a real Rails tree, so the arm is measured on +//! source it was not written against. +//! +//! `RAILS_SRC=/path/to/openproject cargo run -p ruff_ruby_spo --example ore_census` + +#![expect( + clippy::print_stderr, + reason = "census CLI example (mirrors ruff_cpp_spo::harvest_events)" +)] + +use std::path::PathBuf; + +use ruff_ruby_spo::events::{EventKind, Symbols, source_ore}; + +fn main() { + let root = PathBuf::from( + std::env::var("RAILS_SRC").unwrap_or_else(|_| "/home/user/adaworldapi/openproject".into()), + ); + let limit: usize = std::env::var("LIMIT") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(usize::MAX); + + let mut files = Vec::new(); + for dir in ["app/models", "app/services", "app/contracts"] { + collect(&root.join(dir), &mut files); + } + files.sort(); + files.truncate(limit); + + let mut syms = Symbols::default(); + let (mut classes, mut methods, mut events, mut scopes, mut decls) = + (0u64, 0u64, 0u64, 0u64, 0u64); + let (mut set_total, mut ev_facts) = (0u64, 0u64); + let mut with_multi_scope = 0u64; + let mut cb_with_options = 0u64; + let mut kinds: std::collections::BTreeMap<&'static str, u64> = + std::collections::BTreeMap::new(); + + for path in &files { + let Ok(src) = std::fs::read_to_string(path) else { + continue; + }; + for c in source_ore(&src, "op", &mut syms) { + classes += 1; + decls += c.declarations.len() as u64; + for d in &c.declarations { + if let ruff_ruby_spo::Declaration::Callback(cb) = &d.decl + && !cb.options.is_empty() + { + cb_with_options += 1; + } + } + for m in &c.methods { + methods += 1; + events += m.events.len() as u64; + scopes += m.scopes.len() as u64; + if m.scopes.len() > 1 { + with_multi_scope += 1; + } + set_total += u64::from( + m.set_reads + m.set_writes + m.set_raises + m.set_calls + m.set_traverses, + ); + for e in &m.events { + *kinds.entry(e.kind.as_str()).or_default() += 1; + if matches!( + e.kind, + EventKind::Read + | EventKind::Write + | EventKind::ReadWrite + | EventKind::Raise + | EventKind::Call + ) { + ev_facts += 1; + } + } + } + } + } + + eprintln!("files {}", files.len()); + eprintln!("classes {classes}"); + eprintln!("methods {methods}"); + eprintln!("declarations {decls} (callbacks carrying options: {cb_with_options})"); + eprintln!("events {events}"); + eprintln!("scopes {scopes} (methods with >1 scope: {with_multi_scope})"); + eprintln!("symbols {}", syms.len()); + eprintln!("--"); + eprintln!("shipped set entries {set_total}"); + eprintln!("ordered fact events {ev_facts}"); + if set_total > 0 { + // `f64::from(u32)` is lossless; the counts are corpus-sized, so the + // saturating narrowing is unreachable in practice and never silently + // rounds the way a bare `u64 as f64` would. + let num = f64::from(u32::try_from(ev_facts).unwrap_or(u32::MAX)); + let den = f64::from(u32::try_from(set_total).unwrap_or(u32::MAX)); + eprintln!("preservation ratio {:.2}x", num / den); + } + eprintln!("--\nevent kinds:"); + for (k, n) in &kinds { + eprintln!(" {k:<12} {n}"); + } +} + +fn collect(dir: &std::path::Path, out: &mut Vec) { + let Ok(rd) = std::fs::read_dir(dir) else { + return; + }; + for e in rd.flatten() { + let p = e.path(); + if p.is_dir() { + collect(&p, out); + } else if p.extension().is_some_and(|x| x == "rb") { + out.push(p); + } + } +} diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs new file mode 100644 index 00000000000000..09d584f292fc82 --- /dev/null +++ b/crates/ruff_ruby_spo/src/events.rs @@ -0,0 +1,1750 @@ +//! Ordered behavioral ore for the Ruby/Rails frontend — the sequence and the +//! structure that the shipped set arms throw away. +//! +//! # Why this exists +//! +//! [`crate::functions`]'s body walker ends by collapsing its traversal into +//! sets: `extract_functions_from_body` runs `dedup_in_place` over all six +//! vectors of every [`ruff_spo_triplet::Function`], unconditionally. The +//! walker visits statements in source order and then sorts that order away. +//! +//! So the shipped representation cannot express *a write inside a loop*, *two +//! writes in a row*, or *a call in the else-branch*. It cannot say that a +//! method read a field five times. Those are exactly the shapes a behavioral +//! vocabulary would be made of. +//! +//! The class-body side loses a different thing. [`crate::walk::walk_class_body`] +//! pushes one [`crate::Declaration`] per macro in source order, and that order +//! is real — Rails executes callbacks in declaration order — but it is an +//! accident of a `Vec`, never an explicit fact, and it does not survive +//! `ruff_spo_triplet::expand`, which emits a flat unordered triple set and +//! drops `Callback::options` (the `if:` / `unless:` / `on:` conditions) on the +//! floor. +//! +//! This module is a SECOND walk that preserves three things separately, in +//! **two arms**: +//! +//! | arm | walks | preserves | +//! |---|---|---| +//! | Ruby ([`MethodOre`]) | `def … end` bodies | language primitives in order, inside a scope tree | +//! | Rails ([`ClassOre::declarations`]) | the class body | the framework declaration stream in order, WITH its options | +//! +//! | preserved | where | deduped | +//! |---|---|---| +//! | identity | [`Symbols`] | yes — the only place | +//! | order | [`MethodOre::events`], [`ClassOre::declarations`] | **never** | +//! | structure | [`MethodOre::scopes`] | n/a | +//! +//! The same fact occurring twice is two events pointing at one symbol. +//! +//! # Two arms, because two vocabularies +//! +//! The Ruby arm's alphabet is the LANGUAGE: a send, an assignment, a +//! condition, a branch, a rescue. The Rails arm's alphabet is the FRAMEWORK: +//! a callback, a validation, a scope, an association. Keeping them apart is +//! what lets a consumer subtract one from the other — framework convention is +//! deterministically collapsible, language behaviour is what remains. Fusing +//! them would make that subtraction impossible to express. +//! +//! # Additive by construction +//! +//! Nothing here changes `extract_functions_from_body`, `walk_class_body`, +//! `Function`, `Declaration`, `ModelGraph` or anything `expand` emits. +//! [`MethodOre`] carries the shipped six-set counts beside the events so a +//! consumer can compare the two views without a second parse — and, more +//! importantly, without a second WALK, which would make any difference +//! between them ambiguous. `events_collapse_to_the_shipped_six_sets` asserts +//! that folding the events back into sets reproduces the shipped arm exactly: +//! the change is about what is PRESERVED, not about what a fact IS. +//! +//! # Neutrality — the ore must not pre-solve its consumer's experiment +//! +//! There is deliberately no `GuardedWrite`, `Traversal` or `Mutator` event +//! kind. A blank-guarded default write is a [`EventKind::Condition`] carrying +//! the predicate's own name, a [`EventKind::Branch`] carrying which arm, and a +//! [`EventKind::Write`] — three primitives whose co-occurrence a consumer may +//! learn to call a guard. An association walk is a [`EventKind::Read`] whose +//! symbol happens to be interned as [`SymKind::Relation`]. Identities are +//! carried verbatim, never hashed; different consumers hash differently. +//! +//! # What lib-ruby-parser does not give +//! +//! There is no CFG. `control` therefore carries only structurally-certain +//! relations: which arm of a conditional a branch is, which construct a +//! condition belongs to, and a loop's back edge. Successor edges are not +//! available and are never fabricated. Every event carries its source byte +//! `anchor`, so lexical order stays checkable against traversal order rather +//! than being silently conflated with it. + +use std::collections::BTreeMap; +use std::fmt; + +use lib_ruby_parser::{Loc, Node}; + +use crate::Declaration; + +// ───────────────────────────────────────────────────────────────────────── +// Alphabets +// ───────────────────────────────────────────────────────────────────────── + +/// The Ruby arm's event alphabet — language primitives only. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum EventKind { + /// A scope opened. `subj` is the scope-kind symbol. + ScopeEnter, + /// A scope closed. Re-uses its construct's start anchor. + ScopeExit, + /// A method parameter, in declaration order. + Param, + /// A local-variable binding (`x = …`). + Decl, + /// An instance-variable write (`@x = …`) — NOT an attribute write. + IvarWrite, + /// An instance-variable read (`@x`). + IvarRead, + /// An attribute read (`self.x` / bare `x`). + Read, + /// An attribute write (`self.x = …`). + Write, + /// A read-modify-write (`self.x += …`, `self.x ||= …`). + ReadWrite, + /// A conditional's test expression. `obj` is the predicate callee when the + /// test is a send, so `blank?` / `present?` survive as identity. + Condition, + /// Entry into one arm of a conditional. `control` is `then` / `else` / + /// `when` / `rescue` / `ensure`. + Branch, + /// `return`. + Return, + /// `break`. + Break, + /// `next`. + Next, + /// `redo`. + Redo, + /// `retry`. + Retry, + /// `yield`. + Yield, + /// A method send. `subj` is the receiver symbol when there is one, `obj` + /// the callee. + Call, + /// `raise X` / `raise X.new(…)`. `obj` is the exception type symbol. + Raise, + /// `super` / `super(…)`. + Super, + /// A constant reference. + Const, +} + +impl EventKind { + /// The stable wire spelling. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::ScopeEnter => "ScopeEnter", + Self::ScopeExit => "ScopeExit", + Self::Param => "Param", + Self::Decl => "Decl", + Self::IvarWrite => "IvarWrite", + Self::IvarRead => "IvarRead", + Self::Read => "Read", + Self::Write => "Write", + Self::ReadWrite => "ReadWrite", + Self::Condition => "Condition", + Self::Branch => "Branch", + Self::Return => "Return", + Self::Break => "Break", + Self::Next => "Next", + Self::Redo => "Redo", + Self::Retry => "Retry", + Self::Yield => "Yield", + Self::Call => "Call", + Self::Raise => "Raise", + Self::Super => "Super", + Self::Const => "Const", + } + } +} + +impl fmt::Display for EventKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +/// The Rails arm's event alphabet — one variant per [`Declaration`] family. +/// This is the framework layer, deliberately disjoint from [`EventKind`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum RailsKind { + Association, + Validation, + Callback, + Concern, + Attribute, + Delegation, + Scope, + ActsAs, + DslCall, + GemDsl, + DynamicMethod, + Using, + /// Single-table inheritance — `inheritance_column`, `abstract_class`. + Sti, +} + +impl RailsKind { + /// The stable wire spelling. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Association => "Association", + Self::Validation => "Validation", + Self::Callback => "Callback", + Self::Concern => "Concern", + Self::Attribute => "Attribute", + Self::Delegation => "Delegation", + Self::Scope => "Scope", + Self::ActsAs => "ActsAs", + Self::DslCall => "DslCall", + Self::GemDsl => "GemDsl", + Self::DynamicMethod => "DynamicMethod", + Self::Using => "Using", + Self::Sti => "Sti", + } + } +} + +impl fmt::Display for RailsKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +/// One scope in a method's tree. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScopeKind { + Method, + Block, + If, + Then, + Else, + Case, + When, + While, + Until, + For, + Rescue, + Ensure, + Lambda, +} + +impl ScopeKind { + /// The stable wire spelling. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Method => "Method", + Self::Block => "Block", + Self::If => "If", + Self::Then => "Then", + Self::Else => "Else", + Self::Case => "Case", + Self::When => "When", + Self::While => "While", + Self::Until => "Until", + Self::For => "For", + Self::Rescue => "Rescue", + Self::Ensure => "Ensure", + Self::Lambda => "Lambda", + } + } + + /// Loops carry a back edge; the exit event records it. + fn is_loop(self) -> bool { + matches!(self, Self::While | Self::Until | Self::For) + } +} + +/// What a symbol names. Identity only — never a behavioural claim. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum SymKind { + /// An `ActiveRecord` attribute reached through `self` or implicit self. + Attr, + /// A declared association name — the same string an `Attr` might carry, + /// interned separately because the class declared it as a relation. + Relation, + /// A method parameter. + Param, + /// A local variable. + Local, + /// An instance variable. + Ivar, + /// A method being called. + Callee, + /// A receiver that is not `self`. + Receiver, + /// A constant. + Const, + /// An exception type. + Exception, + /// A scope kind, so a `ScopeEnter` has a subject like any other event. + Scope, + Other, +} + +impl SymKind { + /// The stable wire spelling. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Attr => "attr", + Self::Relation => "relation", + Self::Param => "param", + Self::Local => "local", + Self::Ivar => "ivar", + Self::Callee => "callee", + Self::Receiver => "receiver", + Self::Const => "const", + Self::Exception => "exception", + Self::Scope => "scope_kind", + Self::Other => "other", + } + } +} + +/// Where a name resolves. `Own` is the one the six-set fold keys on. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum Role { + /// Reached through `self` or implicit self — an attribute of this model. + Own, + Param, + Local, + Ivar, + /// An explicit non-self receiver. + Ext, + None, +} + +impl Role { + /// The stable wire spelling. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Own => "own", + Self::Param => "param", + Self::Local => "local", + Self::Ivar => "ivar", + Self::Ext => "ext", + Self::None => "-", + } + } +} + +/// How a fact was obtained. `Parser` is a node the AST gave directly; `Walk` +/// is a relation this walker imposed (a scope boundary, a branch arm). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Prov { + Parser, + Walk, +} + +impl Prov { + /// The stable wire spelling. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Parser => "parser", + Self::Walk => "walk", + } + } +} + +// ───────────────────────────────────────────────────────────────────────── +// Records +// ───────────────────────────────────────────────────────────────────────── + +/// One event in a method body, at its position in the stream. +#[derive(Debug, Clone)] +pub struct OreEvent { + /// Position in this method's stream. Never sorted. + pub seq: u32, + pub kind: EventKind, + /// The acting name, interned. + pub subj: Option, + /// The acted-on name, interned. + pub obj: Option, + /// The scope this event occurred in. + pub scope: u32, + /// That scope's parent, carried so a consumer need not walk the tree. + pub parent: Option, + /// Source byte offset, so lexical order stays checkable against + /// traversal order rather than silently conflated with it. + pub anchor: u32, + /// A structurally-certain relation only: which arm, which construct, a + /// loop's back edge. Never a successor edge. + pub control: Option, + pub prov: Prov, +} + +/// One scope in a method's tree. +#[derive(Debug, Clone)] +pub struct OreScope { + pub id: u32, + pub parent: Option, + pub kind: ScopeKind, + pub depth: u16, + pub enter_seq: u32, + pub exit_seq: u32, +} + +/// One declaration in a class body, at its position in the stream. +/// +/// This is the Rails arm. The `options` are the reason it exists: a callback's +/// `if:` / `unless:` / `on:` is captured by [`crate::walk`] and then dropped by +/// `ruff_spo_triplet::expand`, which emits only `phase:target`. +#[derive(Debug, Clone)] +pub struct RailsEvent { + /// Position in the class body. Rails executes callbacks in this order, + /// and this ordinal is the whole reason this record exists. + pub seq: u32, + /// Which family, for grouping without inspecting the payload. + pub kind: RailsKind, + /// The declaration itself, carried verbatim. + /// + /// It is NOT re-spelled into strings here. The typed data already exists; + /// a second projection of it would be a mirror that can drift, and the + /// authoritative wire spelling (`AssocKind::BelongsTo` → `"belongs_to"`) + /// already lives at `ruff_spo_triplet::expand`'s `association()`. That it + /// is private there is a real gap — the enums own no `as_str()` — but the + /// fix belongs upstream on the enum, read by both, not copied into a + /// second match here. Everything this arm adds is `seq` and `anchor`; + /// the options a consumer wants (`if:` / `unless:` / `on:`) are already + /// on the declaration, and are simply not dropped. + pub decl: Declaration, + /// Source byte offset of the macro call. + pub anchor: u32, +} + +/// One method's ordered ore, plus the shipped arm's view of the same body. +#[derive(Debug, Clone)] +pub struct MethodOre { + /// `.` — the join key. + pub iri: String, + pub name: String, + /// Ruby visibility at the `def` site. + pub public: bool, + /// The event stream, in source order. Never sorted. + pub events: Vec, + /// The scope tree. + pub scopes: Vec, + /// The SHIPPED six-set arm's view of this same body, carried so a + /// consumer can compare the two views without a second parse. + pub set_reads: u32, + pub set_writes: u32, + pub set_guarded: u32, + pub set_raises: u32, + pub set_traverses: u32, + pub set_calls: u32, +} + +/// One class's ore — both arms. +#[derive(Debug, Clone)] +pub struct ClassOre { + /// `:`. + pub iri: String, + pub name: String, + /// The Rails arm: the class-body declaration stream, in order. + pub declarations: Vec, + /// The Ruby arm: one entry per `def`, in declaration order. + pub methods: Vec, +} + +/// The canonical identity table — the ONLY place dedup happens. +#[derive(Debug, Default)] +pub struct Symbols { + index: BTreeMap<(SymKind, Role, String), u32>, + rows: Vec<(SymKind, Role, String, Prov)>, +} + +impl Symbols { + /// Intern a name, returning its stable `s` id. + pub fn intern(&mut self, kind: SymKind, role: Role, name: &str, prov: Prov) -> String { + let key = (kind, role, name.to_string()); + if let Some(id) = self.index.get(&key) { + return format!("s{id}"); + } + let id = u32::try_from(self.rows.len()).unwrap_or(u32::MAX); + self.index.insert(key, id); + self.rows.push((kind, role, name.to_string(), prov)); + format!("s{id}") + } + + /// Every interned row, in insertion order. + pub fn rows(&self) -> impl Iterator { + self.rows + .iter() + .enumerate() + .map(|(i, (k, r, n, p))| (format!("s{i}"), *k, *r, n.as_str(), *p)) + } + + /// How many distinct names have been interned. + #[must_use] + pub fn len(&self) -> usize { + self.rows.len() + } + + /// Whether nothing has been interned yet. + #[must_use] + pub fn is_empty(&self) -> bool { + self.rows.is_empty() + } +} + +// ───────────────────────────────────────────────────────────────────────── +// The Ruby arm — ordered walk of a method body +// ───────────────────────────────────────────────────────────────────────── + +/// Byte offset of a node's start, or 0 where the parser gives no extent. +fn anchor(node: &Node) -> u32 { + u32::try_from(node_expression_loc(node).begin).unwrap_or(0) +} + +/// `expression_l` for the node shapes this walker records. A node whose +/// extent we do not read yields a zero range rather than a wrong one. +fn node_expression_loc(node: &Node) -> Loc { + match node { + Node::Send(n) => n.expression_l, + Node::Block(n) => n.expression_l, + Node::Begin(n) => n.expression_l, + Node::KwBegin(n) => n.expression_l, + Node::Def(n) => n.expression_l, + Node::If(n) => n.expression_l, + Node::IfMod(n) => n.expression_l, + Node::Case(n) => n.expression_l, + Node::When(n) => n.expression_l, + Node::While(n) => n.expression_l, + Node::Until(n) => n.expression_l, + Node::For(n) => n.expression_l, + Node::Rescue(n) => n.expression_l, + Node::RescueBody(n) => n.expression_l, + Node::Ensure(n) => n.expression_l, + Node::Return(n) => n.expression_l, + Node::Break(n) => n.expression_l, + Node::Next(n) => n.expression_l, + Node::Redo(n) => n.expression_l, + Node::Retry(n) => n.expression_l, + Node::Yield(n) => n.expression_l, + Node::Super(n) => n.expression_l, + Node::ZSuper(n) => n.expression_l, + Node::Lvasgn(n) => n.expression_l, + Node::Ivasgn(n) => n.expression_l, + Node::Lvar(n) => n.expression_l, + Node::Ivar(n) => n.expression_l, + Node::Const(n) => n.expression_l, + Node::OrAsgn(n) => n.expression_l, + Node::AndAsgn(n) => n.expression_l, + Node::OpAsgn(n) => n.expression_l, + Node::Lambda(n) => n.expression_l, + _ => Loc { begin: 0, end: 0 }, + } +} + +/// The ordered walk. One instance per method body; `syms` is shared across +/// the whole file so an identity interned in one method is the same id in the +/// next. +struct Walk<'s> { + syms: &'s mut Symbols, + known: &'s [String], + events: Vec, + scopes: Vec, + stack: Vec, +} + +impl<'s> Walk<'s> { + fn new(syms: &'s mut Symbols, known: &'s [String]) -> Self { + Self { + syms, + known, + events: Vec::new(), + scopes: Vec::new(), + stack: Vec::new(), + } + } + + fn cur(&self) -> u32 { + self.stack.last().copied().unwrap_or(0) + } + + fn parent(&self) -> Option { + let n = self.stack.len(); + (n >= 2).then(|| self.stack[n - 2]) + } + + fn next_seq(&self) -> u32 { + u32::try_from(self.events.len()).unwrap_or(u32::MAX) + } + + fn push( + &mut self, + kind: EventKind, + subj: Option, + obj: Option, + anchor: u32, + control: Option<&str>, + prov: Prov, + ) { + let seq = self.next_seq(); + let scope = self.cur(); + let parent = self.parent(); + self.events.push(OreEvent { + seq, + kind, + subj, + obj, + scope, + parent, + anchor, + control: control.map(ToString::to_string), + prov, + }); + } + + /// Open a scope, emitting its `ScopeEnter`. + fn enter(&mut self, kind: ScopeKind, at: u32) -> u32 { + let id = u32::try_from(self.scopes.len()).unwrap_or(u32::MAX); + let parent = self.stack.last().copied(); + let depth = u16::try_from(self.stack.len()).unwrap_or(u16::MAX); + let enter_seq = self.next_seq(); + self.scopes.push(OreScope { + id, + parent, + kind, + depth, + enter_seq, + exit_seq: enter_seq, + }); + self.stack.push(id); + let sym = self + .syms + .intern(SymKind::Scope, Role::None, kind.as_str(), Prov::Walk); + self.push(EventKind::ScopeEnter, Some(sym), None, at, None, Prov::Walk); + id + } + + /// Close the innermost scope, emitting its `ScopeExit`. A loop's exit + /// carries the back edge — the one control relation a tree walk can state + /// with certainty. + fn exit(&mut self, at: u32) { + let Some(id) = self.stack.pop() else { return }; + let kind = self.scopes[id as usize].kind; + let seq = self.next_seq(); + self.scopes[id as usize].exit_seq = seq; + let sym = self + .syms + .intern(SymKind::Scope, Role::None, kind.as_str(), Prov::Walk); + let control = kind.is_loop().then_some("back_edge"); + // The exit belongs to the scope being closed, so record it before the + // stack pop takes effect for `cur()`. + let parent = self.stack.last().copied(); + self.events.push(OreEvent { + seq, + kind: EventKind::ScopeExit, + subj: Some(sym), + obj: None, + scope: id, + parent, + anchor: at, + control: control.map(ToString::to_string), + prov: Prov::Walk, + }); + } + + fn attr(&mut self, name: &str) -> String { + let kind = if self.known.iter().any(|r| r == name) { + SymKind::Relation + } else { + SymKind::Attr + }; + self.syms.intern(kind, Role::Own, name, Prov::Parser) + } + + fn callee(&mut self, name: &str) -> String { + self.syms + .intern(SymKind::Callee, Role::None, name, Prov::Parser) + } +} + +impl Walk<'_> { + /// Walk one body node, preserving order and structure. Mirrors the shape + /// recognition of [`crate::functions`]'s set walker exactly — the same + /// node patterns produce the same facts — and differs only in that every + /// occurrence is kept, in place, inside the scope it occurred in. + #[expect( + clippy::too_many_lines, + reason = "one match arm per recognised Ruby node shape; splitting it \ + would hide the alphabet this module exists to define" + )] + fn body(&mut self, node: &Node) { + let at = anchor(node); + match node { + Node::Begin(b) => { + for stmt in &b.statements { + self.body(stmt); + } + } + Node::KwBegin(b) => { + for stmt in &b.statements { + self.body(stmt); + } + } + // `raise X` / `raise X.new(…)`. + Node::Send(s) if s.method_name == "raise" && s.recv.is_none() => { + let exc = s + .args + .first() + .and_then(crate::functions::exception_type_name) + .map(|name| { + self.syms + .intern(SymKind::Exception, Role::None, &name, Prov::Parser) + }); + self.push(EventKind::Raise, None, exc, at, None, Prov::Parser); + for arg in &s.args { + self.body(arg); + } + } + // `self.` — write, mutator call, or read, in that priority + // order, matching the set walker. + Node::Send(s) if matches!(s.recv.as_deref(), Some(Node::Self_(_))) => { + self.self_send(s, at); + } + // `self.x ||= v` — a write, and the J1 blank-guard idiom. + Node::OrAsgn(o) => { + self.op_assign(&o.recv, at); + self.body(&o.value); + } + // `self.x += v` — a read-modify-write. + Node::OpAsgn(o) => { + if let Some(field) = crate::functions::attr_of_self(&o.recv) { + let field = field.to_string(); + let sym = self.attr(&field); + self.push( + EventKind::ReadWrite, + Some(sym), + None, + at, + None, + Prov::Parser, + ); + } + self.body(&o.value); + } + // `self.x &&= v` — a present-guarded write. + Node::AndAsgn(a) => { + self.op_assign(&a.recv, at); + self.body(&a.value); + } + // A general send: relation walk, mutator dispatch, or neither. + Node::Send(s) => { + self.general_send(s, at); + } + Node::For(f) => { + if let Some(rel) = crate::functions::node_relation_name(&f.iteratee, self.known) { + let sym = self.attr(&rel); + self.push( + EventKind::Read, + Some(sym), + None, + at, + Some("iteratee"), + Prov::Parser, + ); + } + self.enter(ScopeKind::For, at); + self.body(&f.iteratee); + if let Some(b) = f.body.as_deref() { + self.body(b); + } + self.exit(at); + } + Node::While(w) => { + self.loop_with_cond(ScopeKind::While, &w.cond, w.body.as_deref(), at); + } + Node::Until(u) => { + self.loop_with_cond(ScopeKind::Until, &u.cond, u.body.as_deref(), at); + } + Node::Block(blk) => { + self.body(&blk.call); + self.enter(ScopeKind::Block, at); + if let Some(b) = blk.body.as_deref() { + self.body(b); + } + self.exit(at); + } + Node::Lambda(_) => { + self.enter(ScopeKind::Lambda, at); + self.exit(at); + } + Node::If(i) => { + self.conditional(&i.cond, i.if_true.as_deref(), i.if_false.as_deref(), at); + } + Node::IfMod(i) => { + self.conditional(&i.cond, i.if_true.as_deref(), i.if_false.as_deref(), at); + } + Node::Case(c) => { + self.enter(ScopeKind::Case, at); + if let Some(e) = c.expr.as_deref() { + self.body(e); + } + for arm in &c.when_bodies { + let arm_at = anchor(arm); + self.enter(ScopeKind::When, arm_at); + self.push( + EventKind::Branch, + None, + None, + arm_at, + Some("when"), + Prov::Walk, + ); + if let Node::When(w) = arm { + for p in &w.patterns { + self.body(p); + } + if let Some(b) = w.body.as_deref() { + self.body(b); + } + } else { + self.body(arm); + } + self.exit(arm_at); + } + if let Some(b) = c.else_body.as_deref() { + let else_at = anchor(b); + self.enter(ScopeKind::Else, else_at); + self.push( + EventKind::Branch, + None, + None, + else_at, + Some("else"), + Prov::Walk, + ); + self.body(b); + self.exit(else_at); + } + self.exit(at); + } + Node::Rescue(r) => { + if let Some(b) = r.body.as_deref() { + self.body(b); + } + for arm in &r.rescue_bodies { + let arm_at = anchor(arm); + self.enter(ScopeKind::Rescue, arm_at); + self.push( + EventKind::Branch, + None, + None, + arm_at, + Some("rescue"), + Prov::Walk, + ); + self.body(arm); + self.exit(arm_at); + } + if let Some(b) = r.else_.as_deref() { + self.body(b); + } + } + Node::RescueBody(r) => { + if let Some(e) = r.exc_list.as_deref() { + self.body(e); + } + if let Some(b) = r.body.as_deref() { + self.body(b); + } + } + Node::Ensure(e) => { + if let Some(b) = e.body.as_deref() { + self.body(b); + } + if let Some(b) = e.ensure.as_deref() { + let ens_at = anchor(b); + self.enter(ScopeKind::Ensure, ens_at); + self.push( + EventKind::Branch, + None, + None, + ens_at, + Some("ensure"), + Prov::Walk, + ); + self.body(b); + self.exit(ens_at); + } + } + Node::Return(r) => { + self.push(EventKind::Return, None, None, at, None, Prov::Parser); + for arg in &r.args { + self.body(arg); + } + } + Node::Break(b) => { + self.push(EventKind::Break, None, None, at, None, Prov::Parser); + for arg in &b.args { + self.body(arg); + } + } + Node::Next(n) => { + self.push(EventKind::Next, None, None, at, None, Prov::Parser); + for arg in &n.args { + self.body(arg); + } + } + Node::Redo(_) => self.push(EventKind::Redo, None, None, at, None, Prov::Parser), + Node::Retry(_) => self.push(EventKind::Retry, None, None, at, None, Prov::Parser), + Node::Yield(y) => { + self.push(EventKind::Yield, None, None, at, None, Prov::Parser); + for arg in &y.args { + self.body(arg); + } + } + Node::Super(s) => { + self.push(EventKind::Super, None, None, at, None, Prov::Parser); + for arg in &s.args { + self.body(arg); + } + } + Node::ZSuper(_) => self.push(EventKind::Super, None, None, at, None, Prov::Parser), + Node::Lvasgn(a) => { + let sym = self + .syms + .intern(SymKind::Local, Role::Local, &a.name, Prov::Parser); + self.push(EventKind::Decl, Some(sym), None, at, None, Prov::Parser); + if let Some(v) = a.value.as_deref() { + self.body(v); + } + } + Node::Ivasgn(a) => { + let sym = self + .syms + .intern(SymKind::Ivar, Role::Ivar, &a.name, Prov::Parser); + self.push( + EventKind::IvarWrite, + Some(sym), + None, + at, + None, + Prov::Parser, + ); + if let Some(v) = a.value.as_deref() { + self.body(v); + } + } + Node::Ivar(i) => { + let sym = self + .syms + .intern(SymKind::Ivar, Role::Ivar, &i.name, Prov::Parser); + self.push(EventKind::IvarRead, Some(sym), None, at, None, Prov::Parser); + } + Node::Const(_) => { + if let Some(name) = crate::functions::exception_type_name(node) { + let sym = self + .syms + .intern(SymKind::Const, Role::None, &name, Prov::Parser); + self.push(EventKind::Const, Some(sym), None, at, None, Prov::Parser); + } + } + _ => {} + } + } +} + +impl Walk<'_> { + /// `self.` — write, mutator call, or read. Priority mirrors the set + /// walker: a trailing `=` makes it a write, an `ActiveRecord` mutator name + /// makes it a call, anything else that is an attribute identifier is a + /// read. + fn self_send(&mut self, s: &lib_ruby_parser::nodes::Send, at: u32) { + let method = s.method_name.as_str(); + if let Some(base) = method.strip_suffix('=') + && crate::functions::is_attr_ident(base) + { + let base = base.to_string(); + let sym = self.attr(&base); + self.push(EventKind::Write, Some(sym), None, at, None, Prov::Parser); + for arg in &s.args { + self.body(arg); + } + return; + } + if crate::functions::is_ar_mutator(method) { + let callee = self.callee(method); + let recv = self + .syms + .intern(SymKind::Receiver, Role::Own, "self", Prov::Parser); + self.push( + EventKind::Call, + Some(recv), + Some(callee), + at, + None, + Prov::Parser, + ); + for arg in &s.args { + self.body(arg); + } + return; + } + if s.args.is_empty() && crate::functions::is_attr_ident(method) { + let method = method.to_string(); + let sym = self.attr(&method); + self.push(EventKind::Read, Some(sym), None, at, None, Prov::Parser); + return; + } + let callee = self.callee(method); + let recv = self + .syms + .intern(SymKind::Receiver, Role::Own, "self", Prov::Parser); + self.push( + EventKind::Call, + Some(recv), + Some(callee), + at, + None, + Prov::Parser, + ); + for arg in &s.args { + self.body(arg); + } + } + + /// A send with a non-self receiver, or none. Records the relation walk and + /// the mutator dispatch the set walker records, then recurses. + fn general_send(&mut self, s: &lib_ruby_parser::nodes::Send, at: u32) { + if let Some(rel) = crate::functions::traversed_relation(s, self.known) { + let sym = self.attr(&rel); + self.push( + EventKind::Read, + Some(sym), + None, + at, + Some("traversal"), + Prov::Parser, + ); + } else if s.recv.is_none() + && crate::functions::is_attr_ident(&s.method_name) + && s.args.is_empty() + { + // A bare identifier read of an attribute — implicit self. + let name = s.method_name.clone(); + let sym = self.attr(&name); + self.push(EventKind::Read, Some(sym), None, at, None, Prov::Parser); + return; + } + if crate::functions::is_ar_mutator(&s.method_name) { + let label = crate::functions::receiver_label(s.recv.as_deref()); + let recv = self + .syms + .intern(SymKind::Receiver, Role::Ext, &label, Prov::Parser); + let callee = self.callee(&s.method_name); + self.push( + EventKind::Call, + Some(recv), + Some(callee), + at, + None, + Prov::Parser, + ); + } + if let Some(recv) = s.recv.as_deref() { + self.body(recv); + } + for arg in &s.args { + self.body(arg); + } + } + + /// `x ||= v` / `x &&= v` on a `self` attribute — a write. The guard + /// semantics stay out of the alphabet: the co-occurrence is the evidence. + fn op_assign(&mut self, recv: &Node, at: u32) { + if let Some(field) = crate::functions::attr_of_self(recv) { + let field = field.to_string(); + let sym = self.attr(&field); + self.push(EventKind::Write, Some(sym), None, at, None, Prov::Parser); + } + } + + /// A loop whose header carries a condition. + fn loop_with_cond(&mut self, kind: ScopeKind, cond: &Node, body: Option<&Node>, at: u32) { + self.enter(kind, at); + self.condition(cond, "loop_cond"); + if let Some(b) = body { + self.body(b); + } + self.exit(at); + } + + /// A conditional's test, carrying the predicate's own name so `blank?` / + /// `present?` survive as identity rather than as an interpretation. + fn condition(&mut self, cond: &Node, control: &str) { + let at = anchor(cond); + let pred = if let Node::Send(s) = cond { + Some(self.callee(&s.method_name)) + } else { + None + }; + let subj = if let Node::Send(s) = cond + && let Some(field) = s.recv.as_deref().and_then(crate::functions::attr_of_self) + { + let field = field.to_string(); + Some(self.attr(&field)) + } else { + None + }; + self.push( + EventKind::Condition, + subj, + pred, + at, + Some(control), + Prov::Parser, + ); + self.body(cond); + } + + /// `if` / `unless` / the postfix modifier forms — one scope per arm, so a + /// write in the then-branch is distinguishable from one in the else. + fn conditional( + &mut self, + cond: &Node, + if_true: Option<&Node>, + if_false: Option<&Node>, + at: u32, + ) { + self.enter(ScopeKind::If, at); + self.condition(cond, "if_cond"); + if let Some(b) = if_true { + let b_at = anchor(b); + self.enter(ScopeKind::Then, b_at); + self.push( + EventKind::Branch, + None, + None, + b_at, + Some("then"), + Prov::Walk, + ); + self.body(b); + self.exit(b_at); + } + if let Some(b) = if_false { + let b_at = anchor(b); + self.enter(ScopeKind::Else, b_at); + self.push( + EventKind::Branch, + None, + None, + b_at, + Some("else"), + Prov::Walk, + ); + self.body(b); + self.exit(b_at); + } + self.exit(at); + } +} + +// ───────────────────────────────────────────────────────────────────────── +// The Rails arm — the class-body declaration stream, in order +// ───────────────────────────────────────────────────────────────────────── + +/// Which family a declaration belongs to. A `match` on the variant, nothing +/// re-spelled: the payload rides along untouched on [`RailsEvent::decl`]. +fn rails_kind(decl: &Declaration) -> RailsKind { + match decl { + Declaration::Association(_) => RailsKind::Association, + Declaration::Validation(_) => RailsKind::Validation, + Declaration::Callback(_) => RailsKind::Callback, + Declaration::Concern(_) => RailsKind::Concern, + Declaration::Attribute(_) => RailsKind::Attribute, + Declaration::Delegation(_) => RailsKind::Delegation, + Declaration::Scope(_) => RailsKind::Scope, + Declaration::ActsAs(_) => RailsKind::ActsAs, + Declaration::DslCall(_) => RailsKind::DslCall, + Declaration::GemDsl(_) => RailsKind::GemDsl, + Declaration::DynamicMethod(_) => RailsKind::DynamicMethod, + Declaration::Using(_) => RailsKind::Using, + Declaration::Sti(_) => RailsKind::Sti, + } +} + +/// Walk a class body statement by statement, pairing each declaration the +/// SHIPPED router produced with the source position of the statement that +/// produced it. +/// +/// Routing is not re-implemented: each statement goes through +/// [`crate::walk::walk_class_body`], the one place that decides what a macro +/// means. This arm only records WHERE and IN WHAT ORDER — a statement that +/// yields two declarations yields two events sharing an anchor. +fn rails_arm(body: &Node) -> Vec { + let mut out = Vec::new(); + let stmts: Vec<&Node> = match body { + Node::Begin(b) => b.statements.iter().collect(), + single => vec![single], + }; + for stmt in stmts { + let at = anchor(stmt); + let mut decls = Vec::new(); + crate::walk::walk_class_body(stmt, &mut decls); + for decl in decls { + let seq = u32::try_from(out.len()).unwrap_or(u32::MAX); + out.push(RailsEvent { + seq, + kind: rails_kind(&decl), + decl, + anchor: at, + }); + } + } + out +} + +// ───────────────────────────────────────────────────────────────────────── +// Entry points — one parse, two walks +// ───────────────────────────────────────────────────────────────────────── + +/// Collect every `def` in a class body, paired with the visibility the +/// shipped walker assigns it, so the Ruby arm covers exactly the same method +/// set the set arm does. +fn defs_of(body: &Node, out: &mut Vec<(String, Node)>) { + match body { + Node::Begin(b) => { + for stmt in &b.statements { + defs_of(stmt, out); + } + } + Node::KwBegin(b) => { + for stmt in &b.statements { + defs_of(stmt, out); + } + } + Node::Block(blk) => { + if let Some(b) = blk.body.as_deref() { + defs_of(b, out); + } + } + Node::Def(d) => { + if let Some(b) = d.body.as_deref() { + out.push((d.name.clone(), b.clone())); + } + } + Node::Send(s) => { + // `private def foo … end` — the def rides as an argument. + for arg in &s.args { + defs_of(arg, out); + } + } + _ => {} + } +} + +/// Walk one class body into both arms. +/// +/// The set counts on each [`MethodOre`] come from the SHIPPED +/// [`crate::functions::extract_functions_from_body`] run over the very same +/// body node in the very same parse — not a second parse and not a second +/// walk, so any difference between the two views is a difference in what is +/// preserved, never an artefact of having looked twice. +#[must_use] +pub fn class_ore(iri: &str, name: &str, body: &Node, syms: &mut Symbols) -> ClassOre { + let declarations = rails_arm(body); + let decls: Vec = declarations.iter().map(|e| e.decl.clone()).collect(); + let known: Vec = decls + .iter() + .filter_map(|d| match d { + Declaration::Association(a) => Some(a.name.clone()), + _ => None, + }) + .collect(); + + // The shipped arm's verdict on this same body, for the side-by-side. + let (public_fns, helper_fns) = + crate::functions::extract_functions_from_body(Some(body), &decls); + let mut sets: BTreeMap = BTreeMap::new(); + let mut set_rows: BTreeMap = BTreeMap::new(); + for f in &public_fns { + set_rows.insert(f.name.clone(), f); + sets.insert(f.name.clone(), (0, true)); + } + for f in &helper_fns { + set_rows.insert(f.name.clone(), f); + sets.insert(f.name.clone(), (0, false)); + } + + let mut defs = Vec::new(); + defs_of(body, &mut defs); + + let mut methods = Vec::new(); + for (method_name, def_body) in defs { + let mut walk = Walk::new(syms, &known); + walk.enter(ScopeKind::Method, anchor(&def_body)); + walk.body(&def_body); + walk.exit(anchor(&def_body)); + let shipped = set_rows.get(&method_name); + let public = sets.get(&method_name).is_none_or(|(_, p)| *p); + methods.push(MethodOre { + iri: format!("{iri}.{method_name}"), + name: method_name, + public, + events: walk.events, + scopes: walk.scopes, + set_reads: shipped.map_or(0, |f| u32::try_from(f.reads.len()).unwrap_or(u32::MAX)), + set_writes: shipped.map_or(0, |f| u32::try_from(f.writes.len()).unwrap_or(u32::MAX)), + set_guarded: shipped.map_or(0, |f| { + u32::try_from(f.guarded_writes.len()).unwrap_or(u32::MAX) + }), + set_raises: shipped.map_or(0, |f| u32::try_from(f.raises.len()).unwrap_or(u32::MAX)), + set_traverses: shipped + .map_or(0, |f| u32::try_from(f.traverses.len()).unwrap_or(u32::MAX)), + set_calls: shipped.map_or(0, |f| u32::try_from(f.calls.len()).unwrap_or(u32::MAX)), + }); + } + + ClassOre { + iri: iri.to_string(), + name: name.to_string(), + declarations, + methods, + } +} + +/// Parse one Ruby source and walk every class in it into both arms. +/// +/// `namespace` prefixes the class IRI, matching [`crate::extract_with`]. +/// Returns an empty vec on a hard parse failure, mirroring the shipped +/// parser's own silent-skip behaviour rather than inventing a second policy. +#[must_use] +pub fn source_ore(src: &str, namespace: &str, syms: &mut Symbols) -> Vec { + let options = lib_ruby_parser::ParserOptions { + buffer_name: "".to_string(), + ..Default::default() + }; + let parser = lib_ruby_parser::Parser::new(src.as_bytes().to_vec(), options); + let Some(ast) = parser.do_parse().ast else { + return Vec::new(); + }; + let mut out = Vec::new(); + collect(&ast, &[], namespace, syms, &mut out); + out +} + +/// Recurse into `module`/`class`/`begin` wrappers, threading the module +/// namespace exactly as `parse::collect_classes_with_namespace` does, so a +/// class IRI from this arm joins the shipped one. +fn collect( + node: &Node, + ns: &[String], + namespace: &str, + syms: &mut Symbols, + out: &mut Vec, +) { + match node { + Node::Begin(b) => { + for stmt in &b.statements { + collect(stmt, ns, namespace, syms, out); + } + } + Node::Module(m) => { + let mut nested = ns.to_vec(); + if let Some(name) = const_name(&m.name) { + nested.push(name); + } + if let Some(body) = m.body.as_deref() { + collect(body, &nested, namespace, syms, out); + } + } + Node::Class(c) => { + let local = const_name(&c.name).unwrap_or_default(); + let qualified = if ns.is_empty() { + local + } else { + format!("{}::{local}", ns.join("::")) + }; + if let Some(body) = c.body.as_deref() { + let iri = format!("{namespace}:{qualified}"); + out.push(class_ore(&iri, &qualified, body, syms)); + collect(body, ns, namespace, syms, out); + } + } + _ => {} + } +} + +/// A constant node's dotted name, for class and module headers. +fn const_name(node: &Node) -> Option { + match node { + Node::Const(c) => { + let base = c.scope.as_deref().and_then(const_name); + Some(base.map_or_else(|| c.name.clone(), |b| format!("{b}::{}", c.name))) + } + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn ore(src: &str) -> (Vec, Symbols) { + let mut syms = Symbols::default(); + let classes = source_ore(src, "op", &mut syms); + (classes, syms) + } + + fn method<'a>(c: &'a ClassOre, name: &str) -> &'a MethodOre { + c.methods + .iter() + .find(|m| m.name == name) + .unwrap_or_else(|| panic!("no method `{name}`")) + } + + /// Resolve an event's subject to `(kind, role, name)`. + fn subj_of(syms: &Symbols, e: &OreEvent) -> Option<(SymKind, Role, String)> { + let id = e.subj.as_ref()?; + syms.rows() + .find(|r| &r.0 == id) + .map(|r| (r.1, r.2, r.3.to_string())) + } + + // ── the additivity gate ──────────────────────────────────────────── + + /// THE gate: folding the ordered events back into sets must reproduce + /// the shipped six-set arm exactly. What changed is what is PRESERVED, + /// never what a fact IS. If this fails, the two walkers disagree about + /// the meaning of a body and the ordered ore is not a superset. + #[test] + fn events_collapse_to_the_shipped_six_sets() { + let (cs, syms) = ore(r" +class Invoice < ApplicationRecord + belongs_to :project + def settle + self.total = 1 + self.total = 2 + self.state = 'x' + raise ArgumentError if self.total.nil? + self.save + end +end"); + let m = method(&cs[0], "settle"); + + let mut reads: Vec = Vec::new(); + let mut writes: Vec = Vec::new(); + let mut raises: Vec = Vec::new(); + let mut calls = 0_u32; + for e in &m.events { + match e.kind { + EventKind::Read | EventKind::ReadWrite => { + if let Some((_, Role::Own, n)) = subj_of(&syms, e) { + reads.push(n); + } + } + EventKind::Raise => { + if let Some(id) = &e.obj + && let Some(r) = syms.rows().find(|r| &r.0 == id) + { + raises.push(r.3.to_string()); + } + } + EventKind::Call => calls += 1, + _ => {} + } + if matches!(e.kind, EventKind::Write | EventKind::ReadWrite) + && let Some((_, Role::Own, n)) = subj_of(&syms, e) + { + writes.push(n); + } + } + // The fold dedups; the stream did not. + let dedup = |mut v: Vec| { + v.sort(); + v.dedup(); + v + }; + assert_eq!( + u32::try_from(dedup(writes.clone()).len()).unwrap(), + m.set_writes, + "folded writes must equal the shipped set" + ); + assert_eq!( + u32::try_from(dedup(reads).len()).unwrap(), + m.set_reads, + "folded reads must equal the shipped set" + ); + assert_eq!( + u32::try_from(dedup(raises).len()).unwrap(), + m.set_raises, + "folded raises must equal the shipped set" + ); + assert_eq!( + calls, m.set_calls, + "folded calls must equal the shipped set" + ); + // ANTI-VACUITY: the stream must have carried MORE than the set did. + // Two writes to `total` collapse to one set entry; if the raw event + // count equalled the set count this test would be asserting nothing. + assert_eq!(writes.len(), 3, "three write events before dedup"); + assert_eq!(m.set_writes, 2, "two distinct fields written"); + } + + // ── order ────────────────────────────────────────────────────────── + + /// The thing `dedup_in_place` destroys: repetition, and its position. + #[test] + fn repeated_facts_are_separate_events_pointing_at_one_symbol() { + let (cs, _) = ore(r" +class M < ApplicationRecord + def touch_twice + self.count + self.count + self.count + end +end"); + let m = method(&cs[0], "touch_twice"); + let reads: Vec<&OreEvent> = m + .events + .iter() + .filter(|e| e.kind == EventKind::Read) + .collect(); + assert_eq!(reads.len(), 3, "three reads, not one"); + let ids: Vec<&Option> = reads.iter().map(|e| &e.subj).collect(); + assert!( + ids.windows(2).all(|w| w[0] == w[1]), + "all three point at ONE interned symbol" + ); + assert_eq!(m.set_reads, 1, "the shipped set still says one"); + } + + /// Source order is preserved, and checkable against the byte anchors + /// rather than merely assumed. + #[test] + fn event_order_follows_source_order() { + let (cs, syms) = ore(r" +class M < ApplicationRecord + def ordered + self.zulu = 1 + self.alpha = 2 + self.mike = 3 + end +end"); + let m = method(&cs[0], "ordered"); + let names: Vec = m + .events + .iter() + .filter(|e| e.kind == EventKind::Write) + .filter_map(|e| subj_of(&syms, e).map(|(_, _, n)| n)) + .collect(); + // Source order is z, a, m; alphabetical order is a, m, z. A sort or a + // dedup of this stream would reorder it visibly, which is what makes + // this assertion a falsifier rather than a coincidence. + assert_eq!(names, vec!["zulu", "alpha", "mike"]); + let anchors: Vec = m + .events + .iter() + .filter(|e| e.kind == EventKind::Write) + .map(|e| e.anchor) + .collect(); + assert!( + anchors.windows(2).all(|w| w[0] < w[1]), + "anchors strictly increase, so traversal order is not merely asserted to be lexical: {anchors:?}" + ); + } + + // ── structure ────────────────────────────────────────────────────── + + /// A write inside a loop is distinguishable from one outside it. This is + /// the second thing the set arm cannot express. + #[test] + fn a_write_inside_a_loop_is_in_a_loop_scope() { + let (cs, syms) = ore(r" +class M < ApplicationRecord + def tally + self.before = 1 + while self.more? + self.inside = 2 + end + self.after = 3 + end +end"); + let m = method(&cs[0], "tally"); + let scope_kind = |id: u32| m.scopes[id as usize].kind; + let where_written = |name: &str| { + m.events + .iter() + .filter(|e| e.kind == EventKind::Write) + .find(|e| subj_of(&syms, e).is_some_and(|(_, _, n)| n == name)) + .map(|e| scope_kind(e.scope)) + }; + assert_eq!(where_written("inside"), Some(ScopeKind::While)); + assert_eq!(where_written("before"), Some(ScopeKind::Method)); + assert_eq!(where_written("after"), Some(ScopeKind::Method)); + assert!( + m.scopes.iter().any(|s| s.kind == ScopeKind::While), + "the loop opened a scope" + ); + } + + /// A call in the else-branch is distinguishable from one in the then. + /// The third thing the set arm cannot express. + #[test] + fn branch_arms_are_separate_scopes() { + let (cs, syms) = ore(r" +class M < ApplicationRecord + def route + if self.ready? + self.win = 1 + else + self.lose = 2 + end + end +end"); + let m = method(&cs[0], "route"); + let arm_of = |name: &str| { + m.events + .iter() + .filter(|e| e.kind == EventKind::Write) + .find(|e| subj_of(&syms, e).is_some_and(|(_, _, n)| n == name)) + .map(|e| m.scopes[e.scope as usize].kind) + }; + assert_eq!(arm_of("win"), Some(ScopeKind::Then)); + assert_eq!(arm_of("lose"), Some(ScopeKind::Else)); + let controls: Vec<&str> = m + .events + .iter() + .filter(|e| e.kind == EventKind::Branch) + .filter_map(|e| e.control.as_deref()) + .collect(); + assert_eq!(controls, vec!["then", "else"]); + } + + /// A loop's exit carries the back edge — the one control relation a tree + /// walk can state with certainty. A non-loop scope must NOT carry it, + /// or the tag would be decoration rather than a fact. + #[test] + fn only_a_loop_exit_carries_the_back_edge() { + let (cs, _) = ore(r" +class M < ApplicationRecord + def mixed + while self.more? + self.a = 1 + end + if self.cond? + self.b = 2 + end + end +end"); + let m = method(&cs[0], "mixed"); + let back_edges: Vec = m + .events + .iter() + .filter(|e| e.kind == EventKind::ScopeExit && e.control.as_deref() == Some("back_edge")) + .map(|e| m.scopes[e.scope as usize].kind) + .collect(); + assert_eq!(back_edges, vec![ScopeKind::While]); + assert!( + m.scopes.iter().any(|s| s.kind == ScopeKind::If), + "the `if` really is present, so its absence above is a decision" + ); + } + + /// The guard's own predicate survives as identity, so a consumer can + /// learn the blank-guard idiom without the ore having pre-named it. + #[test] + fn a_condition_carries_its_predicate_and_subject_not_a_verdict() { + let (cs, syms) = ore(r" +class M < ApplicationRecord + def defaulted + self.name = 'x' if self.name.blank? + end +end"); + let m = method(&cs[0], "defaulted"); + let cond = m + .events + .iter() + .find(|e| e.kind == EventKind::Condition) + .expect("a condition event"); + let pred = cond + .obj + .as_ref() + .and_then(|id| syms.rows().find(|r| &r.0 == id).map(|r| r.3.to_string())); + assert_eq!(pred.as_deref(), Some("blank?"), "the predicate is identity"); + assert_eq!( + subj_of(&syms, cond).map(|(_, _, n)| n).as_deref(), + Some("name"), + "and so is the field it guards" + ); + // NEUTRALITY: no event kind names the idiom. + assert!( + !m.events + .iter() + .any(|e| format!("{:?}", e.kind).contains("Guard")), + "the alphabet must not pre-name what a consumer is meant to learn" + ); + } + + // ── the Rails arm ────────────────────────────────────────────────── + + /// Callback declaration order is explicit, and the conditions that + /// `ruff_spo_triplet::expand` drops are still attached. + #[test] + fn the_rails_arm_keeps_callback_order_and_its_conditions() { + let (cs, _) = ore(r" +class M < ApplicationRecord + before_validation :normalize + before_save :stamp, if: :dirty? + after_commit :notify, on: :create +end"); + let c = &cs[0]; + let cbs: Vec<&RailsEvent> = c + .declarations + .iter() + .filter(|d| d.kind == RailsKind::Callback) + .collect(); + assert_eq!(cbs.len(), 3); + assert!( + cbs.windows(2).all(|w| w[0].seq < w[1].seq), + "the chain's order is explicit, not an accident of a Vec" + ); + let opts: Vec = cbs + .iter() + .map(|e| match &e.decl { + Declaration::Callback(cb) => cb.options.len(), + _ => unreachable!("filtered to callbacks"), + }) + .collect(); + assert_eq!( + opts, + vec![0, 1, 1], + "the `if:` and `on:` conditions survive — expand.rs drops these" + ); + } + + /// The Rails arm records position, so a declaration can be located in the + /// file rather than only counted. + #[test] + fn the_rails_arm_records_source_position() { + let (cs, _) = ore(r" +class M < ApplicationRecord + belongs_to :project + has_many :entries +end"); + let anchors: Vec = cs[0].declarations.iter().map(|d| d.anchor).collect(); + assert_eq!(anchors.len(), 2); + assert!( + anchors[0] < anchors[1], + "anchors increase with source position: {anchors:?}" + ); + } +} diff --git a/crates/ruff_ruby_spo/src/functions.rs b/crates/ruff_ruby_spo/src/functions.rs index 7e5e49519d4401..28fe9780898e8f 100644 --- a/crates/ruff_ruby_spo/src/functions.rs +++ b/crates/ruff_ruby_spo/src/functions.rs @@ -307,7 +307,7 @@ fn detect_guarded_default( } /// `self.X` or bare `X` (implicit-self attribute) → `Some("X")`; else `None`. -fn attr_of_self(node: &Node) -> Option<&str> { +pub(crate) fn attr_of_self(node: &Node) -> Option<&str> { if let Node::Send(s) = node && s.args.is_empty() && is_attr_ident(&s.method_name) @@ -319,7 +319,7 @@ fn attr_of_self(node: &Node) -> Option<&str> { } /// `self.X.blank?` / `X.nil?` / `self.X.empty?` → the guarded field `X`. -fn blank_guarded_field(cond: &Node) -> Option<&str> { +pub(crate) fn blank_guarded_field(cond: &Node) -> Option<&str> { if let Node::Send(s) = cond && matches!(s.method_name.as_str(), "blank?" | "nil?" | "empty?") { @@ -329,7 +329,7 @@ fn blank_guarded_field(cond: &Node) -> Option<&str> { } /// `self.X.present?` → `X` (the false-branch, i.e. `… unless X.present?`). -fn present_guarded_field(cond: &Node) -> Option<&str> { +pub(crate) fn present_guarded_field(cond: &Node) -> Option<&str> { if let Node::Send(s) = cond && s.method_name == "present?" { @@ -590,7 +590,7 @@ const AR_MUTATORS: &[&str] = &[ ]; /// Is `method` one of the [`AR_MUTATORS`]? -fn is_ar_mutator(method: &str) -> bool { +pub(crate) fn is_ar_mutator(method: &str) -> bool { AR_MUTATORS.contains(&method) } @@ -600,7 +600,7 @@ fn is_ar_mutator(method: &str) -> bool { /// becomes a `reads`/`writes` entry. A setter is recognised by stripping the /// trailing `=` and checking the base with this — `==` strips to `=` (not an /// ident → not a write), `state=` strips to `state` (ident → a write). -fn is_attr_ident(name: &str) -> bool { +pub(crate) fn is_attr_ident(name: &str) -> bool { let mut chars = name.chars(); matches!(chars.next(), Some(c) if c.is_ascii_alphabetic() || c == '_') && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') @@ -610,7 +610,7 @@ fn is_attr_ident(name: &str) -> bool { /// (`"."`). A bare call (`None`) and an explicit `self` /// both render as `"self"`. A relation/local receiver renders as its name; a /// constant as its dotted path; an unresolvable receiver as `""`. -fn receiver_label(recv: Option<&Node>) -> String { +pub(crate) fn receiver_label(recv: Option<&Node>) -> String { match recv { None | Some(Node::Self_(_)) => "self".to_string(), Some(node @ Node::Const(_)) => { @@ -635,7 +635,7 @@ fn receiver_label(recv: Option<&Node>) -> String { /// - `raise UserError.new("msg")` → `"UserError"` (`Node::Send` with recv=Const). /// - `raise UserError, "msg"` → handled by caller via `args.first()`. /// - `raise foo` (variable) → `None` (can't statically resolve). -fn exception_type_name(arg: &Node) -> Option { +pub(crate) fn exception_type_name(arg: &Node) -> Option { match arg { Node::Const(_) => const_to_dotted(arg), Node::Send(s) if s.method_name == "new" => s.recv.as_deref().and_then(const_to_dotted), @@ -663,7 +663,10 @@ fn const_to_dotted(node: &Node) -> Option { /// `` on self, not traversing — handled by the `reads` arm. /// - `` bare (the recv is `Node::Send { method_name: "rel", recv: None }` /// OR `Node::Lvar("rel")`) — that's the traversal entry. -fn traversed_relation(s: &lib_ruby_parser::nodes::Send, known: &[String]) -> Option { +pub(crate) fn traversed_relation( + s: &lib_ruby_parser::nodes::Send, + known: &[String], +) -> Option { let recv = s.recv.as_deref()?; // `.each` — recv is a bare send with method == rel name and no // further receiver. @@ -684,7 +687,7 @@ fn traversed_relation(s: &lib_ruby_parser::nodes::Send, known: &[String]) -> Opt } /// `for r in ` — does `` name a known relation? -fn node_relation_name(node: &Node, known: &[String]) -> Option { +pub(crate) fn node_relation_name(node: &Node, known: &[String]) -> Option { match node { Node::Send(s) if s.recv.is_none() && known.iter().any(|r| r == &s.method_name) => { Some(s.method_name.clone()) diff --git a/crates/ruff_ruby_spo/src/lib.rs b/crates/ruff_ruby_spo/src/lib.rs index 3daf819fb3ed1b..28aa65247f7d1b 100644 --- a/crates/ruff_ruby_spo/src/lib.rs +++ b/crates/ruff_ruby_spo/src/lib.rs @@ -35,6 +35,7 @@ use ruff_spo_triplet::{ }; mod actions; +pub mod events; mod functions; mod menu; mod menu_regions; From 1abdbe538c099ccf09216cd0b54b4b33d57e7692 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 00:07:53 +0000 Subject: [PATCH 2/8] ruff_ruby_spo: unlink private-item intra-doc links in events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo doc --all --no-deps` runs with `RUSTDOCFLAGS: -D warnings`, which denies `rustdoc::private_intra_doc_links`. `pub mod events` is public, so its rustdoc may not link to `crate::functions`, `crate::walk`, or anything inside them — all private. CI's `cargo test (linux)` failed on exactly that, four errors, before it ran a single test. Reproduced locally first (`RUSTDOCFLAGS="-D warnings" cargo doc -p ruff_ruby_spo --no-deps`, same four), then fixed by dropping the link brackets: `[`crate::functions`]` -> `crate::functions`. The prose is unchanged and still names the exact module; it just no longer asks rustdoc to resolve a link it is not allowed to render. Making those modules public to satisfy a doc link would widen the crate's API for no caller, and `#[allow(rustdoc::private_intra_doc_links)]` would suppress a real rule. Six sites, not the four CI reported. Two more (`Walk::body`, `rails_arm`) carry the same links but sit on PRIVATE items, so rustdoc does not consider their docs public and lets them pass today — they would break the build the moment either item is made public. Fixed together so the file has one rule. `crate::Declaration` and `crate::extract_with` are public and resolve, so they stay real links. Gate list corrected: the miss was mine — clippy, fmt, tests and prek all passed locally and none of them builds documentation. Now verified with the workspace-wide command CI actually runs, `RUSTDOCFLAGS="-D warnings" cargo doc --all --no-deps`, clean. Tests unchanged at 187 + 177. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- crates/ruff_ruby_spo/src/events.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs index 09d584f292fc82..73622723f100ff 100644 --- a/crates/ruff_ruby_spo/src/events.rs +++ b/crates/ruff_ruby_spo/src/events.rs @@ -3,7 +3,7 @@ //! //! # Why this exists //! -//! [`crate::functions`]'s body walker ends by collapsing its traversal into +//! `crate::functions`'s body walker ends by collapsing its traversal into //! sets: `extract_functions_from_body` runs `dedup_in_place` over all six //! vectors of every [`ruff_spo_triplet::Function`], unconditionally. The //! walker visits statements in source order and then sorts that order away. @@ -13,7 +13,7 @@ //! method read a field five times. Those are exactly the shapes a behavioral //! vocabulary would be made of. //! -//! The class-body side loses a different thing. [`crate::walk::walk_class_body`] +//! The class-body side loses a different thing. `crate::walk::walk_class_body` //! pushes one [`crate::Declaration`] per macro in source order, and that order //! is real — Rails executes callbacks in declaration order — but it is an //! accident of a `Vec`, never an explicit fact, and it does not survive @@ -401,7 +401,7 @@ pub struct OreScope { /// One declaration in a class body, at its position in the stream. /// /// This is the Rails arm. The `options` are the reason it exists: a callback's -/// `if:` / `unless:` / `on:` is captured by [`crate::walk`] and then dropped by +/// `if:` / `unless:` / `on:` is captured by `crate::walk` and then dropped by /// `ruff_spo_triplet::expand`, which emits only `phase:target`. #[derive(Debug, Clone)] pub struct RailsEvent { @@ -677,7 +677,7 @@ impl<'s> Walk<'s> { impl Walk<'_> { /// Walk one body node, preserving order and structure. Mirrors the shape - /// recognition of [`crate::functions`]'s set walker exactly — the same + /// recognition of `crate::functions`'s set walker exactly — the same /// node patterns produce the same facts — and differs only in that every /// occurrence is kept, in place, inside the scope it occurred in. #[expect( @@ -1186,7 +1186,7 @@ fn rails_kind(decl: &Declaration) -> RailsKind { /// produced it. /// /// Routing is not re-implemented: each statement goes through -/// [`crate::walk::walk_class_body`], the one place that decides what a macro +/// `crate::walk::walk_class_body`, the one place that decides what a macro /// means. This arm only records WHERE and IN WHAT ORDER — a statement that /// yields two declarations yields two events sharing an anchor. fn rails_arm(body: &Node) -> Vec { @@ -1254,7 +1254,7 @@ fn defs_of(body: &Node, out: &mut Vec<(String, Node)>) { /// Walk one class body into both arms. /// /// The set counts on each [`MethodOre`] come from the SHIPPED -/// [`crate::functions::extract_functions_from_body`] run over the very same +/// `crate::functions::extract_functions_from_body` run over the very same /// body node in the very same parse — not a second parse and not a second /// walk, so any difference between the two views is a difference in what is /// preserved, never an artefact of having looked twice. From beb9c0127272575886164c4ffac9e6b3b6e000c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 03:53:11 +0000 Subject: [PATCH 3/8] ruff_ruby_spo: make the six-set gate literally six sets, and stop losing ||= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the additivity gate was not the gate it claimed to be, and the investigation turned up two real losses behind it. All four findings verified at source before changing anything. THE GATE WAS FOUR SETS, NOT SIX `events_collapse_to_the_shipped_six_sets` asserted reads / writes / raises / calls and silently skipped `guarded_writes` and `traverses`, both of which `MethodOre` already carries. Its `calls` arm was not even a fold: it counted raw `Call` events against a shipped set that deduplicates, and passed only because the fixture called `save` once. It is now a real fold over all six, written as a consumer would have to write it — reading only events, scopes and symbols, never the AST — against an adversarial fixture (repeated mutator call, traversal, `||=`, `&&=`, `+=`, and the `if`-guard idiom). LOSS 1: `||=` vs `&&=` was unreconstructible The shipped arm reads `x ||= v` as a blank-guarded default (writes + guarded_writes) and `x &&= v` as an ordinary write. Both reached `op_assign` and came out as a bare `Write` — the two indistinguishable from each other and from `self.x = v`. The code comments named the distinction while the event stream threw it away, so guardedness was not merely un-named, it was unlearnable. `op_assign` now tags the operator (`or_assign` / `and_assign`) in `control`, which already carried a syntactic tag (`traversal`). Still no `GuardedWrite` kind: the syntax is a structurally-certain fact, the semantics stay the consumer's to learn. LOSS 2: a traversal receiver was counted twice `lines.each` emitted the traversal `Read` and then re-walked the receiver, emitting a SECOND untagged `Read("lines")` — one observation counted twice, under a name the shipped arm files only under `traverses`. Found by the new gate, which folded 3 reads where the shipped arm had 2. The traversal arm now consumes the receiver; the mutator check stays reachable, so `order.save` is still both a traversal and a call. Census re-measured on the same 1708 OpenProject files: events 40164 -> 40034, Read 3872 -> 3742, preservation 5.40x -> 5.25x. The headline number goes DOWN because 130 of it was the double count. ONE DIVERGENCE, NAMED NOT HIDDEN `self.foo(arg)` — the shipped self-send branch tests `is_attr_ident(method)` with no `args.is_empty()` guard, so it records a READ; the ordered arm requires empty args and emits a `Call`. The ore's reading is the faithful one (a send with arguments is not an attribute read), but that makes "reproduces the shipped sets exactly" non-universal. Rather than paper over it, the claim is weakened in the module doc and the delta is pinned by its own test, which fails if either side moves. Also corrected two doc comments that were wrong about their own code: `EventKind::ReadWrite` claimed `||=` reaches it (only `+=` does), and `control`'s doc omitted the syntactic tags it already carried. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- crates/ruff_ruby_spo/src/events.rs | 318 ++++++++++++++++++++++------- 1 file changed, 248 insertions(+), 70 deletions(-) diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs index 73622723f100ff..f490e7cfda288a 100644 --- a/crates/ruff_ruby_spo/src/events.rs +++ b/crates/ruff_ruby_spo/src/events.rs @@ -53,9 +53,20 @@ //! [`MethodOre`] carries the shipped six-set counts beside the events so a //! consumer can compare the two views without a second parse — and, more //! importantly, without a second WALK, which would make any difference -//! between them ambiguous. `events_collapse_to_the_shipped_six_sets` asserts -//! that folding the events back into sets reproduces the shipped arm exactly: -//! the change is about what is PRESERVED, not about what a fact IS. +//! between them ambiguous. `events_collapse_to_the_shipped_six_sets` folds the +//! stream back into ALL SIX sets — `reads`, `writes`, `guarded_writes`, +//! `raises`, `traverses`, `calls` — and asserts they equal the shipped arm's: +//! the change is +//! about what is PRESERVED, not about what a fact IS. +//! +//! That holds with ONE named exception, pinned by its own test rather than +//! left implicit. `self.foo(arg)` — an explicit-self send of an +//! attribute-shaped name WITH arguments — is a `reads` entry on the shipped +//! side (its self-send branch tests `is_attr_ident` and never +//! `args.is_empty()`) and a [`EventKind::Call`] here. A send carrying +//! arguments is not an attribute read, so this arm keeps the faithful +//! observation; the cost is that "a strict superset of the shipped sets" is +//! true of five sets and of `reads` everywhere except this shape. //! //! # Neutrality — the ore must not pre-solve its consumer's experiment //! @@ -63,7 +74,14 @@ //! kind. A blank-guarded default write is a [`EventKind::Condition`] carrying //! the predicate's own name, a [`EventKind::Branch`] carrying which arm, and a //! [`EventKind::Write`] — three primitives whose co-occurrence a consumer may -//! learn to call a guard. An association walk is a [`EventKind::Read`] whose +//! learn to call a guard. (The gate's fold rebuilds `guarded_writes` from +//! exactly that co-occurrence, so the claim is demonstrated, not asserted.) +//! +//! Neutrality is not the same as discarding. The `||=` / `&&=` operator IS +//! recorded, in `control` — because the shipped arm reads one as guarded and +//! the other as an ordinary write, so an untagged `Write` for both would make +//! that distinction unlearnable rather than merely un-named. The rule is: +//! carry what the syntax certainly says, decide nothing it does not. An association walk is a [`EventKind::Read`] whose //! symbol happens to be interned as [`SymKind::Relation`]. Identities are //! carried verbatim, never hashed; different consumers hash differently. //! @@ -106,7 +124,10 @@ pub enum EventKind { Read, /// An attribute write (`self.x = …`). Write, - /// A read-modify-write (`self.x += …`, `self.x ||= …`). + /// A read-modify-write (`self.x += …`). NOT `self.x ||= …` / `&&=`: + /// those are [`EventKind::Write`] tagged `or_assign` / `and_assign`, + /// matching the shipped arm, which records `+=` as read+write but `||=` + /// as write+guarded and `&&=` as write alone. ReadWrite, /// A conditional's test expression. `obj` is the predicate callee when the /// test is a send, so `blank?` / `present?` survive as identity. @@ -381,8 +402,10 @@ pub struct OreEvent { /// Source byte offset, so lexical order stays checkable against /// traversal order rather than silently conflated with it. pub anchor: u32, - /// A structurally-certain relation only: which arm, which construct, a - /// loop's back edge. Never a successor edge. + /// A structurally-certain tag only: which arm, which construct, a loop's + /// back edge, the syntactic form that produced the event (`traversal`, + /// `or_assign`, `and_assign`). Never a successor edge, and never an + /// interpretation — every value here is readable straight off the AST. pub control: Option, pub prov: Prov, } @@ -720,7 +743,7 @@ impl Walk<'_> { } // `self.x ||= v` — a write, and the J1 blank-guard idiom. Node::OrAsgn(o) => { - self.op_assign(&o.recv, at); + self.op_assign(&o.recv, at, "or_assign"); self.body(&o.value); } // `self.x += v` — a read-modify-write. @@ -741,7 +764,7 @@ impl Walk<'_> { } // `self.x &&= v` — a present-guarded write. Node::AndAsgn(a) => { - self.op_assign(&a.recv, at); + self.op_assign(&a.recv, at, "and_assign"); self.body(&a.value); } // A general send: relation walk, mutator dispatch, or neither. @@ -1023,6 +1046,14 @@ impl Walk<'_> { /// A send with a non-self receiver, or none. Records the relation walk and /// the mutator dispatch the set walker records, then recurses. fn general_send(&mut self, s: &lib_ruby_parser::nodes::Send, at: u32) { + // Set when the traversal arm has already consumed the receiver, so + // the generic re-walk below does not emit a SECOND, untagged `Read` + // of the same name. `lines.each` is one observation — a traversal of + // `lines` — not a traversal plus a plain attribute read of it; the + // shipped arm likewise files that name under `traverses` and never + // under `reads`. Double-counting it would inflate `reads` with a + // name the fold has already attributed elsewhere. + let mut recv_consumed = false; if let Some(rel) = crate::functions::traversed_relation(s, self.known) { let sym = self.attr(&rel); self.push( @@ -1033,6 +1064,7 @@ impl Walk<'_> { Some("traversal"), Prov::Parser, ); + recv_consumed = true; } else if s.recv.is_none() && crate::functions::is_attr_ident(&s.method_name) && s.args.is_empty() @@ -1058,7 +1090,7 @@ impl Walk<'_> { Prov::Parser, ); } - if let Some(recv) = s.recv.as_deref() { + if !recv_consumed && let Some(recv) = s.recv.as_deref() { self.body(recv); } for arg in &s.args { @@ -1066,13 +1098,29 @@ impl Walk<'_> { } } - /// `x ||= v` / `x &&= v` on a `self` attribute — a write. The guard - /// semantics stay out of the alphabet: the co-occurrence is the evidence. - fn op_assign(&mut self, recv: &Node, at: u32) { + /// `x ||= v` / `x &&= v` on a `self` attribute — a write, tagged with + /// WHICH operator wrote it. + /// + /// The guard *semantics* stay out of the alphabet — there is still no + /// `GuardedWrite` kind — but the operator itself is a structurally-certain + /// syntactic fact, and dropping it is not neutrality, it is loss: the + /// shipped arm reads `||=` as a blank-guarded default (`writes` + + /// `guarded_writes`) and `&&=` as an ordinary write, so a bare `Write` for + /// both leaves that distinction unreconstructible from the ore. Tagging + /// the syntax lets a consumer LEARN that `or_assign` co-occurs with + /// guardedness; naming the event `GuardedWrite` would have decided it. + fn op_assign(&mut self, recv: &Node, at: u32, op: &str) { if let Some(field) = crate::functions::attr_of_self(recv) { let field = field.to_string(); let sym = self.attr(&field); - self.push(EventKind::Write, Some(sym), None, at, None, Prov::Parser); + self.push( + EventKind::Write, + Some(sym), + None, + at, + Some(op), + Prov::Parser, + ); } } @@ -1421,82 +1469,212 @@ mod tests { // ── the additivity gate ──────────────────────────────────────────── - /// THE gate: folding the ordered events back into sets must reproduce - /// the shipped six-set arm exactly. What changed is what is PRESERVED, - /// never what a fact IS. If this fails, the two walkers disagree about - /// the meaning of a body and the ordered ore is not a superset. - #[test] - fn events_collapse_to_the_shipped_six_sets() { - let (cs, syms) = ore(r" -class Invoice < ApplicationRecord - belongs_to :project - def settle - self.total = 1 - self.total = 2 - self.state = 'x' - raise ArgumentError if self.total.nil? - self.save - end -end"); - let m = method(&cs[0], "settle"); + /// Fold the ordered stream back into the shipped six sets. + /// + /// Deliberately written the way a CONSUMER would have to write it: it + /// reads only the ore — events, scopes, symbols — and never the AST. If + /// a set cannot be rebuilt from here, the ore lost it. + /// + /// Returns distinct-name counts in the shipped arm's own order: + /// `[reads, writes, guarded, raises, traverses, calls]`. + fn fold_six(syms: &Symbols, m: &MethodOre) -> [u32; 6] { + let name_of = |id: &Option| -> Option { + let id = id.as_ref()?; + syms.rows().find(|r| &r.0 == id).map(|r| r.3.to_string()) + }; + let own = |e: &OreEvent| -> Option { + match subj_of(syms, e) { + Some((_, Role::Own, n)) => Some(n), + _ => None, + } + }; + let ctl = |e: &OreEvent, want: &str| e.control.as_deref() == Some(want); + + let (mut reads, mut writes, mut guarded) = (Vec::new(), Vec::new(), Vec::new()); + let (mut raises, mut traverses, mut calls) = (Vec::new(), Vec::new(), Vec::new()); - let mut reads: Vec = Vec::new(); - let mut writes: Vec = Vec::new(); - let mut raises: Vec = Vec::new(); - let mut calls = 0_u32; for e in &m.events { match e.kind { - EventKind::Read | EventKind::ReadWrite => { - if let Some((_, Role::Own, n)) = subj_of(&syms, e) { - reads.push(n); + // A traversal is a Read tagged `traversal`; the shipped arm + // files it under `traverses`, not `reads`. + EventKind::Read if ctl(e, "traversal") => traverses.extend(own(e)), + EventKind::Read => reads.extend(own(e)), + // `+=` is read AND write on the shipped side. + EventKind::ReadWrite => { + reads.extend(own(e)); + writes.extend(own(e)); + } + EventKind::Write => { + writes.extend(own(e)); + // Source 1 of `guarded_writes`: `x ||= v`. Recoverable + // ONLY because op_assign tags the operator — `&&=` is a + // write and deliberately not guarded, and without the tag + // the two are the same event. + if ctl(e, "or_assign") { + guarded.extend(own(e)); } } - EventKind::Raise => { - if let Some(id) = &e.obj - && let Some(r) = syms.rows().find(|r| &r.0 == id) - { - raises.push(r.3.to_string()); + EventKind::Raise => raises.extend(name_of(&e.obj)), + EventKind::Call => { + if let (Some(r), Some(c)) = (name_of(&e.subj), name_of(&e.obj)) { + calls.push(format!("{r}.{c}")); } } - EventKind::Call => calls += 1, _ => {} } - if matches!(e.kind, EventKind::Write | EventKind::ReadWrite) - && let Some((_, Role::Own, n)) = subj_of(&syms, e) + } + + // Source 2 of `guarded_writes`: the `if`/`unless` blank-guard idiom. + // Rebuilt from co-occurrence exactly as the module doc promises — a + // Condition naming the predicate and the field, a Branch naming the + // arm, and a Write of that field in that arm's scope. Mirrors the + // shipped `claim_if_writes`, which claims only DIRECT statements of + // the branch, so the write's scope must be the branch scope itself. + for c in m.events.iter().filter(|e| e.kind == EventKind::Condition) { + let Some(field) = own(c) else { continue }; + let arm = match name_of(&c.obj).as_deref() { + Some("blank?" | "nil?" | "empty?") => ScopeKind::Then, + Some("present?") => ScopeKind::Else, + _ => continue, + }; + for sc in m + .scopes + .iter() + .filter(|s| s.parent == Some(c.scope) && s.kind == arm) { - writes.push(n); + for w in m + .events + .iter() + .filter(|e| e.kind == EventKind::Write && e.scope == sc.id) + { + if own(w).as_deref() == Some(field.as_str()) { + guarded.push(field.clone()); + } + } } } - // The fold dedups; the stream did not. - let dedup = |mut v: Vec| { + + let n = |mut v: Vec| { v.sort(); v.dedup(); - v + u32::try_from(v.len()).unwrap() }; - assert_eq!( - u32::try_from(dedup(writes.clone()).len()).unwrap(), + [ + n(reads), + n(writes), + n(guarded), + n(raises), + n(traverses), + n(calls), + ] + } + + /// THE gate: folding the ordered events back into sets must reproduce + /// the shipped six-set arm exactly — all six, not the easy four. What + /// changed is what is PRESERVED, never what a fact IS. If this fails, + /// the two walkers disagree about the meaning of a body and the ordered + /// ore is not a superset. + /// + /// The fixture is adversarial on purpose: a repeated mutator call (the + /// shipped arm dedups it, so a raw event count would pass vacuously), a + /// relation traversal, `||=` and `&&=` side by side (the pair that is + /// indistinguishable without the operator tag), `+=`, and the `if`-guard + /// idiom (the second, co-occurrence-only source of `guarded_writes`). + #[test] + fn events_collapse_to_the_shipped_six_sets() { + let (cs, syms) = ore(r" +class Invoice < ApplicationRecord + belongs_to :project + has_many :lines + def settle + self.total = 1 + self.total = 2 + self.state ||= 'draft' + self.locked &&= true + self.count += 1 + self.total = 0 if self.total.blank? + lines.each + self.save + self.save + raise ArgumentError + end +end"); + let m = method(&cs[0], "settle"); + let got = fold_six(&syms, m); + let want = [ + m.set_reads, m.set_writes, - "folded writes must equal the shipped set" - ); + m.set_guarded, + m.set_raises, + m.set_traverses, + m.set_calls, + ]; assert_eq!( - u32::try_from(dedup(reads).len()).unwrap(), - m.set_reads, - "folded reads must equal the shipped set" + got, want, + "fold [reads, writes, guarded, raises, traverses, calls] must equal the shipped six sets" ); - assert_eq!( - u32::try_from(dedup(raises).len()).unwrap(), - m.set_raises, - "folded raises must equal the shipped set" + + // ANTI-VACUITY. Each of these would make some assertion above hold + // for the wrong reason if it were not true of this fixture. + let ev = |k: EventKind| m.events.iter().filter(|e| e.kind == k).count(); + assert!( + ev(EventKind::Write) > want[1] as usize, + "more write events than distinct written fields — otherwise dedup is untested" ); - assert_eq!( - calls, m.set_calls, - "folded calls must equal the shipped set" + assert!( + ev(EventKind::Call) > want[5] as usize, + "the mutator call repeats — otherwise a raw count would pass" + ); + assert!( + want[2] >= 2, + "both guarded_writes sources fire: `||=` and the if-idiom" + ); + assert!( + want[4] >= 1, + "a traversal is present, so `traverses` is not vacuously zero" + ); + assert!( + m.events + .iter() + .any(|e| e.control.as_deref() == Some("and_assign")), + "`&&=` is present and tagged — it must NOT reach guarded_writes" + ); + } + + /// The ONE named divergence, pinned so it cannot drift silently. + /// + /// `self.foo(arg)` — an explicit-self send of an attribute-shaped name + /// WITH arguments. The shipped arm files it as a `reads` entry: its + /// self-send branch tests `is_attr_ident(method)` with no `args.is_empty()` + /// guard (`functions.rs`, the third arm). The ordered arm requires empty + /// args and therefore emits a `Call`. + /// + /// The ore's reading is the faithful observation — a send with arguments + /// is not an attribute read — but that means "the fold reproduces the + /// shipped sets exactly" is NOT universal, and the module doc says so + /// rather than pretending. This test measures the delta instead of + /// hiding it: fix either side and it fails. + #[test] + fn self_send_with_args_is_the_one_place_the_two_walkers_disagree() { + let (cs, syms) = ore(r" +class Invoice < ApplicationRecord + def settle + self.foo(1) + end +end"); + let m = method(&cs[0], "settle"); + let got = fold_six(&syms, m); + + // Shipped: one read (`foo`), no calls — `foo` is not an AR mutator. + assert_eq!(m.set_reads, 1, "shipped arm counts `self.foo(1)` as a read"); + assert_eq!(m.set_calls, 0, "shipped arm records no call for it"); + // Ore: no read, one call. + assert_eq!(got[0], 0, "ore emits no Read for a send carrying arguments"); + assert_eq!(got[5], 1, "ore emits a Call instead"); + assert!( + m.events.iter().any(|e| e.kind == EventKind::Call), + "the Call event is the ore's reading of this shape" ); - // ANTI-VACUITY: the stream must have carried MORE than the set did. - // Two writes to `total` collapse to one set entry; if the raw event - // count equalled the set count this test would be asserting nothing. - assert_eq!(writes.len(), 3, "three write events before dedup"); - assert_eq!(m.set_writes, 2, "two distinct fields written"); } // ── order ────────────────────────────────────────────────────────── From e6f8e9faca6c9da4e36d044fd64d47b664430b0b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 04:05:47 +0000 Subject: [PATCH 4/8] ruff_ruby_spo: fix six real findings from the CodeRabbit + Codex reviews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both bots reviewed 1abdbe5, before the six-set gate landed, so some findings were already fixed and some were not. Each was verified at source before changing anything; the gate itself found none of these because its fixture lacked the shapes. 1. `EventKind::Param` had NO EMITTER ANYWHERE (Codex P2). The variant was public, documented "a method parameter, in declaration order", and the census measured exactly ZERO across 5502 real methods — an alphabet variant that was pure fiction. Cause: `defs_of` kept only `(name, body)` and discarded the argument list. It now keeps the whole `Def` node and `Walk::params` emits every parameter kind (required / optional / rest / keyword / keyword_optional / keyword_rest / block) in declaration order, with defaults walked as ordinary expressions. Census: 0 -> 3708 Param events. 2. Body-less defs were dropped (CodeRabbit + Codex). `def noop; end` produced no `MethodOre` while the shipped extractor makes a `Function` for it, so the two arms covered different method sets — the one thing the side-by-side must never do. 3. The `For` arm double-counted and mis-filed its iteratee (both bots). It tagged the relation `iteratee` (so the fold filed it under `reads`) and then walked the iteratee again, sending `self.items` through `self_send` for a second untagged Read. The shipped arm files it under `traverses` and never walks it. Now tagged `traversal`, not walked. This is the same defect class as the `general_send` double count fixed in beb9c01 — a second instance the earlier fix did not reach. 4. Raise arguments inflated `reads` (Codex P2). `raise Error, self.message` really does read `message`, but the shipped arm records the exception and returns. The observation is kept — that is the point of this arm — inside a new `ScopeKind::RaiseArgs`, and the fold skips that subtree. A scope, not a `control` tag, because raise arguments nest and a flat tag marks only the outermost event. 5. The non-mutator self-send fallback inflated `calls` (Codex P1). `self.valid?` emitted an untagged `Call` where the shipped arm records nothing at all. Now tagged `self_send` and excluded from the fold. This NARROWS the one named divergence: `self.foo(arg)` now disagrees only in `reads`, not in `calls` as well, and the test asserts the other five sets agree so the disagreement cannot silently widen. 6. The census denominator omitted `set_guarded` (Codex P2). It summed five of the six shipped sets, so the preservation ratio was overstated on any corpus with a blank-guarded default — while `MethodOre` carried the sixth count all along. Also: name-keyed shipped-row lookup became an occurrence-ordered queue, so a class defining the same method name twice no longer hands every `MethodOre` the last definition's counts (Codex P2); and `ore_census` now REQUIRES `RAILS_SRC` instead of defaulting to a machine-specific absolute path, which produced a silent all-zero census — measured once in this session and briefly mistaken for a real result (CodeRabbit). MEASURED, same 1708 OpenProject files: events 40034 -> 44170 (Param 0 -> 3708; RaiseArgs scopes added) scopes 11987 -> 12172 symbols 3526 -> 4352 shipped entries 850 -> 857 (set_guarded contributes 7 corpus-wide) preservation 5.25x -> 5.21x The headline has now moved twice, both times DOWN and both times because a number was wrong: 5.40 -> 5.25 (traversal receiver counted twice) -> 5.21 (denominator missing a set). Worth noting `set_guarded` is only 7 entries across the entire corpus, so the fold's guarded path is barely exercised by real data — which is exactly why the gate fixture constructs it synthetically. The gate fixture now carries every shape above: a relation `for` loop, a raise with a field-reading argument, a non-mutator self-send, and parameters — each with an anti-vacuity assertion that the shape is actually present, so none of these can silently re-open. NOT changed, with reasons: - Codex P1 "correlate bodies instead of transcribing them": answered on the PR. The recipe codebook needs ordered body facts to correlate FROM; this arm is that input, not a competing consumer contract. - Codex P2 "nested declarations share the outer anchor": real, but fixing it needs `walk_class_body` to return per-declaration locations — a change to a shipped API that deserves its own PR rather than a drive-by. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- crates/ruff_ruby_spo/examples/ore_census.rs | 23 +- crates/ruff_ruby_spo/src/events.rs | 281 ++++++++++++++++++-- 2 files changed, 275 insertions(+), 29 deletions(-) diff --git a/crates/ruff_ruby_spo/examples/ore_census.rs b/crates/ruff_ruby_spo/examples/ore_census.rs index bb389887c4876e..21f8c8df12ae0e 100644 --- a/crates/ruff_ruby_spo/examples/ore_census.rs +++ b/crates/ruff_ruby_spo/examples/ore_census.rs @@ -13,9 +13,14 @@ use std::path::PathBuf; use ruff_ruby_spo::events::{EventKind, Symbols, source_ore}; fn main() { - let root = PathBuf::from( - std::env::var("RAILS_SRC").unwrap_or_else(|_| "/home/user/adaworldapi/openproject".into()), - ); + // Required, not defaulted. A machine-specific absolute default meant an + // unset variable produced a silent all-zero census with no diagnostic — + // measured once, in this session, and mistaken for a real result. + let Ok(root) = std::env::var("RAILS_SRC") else { + eprintln!("RAILS_SRC is unset — point it at a Rails source tree"); + return; + }; + let root = PathBuf::from(root); let limit: usize = std::env::var("LIMIT") .ok() .and_then(|v| v.parse().ok()) @@ -58,8 +63,18 @@ fn main() { if m.scopes.len() > 1 { with_multi_scope += 1; } + // All SIX, including `set_guarded`. Summing five made the + // denominator too small on any corpus containing a + // blank-guarded default, so the preservation ratio was + // overstated — while `MethodOre` carried the sixth count all + // along. set_total += u64::from( - m.set_reads + m.set_writes + m.set_raises + m.set_calls + m.set_traverses, + m.set_reads + + m.set_writes + + m.set_guarded + + m.set_raises + + m.set_calls + + m.set_traverses, ); for e in &m.events { *kinds.entry(e.kind.as_str()).or_default() += 1; diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs index f490e7cfda288a..1bd389f7b1a895 100644 --- a/crates/ruff_ruby_spo/src/events.rs +++ b/crates/ruff_ruby_spo/src/events.rs @@ -94,7 +94,7 @@ //! `anchor`, so lexical order stays checkable against traversal order rather //! than being silently conflated with it. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, VecDeque}; use std::fmt; use lib_ruby_parser::{Loc, Node}; @@ -258,6 +258,13 @@ pub enum ScopeKind { Rescue, Ensure, Lambda, + /// The argument list of a `raise`. A scope rather than a `control` tag + /// because the arguments nest (`raise Error, self.a.b`) and a flat tag + /// would mark only the outermost event. Everything under it is a real + /// observation that answers to NO shipped set — the shipped arm records + /// the exception and returns — so the fold skips this subtree instead of + /// counting reads the set arm never had. + RaiseArgs, } impl ScopeKind { @@ -277,6 +284,7 @@ impl ScopeKind { Self::For => "For", Self::Rescue => "Rescue", Self::Ensure => "Ensure", + Self::RaiseArgs => "RaiseArgs", Self::Lambda => "Lambda", } } @@ -732,9 +740,17 @@ impl Walk<'_> { .intern(SymKind::Exception, Role::None, &name, Prov::Parser) }); self.push(EventKind::Raise, None, exc, at, None, Prov::Parser); + // `raise Error, self.message` really does read `message`, and + // the shipped arm drops it — it records the exception and + // returns. Keeping the observation is the point of this arm, + // so the walk stays and the scope tag says where the facts + // came from; the fold then knows they answer to no shipped + // set rather than inflating `reads`. + self.enter(ScopeKind::RaiseArgs, at); for arg in &s.args { self.body(arg); } + self.exit(at); } // `self.` — write, mutator call, or read, in that priority // order, matching the set walker. @@ -772,6 +788,12 @@ impl Walk<'_> { self.general_send(s, at); } Node::For(f) => { + // Tagged `traversal`, not `iteratee`: the shipped arm files + // `for x in self.items` under `traverses` and never touches + // `reads`, and one fact must not answer to two tag names. + // The iteratee is NOT walked afterwards — doing so sent + // `self.items` through `self_send` and produced a second, + // untagged Read of the same relation. if let Some(rel) = crate::functions::node_relation_name(&f.iteratee, self.known) { let sym = self.attr(&rel); self.push( @@ -779,12 +801,11 @@ impl Walk<'_> { Some(sym), None, at, - Some("iteratee"), + Some("traversal"), Prov::Parser, ); } self.enter(ScopeKind::For, at); - self.body(&f.iteratee); if let Some(b) = f.body.as_deref() { self.body(b); } @@ -1026,6 +1047,12 @@ impl Walk<'_> { self.push(EventKind::Read, Some(sym), None, at, None, Prov::Parser); return; } + // The fallback: an explicit-self send that is neither a setter, nor an + // ActiveRecord mutator, nor an argument-free attribute read — + // `self.valid?`, `self.foo(1)`, `self == other`. The shipped arm + // records NOTHING for these (its three arms all miss), so the Call is + // tagged: it is a real observation, but it answers to no shipped set + // and must not inflate `calls`. let callee = self.callee(method); let recv = self .syms @@ -1035,7 +1062,7 @@ impl Walk<'_> { Some(recv), Some(callee), at, - None, + Some("self_send"), Prov::Parser, ); for arg in &s.args { @@ -1098,6 +1125,49 @@ impl Walk<'_> { } } + /// A method's parameters, in declaration order. + /// + /// [`EventKind::Param`] had no emitter before this: the variant was + /// documented, the alphabet advertised it, and a census over 5502 + /// `OpenProject` methods measured exactly zero of them, because `defs_of` + /// kept only `(name, body)` and threw the argument list away. `control` + /// carries the parameter's syntactic kind, which is the part the AST + /// states with certainty — required vs optional vs splat vs keyword — + /// while what a default EXPRESSION does stays an ordinary walked fact. + fn params(&mut self, args: Option<&Node>) { + let Some(Node::Args(a)) = args else { return }; + for p in &a.args { + let at = anchor(p); + let (name, kind) = match p { + Node::Arg(x) => (x.name.clone(), "required"), + Node::Optarg(x) => (x.name.clone(), "optional"), + Node::Restarg(x) => (x.name.clone().unwrap_or_default(), "rest"), + Node::Kwarg(x) => (x.name.clone(), "keyword"), + Node::Kwoptarg(x) => (x.name.clone(), "keyword_optional"), + Node::Kwrestarg(x) => (x.name.clone().unwrap_or_default(), "keyword_rest"), + Node::Blockarg(x) => (x.name.clone().unwrap_or_default(), "block"), + _ => continue, + }; + let sym = self + .syms + .intern(SymKind::Param, Role::Param, &name, Prov::Parser); + self.push( + EventKind::Param, + Some(sym), + None, + at, + Some(kind), + Prov::Parser, + ); + // A default is a real expression evaluated at call time. + match p { + Node::Optarg(x) => self.body(&x.default), + Node::Kwoptarg(x) => self.body(&x.default), + _ => {} + } + } + } + /// `x ||= v` / `x &&= v` on a `self` attribute — a write, tagged with /// WHICH operator wrote it. /// @@ -1267,7 +1337,7 @@ fn rails_arm(body: &Node) -> Vec { /// Collect every `def` in a class body, paired with the visibility the /// shipped walker assigns it, so the Ruby arm covers exactly the same method /// set the set arm does. -fn defs_of(body: &Node, out: &mut Vec<(String, Node)>) { +fn defs_of(body: &Node, out: &mut Vec) { match body { Node::Begin(b) => { for stmt in &b.statements { @@ -1284,11 +1354,12 @@ fn defs_of(body: &Node, out: &mut Vec<(String, Node)>) { defs_of(b, out); } } - Node::Def(d) => { - if let Some(b) = d.body.as_deref() { - out.push((d.name.clone(), b.clone())); - } - } + // The WHOLE `Def` node, not `(name, body)`. Keeping only the body + // dropped `def noop; end` entirely (the shipped extractor makes a + // `Function` for every `Def`, body or not) and threw away the + // argument list, which is why `EventKind::Param` had no emitter and + // measured zero across 5502 OpenProject methods. + Node::Def(_) => out.push(body.clone()), Node::Send(s) => { // `private def foo … end` — the def rides as an argument. for arg in &s.args { @@ -1322,13 +1393,13 @@ pub fn class_ore(iri: &str, name: &str, body: &Node, syms: &mut Symbols) -> Clas let (public_fns, helper_fns) = crate::functions::extract_functions_from_body(Some(body), &decls); let mut sets: BTreeMap = BTreeMap::new(); - let mut set_rows: BTreeMap = BTreeMap::new(); + let mut set_rows: BTreeMap> = BTreeMap::new(); for f in &public_fns { - set_rows.insert(f.name.clone(), f); + set_rows.entry(f.name.clone()).or_default().push_back(f); sets.insert(f.name.clone(), (0, true)); } for f in &helper_fns { - set_rows.insert(f.name.clone(), f); + set_rows.entry(f.name.clone()).or_default().push_back(f); sets.insert(f.name.clone(), (0, false)); } @@ -1336,12 +1407,27 @@ pub fn class_ore(iri: &str, name: &str, body: &Node, syms: &mut Symbols) -> Clas defs_of(body, &mut defs); let mut methods = Vec::new(); - for (method_name, def_body) in defs { + for def_node in defs { + let Node::Def(d) = &def_node else { continue }; + let method_name = d.name.clone(); + let at = anchor(&def_node); let mut walk = Walk::new(syms, &known); - walk.enter(ScopeKind::Method, anchor(&def_body)); - walk.body(&def_body); - walk.exit(anchor(&def_body)); - let shipped = set_rows.get(&method_name); + walk.enter(ScopeKind::Method, at); + // Parameters first, in declaration order, before any body fact. + walk.params(d.args.as_deref()); + // A body-less `def noop; end` still yields a MethodOre — the shipped + // extractor counts it, so omitting it would make the two arms cover + // different method sets. + if let Some(b) = d.body.as_deref() { + walk.body(b); + } + walk.exit(at); + // Popped, not looked up: a class may legally define the same name + // twice, and a plain `get` handed every such MethodOre the LAST + // definition's counts. Both arms enumerate the same body in the same + // order, so consuming in order pairs each ore with its own row. + let shipped = set_rows.get_mut(&method_name).and_then(VecDeque::pop_front); + let shipped = shipped.as_ref(); let public = sets.get(&method_name).is_none_or(|(_, p)| *p); methods.push(MethodOre { iri: format!("{iri}.{method_name}"), @@ -1490,10 +1576,35 @@ mod tests { }; let ctl = |e: &OreEvent, want: &str| e.control.as_deref() == Some(want); + // Every scope that is, or descends from, a `RaiseArgs` scope. The + // shipped arm records a raise's exception and returns without + // touching its arguments, so those observations answer to no shipped + // set. Computed as a closure over the tree rather than a flat tag + // because `raise Error, self.a.b` nests. + let mut in_raise: Vec = Vec::new(); + loop { + let before = in_raise.len(); + for sc in &m.scopes { + if in_raise.contains(&sc.id) { + continue; + } + let inherited = sc.parent.is_some_and(|pa| in_raise.contains(&pa)); + if sc.kind == ScopeKind::RaiseArgs || inherited { + in_raise.push(sc.id); + } + } + if in_raise.len() == before { + break; + } + } + let (mut reads, mut writes, mut guarded) = (Vec::new(), Vec::new(), Vec::new()); let (mut raises, mut traverses, mut calls) = (Vec::new(), Vec::new(), Vec::new()); for e in &m.events { + if in_raise.contains(&e.scope) { + continue; + } match e.kind { // A traversal is a Read tagged `traversal`; the shipped arm // files it under `traverses`, not `reads`. @@ -1515,7 +1626,9 @@ mod tests { } } EventKind::Raise => raises.extend(name_of(&e.obj)), - EventKind::Call => { + // `self_send` marks the non-mutator explicit-self fallback, + // for which the shipped arm records nothing at all. + EventKind::Call if !ctl(e, "self_send") => { if let (Some(r), Some(c)) = (name_of(&e.subj), name_of(&e.obj)) { calls.push(format!("{r}.{c}")); } @@ -1586,7 +1699,7 @@ mod tests { class Invoice < ApplicationRecord belongs_to :project has_many :lines - def settle + def settle(amount, note = 'x') self.total = 1 self.total = 2 self.state ||= 'draft' @@ -1594,9 +1707,13 @@ class Invoice < ApplicationRecord self.count += 1 self.total = 0 if self.total.blank? lines.each + for row in lines + self.seen = row + end + self.valid? self.save self.save - raise ArgumentError + raise ArgumentError, self.memo end end"); let m = method(&cs[0], "settle"); @@ -1639,6 +1756,100 @@ end"); .any(|e| e.control.as_deref() == Some("and_assign")), "`&&=` is present and tagged — it must NOT reach guarded_writes" ); + // Each shape below was a real bug found in review; the fixture must + // actually contain it or this gate re-opens silently. + assert!( + m.events.iter().any(|e| e.kind == EventKind::Param), + "parameters are emitted — `EventKind::Param` had no producer at \ + all and measured zero over 5502 real methods" + ); + assert!( + m.scopes.iter().any(|sc| sc.kind == ScopeKind::For), + "a relation `for` loop is present — its iteratee must fold to \ + `traverses`, never `reads`" + ); + assert!( + m.scopes.iter().any(|sc| sc.kind == ScopeKind::RaiseArgs), + "a raise carries an argument that reads a field — the shipped arm \ + never walks those, so they must not inflate `reads`" + ); + assert!( + m.events + .iter() + .any(|e| e.control.as_deref() == Some("self_send")), + "a non-mutator explicit-self send is present — the shipped arm \ + records nothing for it, so it must not inflate `calls`" + ); + } + + /// Both arms must cover the SAME method set, and parameters must exist. + /// + /// `defs_of` used to keep only `(name, body)`, which dropped every + /// body-less `def` — the shipped extractor builds a `Function` for each + /// one, so the two arms silently disagreed about which methods exist — + /// and threw the argument list away, leaving [`EventKind::Param`] with no + /// producer anywhere in the crate. + #[test] + fn every_def_yields_an_ore_and_parameters_are_emitted() { + let (cs, syms) = ore(r" +class Invoice < ApplicationRecord + def noop; end + def settle(amount, note = 1, *rest, key:, opt: 2, **kw, &blk) + self.total = amount + end +end"); + let c = &cs[0]; + assert_eq!( + c.methods.len(), + 2, + "a body-less `def noop; end` still gets a MethodOre" + ); + let noop = method(c, "noop"); + assert!( + noop.events.iter().any(|e| e.kind == EventKind::ScopeEnter), + "an empty method is an empty body, not an absent method" + ); + + let m = method(c, "settle"); + let params: Vec<(String, String)> = m + .events + .iter() + .filter(|e| e.kind == EventKind::Param) + .filter_map(|e| { + let n = subj_of(&syms, e)?.2; + Some((n, e.control.clone()?)) + }) + .collect(); + assert_eq!( + params, + vec![ + ("amount".into(), "required".into()), + ("note".into(), "optional".into()), + ("rest".into(), "rest".into()), + ("key".into(), "keyword".into()), + ("opt".into(), "keyword_optional".into()), + ("kw".into(), "keyword_rest".into()), + ("blk".into(), "block".into()), + ], + "every parameter kind, in declaration order" + ); + // Order is the whole point: params precede the first body fact. + let first_param = m.events.iter().position(|e| e.kind == EventKind::Param); + let first_write = m.events.iter().position(|e| e.kind == EventKind::Write); + assert!(first_param < first_write, "parameters come before the body"); + // And the six-set fold still holds with parameters in the stream. + assert_eq!( + fold_six(&syms, m), + [ + m.set_reads, + m.set_writes, + m.set_guarded, + m.set_raises, + m.set_traverses, + m.set_calls + ], + "parameters are additive — they belong to no shipped set" + ); } /// The ONE named divergence, pinned so it cannot drift silently. @@ -1668,13 +1879,33 @@ end"); // Shipped: one read (`foo`), no calls — `foo` is not an AR mutator. assert_eq!(m.set_reads, 1, "shipped arm counts `self.foo(1)` as a read"); assert_eq!(m.set_calls, 0, "shipped arm records no call for it"); - // Ore: no read, one call. + // Ore: no read. The disagreement is ONE-SIDED and confined to `reads` + // — the ore is missing a fact the shipped arm invents, not inventing + // one of its own. assert_eq!(got[0], 0, "ore emits no Read for a send carrying arguments"); - assert_eq!(got[5], 1, "ore emits a Call instead"); + assert_eq!( + got[5], 0, + "and the Call it does emit is tagged `self_send`, so it does NOT \ + inflate `calls` — the shipped arm has none either" + ); + // The observation is still carried; only its FOLD is suppressed. assert!( - m.events.iter().any(|e| e.kind == EventKind::Call), - "the Call event is the ore's reading of this shape" + m.events + .iter() + .any(|e| e.kind == EventKind::Call && e.control.as_deref() == Some("self_send")), + "the tagged Call is the ore's reading of this shape — preserved, \ + just not answerable to a shipped set" ); + // Every other set agrees, so this really is the only disagreement. + let want = [ + m.set_reads, + m.set_writes, + m.set_guarded, + m.set_raises, + m.set_traverses, + m.set_calls, + ]; + assert_eq!(got[1..], want[1..], "only `reads` may disagree"); } // ── order ────────────────────────────────────────────────────────── From bc8e25ab15906505f41abf28812d271520aba8e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 04:09:15 +0000 Subject: [PATCH 5/8] ruff_ruby_spo: narrow the traversal-receiver suppression to BARE receivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suppression added in beb9c01 was over-broad, and CodeRabbit caught it. The shipped walker walks EVERY receiver of a traversed send. The two forms then diverge, because the receiver node is different: lines.each recv is `lines` -> Send{recv: None} -> shipped's general Send arm records NOTHING self.lines.each recv is `self.lines` -> Send{recv: Self_} -> shipped's self-send arm records reads["lines"] So suppressing the ore's own receiver re-walk is correct for the bare form (it was emitting a second untagged Read of a name the set arm files only under `traverses`) and WRONG for the explicit-self form, where it deleted a read the shipped arm genuinely has. Now conditioned on the receiver actually being a bare send. Every traversal fixture in the crate used the bare form, so nothing caught it — the third time in this arc that a fix was right about the case in the fixture and wrong about the case that was not. The new test carries both forms side by side and asserts they are DIFFERENT shipped facts (bare: reads 0 / traverses 1; explicit: reads 1 / traverses 1), so neither can be silently collapsed into the other again. Disable-verified: restoring the blanket `recv_consumed = true` fails the new test with reads folding 0 where the shipped arm has 1. Also adds a Delegation section to AGENTS.md: grindwork (call-site sweeps, existence checks, scoped command runs, corpus censuses) goes to Sonnet subagents; judgment (is this finding right, what does this measurement mean, is this divergence defensible) stays on the main thread. Operator-directed, and general across repositories rather than specific to this one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- AGENTS.md | 17 +++++++++ crates/ruff_ruby_spo/src/events.rs | 60 +++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 113063e2671d87..c43eb205abeffc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,23 @@ This repository contains both Ruff (a Python linter and formatter) and ty (a Pyt The `ruff_*_spo` crates (`ruff_ruby_spo`, `ruff_python_spo`, `ruff_csharp_spo`, `ruff_cpp_spo`, `ruff_spo_triplet`, …) are the AdaWorldAPI **SPO/transcode** side — AST → `(subject, predicate, object)` fact harvest feeding the OGAR transpiler. Methods for that side are curated in `.claude/knowledge/` (each with a `READ BY:` header) and applied by agents in `.claude/agents/`. **Before harvesting method-body facts or lowering a behaviour arm, read `.claude/knowledge/fuzzy-recipe-codebook.md`** — it teaches how to cook a `(verb, criteria)` recipe codebook and correlate fuzzy imperative bodies to declarative recipes, rather than transcribing bodies. The `fuzzy-proposer` agent carries that method. **Before starting (or resuming) any whole legacy-app→Rust transcode — odoo→odoo-rs, redmine/OpenProject→openproject-nexgen-rs, WoA→woa-rs (worked reference: MedCare→MedCare-rs) — read `.claude/knowledge/consumer-transcode-furnace-playbook.md`** — it frames the loop *around* the codebook: the ore/slag furnace, the two parity oracles (value + Klickwege structure), the three-axis mint gate, the no-hand-roll rule, and a per-consumer portability map. +## Delegation: grindwork goes to Sonnet agents + +Fan mechanical work out to Sonnet subagents rather than running it on the main +thread. The split is by SHAPE of the work, not by how hard it looks: + +| | | +|---|---| +| **Sonnet agents** | find every call site of X; quote a definition; trace a constant's readers; check whether a symbol exists anywhere; run a scoped command and report the tail; census a corpus | +| **Main thread** | whether a review finding is actually correct; what a measurement means; designing a falsifier; deciding a divergence is defensible rather than a defect | + +The rule bites on SWEEPS — "every site that reads this", "does this exist", +"which of these five shapes are real". For a single lookup where the file and +symbol are already known, going direct is cheaper than briefing an agent. + +Never Haiku for any of it. This mirrors the grindwork/accumulation split +already codified in the sibling `lance-graph` workspace. + ## Code Review Rules When reviewing a branch or pull request, be deliberately nitpicky. Report not diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs index 1bd389f7b1a895..cbdef8514bf6ac 100644 --- a/crates/ruff_ruby_spo/src/events.rs +++ b/crates/ruff_ruby_spo/src/events.rs @@ -1091,7 +1091,16 @@ impl Walk<'_> { Some("traversal"), Prov::Parser, ); - recv_consumed = true; + // ONLY for a BARE relation receiver (`lines.each`). The shipped + // arm walks every receiver: a bare `lines` reaches its general + // Send arm and records nothing, but `self.lines` reaches its + // self-send arm and DOES record a read. Suppressing both was + // over-broad — it fixed the bare double count and silently + // deleted a read the set arm really has. + recv_consumed = matches!( + s.recv.as_deref(), + Some(Node::Send(r)) if r.recv.is_none() + ); } else if s.recv.is_none() && crate::functions::is_attr_ident(&s.method_name) && s.args.is_empty() @@ -1852,6 +1861,55 @@ end"); ); } + /// BOTH traversal receiver forms, because they behave differently. + /// + /// The shipped arm walks every receiver. A BARE `lines` reaches its + /// general Send arm and records nothing; an explicit-self `self.lines` + /// reaches its self-send arm and records a READ. So the ore may suppress + /// its own receiver re-walk for the bare form only — suppressing both + /// (the first version of this fix) deleted a read the set arm really has, + /// and no fixture caught it because every traversal fixture was bare. + #[test] + fn both_traversal_receiver_forms_fold_to_the_shipped_sets() { + let (cs, syms) = ore(r" +class Invoice < ApplicationRecord + has_many :lines + def bare + lines.each + end + def explicit + self.lines.each + end +end"); + for name in ["bare", "explicit"] { + let m = method(&cs[0], name); + assert_eq!( + fold_six(&syms, m), + [ + m.set_reads, + m.set_writes, + m.set_guarded, + m.set_raises, + m.set_traverses, + m.set_calls + ], + "`{name}` traversal must fold to the shipped six sets" + ); + } + // The two forms are NOT the same shipped fact, which is the whole + // reason one may be suppressed and the other may not. + let bare = method(&cs[0], "bare"); + let explicit = method(&cs[0], "explicit"); + assert_eq!(bare.set_traverses, 1, "both are traversals"); + assert_eq!(explicit.set_traverses, 1, "both are traversals"); + assert_eq!(bare.set_reads, 0, "a BARE receiver is not a read"); + assert_eq!( + explicit.set_reads, 1, + "an explicit-self receiver IS a read — suppressing it would delete \ + a shipped fact" + ); + } + /// The ONE named divergence, pinned so it cannot drift silently. /// /// `self.foo(arg)` — an explicit-self send of an attribute-shaped name From 54c533d790c6f7cf949fd9748a41cc32799bcae8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 04:10:42 +0000 Subject: [PATCH 6/8] AGENTS.md: document the session traps that cost time in this repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four, each from a measured incident in this session rather than from general advice: 1. Commit before disabling. The disable cycle restores with `git checkout `, which reverts to the last COMMIT — so uncommitted work under test is deleted by its own verification step. A fix and its new test were both lost this way; the loss surfaced only when a later grep showed the old code back in place. 2. A disable that does not apply looks exactly like a guard that is not load-bearing. Twice a patch script anchored on text `cargo fmt` had reformatted, no-opped silently, and the green run nearly entered the record as evidence of inertness. Assert the replacement count. 3. `git status` says clean, not current. A stale branch manufactures findings that do not exist on main — with the fetch/merge-base commands to check, and the rule that a measurement must be re-taken after a rebase rather than carried across it. 4. A green fixture proves only the shape it contains. Three fixes in ruff_ruby_spo were right about the case in the fixture and wrong about the case that was not; when a fix distinguishes two shapes, the test carries both and asserts they differ. Operator-directed, and general rather than crate-specific. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- AGENTS.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index c43eb205abeffc..42cb782532cc6b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,48 @@ symbol are already known, going direct is cheaper than briefing an agent. Never Haiku for any of it. This mirrors the grindwork/accumulation split already codified in the sibling `lance-graph` workspace. +## Session traps (each one cost real time in this repo) + +**Commit BEFORE you disable, not after.** The disable-verification cycle is +`break the guard → watch the test fail → restore`, and the restore is +`git checkout `. That reverts to the last COMMIT, not to the state you +started from — so if the work under test is uncommitted, the restore deletes +it. Measured: a fix plus its new test were both eaten this way, and the loss +was only noticed because a later grep showed the old code back in place. +Order is: commit, then disable, then checkout. + +**A disable that does not apply is indistinguishable from a guard that is not +load-bearing.** Both look like "the test still passes". Twice here a patch +script anchored on text that `cargo fmt` had since reformatted, the edit +silently no-opped, and the green run nearly went into the record as evidence +that a guard was inert. Always assert the replacement actually happened: + +```python +assert s.count(old) == 1, "anchor moved" +``` + +Related: turning a knob is only a disable if the knob BINDS. Zeroing a +constant proves nothing when the guarded quantity can reach the same outcome +by another route, or when a different `min()` term is the binding one. + +**`git status` reports CLEAN, not CURRENT.** A stale branch does not merely +give stale answers — it manufactures findings that do not exist on `main`. +Before the first measurement in a fresh container, and after any rebase: + +```sh +git fetch origin main && git log --oneline -1 origin/main +git merge-base --is-ancestor origin/main HEAD || echo "STALE — re-measure after rebase" +``` + +Re-run the measurement after a rebase; a conclusion drawn before it does not +survive it, and has previously outlived the rebase in a PR body. + +**A green fixture proves the shape it contains, nothing else.** Several fixes +in this crate were correct for the case in the fixture and wrong for the case +that was not — a bare `lines.each` fixture passing while `self.lines.each` +was silently broken. When a fix distinguishes two shapes, the test carries +both, and asserts they differ. + ## Code Review Rules When reviewing a branch or pull request, be deliberately nitpicky. Report not From b2a9e98d201ec0b64ddb01fb35788192b36749ee Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 04:20:04 +0000 Subject: [PATCH 7/8] Fix the prek failure this file's own traps section caused The delegation table added in 07d5c34 used a compact pipe table; mdformat pads every cell to the widest in its column, so the prek CI job failed with "Files were modified by following hooks". Replaced with bold-lead prose, which matches the surrounding style and mdformat leaves alone. Recorded as a trap in the same section: prek is unrunnable in this sandbox (uv cannot parse the repo's exclude-newer), so a Markdown edit here ships unchecked, and a green prek on the previous commit is not evidence about this one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- AGENTS.md | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 42cb782532cc6b..a1b53da777fa5b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,10 +9,13 @@ The `ruff_*_spo` crates (`ruff_ruby_spo`, `ruff_python_spo`, `ruff_csharp_spo`, Fan mechanical work out to Sonnet subagents rather than running it on the main thread. The split is by SHAPE of the work, not by how hard it looks: -| | | -|---|---| -| **Sonnet agents** | find every call site of X; quote a definition; trace a constant's readers; check whether a symbol exists anywhere; run a scoped command and report the tail; census a corpus | -| **Main thread** | whether a review finding is actually correct; what a measurement means; designing a falsifier; deciding a divergence is defensible rather than a defect | +**Sonnet agents** — find every call site of X; quote a definition; trace a +constant's readers; check whether a symbol exists anywhere; run a scoped command +and report the tail; census a corpus. + +**Main thread** — whether a review finding is actually correct; what a +measurement means; designing a falsifier; deciding a divergence is defensible +rather than a defect. The rule bites on SWEEPS — "every site that reads this", "does this exist", "which of these five shapes are real". For a single lookup where the file and @@ -57,6 +60,14 @@ git merge-base --is-ancestor origin/main HEAD || echo "STALE — re-measure afte Re-run the measurement after a rebase; a conclusion drawn before it does not survive it, and has previously outlived the rebase in a PR body. +**`prek` cannot run in this sandbox, so Markdown edits ship unchecked.** The +local `uv run --locked prek` fails on a `uv` version that cannot parse this +repo's `exclude-newer`, so the hook set never runs here — and CI's `prek` job +then fails on formatting the edit introduced. It has already happened once, on +the commit that added *this* section. Avoid Markdown constructs `mdformat` +rewrites (pipe tables are padded to the widest cell); prefer prose and bullets, +which it leaves alone. A `prek` green on commit N says nothing about commit N+1. + **A green fixture proves the shape it contains, nothing else.** Several fixes in this crate were correct for the case in the fixture and wrong for the case that was not — a bare `lines.each` fixture passing while `self.lines.each` From d860a5e8882e992ba73cb1b886dfdcbe104c2d6e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 04:24:51 +0000 Subject: [PATCH 8/8] ruff_ruby_spo: document the walker's scope and interning helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit flagged docstring coverage at 79.69%. Documented the five helpers where the invariant is not evident from the signature: an empty scope stack means the method scope rather than an error; parent() is what lets the fold attribute a Write to the branch guarding it; attr() consults the class's own declarations, so the same identifier is a relation in one class and an attribute in another, which is why it cannot be free. Left the two Display impls and the two test helpers undocumented — a doc comment there would narrate the signature, which AGENTS.md rules out. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011JRoXmYPrE8KubXVrDB71L --- crates/ruff_ruby_spo/src/events.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/ruff_ruby_spo/src/events.rs b/crates/ruff_ruby_spo/src/events.rs index cbdef8514bf6ac..61498e4ea22ff4 100644 --- a/crates/ruff_ruby_spo/src/events.rs +++ b/crates/ruff_ruby_spo/src/events.rs @@ -603,15 +603,25 @@ impl<'s> Walk<'s> { } } + /// The scope the walker is currently inside. Zero is the method scope + /// itself, which is never pushed — so an empty stack is not an error. fn cur(&self) -> u32 { self.stack.last().copied().unwrap_or(0) } + /// The scope enclosing [`Self::cur`], or `None` at the method scope. + /// Used to link a scope to its parent so the fold can walk the chain + /// upwards — that is how a `Write` inside an `if` is attributed to the + /// branch that guards it. fn parent(&self) -> Option { let n = self.stack.len(); (n >= 2).then(|| self.stack[n - 2]) } + /// The sequence number the next event will carry. Events are numbered by + /// emission order, which IS the source order this module exists to keep; + /// the saturating conversion is unreachable in practice (it would need a + /// single method with 4 billion events) and is preferred to a panic. fn next_seq(&self) -> u32 { u32::try_from(self.events.len()).unwrap_or(u32::MAX) } @@ -691,6 +701,11 @@ impl<'s> Walk<'s> { }); } + /// Intern `name` as an own-role symbol, classifying it as a relation when + /// the class declared one under that name and as a plain attribute + /// otherwise. The `known` list is the class's own declarations, so the + /// same identifier can be a relation in one class and an attribute in + /// another — which is why this cannot be a free function. fn attr(&mut self, name: &str) -> String { let kind = if self.known.iter().any(|r| r == name) { SymKind::Relation @@ -700,6 +715,9 @@ impl<'s> Walk<'s> { self.syms.intern(kind, Role::Own, name, Prov::Parser) } + /// Intern `name` as a call target. Callees carry no role: the receiver is + /// recorded separately as the event's subject, so the symbol itself is + /// just the method name. fn callee(&mut self, name: &str) -> String { self.syms .intern(SymKind::Callee, Role::None, name, Prov::Parser)