diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ceebc1d8..5dd5f7ed2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The call paths `codegraph context` prints now say what bridges each dynamic-dispatch hop, in the same words `codegraph_explore` uses: a call through an interface reads `interface → impl`, and a C++ override, a Redux thunk or a Gin middleware chain is named as one. Before, every hop of a kind it didn't know was labeled an "event", so a call through an interface looked like an event being emitted. - `codegraph_explore` and `codegraph context` now include some of the subtypes and implementors of a class or interface you ask about, and the type hierarchy the library returns for one (`getTypeHierarchy`) now lists all of them. Before, a bug in the hierarchy walk left every type without subtypes, so only what it extends or implements came back. Thanks @jocax for the report and @danusha2345 for the fix. (#2833) - In Rust, code that uses an enum only through its variants, like `mode::Mode::A` in an expression, a `Mode::B =>` match arm or `Self::A` inside the enum's own `impl`, now shows up in that enum's callers and impact, linked to the enum that actually declares the variant rather than to a same-named type elsewhere. Standard-library variants like `Ordering::Less` or `Option::Some` and associated items like `Foo::new()` or `Foo::MAX` don't count as using a project enum. Re-index Rust projects after upgrading. Thanks @mg-mg-mg for the report and @danusha2345. (#2328) +- In Rust, a call written through a module path, like `crate::util::take(3)`, `super::util::take(3)` or `util::take(3)`, and a `use` of a function, like `use crate::util::take;`, now link to the function that module declares. Before, they could land on a method of the same name declared above it in that file, like an `impl Buf`'s `take`, or on a function written inside another function, and a call through a module the standard library provides, like `fs::read(…)` under `use std::fs;`, could link to your own `fs` module's `read`. Re-index Rust projects after upgrading. Thanks @danusha2345. (#2308) - Axum routes now link to their handler when rustfmt wraps the `.route(` call so the handler sits on a later line, and when the handler's name also appears elsewhere on the line, as in `delete(handlers::user::delete)` or `.route("/login", post(handlers::user::login))`; Actix's `web::resource(…)` and `.route(…)` registrations get the same fix. A call inside a closure handler is no longer mistaken for a route, and an Actix resource no longer takes the next route's handler as its own. Thanks @mg-mg-mg for the report and @danusha2345. (#2326) - In Play projects, a route or a call like `MediaType.parse(...)` now links to a method of the class or object it names, and a call to an overloaded one to the overload its arguments fit. Before, it went to the first method of that name anywhere in the class's file, often a nested or sibling class's. When a Scala and a Java type share a name, as Play's own `RequestAuthority` does, each language's calls now reach their own. Thanks @danusha2345. - In JavaScript and TypeScript, the functions written inside a named object literal are now symbols of their own even when the object isn't exported: a plain `const api = { load() {…} }`, an object declared inside an IIFE or a function, and a namespace hung on the page or on another object, like `window.App = { init() {…} }`, `App.utils = {…}` or `dw_page = {…}`. Calls such as `api.load()`, `window.App.init()`, `App.utils.pad()`, a sibling's `this.render()`, and `App.init()` from another script on the page now reach them, the calls made inside a member belong to that member instead of the object, and a bare `init()` is no longer taken for `App.init`, so script-tag apps no longer lose most of their code from `callers` and impact. A member's qualified name now carries its object, exported or not (`api::load`, `window.App::init`), so asking for `App.init` finds it. Re-index JavaScript and TypeScript projects after upgrading. Thanks @tkhoaaa for the report and @danusha2345. (#2300) diff --git a/__tests__/rust-module-path-call.test.ts b/__tests__/rust-module-path-call.test.ts new file mode 100644 index 000000000..e822516df --- /dev/null +++ b/__tests__/rust-module-path-call.test.ts @@ -0,0 +1,223 @@ +/** + * A Rust call written through a module path — `crate::util::take(3)`, + * `super::util::take(3)`, `self::util::take(3)`, `util::take(3)` after + * `use crate::util;` — is the free function that module declares, and so is + * a `use crate::util::take;`. A path never names a method (that takes + * `Type::method`), nor a function local to another function's body, even when + * the module's file declares one of the same name above the free function. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rust-module-path-')); + const files: Record = { + 'Cargo.toml': '[package]\nname = "app"\nversion = "0.1.0"\n', + 'src/lib.rs': `mod util; +mod only; +mod a; +mod caller; +mod importer; +mod handle; +mod worker; +mod twice; +mod inline_reexport; +mod fs; +mod uses_std; + +pub fn via_self() -> usize { + self::util::take(1) +} +`, + // A namesake in an inline module, before the module's own item. + 'src/twice.rs': `#[cfg(test)] +mod tests { + pub fn pick() -> usize { + 0 + } +} + +pub fn pick() -> usize { + 1 +} + +pub fn via_twice() -> usize { + crate::twice::pick() +} +`, + // The item lives in an inline module the file passes on with a glob. + 'src/inline_reexport.rs': `#[cfg(unix)] +mod imp { + pub fn lone() {} +} +pub use imp::*; + +pub fn via_inline() { + crate::inline_reexport::lone(); +} +`, + // A project module named like a module of the standard library. + 'src/fs/mod.rs': 'pub fn read(path: &str) -> usize {\n path.len()\n}\n', + 'src/uses_std.rs': `use std::fs; + +pub fn loads() { + let _ = fs::read("config.toml"); +} +`, + 'src/util.rs': `pub struct Buf; + +impl Buf { + pub fn take(&self, n: usize) -> usize { + n + } +} + +pub trait Take { + fn take(&self) -> usize; +} + +fn outer() -> usize { + fn take() -> usize { + 0 + } + take() +} + +pub fn take(n: usize) -> usize { + n +} +`, + 'src/only.rs': `pub struct Only; + +impl Only { + pub fn grab(&self) -> usize { + 0 + } +} +`, + 'src/a/mod.rs': 'pub mod b;\n', + 'src/a/b.rs': `pub struct Deep; + +impl Deep { + pub fn get(&self) -> usize { + 0 + } +} + +pub fn get() -> usize { + 1 +} +`, + 'src/importer.rs': 'use crate::util::take;\n', + 'src/handle.rs': 'pub struct Handle;\n', + 'src/worker.rs': `pub use crate::handle::Handle; +mod dump; + +pub trait Lock { + type Handle; +} + +pub struct Guard; + +impl Lock for Guard { + type Handle = u8; +} +`, + 'src/worker/dump.rs': 'use super::Handle;\n', + 'src/caller.rs': `use crate::util; + +pub fn via_crate() -> usize { + crate::util::take(1) +} + +pub fn via_super() -> usize { + super::util::take(1) +} + +pub fn via_module() -> usize { + util::take(1) +} + +pub fn via_nested() -> usize { + crate::a::b::get() +} + +pub fn via_method_only() -> usize { + crate::only::grab() +} + +pub fn via_missing() -> usize { + crate::nowhere::take(1) +} +`, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** What the function `name` in `file` calls, as `kind qualifiedName file:line`. */ +const callsFrom = (file: string, name: string): string[] => { + const fn = cg.getNodesInFile(file).find((n) => n.name === name)!; + return cg.getOutgoingEdgesFrom([fn.id]).filter((e) => e.kind === 'calls') + .map((e) => cg.getNode(e.target)!).map((t) => `${t.kind} ${t.qualifiedName} ${t.filePath}:${t.startLine}`); +}; + +describe('Rust calls through a module path', () => { + it('reach the free function the module declares, never a same-named method or local fn', () => { + const take = 'function take src/util.rs:20'; + expect(callsFrom('src/caller.rs', 'via_crate')).toEqual([take]); + expect(callsFrom('src/caller.rs', 'via_super')).toEqual([take]); + expect(callsFrom('src/caller.rs', 'via_module')).toEqual([take]); + expect(callsFrom('src/lib.rs', 'via_self')).toEqual([take]); + }); + + it('bind a use of the function to it', () => { + const ids = cg.getNodesInFile('src/importer.rs').map((n) => n.id); + const targets = cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind === 'imports') + .map((e) => cg.getNode(e.target)!).map((t) => `${t.kind} ${t.qualifiedName} ${t.filePath}:${t.startLine}`); + expect(targets).toEqual(['function take src/util.rs:20']); + }); + + it('never bind a use to an associated type of the module\'s impl or trait', () => { + const ids = cg.getNodesInFile('src/worker/dump.rs').map((n) => n.id); + const targets = cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind === 'imports') + .map((e) => cg.getNode(e.target)!).map((t) => `${t.kind} ${t.qualifiedName} ${t.filePath}:${t.startLine}`); + expect(targets.filter((t) => t.includes('src/worker.rs'))).toEqual([]); + }); + + it('walk a nested module path to its file', () => { + expect(callsFrom('src/caller.rs', 'via_nested')).toEqual(['function get src/a/b.rs:9']); + }); + + it('link nothing when the module declares no such free function, or is not in the index', () => { + expect(callsFrom('src/caller.rs', 'via_method_only')).toEqual([]); + expect(callsFrom('src/caller.rs', 'via_missing')).toEqual([]); + }); + + it('prefer the module\'s own item to a namesake in a module it declares inline', () => { + expect(callsFrom('src/twice.rs', 'via_twice')).toEqual(['function pick src/twice.rs:8']); + }); + + it('reach an inline module\'s item the module passes on, from the module\'s own file', () => { + expect(callsFrom('src/inline_reexport.rs', 'via_inline')).toEqual(['function lone src/inline_reexport.rs:3']); + }); + + it('leave a path through a module a `use` takes from the standard library to it', () => { + // `fs::read` under `use std::fs;` is std's, not `src/fs/mod.rs`'s `read`. + expect(callsFrom('src/uses_std.rs', 'loads')).toEqual([]); + }); +}); diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 58a571da9..20f783683 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -33,6 +33,8 @@ import { isPythonFromImportModule, isRustAssociatedItem, tsGlobalScope, + isLexicallyReachable, + rustItemPlacement, } from './name-matcher'; import { JS_ARRAY_METHODS } from './js-builtins'; @@ -3694,11 +3696,18 @@ function resolveRustPathReference( const modSegs = segments.slice(0, -1); const file = resolveRustModuleFile(modSegs, ref.filePath, context); - if (!file || file === ref.filePath) return null; - - // A module's item, never an impl's associated one: serde's `use - // crate::de::Error` is not `de/mod.rs`'s `impl SeqAccess`'s `type Error`. - const target = context.getNodesInFile(file).find( + if (!file) return null; + + // The module's own item. A path never names a method (that is + // `Type::method`), nor a fn local to another fn's body: the file's first + // same-named node was taken, so an `impl Buf { fn take }` above + // `pub fn take` caught every `crate::util::take(…)`. Two module-level items + // of one name are `#[cfg]` variants of it; the first one stands for both. + // With none, the path reaches an inline module's item through a re-export + // (`#[cfg(…)] mod imp { … }` + `pub use imp::*;`), never an associated item + // of an `impl` or `trait` body: serde's `use crate::de::Error` is not + // `de/mod.rs`'s `impl SeqAccess`'s `type Error`. + const candidates = context.getNodesInFile(file).filter( (n) => n.name === leaf && !isRustAssociatedItem(n) && @@ -3709,14 +3718,22 @@ function resolveRustPathReference( n.kind === 'trait' || n.kind === 'type_alias' || n.kind === 'constant' || - n.kind === 'method' || n.kind === 'class' || - n.kind === 'interface') + n.kind === 'interface') && + isLexicallyReachable(n, ref, context) ); - if (target) { - return { original: ref, targetNodeId: target.id, confidence: 0.9, resolvedBy: 'import' }; - } - return null; + const target = + candidates.find((n) => rustItemPlacement(n, context) === 'top') ?? + candidates.find((n) => rustItemPlacement(n, context) === 'inline'); + if (!target) return null; + // A path whose head a `use` binds from the standard library — `fs::read` + // under `use std::fs;`, `error::Error` under `use std::{error, io};` — is + // std's, whatever the module the project has of that name: `fs/mod.rs`'s own + // `read` is not `std::fs::read`. (Asked only here, where it changes the + // answer: the check reads the file's `use`s.) + const head = modSegs[0]!; + if (!/^(?:crate|self|super)$/.test(head) && isBoundToOutOfRepoImport({ ...ref, referenceName: head }, context)) return null; + return { original: ref, targetNodeId: target.id, confidence: 0.9, resolvedBy: 'import' }; } /** The crate-root directory (holds `lib.rs`/`main.rs`), walking up from a file. */ diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index e93d4ed7a..224c4b118 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -8994,6 +8994,21 @@ export function isRustAssociatedItem(n: Node): boolean { return n.language === 'rust' && RUST_ASSOCIATED_KINDS.has(n.kind) && n.qualifiedName.includes('::'); } +/** + * Where a Rust item sits in its file's module, as a path through the module + * reaches it: written at the file's top level ('top'), or in a module the + * file declares inline (`mod imp { … }`, 'inline'), which adds nothing to its + * qualified name. Null for an item a module path never names: a method, an + * `impl`'s or a trait's associated item, an item of a function body — each + * qualified by its owner. + */ +export function rustItemPlacement(n: Node, context: ResolutionContext): 'top' | 'inline' | null { + if (n.language !== 'rust' || n.qualifiedName !== n.name) return null; + const layout = rustUsesOf(n.filePath, context); + const at = (layout.lineStarts[n.startLine - 1] ?? 0) + Math.max(0, n.startColumn ?? 0); + return rustBlockAt(layout.blocks, at) === null ? 'top' : 'inline'; +} + /** Does one of the file's globs bring in this candidate's module (or, for `use super::*`, its parent's)? */ function rustGlobCovers(uses: RustUses, candidate: Node, ref: UnresolvedRef): boolean { if (uses.globs.has(rustModuleName(candidate.filePath))) return true;