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/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/), diff --git a/src/fold.rs b/src/fold.rs index 56613fa..77390a9 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 { @@ -277,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 @@ -327,9 +439,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 +475,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 +529,10 @@ 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 +557,7 @@ mod tests { &report, r#"{ "runs": [{ + "tool": {"driver": {"name": "fold"}}, "results": [ { "ruleId": "fold.valid", @@ -485,6 +604,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 +754,131 @@ 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 + ); + } + + #[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")) + ); + } } diff --git a/src/main.rs b/src/main.rs index 94b6eac..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)?; @@ -227,8 +239,24 @@ 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 !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}", @@ -402,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 244eca9..c32e56a 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, @@ -140,16 +148,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. @@ -233,11 +246,25 @@ 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>>, + /// `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(), @@ -255,6 +282,7 @@ impl ScanState { cpp: CppIndex::default(), rust: RustIndex::default(), references: HashMap::new(), + imported_paths: HashMap::new(), } } @@ -340,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() @@ -408,9 +458,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 @@ -421,9 +476,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()) @@ -525,7 +582,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), @@ -611,6 +672,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)); } @@ -657,6 +724,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 +751,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()); } @@ -699,6 +774,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 { @@ -1226,6 +1304,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) { @@ -1304,6 +1408,11 @@ impl ScanState { if let Some(id) = self.resolve_qualified_symbol(relationship) { return (id, None, None); } + let imported = if language == "python" || language == "dart" { + self.imported_paths(&relationship.path) + } else { + Rc::default() + }; let candidates: Vec<_> = self .symbols_by_name .get(name) @@ -1339,10 +1448,49 @@ 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"; + // 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 + // 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 @@ -1467,11 +1615,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") } @@ -1588,7 +1749,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), @@ -1598,21 +1779,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. @@ -1690,7 +1874,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 +1883,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 +1930,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 +2018,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); @@ -1986,10 +2227,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/src/source.rs b/src/source.rs index 8228191..31726aa 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,8 +132,61 @@ 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 { + // `.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"), @@ -144,6 +200,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()) @@ -181,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() { @@ -251,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 @@ -304,7 +371,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,13 +394,14 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result Result = doc .declarations .iter() @@ -365,7 +437,13 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result 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!( @@ -779,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 } @@ -869,7 +962,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(), @@ -882,8 +992,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")), + ); } } } @@ -927,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") @@ -962,7 +1089,7 @@ fn parameter_shadows(node: Node<'_>, name: &str, source: &str) -> bool { ); } } - parent = n.parent(); + parent = parent_of(n, parents); } false } @@ -994,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, @@ -1847,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, diff --git a/tests/cpp_xtu.rs b/tests/cpp_xtu.rs index f2e2ebe..1ecdb0b 100644 --- a/tests/cpp_xtu.rs +++ b/tests/cpp_xtu.rs @@ -326,3 +326,110 @@ 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); +} + +#[test] +fn preprocessor_alternative_branches_are_not_call_targets() { + // An `#else` fallback (yams' spdlog stubs in InternalEventBus.h) was indexed as + // live, so every spdlog::warn call site in the repository resolved to the stub and + // the stubs became false hubs. Brick cannot know which branch is active, so only + // the first branch may be a resolution target. + let model = scan_files(&[ + ( + "include/log.h", + "#if __has_include()\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") + ); +} + +#[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); +} diff --git a/tests/diff_cli.rs b/tests/diff_cli.rs index e732ebb..19c09cb 100644 --- a/tests/diff_cli.rs +++ b/tests/diff_cli.rs @@ -259,3 +259,97 @@ 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}"); +} + +#[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"); +} diff --git a/tests/languages.rs b/tests/languages.rs index 8c174c6..c26f721 100644 --- a/tests/languages.rs +++ b/tests/languages.rs @@ -539,3 +539,193 @@ 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"] + ); +} + +#[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" + ); +} + +#[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())] + ); +} + +#[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")) + ); +} 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())]); +}