Skip to content

Commit 1b96f24

Browse files
authored
fix(native): seed typeMap entries for let/var object-literal methods
* fix(native): seed typeMap entries for let/var object-literal methods (#1551) match_js_type_map's variable_declarator case now calls seed_objlit_type_map_entries for all declaration kinds (const/let/var) when at non-function scope. This mirrors WASM's handleVarDeclaratorTypeMap which has no isConst guard for object properties. For const, extract_object_literal_functions already seeds these entries; dedup_type_map collapses duplicates at equal confidence so existing behavior is unchanged. * fix(native): use bare type_name for let/var method-shorthand typeMap entries For let/var object-literal declarations, match_js_objlit_qualified_method_defs is const-only, so no qualified definition (e.g. 'obj.baz') exists. Pointing typeMap['obj.baz'] → 'obj.baz' left resolution broken because the resolver found no matching node. Fix: use the bare method name as type_name, mirroring extract_object_literal_functions for const. Also strengthen the regression test with type_name value assertion and end-to-end call-edge creation check. * fix(native): emit qualified definitions for let/var pair+arrow/function object-literal methods For 'let api = { save: () => {} }', seed_objlit_type_map_entries correctly seeds typeMap['api.save'] = 'api.save' (qualified), but no matching definition named 'api.save' existed for let/var — the resolver followed the typeMap entry and dead-ended on a missing node. Fix: extend match_js_objlit_qualified_method_defs to cover all declaration kinds for method_definition (was const-only) and to emit qualified definitions for let/var pair+arrow/function values (const pairs are handled by extract_object_literal_functions, so they are excluded to avoid duplicates). Tests: add definition-existence and call-edge assertions for the s_let_arrow case, matching the pattern already established for s_let_method.
1 parent 04d1e5c commit 1b96f24

2 files changed

Lines changed: 229 additions & 18 deletions

File tree

crates/codegraph-core/src/extractors/javascript.rs

Lines changed: 220 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,17 @@ fn match_js_type_map(node: &Node, source: &[u8], symbols: &mut FileSymbols, _dep
138138
if value_n.kind() == "call_expression" {
139139
seed_object_create_entries(var_name, &value_n, source, symbols);
140140
}
141+
// Phase 8.3f parity: seed composite typeMap keys for ALL object-literal
142+
// declarations (`const`, `let`, `var`) when at non-function scope.
143+
// Mirrors WASM handleVarDeclaratorTypeMap (no isConst guard there).
144+
// For `const`, extract_object_literal_functions already seeds these entries;
145+
// dedup_type_map collapses any duplicates at equal confidence.
146+
if value_n.kind() == "object" && find_parent_of_types(node, &[
147+
"function_declaration", "arrow_function", "function_expression",
148+
"method_definition", "generator_function_declaration", "generator_function",
149+
]).is_none() {
150+
seed_objlit_type_map_entries(var_name, &value_n, source, symbols);
151+
}
141152
}
142153
}
143154
}
@@ -555,16 +566,100 @@ fn extract_object_literal_functions(
555566
}
556567
}
557568

569+
/// Seed composite typeMap keys from an object literal for ALL declaration kinds
570+
/// (`const`, `let`, `var`) at non-function scope.
571+
///
572+
/// Mirrors WASM `handleVarDeclaratorTypeMap`'s object-literal branch (no `isConst` guard).
573+
/// Called from `match_js_type_map` so that `let obj = { f() {} }` and
574+
/// `var routes = { get: handler }` resolve correctly just like `const` variants.
575+
///
576+
/// For `const` declarations this produces the same entries as `extract_object_literal_functions`,
577+
/// but `dedup_type_map` collapses duplicates at equal confidence.
578+
fn seed_objlit_type_map_entries(var_name: &str, obj_node: &Node, source: &[u8], symbols: &mut FileSymbols) {
579+
for i in 0..obj_node.child_count() {
580+
let Some(child) = obj_node.child(i) else { continue };
581+
match child.kind() {
582+
"shorthand_property_identifier" => {
583+
let prop_name = node_text(&child, source);
584+
symbols.type_map.push(TypeMapEntry {
585+
name: format!("{}.{}", var_name, prop_name),
586+
type_name: prop_name.to_string(),
587+
confidence: 0.85,
588+
});
589+
}
590+
"pair" => {
591+
let Some(key_n) = child.child_by_field_name("key") else { continue };
592+
let Some(val_n) = child.child_by_field_name("value") else { continue };
593+
let key = if key_n.kind() == "string" {
594+
extract_string_fragment(&key_n, source).map(|s| s.to_string())
595+
} else {
596+
Some(node_text(&key_n, source).to_string())
597+
};
598+
let Some(key) = key else { continue };
599+
let qualified = format!("{}.{}", var_name, key);
600+
match val_n.kind() {
601+
"arrow_function" | "function_expression" | "function" => {
602+
// Store qualified name as value so the resolver finds the qualified def.
603+
// Mirrors WASM: setTypeMapEntry(typeMap, qualifiedKey, qualifiedKey, 0.85).
604+
// For `const`, `extract_object_literal_functions` creates the matching definition.
605+
// For `let`/`var`, `match_js_objlit_qualified_method_defs` creates it in its
606+
// deferred second pass (now covers all declaration kinds, not just `const`).
607+
symbols.type_map.push(TypeMapEntry {
608+
name: qualified.clone(),
609+
type_name: qualified,
610+
confidence: 0.85,
611+
});
612+
}
613+
"identifier" => {
614+
let target = node_text(&val_n, source);
615+
symbols.type_map.push(TypeMapEntry {
616+
name: qualified,
617+
type_name: target.to_string(),
618+
confidence: 0.85,
619+
});
620+
}
621+
_ => {}
622+
}
623+
}
624+
"method_definition" => {
625+
// Method shorthand: `let obj = { baz() {} }` → typeMap['obj.baz'] = 'baz'
626+
// Points to the bare-name definition so the two-step accessor dispatch resolves
627+
// via the bare node. `handle_method_def` always creates a bare definition for
628+
// method_definition nodes; `match_js_objlit_qualified_method_defs` (which now
629+
// covers all declaration kinds) adds the qualified definition in its deferred
630+
// second pass. Using the bare name here keeps resolution consistent across all
631+
// declaration kinds (const/let/var).
632+
let Some(method_name) = resolve_method_def_name(&child, source) else { continue };
633+
let qualified = format!("{}.{}", var_name, method_name);
634+
symbols.type_map.push(TypeMapEntry {
635+
name: qualified,
636+
type_name: method_name.to_string(),
637+
confidence: 0.85,
638+
});
639+
}
640+
_ => {}
641+
}
642+
}
643+
}
644+
558645
/// Second-pass walker: emit qualified `obj.method(function)` definitions for
559-
/// `method_definition` children of object literals.
646+
/// `method_definition` and (for `let`/`var`) `pair+arrow/function` children of object literals.
560647
///
648+
/// **method_definition** (all declaration kinds — `const`, `let`, `var`):
561649
/// This must run AFTER the main `match_js_node` walk so that the bare `f(method)` node
562650
/// created by `handle_method_def` appears BEFORE the qualified `obj.f(function)` node
563651
/// in `symbols.definitions`. `findCaller` picks the narrowest-span enclosing definition;
564652
/// when spans are equal it keeps the first inserted one (strict `<`), so `f(method)` wins
565653
/// and call-edge attribution matches WASM (which runs `handleMethodCapture` via the query
566654
/// path before `extractObjectLiteralFunctions` via `runCollectorWalk`).
567655
///
656+
/// **pair + arrow_function / function_expression / function** (`let`/`var` only):
657+
/// For `const`, `extract_object_literal_functions` already creates the qualified definition;
658+
/// repeating it here would produce a duplicate. For `let`/`var`, no other pass emits the
659+
/// qualified definition, so we must emit it here. Without the definition, the typeMap entry
660+
/// seeded by `seed_objlit_type_map_entries` (`"api.save" → "api.save"`) dead-ends: the
661+
/// resolver finds the typeMap entry but then fails to locate a node named `"api.save"`.
662+
///
568663
/// WASM produces both nodes — the qualified one via `extractObjectLiteralFunctions` and the
569664
/// bare one via `handleMethodCapture`. This pass mirrors that by adding only the qualified
570665
/// definitions, deferred so ordering is correct.
@@ -583,7 +678,6 @@ fn match_js_objlit_qualified_method_defs(
583678
return;
584679
}
585680
let is_const = node.child(0).map(|c| node_text(&c, source) == "const").unwrap_or(false);
586-
if !is_const { return; }
587681
for i in 0..node.child_count() {
588682
let Some(declarator) = node.child(i) else { continue };
589683
if declarator.kind() != "variable_declarator" { continue; }
@@ -593,22 +687,54 @@ fn match_js_objlit_qualified_method_defs(
593687
let var_name = node_text(&name_n, source);
594688
for j in 0..value_n.child_count() {
595689
let Some(child) = value_n.child(j) else { continue };
596-
if child.kind() != "method_definition" { continue; }
597-
// Use resolve_method_def_name to strip brackets from computed string keys
598-
// (e.g. ['foo'] → "foo") and skip non-string computed keys ([Symbol.iterator]).
599-
let Some(method_name) = resolve_method_def_name(&child, source) else { continue };
600-
let qualified = format!("{}.{}", var_name, method_name);
601-
let body = child.child_by_field_name("body");
602-
symbols.definitions.push(Definition {
603-
name: qualified,
604-
kind: "function".to_string(),
605-
line: start_line(&child),
606-
end_line: Some(end_line(&child)),
607-
decorators: None,
608-
complexity: body.and_then(|b| compute_all_metrics(&b, source, "javascript")),
609-
cfg: body.and_then(|b| build_function_cfg(&b, "javascript", source)),
610-
children: None,
611-
});
690+
match child.kind() {
691+
"method_definition" => {
692+
// Emit qualified definition for ALL declaration kinds.
693+
// Use resolve_method_def_name to strip brackets from computed string keys
694+
// (e.g. ['foo'] → "foo") and skip non-string computed keys ([Symbol.iterator]).
695+
let Some(method_name) = resolve_method_def_name(&child, source) else { continue };
696+
let qualified = format!("{}.{}", var_name, method_name);
697+
let body = child.child_by_field_name("body");
698+
symbols.definitions.push(Definition {
699+
name: qualified,
700+
kind: "function".to_string(),
701+
line: start_line(&child),
702+
end_line: Some(end_line(&child)),
703+
decorators: None,
704+
complexity: body.and_then(|b| compute_all_metrics(&b, source, "javascript")),
705+
cfg: body.and_then(|b| build_function_cfg(&b, "javascript", source)),
706+
children: None,
707+
});
708+
}
709+
"pair" if !is_const => {
710+
// Emit qualified definition for `let`/`var` pair+arrow/function values only.
711+
// For `const`, `extract_object_literal_functions` already creates this definition;
712+
// creating it again here would be a duplicate.
713+
let Some(key_n) = child.child_by_field_name("key") else { continue };
714+
let Some(val_n) = child.child_by_field_name("value") else { continue };
715+
if !matches!(val_n.kind(), "arrow_function" | "function_expression" | "function") {
716+
continue;
717+
}
718+
let key = if key_n.kind() == "string" {
719+
extract_string_fragment(&key_n, source).map(|s| s.to_string())
720+
} else {
721+
Some(node_text(&key_n, source).to_string())
722+
};
723+
let Some(key) = key else { continue };
724+
let qualified = format!("{}.{}", var_name, key);
725+
symbols.definitions.push(Definition {
726+
name: qualified,
727+
kind: "function".to_string(),
728+
line: start_line(&child),
729+
end_line: Some(end_line(&val_n)),
730+
decorators: None,
731+
complexity: compute_all_metrics(&val_n, source, "javascript"),
732+
cfg: build_function_cfg(&val_n, "javascript", source),
733+
children: None,
734+
});
735+
}
736+
_ => {}
737+
}
612738
}
613739
}
614740
}
@@ -4108,6 +4234,82 @@ mod tests {
41084234
assert_eq!(tm_f.unwrap().type_name, "f");
41094235
}
41104236

4237+
/// Issue #1551: `let` and `var` object-literal declarations must seed composite typeMap keys
4238+
/// just like `const` declarations. Regression test for the parity gap where native bailed
4239+
/// early for non-`const` declarations in the object-literal typeMap walk.
4240+
#[test]
4241+
fn let_var_objlit_seeds_type_map_entries() {
4242+
// Method shorthand: `let obj = { f() {} }` → typeMap['obj.f'] present
4243+
let s_let_method = parse_js(
4244+
"let obj = { f() { return 1; } };\n\
4245+
obj.f();",
4246+
);
4247+
let tm = s_let_method.type_map.iter().find(|e| e.name == "obj.f");
4248+
assert!(tm.is_some(), "let obj method: typeMap 'obj.f' missing; got: {:?}",
4249+
s_let_method.type_map.iter().map(|e| &e.name).collect::<Vec<_>>());
4250+
assert_eq!(tm.unwrap().type_name, "f",
4251+
"typeMap 'obj.f' must point at bare name 'f', not the qualified key");
4252+
let call = s_let_method.calls.iter().find(|c| c.name == "f" && c.receiver.as_deref() == Some("obj"));
4253+
assert!(call.is_some(),
4254+
"calls must contain obj.f() with receiver='obj'; got: {:?}",
4255+
s_let_method.calls.iter().map(|c| (&c.name, &c.receiver)).collect::<Vec<_>>());
4256+
4257+
// Shorthand property: `var obj = { e4 }` → typeMap['obj.e4'] = 'e4'
4258+
let s_var_shorthand = parse_js(
4259+
"function e4() {}\n\
4260+
var obj = { e4 };",
4261+
);
4262+
let tm2 = s_var_shorthand.type_map.iter().find(|e| e.name == "obj.e4");
4263+
assert!(tm2.is_some(), "var obj shorthand: typeMap 'obj.e4' missing; got: {:?}",
4264+
s_var_shorthand.type_map.iter().map(|e| &e.name).collect::<Vec<_>>());
4265+
assert_eq!(tm2.unwrap().type_name, "e4");
4266+
4267+
// Pair with identifier value: `var routes = { get: handler }` → typeMap['routes.get'] = 'handler'
4268+
let s_var_pair = parse_js(
4269+
"function handler() {}\n\
4270+
var routes = { get: handler };",
4271+
);
4272+
let tm3 = s_var_pair.type_map.iter().find(|e| e.name == "routes.get");
4273+
assert!(tm3.is_some(), "var routes pair: typeMap 'routes.get' missing; got: {:?}",
4274+
s_var_pair.type_map.iter().map(|e| &e.name).collect::<Vec<_>>());
4275+
assert_eq!(tm3.unwrap().type_name, "handler");
4276+
4277+
// Pair with arrow value: `let api = { save: () => {} }` → typeMap['api.save'] = 'api.save'
4278+
// and a qualified definition 'api.save' must exist (emitted by the deferred
4279+
// match_js_objlit_qualified_method_defs pass for non-const pair+arrow/function).
4280+
let s_let_arrow = parse_js(
4281+
"let api = { save: () => {} };\n\
4282+
api.save();",
4283+
);
4284+
let tm4 = s_let_arrow.type_map.iter().find(|e| e.name == "api.save");
4285+
assert!(tm4.is_some(), "let api arrow: typeMap 'api.save' missing; got: {:?}",
4286+
s_let_arrow.type_map.iter().map(|e| &e.name).collect::<Vec<_>>());
4287+
assert_eq!(tm4.unwrap().type_name, "api.save",
4288+
"typeMap 'api.save' must point at the qualified name 'api.save' (qualified definition exists)");
4289+
assert!(
4290+
s_let_arrow.definitions.iter().any(|d| d.name == "api.save"),
4291+
"let api arrow: qualified definition 'api.save' missing; got: {:?}",
4292+
s_let_arrow.definitions.iter().map(|d| &d.name).collect::<Vec<_>>()
4293+
);
4294+
let call4 = s_let_arrow.calls.iter().find(|c| c.name == "save" && c.receiver.as_deref() == Some("api"));
4295+
assert!(call4.is_some(),
4296+
"calls must contain api.save() with receiver='api'; got: {:?}",
4297+
s_let_arrow.calls.iter().map(|c| (&c.name, &c.receiver)).collect::<Vec<_>>());
4298+
4299+
// Scope guard: object literal inside a function body must NOT seed module-level typeMap.
4300+
let s_scoped = parse_js(
4301+
"function init() {\n\
4302+
let local = { run() {} };\n\
4303+
local.run();\n\
4304+
}",
4305+
);
4306+
assert!(
4307+
s_scoped.type_map.iter().all(|e| e.name != "local.run"),
4308+
"function-scoped let obj must not pollute typeMap; got: {:?}",
4309+
s_scoped.type_map.iter().map(|e| &e.name).collect::<Vec<_>>()
4310+
);
4311+
}
4312+
41114313
/// Phase 8.3e: call receiver is correctly recorded for obj.f() inside defProp body.
41124314
#[test]
41134315
fn call_receiver_for_define_property() {

tests/parsers/javascript.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,15 @@ describe('JavaScript parser', () => {
10071007
expect(symbols.typeMap.get('routes.get')).toEqual({ type: 'handler', confidence: 0.85 });
10081008
});
10091009

1010+
// Issue #1551: let/var object-literal method definitions must seed typeMap entries
1011+
it('seeds composite typeMap keys for let-declared object-literal method shorthand', () => {
1012+
const symbols = parseJS(`
1013+
let obj = { f() { return 1; } };
1014+
obj.f();
1015+
`);
1016+
expect(symbols.typeMap.get('obj.f')).toBeDefined();
1017+
});
1018+
10101019
it('extracts rest binding from a class method', () => {
10111020
const symbols = parseJS(`
10121021
class Service {

0 commit comments

Comments
 (0)