diff --git a/crates/tracedecay-code-index/src/chunks.rs b/crates/tracedecay-code-index/src/chunks.rs index cdcc7b8a66..aae9069fa9 100644 --- a/crates/tracedecay-code-index/src/chunks.rs +++ b/crates/tracedecay-code-index/src/chunks.rs @@ -2412,10 +2412,11 @@ fn resolve_file_references( }); } } - // Java overloads the call's arguments cannot tell apart (equal - // arity, a variadic tail) stay a disclosed caller gap. - candidates if candidates.is_empty() || language == "java" => { - if let Some(candidate) = cross_file_reference_candidate( + // No candidate: the reference is retained for cross-file + // resolution. Several it cannot choose between: a disclosed + // caller gap. Java overloads resolve through Java's own rules. + _ => { + if let Some(mut candidate) = cross_file_reference_candidate( source, offsets, &references_by_site, @@ -2434,12 +2435,10 @@ fn resolve_file_references( candidate.reference_name.clone(), ))) { + candidate.ambiguous_local = !compatible.is_empty() && language != "java"; retained.push(candidate); } } - // Same-file ambiguity: adding cross-file candidates can only make - // it more ambiguous, so the reference stays unresolved. - _ => {} } } // The parser may describe one invocation both as a receiver expression @@ -2539,6 +2538,7 @@ fn cross_file_reference_candidate( .unwrap_or(from.span), unmodeled_import: reference.unmodeled_import, argument_count: reference.argument_count, + ambiguous_local: false, }) } @@ -5289,7 +5289,14 @@ pub fn real_symbol() {} &[], ); assert!(resolved.is_empty()); - assert!(retained.is_empty()); + assert_eq!( + retained + .iter() + .map(|reference| reference.reference_name.as_str()) + .collect::>(), + ["Base"], + "an ambiguous bare name stays a retained reference, not a silent drop" + ); } #[test] diff --git a/crates/tracedecay-code-index/src/chunks/artifacts.rs b/crates/tracedecay-code-index/src/chunks/artifacts.rs index 71666ab0b5..b8b3614315 100644 --- a/crates/tracedecay-code-index/src/chunks/artifacts.rs +++ b/crates/tracedecay-code-index/src/chunks/artifacts.rs @@ -163,6 +163,10 @@ pub struct CodeIndexUnresolvedReferenceV1 { /// Arguments the call site passes, where the extractor counts them. #[serde(default, skip_serializing_if = "Option::is_none")] pub argument_count: Option, + /// The file defines more than one candidate the call site cannot choose + /// between, so it is a disclosed caller gap and never binds cross-file. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub ambiguous_local: bool, } impl CodeIndexUnresolvedReferenceV1 { diff --git a/crates/tracedecay-code-index/src/production/go_satisfaction.rs b/crates/tracedecay-code-index/src/production/go_satisfaction.rs index 8be6f1ce98..4cfa475449 100644 --- a/crates/tracedecay-code-index/src/production/go_satisfaction.rs +++ b/crates/tracedecay-code-index/src/production/go_satisfaction.rs @@ -369,6 +369,7 @@ where evidence_span: interface.span, unmodeled_import: None, argument_count: None, + ambiguous_local: false, }; let methods = match expansion { Ok(methods) => methods, diff --git a/crates/tracedecay-code-index/src/production/helpers.rs b/crates/tracedecay-code-index/src/production/helpers.rs index 088f67f140..fb30527c21 100644 --- a/crates/tracedecay-code-index/src/production/helpers.rs +++ b/crates/tracedecay-code-index/src/production/helpers.rs @@ -314,8 +314,8 @@ type EdgeEvidenceV1 = ( u64, ); -/// `files`' edge evidence resolved whole, with the interfaces whose -/// implementors the seal cannot decide. +/// `files`' edge evidence resolved whole, with the references the seal +/// cannot decide. pub(crate) fn collect_edge_evidence( files: &[T], ) -> Result @@ -327,11 +327,11 @@ where // the edges this returns plus one sort buffer. let CrossFileResolutionV1 { edges, - implementor_gaps, + gaps, ambiguous_name_drops, } = resolve_cross_file_references(files)?; let (edges, abstentions) = edge_evidence(files, edges); - Ok((edges, abstentions, implementor_gaps, ambiguous_name_drops)) + Ok((edges, abstentions, gaps, ambiguous_name_drops)) } /// `files`' edge evidence: each file's own edges and `cross_file`, the @@ -398,7 +398,8 @@ pub(crate) type ReferenceSelectionV1 = [(usize, Vec)]; /// Resolves only `selection`'s references against the whole file set whose /// symbols `by_simple_name` indexes. Each reference binds exactly as -/// [`resolve_cross_file_references`] binds it. +/// [`resolve_cross_file_references`] binds it; the result is the edges those +/// references contribute and the ambiguous calls among them. #[tracing::instrument(name = "code_index.seal.resolve_selected", level = "trace", skip_all)] pub(crate) fn resolve_selected_cross_file_references( files: &[T], @@ -411,11 +412,13 @@ where resolve_references(files, by_simple_name, Some(selection)) } -/// A whole-set resolution: the cross-file edges, and one row per interface -/// whose implementors the seal cannot decide. +/// A resolution: the cross-file edges, and the references the seal cannot +/// decide, each a disclosed gap: calls with more than one candidate, and, for +/// a whole-set pass, one row per interface whose implementors it cannot +/// decide. pub(crate) struct CrossFileResolutionV1 { pub(crate) edges: Vec, - pub(crate) implementor_gaps: Vec, + pub(crate) gaps: Vec, pub(crate) ambiguous_name_drops: u64, } @@ -505,7 +508,7 @@ where })?; let ambiguous_name_drops = per_file .iter() - .fold(0_u64, |total, (_, drops)| total.saturating_add(*drops)); + .fold(0_u64, |total, (_, _, drops)| total.saturating_add(*drops)); // Satisfaction needs every Go method set, so only a whole-set pass // decides it, and only a file set with Go types builds the module index. let satisfaction = if selection.is_none() @@ -521,12 +524,14 @@ where let mut edges = Vec::with_capacity( per_file .iter() - .map(|(file_edges, _)| file_edges.len()) + .map(|(edges, _, _)| edges.len()) .sum::() .saturating_add(satisfaction.edges.len()), ); - for (file_edges, _) in per_file { + let mut gaps = satisfaction.gaps; + for (file_edges, file_gaps, _) in per_file { edges.extend(file_edges); + gaps.extend(file_gaps); } edges.extend(satisfaction.edges); { @@ -538,7 +543,7 @@ where }; Ok(CrossFileResolutionV1 { edges, - implementor_gaps: satisfaction.gaps, + gaps, ambiguous_name_drops, }) } @@ -677,7 +682,7 @@ where } /// Resolve one file's retained unresolved references against the whole staged -/// file set. +/// file set: the edges they bind, and the calls left ambiguous. /// /// The memo is file-local on purpose. `ResolvedReferenceCacheV1` is keyed by /// source-file index, so a shared map could never serve another file's entry. @@ -689,7 +694,11 @@ fn resolve_one_file_cross_file_references( modules: &ResolutionModulesV1<'_, T>, index: usize, picks: Option<&[usize]>, -) -> (Vec, u64) +) -> ( + Vec, + Vec, + u64, +) where T: ResolutionFileV1, { @@ -698,6 +707,7 @@ where let same_file_binds = is_module_import_language(files[index].language()); let mut resolved_references = ResolvedReferenceCacheV1::new(); let mut edges = Vec::new(); + let mut gaps = Vec::new(); let mut ambiguous_name_drops = 0_u64; let references = &files[index].as_ref().artifacts.unresolved_references; let every = picks.is_none().then(|| references.iter()); @@ -708,6 +718,7 @@ where reference.reference_name.as_str(), reference.kind, reference.argument_count, + reference.ambiguous_local, ); let (resolved, ambiguous) = if let Some(resolved) = resolved_references.get(&cache_key) { resolved.clone() @@ -732,8 +743,15 @@ where if ambiguous { ambiguous_name_drops = ambiguous_name_drops.saturating_add(1); } - let Some((target_index, targets)) = resolved else { - continue; + let (target_index, targets) = match resolved { + Some(ReferenceResolutionV1::Bound(target_index, targets)) => (target_index, targets), + Some(ReferenceResolutionV1::Ambiguous) => { + if reference.kind == RelationEdgeKindV1::Calls { + gaps.push(reference.clone()); + } + continue; + } + None => continue, }; if target_index == index && !same_file_binds { continue; @@ -746,7 +764,7 @@ where evidence_span: reference.evidence_span, })); } - (edges, ambiguous_name_drops) + (edges, gaps, ambiguous_name_drops) } #[cfg(test)] @@ -760,10 +778,20 @@ pub(super) fn take_seal_reference_resolutions() -> usize { } type ResolvedReferenceCacheV1<'a> = HashMap< - (usize, &'a str, RelationEdgeKindV1, Option), - (Option<(usize, Vec)>, bool), + (usize, &'a str, RelationEdgeKindV1, Option, bool), + (Option, bool), >; +/// What one retained reference resolves to; `None` beside it is a reference +/// with no cross-file binding. +#[derive(Clone)] +enum ReferenceResolutionV1 { + /// The targets, all in one file. + Bound(usize, Vec), + /// More than one candidate the reference cannot choose between. + Ambiguous, +} + fn resolve_cross_file_reference( files: &[T], by_simple_name: &dyn SymbolsByNameV1, @@ -771,19 +799,22 @@ fn resolve_cross_file_reference( index: usize, reference: &CodeIndexUnresolvedReferenceV1, ambiguous: &mut bool, -) -> Option<(usize, Vec)> +) -> Option where T: ResolutionFileV1, { + if reference.ambiguous_local { + return Some(ReferenceResolutionV1::Ambiguous); + } let rust = &modules.rust; let file = files[index].as_ref(); // These languages bind one exact module member through their own import // and package rules, never by name matching. if is_module_import_language(file.extraction.language.as_str()) { return match modules.modules().call_outcome(index, reference)? { - ImportBindingOutcomeV1::Bound(target_index, symbol) => { - Some((target_index, vec![symbol.occurrence.clone()])) - } + ImportBindingOutcomeV1::Bound(target_index, symbol) => Some( + ReferenceResolutionV1::Bound(target_index, vec![symbol.occurrence.clone()]), + ), ImportBindingOutcomeV1::External | ImportBindingOutcomeV1::Unresolved | ImportBindingOutcomeV1::ValueMember => None, @@ -810,9 +841,9 @@ where ) { return match outcome { - ImportBindingOutcomeV1::Bound(target_index, symbol) if target_index != index => { - Some((target_index, vec![symbol.occurrence.clone()])) - } + ImportBindingOutcomeV1::Bound(target_index, symbol) if target_index != index => Some( + ReferenceResolutionV1::Bound(target_index, vec![symbol.occurrence.clone()]), + ), ImportBindingOutcomeV1::Bound(..) | ImportBindingOutcomeV1::External | ImportBindingOutcomeV1::Unresolved @@ -937,7 +968,8 @@ where .collect::>(); // A type-path call may match both an inherent `Type::method` and one or // more `::method` aliases; Rust prefers the inherent, so - // keep a unique non-UFCS hit when aliases also matched. + // keep a unique non-UFCS hit when aliases also matched. Any other choice + // between candidates is ambiguous. let compatible = match compatible.as_slice() { [] => return None, [_] => compatible, @@ -965,7 +997,7 @@ where [_] => inherent, _ => { *ambiguous = true; - return None; + return Some(ReferenceResolutionV1::Ambiguous); } } } @@ -974,7 +1006,7 @@ where if *target_index == index { return None; } - Some(( + Some(ReferenceResolutionV1::Bound( *target_index, compatible .iter() diff --git a/crates/tracedecay-code-index/src/production/mod.rs b/crates/tracedecay-code-index/src/production/mod.rs index c88824dec6..448cc27bd8 100644 --- a/crates/tracedecay-code-index/src/production/mod.rs +++ b/crates/tracedecay-code-index/src/production/mod.rs @@ -2155,14 +2155,14 @@ where let _span = tracing::trace_span!("code_index.build.assemble.graph_outputs").entered(); { - let (edges, abstentions, implementor_gaps, ambiguous_name_drops) = + let (edges, abstentions, gaps, ambiguous_name_drops) = collect_edge_evidence(&staged.files)?; let mut unresolved = resolution_outputs::unresolved_calls_for_edges( &staged.files, &edges, &|| Ok(()), )?; - unresolved.extend(implementor_gaps); + unresolved.extend(gaps); unresolved.sort(); unresolved.dedup(); Ok::<_, CodeIndexProductionErrorV1>(( diff --git a/crates/tracedecay-code-index/src/production/sparse_increment_tests.rs b/crates/tracedecay-code-index/src/production/sparse_increment_tests.rs index 2810783b82..1692e6de79 100644 --- a/crates/tracedecay-code-index/src/production/sparse_increment_tests.rs +++ b/crates/tracedecay-code-index/src/production/sparse_increment_tests.rs @@ -556,3 +556,55 @@ fn ambiguity_census_preserves_unknown_parent_count() { ); assert_eq!(answers(&store).statistics.ambiguous_name_drops, None); } + +#[test] +fn sparse_local_shadowing_preserves_gaps_and_the_generic_ambiguity_census() { + let clear = + "import { scale } from './lib';\nexport function clear(): number { return scale(1); }\n"; + let shadowed = "import { scale } from './lib';\ndescribe('scope', () => {\n function scale(x: number) { return x; }\n function scale(x: number) { return x + 1; }\n it('shadowed', () => { scale(1); });\n});\nexport function clear(): number { return scale(1); }\n"; + let tree = |app| { + vec![ + ("web/app.ts", "typescript", app), + ( + "web/lib.ts", + "typescript", + "export function scale(x: number) { return x * 2; }\n", + ), + ("web/a.ts", "typescript", "export const a = 1;\n"), + ("web/b.ts", "typescript", "export const b = 1;\n"), + ("web/c.ts", "typescript", "export const c = 1;\n"), + ("web/d.ts", "typescript", "export const d = 1;\n"), + ("web/e.ts", "typescript", "export const e = 1;\n"), + ("web/f.ts", "typescript", "export const f = 1;\n"), + ("web/g.ts", "typescript", "export const g = 1;\n"), + ] + }; + assert_sparse_matches_cold(&tree(clear), &tree(shadowed)); + assert_sparse_matches_cold(&tree(shadowed), &tree(clear)); + + let store = MemorySealedPublicationStoreV1::default(); + let mut incremental = owner(&store); + publish(&mut incremental, &tree(clear), 1_100_000); + for (app, expected_gaps, sealed_at) in [(shadowed, 1, 1_200_000), (clear, 0, 1_300_000)] { + let successor = publish(&mut incremental, &tree(app), sealed_at); + assert_eq!(successor.cold_reason(), None); + let restored = answers(&store); + assert_eq!(restored.statistics.ambiguous_name_drops, Some(0)); + assert_eq!( + restored + .unresolved_calls + .iter() + .filter(|call| call.ambiguous_local && call.reference_name == "scale") + .count(), + expected_gaps + ); + assert_eq!( + restored + .edges + .iter() + .filter(|edge| edge.kind == RelationEdgeKindV1::Calls) + .count(), + 1 + ); + } +} diff --git a/crates/tracedecay-code-index/src/production/sparse_resolution.rs b/crates/tracedecay-code-index/src/production/sparse_resolution.rs index 1f0cb1abc4..ab7c83f706 100644 --- a/crates/tracedecay-code-index/src/production/sparse_resolution.rs +++ b/crates/tracedecay-code-index/src/production/sparse_resolution.rs @@ -640,7 +640,7 @@ pub(super) fn resolve_edit( &|| Ok(()), ) .map_err(|error| CodeIndexProductionErrorV1::Contract(error.to_string()))?; - for call in rederived { + for call in rederived.into_iter().chain(resolved.gaps) { let owner = owner_of .get(&call.from_occurrence) .ok_or_else(|| contract("a re-derived call limitation leaves the selection"))?; diff --git a/crates/tracedecay-code-index/tests/code_index_suite/relation_coverage.rs b/crates/tracedecay-code-index/tests/code_index_suite/relation_coverage.rs index ed445ed2b7..ae562d9a11 100644 --- a/crates/tracedecay-code-index/tests/code_index_suite/relation_coverage.rs +++ b/crates/tracedecay-code-index/tests/code_index_suite/relation_coverage.rs @@ -327,3 +327,111 @@ fn a_crlf_cargo_manifest_names_its_crate_for_cross_crate_calls() { ); assert!(!graph.callers_partial("lib/src/lib.rs::helper")); } + +#[test] +fn a_type_path_call_with_two_inherent_candidates_makes_both_callers_partial() { + let graph = sealed_graph_of(&[ + ( + "rs/src/lib.rs", + "mod unix;\nmod windows;\n\npub struct Clock;\n\npub fn now() -> u64 {\n Clock::tick()\n}\n", + ), + ( + "rs/src/unix.rs", + "use crate::Clock;\n\n#[cfg(unix)]\nimpl Clock {\n pub fn tick() -> u64 {\n 1\n }\n}\n", + ), + ( + "rs/src/windows.rs", + "use crate::Clock;\n\n#[cfg(windows)]\nimpl Clock {\n pub fn tick() -> u64 {\n 2\n }\n}\n", + ), + ]); + + for target in [ + "rs/src/unix.rs::Clock::tick", + "rs/src/windows.rs::Clock::tick", + ] { + assert_eq!(graph.callers(target), Vec::::new(), "{target}"); + assert!(graph.callers_partial(target), "{target} callers partial"); + } + assert!(graph.callees_partial("rs/src/lib.rs::now")); +} + +#[test] +fn a_same_file_call_with_two_candidates_makes_both_callers_partial() { + let graph = sealed_graph_of(&[( + "rb/units.rb", + "def scale(x)\n 3\nend\n\ndef scale(x)\n 1\nend\n\ndef total\n scale(1)\nend\n", + )]); + + let scales = graph + .generation + .symbols() + .symbols + .iter() + .filter(|symbol| symbol.qualified_name == "rb/units.rb::scale") + .map(|symbol| symbol.occurrence.clone()) + .collect::>(); + assert_eq!(scales.len(), 2); + let caller_gaps = graph + .reader + .unresolved_caller_gaps(&scales, None, Arc::new(NeverCancelled)) + .expect("unresolved caller probe"); + assert!(!caller_gaps.is_empty()); + assert!(graph.callees_partial("rb/units.rb::total")); +} + +#[test] +fn a_same_file_python_call_with_two_candidates_makes_its_callees_partial() { + let graph = sealed_graph_of(&[( + "py/units.py", + "def scale(x):\n return x\n\ndef scale(x):\n return x + 1\n\ndef total():\n return scale(1)\n", + )]); + + assert!(graph.callees_partial("py/units.py::total")); +} + +#[test] +fn locally_ambiguous_definitions_shadow_an_imported_name() { + let graph = sealed_graph_of(&[ + ("py/pkg/__init__.py", ""), + ("py/pkg/lib.py", "def scale(x):\n return x * 2\n"), + ( + "py/pkg/app.py", + "from pkg.lib import scale\n\ndef scale(x):\n return x\n\ndef scale(x):\n return x + 1\n\ndef total():\n return scale(1)\n", + ), + ]); + + assert_eq!(graph.callers("py/pkg/lib.py::scale"), Vec::::new()); + assert!(graph.callees_partial("py/pkg/app.py::total")); +} + +#[test] +fn go_ambiguous_local_calls_disclose_gaps_beside_bound_calls() { + let graph = sealed_graph_of(&[( + "go/units.go", + "package units\n\nfunc scale(x int) int { return x }\nfunc scale(x int) int { return x + 1 }\nfunc known(x int) int { return x * 2 }\nfunc total() int { return scale(1) + known(1) }\n", + )]); + + assert_eq!(graph.callers("go/units.go::known"), ["go/units.go::total"]); + assert!(graph.callers_partial("go/units.go::scale")); + assert!(graph.callees_partial("go/units.go::total")); +} + +#[test] +fn typescript_ambiguous_local_calls_do_not_poison_imported_call_resolution() { + let shadowed = "describe('scope', () => {\n function scale(x: number) { return x; }\n function scale(x: number) { return x + 1; }\n it('shadowed', () => { scale(1); });\n});\n"; + let clear = "export function clear(): number { return scale(1); }\n"; + for functions in [format!("{shadowed}{clear}"), format!("{clear}{shadowed}")] { + let app = format!("import {{ scale }} from './lib';\n{functions}"); + let graph = sealed_graph_of(&[ + ( + "ts/lib.ts", + "export function scale(x: number) { return x * 2; }\n", + ), + ("ts/app.ts", &app), + ]); + + assert_eq!(graph.callers("ts/lib.ts::scale"), ["ts/app.ts::clear"]); + assert!(graph.callees_partial("ts/app.ts::scope::shadowed")); + assert!(!graph.callees_partial("ts/app.ts::clear")); + } +}