fix(resolve): resolve Rust crate::/self::/super:: module paths to real files - #2215
fix(resolve): resolve Rust crate::/self::/super:: module paths to real files#2215carlos-alm merged 3 commits into
Conversation
Sorry, something went wrong.
…l files resolveImportPathJS (and the mirrored native resolve_import_path_inner) had no handling for Rust's use-path syntax: any non-relative import source fell straight through to workspace/exports resolution (npm-package semantics) and, finding no match, returned the literal unresolved string as if it were a resolved file path. This broke cross-file return-type propagation (Phase 8.2) for any bare function call whose target was imported via `use crate::...`, since the resolver never learned which file actually declared it. Adds crate::/self::/super:: resolution to both engines, walking the project's directory tree per Rust's module-file conventions (mod.rs/lib.rs/main.rs vs sibling foo/ directories), gated on the project's known-files list. Also fixes the native resolve_import_path singular entry point (and its resolve_import FFI binding), which unconditionally dropped known_files even when the caller had a real set available — making Rust path resolution structurally impossible on that code path regardless of the batch-path fix above. Verified against tests/benchmarks/resolution/fixtures/rust/: recall 23/24 (up from 20/24) on both engines, zero precision regression, full dual-engine parity maintained. The one remaining gap requires method-call return-type propagation through if-let pattern bindings, a distinct mechanism — filed as #2214. docs check acknowledged — bug fix to existing Rust support, no new language/feature/architecture change. Closes #2007 Impact: 12 functions changed, 33 affected
Greptile SummaryThe PR adds matching TypeScript and native Rust resolution for crate::, self::, and super:: paths and threads project file lists through several resolver entry points.
Confidence Score: 4/5The PR is not yet safe to merge because nested target-named module directories can resolve from the wrong crate root, while the previously reported incremental path still drops Rust imports. Cargo target detection relies only on the immediate parent directory name, so ordinary nested modules can be treated as independent crates; separately, incremental rebuild callers still omit the known-files set required to resolve Rust-qualified imports. Files Needing Attention: src/domain/graph/resolve.ts, crates/codegraph-core/src/domain/graph/resolve.rs, src/domain/graph/builder/incremental.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Rust use path"] --> B{"Build path"}
B -->|Full build| C["Pass allFiles"]
B -->|Watch rebuild| D["knownFiles omitted"]
C --> E["Resolve crate root and modules"]
D --> F["Return raw Rust path"]
E --> G["Create import and call edges"]
F --> H["File lookup misses; edges omitted"]
Reviews (3): Last reviewed commit: "docs(resolve): document Cargo.toml custo..." | Re-trigger Greptile |
Sorry, something went wrong.
| const knownFilesSet = toKnownFilesSet(knownFiles); | ||
| if (knownFilesSet) { | ||
| const rustResolved = resolveRustUsePath(fromFile, importSource, rootDir, knownFilesSet); | ||
| if (rustResolved) return rustResolved; | ||
| } |
There was a problem hiding this comment.
Incremental Rust imports remain unresolved
When a Rust file containing a crate::, self::, or super:: import is processed by the incremental rebuild path, its callers omit knownFiles, so this branch is skipped and the literal import path reaches the file-node lookup. That lookup fails, silently omitting import and dependent call edges that a full rebuild creates.
Knowledge Base Used: Graph Build Pipeline
Sorry, something went wrong.
| for (const name of ['main.rs', 'lib.rs']) { | ||
| const candidate = path.join(dir, name); | ||
| const rel = normalizePath(path.relative(rootDir, candidate)); | ||
| if (knownFiles.has(rel)) return candidate; | ||
| } |
There was a problem hiding this comment.
Crate-root discovery excludes Cargo targets
When a collected target such as src/bin/tool.rs, examples/example.rs, tests/integration.rs, or a custom Cargo target uses crate::, this search recognizes only ancestor files named main.rs or lib.rs. Root discovery therefore fails, leaving the raw import path unresolved and causing the corresponding import and call edges to be omitted.
Knowledge Base Used:
Sorry, something went wrong.
Codegraph Impact Analysis13 functions changed → 35 callers affected across 5 files
|
Sorry, something went wrong.
…ate root Greptile correctly flagged that crate-root discovery only recognized main.rs/lib.rs, so a use crate::... in a standalone Cargo target file (src/bin/foo.rs, examples/foo.rs, tests/foo.rs, benches/foo.rs) would walk up past its own directory and wrongly attribute crate:: to an unrelated src/main.rs or src/lib.rs elsewhere in the project — each such file compiles as its own independent crate and never shares a crate:: module tree with the package's main binary/library or with its sibling target files. Incremental-rebuild resolution still lacks known_files entirely (a separate, larger plumbing gap Greptile also flagged) — filed as #2216. docs check acknowledged — bug fix to the resolver added earlier in this same PR, no new language/feature/architecture change. Impact: 3 functions changed, 5 affected
|
Thanks for the careful review — both findings are valid. Issue 2 (Cargo target roots) — fixed in 278fd12. findRustCrateRoot/find_rust_crate_root now recognize standalone Cargo target files (src/bin/foo.rs, examples/foo.rs, tests/foo.rs, benches/foo.rs) as their own independent crate root, returning the file itself immediately instead of walking up and wrongly attributing crate:: to an unrelated src/main.rs/src/lib.rs elsewhere in the project (each such file compiles as its own crate and never shares a module tree with the package's main binary/library or its sibling targets). Same fix applied to rustParentModuleFile/rust_parent_module_file for super:: — these files have no parent module to walk up to. Added regression tests on both engines confirming this, and re-verified end-to-end against the fixture: still 23/24, zero regression. Issue 1 (incremental path) — filed as #2216, not fixed in this PR. You're right that incremental.ts's three resolveImportPath(...) call sites (and the Rust equivalents) never thread knownFiles/known_files through at all, so a Rust file rebuilt incrementally would silently miss these edges even though a full rebuild resolves them correctly. This is a materially larger, separately-scoped plumbing gap: unlike the full-build path (which already collects the project's file list in memory before import resolution runs), the incremental path has no in-memory project-wide file list at all — it would need a DB query (or a project-wide list threaded down from the watcher loop) added across several function signatures (rebuildFile, rebuildEdgesForTargetFile, buildImportEdges, emitEdgesForImport, runReverseDepCascade, buildImportedNamesMap, plus the Rust mirrors). Filing separately per repo scope-discipline rather than expanding this PR. |
Sorry, something went wrong.
Sorry, something went wrong.
Notes the resolver's other known convention-based-resolver boundary alongside the existing #[path] attribute-override note: a Cargo.toml [[bin]]/[[example]]/[[test]]/[[bench]] section declaring a target at a custom, non-conventional path isn't modeled by this resolver, since it needs Cargo.toml metadata this file never reads. Filed as #2217. docs check acknowledged — doc-comment addition only, no behavior change.
|
The custom-Cargo.toml-target-path case you raised is real and is the same class of limitation as the `#[path]` attribute override this resolver already documented as out of scope: a convention-based, known-files-only resolver can't know a target lives at a non-conventional path without parsing Cargo.toml metadata it never reads. That's a materially different mechanism (TOML parsing + a target-path map, not file-tree convention) from this PR's fix — filed as #2217, and added a doc-comment note in 38c7c5e alongside the existing `#[path]` note so this boundary is visible in the code, not just the tracker. On the incremental-path gap (#2216): still tracked separately and unchanged from my last reply — it's genuinely a larger, separately-scoped plumbing task (no in-memory project-wide file list exists on that path today, for any language, not just Rust), not something this PR's resolver-logic fix can reasonably absorb. To be clear about this PR's actual scope: it fixes `crate::`/`self::`/`super::` resolution for the full-build path (verified 23/24 on the real fixture, up from 20/24, on both engines with zero precision regression) and the conventional Cargo target layout. It was never scoped to close every convention-based-resolver edge case in one pass — Cargo.toml path overrides and incremental-path threading are both real, distinct follow-on features with their own tracked issues. |
Sorry, something went wrong.
Sorry, something went wrong.
| function isRustCargoTargetRoot(file: string): boolean { | ||
| const base = path.basename(file, '.rs'); | ||
| if (base === 'main' || base === 'lib' || base === 'mod') return false; | ||
| return CARGO_STANDALONE_TARGET_DIRS.has(path.basename(path.dirname(file))); |
There was a problem hiding this comment.
Nested modules become crate roots
When an ordinary Rust module is directly inside any nested directory named bin, examples, tests, or benches, isRustCargoTargetRoot classifies it as an independent Cargo target based only on the immediate parent name. This starts crate:: resolution from the wrong file and prevents super:: from ascending, causing import and call edges to be omitted or misdirected in both engines.
Knowledge Base Used: Rust Native Core and the Native/WASM Bridge
Sorry, something went wrong.
Summary
resolveImportPathJS (and the mirrored native resolve_import_path_inner) had no handling for Rust's use-path syntax. Any non-relative import source (which covers every Rust use path — Rust has no relative-import syntax) fell straight through workspace/exports resolution (npm-package semantics) and, finding no match, returned the literal unresolved string as if it were a resolved file path. This broke cross-file return-type propagation (Phase 8.2) for any bare function call whose target was imported via use crate::..., since the resolver never learned which file actually declared it.
Changes
Verification
Rebuilt both dist/ and the native .node addon, then verified end-to-end against tests/benchmarks/resolution/fixtures/rust/ on both engines directly (not just the threshold-gated benchmark test, which was already passing at the old 83.3% recall):
The one remaining gap (main -> User.display_name) needs method-call return-type propagation through if let Some(x) = ... pattern bindings — a distinct mechanism from import-path resolution. Filed as #2214.
Test plan
Closes #2007