diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 21d91a8c6..5c343e820 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -1896,7 +1896,14 @@ fn walk_ast_nodes_depth(node: &Node, source: &[u8], ast_nodes: &mut Vec } return; } - "string" | "template_string" => { + // Guard on `is_named()`: tree-sitter-typescript's `predefined_type` + // production (the `string`/`number`/`boolean`/... primitive type + // keywords) lexes its keyword as an anonymous token whose `kind()` + // string is identical to the *named* `string` literal node type. + // Without this guard, `name: string` type annotations are + // misclassified as string-literal ast_nodes (#1729). Mirrors the + // WASM-side guard in `ast-store-visitor.ts::resolveAstKind`. + "string" | "template_string" if node.is_named() => { let raw = node_text(node, source); // Strip quotes to get content let content = raw diff --git a/src/ast-analysis/engine.ts b/src/ast-analysis/engine.ts index 6edc65413..706df0161 100644 --- a/src/ast-analysis/engine.ts +++ b/src/ast-analysis/engine.ts @@ -45,6 +45,7 @@ import { computeLOCMetrics, computeMaintainabilityIndex } from './metrics.js'; import { AST_STRING_CONFIGS, AST_TYPE_MAPS, + astRequiresNamedNode, astStopRecurseKinds, CFG_RULES, COMPLEXITY_RULES, @@ -475,6 +476,7 @@ function setupAstVisitor( nodeIdMap, stringConfig, astStopRecurseKinds(langId), + astRequiresNamedNode(langId), ); } diff --git a/src/ast-analysis/rules/index.ts b/src/ast-analysis/rules/index.ts index 01a599eb4..298ce6fb0 100644 --- a/src/ast-analysis/rules/index.ts +++ b/src/ast-analysis/rules/index.ts @@ -378,3 +378,23 @@ export function astStopRecurseKinds(langId: string): ReadonlySet { } return EMPTY_STOP_RECURSE; } + +// ─── Per-language "named-node-required" guard ──────────────────────────── +// +// tree-sitter-typescript's `predefined_type` production (the `string`, +// `number`, `boolean`, `void`, ... primitive type keywords) lexes its +// keyword as an anonymous grammar token whose `type` string is identical to +// the *named* `string` node type used for real string literals — the +// grammar's own node-types.json lists both a `"string", named: true` entry +// (the literal) and a `"string", named: false` entry (the keyword token). +// Matching by `node.type` alone therefore misclassifies `name: string` +// type annotations as string-literal ast_nodes (#1729). Mirrors the native +// `is_named()` guard in `extractors/javascript.rs::walk_ast_nodes_depth`. +// +// Scoped to the JS family: no other bundled grammar currently maps an +// AST_TYPE_MAPS key that collides with an anonymous token of the same name. +const JS_REQUIRES_NAMED_NODE: ReadonlySet = new Set(['javascript', 'typescript', 'tsx']); + +export function astRequiresNamedNode(langId: string): boolean { + return JS_REQUIRES_NAMED_NODE.has(langId); +} diff --git a/src/ast-analysis/visitors/ast-store-visitor.ts b/src/ast-analysis/visitors/ast-store-visitor.ts index dd63515be..10d5ead93 100644 --- a/src/ast-analysis/visitors/ast-store-visitor.ts +++ b/src/ast-analysis/visitors/ast-store-visitor.ts @@ -273,9 +273,28 @@ function collectNode(ctx: CollectCtx, node: TreeSitterNode, kind: string): void * Object() function), which then crashes the worker boundary with * "function Object() { [native code] } could not be cloned" when the * resulting astNodes row is structured-cloned back to the main thread. + * + * When `requireNamedNode` is set, anonymous `string`/`template_string` + * grammar tokens are rejected even though their `type` string matches a + * mapped key — e.g. TypeScript's `predefined_type` keyword (`string`, + * `number`, ...) lexes as an unnamed token whose type collides with the + * named `string` literal node (#1729). Scoped to just those two node types + * (not every mapped kind) to mirror the native `is_named()` guard, which + * only appears on the `"string" | "template_string"` match arm in + * `extractors/javascript.rs::walk_ast_nodes_depth` — every other mapped kind + * (call, new, throw, await, regex) is always a named production, so gating + * them too would silently diverge from the native engine's behavior the + * moment that stops being true. */ -function resolveAstKind(node: TreeSitterNode, astTypeMap: Record): string | null { +const NAMED_NODE_REQUIRED_TYPES: ReadonlySet = new Set(['string', 'template_string']); + +function resolveAstKind( + node: TreeSitterNode, + astTypeMap: Record, + requireNamedNode: boolean, +): string | null { if (!Object.hasOwn(astTypeMap, node.type)) return null; + if (requireNamedNode && !node.isNamed && NAMED_NODE_REQUIRED_TYPES.has(node.type)) return null; return astTypeMap[node.type] || null; } @@ -286,6 +305,7 @@ export function createAstStoreVisitor( nodeIdMap: Map, stringConfig: AstStringConfig = DEFAULT_STRING_CONFIG, stopRecurseKinds: ReadonlySet = new Set(), + requireNamedNode = false, ): Visitor { const newTypes = newTypesFor(astTypeMap); // When nodeIdMap is empty, parentNodeId resolution is wasted work — the @@ -311,7 +331,7 @@ export function createAstStoreVisitor( // unrelated subtree. The parent call's skipChildren handles the intended case. if (ctx.matched.has(node.id)) return; - const kind = resolveAstKind(node, astTypeMap); + const kind = resolveAstKind(node, astTypeMap, requireNamedNode); if (!kind) return; collectNode(ctx, node, kind); diff --git a/src/domain/wasm-worker-entry.ts b/src/domain/wasm-worker-entry.ts index d55f5f7e6..2e02b4ed8 100644 --- a/src/domain/wasm-worker-entry.ts +++ b/src/domain/wasm-worker-entry.ts @@ -30,6 +30,7 @@ import { computeLOCMetrics, computeMaintainabilityIndex } from '../ast-analysis/ import { AST_STRING_CONFIGS, AST_TYPE_MAPS, + astRequiresNamedNode, astStopRecurseKinds, CFG_RULES, COMPLEXITY_RULES, @@ -606,6 +607,7 @@ function buildAstVisitor( new Map(), stringConfig, astStopRecurseKinds(langId), + astRequiresNamedNode(langId), ); } diff --git a/src/features/ast.ts b/src/features/ast.ts index da9aff195..d227b4bcc 100644 --- a/src/features/ast.ts +++ b/src/features/ast.ts @@ -2,6 +2,7 @@ import path from 'node:path'; import { AST_STRING_CONFIGS, AST_TYPE_MAPS, + astRequiresNamedNode, astStopRecurseKinds, } from '../ast-analysis/rules/index.js'; import { buildExtensionSet } from '../ast-analysis/shared.js'; @@ -244,6 +245,7 @@ function walkAst( nodeIdMap, stringConfig, astStopRecurseKinds(langId), + astRequiresNamedNode(langId), ); const results = walkWithVisitors(rootNode, [visitor], langId); const collected = (results['ast-store'] || []) as AstRow[]; diff --git a/src/types.ts b/src/types.ts index 8374a2a0c..81fafbe03 100644 --- a/src/types.ts +++ b/src/types.ts @@ -777,6 +777,16 @@ export interface TreeSitterNode { id: number; type: string; text: string; + /** + * Whether this node is a named grammar production vs. an anonymous token + * (keyword/punctuation). Tree-sitter grammars can define an anonymous + * token whose text matches a *named* node's type string — e.g. + * tree-sitter-typescript's `predefined_type` wraps an anonymous `string` + * keyword token that collides with the named `string` literal node type + * (#1729). Consumers matching by `type` alone must also check `isNamed` + * to avoid misclassifying keyword tokens as literal/expression nodes. + */ + isNamed: boolean; startPosition: { row: number; column: number }; endPosition: { row: number; column: number }; childCount: number; diff --git a/tests/engines/ast-parity.test.ts b/tests/engines/ast-parity.test.ts index a73c5852f..4e4e50192 100644 --- a/tests/engines/ast-parity.test.ts +++ b/tests/engines/ast-parity.test.ts @@ -11,6 +11,7 @@ import { beforeAll, describe, expect, it } from 'vitest'; import { AST_STRING_CONFIGS, AST_TYPE_MAPS, + astRequiresNamedNode, astStopRecurseKinds, } from '../../src/ast-analysis/rules/index.js'; import { walkWithVisitors } from '../../src/ast-analysis/visitor.js'; @@ -420,6 +421,7 @@ let run () = new Map(), stringConfig, astStopRecurseKinds(langId), + astRequiresNamedNode(langId), ); const results = walkWithVisitors(tree.rootNode as any, [visitor], langId); const rows = (results['ast-store'] || []) as unknown[]; diff --git a/tests/parsers/ast-nodes.test.ts b/tests/parsers/ast-nodes.test.ts index 4ebd79f7d..d0a0da2ad 100644 --- a/tests/parsers/ast-nodes.test.ts +++ b/tests/parsers/ast-nodes.test.ts @@ -182,6 +182,168 @@ describe('buildAstNodes — JS extraction', () => { }); }); +// ─── TypeScript fixture (#1729: predefined_type keyword false-positives) ── +// +// tree-sitter-typescript's `predefined_type` production (the `string`, +// `number`, `boolean`, ... primitive type keywords) lexes its keyword as an +// anonymous token whose `type` string collides with the *named* `string` +// literal node type. `--kind string` must only match the latter. + +const TS_FIXTURE_CODE = ` +interface Person { + name: string; + age: number; +} + +type UserId = "user-id-literal"; + +function greet(name: string): string { + return \`Hello, \${name}!\`; +} + +function processItems(items: string[]): void { + console.log(items); +} + +import { helper } from './helper.js'; + +const greeting = "hello world"; +`; + +describe('buildAstNodes — TypeScript extraction (#1729)', () => { + let tsTmpDir: string, tsDb: any; + + function queryTsAstNodes(kind: string) { + return tsDb.prepare('SELECT * FROM ast_nodes WHERE kind = ? ORDER BY line').all(kind); + } + + beforeAll(async () => { + tsTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-ast-ts-extract-')); + const srcDir = path.join(tsTmpDir, 'src'); + fs.mkdirSync(srcDir, { recursive: true }); + fs.mkdirSync(path.join(tsTmpDir, '.codegraph')); + + const fixturePath = path.join(srcDir, 'fixture.ts'); + fs.writeFileSync(fixturePath, TS_FIXTURE_CODE); + + const allSymbols = await parseFilesAuto([fixturePath], tsTmpDir, { engine: 'wasm' }); + const symbols = allSymbols.get('src/fixture.ts'); + if (!symbols) throw new Error('Failed to parse TS fixture file'); + + const dbPath = path.join(tsTmpDir, '.codegraph', 'graph.db'); + tsDb = new Database(dbPath); + tsDb.pragma('journal_mode = WAL'); + initSchema(tsDb); + + const insertNode = tsDb.prepare( + 'INSERT INTO nodes (name, kind, file, line, end_line) VALUES (?, ?, ?, ?, ?)', + ); + for (const def of symbols.definitions) { + insertNode.run(def.name, def.kind, 'src/fixture.ts', def.line, def.endLine); + } + + await buildAstNodes(tsDb, allSymbols, tsTmpDir); + }); + + afterAll(() => { + if (tsDb) tsDb.close(); + fs.rmSync(tsTmpDir, { recursive: true, force: true }); + }); + + test('does not misclassify interface field type annotation as kind:string', () => { + // `name: string;` (line 3) must never surface as a bare, unquoted "string" + // row — genuine literals are always quoted in `text` (e.g. "hello world"). + const nodes = queryTsAstNodes('string'); + expect(nodes.some((n) => n.line === 3)).toBe(false); + }); + + test('does not misclassify parameter or return type annotations as kind:string', () => { + // `greet(name: string): string` (param + return type, line 9) must not + // contribute bare "string" rows. + const nodes = queryTsAstNodes('string'); + expect(nodes.some((n) => n.line === 9)).toBe(false); + }); + + test('does not misclassify array-of-primitive parameter type as kind:string', () => { + // `processItems(items: string[])` (line 13) wraps predefined_type in + // array_type — must not contribute a bare "string" row either. + const nodes = queryTsAstNodes('string'); + expect(nodes.some((n) => n.line === 13)).toBe(false); + }); + + test('still captures genuine string literals, template literals, and string-literal types', () => { + const nodes = queryTsAstNodes('string'); + const names = nodes.map((n) => n.name); + expect(names).toContain('user-id-literal'); // type UserId = "user-id-literal" + expect(names).toContain('./helper.js'); // import source path + expect(names).toContain('hello world'); // genuine string literal + expect(nodes.some((n) => n.text?.startsWith('`Hello, '))).toBe(true); // template literal + }); + + test('captures exactly the 4 genuine literals — no keyword false-positives', () => { + const nodes = queryTsAstNodes('string'); + expect(nodes.length).toBe(4); + }); +}); + +// ─── TSX fixture (#1729) — shares JS_AST_TYPES + grammar family with TS ─── + +const TSX_FIXTURE_CODE = ` +interface Props { + label: string; +} + +function Greeting(props: Props) { + return
{"hello world"}
; +} +`; + +describe('buildAstNodes — TSX extraction (#1729)', () => { + let tsxTmpDir: string, tsxDb: any; + + beforeAll(async () => { + tsxTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-ast-tsx-extract-')); + const srcDir = path.join(tsxTmpDir, 'src'); + fs.mkdirSync(srcDir, { recursive: true }); + fs.mkdirSync(path.join(tsxTmpDir, '.codegraph')); + + const fixturePath = path.join(srcDir, 'fixture.tsx'); + fs.writeFileSync(fixturePath, TSX_FIXTURE_CODE); + + const allSymbols = await parseFilesAuto([fixturePath], tsxTmpDir, { engine: 'wasm' }); + const symbols = allSymbols.get('src/fixture.tsx'); + if (!symbols) throw new Error('Failed to parse TSX fixture file'); + + const dbPath = path.join(tsxTmpDir, '.codegraph', 'graph.db'); + tsxDb = new Database(dbPath); + tsxDb.pragma('journal_mode = WAL'); + initSchema(tsxDb); + + const insertNode = tsxDb.prepare( + 'INSERT INTO nodes (name, kind, file, line, end_line) VALUES (?, ?, ?, ?, ?)', + ); + for (const def of symbols.definitions) { + insertNode.run(def.name, def.kind, 'src/fixture.tsx', def.line, def.endLine); + } + + await buildAstNodes(tsxDb, allSymbols, tsxTmpDir); + }); + + afterAll(() => { + if (tsxDb) tsxDb.close(); + fs.rmSync(tsxTmpDir, { recursive: true, force: true }); + }); + + test('does not misclassify interface field type annotation as kind:string, still captures JSX-embedded literal', () => { + const nodes = tsxDb + .prepare('SELECT * FROM ast_nodes WHERE kind = ? ORDER BY line') + .all('string'); + expect(nodes.some((n) => n.text === 'string')).toBe(false); + expect(nodes.length).toBe(1); + expect(nodes[0].name).toBe('hello world'); + }); +}); + // ─── Native engine AST node extraction ─────────────────────────────── // Check if native addon is available AND supports ast_nodes. @@ -308,3 +470,70 @@ describe.skipIf(!canTestNative)('buildAstNodes — native engine', () => { expect(withParent.length).toBeGreaterThan(0); }); }); + +// ─── Native engine: TypeScript predefined_type false-positives (#1729) ──── + +describe.skipIf(!canTestNative)('buildAstNodes — native TypeScript extraction (#1729)', () => { + let nativeTsTmpDir: string, nativeTsDb: any; + + function queryNativeTsAstNodes(kind: string) { + return nativeTsDb.prepare('SELECT * FROM ast_nodes WHERE kind = ? ORDER BY line').all(kind); + } + + beforeAll(async () => { + nativeTsTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-ast-ts-native-')); + const srcDir = path.join(nativeTsTmpDir, 'src'); + fs.mkdirSync(srcDir, { recursive: true }); + fs.mkdirSync(path.join(nativeTsTmpDir, '.codegraph')); + + const fixturePath = path.join(srcDir, 'fixture.ts'); + fs.writeFileSync(fixturePath, TS_FIXTURE_CODE); + + const allSymbols = await parseFilesAuto([fixturePath], nativeTsTmpDir, { engine: 'native' }); + const symbols = allSymbols.get('src/fixture.ts'); + if (!symbols) throw new Error('Failed to parse TS fixture file with native engine'); + + const dbPath = path.join(nativeTsTmpDir, '.codegraph', 'graph.db'); + nativeTsDb = new Database(dbPath); + nativeTsDb.pragma('journal_mode = WAL'); + initSchema(nativeTsDb); + + const insertNode = nativeTsDb.prepare( + 'INSERT INTO nodes (name, kind, file, line, end_line) VALUES (?, ?, ?, ?, ?)', + ); + for (const def of symbols.definitions) { + insertNode.run(def.name, def.kind, 'src/fixture.ts', def.line, def.endLine); + } + + await buildAstNodes(nativeTsDb, allSymbols, nativeTsTmpDir); + }); + + afterAll(() => { + if (nativeTsDb) nativeTsDb.close(); + if (nativeTsTmpDir) fs.rmSync(nativeTsTmpDir, { recursive: true, force: true }); + }); + + test('does not misclassify interface field type annotation as kind:string', () => { + const nodes = queryNativeTsAstNodes('string'); + expect(nodes.some((n) => n.text === 'string')).toBe(false); + }); + + test('does not misclassify parameter, return, or array-element type annotations as kind:string', () => { + const nodes = queryNativeTsAstNodes('string'); + expect(nodes.filter((n) => n.text === 'string').length).toBe(0); + }); + + test('still captures genuine string literals, template literals, and string-literal types', () => { + const nodes = queryNativeTsAstNodes('string'); + const names = nodes.map((n) => n.name); + expect(names).toContain('user-id-literal'); + expect(names).toContain('./helper.js'); + expect(names).toContain('hello world'); + expect(nodes.some((n) => n.text?.startsWith('`Hello, '))).toBe(true); + }); + + test('captures exactly the 4 genuine literals — no keyword false-positives', () => { + const nodes = queryNativeTsAstNodes('string'); + expect(nodes.length).toBe(4); + }); +});