From 3a0f2adf59aa2e82b5b1437ece8a7a17b1aa68b6 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:54:36 -0600 Subject: [PATCH 1/8] fix(fold): decode SARIF escapes safely, lane per run, follow absolute symlinked paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three import fixes from review of #5: - sarif_uri_path sliced `&rest[i + 1..i + 3]` by byte index, so a `%` followed by a multibyte character (`file:///src/%aé.c`) panicked the whole build, and from_str_radix accepted `%+1` as byte 0x01. Escapes are now decoded on bytes and only from two hex digits. - Whether findings keep Fold's Symbolic lane was decided for the whole report with all(): a merged SARIF log with one Fold run and one CodeQL run demoted every Fold finding to Imported. The lane is now decided per SARIF run (per report for native findings), and the diagnostic names only the non-Fold producers. - The rule that `..` after a symlinked directory follows the filesystem applied only to relative paths; `/root/linkout/../src/main.rs` and its file:// form still normalized lexically onto an in-repository file. Absolute paths that exist now resolve through the filesystem too. Tests (each failed before this change): percent-escape decoding without panics, per-run lanes in a merged SARIF, and absolute/file:// paths through a symlinked directory. --- src/fold.rs | 150 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 139 insertions(+), 11 deletions(-) diff --git a/src/fold.rs b/src/fold.rs index 77390a9..da2fd9e 100644 --- a/src/fold.rs +++ b/src/fold.rs @@ -31,16 +31,18 @@ pub fn import_findings(building: &mut Building, report: &Path) -> Result .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 { + // Decided per run, not per report: a merged SARIF log with one Fold run and one + // other tool's run must keep the Fold run's Symbolic lane. + if producers.is_empty() || producers.iter().any(|name| *name != Some("fold")) { let named: Vec<&str> = producers .iter() + .filter(|name| **name != Some("fold")) .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 {} has findings not produced by Fold (producer: {}); those are imported, never symbolic", report.display(), named.join(", ") ), @@ -62,7 +64,12 @@ pub fn import_findings(building: &mut Building, report: &Path) -> Result let (findings, unlocated) = if sarif_runs.is_some() { sarif_findings(&value) } else { - native_findings(&value) + let from_fold = value.pointer("/tool/name").and_then(Value::as_str) == Some("fold"); + let (mut findings, unlocated) = native_findings(&value); + for finding in &mut findings { + finding.from_fold = from_fold; + } + (findings, unlocated) }; if unlocated > 0 { building.diagnostics.push(crate::model::Diagnostic { @@ -139,7 +146,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(), from_fold), + lane: fold_lane(&backend, finding.confidence.as_deref(), finding.from_fold), backend: format!("fold:discover:{backend}"), confidence: finding.confidence.clone().map(Confidence::Label), provenance: format!("{}:{}", report.display(), finding.line), @@ -195,6 +202,8 @@ struct Finding { line: usize, confidence: Option, backend: Option, + /// Produced by Fold (native report tool, or this SARIF run's driver). + from_fold: bool, } /// Findings plus the number dropped for lack of a file location. @@ -218,6 +227,7 @@ fn native_findings(value: &Value) -> (Vec, usize) { line: native_line(item), confidence: string(item, ["confidence"]), backend: native_backend(item), + from_fold: false, }) }) .collect(); @@ -227,22 +237,25 @@ fn native_findings(value: &Value) -> (Vec, usize) { /// Findings plus the number dropped for lack of a file location. fn sarif_findings(value: &Value) -> (Vec, usize) { - let items: Vec<&Value> = value + let items: Vec<(&Value, bool)> = value .get("runs") .and_then(Value::as_array) .into_iter() .flatten() .flat_map(|run| { + let from_fold = + run.pointer("/tool/driver/name").and_then(Value::as_str) == Some("fold"); run.get("results") .and_then(Value::as_array) .into_iter() .flatten() + .map(move |item| (item, from_fold)) }) .collect(); let total = items.len(); let findings: Vec = items .into_iter() - .filter_map(|item| { + .filter_map(|(item, from_fold)| { let location = item.pointer("/locations/0/physicalLocation")?; Some(Finding { stable_id: item @@ -275,6 +288,7 @@ fn sarif_findings(value: &Value) -> (Vec, usize) { .pointer("/properties/backend") .and_then(Value::as_str) .map(str::to_owned), + from_fold, }) }) .collect(); @@ -293,11 +307,16 @@ fn sarif_uri_path(uri: &str) -> String { let mut decoded = Vec::with_capacity(bytes.len()); let mut i = 0; while i < bytes.len() { + // Decode on bytes, and only two hex digits: slicing the &str at `i + 1..i + 3` + // panicked when a multibyte character followed `%`, and from_str_radix + // accepted a leading `+`. if bytes[i] == b'%' - && i + 2 < bytes.len() - && let Ok(byte) = u8::from_str_radix(&rest[i + 1..i + 3], 16) + && let (Some(high), Some(low)) = ( + bytes.get(i + 1).and_then(|b| (*b as char).to_digit(16)), + bytes.get(i + 2).and_then(|b| (*b as char).to_digit(16)), + ) { - decoded.push(byte); + decoded.push((high * 16 + low) as u8); i += 3; continue; } @@ -348,7 +367,20 @@ fn native_backend(value: &Value) -> Option { fn find_file(building: &Building, path: &str) -> Option { let normalized = normalize_lexical_path(path); - let relative = if is_portable_absolute(path) { + let relative = if is_portable_absolute(path) + && path.contains("..") + && let (Ok(canonical), Ok(root)) = ( + Path::new(path).canonicalize(), + Path::new(&building.root).canonicalize(), + ) { + // As for relative paths: a `..` in a path that exists here follows the + // filesystem, so `/root/link/../x` names what the OS resolves through `link`. + canonical + .strip_prefix(&root) + .ok()? + .to_str()? + .replace('\\', "/") + } else if is_portable_absolute(path) { let root = normalize_lexical_path(&building.root); let prefix = format!("{}/", root.trim_end_matches('/')); if let Some(relative) = normalized.strip_prefix(&prefix) { @@ -881,4 +913,100 @@ mod tests { .any(|d| d.message.contains("did not match a scanned file")) ); } + + #[test] + fn sarif_percent_escapes_decode_only_hex_digits_and_never_panic() { + // `&rest[i + 1..i + 3]` sliced by byte index: a `%` before a multibyte + // character panicked on a non-char boundary, and from_str_radix accepted `+`. + use super::sarif_uri_path; + assert_eq!(sarif_uri_path("file:///src/%aé.c"), "/src/%aé.c"); + assert_eq!(sarif_uri_path("file:///src/%+1.c"), "/src/%+1.c"); + assert_eq!(sarif_uri_path("file:///src/a%20b.c"), "/src/a b.c"); + assert_eq!(sarif_uri_path("file:///src/%C3%A9.c"), "/src/é.c"); + assert_eq!(sarif_uri_path("file:///src/%4"), "/src/%4"); + } + + #[test] + fn merged_sarif_decides_the_symbolic_lane_per_run() { + // The lane was decided for the whole report with all(): one non-Fold run + // in a merged log demoted every Fold finding to Imported. + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("a.cpp"), "int a() { return 0; }\n").unwrap(); + fs::write(dir.path().join("b.cpp"), "int b() { return 0; }\n").unwrap(); + let result = |file: &str, rule: &str| { + serde_json::json!({"ruleId": rule, "message": {"text": "x"}, + "locations": [{"physicalLocation": {"artifactLocation": {"uri": file}, + "region": {"startLine": 1}}}], + "properties": {"confidence": "proved", "backend": "source"}}) + }; + let value = serde_json::json!({ + "version": "2.1.0", + "runs": [ + {"tool": {"driver": {"name": "fold"}}, "results": [result("a.cpp", "fold-rule")]}, + {"tool": {"driver": {"name": "codeql"}}, "results": [result("b.cpp", "codeql-rule")]} + ] + }); + let report = dir.path().join("merged.sarif"); + 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(), 2); + let lane_for = |rule: &str| { + let node = building + .nodes + .iter() + .find(|node| node.kind == NodeKind::Finding && node.name == rule) + .unwrap(); + building + .edges + .iter() + .find(|edge| edge.source == node.id && edge.kind == EdgeKind::Flags) + .unwrap() + .evidence[0] + .lane + }; + assert_eq!(lane_for("fold-rule"), crate::model::EvidenceLane::Symbolic); + assert_eq!( + lane_for("codeql-rule"), + crate::model::EvidenceLane::Imported + ); + } + + #[cfg(unix)] + #[test] + fn absolute_parent_steps_after_a_symlinked_directory_follow_the_filesystem() { + // The filesystem rule covered only relative paths: `/root/linkout/../src/x` + // (or its file:// form) still normalized lexically into the repository. + 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 mut model = scan(&root).unwrap(); + let absolute = format!( + "{}/linkout/../src/main.rs", + model.root.trim_end_matches('/') + ); + for path in [absolute.clone(), format!("file://{absolute}")] { + let report = workspace.path().join("fold.sarif"); + let value = serde_json::json!({ + "version": "2.1.0", + "runs": [{"tool": {"driver": {"name": "fold"}}, "results": [{ + "ruleId": "through-link", "message": {"text": "x"}, + "locations": [{"physicalLocation": {"artifactLocation": {"uri": path}, + "region": {"startLine": 1}}}], + "properties": {"confidence": "heuristic", "backend": "source"}}]}] + }); + fs::write(&report, serde_json::to_vec(&value).unwrap()).unwrap(); + assert_eq!(import_findings(&mut model, &report).unwrap(), 0, "{path}"); + } + } } -- 2.52.0 From 5a6f8b143a83787fe99d88005b0bee99dc6dbf47 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:57:07 -0600 Subject: [PATCH 2/8] refactor: one definition of "the model scanned a source file" The empty-model condition was written three times: a scan.rs diagnostic, a helper in main.rs for `build`'s warning, and the same helper for `check`'s failure. They could drift. model.rs now owns `has_source_files` (and `Building::has_source_files`), used by all three. No behaviour change; the empty-scan CLI test still passes. --- src/main.rs | 12 ++---------- src/model.rs | 11 +++++++++++ src/scan.rs | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main.rs b/src/main.rs index 37efe92..1abb5b8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -199,7 +199,7 @@ fn run(cli: Cli) -> Result<()> { json.display(), html.display() ); - if !has_source_files(&building) { + if !building.has_source_files() { eprintln!( "brick: warning: no supported source files were found; the model is empty and its score describes nothing" ); @@ -252,7 +252,7 @@ fn run(cli: Cli) -> Result<()> { ); } let metrics = &recomputed.metrics; - if !has_source_files(&building) { + if !building.has_source_files() { failures.push( "the model contains no scanned source files; nothing was analysed".into(), ); @@ -430,14 +430,6 @@ 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/model.rs b/src/model.rs index d227996..6e386a6 100644 --- a/src/model.rs +++ b/src/model.rs @@ -20,6 +20,12 @@ pub struct Building { } impl Building { + /// At least one source file was scanned into the model. The one definition + /// `scan`, `build` and `check` share for "this analysed something". + pub fn has_source_files(&self) -> bool { + has_source_files(&self.nodes) + } + pub fn validate_current_schema(&self) -> Result<(), CompatibilityError> { if self.schema_version != SCHEMA_VERSION { return Err(CompatibilityError::SchemaVersion { @@ -601,6 +607,11 @@ fn compare_confidence(left: &Option, right: &Option) -> } } +/// At least one of `nodes` is a scanned source file. +pub fn has_source_files(nodes: &[Node]) -> bool { + nodes.iter().any(|node| node.kind == NodeKind::File) +} + #[cfg(test)] mod tests { use super::{ diff --git a/src/scan.rs b/src/scan.rs index c32e56a..ea64368 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -101,7 +101,7 @@ pub fn scan_with_options(root: &Path, options: &ScanOptions) -> Result 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) { + if !crate::model::has_source_files(&state.nodes) { state.diagnostics.push(Diagnostic { level: "warning".into(), message: "no supported source files were found; the model is empty".into(), -- 2.52.0 From d0d52edfd14146957671ac86539658807e23edf7 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 20:57:08 -0600 Subject: [PATCH 3/8] fix(check): warn, don't fail, when stored metrics differ from recomputed `check` treated any difference between stored and recomputed metrics as a gate failure, so a model written by an older brick (without the `metrics.resolution` field, or with an older formula) always failed, even with every threshold met. CI checking an archived or cached model broke on upgrade. The gate already uses recomputed metrics, so a mismatch is now a stderr warning. Only fields present in both copies are compared, so a missing newer field does not count as a mismatch. Tampered metrics still cannot pass: the existing test's recomputed cycle count fails the gate, and it still reports "stored metrics". Test: a freshly built model with `metrics.resolution` removed passes `check --min-score 0`; it failed before this change. --- src/main.rs | 30 +++++++++++++++++++++++++----- tests/diff_cli.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index 1abb5b8..641a304 100644 --- a/src/main.rs +++ b/src/main.rs @@ -244,11 +244,15 @@ fn run(cli: Cli) -> Result<()> { 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(), + // Compare only fields both copies carry: a model from an older build lacks + // newer fields (`metrics.resolution`) and may use an older formula. That is + // worth a warning, never a failure; the gate below uses recomputed values. + if stored_metrics_differ( + &serde_json::to_value(&building.metrics)?, + &serde_json::to_value(&recomputed.metrics)?, + ) { + eprintln!( + "brick: warning: stored metrics do not match the graph; gating on recomputed metrics" ); } let metrics = &recomputed.metrics; @@ -430,6 +434,22 @@ fn write_json(building: &Building, path: &PathBuf) -> Result<()> { .with_context(|| format!("cannot write graph {}", path.display())) } +/// True when a stored metrics value disagrees with the recomputed one on a field +/// both carry. Fields missing from the stored copy (written by an older build) +/// are not compared. +fn stored_metrics_differ(stored: &serde_json::Value, recomputed: &serde_json::Value) -> bool { + match (stored, recomputed) { + (serde_json::Value::Object(stored), serde_json::Value::Object(recomputed)) => { + stored.iter().any(|(key, value)| { + recomputed + .get(key) + .is_some_and(|fresh| stored_metrics_differ(value, fresh)) + }) + } + _ => stored != recomputed, + } +} + fn read_json(path: &PathBuf) -> Result { let raw = fs::read_to_string(path) .with_context(|| format!("cannot read graph {}", path.display()))?; diff --git a/tests/diff_cli.rs b/tests/diff_cli.rs index 19c09cb..ca0e0cf 100644 --- a/tests/diff_cli.rs +++ b/tests/diff_cli.rs @@ -306,6 +306,35 @@ fn check_recomputes_metrics_instead_of_trusting_the_file() { assert!(text.contains("stored metrics"), "{text}"); } +#[test] +fn a_model_from_an_older_build_passes_check_on_recomputed_metrics() { + // Any difference between stored and recomputed metrics failed the gate, so a + // model written before `metrics.resolution` existed always failed, even with + // every threshold met. Gate on the recomputed values; a mismatch is a warning. + 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() -> u32 { 1 }\n").unwrap(); + let model = scan::scan(&project).unwrap(); + let mut value: Value = serde_json::to_value(&model).unwrap(); + value["metrics"] + .as_object_mut() + .unwrap() + .remove("resolution"); + let older = directory.path().join("older.json"); + fs::write(&older, serde_json::to_vec_pretty(&value).unwrap()).unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_brick")) + .args(["check", older.to_str().unwrap(), "--min-score", "0"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + #[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 -- 2.52.0 From c3c7c6c3f4a2037d0ba3ef233ff125b979d98e83 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 21:00:59 -0600 Subject: [PATCH 4/8] fix(swift): give the lexical preview Swift's forms and mask block comments Swift ran through the C-style lexical patterns, which never recognize `func`. On a small file (`func realTarget()`, `func ghost()`, a class method `caller` that calls `realTarget()` below a `/* ghost() */` comment) the preview reported: a.swift -> ghost (a.swift:2 declaration, a.swift:5 in a comment) a.swift -> caller (its declaration line) a.swift -> realTarget (its declaration line) and missed the real call. Now: caller -> realTarget (a.swift:7) - Swift gets its own patterns: `func` declarations (with attributes and modifiers), class/struct/enum/protocol/actor/extension declarations (`class func` / `class var` are members, not classes), and imports. - Block comments are masked across lines, nested as Swift allows, and a `/*` inside a string literal does not open one. The masker is salvaged from the unmerged language-parity branch (543bafc). - Calls are attributed to their enclosing function through the existing brace-scoped scopes, now that functions are recognized. Test: tests/swift_preview.rs (functions and a class recognized, the call attributed to `caller`, nothing from comments, nothing from the file node); it failed before this change. --- src/scan.rs | 82 ++++++++++++++++++++++++++++++++++- tests/swift_preview.rs | 98 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 tests/swift_preview.rs diff --git a/src/scan.rs b/src/scan.rs index ea64368..602ae7e 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -452,10 +452,18 @@ impl ScanState { let patterns = Patterns::new(language); let mut scopes: Vec = Vec::new(); let mut brace_depth = 0_i32; + let mut comment_depth = 0_usize; for (index, raw_line) in content.lines().enumerate() { let line_number = index + 1; - let line = strip_line_comment(raw_line, language); + // Block comments (nested, as Swift allows) span lines; without masking, + // a call inside `/* ... */` became an edge. + let masked = if language == "python" { + raw_line.to_owned() + } else { + mask_block_comments(raw_line, &mut comment_depth) + }; + let line = strip_line_comment(&masked, language); let call_line = mask_quoted_strings(line); let indent = raw_line.len() - raw_line.trim_start().len(); if language == "python" { @@ -479,6 +487,7 @@ impl ScanState { let mut class_name = None; if let Some(captures) = patterns.class.captures(line) && let Some(name) = captures.name("name") + && !(language == "swift" && is_swift_member_keyword(name.as_str())) { class_name = Some(name.as_str().to_owned()); let owner_id = scopes @@ -2465,6 +2474,14 @@ struct Patterns { impl Patterns { fn new(language: &str) -> Self { let (class, function, import) = match language { + // Swift's own forms: `func name(`, type declarations with modifiers and + // attributes, and module imports. It used to run through the C patterns, + // which never recognized `func`. + "swift" => ( + r"^\s*(?:@\w+(?:\([^)]*\))?\s+)*(?:(?:public|private|internal|fileprivate|open|final|indirect)\s+)*(?:class|struct|enum|protocol|actor|extension)\s+(?P[A-Za-z_]\w*)(?:\s*:\s*(?P[A-Za-z_][\w.]*))?", + r"^\s*(?:@\w+(?:\([^)]*\))?\s+)*(?:(?:public|private|internal|fileprivate|open|static|class|override|mutating|nonmutating|final|required|convenience|dynamic|nonisolated)\s+)*func\s+(?P[A-Za-z_]\w*)", + r"^\s*(?:@\w+\s+)?import\s+(?:(?:typealias|struct|class|enum|protocol|let|var|func)\s+)?(?P[A-Za-z_][\w.]*)", + ), "python" => ( r"^\s*class\s+(?P[A-Za-z_]\w*)(?:\((?P[A-Za-z_][\w.]*)\))?", r"^\s*(?:async\s+)?def\s+(?P[A-Za-z_]\w*)\s*\(", @@ -2509,6 +2526,69 @@ fn strip_line_comment<'a>(line: &'a str, language: &str) -> &'a str { } } +/// `class func` / `class var` declare members, not a class named `func`. +fn is_swift_member_keyword(name: &str) -> bool { + matches!( + name, + "func" | "var" | "let" | "subscript" | "init" | "deinit" | "case" | "typealias" + ) +} + +/// Mask `/* ... */` comments (nested, as Swift allows) with spaces, carrying the +/// open-comment depth across lines. Quoted strings are left intact, so a `/*` +/// inside a string (`"http://x/*y"`) does not open a comment. +fn mask_block_comments(line: &str, depth: &mut usize) -> String { + let chars: Vec = line.chars().collect(); + let mut result = String::with_capacity(line.len()); + let mut quote = None; + let mut escaped = false; + let mut index = 0; + while index < chars.len() { + let ch = chars[index]; + let next = chars.get(index + 1).copied(); + if *depth > 0 { + if ch == '/' && next == Some('*') { + *depth += 1; + result.push_str(" "); + index += 2; + } else if ch == '*' && next == Some('/') { + *depth -= 1; + result.push_str(" "); + index += 2; + } else { + result.push(' '); + index += 1; + } + } else if let Some(active) = quote { + result.push(ch); + if escaped { + escaped = false; + } else if ch == '\\' { + escaped = true; + } else if ch == active { + quote = None; + } + index += 1; + } else if ch == '"' && chars[index + 1..].contains(&'"') { + quote = Some(ch); + result.push(ch); + index += 1; + } else if ch == '/' && next == Some('/') { + // The rest is a line comment; strip_line_comment removes it. + result.extend(&chars[index..]); + break; + } else if ch == '/' && next == Some('*') { + *depth = 1; + result.push_str(" "); + index += 2; + } else { + result.push(ch); + index += 1; + } + } + result +} + fn brace_delta(line: &str) -> i32 { line.chars().fold(0, |depth, ch| match ch { '{' => depth + 1, diff --git a/tests/swift_preview.rs b/tests/swift_preview.rs new file mode 100644 index 0000000..3ade2c7 --- /dev/null +++ b/tests/swift_preview.rs @@ -0,0 +1,98 @@ +use brick::model::{Building, EdgeKind, NodeKind}; +use brick::scan; +use std::fs; + +fn name_of<'a>(model: &'a Building, id: &str) -> &'a str { + model + .nodes + .iter() + .find(|node| node.id == id) + .map_or("", |node| node.name.as_str()) +} + +fn calls(model: &Building) -> Vec<(String, String, NodeKind)> { + model + .edges + .iter() + .filter(|edge| edge.kind == EdgeKind::Calls) + .map(|edge| { + let source_kind = model + .nodes + .iter() + .find(|node| node.id == edge.source) + .map(|node| node.kind) + .unwrap(); + ( + name_of(model, &edge.source).to_owned(), + name_of(model, &edge.target).to_owned(), + source_kind, + ) + }) + .collect() +} + +#[test] +fn swift_preview_declares_functions_and_ignores_comments() { + // Swift ran through the C-style lexical patterns: `func` declarations were + // never functions, each declaration line became a call from the file, calls + // inside (nested) block comments became edges, and real calls were attributed + // to the file instead of their function. + let dir = tempfile::tempdir().unwrap(); + fs::write( + dir.path().join("a.swift"), + r#"import Foundation + +func realTarget() -> Int { return 1 } +func ghost() -> Int { return 2 } + +final class Worker: NSObject { + let url = "http://example.com/*not-a-comment" + func caller() -> Int { + /* disabled: + ghost() + /* nested: ghost() */ + still disabled: ghost() + */ + return realTarget() + } +} +"#, + ) + .unwrap(); + let model = scan::scan(dir.path()).unwrap(); + + let functions: Vec<&str> = model + .nodes + .iter() + .filter(|node| node.kind == NodeKind::Function) + .map(|node| node.name.as_str()) + .collect(); + for name in ["realTarget", "ghost", "caller"] { + assert!( + functions.contains(&name), + "{name} not a function: {functions:?}" + ); + } + assert!( + model + .nodes + .iter() + .any(|node| node.kind == NodeKind::Class && node.name == "Worker") + ); + + let calls = calls(&model); + assert!( + calls.iter().any(|(source, target, kind)| source == "caller" + && target == "realTarget" + && *kind == NodeKind::Function), + "{calls:?}" + ); + assert!( + !calls.iter().any(|(_, target, _)| target == "ghost"), + "a call inside a block comment became an edge: {calls:?}" + ); + assert!( + !calls.iter().any(|(_, _, kind)| *kind == NodeKind::File), + "a declaration or statement was attributed to the file: {calls:?}" + ); +} -- 2.52.0 From 67334bec5928ef69fc549d95d4ad177c44047a46 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 21:03:08 -0600 Subject: [PATCH 5/8] fix(python): resolve relative, package and dotted imports to the right file Since #5 Python calls resolve only through import edges, but the lexical importer could not produce those edges for the common forms: - `from .util import helper` never matched (the regex needed a leading letter), so the call went unresolved; - `from pkg import f` looked up the stem "pkg" instead of pkg/__init__.py; - `from x.common import f` failed when y/common.py also existed, and `from other.common import f` (a module the repository does not have) resolved to y/common.py by basename. Imports now resolve Python-specifically: leading dots are relative to the importing package, and a dotted name maps to `a/b.py` or `a/b/__init__.py` from the repository root or `src/`. Only a single-component name (`import foo`) may still fall back to a unique module basename; a dotted name never does. `from . import x` stays unresolved (the module is in the import list, not the name). Tests (both failed before): relative, package and duplicate-stem imports resolve their calls to pkg/util.py, pkg/__init__.py and x/common.py; an unknown dotted module does not bind to y/common.py. --- src/scan.rs | 52 ++++++++++++++++++++++++++- tests/python_imports.rs | 80 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 tests/python_imports.rs diff --git a/src/scan.rs b/src/scan.rs index 602ae7e..a3a0b4b 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -2208,6 +2208,55 @@ impl ScanState { (candidates.len() == 1).then(|| candidates[0].clone()) } + /// A Python module name as a repository file. Leading dots are relative to the + /// importing package; a dotted name maps to `a/b.py` or `a/b/__init__.py` from the + /// repository root or `src/`. Only a single-component name (`import foo`) may + /// fall back to a unique module basename elsewhere; a dotted name never does, + /// since `x.common` is not just any `common.py`. + fn resolve_python_import(&self, name: &str, directory: &str) -> Option { + let level = name.chars().take_while(|ch| *ch == '.').count(); + let module = &name[level..]; + if module.is_empty() { + // `from . import x`: the module is in the import list, not the name. + return None; + } + let relative = module.replace('.', "/"); + let bases: Vec = if level > 0 { + let mut base = Path::new(directory).to_path_buf(); + for _ in 1..level { + base = base.parent()?.to_path_buf(); + } + vec![base.to_str()?.to_owned()] + } else { + vec![".".into(), "src".into()] + }; + for base in &bases { + let base = if base.is_empty() { "." } else { base.as_str() }; + for candidate in [ + format!("{base}/{relative}.py"), + format!("{base}/{relative}/__init__.py"), + ] { + if let Some(path) = normalize_import_path(&candidate) + && let Some(node) = self.node(&stable_id("file", &path)).filter(|n| { + n.kind == NodeKind::File && n.language.as_deref() == Some("python") + }) + { + return Some(node.id.clone()); + } + } + } + if level == 0 && !module.contains('.') { + for key in import_keys(module, "python") { + if let Some(ids) = self.files_by_key.get(&key) + && ids.len() == 1 + { + return Some(ids[0].clone()); + } + } + } + None + } + fn resolve_import(&self, name: &str, language: &str, source_path: &str) -> Option { let directory = Path::new(source_path).parent()?.to_str()?; let directory = if directory.is_empty() { "." } else { directory }; @@ -2267,6 +2316,7 @@ impl ScanState { } } "rust" => paths.extend(rust_import_paths(name, directory)?), + "python" => return self.resolve_python_import(name, directory), _ => { for key in import_keys(name, language) { if let Some(ids) = self.files_by_key.get(&key) @@ -2485,7 +2535,7 @@ impl Patterns { "python" => ( r"^\s*class\s+(?P[A-Za-z_]\w*)(?:\((?P[A-Za-z_][\w.]*)\))?", r"^\s*(?:async\s+)?def\s+(?P[A-Za-z_]\w*)\s*\(", - r"^\s*(?:from\s+(?P[A-Za-z_][\w.]*)\s+import|import\s+(?P[A-Za-z_][\w.]*))", + r"^\s*(?:from\s+(?P\.*[A-Za-z_][\w.]*|\.+)\s+import|import\s+(?P[A-Za-z_][\w.]*))", ), _ => ( r"^\s*(?:class|struct)\s+(?P[A-Za-z_]\w*)(?:\s*:\s*(?:public\s+|protected\s+|private\s+)?(?P[A-Za-z_:]\w*(?:::\w+)*))?", diff --git a/tests/python_imports.rs b/tests/python_imports.rs new file mode 100644 index 0000000..9d073ca --- /dev/null +++ b/tests/python_imports.rs @@ -0,0 +1,80 @@ +use brick::model::{Building, EdgeKind}; +use brick::scan; +use std::fs; + +fn scan_files(files: &[(&str, &str)]) -> Building { + let root = tempfile::tempdir().unwrap(); + for (path, content) in files { + let file = root.path().join(path); + fs::create_dir_all(file.parent().unwrap()).unwrap(); + fs::write(file, content).unwrap(); + } + scan::scan(root.path()).unwrap() +} + +/// (caller name, target path) for every resolved call. +fn resolved_calls(model: &Building) -> Vec<(String, String)> { + model + .edges + .iter() + .filter(|edge| edge.kind == EdgeKind::Calls) + .filter_map(|edge| { + let source = model.nodes.iter().find(|n| n.id == edge.source)?; + let target = model.nodes.iter().find(|n| n.id == edge.target)?; + Some((source.name.clone(), target.path.clone()?)) + }) + .collect() +} + +#[test] +fn relative_package_and_qualified_python_imports_resolve_to_the_right_file() { + // Python calls resolve only through import edges, and the lexical importer + // produced none for relative imports (the regex needed a leading letter), + // for packages (`from pkg import f` looked up the stem "pkg", not + // pkg/__init__.py), or when two files shared a stem. + let model = scan_files(&[ + ("pkg/__init__.py", "def from_package():\n return 1\n"), + ("pkg/util.py", "def helper():\n return 1\n"), + ( + "pkg/rel.py", + "from .util import helper\n\ndef use_relative():\n return helper()\n", + ), + ("x/common.py", "def only_here():\n return 1\n"), + ("y/common.py", "def elsewhere():\n return 2\n"), + ( + "main.py", + "from x.common import only_here\nfrom pkg import from_package\n\ndef run():\n only_here()\n return from_package()\n", + ), + ]); + let calls = resolved_calls(&model); + for (caller, target) in [ + ("use_relative", "pkg/util.py"), + ("run", "x/common.py"), + ("run", "pkg/__init__.py"), + ] { + assert!( + calls + .iter() + .any(|(source, path)| source == caller && path == target), + "{caller} -> {target} missing: {calls:?}" + ); + } +} + +#[test] +fn an_unknown_dotted_python_module_does_not_fall_back_to_a_basename() { + // `from other.common import f` names a module this repository does not have; + // matching the stem "common" to y/common.py would invent a dependency. + let model = scan_files(&[ + ("y/common.py", "def f():\n return 2\n"), + ( + "main.py", + "from other.common import f\n\ndef run():\n return f()\n", + ), + ]); + let calls = resolved_calls(&model); + assert!( + !calls.iter().any(|(_, path)| path == "y/common.py"), + "{calls:?}" + ); +} -- 2.52.0 From 3fbb9275a55ef2e784d72e24dbb2b3a3b57a5ffa Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 21:05:11 -0600 Subject: [PATCH 6/8] perf(scan): index import edges once instead of scanning per file imported_paths found the file node with a linear scan of all nodes and then filtered all edges, for every Python/Dart file it built a set for: O(files x (nodes + edges)) during resolution, about 10^10 comparisons for 10k files and a million edges. Imports edges are now indexed by source file on first use and the file node is looked up by its stable id, as resolve_import already does. The index is complete when first built: resolve_relationships resolves every import before any call. Output is unchanged (all 124 tests pass). --- src/scan.rs | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index a3a0b4b..a0d4047 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -248,6 +248,9 @@ struct ScanState { references: HashMap>, /// Lexical-language file -> repository files it imports (resolved edges). imported_paths: HashMap>>, + /// Imports edges by source file id, built once on first use: scanning every + /// node and edge per file made resolution O(files x (nodes + edges)). + imports_by_file: Option>>, /// `name:` of the root pubspec.yaml, so `package:/x.dart` maps to lib/x.dart. dart_package: Option, } @@ -283,6 +286,7 @@ impl ScanState { rust: RustIndex::default(), references: HashMap::new(), imported_paths: HashMap::new(), + imports_by_file: None, } } @@ -1319,20 +1323,27 @@ impl ScanState { if let Some(found) = self.imported_paths.get(path) { return Rc::clone(found); } + if self.imports_by_file.is_none() { + let mut index: HashMap> = HashMap::new(); + for edge in self.edges.iter().filter(|e| e.kind == EdgeKind::Imports) { + index + .entry(edge.source.clone()) + .or_default() + .push(edge.target.clone()); + } + self.imports_by_file = Some(index); + } + let file_id = stable_id("file", path); 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(); + .imports_by_file + .as_ref() + .and_then(|index| index.get(&file_id)) + .into_iter() + .flatten() + .filter_map(|target| self.node(target)) + .filter(|n| n.kind == NodeKind::File) + .filter_map(|n| n.path.clone()) + .collect(); let paths = Rc::new(paths); self.imported_paths .insert(path.to_owned(), Rc::clone(&paths)); -- 2.52.0 From 04653788fa196359bf6b14fd0743f9f1cdc8ce31 Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 21:07:34 -0600 Subject: [PATCH 7/8] fix(rust): keep impls written on a qualified path when leaf names collide When several files declare the same type leaf name, rust_method keeps only the impls of the type a call site means, asking each impl's file which `Foo` it means. An impl in a third file written as `impl crate::a::Foo` resolved nothing (that file has no `Foo` declaration or `use` binding), so its methods were dropped and `x.run()` on an `a::Foo` read "member-not-found". It resolved before #5. The extractor now records the impl's type path when it is written as one (`impl_type_path`, generic arguments removed), the method index keeps it, and the impl filter resolves that path to its declaring file before falling back to the file's own view of the leaf name. Glob imports (`use super::*`) are still not expanded; such an impl stays unresolved rather than guessed. Test: with `Foo` declared in a.rs and c.rs and `impl crate::a::Foo` in ext.rs, `x.run()` on `crate::a::Foo` resolves to ext.rs's `run`; it failed before this change. --- src/scan.rs | 27 +++++++++++++++++++++------ src/source.rs | 39 ++++++++++++++++++++++++++++++++------- tests/rust_methods.rs | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/src/scan.rs b/src/scan.rs index a0d4047..5de20a0 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -149,7 +149,8 @@ struct RustCall { } /// A method node id, the trait it implements (if any), and the file of its `impl`. -type RustMethod = (String, Option, String); +/// (method id, implemented trait, impl file, impl type path when written as one). +type RustMethod = (String, Option, String, Option); /// 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 @@ -764,7 +765,12 @@ impl ScanState { .methods .entry((owner, declaration.name.clone())) .or_default() - .push((id.clone(), implemented, path.to_owned())); + .push(( + id.clone(), + implemented, + path.to_owned(), + declaration.impl_type_path.clone(), + )); if let Some(returns) = &declaration.return_type { self.rust.returns.insert(id.clone(), returns.clone()); } @@ -1973,8 +1979,17 @@ impl ScanState { }; candidates .iter() - .filter(|(_, _, impl_path)| { - self.rust_type_file(ty, impl_path).as_deref() == Some(meant.as_str()) + .filter(|(_, _, impl_path, impl_type_path)| { + // `impl crate::a::Foo` says which Foo itself; otherwise ask what + // the impl's own file means by the leaf name. + let implemented = match impl_type_path { + Some(qualified) => self + .resolve_import(qualified, "rust", impl_path) + .and_then(|id| self.node(&id)) + .and_then(|node| node.path.clone()), + None => self.rust_type_file(ty, impl_path), + }; + implemented.as_deref() == Some(meant.as_str()) }) .collect() } else { @@ -1983,7 +1998,7 @@ impl ScanState { let inherent: Vec<_> = candidates .iter() .copied() - .filter(|(_, t, _)| t.is_none()) + .filter(|(_, t, _, _)| t.is_none()) .collect(); let chosen: Vec<_> = if inherent.is_empty() { candidates @@ -1991,7 +2006,7 @@ impl ScanState { 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"), diff --git a/src/source.rs b/src/source.rs index 31726aa..ac5aa9b 100644 --- a/src/source.rs +++ b/src/source.rs @@ -23,6 +23,9 @@ pub(crate) struct Declaration { pub impl_type: Option, /// Rust: trait of the enclosing `impl Trait for T` or `trait` block. pub impl_trait: Option, + /// Rust: the enclosing impl's type as written when it is a path + /// (`impl crate::a::Foo`), generic arguments removed; `None` for a bare name. + pub impl_type_path: Option, /// Rust: declared return type, whitespace removed. pub return_type: Option, /// C++ `virtual` or `override`/`final` on this definition. @@ -378,10 +381,10 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result Result, source: &str) -> Option { } /// Rust `impl T` / `impl Trait for T` / `trait Trait` enclosing a declaration. -fn rust_impl_context(node: Node<'_>, source: &str) -> (Option, Option) { +type RustImplContext = (Option, Option, Option); + +/// (impl type leaf, implemented or declared trait, impl type path) around `node`. +fn rust_impl_context(node: Node<'_>, source: &str) -> RustImplContext { let mut parent = node.parent(); while let Some(n) = parent { match n.kind() { "impl_item" => { + let ty = n.child_by_field_name("type"); return ( - n.child_by_field_name("type") - .and_then(|ty| rust_type_name(ty, source)), + ty.and_then(|ty| rust_type_name(ty, source)), n.child_by_field_name("trait") .and_then(|ty| rust_type_name(ty, source)), + ty.and_then(|ty| rust_type_path(ty, source)), ); } "trait_item" => { @@ -1661,13 +1669,30 @@ fn rust_impl_context(node: Node<'_>, source: &str) -> (Option, Option return (None, None), + "function_item" | "closure_expression" => return (None, None, None), _ => parent = n.parent(), } } - (None, None) + (None, None, None) +} + +/// A type written as a path (`crate::a::Foo`), references and generic arguments +/// removed; `None` for a bare `Foo`. +fn rust_type_path(ty: Node<'_>, source: &str) -> Option { + match ty.kind() { + "scoped_type_identifier" => Some(format!( + "{}::{}", + text(ty.child_by_field_name("path")?, source), + text(ty.child_by_field_name("name")?, source) + )), + "generic_type" | "reference_type" => { + rust_type_path(ty.child_by_field_name("type")?, source) + } + _ => None, + } } /// Leaf type name with references and generic arguments removed. diff --git a/tests/rust_methods.rs b/tests/rust_methods.rs index 72849b2..bde2e52 100644 --- a/tests/rust_methods.rs +++ b/tests/rust_methods.rs @@ -202,3 +202,39 @@ fn same_named_types_in_different_modules_resolve_through_use_bindings() { let guess = function(&model, "src/other.rs", "guess"); assert_eq!(targets(guess), [(String::new(), "ambiguous".to_owned())]); } + +#[test] +fn an_impl_on_a_qualified_path_in_another_file_keeps_its_methods() { + // With several files declaring `Foo`, an impl was kept only if its own file + // resolved `Foo`; `impl crate::a::Foo` in a third file resolved nothing and its + // methods were dropped, so the call read "member-not-found". + let model = scan_files(&[ + ( + "src/lib.rs", + "pub mod a;\npub mod c;\npub mod ext;\npub mod main_use;\n", + ), + ("src/a.rs", "pub struct Foo;\n"), + ("src/c.rs", "pub struct Foo;\n"), + ( + "src/ext.rs", + "impl crate::a::Foo {\n pub fn run(&self) {}\n}\n", + ), + ( + "src/main_use.rs", + "use crate::a::Foo;\npub fn f(x: Foo) {\n x.run();\n}\n", + ), + ]); + let caller = function(&model, "src/main_use.rs", "f"); + let targets = calls(&model, caller, "source.resolution"); + assert!( + targets.iter().any(|(_, name, _)| name == "run"), + "{targets:?}" + ); + let run = function(&model, "src/ext.rs", "run"); + assert!( + model + .edges + .iter() + .any(|e| e.kind == EdgeKind::Calls && e.source == caller.id && e.target == run.id) + ); +} -- 2.52.0 From 324900287b262064f6daf500edcb6ed5de0bc31a Mon Sep 17 00:00:00 2001 From: trvon Date: Thu, 24 Sep 2026 21:11:48 -0600 Subject: [PATCH 8/8] fix(dart): honour import prefixes, show/hide and one level of export Problem: a bare call in a Dart file resolved against every imported library. An `import 'a.dart' as p;` still bound `foo()` to a.dart's foo, `hide` and `show` combinators were ignored, and a name re-exported through a barrel library (`export 'impl.dart';`) never resolved. Fix: record each Dart import/export directive with its `as` prefix and show/hide lists. A bare name resolves only in libraries imported without a prefix whose combinators let it through, plus that library's own exports that also let it through (one level). Test: tests/dart_imports.rs covers a prefixed, a hiding, a showing and a barrelled import; it failed on the prefixed case before this change. --- README.md | 3 +- src/scan.rs | 44 +++++++++++++++++++++++++- src/source.rs | 55 +++++++++++++++++++++++++++++++++ tests/dart_imports.rs | 72 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 tests/dart_imports.rs diff --git a/README.md b/README.md index 7dcc0cf..1adeaa9 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,8 @@ 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 +(Dart `show`/`hide` are honoured, `as`-prefixed imports never bind bare names, +and a library's own `export`s are followed one level). 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). diff --git a/src/scan.rs b/src/scan.rs index 5de20a0..dee509c 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -252,6 +252,8 @@ struct ScanState { /// Imports edges by source file id, built once on first use: scanning every /// node and edge per file made resolution O(files x (nodes + edges)). imports_by_file: Option>>, + /// Dart file -> its import/export directives (prefixes, show/hide, exports). + dart_directives: HashMap>, /// `name:` of the root pubspec.yaml, so `package:/x.dart` maps to lib/x.dart. dart_package: Option, } @@ -288,6 +290,7 @@ impl ScanState { references: HashMap::new(), imported_paths: HashMap::new(), imports_by_file: None, + dart_directives: HashMap::new(), } } @@ -639,6 +642,10 @@ impl ScanState { return; } }; + if language == "dart" { + self.dart_directives + .insert(path.to_owned(), document.dart_directives.clone()); + } for message in &document.diagnostics { self.diagnostics.push(Diagnostic { level: "warning".into(), @@ -1356,6 +1363,39 @@ impl ScanState { paths } + /// Libraries whose public top-level `name` a Dart file can call bare: imports + /// without an `as` prefix whose `show`/`hide` let `name` through, plus one level + /// of those libraries' own `export`s that also let it through. + fn dart_visible_libraries(&self, path: &str, name: &str) -> HashSet { + let library_of = |target: &str, from: &str| { + self.resolve_import(target, "dart", from) + .and_then(|id| self.node(&id)) + .and_then(|node| node.path.clone()) + }; + let mut visible = HashSet::new(); + for directive in self.dart_directives.get(path).into_iter().flatten() { + if directive.export || directive.prefix.is_some() || !directive.allows(name) { + continue; + } + let Some(library) = library_of(&directive.target, path) else { + continue; + }; + for export in self + .dart_directives + .get(&library) + .into_iter() + .flatten() + .filter(|export| export.export && export.allows(name)) + { + if let Some(exported) = library_of(&export.target, &library) { + visible.insert(exported); + } + } + visible.insert(library); + } + visible + } + /// 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) { @@ -1434,8 +1474,10 @@ impl ScanState { if let Some(id) = self.resolve_qualified_symbol(relationship) { return (id, None, None); } - let imported = if language == "python" || language == "dart" { + let imported = if language == "python" { self.imported_paths(&relationship.path) + } else if language == "dart" { + Rc::new(self.dart_visible_libraries(&relationship.path, name)) } else { Rc::default() }; diff --git a/src/source.rs b/src/source.rs index ac5aa9b..49baf26 100644 --- a/src/source.rs +++ b/src/source.rs @@ -38,6 +38,25 @@ pub(crate) struct Declaration { pub preprocessor_alternative: bool, } +/// A Dart `import` or `export` directive and what it exposes. +#[derive(Clone, Debug)] +pub(crate) struct DartDirective { + pub target: String, + /// `as p`: names are reachable only as `p.name`, never bare. + pub prefix: Option, + pub show: Vec, + pub hide: Vec, + pub export: bool, +} + +impl DartDirective { + /// The directive's combinators let `name` through. + pub fn allows(&self, name: &str) -> bool { + (self.show.is_empty() || self.show.iter().any(|shown| shown == name)) + && !self.hide.iter().any(|hidden| hidden == name) + } +} + pub(crate) struct ImportBinding { pub local: String, pub exported: String, @@ -107,6 +126,8 @@ pub(crate) struct Document { pub diagnostics: Vec, pub backend: String, pub bindings: Vec, + /// Dart import/export directives with their prefixes and combinators. + pub dart_directives: Vec, /// C++ qualified names declared (prototype or definition) by this file. pub declared: Vec, /// C/C++ preprocessor macro names defined by this file. @@ -221,6 +242,7 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result Result( false } +fn dart_directive(node: Node<'_>, target: &str, source: &str) -> DartDirective { + let mut directive = DartDirective { + target: target.to_owned(), + prefix: node + .child_by_field_name("alias") + .map(|alias| text(alias, source).to_owned()), + show: Vec::new(), + hide: Vec::new(), + export: node.kind() == "library_export", + }; + let mut cursor = node.walk(); + for combinator in node + .named_children(&mut cursor) + .filter(|child| child.kind() == "combinator") + { + let mut names = combinator.walk(); + let identifiers = combinator + .named_children(&mut names) + .filter(|child| child.kind() == "identifier") + .map(|identifier| text(identifier, source).to_owned()); + if text(combinator, source).trim_start().starts_with("hide") { + directive.hide.extend(identifiers); + } else { + directive.show.extend(identifiers); + } + } + directive +} + fn import_bindings(node: Node<'_>, module: &str, source: &str) -> Vec { let mut result = Vec::new(); // Type-only imports must not authorize runtime call resolution. diff --git a/tests/dart_imports.rs b/tests/dart_imports.rs new file mode 100644 index 0000000..eaff7cc --- /dev/null +++ b/tests/dart_imports.rs @@ -0,0 +1,72 @@ +use brick::model::{Building, EdgeKind}; +use brick::scan; +use std::fs; + +fn scan_files(files: &[(&str, &str)]) -> Building { + let root = tempfile::tempdir().unwrap(); + for (path, content) in files { + let file = root.path().join(path); + fs::create_dir_all(file.parent().unwrap()).unwrap(); + fs::write(file, content).unwrap(); + } + scan::scan(root.path()).unwrap() +} + +/// Paths of the resolved targets of calls leaving functions named `caller`. +fn call_targets(model: &Building, caller: &str) -> Vec { + model + .edges + .iter() + .filter(|edge| edge.kind == EdgeKind::Calls) + .filter(|edge| { + model + .nodes + .iter() + .any(|n| n.id == edge.source && n.name == caller) + }) + .filter_map(|edge| { + let target = model.nodes.iter().find(|n| n.id == edge.target)?; + Some(format!("{}:{}", target.path.clone()?, target.name)) + }) + .collect() +} + +#[test] +fn dart_import_prefixes_combinators_and_exports_decide_what_a_bare_name_binds() { + // Dart calls resolved through every imported library, ignoring `as` prefixes + // and `show`/`hide`, and never through a library's own `export`s. + let model = scan_files(&[ + ("lib/a.dart", "void foo() {}\n"), + ("lib/h.dart", "void hidden() {}\nvoid visible() {}\n"), + ("lib/s.dart", "void shown() {}\nvoid other() {}\n"), + ("lib/impl.dart", "void exported() {}\n"), + ("lib/barrel.dart", "export 'impl.dart';\n"), + ( + "lib/prefixed.dart", + "import 'a.dart' as a;\nvoid usePrefix() { foo(); }\n", + ), + ( + "lib/hiding.dart", + "import 'h.dart' hide hidden;\nvoid useHide() { hidden(); visible(); }\n", + ), + ( + "lib/showing.dart", + "import 's.dart' show shown;\nvoid useShow() { shown(); other(); }\n", + ), + ( + "lib/barrelled.dart", + "import 'barrel.dart';\nvoid useExport() { exported(); }\n", + ), + ]); + assert!( + call_targets(&model, "usePrefix").is_empty(), + "{:?}", + call_targets(&model, "usePrefix") + ); + assert_eq!(call_targets(&model, "useHide"), ["lib/h.dart:visible"]); + assert_eq!(call_targets(&model, "useShow"), ["lib/s.dart:shown"]); + assert_eq!( + call_targets(&model, "useExport"), + ["lib/impl.dart:exported"] + ); +} -- 2.52.0