diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index edafba2a..70256cbb 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_pair_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_pair_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), @@ -659,12 +649,7 @@ 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 }; + let Some(key) = resolve_pair_key_name(&key_n, source) else { continue }; let qualified = format!("{}.{}", var_name, key); match val_n.kind() { "arrow_function" | "function_expression" | "function" => { @@ -962,19 +947,9 @@ 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) - }; + let Some(method_name) = resolve_pair_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); } } _ => {} @@ -4187,24 +4162,21 @@ fn collect_object_rest_params(node: &Node, source: &[u8], symbols: &mut FileSymb } "pair" => { // object-literal method: `{ bar: function({ ...rest }) {} }`. - // Computed keys are skipped — they can never match a paramBinding callee. + // Computed keys resolve through resolve_pair_key_name, which unwraps resolvable + // string literals (e.g. `['bar']`) and returns None for non-string computed keys + // (e.g. `[Symbol.iterator]`) — those can never match a paramBinding callee. if let (Some(key_n), Some(value_n)) = ( node.child_by_field_name("key"), node.child_by_field_name("value"), ) { let vt = value_n.kind(); - if key_n.kind() != "computed_property_name" - && (vt == "arrow_function" || vt == "function_expression" || vt == "generator_function") - { - let key_text = node_text(&key_n, source); - fn_name = Some(if key_n.kind() == "string" { - key_text[1..key_text.len() - 1].to_string() - } else { - key_text.to_string() - }); - params_node = value_n - .child_by_field_name("parameters") - .or_else(|| find_child(&value_n, "formal_parameters")); + if vt == "arrow_function" || vt == "function_expression" || vt == "generator_function" { + if let Some(key_name) = resolve_pair_key_name(&key_n, source) { + fn_name = Some(key_name); + params_node = value_n + .child_by_field_name("parameters") + .or_else(|| find_child(&value_n, "formal_parameters")); + } } } } @@ -6217,6 +6189,124 @@ mod tests { assert!(names.contains(&"obj3.computedVar"), "expected 'obj3.computedVar'; got: {:?}", names); } + /// Issue #1884: `seed_object_create_entries`'s pair arm must unwrap a computed + /// string-literal key (`Object.create({ ['foo']: fn })`) instead of falling back to the + /// raw bracket/quote source text. + #[test] + fn computed_key_in_object_create_resolves() { + let s = parse_js( + "function fn() {}\n\ + const obj = Object.create({ ['foo']: fn });", + ); + let entry = s.type_map.iter().find(|e| e.name == "obj.foo"); + assert!(entry.is_some(), "type_map should contain 'obj.foo'; got: {:?}", s.type_map); + assert_eq!(entry.unwrap().type_name, "fn"); + } + + /// Issue #1884: `seed_descriptor_object`'s pair arm (Object.defineProperties) must unwrap + /// a computed string-literal key instead of falling back to the raw bracket/quote text. + #[test] + fn computed_key_in_define_properties_resolves() { + let s = parse_js( + "function f1() {}\n\ + const obj = {};\n\ + Object.defineProperties(obj, { ['foo']: { value: f1 } });", + ); + let entry = s.type_map.iter().find(|e| e.name == "obj.foo"); + assert!(entry.is_some(), "type_map should contain 'obj.foo'; got: {:?}", s.type_map); + assert_eq!(entry.unwrap().type_name, "f1"); + } + + /// Issue #1884: a non-string computed key in Object.defineProperties has no statically + /// resolvable name — must be skipped rather than emitting a garbled entry. + #[test] + fn non_string_computed_key_in_define_properties_skipped() { + let s = parse_js( + "function f1() {}\n\ + const obj = {};\n\ + Object.defineProperties(obj, { [Symbol.iterator]: { value: f1 } });", + ); + assert!( + !s.type_map.iter().any(|e| e.name.contains("Symbol")), + "non-string computed key must not produce a type_map entry; got: {:?}", s.type_map + ); + } + + /// Issue #1884: `seed_objlit_type_map_entries`'s pair arm (let/var object literals) must + /// unwrap a computed string-literal key instead of falling back to the raw bracket/quote text. + #[test] + fn computed_key_in_let_objlit_pair_seeds_type_map() { + let s = parse_js( + "function handler() {}\n\ + var routes = { ['get']: handler };", + ); + let entry = s.type_map.iter().find(|e| e.name == "routes.get"); + assert!(entry.is_some(), "type_map should contain 'routes.get'; got: {:?}", s.type_map); + assert_eq!(entry.unwrap().type_name, "handler"); + } + + /// Issue #1884: `extract_js_prototype_object_literal`'s pair arm must unwrap a computed + /// string-literal key instead of falling back to the raw bracket/quote text. + #[test] + fn computed_key_in_prototype_object_literal_pair_resolves() { + let s = parse_js( + "function helper() {}\n\ + function C() {}\n\ + C.prototype = { ['run']: helper };", + ); + let entry = s.type_map.iter().find(|e| e.name == "C.run"); + assert!(entry.is_some(), "type_map should contain 'C.run'; got: {:?}", s.type_map); + assert_eq!(entry.unwrap().type_name, "helper"); + } + + /// Issue #1884: a computed string-literal pair key with a function value in a prototype + /// object literal must emit a method definition under the plain qualified name. + #[test] + fn computed_key_in_prototype_object_literal_pair_fn_value_emits_definition() { + let s = parse_js( + "function C() {}\n\ + C.prototype = { ['foo']: function() { return 1; } };", + ); + let names: Vec<_> = s.definitions.iter().map(|d| d.name.as_str()).collect(); + assert!(names.contains(&"C.foo"), "expected 'C.foo'; got: {:?}", names); + } + + /// Issue #1884: `collect_object_rest_params`'s pair arm previously skipped ALL computed + /// keys, including resolvable string literals — it must now unwrap them the same way + /// `resolve_pair_key_name` does elsewhere, instead of blanket-skipping. + #[test] + fn computed_string_literal_key_unwrapped_for_object_rest_param_binding() { + let s = parse_js( + "const api = {\n\ + ['process']: function({ items, ...rest }) {\n\ + rest.flush();\n\ + }\n\ + };", + ); + let b = s.object_rest_param_bindings.iter().find(|b| b.callee == "process"); + assert!(b.is_some(), "object_rest_param_bindings missing; got: {:?}", s.object_rest_param_bindings); + let b = b.unwrap(); + assert_eq!(b.rest_name, "rest"); + assert_eq!(b.arg_index, 0); + } + + /// Issue #1884: a non-string computed key must still be skipped for rest-param binding + /// extraction — there's no statically resolvable callee name to bind against. + #[test] + fn non_string_computed_key_still_skipped_for_object_rest_param_binding() { + let s = parse_js( + "const api = {\n\ + [Symbol.iterator]: function({ ...rest }) {\n\ + rest.flush();\n\ + }\n\ + };", + ); + assert!( + !s.object_rest_param_bindings.iter().any(|b| b.rest_name == "rest"), + "non-string computed key must not produce a binding; got: {:?}", s.object_rest_param_bindings + ); + } + /// 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 4d2c546b..65574839 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -2195,6 +2195,21 @@ function resolveMethodDefinitionName(nameNode: TreeSitterNode): string { return resolveComputedKeyName(nameNode); } +/** + * Resolve an object-literal `pair` node's key field to its plain string form. + * + * Mirrors resolveMethodDefinitionName's computed-key handling so `{ ['foo']: () => {} }` and + * `{ ['foo']() {} }` resolve identically: quoted string keys have their quotes stripped, + * computed string-literal keys (`['foo']`) are unwrapped, and non-string computed keys + * (e.g. `[Symbol.iterator]`) return '' (no resolvable name — caller skips the pair) rather + * than falling back to the raw bracket/quote source text. + */ +function resolvePairKeyName(keyNode: TreeSitterNode): string { + if (keyNode.type === 'string') return keyNode.text.replace(/^['"]|['"]$/g, ''); + if (keyNode.type === 'computed_property_name') return resolveComputedKeyName(keyNode); + 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 @@ -2584,8 +2599,7 @@ 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; + const keyName = resolvePairKeyName(keyNode); if (!keyName) continue; const qualifiedKey = `${lhsName}.${keyName}`; if ( @@ -2604,7 +2618,9 @@ function handleObjectLiteralTypeMap( // seed the matching typeMap entry so the two-step accessor dispatch finds it. const nameNode = child.childForFieldName('name'); if (!nameNode) continue; - setTypeMapEntry(typeMap, `${lhsName}.${nameNode.text}`, `${lhsName}.${nameNode.text}`, 0.85); + const methName = resolveMethodDefinitionName(nameNode); + if (!methName) continue; + setTypeMapEntry(typeMap, `${lhsName}.${methName}`, `${lhsName}.${methName}`, 0.85); } } } @@ -2839,7 +2855,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 = resolvePairKeyName(keyN); + if (!key) continue; const target = findDescriptorValue(valN); if (!target) continue; setTypeMapEntry(typeMap, `${arg0.text}.${key}`, target, 0.85); @@ -2895,7 +2912,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 = resolvePairKeyName(keyN); + if (!key) continue; setTypeMapEntry(typeMap, `${varName}.${key}`, valN.text, 0.85); } } @@ -3158,16 +3176,20 @@ function collectObjectRestParams( } } else if (t === 'pair') { // object-literal method: `{ bar: function({ a, ...rest }) {} }` - // Skip computed property keys (e.g. `{ [Symbol.iterator]: function({ ...rest }) {} }`) - // because `callee: '[Symbol.iterator]'` can never match a paramBinding callee. + // Computed keys resolve through resolvePairKeyName, which unwraps resolvable + // string literals (e.g. `['bar']`) and returns '' for non-string computed keys + // (e.g. `[Symbol.iterator]`) — `callee: ''` can never match a paramBinding callee. const keyN = node.childForFieldName('key'); const valueN = node.childForFieldName('value'); - if (keyN && valueN && keyN.type !== 'computed_property_name') { + if (keyN && valueN) { const vt = valueN.type; if (vt === 'arrow_function' || vt === 'function_expression' || vt === 'generator_function') { - fnName = keyN.type === 'string' ? keyN.text.slice(1, -1) : keyN.text; - paramsNode = - valueN.childForFieldName('parameters') ?? findChild(valueN, 'formal_parameters'); + const keyName = resolvePairKeyName(keyN); + if (keyName) { + fnName = keyName; + paramsNode = + valueN.childForFieldName('parameters') ?? findChild(valueN, 'formal_parameters'); + } } } } @@ -4685,12 +4707,15 @@ function extractPrototypeObjectLiteral( // Shorthand method: `Foo.prototype = { bar() {} }` const nameNode = child.childForFieldName('name'); if (nameNode) { - definitions.push({ - name: `${className}.${nameNode.text}`, - kind: 'method', - line: nodeStartLine(child), - endLine: nodeEndLine(child), - }); + const methodName = resolveMethodDefinitionName(nameNode); + if (methodName) { + definitions.push({ + name: `${className}.${methodName}`, + kind: 'method', + line: nodeStartLine(child), + endLine: nodeEndLine(child), + }); + } } continue; } @@ -4709,7 +4734,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 = resolvePairKeyName(keyNode); if (!methodName) continue; emitPrototypeMethod(className, methodName, valueNode, definitions, typeMap); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index f230170e..f57f3513 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -2135,6 +2135,141 @@ function runDemo(reporter: Reporter, users: string[]): void { }); }); + describe('computed string-literal keys across typeMap/prototype extraction sites (#1884)', () => { + it('seeds a qualified typeMap entry (not a garbled bracket key) for a computed pair with a function value', () => { + // handleObjectLiteralTypeMap's pair branch: the extractObjectLiteralFunctions + // Definition ('obj.foo') already resolves correctly since #1764 — this covers the + // *typeMap* entry that the two-step accessor dispatch also needs. + const symbols = parseJS(`const obj = { ['foo']: () => {} };`); + expect(symbols.typeMap.get('obj.foo')).toEqual({ type: 'obj.foo', confidence: 0.85 }); + expect(symbols.typeMap.has("obj.['foo']")).toBe(false); + }); + + it('seeds typeMap from a computed pair key with an identifier value', () => { + const symbols = parseJS(` + function handler() {} + var routes = { ['get']: handler }; + `); + expect(symbols.typeMap.get('routes.get')).toEqual({ type: 'handler', confidence: 0.85 }); + }); + + it('seeds a qualified typeMap entry for a computed method_definition shorthand in an object literal', () => { + const symbols = parseJS(`let obj = { ['foo']() { return 1; } };`); + expect(symbols.typeMap.get('obj.foo')).toEqual({ type: 'obj.foo', confidence: 0.85 }); + }); + + it('does not seed a garbage typeMap entry for a non-string computed method_definition key', () => { + const symbols = parseJS(`let obj = { [Symbol.iterator]() { return 1; } };`); + for (const key of symbols.typeMap.keys()) { + expect(key).not.toContain('Symbol'); + expect(key).not.toContain('['); + } + }); + + it('seeds typeMap from a computed key in Object.defineProperties', () => { + const symbols = parseJS(` + function f1() {} + const obj = {}; + Object.defineProperties(obj, { ['foo']: { value: f1 } }); + `); + expect(symbols.typeMap.get('obj.foo')).toEqual({ type: 'f1', confidence: 0.85 }); + }); + + it('skips a non-string computed key in Object.defineProperties instead of emitting garbage', () => { + const symbols = parseJS(` + function f1() {} + const obj = {}; + Object.defineProperties(obj, { [Symbol.iterator]: { value: f1 } }); + `); + for (const key of symbols.typeMap.keys()) { + expect(key).not.toContain('Symbol'); + } + }); + + it('seeds typeMap from a computed key in an Object.create prototype literal', () => { + const symbols = parseJS(` + function fn() {} + const obj = Object.create({ ['foo']: fn }); + `); + expect(symbols.typeMap.get('obj.foo')).toEqual({ type: 'fn', confidence: 0.85 }); + }); + + it('unwraps a resolvable computed key instead of blanket-skipping the rest-param binding', () => { + const symbols = parseJS(` + const api = { + ['process']: function({ items, ...rest }) { + rest.flush(); + } + }; + `); + expect(symbols.objectRestParamBindings).toContainEqual({ + callee: 'process', + restName: 'rest', + argIndex: 0, + }); + }); + + it('still skips a non-string computed key for rest-param binding extraction', () => { + const symbols = parseJS(` + const api = { + [Symbol.iterator]: function({ ...rest }) { + rest.flush(); + } + }; + `); + expect(symbols.objectRestParamBindings ?? []).not.toContainEqual( + expect.objectContaining({ restName: 'rest' }), + ); + }); + + it('extracts a computed method_definition in a Foo.prototype = {...} object literal', () => { + const symbols = parseJS(` + function C() {} + C.prototype = { ['bar']() { return 1; } }; + `); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'C.bar', kind: 'method' }), + ); + }); + + it('does not extract a non-string computed method_definition key in a prototype object literal', () => { + const symbols = parseJS(` + function C() {} + C.prototype = { [Symbol.iterator]() { return 1; } }; + `); + const def = symbols.definitions.find((d) => d.name.includes('iterator')); + expect(def).toBeUndefined(); + }); + + it('extracts a computed pair key with a function value in a prototype object literal', () => { + const symbols = parseJS(` + function C() {} + C.prototype = { ['foo']: function() { return 1; } }; + `); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'C.foo', kind: 'method' }), + ); + }); + + it('seeds typeMap for a computed pair key with an identifier value in a prototype object literal', () => { + const symbols = parseJS(` + function helper() {} + function C() {} + C.prototype = { ['run']: helper }; + `); + expect(symbols.typeMap.get('C.run')).toEqual({ type: 'helper', confidence: 0.9 }); + }); + + it('does not extract a non-string computed pair key in a prototype object literal', () => { + const symbols = parseJS(` + function C() {} + C.prototype = { [Symbol.iterator]: function() { return 1; } }; + `); + 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(