diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index e5c73d2e..7b2dbd6b 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -399,12 +399,7 @@ fn seed_object_create_entries(var_name: &str, call_node: &Node, source: &[u8], s let Some(key_n) = child.child_by_field_name("key") else { continue }; let Some(val_n) = child.child_by_field_name("value") else { continue }; if val_n.kind() != "identifier" { continue; } - let key = if key_n.kind() == "string" { - extract_string_fragment(&key_n, source).map(|s| s.to_string()) - } else { - Some(node_text(&key_n, source).to_string()) - }; - let Some(key) = key else { continue }; + let Some(key) = resolve_property_key_name(&key_n, source) else { continue }; symbols.type_map.push(TypeMapEntry { name: format!("{}.{}", var_name, key), type_name: node_text(&val_n, source).to_string(), @@ -423,12 +418,7 @@ fn seed_descriptor_object(obj_name: &str, obj_node: &Node, source: &[u8], symbol if child.kind() != "pair" { continue; } let Some(key_n) = child.child_by_field_name("key") else { continue }; let Some(val_n) = child.child_by_field_name("value") else { continue }; - let key = if key_n.kind() == "string" { - extract_string_fragment(&key_n, source).map(|s| s.to_string()) - } else { - Some(node_text(&key_n, source).to_string()) - }; - let Some(key) = key else { continue }; + let Some(key) = resolve_property_key_name(&key_n, source) else { continue }; let Some(target) = find_descriptor_value(&val_n, source) else { continue }; symbols.type_map.push(TypeMapEntry { name: format!("{}.{}", obj_name, key), @@ -444,6 +434,30 @@ fn extract_string_fragment<'a>(node: &Node<'a>, source: &'a [u8]) -> Option<&'a find_child(node, "string_fragment").map(|n| node_text(&n, source)) } +/// Resolve a plain name from an object-literal `pair`'s key field (or an equivalent +/// property-key node), unwrapping `computed_property_name` string literals the same way +/// `resolve_method_def_name` does for `method_definition` name nodes (e.g. `['foo']: fn` +/// -> "foo"). Plain string-literal keys have their surrounding quotes stripped; all other +/// key node types (identifier, property_identifier, number, private_property_identifier) +/// are returned as-is. Returns `None` for computed keys that aren't a resolvable string +/// literal (e.g. `[Symbol.iterator]: fn`), mirroring `resolve_method_def_name`'s skip +/// behavior for non-string computed keys. +fn resolve_property_key_name(key_node: &Node, source: &[u8]) -> Option { + if key_node.kind() == "computed_property_name" { + // child(0)='[', child(1)=string literal, child(2)=']' + let inner = key_node.child(1)?; + return match inner.kind() { + "string" => extract_string_fragment(&inner, source).map(|s| s.to_string()), + "string_fragment" => Some(node_text(&inner, source).to_string()), + _ => None, // non-string computed key — skip + }; + } + if key_node.kind() == "string" { + return extract_string_fragment(key_node, source).map(|s| s.to_string()); + } + Some(node_text(key_node, source).to_string()) +} + /// Find the `value` identifier in a property descriptor object `{ value: fn }`. fn find_descriptor_value<'a>(node: &Node<'a>, source: &'a [u8]) -> Option<&'a str> { if node.kind() != "object" { return None; } @@ -512,12 +526,8 @@ fn extract_object_literal_functions( "pair" => { let Some(key_n) = child.child_by_field_name("key") else { continue }; let Some(val_n) = child.child_by_field_name("value") else { continue }; - let key = if key_n.kind() == "string" { - extract_string_fragment(&key_n, source).map(|s| s.to_string()) - } else { - Some(node_text(&key_n, source).to_string()) - }; - let Some(key) = key else { continue }; + // Non-string computed keys (e.g. `[Symbol.iterator]`) resolve to None and are skipped. + let Some(key) = resolve_property_key_name(&key_n, source) else { continue }; let qualified = format!("{}.{}", var_name, key); match val_n.kind() { "arrow_function" | "function_expression" | "function" => { @@ -597,12 +607,8 @@ fn seed_objlit_type_map_entries(var_name: &str, obj_node: &Node, source: &[u8], "pair" => { let Some(key_n) = child.child_by_field_name("key") else { continue }; let Some(val_n) = child.child_by_field_name("value") else { continue }; - let key = if key_n.kind() == "string" { - extract_string_fragment(&key_n, source).map(|s| s.to_string()) - } else { - Some(node_text(&key_n, source).to_string()) - }; - let Some(key) = key else { continue }; + // Non-string computed keys (e.g. `[Symbol.iterator]`) resolve to None and are skipped. + let Some(key) = resolve_property_key_name(&key_n, source) else { continue }; let qualified = format!("{}.{}", var_name, key); match val_n.kind() { "arrow_function" | "function_expression" | "function" => { @@ -722,12 +728,8 @@ fn match_js_objlit_qualified_method_defs( if !matches!(val_n.kind(), "arrow_function" | "function_expression" | "function") { continue; } - let key = if key_n.kind() == "string" { - extract_string_fragment(&key_n, source).map(|s| s.to_string()) - } else { - Some(node_text(&key_n, source).to_string()) - }; - let Some(key) = key else { continue }; + // Non-string computed keys (e.g. `[Symbol.iterator]`) resolve to None and are skipped. + let Some(key) = resolve_property_key_name(&key_n, source) else { continue }; let qualified = format!("{}.{}", var_name, key); symbols.definitions.push(Definition { name: qualified, @@ -998,19 +1000,10 @@ fn extract_js_prototype_object_literal(class_name: &str, obj_node: &Node, source let key_node = child.child_by_field_name("key"); let value_node = child.child_by_field_name("value"); if let (Some(key_node), Some(value_node)) = (key_node, value_node) { - let method_name: &str = if key_node.kind() == "string" { - let s = node_text(&key_node, source); - // Strip exactly one matching pair of surrounding quote characters. - // `trim_matches` would also strip embedded quotes; we only want the - // outermost delimiter pair so `"it's"` stays `it's`. - s.strip_prefix('"').and_then(|s| s.strip_suffix('"')) - .or_else(|| s.strip_prefix('\'').and_then(|s| s.strip_suffix('\''))) - .unwrap_or(s) - } else { - node_text(&key_node, source) - }; + // Non-string computed keys (e.g. `[Symbol.iterator]`) resolve to None and are skipped. + let Some(method_name) = resolve_property_key_name(&key_node, source) else { continue }; if method_name.is_empty() { continue; } - emit_js_prototype_method(class_name, method_name, &value_node, source, symbols); + emit_js_prototype_method(class_name, &method_name, &value_node, source, symbols); } } _ => {} @@ -4665,6 +4658,23 @@ mod tests { assert!(bar.cfg.is_some(), "C.bar should have a CFG"); } + /// Regression test for #1764: a computed string-literal key in a prototype object + /// literal (`['foo']: function() {}`) must resolve to the plain name `C.foo`, not + /// the bracketed/quoted text `C.['foo']`. + #[test] + fn prototype_object_literal_computed_key_emits_plain_name() { + let s = parse_js( + "function C() {}\n\ + C.prototype = {\n\ + ['foo']: function() {},\n\ + };", + ); + let def = s.definitions.iter().find(|d| d.name == "C.foo"); + assert!(def.is_some(), "C.foo missing; got: {:?}", s.definitions.iter().map(|d| &d.name).collect::>()); + let bracketed = s.definitions.iter().find(|d| d.name.contains('[')); + assert!(bracketed.is_none(), "bracketed name leaked: {:?}", bracketed); + } + #[test] fn prototype_object_literal_shorthand_method() { let s = parse_js( @@ -4885,6 +4895,25 @@ mod tests { assert_eq!(e2.unwrap().type_name, "f2"); } + /// Regression test for #1764: a computed string-literal key in an + /// Object.defineProperties descriptor object must seed the plain qualified type_map + /// key `obj.f1`, not the bracketed/quoted `obj.['f1']`. + #[test] + fn type_map_from_define_properties_computed_key() { + let s = parse_js( + "function f1() {}\n\ + const obj = {};\n\ + Object.defineProperties(obj, {\n\ + ['f1']: { value: f1 },\n\ + });", + ); + let entry = s.type_map.iter().find(|e| e.name == "obj.f1"); + assert!(entry.is_some(), "type_map should contain 'obj.f1'; got: {:?}", s.type_map); + assert_eq!(entry.unwrap().type_name, "f1"); + let bracketed = s.type_map.iter().find(|e| e.name.contains('[')); + assert!(bracketed.is_none(), "bracketed type_map key leaked: {:?}", bracketed); + } + /// Phase 8.3e: Object.create seeds composite type_map keys from shorthand proto. #[test] fn type_map_from_object_create() { @@ -4940,6 +4969,36 @@ mod tests { assert_eq!(tm_f.unwrap().type_name, "f"); } + /// Regression test for #1764: a computed string-literal key on an object-literal + /// arrow/function property (`{ ['foo']: () => {} }`) must produce a plain qualified + /// definition `obj.foo`, not the bracketed/quoted `obj.['foo']` — and the seeded + /// typeMap entry must match. + #[test] + fn object_literal_pair_computed_key_emits_plain_name() { + let s = parse_js( + "const obj = {\n\ + ['foo']: () => { return 1; },\n\ + bar: () => { return 2; },\n\ + };", + ); + let names: Vec<_> = s.definitions.iter().map(|d| &d.name).collect(); + assert!(names.contains(&&"obj.foo".to_string()), "obj.foo missing; got: {:?}", names); + assert!(names.contains(&&"obj.bar".to_string()), "obj.bar missing; got: {:?}", names); + let bracketed = s.definitions.iter().find(|d| d.name.contains('[')); + assert!(bracketed.is_none(), "bracketed name leaked: {:?}", bracketed); + let tm = s.type_map.iter().find(|e| e.name == "obj.foo"); + assert!(tm.is_some(), "typeMap obj.foo missing; got: {:?}", s.type_map.iter().map(|e| &e.name).collect::>()); + } + + /// Regression test for #1764: a non-string computed key (`[Symbol.iterator]`) on an + /// object-literal arrow/function property must be skipped, not produce a garbage name. + #[test] + fn object_literal_pair_non_string_computed_key_is_skipped() { + let s = parse_js("const obj = { [Symbol.iterator]: () => {} };"); + let leaked = s.definitions.iter().find(|d| d.name.contains("iterator") || d.name.contains('[')); + assert!(leaked.is_none(), "non-string computed key should be skipped; got: {:?}", leaked); + } + /// Issue #1551: `let` and `var` object-literal declarations must seed composite typeMap keys /// just like `const` declarations. Regression test for the parity gap where native bailed /// early for non-`const` declarations in the object-literal typeMap walk. diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index b6c43417..96a47cfa 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -1296,8 +1296,8 @@ function extractObjectLiteralFunctions( const keyNode = child.childForFieldName('key'); const valueNode = child.childForFieldName('value'); if (!keyNode || !valueNode) continue; - const keyName = - keyNode.type === 'string' ? keyNode.text.replace(/^['"]|['"]$/g, '') : keyNode.text; + // Non-string computed keys (e.g. `[Symbol.iterator]`) resolve to '' and are skipped. + const keyName = resolvePropertyKeyName(keyNode); if (!keyName) continue; if ( valueNode.type === 'arrow_function' || @@ -2087,6 +2087,25 @@ function resolveMethodDefinitionName(nameNode: TreeSitterNode): string { return inner.text.replace(/^['"]|['"]$/g, ''); } +/** + * Resolve a plain name from an object-literal `pair`'s key field (or an equivalent + * property-key node), unwrapping `computed_property_name` string literals the same way + * `resolveMethodDefinitionName` does for `method_definition` name nodes (e.g. `['foo']: fn` + * -> 'foo'). Plain string-literal keys have their surrounding quotes stripped; all other + * key node types (identifier, property_identifier, number, private_property_identifier) + * are returned as-is. Returns '' for computed keys that aren't a resolvable string literal + * (e.g. `[Symbol.iterator]: fn`), mirroring resolveMethodDefinitionName's skip behavior. + */ +function resolvePropertyKeyName(keyNode: TreeSitterNode): string { + if (keyNode.type === 'computed_property_name') { + const inner = keyNode.child(1); + if (!inner || (inner.type !== 'string' && inner.type !== 'string_fragment')) return ''; + return inner.text.replace(/^['"]|['"]$/g, ''); + } + if (keyNode.type === 'string') return keyNode.text.replace(/^['"]|['"]$/g, ''); + return keyNode.text; +} + /** * Push node onto funcStack for a method_definition, qualified with the enclosing class * name so the PTS key matches callerName from findCaller (which uses @@ -2476,8 +2495,8 @@ function handleObjectLiteralTypeMap( const keyNode = child.childForFieldName('key'); const valNode = child.childForFieldName('value'); if (!keyNode || !valNode) continue; - const keyName = - keyNode.type === 'string' ? keyNode.text.replace(/^['"]|['"]$/g, '') : keyNode.text; + // Non-string computed keys (e.g. `[Symbol.iterator]`) resolve to '' and are skipped. + const keyName = resolvePropertyKeyName(keyNode); if (!keyName) continue; const qualifiedKey = `${lhsName}.${keyName}`; if ( @@ -2731,7 +2750,8 @@ function handleDefinePropertyTypeMap( const keyN = pair.childForFieldName('key'); const valN = pair.childForFieldName('value'); if (!keyN || !valN) continue; - const key = keyN.type === 'string' ? keyN.text.replace(/^['"]|['"]$/g, '') : keyN.text; + const key = resolvePropertyKeyName(keyN); + if (!key) continue; const target = findDescriptorValue(valN); if (!target) continue; setTypeMapEntry(typeMap, `${arg0.text}.${key}`, target, 0.85); @@ -2787,7 +2807,8 @@ function seedProtoProperties( const keyN = child.childForFieldName('key'); const valN = child.childForFieldName('value'); if (!keyN || !valN || valN.type !== 'identifier') continue; - const key = keyN.type === 'string' ? keyN.text.replace(/^['"]|['"]$/g, '') : keyN.text; + const key = resolvePropertyKeyName(keyN); + if (!key) continue; setTypeMapEntry(typeMap, `${varName}.${key}`, valN.text, 0.85); } } @@ -4195,7 +4216,7 @@ function extractPrototypeObjectLiteral( const valueNode = child.childForFieldName('value'); if (!keyNode || !valueNode) continue; - const methodName = keyNode.type === 'string' ? keyNode.text.replace(/['"]/g, '') : keyNode.text; + const methodName = resolvePropertyKeyName(keyNode); if (!methodName) continue; emitPrototypeMethod(className, methodName, valueNode, definitions, typeMap); diff --git a/tests/integration/issue-1764-computed-objlit-key.test.ts b/tests/integration/issue-1764-computed-objlit-key.test.ts new file mode 100644 index 00000000..7829f0bd --- /dev/null +++ b/tests/integration/issue-1764-computed-objlit-key.test.ts @@ -0,0 +1,180 @@ +/** + * Integration test for #1764: object literal computed method keys extract as + * `obj.['foo']` instead of `obj.foo`. + * + * `extractObjectLiteralFunctions`'s `pair` branch (and several sibling helpers that + * resolve a `pair`'s key field — typeMap seeding, Object.defineProperties, prototype + * assignment) only stripped quotes for plain `string`-typed keys. For a computed + * string-literal key like `{ ['foo']: () => {} }`, the key node type is + * `computed_property_name`, so the old code fell through to the raw bracket/quote + * text, producing `obj.['foo']` instead of `obj.foo` — call sites like `obj.foo()` + * could never resolve to it. + * + * Fix: a shared `resolvePropertyKeyName` (WASM) / `resolve_property_key_name` + * (native) helper unwraps `computed_property_name` string literals the same way the + * existing `resolveMethodDefinitionName` / `resolve_method_def_name` helpers already + * did for `method_definition` name nodes. Non-string computed keys (e.g. + * `[Symbol.iterator]`) are skipped rather than producing a garbage name. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +const FIXTURE = { + 'service.js': ` +const obj = { + ['foo']: () => { + return 1; + }, + bar: () => { + return 2; + }, +}; + +obj.foo(); +obj.bar(); +`, +}; + +let tmpDir: string; + +beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1764-')); + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(tmpDir, rel), content); + } + await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true }); +}); + +afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +function readCallEdges(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS src, n2.name AS tgt + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' + ORDER BY n1.name, n2.name`, + ) + .all() as Array<{ src: string; tgt: string }>; + } finally { + db.close(); + } +} + +function readNodes(dbPath: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db.prepare('SELECT name, kind FROM nodes ORDER BY name').all() as Array<{ + name: string; + kind: string; + }>; + } finally { + db.close(); + } +} + +describe('computed object-literal key extraction (#1764) — WASM', () => { + it('stores computed object-literal property under plain qualified name (no brackets)', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const nodes = readNodes(dbPath); + const fooNode = nodes.find((n) => n.name === 'obj.foo'); + expect( + fooNode, + 'obj.foo node missing — computed object-literal key stored with brackets instead of plain name', + ).toBeDefined(); + expect(fooNode!.kind).toBe('function'); + }); + + it('does not store any node with brackets in its name from computed keys', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const nodes = readNodes(dbPath); + const bracketedNodes = nodes.filter((n) => n.name.includes('[')); + expect( + bracketedNodes, + `Found nodes with brackets in name (bracket representation leaked): ${bracketedNodes.map((n) => n.name).join(', ')}`, + ).toHaveLength(0); + }); + + it('resolves call to computed object-literal property at dot-notation call site', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readCallEdges(dbPath); + const edge = edges.find((e) => e.tgt === 'obj.foo'); + expect( + edge, + 'No call edge to obj.foo — computed object-literal key not resolvable at call site', + ).toBeDefined(); + }); + + it('resolves call to regular (non-computed) object-literal property at dot-notation call site', () => { + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const edges = readCallEdges(dbPath); + const edge = edges.find((e) => e.tgt === 'obj.bar'); + expect( + edge, + 'No call edge to obj.bar — regular object-literal key resolution broken by fix', + ).toBeDefined(); + }); +}); + +// ── Native engine parity ──────────────────────────────────────────────────── +// Guards that resolve_property_key_name in Rust applies the same unwrapping as +// the WASM path. Skipped when the native addon is not installed. + +describe.skipIf(!isNativeAvailable())( + 'computed object-literal key extraction (#1764) — native', + () => { + let nativeTmpDir: string; + + beforeAll(async () => { + nativeTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1764-native-')); + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(nativeTmpDir, rel), content); + } + await buildGraph(nativeTmpDir, { engine: 'native', incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(nativeTmpDir, { recursive: true, force: true }); + }); + + it('stores computed object-literal property under plain qualified name (no brackets)', () => { + const nodes = readNodes(path.join(nativeTmpDir, '.codegraph', 'graph.db')); + const node = nodes.find((n) => n.name === 'obj.foo'); + expect( + node, + 'obj.foo missing in native output — resolve_property_key_name may not strip brackets', + ).toBeDefined(); + expect(node!.kind).toBe('function'); + }); + + it('does not store any node with brackets in its name from computed keys', () => { + const nodes = readNodes(path.join(nativeTmpDir, '.codegraph', 'graph.db')); + const bracketed = nodes.filter((n) => n.name.includes('[')); + expect( + bracketed, + `Native output has bracket-leaked nodes: ${bracketed.map((n) => n.name).join(', ')}`, + ).toHaveLength(0); + }); + + it('resolves call to computed object-literal property at dot-notation call site', () => { + const edges = readCallEdges(path.join(nativeTmpDir, '.codegraph', 'graph.db')); + const edge = edges.find((e) => e.tgt === 'obj.foo'); + expect( + edge, + 'No native call edge to obj.foo — computed object-literal key not resolvable in native engine', + ).toBeDefined(); + }); + }, +); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index c1934034..1deafaf2 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -1548,6 +1548,34 @@ describe('JavaScript parser', () => { }); }); + describe('computed object-literal property key extraction (#1764)', () => { + it('extracts computed arrow-function property with plain qualified name', () => { + const symbols = parseJS(`const obj = { ['foo']: () => { return 1; } };`); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'obj.foo', kind: 'function' }), + ); + }); + + it('extracts computed function-expression property with plain qualified name', () => { + const symbols = parseJS(`const obj = { ['bar']: function () { return 2; } };`); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'obj.bar', kind: 'function' }), + ); + }); + + it('does not use the bracketed form in the stored name for pair values', () => { + const symbols = parseJS(`const obj = { ['foo']: () => { return 1; }, bar: () => 2 };`); + const def = symbols.definitions.find((d) => d.name.includes('[')); + expect(def).toBeUndefined(); + }); + + it('does not extract non-string computed key value (Symbol.iterator)', () => { + const symbols = parseJS(`const obj = { [Symbol.iterator]: () => {} };`); + const def = symbols.definitions.find((d) => d.name.includes('iterator')); + expect(def).toBeUndefined(); + }); + }); + describe('class expression inside function extraction (#1471)', () => { it('extracts named class expression returned from a function', () => { const symbols = parseJS(