Skip to content

fix(extractors): unwrap computed property keys in object-literal key resolution #1945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
carlos-alm wants to merge 1 commit into from
+337 −49
Closed

fix(extractors): unwrap computed property keys in object-literal key resolution #1945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 101 additions & 42 deletions crates/codegraph-core/src/extractors/javascript.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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),
Expand All @@ -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<String> {
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; }
Expand Down Expand Up @@ -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" => {
Expand Down Expand Up @@ -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" => {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
_ => {}
Expand Down Expand Up @@ -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::<Vec<_>>());
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(
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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::<Vec<_>>());
}

/// 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.
Expand Down
35 changes: 28 additions & 7 deletions src/extractors/javascript.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' ||
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading