From b281a52255aca76ac2d77173c88b98115530840c Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 19:51:11 -0600 Subject: [PATCH 01/14] fix(rust): resolve methods on the type the caller means, not its leaf name The Rust impl index keyed types by leaf name, so `a::Config` and `b::Config` shared one `(Config, load)` entry and the inherent method won: `use crate::b::Config; c.load()` produced a `declared-receiver-type` edge to `a::Config::load`. The index now records which files declare each type name and the file of every impl. When several files declare a name, a call resolves only among impls of the type the calling file means (its own declaration, then a `use` binding, then the only declaration); with no such binding the call is left external as `ambiguous` instead of guessed. Unique names behave as before. Test: rust_methods::same_named_types_in_different_modules_resolve_through_use_bindings fails before this change (edge to src/a.rs). --- src/scan.rs | 94 +++++++++++++++++++++++++++++++++++++------ tests/rust_methods.rs | 49 ++++++++++++++++++++++ 2 files changed, 131 insertions(+), 12 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index 244eca9..87d98d2 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -140,16 +140,21 @@ struct RustCall { receiver: Option, } -/// A method node id and the trait it implements, if any. -type RustMethod = (String, Option); +/// A method node id, the trait it implements (if any), and the file of its `impl`. +type RustMethod = (String, Option, String); -/// Crate-wide Rust impl/trait facts keyed by leaf type names. +/// Crate-wide Rust impl/trait facts keyed by leaf type names. Leaf names are not +/// identities: when several files declare the same name, `rust_type_file` settles +/// which one a use site means (declaring file, then `use` binding) or reports it +/// as ambiguous. #[derive(Default)] struct RustIndex { /// (type or trait, method) -> candidate methods. methods: HashMap<(String, String), Vec>, /// Struct, enum, and trait names declared in the repository. types: HashSet, + /// Leaf type/trait name -> files declaring it. + type_files: HashMap>, /// (struct, field) -> declared type. fields: HashMap<(String, String), crate::source::CppType>, /// Method node id -> declared return type. @@ -657,6 +662,14 @@ impl ScanState { let Some(id) = id else { continue }; if declaration.kind == NodeKind::Class { self.rust.types.insert(declaration.name.clone()); + let files = self + .rust + .type_files + .entry(declaration.name.clone()) + .or_default(); + if !files.iter().any(|file| file == path) { + files.push(path.to_owned()); + } continue; } let Some(owner) = declaration @@ -676,7 +689,7 @@ impl ScanState { .methods .entry((owner, declaration.name.clone())) .or_default() - .push((id.clone(), implemented)); + .push((id.clone(), implemented, path.to_owned())); if let Some(returns) = &declaration.return_type { self.rust.returns.insert(id.clone(), returns.clone()); } @@ -1690,7 +1703,7 @@ impl ScanState { (field.accessed(true), "declared-receiver-type") } Some(Receiver::Returned { ty, func, fallible }) => ( - self.rust_constructed_type(ty, func, *fallible, call) + self.rust_constructed_type(ty, func, *fallible, call, path) .ok_or("receiver-type-unknown")?, "declared-receiver-type", ), @@ -1699,7 +1712,7 @@ impl ScanState { if self.rust_type_is_foreign(&ty, path) { return Err("receiver-type-external"); } - self.rust_method(&ty, method).map(|id| (id, label)) + self.rust_method(&ty, method, path).map(|id| (id, label)) } /// A type name the caller's file imports from outside the crate. @@ -1746,30 +1759,86 @@ impl ScanState { if !self.rust.types.contains(&ty) { return Err("qualifier-unknown"); } - self.rust_method(&ty, method) + self.rust_method(&ty, method, path) } - fn rust_method(&self, ty: &str, method: &str) -> Result { + fn rust_method(&self, ty: &str, method: &str, path: &str) -> Result { let candidates = self .rust .methods .get(&(ty.to_owned(), method.to_owned())) .map(Vec::as_slice) .unwrap_or_default(); - let inherent: Vec<_> = candidates.iter().filter(|(_, t)| t.is_none()).collect(); - let chosen: Vec<_> = if inherent.is_empty() { + // Several repository types share this leaf name: keep only the impls of the + // one this use site means, or refuse to guess. + let candidates: Vec<&RustMethod> = if self + .rust + .type_files + .get(ty) + .is_some_and(|files| files.len() > 1) + { + let Some(meant) = self.rust_type_file(ty, path) else { + return Err("ambiguous"); + }; + candidates + .iter() + .filter(|(_, _, impl_path)| { + self.rust_type_file(ty, impl_path).as_deref() == Some(meant.as_str()) + }) + .collect() + } else { candidates.iter().collect() + }; + let inherent: Vec<_> = candidates + .iter() + .copied() + .filter(|(_, t, _)| t.is_none()) + .collect(); + let chosen: Vec<_> = if inherent.is_empty() { + candidates } else { inherent }; match chosen.as_slice() { - [(id, _)] => Ok(id.clone()), + [(id, _, _)] => Ok(id.clone()), [] if self.rust.types.contains(ty) => Err("member-not-found"), [] => Err("receiver-type-external"), _ => Err("ambiguous"), } } + /// The file declaring the repository type `leaf` as seen from `path`: the file + /// itself, then a `use` binding that names it, then the only declaration. `None` + /// when several files declare `leaf` and nothing in `path` says which. + fn rust_type_file(&self, leaf: &str, path: &str) -> Option { + let files = self.rust.type_files.get(leaf)?; + if files.iter().any(|file| file == path) { + return Some(path.to_owned()); + } + for binding in self + .source_bindings + .get(path) + .into_iter() + .flatten() + .filter(|binding| binding.local == leaf) + { + let Some(file) = self + .resolve_import(&binding.module, "rust", path) + .and_then(|id| self.node(&id)) + .and_then(|node| node.path.clone()) + else { + continue; + }; + if files.contains(&file) { + return Some(file); + } + } + match files.as_slice() { + [only] => Some(only.clone()), + _ => None, + } + } + /// `Type::func(..)` yields `Type` when `func` is declared to return `Self` /// or `Type` (behind `?`, also `Result<..>`/`Option<..>` of either). fn rust_constructed_type( @@ -1778,13 +1847,14 @@ impl ScanState { func: &str, fallible: bool, call: &RustCall, + path: &str, ) -> Option { let ty = if ty == "Self" { call.self_type.clone()? } else { ty.to_owned() }; - let id = self.rust_method(&ty, func).ok()?; + let id = self.rust_method(&ty, func, path).ok()?; let returns = self.rust.returns.get(&id)?; let returns_type = |value: &str| value == "Self" || value == ty; let direct = returns_type(returns); diff --git a/tests/rust_methods.rs b/tests/rust_methods.rs index fdd79a3..72849b2 100644 --- a/tests/rust_methods.rs +++ b/tests/rust_methods.rs @@ -153,3 +153,52 @@ fn foreign_imports_generics_and_competing_trait_impls_never_guess() { ); } } + +#[test] +fn same_named_types_in_different_modules_resolve_through_use_bindings() { + // Types were keyed by leaf name, so `b::Config` (method from a trait impl) and + // `a::Config` (inherent method) shared one key and the inherent one won: a call on + // `b::Config` was attributed to `a::Config::load`. + let model = scan_files(&[ + ("src/lib.rs", "pub mod a;\npub mod b;\npub mod user;\n"), + ( + "src/a.rs", + "pub struct Config;\nimpl Config {\n pub fn load(&self) -> u32 { 1 }\n}\n", + ), + ( + "src/b.rs", + "pub struct Config;\npub trait Loader { fn load(&self) -> u32; }\nimpl Loader for Config {\n fn load(&self) -> u32 { 2 }\n}\n", + ), + ( + "src/user.rs", + "use crate::b::Config;\nuse crate::b::Loader;\npub fn run(c: &Config) -> u32 { c.load() }\n", + ), + ( + "src/other.rs", + "pub fn guess(c: &Config) -> u32 { c.load() }\n", + ), + ]); + let targets = |source: &Node| { + model + .edges + .iter() + .filter(|e| e.kind == EdgeKind::Calls && e.source == source.id) + .map(|e| { + let target = model.nodes.iter().find(|n| n.id == e.target).unwrap(); + ( + target.path.clone().unwrap_or_default(), + e.attributes + .get("source.unresolved-reason") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_owned(), + ) + }) + .collect::>() + }; + let run = function(&model, "src/user.rs", "run"); + assert_eq!(targets(run), [("src/b.rs".to_owned(), String::new())]); + // Without a binding, two same-named types leave the call ambiguous, not guessed. + let guess = function(&model, "src/other.rs", "guess"); + assert_eq!(targets(guess), [(String::new(), "ambiguous".to_owned())]); +} -- 2.52.0 From 195d118702ce8063c0c8ef3ba53f11ff08ddd149 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 19:53:10 -0600 Subject: [PATCH 02/14] fix(cpp): treat rival using-directives as ambiguous, not first-come With `using namespace a; using namespace b;` and both `a::run` and `b::run` declared, a call to `run` resolved to whichever directive came first in file order and was labelled `using-directive`. In C++ that call is ambiguous (or an overload choice Brick does not model). Directives are now grouped: each `using N::x;` and `using namespace N;` nominates at most one declaration (its innermost resolving scope), and the call resolves only when exactly one distinct declaration is nominated; otherwise it stays external as `ambiguous`. Test: cpp_xtu::two_using_directives_naming_the_same_function_are_ambiguous fails before this change; its single-directive case still resolves. --- src/scan.rs | 58 +++++++++++++++++++++++++++++++++++++++--------- tests/cpp_xtu.rs | 30 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index 87d98d2..c2c87e0 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -1480,11 +1480,24 @@ impl ScanState { } } if !absolute { - for qualified in self.cpp_using_candidates(name, &call.lookup_scope, &closure) { - if let Some(result) = self.cpp_lookup(&qualified, &closure) { - return result.map(|id| (id, Some("using-directive"))); + // Every directive is consulted: two that nominate different + // declarations make the call ambiguous rather than first-come. + let mut nominated: Vec<(String, Result)> = Vec::new(); + for group in self.cpp_using_candidate_groups(name, &call.lookup_scope, &closure) { + for qualified in group { + if let Some(result) = self.cpp_lookup(&qualified, &closure) { + if !nominated.iter().any(|(seen, _)| seen == &qualified) { + nominated.push((qualified, result)); + } + break; + } } } + match nominated.as_slice() { + [] => {} + [(_, result)] => return result.clone().map(|id| (id, Some("using-directive"))), + _ => return Err("ambiguous"), + } } Err("not-visible") } @@ -1601,7 +1614,27 @@ impl ScanState { scope: &str, closure: &HashSet, ) -> Vec { - let mut candidates = Vec::new(); + let mut candidates: Vec = self + .cpp_using_candidate_groups(name, scope, closure) + .into_iter() + .flatten() + .collect(); + let mut seen = HashSet::new(); + candidates.retain(|candidate| seen.insert(candidate.clone())); + candidates + } + + /// `cpp_using_candidates` grouped by directive: one group per `using N::x;` + /// and per `using namespace N;`, whose members are the scope levels that + /// directive may name (innermost first). Separate groups are rival + /// nominations, so more than one resolving group is ambiguous. + fn cpp_using_candidate_groups( + &self, + name: &str, + scope: &str, + closure: &HashSet, + ) -> Vec> { + let mut groups = Vec::new(); let (head, rest) = match name.split_once("::") { Some((head, rest)) => (head, Some(rest)), None => (name, None), @@ -1611,21 +1644,24 @@ impl ScanState { for file in files { for (leaf, target) in self.cpp.using_names.get(file).into_iter().flatten() { if leaf == head { - candidates.push(match rest { + groups.push(vec![match rest { Some(rest) => qualify(target, rest), None => target.clone(), - }); + }]); } } for namespace in self.cpp.using_namespaces.get(file).into_iter().flatten() { - for level in scope_levels(scope) { - candidates.push(qualify(&qualify(&level, namespace), name)); - } + groups.push( + scope_levels(scope) + .into_iter() + .map(|level| qualify(&qualify(&level, namespace), name)) + .collect(), + ); } } let mut seen = HashSet::new(); - candidates.retain(|candidate| seen.insert(candidate.clone())); - candidates + groups.retain(|group| seen.insert(group.clone())); + groups } /// Method lookup in a class, then breadth-first through in-repository bases. diff --git a/tests/cpp_xtu.rs b/tests/cpp_xtu.rs index f2e2ebe..0ae41f8 100644 --- a/tests/cpp_xtu.rs +++ b/tests/cpp_xtu.rs @@ -326,3 +326,33 @@ fn brace_default_arguments_do_not_hide_prototypes_or_report_errors() { assert_eq!(reason, None); assert!(model.diagnostics.is_empty(), "{:?}", model.diagnostics); } + +#[test] +fn two_using_directives_naming_the_same_function_are_ambiguous() { + // With `using namespace a; using namespace b;` and both `a::run` and `b::run` + // defined, the call resolved to whichever directive came first. C++ makes that + // ambiguous (or an overload choice Brick cannot make), so it must stay external. + let model = scan_files(&[ + ("src/a.h", "namespace a { int run(int x); }\n"), + ("src/b.h", "namespace b { int run(int x); }\n"), + ( + "src/defs.cpp", + "#include \"a.h\"\n#include \"b.h\"\nnamespace a { int run(int x) { return x; } }\nnamespace b { int run(int x) { return -x; } }\n", + ), + ( + "src/main.cpp", + "#include \"a.h\"\n#include \"b.h\"\nusing namespace a;\nusing namespace b;\nint main() { return run(1); }\n", + ), + // Clean negative: one directive still resolves with its label. + ( + "src/only_a.cpp", + "#include \"a.h\"\nusing namespace a;\nint only() { return run(1); }\n", + ), + ]); + let (target, reason) = single_call(&model, function(&model, "src/main.cpp", "main")); + assert_eq!(target.kind, NodeKind::External); + assert_eq!(reason.as_deref(), Some("ambiguous")); + let (target, reason) = single_call(&model, function(&model, "src/only_a.cpp", "only")); + assert_eq!(target.path.as_deref(), Some("src/defs.cpp")); + assert_eq!(reason, None); +} -- 2.52.0 From 2b1914255767c8e96d8637eeb0809526dd84fabd Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 19:58:55 -0600 Subject: [PATCH 03/14] fix(cpp): never resolve calls into #elif/#else branches Every preprocessor branch was indexed as live. In yams, InternalEventBus.h guards spdlog with `#if __has_include() ... #else namespace spdlog { stubs }`, so 1,243 spdlog::warn/info/error call sites resolved to the fallback stubs, which became the 4th and 5th highest fan-in nodes. Brick does not evaluate conditions, so only the first branch is treated as live: declarations and prototypes inside `#elif`/`#else` stay in the graph (marked `source.preprocessor-branch: alternative`) but are not definitions, declarations or classes for call resolution. Branches are tracked textually from the directive lines, because conditions like `__has_include()` leave tree-sitter-cpp with an error-recovered tree that has no `preproc_else` node to find. Include-guard (`#ifndef`) code is a first branch and still resolves. Test: cpp_xtu::preprocessor_alternative_branches_are_not_call_targets fails before this change (spdlog::info resolves into the #elif stub). --- src/scan.rs | 9 +++++++ src/source.rs | 66 +++++++++++++++++++++++++++++++++++++++++++++++- tests/cpp_xtu.rs | 50 ++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 1 deletion(-) diff --git a/src/scan.rs b/src/scan.rs index c2c87e0..fab8ed5 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -616,6 +616,12 @@ impl ScanState { if declaration.is_virtual { attributes.insert("source.virtual".into(), serde_json::json!(true)); } + if declaration.preprocessor_alternative { + attributes.insert( + "source.preprocessor-branch".into(), + serde_json::json!("alternative"), + ); + } if document.parse_incomplete && declaration.kind == NodeKind::Function { attributes.insert("source.parse-incomplete".into(), serde_json::json!(true)); } @@ -712,6 +718,9 @@ impl ScanState { } } for (declaration, id) in document.declarations.iter().zip(&ids) { + if declaration.preprocessor_alternative { + continue; + } if declaration.kind == NodeKind::Class && let Some(name) = &declaration.qualified_name { diff --git a/src/source.rs b/src/source.rs index 8228191..95ff1ca 100644 --- a/src/source.rs +++ b/src/source.rs @@ -30,6 +30,9 @@ pub(crate) struct Declaration { /// Rust outer attributes that may register the item with a framework /// (`test`, `tokio::main`, `tool`, ...), excluding lint/doc/cfg attributes. pub attributes: Vec, + /// C/C++: inside an `#elif`/`#else` branch. Which branch is compiled is + /// unknown, so such declarations are structure, never resolution targets. + pub preprocessor_alternative: bool, } pub(crate) struct ImportBinding { @@ -129,6 +132,56 @@ pub(crate) fn supports(language: &str) -> bool { ) } +/// Inside an `#elif`/`#else` branch of a conditional. Brick does not evaluate +/// preprocessor conditions, so only the first branch is treated as live. The +/// directive lines are tracked textually: conditions such as +/// `#if __has_include()` leave tree-sitter with an error-recovered tree +/// that has no `preproc_else` node to find. +fn in_preprocessor_alternative(node: Node<'_>, alternative_lines: &[bool]) -> bool { + alternative_lines + .get(node.start_position().row) + .copied() + .unwrap_or(false) +} + +/// Per 0-based line: inside a non-first branch of some enclosing conditional. +fn preprocessor_alternative_lines(source: &str) -> Vec { + // One entry per open conditional: the index of the branch being read. + let mut branches: Vec = Vec::new(); + source + .lines() + .map(|line| { + let directive = line.trim_start(); + let directive = directive + .strip_prefix('#') + .map(|rest| rest.trim_start()) + .unwrap_or(""); + let word = directive + .split(|c: char| !c.is_ascii_alphanumeric() && c != '_') + .next() + .unwrap_or(""); + match word { + "if" | "ifdef" | "ifndef" => { + let alternative = branches.iter().any(|&branch| branch > 0); + branches.push(0); + alternative + } + "elif" | "elifdef" | "elifndef" | "else" => { + if let Some(branch) = branches.last_mut() { + *branch += 1; + } + true + } + "endif" => { + branches.pop(); + branches.iter().any(|&branch| branch > 0) + } + _ => branches.iter().any(|&branch| branch > 0), + } + }) + .collect() +} + pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result { let (grammar, version) = match language { "cpp" => (tree_sitter_cpp::LANGUAGE, "cpp:0.23.4"), @@ -144,6 +197,11 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result (tree_sitter_dart::LANGUAGE, "dart:0.2.0"), _ => anyhow::bail!("unsupported grammar: {language}"), }; + let alternative_lines = if language == "cpp" { + preprocessor_alternative_lines(source) + } else { + Vec::new() + }; let mut parser = Parser::new(); parser .set_language(&grammar.into()) @@ -304,7 +362,11 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result (name.clone(), String::new(), None, false), }; - if let Some(qualified) = &qualified_name { + let preprocessor_alternative = + language == "cpp" && in_preprocessor_alternative(node, &alternative_lines); + if let Some(qualified) = &qualified_name + && !preprocessor_alternative + { doc.declared.push(qualified.clone()); } let (impl_type, impl_trait) = if language == "rust" { @@ -323,6 +385,7 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result Result)\n#include \n#elif defined(ALT)\nnamespace spdlog { inline void info(const char*) {} }\n#else\nnamespace spdlog { inline void warn(const char*) {} }\n#endif\n#ifndef LOG_H\n#define LOG_H\nnamespace app { inline int level() { return 1; } }\n#endif\n", + ), + ( + "src/main.cpp", + "#include \"log.h\"\nint main() { spdlog::warn(\"x\"); spdlog::info(\"y\"); return app::level(); }\n", + ), + ]); + let found = calls(&model, function(&model, "src/main.cpp", "main")); + let mut by_name: Vec<(String, NodeKind, Option)> = found + .iter() + .map(|(node, reason)| (node.name.clone(), node.kind, reason.clone())) + .collect(); + by_name.sort_by(|a, b| a.0.cmp(&b.0)); + // First-branch (include-guard) code still resolves; the alternatives do not. + assert!( + by_name + .iter() + .any(|(name, kind, _)| name == "level" && *kind == NodeKind::Function) + ); + for (name, kind, _) in &by_name { + if name.ends_with("warn") || name.ends_with("info") { + assert_eq!( + *kind, + NodeKind::External, + "{name} resolved into an #else/#elif branch" + ); + } + } + let stub = model + .nodes + .iter() + .find(|n| n.kind == NodeKind::Function && n.name == "warn") + .expect("alternative branches stay visible as structure"); + assert_eq!( + stub.attributes + .get("source.preprocessor-branch") + .and_then(|v| v.as_str()), + Some("alternative") + ); +} -- 2.52.0 From 46bd3f50f1791c0beece250a63f9830e3403b663 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:00:46 -0600 Subject: [PATCH 04/14] fix(dart,cpp): generic type arguments are not base classes The base walker descended into type-argument lists, so `class MyBloc extends Bloc` emitted inherits edges to Bloc, Ev and St; on pipe, 39 of 145 Dart inherits edges (27%) pointed at type arguments (e.g. `Command` -> `int`). C++ `: public Base` shared the walker through `template_argument_list`. The walker stops at `type_arguments` / `template_argument_list` / `type_parameters`, and handles tree-sitter-dart's shape, where the arguments are sibling `type` fields of `superclass` (only the first names the base) and nested `type` children of a `type`. Test: languages::generic_type_arguments_are_not_base_classes fails before this change (["Bloc", "Ev", "St"]). --- src/source.rs | 33 +++++++++++++++++++++++++++++++-- tests/languages.rs | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/source.rs b/src/source.rs index 95ff1ca..14abce2 100644 --- a/src/source.rs +++ b/src/source.rs @@ -933,7 +933,24 @@ fn bases(node: Node<'_>, source: &str) -> Vec { child.kind(), "base_class_clause" | "class_heritage" | "superclass" | "extends_type_clause" ) { - let mut stack = vec![child]; + let mut stack = Vec::new(); + if child.kind() == "superclass" { + // tree-sitter-dart gives `extends Bloc` as sibling `type` + // fields: the first is the base, later ones are its type arguments. + let mut first_type = true; + let mut c = child.walk(); + for part in child.named_children(&mut c) { + if part.kind() == "type" { + if !first_type { + continue; + } + first_type = false; + } + stack.push(part); + } + } else { + stack.push(child); + } while let Some(base) = stack.pop() { if matches!( base.kind(), @@ -946,8 +963,20 @@ fn bases(node: Node<'_>, source: &str) -> Vec { result.push(base_type(base, source)); continue; } + // Type arguments name parameters of the base, not further bases: + // `extends Bloc` / `: public Base` inherit only Bloc/Base. + if matches!( + base.kind(), + "type_arguments" | "template_argument_list" | "type_parameters" + ) { + continue; + } let mut c = base.walk(); - stack.extend(base.named_children(&mut c)); + let nested_type_arguments = base.kind() == "type"; + stack.extend( + base.named_children(&mut c) + .filter(|part| !(nested_type_arguments && part.kind() == "type")), + ); } } } diff --git a/tests/languages.rs b/tests/languages.rs index 8c174c6..14e6a0e 100644 --- a/tests/languages.rs +++ b/tests/languages.rs @@ -539,3 +539,46 @@ fn cpp_parent_relative_includes_resolve_without_basename_fallback() { .any(|n| n.kind == NodeKind::External && n.name == "../missing/base.h") ); } + +#[test] +fn generic_type_arguments_are_not_base_classes() { + // `extends Bloc` produced inherits edges to Ev and St: the base walker + // descended into the type-argument list. On pipe 39 of 145 Dart inherits edges + // (27%) pointed at type arguments. C++ `: public Base` shared the walker. + let (_, model) = project(&[ + ( + "lib/bloc.dart", + "class Ev {}\nclass St {}\nclass Bloc {}\nclass MyBloc extends Bloc {}\n", + ), + ( + "src/base.cpp", + "template class Base {};\nclass Arg {};\nclass Derived : public Base {};\n", + ), + ]); + let inherits = |source: &Node| { + let mut names: Vec = model + .edges + .iter() + .filter(|e| e.kind == EdgeKind::Inherits && e.source == source.id) + .map(|e| { + model + .nodes + .iter() + .find(|n| n.id == e.target) + .unwrap() + .name + .clone() + }) + .collect(); + names.sort(); + names + }; + assert_eq!( + inherits(symbol(&model, "lib/bloc.dart", "MyBloc")), + ["Bloc"] + ); + assert_eq!( + inherits(symbol(&model, "src/base.cpp", "Derived")), + ["Base"] + ); +} -- 2.52.0 From 4c630ffeb1145bcce68b913ced90b4d060c95be0 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:02:04 -0600 Subject: [PATCH 05/14] fix(check): gate on recomputed metrics, not the stored copy `brick check` read `metrics` from the model JSON and gated on them, so setting `cycle_count` to 0 and `score` to 100 in the file turned a failing `--max-cycles 0` gate into "passed" without changing the graph. check now recomputes the metrics from the loaded nodes and edges and gates on those; if the stored copy differs, that is itself a failure ("stored metrics do not match the graph"). Honest models recompute to identical values, so existing gates are unchanged. Test: diff_cli::check_recomputes_metrics_instead_of_trusting_the_file fails before this change (the tampered model passes). --- src/main.rs | 13 ++++++++++++- tests/diff_cli.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 94b6eac..89b8837 100644 --- a/src/main.rs +++ b/src/main.rs @@ -227,8 +227,19 @@ fn run(cli: Cli) -> Result<()> { bail!("--min-score must be a finite number"); } let building = read_json(&input)?; - let metrics = &building.metrics; + // Gate on metrics recomputed from the graph, never on the stored copy: + // editing `metrics` in the JSON must not be able to pass a gate. + let mut recomputed = building.clone(); + brick::metrics::calculate(&mut recomputed); let mut failures = Vec::new(); + if serde_json::to_value(&recomputed.metrics)? + != serde_json::to_value(&building.metrics)? + { + failures.push( + "stored metrics do not match the graph; gating on recomputed metrics".into(), + ); + } + let metrics = &recomputed.metrics; if metrics.structural_health.score < min_score { failures.push(format!( "score {:.1} is below {:.1}", diff --git a/tests/diff_cli.rs b/tests/diff_cli.rs index e732ebb..7962946 100644 --- a/tests/diff_cli.rs +++ b/tests/diff_cli.rs @@ -259,3 +259,49 @@ fn relocated_artifacts_compare_after_checkouts_are_deleted() { ); } } + +#[test] +fn check_recomputes_metrics_instead_of_trusting_the_file() { + // `check` gated on the metrics stored in the JSON, so editing `cycle_count` and + // `score` turned a failing gate into a pass without changing the graph. + let directory = tempfile::tempdir().unwrap(); + let project = directory.path().join("project"); + fs::create_dir(&project).unwrap(); + fs::write( + project.join("lib.rs"), + "pub fn alpha() { beta(); }\nfn beta() { alpha(); }\n", + ) + .unwrap(); + let model = scan::scan(&project).unwrap(); + assert!(model.metrics.cycle_count > 0); + let honest = directory.path().join("honest.json"); + write_model(&honest, &model); + let gate = |path: &Path| { + Command::new(env!("CARGO_BIN_EXE_brick")) + .args([ + "check", + path.to_str().unwrap(), + "--max-cycles", + "0", + "--min-score", + "0", + ]) + .output() + .unwrap() + }; + assert!(!gate(&honest).status.success()); + + let mut value: Value = serde_json::to_value(&model).unwrap(); + value["metrics"]["cycle_count"] = 0.into(); + value["metrics"]["structural_health"]["score"] = 100.0.into(); + let tampered = directory.path().join("tampered.json"); + fs::write(&tampered, serde_json::to_vec_pretty(&value).unwrap()).unwrap(); + let output = gate(&tampered); + assert!(!output.status.success(), "tampered metrics passed the gate"); + let text = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!(text.contains("stored metrics"), "{text}"); +} -- 2.52.0 From 3cf4d7660a250f1febf12ffe2c64a9ccfd9a6169 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:04:00 -0600 Subject: [PATCH 06/14] fix(build,check): report empty scans instead of scoring them 100 A directory with no supported source (Go/Java only) built "1 nodes / 0 edges; score 100.0" with no diagnostic, and `check --min-score 90 --max-cycles 0` passed it. The one-line summary also hid diagnostics entirely: the yams run's 7,428 warnings (505 files with grammar errors) never appeared. - scan records a warning diagnostic when no source file was scanned; - `build` prints the diagnostic and warning counts in its summary and warns on stderr when the model is empty; - `check` fails a model that contains no scanned source files. Test: diff_cli::a_scan_with_no_supported_source_is_reported_and_fails_check fails before this change (no diagnostic; check passes). --- src/main.rs | 27 +++++++++++++++++++++++++- src/scan.rs | 8 ++++++++ tests/diff_cli.rs | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 89b8837..37efe92 100644 --- a/src/main.rs +++ b/src/main.rs @@ -182,16 +182,28 @@ fn run(cli: Cli) -> Result<()> { .count(); write_json(&building, &json)?; render::render(&building, &html, &cytoscape_url)?; + let warnings = building + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.level == "warning") + .count(); println!( - "built {} nodes / {} edges; score {:.1}; imported {} Fold findings / {} p4c diagnostics\nJSON: {}\nHTML: {}", + "built {} nodes / {} edges; score {:.1}; imported {} Fold findings / {} p4c diagnostics; {} diagnostics ({} warnings)\nJSON: {}\nHTML: {}", building.metrics.node_count, building.metrics.edge_count, building.metrics.structural_health.score, imported_fold, imported_p4c, + building.diagnostics.len(), + warnings, json.display(), html.display() ); + if !has_source_files(&building) { + eprintln!( + "brick: warning: no supported source files were found; the model is empty and its score describes nothing" + ); + } } Command::Mcp { input } => { let building = read_json(&input)?; @@ -240,6 +252,11 @@ fn run(cli: Cli) -> Result<()> { ); } let metrics = &recomputed.metrics; + if !has_source_files(&building) { + failures.push( + "the model contains no scanned source files; nothing was analysed".into(), + ); + } if metrics.structural_health.score < min_score { failures.push(format!( "score {:.1} is below {:.1}", @@ -413,6 +430,14 @@ fn write_json(building: &Building, path: &PathBuf) -> Result<()> { .with_context(|| format!("cannot write graph {}", path.display())) } +/// At least one source file was scanned into the model. +fn has_source_files(building: &Building) -> bool { + building + .nodes + .iter() + .any(|node| node.kind == brick::model::NodeKind::File) +} + fn read_json(path: &PathBuf) -> Result { let raw = fs::read_to_string(path) .with_context(|| format!("cannot read graph {}", path.display()))?; diff --git a/src/scan.rs b/src/scan.rs index fab8ed5..21acb0b 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -100,6 +100,14 @@ pub fn scan_with_options(root: &Path, options: &ScanOptions) -> Result state.walk(&root)?; state.resolve_relationships(); state.annotate_indirect_uses(); + // An analysis of nothing must not read as a healthy repository. + if !state.nodes.iter().any(|node| node.kind == NodeKind::File) { + state.diagnostics.push(Diagnostic { + level: "warning".into(), + message: "no supported source files were found; the model is empty".into(), + path: None, + }); + } let mut building = Building { schema_version: SCHEMA_VERSION, diff --git a/tests/diff_cli.rs b/tests/diff_cli.rs index 7962946..19c09cb 100644 --- a/tests/diff_cli.rs +++ b/tests/diff_cli.rs @@ -305,3 +305,51 @@ fn check_recomputes_metrics_instead_of_trusting_the_file() { ); assert!(text.contains("stored metrics"), "{text}"); } + +#[test] +fn a_scan_with_no_supported_source_is_reported_and_fails_check() { + // A Go/Java-only directory built "1 nodes / 0 edges; score 100.0" with no + // diagnostic, and `check --min-score 90 --max-cycles 0` passed: an analysis of + // nothing looked like a perfectly healthy repository. + let directory = tempfile::tempdir().unwrap(); + let project = directory.path().join("project"); + fs::create_dir(&project).unwrap(); + fs::write(project.join("main.go"), "package main\nfunc main() {}\n").unwrap(); + fs::write(project.join("App.java"), "class App {}\n").unwrap(); + let json = directory.path().join("model.json"); + let html = directory.path().join("model.html"); + let build = Command::new(env!("CARGO_BIN_EXE_brick")) + .args([ + "build", + project.to_str().unwrap(), + "--json", + json.to_str().unwrap(), + "--html", + html.to_str().unwrap(), + ]) + .output() + .unwrap(); + assert!(build.status.success()); + let stderr = String::from_utf8_lossy(&build.stderr); + assert!(stderr.contains("no supported source files"), "{stderr}"); + let model: Value = serde_json::from_slice(&fs::read(&json).unwrap()).unwrap(); + assert!( + model["diagnostics"] + .as_array() + .is_some_and(|d| d.iter().any(|d| d["message"] + .as_str() + .is_some_and(|m| m.contains("no supported source files")))) + ); + let check = Command::new(env!("CARGO_BIN_EXE_brick")) + .args([ + "check", + json.to_str().unwrap(), + "--min-score", + "90", + "--max-cycles", + "0", + ]) + .output() + .unwrap(); + assert!(!check.status.success(), "an empty model passed check"); +} -- 2.52.0 From 4bdae55923513d10d4781e4bde6954bb5dce8704 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:07:34 -0600 Subject: [PATCH 07/14] fix(python,swift): keep calls in their function and resolve only visible names Three lexical-preview bugs, reproduced on one probe: - A blank or comment-only line in a Python function fell through to the brace-depth scope rule, which popped every open scope, so every call after the first blank line in a body was attributed to the file. Blank/comment lines now leave Python scopes unchanged. - `class Child(Base):` matched the call pattern and produced a call edge from the file to Child. The declared class name is no longer a call on its own header line. - Calls resolved to the only function with that name anywhere in the repository, imported or not (a NameError at runtime became an edge). Resolution now prefers the caller's file; Python then considers only files the caller imports (resolved import edges, cached per file); otherwise the call stays external. Swift keeps the module-wide unique name after the same-file check, because files of one Swift module see each other without imports. Imported bases resolve the same way, so `from pkg.b import Base` disambiguates `class Child(Base)`. Test: languages::python_calls_keep_their_function_and_resolve_only_what_is_visible fails before this change (go() had no call edges at all). --- src/scan.rs | 80 +++++++++++++++++++++++++++++++++++++++++----- tests/languages.rs | 66 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 8 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index 21acb0b..7ed12ac 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -246,6 +246,8 @@ struct ScanState { rust: RustIndex, /// Language -> identifiers used outside call/declaration-name positions. references: HashMap>, + /// Lexical-language file -> repository files it imports (resolved edges). + imported_paths: HashMap>>, } impl ScanState { @@ -268,6 +270,7 @@ impl ScanState { cpp: CppIndex::default(), rust: RustIndex::default(), references: HashMap::new(), + imported_paths: HashMap::new(), } } @@ -421,9 +424,14 @@ impl ScanState { let line = strip_line_comment(raw_line, language); let call_line = mask_quoted_strings(line); let indent = raw_line.len() - raw_line.trim_start().len(); - if language == "python" && !line.trim().is_empty() { - while scopes.last().is_some_and(|scope| scope.indent >= indent) { - scopes.pop(); + if language == "python" { + // Blank and comment-only lines say nothing about Python scope. They + // used to fall through to the brace-depth rule below, which popped + // every open scope: calls after a blank line left their function. + if !line.trim().is_empty() { + while scopes.last().is_some_and(|scope| scope.indent >= indent) { + scopes.pop(); + } } } else { while scopes @@ -434,9 +442,11 @@ impl ScanState { } } + let mut class_name = None; if let Some(captures) = patterns.class.captures(line) && let Some(name) = captures.name("name") { + class_name = Some(name.as_str().to_owned()); let owner_id = scopes .last() .map(|scope| scope.id.clone()) @@ -538,7 +548,11 @@ impl ScanState { let target = target_match.as_str(); let prefix = call_line[..target_match.start()].trim_end(); let member_call = prefix.ends_with('.') || prefix.ends_with("->"); - if definition_name.as_deref() != Some(target) && !is_keyword(target) { + // `class Child(Base):` names the class being declared, not a call. + if definition_name.as_deref() != Some(target) + && class_name.as_deref() != Some(target) + && !is_keyword(target) + { self.pending.push(PendingRelationship { source: source.clone(), target_name: clean_target(target), @@ -1256,6 +1270,32 @@ impl ScanState { } } + /// Repository files that `path` imports through already-resolved edges. + /// Imports resolve before calls, so the cached set is complete once built. + fn imported_paths(&mut self, path: &str) -> Rc> { + if let Some(found) = self.imported_paths.get(path) { + return Rc::clone(found); + } + let paths: HashSet = self + .nodes + .iter() + .find(|n| n.kind == NodeKind::File && n.path.as_deref() == Some(path)) + .map(|file| { + self.edges + .iter() + .filter(|e| e.kind == EdgeKind::Imports && e.source == file.id) + .filter_map(|e| self.node(&e.target)) + .filter(|n| n.kind == NodeKind::File) + .filter_map(|n| n.path.clone()) + .collect() + }) + .unwrap_or_default(); + let paths = Rc::new(paths); + self.imported_paths + .insert(path.to_owned(), Rc::clone(&paths)); + paths + } + /// Mark functions that may be used without a resolvable call: named outside /// a call position anywhere in the language, or C++ virtual via a prototype. fn annotate_indirect_uses(&mut self) { @@ -1334,6 +1374,11 @@ impl ScanState { if let Some(id) = self.resolve_qualified_symbol(relationship) { return (id, None, None); } + let imported = if language == "python" { + self.imported_paths(&relationship.path) + } else { + Rc::default() + }; let candidates: Vec<_> = self .symbols_by_name .get(name) @@ -1369,10 +1414,29 @@ impl ScanState { if let Some(id) = self.resolve_imported_symbol(relationship) { return (id, None, None); } - } else if candidates.len() == 1 { - return (candidates[0].id.clone(), None, None); - } else if candidates.len() > 1 { - reason = "ambiguous"; + } else { + // Lexical languages: the caller's own file first, then (Python) + // only files it imports. Swift files in one module see each other + // without imports, so Swift keeps a module-wide unique name. + let same_file: Vec<_> = candidates + .iter() + .filter(|n| n.path.as_deref() == Some(&relationship.path)) + .collect(); + let visible: Vec<_> = if !same_file.is_empty() { + same_file + } else if language == "python" { + candidates + .iter() + .filter(|n| n.path.as_ref().is_some_and(|p| imported.contains(p))) + .collect() + } else { + candidates.iter().collect() + }; + match visible.as_slice() { + [only] => return (only.id.clone(), None, None), + [] => {} + _ => reason = "ambiguous", + } } } if !relationship.member_call diff --git a/tests/languages.rs b/tests/languages.rs index 14e6a0e..20c69cd 100644 --- a/tests/languages.rs +++ b/tests/languages.rs @@ -582,3 +582,69 @@ fn generic_type_arguments_are_not_base_classes() { ["Base"] ); } + +#[test] +fn python_calls_keep_their_function_and_resolve_only_what_is_visible() { + let (_, model) = project(&[ + ( + "pkg/a.py", + "def process(x):\n return x\n\ndef unrelated():\n pass\n", + ), + ( + "pkg/b.py", + "class Base:\n pass\n\ndef helper():\n pass\n", + ), + ( + "pkg/c.py", + // A blank line and a comment inside the body used to pop the function + // scope, attributing the later calls to the file. + "from pkg.b import Base\n\nclass Child(Base):\n def go(self):\n x = 1\n\n # comment\n helper()\n process(1)\n unrelated()\n", + ), + ( + "pkg/d.py", + "from pkg.a import process\n\ndef use():\n return process(2)\n", + ), + ]); + let go = symbol(&model, "pkg/c.py", "go"); + let outgoing = |source: &Node| { + let mut found: Vec<(String, String)> = model + .edges + .iter() + .filter(|e| e.kind == EdgeKind::Calls && e.source == source.id) + .map(|e| { + let target = model.nodes.iter().find(|n| n.id == e.target).unwrap(); + (target.name.clone(), target.path.clone().unwrap_or_default()) + }) + .collect(); + found.sort(); + found + }; + // helper comes from the imported pkg/b.py; process and unrelated are never + // imported by c.py, so they stay external instead of resolving by unique name. + assert_eq!( + outgoing(go), + [ + ("helper".to_owned(), "pkg/b.py".to_owned()), + ("process".to_owned(), String::new()), + ("unrelated".to_owned(), String::new()), + ] + ); + // An imported name still resolves across files. + assert_eq!( + outgoing(symbol(&model, "pkg/d.py", "use")), + [("process".to_owned(), "pkg/a.py".to_owned())] + ); + // A class header is not a call. + let file_c = model + .nodes + .iter() + .find(|n| n.kind == NodeKind::File && n.path.as_deref() == Some("pkg/c.py")) + .unwrap(); + assert!( + !model + .edges + .iter() + .any(|e| e.kind == EdgeKind::Calls && e.source == file_c.id), + "file-level call edges in pkg/c.py" + ); +} -- 2.52.0 From a03c36969eda14e8dd96245e1ef00d7060e918da Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:09:01 -0600 Subject: [PATCH 08/14] feat(dart): resolve calls through imported libraries and own package: URIs A Dart import exposes a library's public top-level names without a per-name binding, so the binding-based lookup never matched and no cross-file Dart call resolved, even when the import itself did (pipe's VerifyCommand::run lost loadProject, analyzeProject, verifyAnalysis and decodeBaselineEntries). After the lexical-owner walk and binding lookup fail, a Dart call now resolves to the one public (not `_`-prefixed) top-level declaration of that name in the libraries the file imports; several such declarations make it `ambiguous`, none leaves it external. `package:/x.dart` imports resolve to `lib/x.dart` when `` is the root pubspec's package; other packages stay external. `show`/`hide` combinators are not modelled (noted as a follow-up). Test: languages::dart_calls_resolve_through_imported_libraries fails before this change (helper stays not-visible); an unimported file and a library-private name stay unresolved. --- src/scan.rs | 46 ++++++++++++++++++++++++++++++++++++--- tests/languages.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index 7ed12ac..d9351d3 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -248,11 +248,23 @@ struct ScanState { references: HashMap>, /// Lexical-language file -> repository files it imports (resolved edges). imported_paths: HashMap>>, + /// `name:` of the root pubspec.yaml, so `package:/x.dart` maps to lib/x.dart. + dart_package: Option, } impl ScanState { fn new(root: PathBuf, root_id: String, options: &ScanOptions) -> Self { + let dart_package = fs::read_to_string(root.join("pubspec.yaml")) + .ok() + .and_then(|pubspec| { + pubspec.lines().find_map(|line| { + line.strip_prefix("name:") + .map(|name| name.trim().trim_matches(['\'', '"']).to_owned()) + .filter(|name| !name.is_empty()) + }) + }); Self { + dart_package, root, root_id, nodes: Vec::new(), @@ -1374,7 +1386,7 @@ impl ScanState { if let Some(id) = self.resolve_qualified_symbol(relationship) { return (id, None, None); } - let imported = if language == "python" { + let imported = if language == "python" || language == "dart" { self.imported_paths(&relationship.path) } else { Rc::default() @@ -1414,6 +1426,26 @@ impl ScanState { if let Some(id) = self.resolve_imported_symbol(relationship) { return (id, None, None); } + // A Dart import exposes the library's public top-level names with no + // per-name binding: resolve through the libraries this file imports. + if language == "dart" && reason != "ambiguous" { + let exposed: Vec<_> = candidates + .iter() + .filter(|n| { + !n.name.starts_with('_') + && n.path.as_ref().is_some_and(|p| imported.contains(p)) + && n.parent + .as_deref() + .and_then(|parent| self.node(parent)) + .is_some_and(|parent| parent.kind == NodeKind::File) + }) + .collect(); + match exposed.as_slice() { + [only] => return (only.id.clone(), None, None), + [] => {} + _ => reason = "ambiguous", + } + } } else { // Lexical languages: the caller's own file first, then (Python) // only files it imports. Swift files in one module see each other @@ -2173,10 +2205,18 @@ impl ScanState { } } "dart" => { - if name.contains(':') { + if let Some(rest) = name.strip_prefix("package:") { + // Only this repository's own package maps into it: `lib/`. + let (package, rest) = rest.split_once('/')?; + if self.dart_package.as_deref() != Some(package) { + return None; + } + paths.push(normalize_import_path(&format!("lib/{rest}"))?); + } else if name.contains(':') { return None; + } else { + paths.push(local()?); } - paths.push(local()?); } "cpp" | "p4" => { if language == "cpp" && !name.contains('/') && Path::new(name).extension().is_none() diff --git a/tests/languages.rs b/tests/languages.rs index 20c69cd..184b0da 100644 --- a/tests/languages.rs +++ b/tests/languages.rs @@ -648,3 +648,57 @@ fn python_calls_keep_their_function_and_resolve_only_what_is_visible() { "file-level call edges in pkg/c.py" ); } + +#[test] +fn dart_calls_resolve_through_imported_libraries() { + // Dart imports expose a library's public top-level names without a binding per + // name, so no cross-file Dart call ever resolved even when its import did. + let (_, model) = project(&[ + ("pubspec.yaml", "name: demo\n"), + ( + "lib/util.dart", + "int helper() => 1;\nint _secret() => 2;\nclass Tool { int use() => 3; }\n", + ), + ("lib/other.dart", "int shared() => 4;\n"), + ("lib/twin.dart", "int shared() => 5;\n"), + ( + "lib/main.dart", + "import 'util.dart';\nimport 'package:demo/other.dart';\nimport 'twin.dart';\nvoid run() {\n helper();\n _secret();\n shared();\n}\n", + ), + // Clean negative: no import, no edge. + ("lib/stranger.dart", "void stray() {\n helper();\n}\n"), + ]); + let targets = |source: &Node| { + let mut found: Vec<(String, String, String)> = model + .edges + .iter() + .filter(|e| e.kind == EdgeKind::Calls && e.source == source.id) + .map(|e| { + let target = model.nodes.iter().find(|n| n.id == e.target).unwrap(); + ( + target.name.clone(), + target.path.clone().unwrap_or_default(), + e.attributes + .get("source.unresolved-reason") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_owned(), + ) + }) + .collect(); + found.sort(); + found + }; + assert_eq!( + targets(symbol(&model, "lib/main.dart", "run")), + [ + ("_secret".into(), String::new(), "not-visible".into()), + ("helper".into(), "lib/util.dart".into(), String::new()), + ("shared".into(), String::new(), "ambiguous".into()), + ] + ); + assert_eq!( + targets(symbol(&model, "lib/stranger.dart", "stray")), + [("helper".into(), String::new(), "not-visible".into())] + ); +} -- 2.52.0 From 088ba3fb1593937025ef4f74e9a0cba6050dcd8d Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:11:14 -0600 Subject: [PATCH 09/14] fix(fold): validate the report envelope and gate the Symbolic lane on Fold The Fold report importer was schema-less and trusted self-description: - `{"hello":1}` imported 0 findings with no diagnostic, like a clean run; - findings without a file location were dropped silently; - any JSON declaring backend "source" and confidence "proved"/"latent" got the Symbolic lane, whoever produced it; - SARIF `file://` URIs (and %-escapes) never matched a scanned file. Now a report must be a SARIF log (`runs`) or a Fold findings report (`findings`), otherwise the import fails with "not a Fold report". The Symbolic lane requires the producer to be Fold (`tool.name`, or every SARIF run's `tool.driver.name`, equal to "fold", which is what Fold writes); other producers' findings are imported in the Imported lane with a diagnostic. An unsupported `output_version` and findings dropped for lack of a location are diagnosed. `file://[localhost]` URIs lose their scheme and are percent-decoded before path matching. Existing tests that model Fold reports now carry Fold's real envelope. Tests (fold::tests) failing before this change: rejects_a_file_that_is_not_a_fold_report, symbolic_lane_requires_a_fold_producer_not_a_self_declared_label, sarif_file_uris_match_and_unlocated_findings_are_diagnosed. --- src/fold.rs | 214 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 201 insertions(+), 13 deletions(-) diff --git a/src/fold.rs b/src/fold.rs index 56613fa..525a922 100644 --- a/src/fold.rs +++ b/src/fold.rs @@ -2,7 +2,7 @@ use crate::metrics; use crate::model::{ Building, Confidence, Edge, EdgeKind, Evidence, EvidenceLane, Node, NodeKind, stable_id, }; -use anyhow::{Context, Result}; +use anyhow::{Context, Result, bail}; use serde_json::Value; use std::collections::BTreeMap; use std::fs; @@ -13,11 +13,67 @@ pub fn import_findings(building: &mut Building, report: &Path) -> Result .with_context(|| format!("cannot read Fold report {}", report.display()))?; let value: Value = serde_json::from_str(&raw) .with_context(|| format!("invalid JSON in Fold report {}", report.display()))?; - let findings = if value.get("runs").is_some() { + let sarif_runs = value.get("runs").and_then(Value::as_array); + let is_native = value.get("findings").and_then(Value::as_array).is_some(); + if sarif_runs.is_none() && !is_native { + // Anything else imported zero findings silently, like a clean run. + bail!( + "{} is not a Fold report: expected a SARIF log (`runs`) or a Fold findings report (`findings`)", + report.display() + ); + } + // Fold's proved/latent tiers mean something only when Fold produced them: + // a report from another tool never earns the Symbolic lane by labelling itself. + let producers: Vec> = match sarif_runs { + Some(runs) => runs + .iter() + .map(|run| run.pointer("/tool/driver/name").and_then(Value::as_str)) + .collect(), + None => vec![value.pointer("/tool/name").and_then(Value::as_str)], + }; + let from_fold = !producers.is_empty() && producers.iter().all(|name| *name == Some("fold")); + if !from_fold { + let named: Vec<&str> = producers + .iter() + .map(|name| name.unwrap_or("unnamed")) + .collect(); + building.diagnostics.push(crate::model::Diagnostic { + level: "warning".into(), + message: format!( + "report {} was not produced by Fold (producer: {}); its findings are imported, never symbolic", + report.display(), + named.join(", ") + ), + path: None, + }); + } + if let Some(version) = value.get("output_version") + && version.as_u64() != Some(1) + { + building.diagnostics.push(crate::model::Diagnostic { + level: "warning".into(), + message: format!( + "Fold report {} has unsupported output_version {version}; read as version 1", + report.display() + ), + path: None, + }); + } + let (findings, unlocated) = if sarif_runs.is_some() { sarif_findings(&value) } else { native_findings(&value) }; + if unlocated > 0 { + building.diagnostics.push(crate::model::Diagnostic { + level: "warning".into(), + message: format!( + "{unlocated} Fold finding(s) in {} had no file location and were not imported", + report.display() + ), + path: None, + }); + } let mut imported = 0; for finding in findings { @@ -83,7 +139,7 @@ pub fn import_findings(building: &mut Building, report: &Path) -> Result .unwrap_or_else(|| format!("{}:{}:{}", finding.rule, relative_path, finding.line)); let node_id = stable_id("finding", &identity); let evidence = Evidence { - lane: fold_lane(&backend, finding.confidence.as_deref()), + lane: fold_lane(&backend, finding.confidence.as_deref(), from_fold), backend: format!("fold:discover:{backend}"), confidence: finding.confidence.clone().map(Confidence::Label), provenance: format!("{}:{}", report.display(), finding.line), @@ -141,12 +197,17 @@ struct Finding { backend: Option, } -fn native_findings(value: &Value) -> Vec { - value +/// Findings plus the number dropped for lack of a file location. +fn native_findings(value: &Value) -> (Vec, usize) { + let items: Vec<&Value> = value .get("findings") .and_then(Value::as_array) .into_iter() .flatten() + .collect(); + let total = items.len(); + let findings: Vec = items + .into_iter() .filter_map(|item| { Some(Finding { stable_id: string(item, ["id", "finding_id"]), @@ -159,11 +220,14 @@ fn native_findings(value: &Value) -> Vec { backend: native_backend(item), }) }) - .collect() + .collect(); + let dropped = total - findings.len(); + (findings, dropped) } -fn sarif_findings(value: &Value) -> Vec { - value +/// Findings plus the number dropped for lack of a file location. +fn sarif_findings(value: &Value) -> (Vec, usize) { + let items: Vec<&Value> = value .get("runs") .and_then(Value::as_array) .into_iter() @@ -174,6 +238,10 @@ fn sarif_findings(value: &Value) -> Vec { .into_iter() .flatten() }) + .collect(); + let total = items.len(); + let findings: Vec = items + .into_iter() .filter_map(|item| { let location = item.pointer("/locations/0/physicalLocation")?; Some(Finding { @@ -194,7 +262,7 @@ fn sarif_findings(value: &Value) -> Vec { .and_then(Value::as_str) .unwrap_or_default() .into(), - path: location.pointer("/artifactLocation/uri")?.as_str()?.into(), + path: sarif_uri_path(location.pointer("/artifactLocation/uri")?.as_str()?), line: location .pointer("/region/startLine") .and_then(Value::as_u64) @@ -209,7 +277,34 @@ fn sarif_findings(value: &Value) -> Vec { .map(str::to_owned), }) }) - .collect() + .collect(); + let dropped = total - findings.len(); + (findings, dropped) +} + +/// SARIF `artifactLocation.uri` as a path: `file://[localhost]/p` loses its scheme and +/// percent-escapes (`%20`) are decoded; relative URIs pass through unchanged. +fn sarif_uri_path(uri: &str) -> String { + let Some(rest) = uri.strip_prefix("file://") else { + return uri.to_owned(); + }; + let rest = rest.strip_prefix("localhost").unwrap_or(rest); + let bytes = rest.as_bytes(); + let mut decoded = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' + && i + 2 < bytes.len() + && let Ok(byte) = u8::from_str_radix(&rest[i + 1..i + 3], 16) + { + decoded.push(byte); + i += 3; + continue; + } + decoded.push(bytes[i]); + i += 1; + } + String::from_utf8(decoded).unwrap_or_else(|_| rest.to_owned()) } fn string(value: &Value, keys: [&str; N]) -> Option { @@ -327,9 +422,10 @@ fn normalize_lexical_path(path: &str) -> String { } } -fn fold_lane(backend: &str, confidence: Option<&str>) -> EvidenceLane { +fn fold_lane(backend: &str, confidence: Option<&str>, from_fold: bool) -> EvidenceLane { const SYMBOLIC_CONFIDENCE: [&str; 3] = ["concrete-witness", "proved", "latent"]; - if backend == "source" + if from_fold + && backend == "source" && confidence.is_some_and(|confidence| SYMBOLIC_CONFIDENCE.contains(&confidence)) { EvidenceLane::Symbolic @@ -362,6 +458,8 @@ mod tests { fs::write(dir.path().join("main.cpp"), "int main() { return 0; }\n").unwrap(); let report = dir.path().join("fold.json"); let value = serde_json::json!({ + "output_version": 1, + "tool": {"name": "fold", "version": "0.1.0"}, "findings": [{ "id": "f1", "kind": "null-deref", @@ -414,7 +512,7 @@ mod tests { })).collect(); fs::write( &report, - serde_json::to_vec(&serde_json::json!({"findings": findings})).unwrap(), + serde_json::to_vec(&serde_json::json!({"tool": {"name": "fold"}, "findings": findings})).unwrap(), ) .unwrap(); let mut model = scan(&root).unwrap(); @@ -439,6 +537,7 @@ mod tests { &report, r#"{ "runs": [{ + "tool": {"driver": {"name": "fold"}}, "results": [ { "ruleId": "fold.valid", @@ -485,6 +584,7 @@ mod tests { let outside = dir.path().parent().unwrap().join("other/main.cpp"); let report = dir.path().join("fold.json"); let value = serde_json::json!({ + "tool": {"name": "fold"}, "findings": [ { "id": "absolute-outside", @@ -634,4 +734,92 @@ mod tests { .any(|evidence| evidence.provenance.contains("second.json")) ); } + + #[test] + fn rejects_a_file_that_is_not_a_fold_report() { + // `{"hello":1}` imported 0 findings with no diagnostic, indistinguishable + // from a clean Fold run. + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("main.rs"), "fn main() {}\n").unwrap(); + let report = dir.path().join("fold.json"); + fs::write(&report, r#"{"hello": 1}"#).unwrap(); + let mut building = scan(dir.path()).unwrap(); + let error = import_findings(&mut building, &report).unwrap_err(); + assert!( + format!("{error:#}").contains("not a Fold report"), + "{error:#}" + ); + } + + #[test] + fn symbolic_lane_requires_a_fold_producer_not_a_self_declared_label() { + // Any JSON claiming backend "source" and confidence "proved" got the Symbolic + // lane. Only a report produced by Fold may carry Fold's proved tier. + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("main.cpp"), "int main() { return 0; }\n").unwrap(); + let report = dir.path().join("other.json"); + let value = serde_json::json!({ + "tool": {"name": "someone-else"}, + "findings": [{ + "id": "x", "kind": "null-deref", + "locations": [{"file": "main.cpp", "line": 1}], + "confidence": "proved", "evidence": {"backend": "source"} + }] + }); + fs::write(&report, serde_json::to_vec(&value).unwrap()).unwrap(); + let mut building = scan(dir.path()).unwrap(); + assert_eq!(import_findings(&mut building, &report).unwrap(), 1); + let flags = building + .edges + .iter() + .find(|edge| edge.kind == EdgeKind::Flags) + .unwrap(); + assert_eq!(flags.evidence[0].lane, crate::model::EvidenceLane::Imported); + assert!( + building + .diagnostics + .iter() + .any(|d| d.message.contains("not produced by Fold")) + ); + } + + #[test] + fn sarif_file_uris_match_and_unlocated_findings_are_diagnosed() { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("my file.cpp"), "int main() { return 0; }\n").unwrap(); + let mut building = scan(dir.path()).unwrap(); + let uri = format!( + "file://{}", + dir.path() + .join("my file.cpp") + .to_str() + .unwrap() + .replace(' ', "%20") + ); + let report = dir.path().join("fold.sarif"); + let value = serde_json::json!({ + "version": "2.1.0", + "runs": [{ + "tool": {"driver": {"name": "fold"}}, + "results": [ + {"ruleId": "div-by-zero", "message": {"text": "x"}, + "locations": [{"physicalLocation": {"artifactLocation": {"uri": uri}, + "region": {"startLine": 1}}}], + "properties": {"confidence": "proved", "backend": "source"}}, + {"ruleId": "no-location", "message": {"text": "y"}, + "properties": {"confidence": "heuristic", "backend": "source"}} + ] + }] + }); + fs::write(&report, serde_json::to_vec(&value).unwrap()).unwrap(); + assert_eq!(import_findings(&mut building, &report).unwrap(), 1); + assert!( + building + .diagnostics + .iter() + .any(|d| d.message.contains("no file location")), + "{:?}", + building.diagnostics + ); + } } -- 2.52.0 From 4b2c87344501da73605ab70607b25e111876d074 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:12:13 -0600 Subject: [PATCH 10/14] fix(fold): follow the filesystem for `..` after a symlinked directory Fold report paths were normalized lexically, so `linkout/../src/main.rs` with `linkout` pointing outside the root attached to the repository's `src/main.rs` although the OS path names a different file. A relative path containing `..` that exists in this checkout is now canonicalized (and must stay under the canonical root); paths absent from the checkout, such as reports from another machine, keep the documented lexical matching. Test: fold::tests::parent_steps_after_a_symlinked_directory_follow_the_filesystem fails before this change (the finding attaches to src/main.rs). --- src/fold.rs | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/src/fold.rs b/src/fold.rs index 525a922..77390a9 100644 --- a/src/fold.rs +++ b/src/fold.rs @@ -372,7 +372,24 @@ fn find_file(building: &Building, path: &str) -> Option { { return None; } - normalized + // When the path exists here, follow the filesystem: `link/../x` names what + // the OS resolves through `link`, not the lexical `x`. Paths absent from this + // checkout (a report from another machine) keep lexical matching. + let on_disk = Path::new(&building.root).join(path.replace('\\', "/")); + if path.contains("..") + && let (Ok(canonical), Ok(root)) = ( + on_disk.canonicalize(), + Path::new(&building.root).canonicalize(), + ) + { + canonical + .strip_prefix(&root) + .ok()? + .to_str()? + .replace('\\', "/") + } else { + normalized + } }; building .nodes @@ -512,7 +529,10 @@ mod tests { })).collect(); fs::write( &report, - serde_json::to_vec(&serde_json::json!({"tool": {"name": "fold"}, "findings": findings})).unwrap(), + serde_json::to_vec( + &serde_json::json!({"tool": {"name": "fold"}, "findings": findings}), + ) + .unwrap(), ) .unwrap(); let mut model = scan(&root).unwrap(); @@ -822,4 +842,43 @@ mod tests { building.diagnostics ); } + + #[cfg(unix)] + #[test] + fn parent_steps_after_a_symlinked_directory_follow_the_filesystem() { + // `linkout/../src/main.rs` with `linkout` pointing outside the root was + // normalized lexically to `src/main.rs`, a file the OS path does not name. + use std::os::unix::fs::symlink; + let workspace = tempfile::tempdir().unwrap(); + let root = workspace.path().join("repo"); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/main.rs"), "fn main() {}\n").unwrap(); + let outside = workspace.path().join("elsewhere/deeper"); + fs::create_dir_all(&outside).unwrap(); + fs::create_dir_all(workspace.path().join("elsewhere/src")).unwrap(); + fs::write( + workspace.path().join("elsewhere/src/main.rs"), + "fn other() {}\n", + ) + .unwrap(); + symlink(&outside, root.join("linkout")).unwrap(); + let report = workspace.path().join("fold.json"); + let value = serde_json::json!({ + "tool": {"name": "fold"}, + "findings": [{ + "id": "through-link", "kind": "test", + "locations": [{"file": "linkout/../src/main.rs", "line": 1}], + "confidence": "heuristic", "evidence": {"backend": "source"} + }] + }); + fs::write(&report, serde_json::to_vec(&value).unwrap()).unwrap(); + let mut model = scan(&root).unwrap(); + assert_eq!(import_findings(&mut model, &report).unwrap(), 0); + assert!( + model + .diagnostics + .iter() + .any(|d| d.message.contains("did not match a scanned file")) + ); + } } -- 2.52.0 From 9f9fc560b70d068502451baac776c6f1fc1bdeb0 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:13:01 -0600 Subject: [PATCH 11/14] fix(scan): decode non-UTF-8 source lossily instead of dropping the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single invalid byte, such as a Latin-1 `é` in a comment, made `read_to_string` fail and the whole file and its symbols were skipped. Files are now read as bytes: a NUL byte still marks a binary file, which is skipped with a diagnostic; otherwise invalid UTF-8 is decoded lossily (U+FFFD) and a warning diagnostic records it, so the file keeps its symbols and calls. Test: languages::a_non_utf8_byte_does_not_drop_the_whole_file fails before this change (latin1.rs has no symbols). --- src/scan.rs | 26 ++++++++++++++++++++++++-- tests/languages.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index d9351d3..c32e56a 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -368,17 +368,39 @@ impl ScanState { return; } }; - let content = match fs::read_to_string(path) { + let bytes = match fs::read(path) { Ok(value) => value, Err(error) => { self.diagnostics.push(Diagnostic { level: "warning".into(), - message: format!("skipped non-UTF-8 or unreadable source: {error}"), + message: format!("skipped unreadable source: {error}"), path: Some(relative), }); return; } }; + // NUL bytes mean a binary file, not source. Any other invalid UTF-8 (a + // Latin-1 byte in a comment) is decoded lossily: dropping the whole file hid + // every symbol in it. + if bytes.contains(&0) { + self.diagnostics.push(Diagnostic { + level: "warning".into(), + message: "skipped binary file with a source extension".into(), + path: Some(relative), + }); + return; + } + let content = match String::from_utf8(bytes) { + Ok(value) => value, + Err(error) => { + self.diagnostics.push(Diagnostic { + level: "warning".into(), + message: "decoded non-UTF-8 bytes lossily (U+FFFD)".into(), + path: Some(relative.clone()), + }); + String::from_utf8_lossy(error.as_bytes()).into_owned() + } + }; let file_id = stable_id("file", &relative); let parent = path .parent() diff --git a/tests/languages.rs b/tests/languages.rs index 184b0da..c26f721 100644 --- a/tests/languages.rs +++ b/tests/languages.rs @@ -702,3 +702,30 @@ fn dart_calls_resolve_through_imported_libraries() { [("helper".into(), String::new(), "not-visible".into())] ); } + +#[test] +fn a_non_utf8_byte_does_not_drop_the_whole_file() { + // One Latin-1 `é` in a comment made the whole file (and its functions) vanish. + let dir = tempfile::tempdir().unwrap(); + let mut source = b"// caf".to_vec(); + source.push(0xE9); + source.extend_from_slice(b"\nfn kept() {}\nfn caller() { kept(); }\n"); + fs::write(dir.path().join("latin1.rs"), source).unwrap(); + fs::write(dir.path().join("blob.rs"), [0u8, 159, 146, 150]).unwrap(); + let model = scan::scan(dir.path()).unwrap(); + let caller = symbol(&model, "latin1.rs", "caller"); + assert!(calls(&model, caller, symbol(&model, "latin1.rs", "kept"))); + assert!( + model + .diagnostics + .iter() + .any(|d| d.path.as_deref() == Some("latin1.rs") && d.message.contains("non-UTF-8")) + ); + // A file with NUL bytes is binary, not source: still skipped with a diagnostic. + assert!( + !model + .nodes + .iter() + .any(|n| n.path.as_deref() == Some("blob.rs")) + ); +} -- 2.52.0 From e03fa40ccc5a5224ee110b601e34b975d5d4b611 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:14:21 -0600 Subject: [PATCH 12/14] fix(c): parse .c files with the C grammar `.c` files were parsed with tree-sitter-cpp, so valid C that uses C++ keywords as identifiers (`int delete = 0; int new = 1;`, a parameter named `class`) produced grammar-error diagnostics and lost symbols. `.c` files now parse with tree-sitter-c (pinned =0.24.2, backend `c:0.24.2`) while staying on the shared C/C++ extraction path, so C sources and the headers they include still share one include/declaration index. Headers (`.h`) keep the C++ grammar, which parses the C subset. Test: cpp_xtu::c_sources_parse_with_the_c_grammar fails before this change (grammar errors in util.c). --- Cargo.lock | 11 +++++++++++ Cargo.toml | 1 + src/source.rs | 3 +++ tests/cpp_xtu.rs | 27 +++++++++++++++++++++++++++ 4 files changed, 42 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index d936bc3..3596402 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -84,6 +84,7 @@ dependencies = [ "serde_json", "tempfile", "tree-sitter", + "tree-sitter-c", "tree-sitter-cpp", "tree-sitter-dart", "tree-sitter-javascript", @@ -423,6 +424,16 @@ dependencies = [ "tree-sitter-language", ] +[[package]] +name = "tree-sitter-c" +version = "0.24.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9b2eb57a55fed6b00812912e730b7a275cf4fe98bfd6a5d76263d4438371728" +dependencies = [ + "cc", + "tree-sitter-language", +] + [[package]] name = "tree-sitter-cpp" version = "0.23.4" diff --git a/Cargo.toml b/Cargo.toml index d492c7d..b2a1318 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ regex = "1.12" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tree-sitter = "=0.26.13" +tree-sitter-c = "=0.24.2" tree-sitter-cpp = "=0.23.4" tree-sitter-javascript = "=0.25.0" tree-sitter-typescript = "=0.23.2" diff --git a/src/source.rs b/src/source.rs index 14abce2..c67fd41 100644 --- a/src/source.rs +++ b/src/source.rs @@ -184,6 +184,9 @@ fn preprocessor_alternative_lines(source: &str) -> Vec { pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result { let (grammar, version) = match language { + // `.c` shares the C/C++ extraction path (and header index) but must parse + // as C: identifiers such as `new`, `delete` or `class` are valid C. + "cpp" if path.ends_with(".c") => (tree_sitter_c::LANGUAGE, "c:0.24.2"), "cpp" => (tree_sitter_cpp::LANGUAGE, "cpp:0.23.4"), "rust" => (tree_sitter_rust::LANGUAGE, "rust:0.24.2"), "javascript" => (tree_sitter_javascript::LANGUAGE, "javascript:0.25.0"), diff --git a/tests/cpp_xtu.rs b/tests/cpp_xtu.rs index 8ad4730..1ecdb0b 100644 --- a/tests/cpp_xtu.rs +++ b/tests/cpp_xtu.rs @@ -406,3 +406,30 @@ fn preprocessor_alternative_branches_are_not_call_targets() { Some("alternative") ); } + +#[test] +fn c_sources_parse_with_the_c_grammar() { + // `.c` files went through the C++ grammar, so valid C using C++ keywords as + // identifiers produced grammar-error diagnostics and damaged extraction. + let model = scan_files(&[ + ("src/util.h", "int twice(int new);\n"), + ( + "src/util.c", + "#include \"util.h\"\nint twice(int new) { int delete = new; return delete * 2; }\n", + ), + ( + "src/main.c", + "#include \"util.h\"\nint main(void) { int class = 1; return twice(class); }\n", + ), + ]); + let grammar_errors: Vec<_> = model + .diagnostics + .iter() + .filter(|d| d.path.as_deref().is_some_and(|p| p.ends_with(".c"))) + .map(|d| d.message.clone()) + .collect(); + assert!(grammar_errors.is_empty(), "{grammar_errors:?}"); + let (target, reason) = single_call(&model, function(&model, "src/main.c", "main")); + assert_eq!(target.path.as_deref(), Some("src/util.c")); + assert_eq!(reason, None); +} -- 2.52.0 From f9804976bcfc8043788cc97f0b4095648cbcb454 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:19:13 -0600 Subject: [PATCH 13/14] perf(source): record parents during traversal instead of re-walking from the root tree-sitter's `Node::parent` re-descends from the root, so each call is O(depth), and `enclosing_function`, `lexical_scope` and `parameter_shadows` walked ancestors through it for every call site and declaration: extraction time grew quadratically with nesting depth (sample profile: >90% of time in ts_node_parent under those three). The main traversal now records each node's parent, and the three ancestor walks look it up (falling back to `Node::parent` for nodes the traversal skips, so results are unchanged). Measured on one C++ file with a call nested N blocks deep (release, macOS arm64): N=2.5k/5k/10k/20k took 0.26/1.05/4.24/16.8 s before; after, 20k takes 0.02 s with the same nodes and call edge. The full suite (111 tests) passes unchanged. --- src/source.rs | 77 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 57 insertions(+), 20 deletions(-) diff --git a/src/source.rs b/src/source.rs index c67fd41..31726aa 100644 --- a/src/source.rs +++ b/src/source.rs @@ -242,6 +242,9 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result> = HashMap::new(); let mut errors: Vec<(usize, usize)> = Vec::new(); let mut stack = vec![tree.root_node()]; while let Some(node) = stack.pop() { @@ -312,6 +315,9 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result = node.named_children(&mut cursor).collect(); + for child in &children { + parents.insert(child.id(), node); + } stack.extend(children.into_iter().rev()); } // Anonymous MISSING tokens (`#endif`, `;`, `}`) are not named children; find @@ -395,7 +401,7 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result Result = doc .declarations .iter() @@ -431,7 +437,13 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result Result Result Result Result, source: &str) -> Option { Some(text(name, source).to_owned()) } -fn lexical_scope(node: Node<'_>, source: &str) -> String { - let mut parent = node.parent(); +/// The parent recorded during traversal, else tree-sitter's (O(depth)) lookup. +fn parent_of<'tree>( + node: Node<'tree>, + parents: &HashMap>, +) -> Option> { + parents.get(&node.id()).copied().or_else(|| node.parent()) +} + +fn lexical_scope<'tree>( + node: Node<'tree>, + source: &str, + parents: &HashMap>, +) -> String { + let mut parent = parent_of(node, parents); let mut names = Vec::new(); while let Some(n) = parent { if matches!( @@ -846,25 +871,26 @@ fn lexical_scope(node: Node<'_>, source: &str) -> String { .unwrap_or(""), ); } - parent = n.parent(); + parent = parent_of(n, parents); } names.reverse(); names.join("::") } -fn enclosing_function( - node: Node<'_>, +fn enclosing_function<'tree>( + node: Node<'tree>, ids: &HashMap, declarations: &[Declaration], + parents: &HashMap>, ) -> Option { - let mut parent = node.parent(); + let mut parent = parent_of(node, parents); while let Some(n) = parent { if let Some(&index) = ids.get(&n.id()) && declarations[index].kind == NodeKind::Function { return Some(index); } - parent = n.parent(); + parent = parent_of(n, parents); } None } @@ -1023,8 +1049,13 @@ fn exported_names(node: Node<'_>, name: &str) -> Vec { Vec::new() } -fn parameter_shadows(node: Node<'_>, name: &str, source: &str) -> bool { - let mut parent = node.parent(); +fn parameter_shadows<'tree>( + node: Node<'tree>, + name: &str, + source: &str, + parents: &HashMap>, +) -> bool { + let mut parent = parent_of(node, parents); while let Some(n) = parent { let signature = n .child_by_field_name("signature") @@ -1058,7 +1089,7 @@ fn parameter_shadows(node: Node<'_>, name: &str, source: &str) -> bool { ); } } - parent = n.parent(); + parent = parent_of(n, parents); } false } @@ -1090,7 +1121,7 @@ fn import_bindings(node: Node<'_>, module: &str, source: &str) -> Vec, source: &str) -> Vec<(String, Option, source: &str) -> Vec<(String, String, CppType)> { struct MacroContext<'a, 'tree> { position: Node<'tree>, + parents: &'a HashMap>, owner: Option, scope: &'a str, self_type: &'a Option, @@ -1943,7 +1975,12 @@ fn rust_macro_calls( }); relationships.push(Relationship { shadowed: !member - && parameter_shadows(context.position, &target, context.source), + && parameter_shadows( + context.position, + &target, + context.source, + context.parents, + ), owner: context.owner, target, member, -- 2.52.0 From 46698cd87da44587caec9d8335e62c7a177881c2 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:21:29 -0600 Subject: [PATCH 14/14] docs: state resolution rules and limits as the code now implements them The audit found README claims the code did not back: resolution was said to respect lexical namespace everywhere (Python/Swift matched a repo-wide unique name, Rust types their leaf name), "ambiguous targets stay external" (not for rival using-directives or #else stubs), Dart imports were said to resolve (no Dart symbol ever did across files), and "C/C++" hid that C went through the C++ grammar. The README now describes the fixed behaviour and its remaining limits: per-language visibility rules (Python imports, Swift module-wide names, Dart imported libraries without show/hide), Rust type identity by declaring file, preprocessor alternatives excluded from resolution, using-directive ambiguity, the C grammar for .c, Fold report validation and producer-gated symbolic lane, lossy UTF-8 decoding, diagnostic counts and empty-scan reporting, check recomputing metrics, low call-site coverage on large C++ code, no virtual-override fan-out, and unrecognized Catch2 TEST_CASE bodies. The research note lists the pinned C grammar. --- README.md | 53 ++++++++++++++++++++------- docs/research/language-correctness.md | 3 +- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index c4cd919..ee9367b 100644 --- a/README.md +++ b/README.md @@ -6,8 +6,8 @@ elements, and dependency edges show calls, imports, and inheritance. The same model is emitted as versioned JSON for other tools, including a future Fold integration. -The default extractor uses pinned Tree-sitter grammars for Rust, Dart, C/C++, -JavaScript/JSX, and TypeScript/TSX. Python and Swift retain the lexical preview; +The default extractor uses pinned Tree-sitter grammars for Rust, Dart, C +(`.c`), C++ (including headers), JavaScript/JSX, and TypeScript/TSX. Python and Swift retain the lexical preview; P4_16 retains its separate lexical and optional pilot paths. Declarations are Syntactic evidence. Calls, imports, and inheritance retain Heuristic target resolution with backend, confidence, and source location, independently of @@ -32,8 +32,16 @@ Layer Fold findings onto the building: --json fold.brick.json \ --html fold.brick.html +A report must be a SARIF log or a Fold findings report; anything else is an +error. Findings keep Fold's symbolic lane only when the report names Fold as its +producer (`tool.name` / `tool.driver.name`); other producers' findings are +imported, never symbolic. Findings without a file location are diagnosed, and +SARIF `file://` URIs are decoded before matching. + Fold artifact paths are normalized lexically across `/`, `\\`, `.`, and `..` -components before exact repository-relative matching. Absolute paths are +components before exact repository-relative matching; a path with `..` that +exists in this checkout follows the filesystem instead, so `link/..` resolves +through the symlink. Absolute paths are accepted only beneath the model's scan root (including existing filesystem aliases whose canonical targets are beneath that root); leading traversal and unrelated suffix paths remain diagnostics. Findings for excluded, generated, or genuinely @@ -152,26 +160,43 @@ Brick extracts syntax, without compiler type checking or build configuration. Macro expansion, overload resolution, dynamic dispatch, generated code, and full package/build-system resolution remain outside the default frontend. Malformed syntax produces diagnostics; recoverable sibling facts remain visible. +A file with invalid UTF-8 is decoded lossily with a diagnostic; files with NUL +bytes are skipped as binary. `build` prints diagnostic counts, and a scan that +finds no supported source says so. `check` recomputes metrics from the graph +(stored metrics are never trusted) and fails a model with no scanned source. Its score is not proof of correctness, maintainability, or security. Use the component metrics and evidence records; do not rank teams or repositories by the composite alone. -Resolution respects language, file, declaration ownership, and lexical namespace. -Relative JS/TS and Dart imports resolve against the importing directory; missing -paths never fall back to unrelated basenames. Explicit JS/TS named/default import -bindings, module-scoped Rust grouped/aliased imports, and module-qualified Rust -public calls can resolve across files. +Grammar-based resolution respects language, file, declaration ownership, and +lexical namespace. Relative JS/TS and Dart imports resolve against the importing +directory (Dart `package:/` against `lib/`); missing paths never +fall back to unrelated basenames. Explicit JS/TS named/default import bindings, +module-scoped Rust grouped/aliased imports, module-qualified Rust public calls, +and public top-level Dart names from imported libraries can resolve across files +(Dart `show`/`hide` combinators are not modelled). The lexical preview resolves +Python names in the calling file, then in files it imports; Swift names in the +calling file, then by a module-wide unique name (Swift files of one module see +each other without imports). C++ calls resolve through the include closure: a header prototype pairs with its definition in another translation unit, lookup walks class (including in-repository bases) and namespace scopes outward, and `static`/anonymous -definitions stay file-local. C++ and Rust member calls resolve only through an -explicitly declared receiver type (parameter, local, field, `this`/`self`, -smart pointer, or `Self`-returning constructor) and carry -`source.resolution`. Calls inside Rust expression macros (`assert!`, `format!`, +definitions stay file-local. Declarations in `#elif`/`#else` branches stay in +the graph (`source.preprocessor-branch: alternative`) but are never call +targets, because Brick does not evaluate conditions; several `using` +directives that nominate different declarations make a call ambiguous. C++ and +Rust member calls resolve only through an explicitly declared receiver type +(parameter, local, field, `this`/`self`, smart pointer, or `Self`-returning +constructor) and carry `source.resolution`. Rust types are identified by their +declaring file, so same-named types in different modules resolve through the +caller's `use` binding or stay ambiguous. Calls inside Rust expression macros (`assert!`, `format!`, `vec!`, ...) are recovered by re-parsing and carry `source.extraction`. Every unresolved call edge records `source.unresolved-reason`, and -`metrics.resolution` reports per-language call-site coverage. Ambiguous -targets stay external. Bare package specifiers, tsconfig aliases, Rust +`metrics.resolution` reports per-language call-site coverage; read the graph +with that coverage in mind (on a large C++ repository only a minority of call +sites resolve). Ambiguous targets stay external. Virtual calls resolve to the +declared type's method only (overrides are not fanned out), and Catch2 +`TEST_CASE` bodies are not yet recognized as functions. Bare package specifiers, tsconfig aliases, Rust wildcard/block-local binding expansion, Cargo workspace module paths, `compile_commands.json` include paths, and compiler-grade dispatch need further adapters. File fan-out remains an observation rather diff --git a/docs/research/language-correctness.md b/docs/research/language-correctness.md index dc1dbaf..b2b343a 100644 --- a/docs/research/language-correctness.md +++ b/docs/research/language-correctness.md @@ -42,7 +42,8 @@ change cycle, isolation, and fan metrics; no health weights are changed. | Rust const/impl | Const function, distinct type-owned methods, stable IDs after unrelated insertions; unresolved impl cannot create cyclic ownership | | Fold absolute alias | `src/fold.rs` tests existing root aliases while rejecting outside and excluded files; original default-tempdir test passes | -The grammar versions are pinned in Cargo.toml and Cargo.lock: C++ 0.23.4, +The grammar versions are pinned in Cargo.toml and Cargo.lock: C 0.24.2 (`.c` +files), C++ 0.23.4, JavaScript 0.25.0, TypeScript/TSX 0.23.2, Dart 0.2.0, Rust 0.24.2, with the Tree-sitter Rust runtime at 0.26.13. Upstream API/grammar references: [C++](https://docs.rs/tree-sitter-cpp/0.23.4/tree_sitter_cpp/), -- 2.52.0