Skip to content

Commit 8fbd2b9

Browse files
committed
fix(native): emit object-literal shorthand method definitions in two passes to match WASM ordering
For `const o1 = { f() {} }`, both engines produce a bare `f(method)` node (from the top-level method_definition walker) and a qualified `o1.f(function)` node (from the object literal scanner). In WASM, the query path inserts the bare node first; findCaller's equal-span tie-break then correctly attributes call edges to `f(method)`, matching the expected-edges manifests. In native, handle_var_decl processes the lexical_declaration and calls extract_object_literal_functions inline, inserting `o1.f(function)` before the DFS walk descends into the method_definition child and calls handle_method_def to insert `f(method)`. This reversed order caused the tie-break to pick the wrong node, diverging from WASM. Fix: remove the method_definition definition from extract_object_literal_functions (keeping only the typeMap entry) and add a new match_js_objlit_qualified_method_defs second-pass walk that emits the qualified definitions AFTER match_js_node. The bare node now precedes the qualified node in definitions, matching WASM ordering. Also adds a regression test asserting that f(method) precedes o1.f(function) in the definitions vec for object literal shorthand methods. Closes #1538
1 parent 4a1329f commit 8fbd2b9

1 file changed

Lines changed: 112 additions & 16 deletions

File tree

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

Lines changed: 112 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ impl SymbolExtractor for JsExtractor {
3131
fn extract(&self, tree: &Tree, source: &[u8], file_path: &str) -> FileSymbols {
3232
let mut symbols = FileSymbols::new(file_path.to_string());
3333
walk_tree(&tree.root_node(), source, &mut symbols, match_js_node);
34+
// Emit qualified `obj.method(function)` definitions for object-literal shorthand
35+
// methods AFTER match_js_node so that the bare `f(method)` node created by
36+
// handle_method_def comes first in definitions — matching WASM ordering where
37+
// handleMethodCapture (query path) runs before extractObjectLiteralFunctions
38+
// (runCollectorWalk). Equal-span tie-break in findCaller keeps the first entry,
39+
// so bare `f(method)` wins for call attribution in both engines.
40+
walk_tree(&tree.root_node(), source, &mut symbols, match_js_objlit_qualified_method_defs);
3441
walk_ast_nodes(&tree.root_node(), source, &mut symbols.ast_nodes);
3542
walk_tree(&tree.root_node(), source, &mut symbols, match_js_type_map);
3643
walk_tree(&tree.root_node(), source, &mut symbols, match_js_return_type_map);
@@ -524,24 +531,18 @@ fn extract_object_literal_functions(
524531
}
525532
}
526533
"method_definition" => {
534+
// The definition (`obj.baz(function)`) is emitted by the second-pass
535+
// `match_js_objlit_qualified_method_defs` walker (runs after `match_js_node`)
536+
// so that `handle_method_def`'s bare `baz(method)` node appears first in
537+
// `definitions`. Only seed the typeMap entry here.
527538
let Some(name_n) = child.child_by_field_name("name") else { continue };
528-
let qualified = format!("{}.{}", var_name, node_text(&name_n, source));
529-
let body = child.child_by_field_name("body");
530-
symbols.definitions.push(Definition {
531-
name: qualified.clone(),
532-
kind: "function".to_string(),
533-
line: start_line(&child),
534-
end_line: Some(end_line(&child)),
535-
decorators: None,
536-
complexity: body.and_then(|b| compute_all_metrics(&b, source, "javascript")),
537-
cfg: body.and_then(|b| build_function_cfg(&b, "javascript", source)),
538-
children: None,
539-
});
540-
// Seed typeMap so the two-step accessor dispatch can find the qualified def.
541-
// `const obj = { baz() {} }` → typeMap['obj.baz'] = 'obj.baz'
539+
let method_name = node_text(&name_n, source);
540+
let qualified = format!("{}.{}", var_name, method_name);
541+
// typeMap['obj.baz'] = 'baz' — points to the bare-name definition so
542+
// the two-step accessor dispatch resolves via the bare node.
542543
symbols.type_map.push(TypeMapEntry {
543-
name: qualified.clone(),
544-
type_name: qualified,
544+
name: qualified,
545+
type_name: method_name.to_string(),
545546
confidence: 0.85,
546547
});
547548
}
@@ -550,6 +551,62 @@ fn extract_object_literal_functions(
550551
}
551552
}
552553

554+
/// Second-pass walker: emit qualified `obj.method(function)` definitions for
555+
/// `method_definition` children of object literals.
556+
///
557+
/// This must run AFTER the main `match_js_node` walk so that the bare `f(method)` node
558+
/// created by `handle_method_def` appears BEFORE the qualified `obj.f(function)` node
559+
/// in `symbols.definitions`. `findCaller` picks the narrowest-span enclosing definition;
560+
/// when spans are equal it keeps the first inserted one (strict `<`), so `f(method)` wins
561+
/// and call-edge attribution matches WASM (which runs `handleMethodCapture` via the query
562+
/// path before `extractObjectLiteralFunctions` via `runCollectorWalk`).
563+
///
564+
/// WASM produces both nodes — the qualified one via `extractObjectLiteralFunctions` and the
565+
/// bare one via `handleMethodCapture`. This pass mirrors that by adding only the qualified
566+
/// definitions, deferred so ordering is correct.
567+
fn match_js_objlit_qualified_method_defs(
568+
node: &Node,
569+
source: &[u8],
570+
symbols: &mut FileSymbols,
571+
_depth: usize,
572+
) {
573+
// Only lexical/variable declarations at non-function scope.
574+
if !matches!(node.kind(), "lexical_declaration" | "variable_declaration") { return; }
575+
if find_parent_of_types(node, &[
576+
"function_declaration", "arrow_function", "function_expression",
577+
"method_definition", "generator_function_declaration", "generator_function",
578+
]).is_some() {
579+
return;
580+
}
581+
let is_const = node.child(0).map(|c| node_text(&c, source) == "const").unwrap_or(false);
582+
if !is_const { return; }
583+
for i in 0..node.child_count() {
584+
let Some(declarator) = node.child(i) else { continue };
585+
if declarator.kind() != "variable_declarator" { continue; }
586+
let Some(name_n) = declarator.child_by_field_name("name") else { continue };
587+
let Some(value_n) = declarator.child_by_field_name("value") else { continue };
588+
if value_n.kind() != "object" || name_n.kind() != "identifier" { continue; }
589+
let var_name = node_text(&name_n, source);
590+
for j in 0..value_n.child_count() {
591+
let Some(child) = value_n.child(j) else { continue };
592+
if child.kind() != "method_definition" { continue; }
593+
let Some(meth_name_n) = child.child_by_field_name("name") else { continue };
594+
let qualified = format!("{}.{}", var_name, node_text(&meth_name_n, source));
595+
let body = child.child_by_field_name("body");
596+
symbols.definitions.push(Definition {
597+
name: qualified,
598+
kind: "function".to_string(),
599+
line: start_line(&child),
600+
end_line: Some(end_line(&child)),
601+
decorators: None,
602+
complexity: body.and_then(|b| compute_all_metrics(&b, source, "javascript")),
603+
cfg: body.and_then(|b| build_function_cfg(&b, "javascript", source)),
604+
children: None,
605+
});
606+
}
607+
}
608+
}
609+
553610
// ── Return-type map extraction (Phase 8.2 parity) ───────────────────────────
554611

555612
/// Walk the AST collecting function/method return types into `symbols.return_type_map`.
@@ -3966,6 +4023,45 @@ mod tests {
39664023
assert_eq!(e2.unwrap().type_name, "f2");
39674024
}
39684025

4026+
/// Object literal shorthand method `{ f() {} }` must produce BOTH a bare `f(method)` node
4027+
/// (from handle_method_def, main walk) AND a qualified `o1.f(function)` node (from the
4028+
/// second-pass match_js_objlit_qualified_method_defs), with the bare node appearing FIRST.
4029+
/// findCaller's equal-span tie-break keeps the first entry, so `f(method)` wins for call
4030+
/// attribution — matching WASM where handleMethodCapture runs before extractObjectLiteralFunctions.
4031+
/// Issue #1538.
4032+
#[test]
4033+
fn object_literal_shorthand_method_bare_node_precedes_qualified() {
4034+
let s = parse_js(
4035+
"const o1 = {\n\
4036+
f() { this.g(); },\n\
4037+
g() { return 1; },\n\
4038+
};",
4039+
);
4040+
let names: Vec<_> = s.definitions.iter().map(|d| (&d.name, &d.kind)).collect();
4041+
let f_bare_pos = s.definitions.iter().position(|d| d.name == "f" && d.kind == "method");
4042+
let g_bare_pos = s.definitions.iter().position(|d| d.name == "g" && d.kind == "method");
4043+
let f_qual_pos = s.definitions.iter().position(|d| d.name == "o1.f" && d.kind == "function");
4044+
let g_qual_pos = s.definitions.iter().position(|d| d.name == "o1.g" && d.kind == "function");
4045+
assert!(f_bare_pos.is_some(), "bare f(method) missing; got: {:?}", names);
4046+
assert!(g_bare_pos.is_some(), "bare g(method) missing; got: {:?}", names);
4047+
assert!(f_qual_pos.is_some(), "qualified o1.f(function) missing; got: {:?}", names);
4048+
assert!(g_qual_pos.is_some(), "qualified o1.g(function) missing; got: {:?}", names);
4049+
assert!(
4050+
f_bare_pos.unwrap() < f_qual_pos.unwrap(),
4051+
"f(method) at {} must precede o1.f(function) at {} — equal-span tie-break",
4052+
f_bare_pos.unwrap(), f_qual_pos.unwrap()
4053+
);
4054+
assert!(
4055+
g_bare_pos.unwrap() < g_qual_pos.unwrap(),
4056+
"g(method) at {} must precede o1.g(function) at {}",
4057+
g_bare_pos.unwrap(), g_qual_pos.unwrap()
4058+
);
4059+
// typeMap entry must point to bare name for two-step accessor dispatch.
4060+
let tm_f = s.type_map.iter().find(|e| e.name == "o1.f");
4061+
assert!(tm_f.is_some(), "typeMap o1.f missing");
4062+
assert_eq!(tm_f.unwrap().type_name, "f");
4063+
}
4064+
39694065
/// Phase 8.3e: call receiver is correctly recorded for obj.f() inside defProp body.
39704066
#[test]
39714067
fn call_receiver_for_define_property() {

0 commit comments

Comments
 (0)