diff --git a/docs/resolution.md b/docs/resolution.md index 95b2a2c..7f7711f 100644 --- a/docs/resolution.md +++ b/docs/resolution.md @@ -22,7 +22,9 @@ tests. named and default import bindings resolve across files. - **Dart:** relative imports resolve against the importing directory, and `package:/` against `lib/`. Public top-level names from imported - libraries resolve across files; `show`/`hide` combinators are not modelled. + libraries resolve across files: `show`/`hide` are honoured, `as`-prefixed + imports never bind bare names, and a library's own `export`s are followed one + level. - **Rust:** module-scoped grouped and aliased imports and module-qualified public calls resolve. Types are identified by their declaring file, so same-named types in different modules resolve through the caller's `use` binding or stay 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}"); + } + } } diff --git a/src/main.rs b/src/main.rs index 37efe92..641a304 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" ); @@ -244,15 +244,19 @@ 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; - if !has_source_files(&building) { + if !building.has_source_files() { failures.push( "the model contains no scanned source files; nothing was analysed".into(), ); @@ -430,12 +434,20 @@ 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) +/// 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 { 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..dee509c 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(), @@ -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 @@ -248,6 +249,11 @@ 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>>, + /// 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, } @@ -283,6 +289,8 @@ impl ScanState { rust: RustIndex::default(), references: HashMap::new(), imported_paths: HashMap::new(), + imports_by_file: None, + dart_directives: HashMap::new(), } } @@ -452,10 +460,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 +495,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 @@ -625,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(), @@ -751,7 +772,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()); } @@ -1310,26 +1336,66 @@ 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)); 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) { @@ -1408,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() }; @@ -1953,8 +2021,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 { @@ -1963,7 +2040,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 @@ -1971,7 +2048,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"), @@ -2199,6 +2276,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 }; @@ -2258,6 +2384,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) @@ -2465,10 +2592,18 @@ 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*\(", - 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+)*))?", @@ -2509,6 +2644,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/src/source.rs b/src/source.rs index 31726aa..49baf26 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. @@ -35,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, @@ -104,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. @@ -218,6 +242,7 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result Result 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. @@ -1644,16 +1703,20 @@ fn cpp_first_template_type(node: Node<'_>, 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 +1724,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/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"] + ); +} 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 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:?}" + ); +} 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) + ); +} 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:?}" + ); +}