Skip to content

Commit 58295d2

Browse files
bartollimasonjames
andcommitted
fix(resolution): dangling relative imports are negative evidence
- `RelativeImportLookup { Bound, NoIndexedFile, Unknown }`: `resolve_relative_import` reports `NoIndexedFile` only when every accepted path is absent from a complete index view, no same-directory file shares the first-dot stem, and the path is not an indexed directory; anything else is `Unknown` - `ImportOrigin::Dangling`: the TypeScript and JavaScript builders register a `NoIndexedFile` binding as dangling, install nothing into scope, and skip the module-string fallback - shared resolver: a receiver-less row of any kind whose name is owned by a dangling binding fails closed before the name-keyed tiers; the file's own definitions and the inheritance witness still resolve; receiver-bearing rows unchanged - `PipelineSymbolCache::{file_presence, sibling_stem_present, directory_present}` answer from file rows; only a cache built from every persisted row (`from_index` over the uncapped `for_each_file_path`) answers `Absent`; walk-scoped caches answer `Unknown` - boundary: a file governed by a tsconfig or jsconfig with `moduleSuffixes` or `rootDirs` gets no negative evidence (`ResolutionRules::relative_specifiers_redirected`; provider cache `RESOLUTION_INDEX_VERSION` 1.1) - preservation goldens under `tests/fixtures/dangling_import/` captured from a `71b6cdb` release build; `EMISSION_SEMANTICS_VERSION` unchanged, `codanna index --force` applies the change to an existing index Co-authored-by: Mason James <297610+masonjames@users.noreply.github.com>
1 parent c965e49 commit 58295d2

58 files changed

Lines changed: 2786 additions & 49 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/indexing/facade.rs‎

Lines changed: 582 additions & 0 deletions
Large diffs are not rendered by default.

‎src/indexing/pipeline/stages/resolve.rs‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::indexing::pipeline::types::{
1919
CallerContext, ResolutionContext, ResolvedBatch, ResolvedRelationship, SymbolLookupCache,
2020
UnresolvedRelationship,
2121
};
22-
use crate::parsing::resolution::{GenericInheritanceResolver, InheritanceResolver};
22+
use crate::parsing::resolution::{GenericInheritanceResolver, ImportOrigin, InheritanceResolver};
2323
use crate::parsing::{Import, LanguageBehavior, LanguageId};
2424
use crate::types::{FileId, SymbolId};
2525
use crate::{RelationKind, Symbol};
@@ -226,6 +226,21 @@ impl ResolveStage {
226226
}
227227
}
228228

229+
// A dangling import owns its name: the builder proved the target
230+
// file absent from the index, so a same-name symbol anywhere else
231+
// is not this reference's target. Receiver-less rows of every kind
232+
// fail closed here, after the file's own definitions (scope hit)
233+
// and the inheritance witness, before any name-keyed tier.
234+
// Receiver-bearing rows carry their own evidence and keep it.
235+
if Self::is_receiver_less(unresolved)
236+
&& context
237+
.scope
238+
.import_binding(&unresolved.to_name)
239+
.is_some_and(|binding| binding.origin == ImportOrigin::Dangling)
240+
{
241+
return None;
242+
}
243+
229244
// For qualified static calls (`Type::method` / `Type.method`), the
230245
// tier-based `cache.resolve()` returns the local same-name candidate
231246
// before consulting any non-local match. Bypass tier logic and filter
@@ -955,6 +970,14 @@ impl ResolveStage {
955970
})
956971
}
957972

973+
/// A row of any kind whose emitted metadata names no receiver.
974+
fn is_receiver_less(unresolved: &UnresolvedRelationship) -> bool {
975+
unresolved
976+
.metadata
977+
.as_ref()
978+
.is_none_or(|m| m.receiver.is_none())
979+
}
980+
958981
/// A Calls row with no receiver metadata and no static flag.
959982
fn is_bare_instance_call(unresolved: &UnresolvedRelationship) -> bool {
960983
unresolved.kind == RelationKind::Calls

‎src/indexing/pipeline/types.rs‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! Key design principle: Parse stage produces "raw" types without IDs,
55
//! Collect stage assigns IDs and produces final types.
66
7+
use crate::parsing::resolution::FilePresence;
78
use crate::parsing::{Import, LanguageId, PipelineSymbolCache, ResolveResult};
89
use crate::relationship::RelationshipMetadata;
910
use crate::symbol::ScopeContext;
@@ -462,6 +463,40 @@ pub struct SymbolLookupCache {
462463
/// Re-exported paths: "pkg.helper" -> the symbol defined at "pkg.a.helper"
463464
/// when pkg's namespace imports it. Populated by the Phase 2 pre-pass.
464465
module_aliases: dashmap::DashMap<Box<str>, crate::types::SymbolId>,
466+
/// Every file row in the index, in the decoded path form symbols
467+
/// carry. `Some` only for a cache built from the persisted index:
468+
/// a walk-scoped cache sees its own run's files and cannot prove a
469+
/// path absent.
470+
indexed_files: Option<IndexedFileView>,
471+
}
472+
473+
/// Every file row in the index, projected three ways for the negative
474+
/// evidence a dangling relative import needs: the paths themselves, the
475+
/// `(directory, first-dot stem)` pairs extension-substituted siblings
476+
/// share, and every ancestor directory.
477+
#[derive(Debug, Default, PartialEq, Eq)]
478+
pub(crate) struct IndexedFileView {
479+
paths: std::collections::HashSet<PathBuf>,
480+
stems: std::collections::HashSet<(PathBuf, String)>,
481+
dirs: std::collections::HashSet<PathBuf>,
482+
}
483+
484+
impl IndexedFileView {
485+
fn from_paths(files: impl IntoIterator<Item = PathBuf>) -> Self {
486+
let mut view = Self::default();
487+
for path in files {
488+
if let Some(stem) = crate::parsing::paths::dir_and_first_dot_stem(&path) {
489+
view.stems.insert(stem);
490+
}
491+
for dir in path.ancestors().skip(1) {
492+
if !dir.as_os_str().is_empty() {
493+
view.dirs.insert(dir.to_path_buf());
494+
}
495+
}
496+
view.paths.insert(path);
497+
}
498+
view
499+
}
465500
}
466501

467502
impl Default for SymbolLookupCache {
@@ -478,6 +513,7 @@ impl SymbolLookupCache {
478513
by_name: dashmap::DashMap::new(),
479514
by_file_id: dashmap::DashMap::new(),
480515
module_aliases: dashmap::DashMap::new(),
516+
indexed_files: None,
481517
}
482518
}
483519

@@ -488,9 +524,24 @@ impl SymbolLookupCache {
488524
by_name: dashmap::DashMap::with_capacity(symbols / 10), // Fewer unique names
489525
by_file_id: dashmap::DashMap::with_capacity(symbols / 50), // ~50 symbols/file avg
490526
module_aliases: dashmap::DashMap::new(),
527+
indexed_files: None,
491528
}
492529
}
493530

531+
/// Create an empty cache that knows the complete set of indexed file
532+
/// paths, so the presence queries can answer `Absent`.
533+
pub fn with_indexed_files(files: impl IntoIterator<Item = PathBuf>) -> Self {
534+
let mut cache = Self::new();
535+
cache.indexed_files = Some(IndexedFileView::from_paths(files));
536+
cache
537+
}
538+
539+
/// The complete file view, when this cache was built from the index.
540+
#[cfg(test)]
541+
pub(crate) fn indexed_files(&self) -> Option<&IndexedFileView> {
542+
self.indexed_files.as_ref()
543+
}
544+
494545
/// Insert a symbol into the cache.
495546
pub fn insert(&self, symbol: crate::Symbol) {
496547
let id = symbol.id;
@@ -875,6 +926,32 @@ impl PipelineSymbolCache for SymbolLookupCache {
875926
fn lookup_candidates(&self, name: &str) -> Vec<SymbolId> {
876927
SymbolLookupCache::lookup_candidates(self, name)
877928
}
929+
930+
fn file_presence(&self, path: &std::path::Path) -> FilePresence {
931+
match &self.indexed_files {
932+
None => FilePresence::Unknown,
933+
Some(view) if view.paths.contains(path) => FilePresence::Present,
934+
Some(_) => FilePresence::Absent,
935+
}
936+
}
937+
938+
fn sibling_stem_present(&self, path: &std::path::Path) -> FilePresence {
939+
let Some(view) = &self.indexed_files else {
940+
return FilePresence::Unknown;
941+
};
942+
match crate::parsing::paths::dir_and_first_dot_stem(path) {
943+
Some(stem) if view.stems.contains(&stem) => FilePresence::Present,
944+
_ => FilePresence::Absent,
945+
}
946+
}
947+
948+
fn directory_present(&self, path: &std::path::Path) -> FilePresence {
949+
match &self.indexed_files {
950+
None => FilePresence::Unknown,
951+
Some(view) if view.dirs.contains(path) => FilePresence::Present,
952+
Some(_) => FilePresence::Absent,
953+
}
954+
}
878955
}
879956

880957
impl SymbolLookupCache {
@@ -890,7 +967,21 @@ impl SymbolLookupCache {
890967
let count = index
891968
.count_symbols()
892969
.map_err(|e| PipelineError::Index(crate::IndexError::Storage(e)))?;
893-
let cache = Self::with_capacity(count);
970+
let mut cache = Self::with_capacity(count);
971+
972+
// File rows decode through the same portable-path boundary as the
973+
// symbols they are compared against. The visitor is uncapped, so
974+
// the view is complete at every index size.
975+
let mut files: Vec<PathBuf> = Vec::new();
976+
index.for_each_file_path(|stored| -> Result<(), PipelineError> {
977+
files.push(match index.to_portable_file_path(&stored) {
978+
Some(portable) => PathBuf::from(portable),
979+
None => PathBuf::from(stored),
980+
});
981+
Ok(())
982+
})?;
983+
cache.indexed_files = Some(IndexedFileView::from_paths(files));
984+
894985
if count == 0 {
895986
return Ok(cache);
896987
}

‎src/parsing/javascript/behavior.rs‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use crate::parsing::LanguageBehavior;
44
use crate::parsing::behavior_state::{BehaviorState, StatefulBehavior};
55
use crate::parsing::paths::strip_extension;
6-
use crate::parsing::resolution::{InheritanceResolver, ResolutionScope};
6+
use crate::parsing::resolution::{InheritanceResolver, RelativeImportLookup, ResolutionScope};
77
use crate::project_resolver::persist::ResolutionPersistence;
88
use crate::types::FileId;
99
use crate::{SymbolId, Visibility};
@@ -343,9 +343,27 @@ impl LanguageBehavior for JavaScriptBehavior {
343343
// Path-domain arm first: relative specifiers resolve by file
344344
// identity (trait default). Module-string normalization cannot
345345
// represent the navigation when stems contain dots.
346-
let file_resolved = importing_file.as_deref().and_then(|f| {
347-
self.resolve_relative_import(cache, lookup_name, &import.path, f, extensions)
348-
});
346+
let lookup = importing_file
347+
.as_deref()
348+
.map_or(RelativeImportLookup::Unknown, |f| {
349+
self.resolve_relative_import(cache, lookup_name, &import.path, f, extensions)
350+
});
351+
// Boundary stated at the TypeScript builder: configured
352+
// redirection withholds negative evidence.
353+
let lookup = if lookup == RelativeImportLookup::NoIndexedFile
354+
&& maybe_enhancer
355+
.as_ref()
356+
.is_some_and(|e| e.redirects_relative_specifiers())
357+
{
358+
RelativeImportLookup::Unknown
359+
} else {
360+
lookup
361+
};
362+
let (file_resolved, dangling) = match lookup {
363+
RelativeImportLookup::Bound(id) => (Some(id), false),
364+
RelativeImportLookup::NoIndexedFile => (None, true),
365+
RelativeImportLookup::Unknown => (None, false),
366+
};
349367
let file_resolved_module = file_resolved
350368
.and_then(|id| cache.get(id))
351369
.and_then(|s| s.module_path.map(String::from));
@@ -389,10 +407,12 @@ impl LanguageBehavior for JavaScriptBehavior {
389407
// module_path. Exact match wins outright; segment-boundary
390408
// suffix matches bind only an exactly-one survivor (candidate
391409
// order is file-processing order, not identity; raw ends_with
392-
// also admitted mid-segment captures).
410+
// also admitted mid-segment captures). A dangling import's
411+
// target file is provably absent, so no module-string match
412+
// can be it.
393413
let mut resolved_symbol: Option<SymbolId> = file_resolved;
394414
let mut suffix_matches: Vec<SymbolId> = Vec::new();
395-
if resolved_symbol.is_none() {
415+
if resolved_symbol.is_none() && !dangling {
396416
for id in cache.lookup_candidates(lookup_name) {
397417
if let Some(symbol) = cache.get(id) {
398418
if let Some(module) = symbol.module_path.as_deref() {
@@ -417,7 +437,9 @@ impl LanguageBehavior for JavaScriptBehavior {
417437
}
418438

419439
// Determine origin
420-
let origin = if resolved_symbol.is_some() {
440+
let origin = if dangling {
441+
ImportOrigin::Dangling
442+
} else if resolved_symbol.is_some() {
421443
ImportOrigin::Internal
422444
} else {
423445
ImportOrigin::External

‎src/parsing/javascript/jsconfig.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ pub struct CompilerOptions {
4444
/// Path mapping for module resolution
4545
#[serde(default)]
4646
pub paths: HashMap<String, Vec<String>>,
47+
48+
/// `moduleSuffixes`: a relative specifier may resolve to a suffixed
49+
/// sibling (`./dep` naming `dep_native.ts`)
50+
#[serde(default, rename = "moduleSuffixes")]
51+
pub moduleSuffixes: Option<Vec<String>>,
52+
53+
/// `rootDirs`: a relative specifier may resolve across the listed roots
54+
#[serde(default, rename = "rootDirs")]
55+
pub rootDirs: Option<Vec<String>>,
4756
}
4857

4958
/// Minimal jsconfig.json representation for path resolution
@@ -153,6 +162,14 @@ fn merge_jsconfig(parent: JsConfig, child: JsConfig) -> JsConfig {
153162
merged.extend(child.compilerOptions.paths);
154163
merged
155164
},
165+
moduleSuffixes: child
166+
.compilerOptions
167+
.moduleSuffixes
168+
.or(parent.compilerOptions.moduleSuffixes),
169+
rootDirs: child
170+
.compilerOptions
171+
.rootDirs
172+
.or(parent.compilerOptions.rootDirs),
156173
},
157174
}
158175
}
@@ -349,6 +366,9 @@ mod tests {
349366
("@parent/*".to_string(), vec!["parent/*".to_string()]),
350367
("@common/*".to_string(), vec!["parent/common/*".to_string()]),
351368
]),
369+
370+
moduleSuffixes: None,
371+
rootDirs: None,
352372
},
353373
};
354374

@@ -360,6 +380,9 @@ mod tests {
360380
("@child/*".to_string(), vec!["child/*".to_string()]),
361381
("@common/*".to_string(), vec!["child/common/*".to_string()]),
362382
]),
383+
384+
moduleSuffixes: None,
385+
rootDirs: None,
363386
},
364387
};
365388

@@ -407,6 +430,9 @@ mod tests {
407430
compilerOptions: CompilerOptions {
408431
baseUrl: Some("./src".to_string()),
409432
paths: HashMap::from([("@/*".to_string(), vec!["*".to_string()])]),
433+
434+
moduleSuffixes: None,
435+
rootDirs: None,
410436
},
411437
};
412438

‎src/parsing/javascript/resolution.rs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,13 +594,16 @@ impl InheritanceResolver for JavaScriptInheritanceResolver {
594594
/// Applies jsconfig.json path mappings to transform import paths.
595595
/// Mirrors TypeScript's TypeScriptProjectEnhancer architecture.
596596
pub struct JavaScriptProjectEnhancer {
597+
/// The governing config redirects relative specifiers
598+
redirects_relative_specifiers: bool,
597599
/// Compiled path alias resolver (built from the resolution rules)
598600
resolver: Option<crate::parsing::javascript::jsconfig::PathAliasResolver>,
599601
}
600602

601603
impl JavaScriptProjectEnhancer {
602604
/// Create a new enhancer from resolution rules
603605
pub fn new(rules: crate::project_resolver::persist::ResolutionRules) -> Self {
606+
let redirects_relative_specifiers = rules.relative_specifiers_redirected;
604607
// Build the PathAliasResolver from rules
605608
let resolver = if !rules.paths.is_empty() || rules.base_url.is_some() {
606609
// Create a minimal JsConfig to use from_jsconfig
@@ -609,6 +612,9 @@ impl JavaScriptProjectEnhancer {
609612
compilerOptions: crate::parsing::javascript::jsconfig::CompilerOptions {
610613
baseUrl: rules.base_url.clone(),
611614
paths: rules.paths.clone(),
615+
616+
moduleSuffixes: None,
617+
rootDirs: None,
612618
},
613619
};
614620

@@ -618,7 +624,16 @@ impl JavaScriptProjectEnhancer {
618624
None
619625
};
620626

621-
Self { resolver }
627+
Self {
628+
redirects_relative_specifiers,
629+
resolver,
630+
}
631+
}
632+
633+
/// Whether the governing config redirects relative specifiers
634+
/// (`moduleSuffixes`, `rootDirs`).
635+
pub fn redirects_relative_specifiers(&self) -> bool {
636+
self.redirects_relative_specifiers
622637
}
623638
}
624639

0 commit comments

Comments
 (0)