diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 5a9292c7e..21d91a8c6 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -1700,6 +1700,10 @@ fn handle_export_declaration(node: &Node, decl: &Node, source: &[u8], symbols: & "class_declaration" | "abstract_class_declaration" => ("class", "name"), "interface_declaration" => ("interface", "name"), "type_alias_declaration" => ("type", "name"), + "lexical_declaration" | "variable_declaration" => { + collect_exported_var_declarations(node, decl, source, symbols); + return; + } _ => return, }; if let Some(n) = decl.child_by_field_name(field) { @@ -1711,6 +1715,55 @@ fn handle_export_declaration(node: &Node, decl: &Node, source: &[u8], symbols: & } } +/// Push `ExportInfo` entries for `export const/let/var …`, one per declarator. +/// +/// Named function/class/interface/type declarations carry their own `name` +/// field (handled above); a lexical/variable declaration doesn't, so each +/// declarator's value is classified the same way `handle_var_decl` classifies +/// it when creating the matching `Definition`: function-valued declarators +/// become kind "function", `const` declarators with a literal/array/object/ +/// new-expression value (per `is_js_literal`) become kind "constant". +/// Mirrors the WASM/TS extractor's `collectExportedDeclarations`. +/// +/// This predicate must stay identical to `handle_var_decl`'s: `insert_nodes.rs` +/// marks `exported = 1` by matching (name, kind, file, line) against +/// already-inserted definition rows, so a mismatched kind here silently +/// no-ops the UPDATE instead of marking the symbol exported (#1728). +fn collect_exported_var_declarations( + node: &Node, + decl: &Node, + source: &[u8], + symbols: &mut FileSymbols, +) { + let is_const = decl + .child(0) + .map(|c| node_text(&c, source) == "const") + .unwrap_or(false); + let line = start_line(node); + for i in 0..decl.child_count() { + let Some(declarator) = decl.child(i) else { continue }; + if declarator.kind() != "variable_declarator" { continue; } + let name_n = declarator.child_by_field_name("name"); + let value_n = declarator.child_by_field_name("value"); + let (Some(name_n), Some(value_n)) = (name_n, value_n) else { continue }; + if name_n.kind() != "identifier" { continue; } + let vt = value_n.kind(); + if vt == "arrow_function" || vt == "function_expression" || vt == "function" || vt == "generator_function" { + symbols.exports.push(ExportInfo { + name: node_text(&name_n, source).to_string(), + kind: "function".to_string(), + line, + }); + } else if is_const && is_js_literal(&value_n) { + symbols.exports.push(ExportInfo { + name: node_text(&name_n, source).to_string(), + kind: "constant".to_string(), + line, + }); + } + } +} + fn handle_reexport(node: &Node, source_node: &Node, source: &[u8], symbols: &mut FileSymbols) { let mod_path = node_text(source_node, source) .replace(&['\'', '"'][..], ""); diff --git a/src/db/index.ts b/src/db/index.ts index 65b81de9c..f08a37677 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -39,6 +39,7 @@ export { findCallers, findCrossFileCallTargets, findDistinctCallers, + findExportedNodesByFile, findFileNodes, findImplementors, findImportDependents, diff --git a/src/db/repository/index.ts b/src/db/repository/index.ts index a7011806a..5fd5d1ff9 100644 --- a/src/db/repository/index.ts +++ b/src/db/repository/index.ts @@ -35,6 +35,7 @@ export { countEdges, countFiles, countNodes, + findExportedNodesByFile, findFileNodes, findNodeById, findNodeByQualifiedName, diff --git a/src/db/repository/nodes.ts b/src/db/repository/nodes.ts index b9465383e..250c4d634 100644 --- a/src/db/repository/nodes.ts +++ b/src/db/repository/nodes.ts @@ -15,6 +15,7 @@ import type { } from '../../types.js'; import { buildFileConditionSQL, NodeQuery } from '../query-builder.js'; import { cachedStmt } from './cached-stmt.js'; +import { findCrossFileCallTargets } from './edges.js'; // ─── Query-builder based lookups (moved from src/db/repository.js) ───── @@ -179,6 +180,66 @@ export function findNodesByFile(db: BetterSqlite3Database, file: string): NodeRo ).all(file); } +/** Cache the schema probe for the `exported` column per db handle. */ +const _hasExportedColCache: WeakMap = new WeakMap(); + +const _findExportedNodesByFileStmt: StmtCache = new WeakMap(); + +/** + * Detect whether this DB's `nodes` table has the `exported` column populated + * (older DBs built before it existed fall back to a cross-file-calls + * heuristic). Cached per db handle to avoid re-probing on every call. + */ +function hasExportedColumn(db: BetterSqlite3Database): boolean { + if (_hasExportedColCache.has(db)) return _hasExportedColCache.get(db)!; + let has = false; + try { + db.prepare('SELECT exported FROM nodes LIMIT 0').raw(true); + has = true; + } catch { + /* older DB predates the exported column */ + } + _hasExportedColCache.set(db, has); + return has; +} + +/** + * Find nodes in `file` that are part of its exported surface. + * + * Prefers the `exported = 1` column — set by the extractor at build time from + * the file's actual `export` statements, the authoritative "was this + * declared as exported" signal — and only falls back to + * `findCrossFileCallTargets` (symbols targeted by a cross-file `calls` edge) + * for DBs built before the column existed. + * + * Shared by `codegraph exports` and `codegraph where --file` so both commands + * agree on what "exported" means; before this was extracted, `where --file` + * used the cross-file-calls heuristic unconditionally, which misses any + * exported symbol that's only ever read as a value (not called) from another + * file — e.g. an exported constant used in a comparison (#1728). + * + * `knownSymbols`, if passed, is used in place of a fresh `findNodesByFile` + * call for the legacy-DB fallback path — callers that already fetched the + * file's symbols (e.g. to build their own result shape) can pass them in to + * avoid running the same query twice. + */ +export function findExportedNodesByFile( + db: BetterSqlite3Database, + file: string, + knownSymbols?: NodeRow[], +): NodeRow[] { + if (hasExportedColumn(db)) { + return cachedStmt( + _findExportedNodesByFileStmt, + db, + "SELECT * FROM nodes WHERE file = ? AND kind != 'file' AND exported = 1 ORDER BY line", + ).all(file); + } + const symbols = knownSymbols ?? findNodesByFile(db, file); + const exportedIds = findCrossFileCallTargets(db, file); + return symbols.filter((s) => exportedIds.has(s.id)); +} + /** * Find file-kind nodes matching a LIKE pattern. */ diff --git a/src/domain/analysis/exports.ts b/src/domain/analysis/exports.ts index 3c9710df6..110279aad 100644 --- a/src/domain/analysis/exports.ts +++ b/src/domain/analysis/exports.ts @@ -1,12 +1,11 @@ import path from 'node:path'; import { - findCrossFileCallTargets, findDbPath, + findExportedNodesByFile, findFileNodes, findNodesByFile, } from '../../db/index.js'; import { cachedStmt } from '../../db/repository/cached-stmt.js'; -import { debug } from '../../infrastructure/logger.js'; import { isTestFile } from '../../infrastructure/test-filter.js'; import { createFileLinesReader, @@ -17,10 +16,6 @@ import { paginateResult } from '../../shared/paginate.js'; import type { BetterSqlite3Database, NodeRow, StmtCache } from '../../types.js'; import { resolveAnalysisOpts, withReadonlyDb } from './query-helpers.js'; -/** Cache the schema probe for the `exported` column per db handle. */ -const _hasExportedColCache: WeakMap = new WeakMap(); - -const _exportedNodesStmtCache: StmtCache = new WeakMap(); const _consumersStmtCache: StmtCache<{ name: string; file: string; line: number }> = new WeakMap(); const _reexportsFromStmtCache: StmtCache<{ file: string }> = new WeakMap(); const _reexportsToStmtCache: StmtCache<{ file: string }> = new WeakMap(); @@ -104,8 +99,6 @@ function collectReexportedSymbols( db: BetterSqlite3Database, fileNodeId: number, reexportsToStmt: ReturnType, - exportedNodesStmt: ReturnType | null, - hasExportedCol: boolean, getFileLines: (file: string) => string[] | null, buildSymbolResult: (s: NodeRow, fileLines: string[] | null) => any, ) { @@ -113,14 +106,7 @@ function collectReexportedSymbols( const reexportedSymbols: Array & { originFile: string }> = []; for (const reexTarget of reexportTargets) { - let targetExported: NodeRow[]; - if (hasExportedCol) { - targetExported = exportedNodesStmt!.all(reexTarget.file) as NodeRow[]; - } else { - const targetSymbols = findNodesByFile(db, reexTarget.file) as NodeRow[]; - const exportedIds = findCrossFileCallTargets(db, reexTarget.file) as Set; - targetExported = targetSymbols.filter((s) => exportedIds.has(s.id)); - } + const targetExported = findExportedNodesByFile(db, reexTarget.file); for (const s of targetExported) { reexportedSymbols.push({ ...buildSymbolResult(s, getFileLines(reexTarget.file)), @@ -142,28 +128,6 @@ function exportsFileImpl( const fileNodes = findFileNodes(db, `%${target}%`) as NodeRow[]; if (fileNodes.length === 0) return []; - // Detect whether exported column exists (cached per db handle) - let hasExportedCol: boolean; - if (_hasExportedColCache.has(db)) { - hasExportedCol = _hasExportedColCache.get(db)!; - } else { - hasExportedCol = false; - try { - db.prepare('SELECT exported FROM nodes LIMIT 0').raw(true); - hasExportedCol = true; - } catch (e: unknown) { - debug(`exported column not available, using fallback: ${(e as Error).message}`); - } - _hasExportedColCache.set(db, hasExportedCol); - } - - const exportedNodesStmt = hasExportedCol - ? cachedStmt( - _exportedNodesStmtCache, - db, - "SELECT * FROM nodes WHERE file = ? AND kind != 'file' AND exported = 1 ORDER BY line", - ) - : null; // Consumers include real call/construct edges plus `imports-type` edges — // the symbol-level edge emitted for `import type { X }` statements (source // is the importing *file* node, since the import statement references the @@ -199,15 +163,7 @@ function exportsFileImpl( return fileNodes.map((fn) => { const symbols = findNodesByFile(db, fn.file) as NodeRow[]; - let exported: NodeRow[]; - if (hasExportedCol) { - // Use the exported column populated during build - exported = exportedNodesStmt!.all(fn.file) as NodeRow[]; - } else { - // Fallback: symbols that have incoming calls from other files - const exportedIds = findCrossFileCallTargets(db, fn.file) as Set; - exported = symbols.filter((s) => exportedIds.has(s.id)); - } + const exported = findExportedNodesByFile(db, fn.file, symbols); const internalCount = symbols.length - exported.length; const buildSymbolResult = (s: NodeRow, fileLines: string[] | null) => { @@ -244,8 +200,6 @@ function exportsFileImpl( db, fn.id, reexportsToStmt, - exportedNodesStmt, - hasExportedCol, getFileLines, buildSymbolResult, ); diff --git a/src/domain/analysis/symbol-lookup.ts b/src/domain/analysis/symbol-lookup.ts index 065c11f4e..ed19e853f 100644 --- a/src/domain/analysis/symbol-lookup.ts +++ b/src/domain/analysis/symbol-lookup.ts @@ -3,7 +3,7 @@ import { findAllIncomingEdges, findAllOutgoingEdges, findCallers, - findCrossFileCallTargets, + findExportedNodesByFile, findFileNodes, findImportSources, findImportTargets, @@ -177,9 +177,7 @@ function whereFileImpl(db: BetterSqlite3Database, target: string) { const importedBy = (findImportSources(db, fn.id) as ImportEdgeRow[]).map((r) => r.file); - const exportedIds = findCrossFileCallTargets(db, fn.file) as Set; - - const exported = symbols.filter((s) => exportedIds.has(s.id)).map((s) => s.name); + const exported = findExportedNodesByFile(db, fn.file, symbols).map((s) => s.name); return { file: fn.file, diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 4d189fa4c..7c4b1ea9e 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -195,6 +195,64 @@ function handleMethodCapture(c: Record, definitions: Def }); } +/** Node types whose own `name` field is the exported symbol's name. */ +const EXPORT_DECL_KIND: Record = { + function_declaration: 'function', + generator_function_declaration: 'function', + class_declaration: 'class', + abstract_class_declaration: 'class', + interface_declaration: 'interface', + type_alias_declaration: 'type', +}; + +/** + * Push Export entries for the declaration wrapped by an `export` statement. + * Shared by both extraction paths (query-based `handleExportCapture` and + * walk-based `handleExportStmt`) so they can't drift apart on what counts as + * an export — see the "two code paths" gotcha for this extractor. + * + * Named function/class/interface/type declarations carry their own `name` + * field. `export const/let/var …` has no such field — each declarator's value + * is classified the same way `handleVariableDeclarator` classifies it when + * building the matching Definition (function-valued → kind 'function', + * literal/array/object/new-expression-valued `const` → kind 'constant'). + * This predicate must stay identical to the Definition-building one: the + * exported=1 UPDATE it feeds matches DB rows by (name, kind, file, line), so + * a mismatched kind silently no-ops instead of marking the symbol exported (#1728). + */ +function collectExportedDeclarations( + decl: TreeSitterNode, + exportLine: number, + exps: Export[], +): void { + const kind = EXPORT_DECL_KIND[decl.type]; + if (kind) { + const n = decl.childForFieldName('name'); + if (n) exps.push({ name: n.text, kind: kind as Export['kind'], line: exportLine }); + return; + } + if (decl.type !== 'lexical_declaration' && decl.type !== 'variable_declaration') return; + const isConst = decl.text.startsWith('const '); + for (let i = 0; i < decl.childCount; i++) { + const declarator = decl.child(i); + if (declarator?.type !== 'variable_declarator') continue; + const nameN = declarator.childForFieldName('name'); + const valueN = declarator.childForFieldName('value'); + if (nameN?.type !== 'identifier' || !valueN) continue; + const valType = valueN.type; + if ( + valType === 'arrow_function' || + valType === 'function_expression' || + valType === 'function' || + valType === 'generator_function' + ) { + exps.push({ name: nameN.text, kind: 'function', line: exportLine }); + } else if (isConst && isConstantValue(valueN)) { + exps.push({ name: nameN.text, kind: 'constant', line: exportLine }); + } + } +} + /** Handle export_statement capture. */ function handleExportCapture( c: Record, @@ -203,22 +261,7 @@ function handleExportCapture( ): void { const exportLine = nodeStartLine(c.exp_node!); const decl = c.exp_node!.childForFieldName('declaration'); - if (decl) { - const declType = decl.type; - const kindMap: Record = { - function_declaration: 'function', - generator_function_declaration: 'function', - class_declaration: 'class', - abstract_class_declaration: 'class', - interface_declaration: 'interface', - type_alias_declaration: 'type', - }; - const kind = kindMap[declType]; - if (kind) { - const n = decl.childForFieldName('name'); - if (n) exps.push({ name: n.text, kind: kind as Export['kind'], line: exportLine }); - } - } + if (decl) collectExportedDeclarations(decl, exportLine, exps); const source = c.exp_node!.childForFieldName('source') || findChild(c.exp_node!, 'string'); if (source && !decl) { const modPath = source.text.replace(/['"]/g, ''); @@ -1448,22 +1491,7 @@ function handleImportStmt(node: TreeSitterNode, ctx: ExtractorOutput): void { function handleExportStmt(node: TreeSitterNode, ctx: ExtractorOutput): void { const exportLine = nodeStartLine(node); const decl = node.childForFieldName('declaration'); - if (decl) { - const declType = decl.type; - const kindMap: Record = { - function_declaration: 'function', - generator_function_declaration: 'function', - class_declaration: 'class', - abstract_class_declaration: 'class', - interface_declaration: 'interface', - type_alias_declaration: 'type', - }; - const kind = kindMap[declType]; - if (kind) { - const n = decl.childForFieldName('name'); - if (n) ctx.exports.push({ name: n.text, kind: kind as Export['kind'], line: exportLine }); - } - } + if (decl) collectExportedDeclarations(decl, exportLine, ctx.exports); const source = node.childForFieldName('source') || findChild(node, 'string'); if (source && !decl) { const modPath = source.text.replace(/['"]/g, ''); diff --git a/tests/engines/parity.test.ts b/tests/engines/parity.test.ts index 679207e79..f9c5b5c4d 100644 --- a/tests/engines/parity.test.ts +++ b/tests/engines/parity.test.ts @@ -144,6 +144,28 @@ add(1, 2); const MAX_RETRIES = 3; const APP_NAME = "codegraph"; const add = (a, b) => a + b; +`, + }, + { + // Regression guard for #1728: `export const …` must be recognized as an + // export in both engines regardless of the initializer's shape — a bare + // literal, a `new Expr(...)` call, and an arrow function must all + // produce a matching exports entry, and a non-exported const must not. + // + // The object-literal-with-methods shape (e.g. `export const command = { + // execute() {} }`, issue #1728's other repro) is deliberately not + // included here: both engines already agree it's exported (see + // tests/parsers/javascript.test.ts and tests/engines/query-walk-parity.test.ts), + // but they emit its qualified/unqualified method definitions in different + // relative array order — a separate, pre-existing, content-neutral gap + // (see #1818) that this test's unsorted `normalize()` would otherwise trip on. + name: 'JavaScript — exported constants of varying initializer shapes', + file: 'exported-const.js', + code: ` +export const MAX_WALK_DEPTH = 200; +export const PUNCTUATION_TOKENS = new Set([',', ';']); +export const add = (a, b) => a + b; +const INTERNAL = 'not exported'; `, }, { diff --git a/tests/engines/query-walk-parity.test.ts b/tests/engines/query-walk-parity.test.ts index 2b1f3e787..37f660264 100644 --- a/tests/engines/query-walk-parity.test.ts +++ b/tests/engines/query-walk-parity.test.ts @@ -170,6 +170,24 @@ export function serve(port) { listen(port); } export class Server { start() { this.init(); } } +`, + }, + { + // Regression guard for #1728: `export const …` must be recognized as an + // export regardless of the initializer's shape (bare literal, new-expression, + // object-literal-with-methods, arrow function) — both extraction paths must + // agree on the resulting (name, kind, line) exports entries. + name: 'exported const declarations of varying initializer shapes', + file: 'test.js', + code: ` +export const MAX_WALK_DEPTH = 200; +export const PUNCTUATION_TOKENS = new Set([',', ';']); +export const add = (a, b) => a + b; +export const command = { + name: 'info', + execute(args, opts, ctx) {}, +}; +const INTERNAL = 'not exported'; `, }, { diff --git a/tests/integration/queries.test.ts b/tests/integration/queries.test.ts index c76d4ac86..38672ced0 100644 --- a/tests/integration/queries.test.ts +++ b/tests/integration/queries.test.ts @@ -84,6 +84,14 @@ beforeAll(() => { const formatResponse = insertNode(db, 'formatResponse', 'function', 'utils.js', 1); const preAuthenticate = insertNode(db, 'preAuthenticate', 'function', 'utils.js', 10); + // Exported constant with zero incoming edges of any kind — regression fixture + // for #1728: "exported" must reflect the DB's exported=1 column (set by the + // extractor from the source's `export` keyword), not "has an incoming + // cross-file `calls` edge." A value-only export like this is never the + // target of a calls edge, so a cross-file-calls heuristic incorrectly + // omitted it from `where --file`'s exported list. + const apiVersion = insertNode(db, 'API_VERSION', 'constant', 'middleware.js', 1); + // Test file + function const fTest = insertNode(db, 'auth.test.js', 'file', 'auth.test.js', 0); const testAuth = insertNode(db, 'testAuthenticate', 'function', 'auth.test.js', 5); @@ -125,7 +133,14 @@ beforeAll(() => { // Mark exported symbols (those declared as exports in source) const markExported = db.prepare('UPDATE nodes SET exported = 1 WHERE id = ?'); - for (const id of [authenticate, validateToken, authMiddleware, formatResponse, preAuthenticate]) { + for (const id of [ + authenticate, + validateToken, + authMiddleware, + formatResponse, + preAuthenticate, + apiVersion, + ]) { markExported.run(id); } @@ -767,8 +782,14 @@ describe('whereData', () => { test('file: exported list', () => { const data = whereData('middleware.js', dbPath, { file: true }); const r = data.results[0]; - // authMiddleware is called from routes.js (cross-file) + // authMiddleware has exported=1 in the DB (and, incidentally, is also + // called from routes.js — but presence in `exported` must not depend on + // that; see the next assertion). expect(r.exported).toContain('authMiddleware'); + // API_VERSION has exported=1 but no incoming edges of any kind (#1728): + // "exported" reflects the DB's exported=1 column, not "has an incoming + // cross-file `calls` edge" — a value-only export is never call-targeted. + expect(r.exported).toContain('API_VERSION'); }); test('file: empty for unknown', () => { diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index b31040e2a..c825df2a9 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -1482,4 +1482,67 @@ describe('JavaScript parser', () => { ); }); }); + + describe('export-list detection for `export const/let/var …` (#1728)', () => { + it('lists named exported function/class declarations (refactor regression guard)', () => { + const symbols = parseJS(`export function greet() {}\nexport class Widget {}`); + expect(symbols.exports).toContainEqual( + expect.objectContaining({ name: 'greet', kind: 'function' }), + ); + expect(symbols.exports).toContainEqual( + expect.objectContaining({ name: 'Widget', kind: 'class' }), + ); + }); + + it('lists an exported const with a bare numeric-literal initializer (repro 1)', () => { + const symbols = parseJS(`export const MAX_WALK_DEPTH = 200;`); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'MAX_WALK_DEPTH', kind: 'constant', line: 1 }), + ); + expect(symbols.exports).toContainEqual( + expect.objectContaining({ name: 'MAX_WALK_DEPTH', kind: 'constant', line: 1 }), + ); + }); + + it('lists an exported const initialized with new Set(...) (sibling regression guard)', () => { + const symbols = parseJS(`export const PUNCTUATION_TOKENS = new Set([',', ';']);`); + expect(symbols.exports).toContainEqual( + expect.objectContaining({ name: 'PUNCTUATION_TOKENS', kind: 'constant', line: 1 }), + ); + }); + + it('lists an exported object-literal-with-methods const, without independently exporting its methods (repro 2)', () => { + const symbols = parseJS( + `export const command = {\n name: 'info',\n execute(args, opts, ctx) {},\n};`, + ); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'command', kind: 'constant', line: 1 }), + ); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'command.execute', kind: 'function' }), + ); + expect(symbols.exports).toContainEqual( + expect.objectContaining({ name: 'command', kind: 'constant', line: 1 }), + ); + // Qualified child methods aren't independently listed as exports — mirrors + // how `Foo.method` isn't exported when only `export class Foo` is (only the + // top-level declared name is; see the class-method exported=0 convention). + expect(symbols.exports.some((e) => e.name === 'command.execute')).toBe(false); + }); + + it('lists an exported arrow-function const with kind "function"', () => { + const symbols = parseJS(`export const add = (a, b) => a + b;`); + expect(symbols.exports).toContainEqual( + expect.objectContaining({ name: 'add', kind: 'function', line: 1 }), + ); + }); + + it('does not list a non-exported const', () => { + const symbols = parseJS(`const INTERNAL = 42;`); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'INTERNAL', kind: 'constant' }), + ); + expect(symbols.exports.some((e) => e.name === 'INTERNAL')).toBe(false); + }); + }); });