diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs b/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs index 38afac2f6..c01692129 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs @@ -953,15 +953,18 @@ pub fn record_deleted_export_advisories(conn: &Connection, removed_files: &[Stri WHERE file = ?1 AND kind IN ('function', 'method', 'class') AND exported = 1 \ ORDER BY line", ); - // `e.kind` (not the source node's kind) is the discriminator: an + // `e.kind` (not the source node's kind) is the primary discriminator: an // `imports-type` edge is always sourced from the importing file's own // node by construction, while a `calls` edge is always a genuine call // even when `findCaller`'s TS/Rust mirror falls back to the file node as // source for a bare top-level call with no enclosing function/binding — // keying on source-node kind instead would misclassify that real call as - // a type-only import (Greptile, #1973). + // a type-only import (Greptile, #1973). `caller.kind` is ALSO selected + // to further split that file-sourced `calls` case into its own + // `'topLevelCall'` kind (#2365), since its `name`/`line` are the file + // node's own values, not a real caller symbol/call-site. let consumers_result = tx.prepare_cached( - "SELECT DISTINCT caller.name, caller.file, caller.line, e.kind \ + "SELECT DISTINCT caller.name, caller.file, caller.line, caller.kind, e.kind \ FROM edges e JOIN nodes caller ON e.source_id = caller.id \ WHERE e.target_id = ?1 AND e.kind IN ('calls', 'imports-type') AND caller.file != ?2", ); @@ -992,16 +995,26 @@ pub fn record_deleted_export_advisories(conn: &Connection, removed_files: &[Stri } let _ = delete_stmt.execute([file]); for (id, name, kind, line) in defs { - let consumers: Vec<(String, String, i64, String)> = match consumers_stmt + let consumers: Vec<(String, String, i64, String, String)> = match consumers_stmt .query_map(rusqlite::params![id, file], |row| { - Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?)) + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + )) }) { Ok(rows) => rows.flatten().collect(), Err(_) => continue, }; - for (consumer_name, consumer_file, consumer_line, edge_kind) in consumers { + for (consumer_name, consumer_file, consumer_line, caller_kind, edge_kind) in + consumers + { let consumer_kind = if edge_kind == "imports-type" { "file" + } else if caller_kind == "file" { + "topLevelCall" } else { "symbol" }; @@ -2112,6 +2125,49 @@ mod tests { ); } + /// Issue #2365: a genuine `calls` edge sourced from a FILE node + /// (findCaller's fallback for a bare top-level call with no enclosing + /// function/binding) must get its own `'topLevelCall'` consumer_kind, + /// distinct from `'symbol'` — `consumer_name`/`consumer_line` here are + /// the file node's own values, not a real caller symbol/call-site, so + /// lumping it in with a genuine named caller would let `codegraph check` + /// present a filename as if it were a calling function. + #[test] + fn record_deleted_export_advisories_discriminates_top_level_call_from_named_caller() { + let conn = test_conn_with_advisories(); + let target = insert_exported_node(&conn, "target", "function", "src/gone.js", 1); + let caller_fn = insert_node(&conn, "callerA", "function", "src/a.js", 1); + let caller_file = insert_node(&conn, "src/c.js", "file", "src/c.js", 0); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_fn, target], + ) + .unwrap(); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_file, target], + ) + .unwrap(); + + record_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + + let mut stmt = conn + .prepare("SELECT consumer_file, consumer_kind FROM deleted_export_advisories WHERE file = ?1 ORDER BY consumer_file") + .unwrap(); + let rows: Vec<(String, Option)> = stmt + .query_map(["src/gone.js"], |row| Ok((row.get(0)?, row.get(1)?))) + .unwrap() + .flatten() + .collect(); + assert_eq!( + rows, + vec![ + ("src/a.js".to_string(), Some("symbol".to_string())), + ("src/c.js".to_string(), Some("topLevelCall".to_string())), + ] + ); + } + #[test] fn record_deleted_export_advisories_skips_export_with_no_external_consumers() { let conn = test_conn_with_advisories(); diff --git a/src/db/repository/deleted-export-advisories.ts b/src/db/repository/deleted-export-advisories.ts index 25a79ac05..9848efe95 100644 --- a/src/db/repository/deleted-export-advisories.ts +++ b/src/db/repository/deleted-export-advisories.ts @@ -250,8 +250,10 @@ export function getDeletedExportAdvisories( line: row.consumer_line, // Rows persisted before migration v22 have consumer_kind = NULL — leave // consumerKind undefined for those rather than guessing, same as any - // other pre-#1973 advisory row (#1973). - ...(row.consumer_kind === 'file' || row.consumer_kind === 'symbol' + // other pre-#1973 advisory row (#1973). 'topLevelCall' added by #2365. + ...(row.consumer_kind === 'file' || + row.consumer_kind === 'symbol' || + row.consumer_kind === 'topLevelCall' ? { consumerKind: row.consumer_kind } : {}), }); diff --git a/src/db/repository/edges.ts b/src/db/repository/edges.ts index a9d75f65c..f35aac7aa 100644 --- a/src/db/repository/edges.ts +++ b/src/db/repository/edges.ts @@ -225,25 +225,30 @@ export function findExternalConsumers( const rows = cachedStmt( _findExternalConsumersStmt, db, - `SELECT DISTINCT caller.name, caller.file, caller.line, e.kind AS edgeKind + `SELECT DISTINCT caller.name, caller.file, caller.line, caller.kind AS callerKind, e.kind AS edgeKind FROM edges e JOIN nodes caller ON e.source_id = caller.id WHERE e.target_id = ? AND e.kind IN ('calls', 'imports-type') AND caller.file != ?`, - ).all(nodeId, file) as Array; - // `consumerKind` discriminates a real caller/constructor symbol (a genuine - // `calls` edge, with a real call-site line) from a whole-file reference - // such as `import type { X }` (an `imports-type` edge, always sourced from - // the importing file node itself — see emitNamedSymbolEdges). Keyed off the - // *edge* kind, not the source node's kind: findCaller falls back to the - // file node as a call's source for a genuine top-level call with no - // enclosing function/binding (e.g. a bare statement at module scope), so a - // `calls` edge can legitimately have a file-kind source too — using source - // kind alone would misclassify that real call as a type-only import - // (Greptile, #1973). Renderers must not treat `name`/`line` on a `'file'` - // entry as a caller symbol/call-site. - return rows.map(({ edgeKind, ...row }) => ({ + ).all(nodeId, file) as Array; + // `consumerKind` discriminates three cases. Keyed primarily off the *edge* + // kind, not the source node's kind: findCaller falls back to the file + // node as a call's source for a genuine top-level call with no enclosing + // function/binding (e.g. a bare statement at module scope), so a `calls` + // edge can legitimately have a file-kind source too — using source kind + // alone would misclassify that real call as a type-only import (Greptile, + // #1973). That file-sourced `calls` case gets its own `'topLevelCall'` + // kind (#2365) rather than being lumped in with `'symbol'`, since + // `name`/`line` there are the file node's own values, not a real caller + // symbol/call-site — renderers must not present either `'file'` or + // `'topLevelCall'` entries as if they were a named caller. + return rows.map(({ callerKind, edgeKind, ...row }) => ({ ...row, - consumerKind: edgeKind === 'imports-type' ? ('file' as const) : ('symbol' as const), + consumerKind: + edgeKind === 'imports-type' + ? ('file' as const) + : callerKind === 'file' + ? ('topLevelCall' as const) + : ('symbol' as const), })); } diff --git a/src/domain/analysis/exports.ts b/src/domain/analysis/exports.ts index 59799dca8..a56aca267 100644 --- a/src/domain/analysis/exports.ts +++ b/src/domain/analysis/exports.ts @@ -184,7 +184,7 @@ function exportsFileImpl( const consumersStmt = cachedStmt( _consumersStmtCache, db, - `SELECT n.name, n.file, n.line, e.kind AS edgeKind FROM edges e JOIN nodes n ON e.source_id = n.id + `SELECT n.name, n.file, n.line, n.kind AS callerKind, e.kind AS edgeKind FROM edges e JOIN nodes n ON e.source_id = n.id WHERE e.target_id = ? AND e.kind IN ('calls', 'imports-type')`, ); const reexportsFromStmt = cachedStmt( @@ -232,6 +232,7 @@ function exportsFileImpl( name: string; file: string; line: number; + callerKind: string; edgeKind: string; }>; if (noTests) consumers = consumers.filter((c) => !isTestFile(c.file)); @@ -244,24 +245,29 @@ function exportsFileImpl( role: s.role || null, signature: fileLines ? extractSignature(fileLines, s.line, displayOpts) : null, summary: fileLines ? extractSummary(fileLines, s.line, displayOpts) : null, - // `consumerKind` discriminates a real caller/constructor symbol (a - // genuine `calls` edge, with a real call-site line) from a - // whole-file reference such as `import type { X }` (an - // `imports-type` edge, always sourced from the importing file node - // itself — see emitNamedSymbolEdges). Keyed off the *edge* kind, - // not the source node's kind: findCaller falls back to the file - // node as a call's source for a genuine top-level call with no - // enclosing function/binding (e.g. a bare statement at module - // scope), so a `calls` edge can legitimately have a file-kind - // source too — using source kind alone would misclassify that real - // call as a type-only import (Greptile, #1973/#2189). Renderers - // must not treat `name`/`line` on a `'file'` entry as a caller - // symbol/call-site (#1830). + // `consumerKind` discriminates three cases. Keyed primarily off the + // *edge* kind, not the source node's kind: findCaller falls back to + // the file node as a call's source for a genuine top-level call + // with no enclosing function/binding (e.g. a bare statement at + // module scope), so a `calls` edge can legitimately have a + // file-kind source too — using source kind alone would misclassify + // that real call as a type-only import (Greptile, #1973/#2189). + // That file-sourced `calls` case gets its own `'topLevelCall'` kind + // (#2365) rather than being lumped in with `'symbol'`, since + // `name`/`line` there are the file node's own values, not a real + // caller symbol/call-site. Renderers must not present either + // `'file'` or `'topLevelCall'` entries as if they were a named + // caller (#1830). consumers: consumers.map((c) => ({ name: c.name, file: c.file, line: c.line, - consumerKind: c.edgeKind === 'imports-type' ? ('file' as const) : ('symbol' as const), + consumerKind: + c.edgeKind === 'imports-type' + ? ('file' as const) + : c.callerKind === 'file' + ? ('topLevelCall' as const) + : ('symbol' as const), })), consumerCount: consumers.length, }; diff --git a/src/features/check.ts b/src/features/check.ts index e544be867..3b1ae049d 100644 --- a/src/features/check.ts +++ b/src/features/check.ts @@ -565,7 +565,7 @@ interface ConsumerRef { file: string; line: number; /** See `ExternalConsumerRow.consumerKind` — absent for advisory-derived rows (#1973). */ - consumerKind?: 'file' | 'symbol'; + consumerKind?: 'file' | 'symbol' | 'topLevelCall'; } interface SignatureViolation { diff --git a/src/presentation/check.ts b/src/presentation/check.ts index f9a7598b3..2d2f2328e 100644 --- a/src/presentation/check.ts +++ b/src/presentation/check.ts @@ -29,7 +29,12 @@ interface CheckViolation { edgeKind?: string; /** Set when this violation comes from `checkNoDeletedExportsInUse` (#1806). */ reason?: string; - consumers?: Array<{ name: string; file: string; line: number; consumerKind?: 'file' | 'symbol' }>; + consumers?: Array<{ + name: string; + file: string; + line: number; + consumerKind?: 'file' | 'symbol' | 'topLevelCall'; + }>; } interface CheckPredicate { @@ -96,6 +101,7 @@ function formatPredicateViolations(pred: CheckPredicate): void { .map((c) => { if (c.consumerKind === 'file') return `${c.file} (type-only import)`; if (c.consumerKind === 'symbol') return `${c.file}:${c.line}`; + if (c.consumerKind === 'topLevelCall') return `${c.file} (top-level call)`; return `${c.file} (kind unknown — pre-existing advisory)`; }) .join(', '); diff --git a/src/presentation/queries-cli/exports.ts b/src/presentation/queries-cli/exports.ts index 177c17c3a..de915a734 100644 --- a/src/presentation/queries-cli/exports.ts +++ b/src/presentation/queries-cli/exports.ts @@ -10,8 +10,11 @@ interface ExportConsumer { * call-site). `'file'` — a whole-file reference such as * `import type { X }`, where `name` equals `file` and `line` is always * `0` because there is no specific call-site to report (#1830). + * `'topLevelCall'` — a genuine `calls` edge sourced from a bare top-level + * statement with no enclosing function/binding: `name`/`line` are the + * file node's own values, not a real caller symbol/call-site (#2365). */ - consumerKind: 'file' | 'symbol'; + consumerKind: 'file' | 'symbol' | 'topLevelCall'; } /** Render one consumer entry, without a fabricated call-site line for file-level entries. */ @@ -19,6 +22,9 @@ function formatConsumer(c: ExportConsumer): string { if (c.consumerKind === 'file') { return `${c.file} (type-only import)`; } + if (c.consumerKind === 'topLevelCall') { + return `${c.file} (top-level call)`; + } return `${c.name} (${c.file}:${c.line})`; } diff --git a/src/types.ts b/src/types.ts index 82c8e2a12..558d4c5be 100644 --- a/src/types.ts +++ b/src/types.ts @@ -232,19 +232,25 @@ export interface ExportedDefRow { * A cross-file consumer of an exported symbol (from findExternalConsumers), * or a persisted deleted-export advisory's consumer row (#1938). * - * `consumerKind` discriminates a real caller/constructor symbol (`name`/`line` - * are a genuine call-site) from a whole-file reference such as - * `import type { X}` (`name` equals `file`, `line` is always `0` because there - * is no specific call-site to report) — mirrors the same discriminator on - * exports' consumer rows (#1830). Optional because the persisted - * deleted-export-advisories snapshot (#1938) doesn't store this discriminator; - * only `findExternalConsumers`'s live-DB query populates it (#1973). + * `consumerKind` discriminates three cases: `'symbol'` — a real + * caller/constructor (`name`/`line` are a genuine call-site); `'file'` — a + * whole-file reference such as `import type { X }` (`name` equals `file`, + * `line` is always `0` because there is no specific call-site to report, + * #1830); `'topLevelCall'` — a genuine `calls` edge whose source is a bare + * top-level statement with no enclosing function/binding, so `findCaller` + * falls back to the FILE node itself as the edge's source (#2365) — `name` + * and `line` are the file node's own values (the file's basename, line `0`), + * not a real caller symbol/call-site, but this is still a genuine call + * (unlike `'file'`, which is never sourced from an actual `calls` edge). + * Optional because the persisted deleted-export-advisories snapshot (#1938) + * doesn't store this discriminator; only `findExternalConsumers`'s live-DB + * query populates it (#1973). */ export interface ExternalConsumerRow { name: string; file: string; line: number; - consumerKind?: 'file' | 'symbol'; + consumerKind?: 'file' | 'symbol' | 'topLevelCall'; } /** Import target/source row. */ @@ -2435,19 +2441,23 @@ export interface FileExportEntry { } /** - * A single caller of an exported symbol. `consumerKind` discriminates two + * A single caller of an exported symbol. `consumerKind` discriminates three * shapes that share this same struct: * - `'symbol'` — a real caller/constructor: `name` is the calling * function/method/class, `line` is the actual call-site line. * - `'file'` — a whole-file reference such as `import type { X }`, where * there is no specific calling symbol: `name` equals `file` and `line` * is always `0` (no real call-site exists to report; see #1830). + * - `'topLevelCall'` — a genuine `calls` edge sourced from a bare + * top-level statement with no enclosing function/binding: `findCaller` + * falls back to the file node itself, so `name`/`line` are the file + * node's own values, not a real caller symbol/call-site (#2365). */ export interface FileExportConsumer { name: string; file: string; line: number; - consumerKind: 'file' | 'symbol'; + consumerKind: 'file' | 'symbol' | 'topLevelCall'; } // ── Path ───────────────────────────────────────────────────────────── diff --git a/tests/integration/check.test.ts b/tests/integration/check.test.ts index 269fba039..b2724f9cc 100644 --- a/tests/integration/check.test.ts +++ b/tests/integration/check.test.ts @@ -979,7 +979,7 @@ describe('checkNoDeletedExportsInUse', () => { const violation = result.violations.find((v) => v.name === 'topLevelTarget'); expect(violation).toBeDefined(); expect(violation.consumers).toEqual([ - expect.objectContaining({ file: 'src/handler.js', consumerKind: 'symbol' }), + expect.objectContaining({ file: 'src/handler.js', consumerKind: 'topLevelCall' }), ]); }); }); diff --git a/tests/integration/exports.test.ts b/tests/integration/exports.test.ts index e32966292..64f22bb0b 100644 --- a/tests/integration/exports.test.ts +++ b/tests/integration/exports.test.ts @@ -337,7 +337,20 @@ describe('exportsData — import type consumer crediting (#1724)', () => { const target = data.results.find((r) => r.name === 'topLevelTarget'); expect(target).toBeDefined(); expect(target.consumers.length).toBe(1); - expect(target.consumers[0].consumerKind).toBe('symbol'); + expect(target.consumers[0].consumerKind).not.toBe('file'); + }); + + // Regression coverage for #2365: that same top-level-call consumer must + // get its OWN discriminator distinct from a real named caller — `name` + // and `line` here are the file node's own values (the file's basename, + // line 0), not a genuine call-site, so lumping it in with 'symbol' would + // let renderers present a filename as if it were a calling function. + test('a top-level call sourced from a file node is discriminated distinctly from a real named caller (#2365)', () => { + const data = exportsData('types.ts', dbPath2); + const target = data.results.find((r) => r.name === 'topLevelTarget'); + expect(target.consumers[0].consumerKind).toBe('topLevelCall'); + expect(target.consumers[0].name).toBe('consumer.ts'); + expect(target.consumers[0].line).toBe(0); }); test('interface consumed only via `import type` is excluded from --unused', () => { diff --git a/tests/presentation/queries-cli.test.ts b/tests/presentation/queries-cli.test.ts index 5c57e8c0f..d28b555e7 100644 --- a/tests/presentation/queries-cli.test.ts +++ b/tests/presentation/queries-cli.test.ts @@ -537,6 +537,34 @@ describe('fileExports', () => { expect(out).not.toContain('consumer.ts:0'); }); + it('renders a top-level-call consumer without presenting the file name as a caller symbol (#2365)', () => { + mocks.exportsData.mockReturnValue({ + file: 'helper.js', + totalExported: 1, + totalInternal: 0, + totalUnused: 0, + results: [ + { + name: 'run', + kind: 'function', + line: 1, + role: null, + signature: null, + consumers: [ + { name: 'consumer.js', file: 'consumer.js', line: 0, consumerKind: 'topLevelCall' }, + ], + }, + ], + reexportedSymbols: [], + reexports: [], + }); + fileExports('helper.js', '/db'); + const out = output(); + expect(out).toContain('consumer.js (top-level call)'); + // Must not render the file's own name/fabricated line as if it were a real caller symbol. + expect(out).not.toContain('consumer.js (consumer.js:0)'); + }); + it('renders barrel file header when no direct exports', () => { mocks.exportsData.mockReturnValue({ file: 'index.js',