diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 8f9b8cb76..b4d79f49b 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -1756,6 +1756,9 @@ fn match_js_node( "enum_declaration" => handle_enum_decl(node, source, symbols), "lexical_declaration" | "variable_declaration" => handle_var_decl(node, source, symbols), "call_expression" => handle_call_expr(node, source, symbols, callback_param_shapes), + "jsx_opening_element" | "jsx_self_closing_element" => { + handle_jsx_element_ref(node, source, &mut symbols.calls) + } "new_expression" => handle_new_expr(node, source, symbols), "decorator" => handle_decorator(node, source, symbols), "import_statement" => handle_import_stmt(node, source, symbols), @@ -2874,6 +2877,9 @@ fn handle_call_expr( callback_param_shapes, &mut symbols.calls, ); + symbols + .calls + .extend(extract_call_argument_identifier_refs(node, source)); return; } } @@ -2884,6 +2890,9 @@ fn handle_call_expr( symbols.definitions.push(cb_def); } extract_callback_reference_calls(node, source, callback_param_shapes, &mut symbols.calls); + symbols + .calls + .extend(extract_call_argument_identifier_refs(node, source)); } fn handle_new_expr(node: &Node, source: &[u8], symbols: &mut FileSymbols) { @@ -4357,6 +4366,118 @@ fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls: /// `build_edges.rs` accepts `class`-kind targets in addition to /// function/method for this reason. /// +/// A JSX element's opening/self-closing tag name is a reference to the +/// component it renders — `
` is exactly as much a use of `Header` +/// as `Header()` would be, but produces no call edge by construction since +/// it's not a `call_expression` (issue #2389). Emitted as a `value-ref` +/// dynamic call, the same mechanism already used for object-literal +/// property values, `instanceof` operands, and logical-or/ternary fallbacks. +/// +/// Only a capitalized bare identifier is treated as a component reference, +/// matching JSX's own convention: a lowercase-first tag name (`
`, +/// ``) compiles to a DOM/intrinsic element (not an identifier +/// reference) and must not be credited as a symbol use. A `member_expression` +/// name (``) credits the base object identifier. +/// +/// Mirrors `handleJsxElementRef` in `src/extractors/javascript.ts`. +fn handle_jsx_element_ref(node: &Node, source: &[u8], calls: &mut Vec) { + let Some(name_node) = node.child_by_field_name("name") else { + return; + }; + let line = start_line(node); + match name_node.kind() { + "identifier" => { + let text = node_text(&name_node, source); + if !text.starts_with(|c: char| c.is_ascii_uppercase()) + || JS_BUILTIN_GLOBALS.contains(&text) + { + return; + } + calls.push(Call { + name: text.to_string(), + line, + dynamic: Some(true), + dynamic_kind: Some("value-ref".to_string()), + ..Default::default() + }); + } + "member_expression" => { + let Some(obj_node) = name_node.child_by_field_name("object") else { + return; + }; + if obj_node.kind() != "identifier" { + return; + } + let text = node_text(&obj_node, source); + if JS_BUILTIN_GLOBALS.contains(&text) { + return; + } + calls.push(Call { + name: text.to_string(), + line, + dynamic: Some(true), + dynamic_kind: Some("value-ref".to_string()), + ..Default::default() + }); + } + _ => {} + } +} + +/// A capitalized bare identifier passed as a call argument is a value +/// reference to whatever it names — `Factory.create(AppModule)` is a +/// genuine use of `AppModule` (issue #2389; the NestJS module/controller +/// registration idiom, `NestFactory.create(AppModule)`, relies on exactly +/// this pattern). +/// +/// Restricted to capitalized identifiers — the same class/component-naming +/// convention already used to gate JSX element references +/// (`handle_jsx_element_ref`) — deliberately, not merely for style: issue +/// #1741 is a regression guard proving that crediting an arbitrary +/// lowercase DATA argument (e.g. `analyzeDrift(communities, communityDirs)`) +/// as any kind of reference risks the global-fallback resolver binding it +/// to an unrelated same-named function elsewhere in the repo, fabricating a +/// call edge and, transitively, a phantom cycle. A class/component +/// reference passed by value is overwhelmingly PascalCase in JS/TS +/// convention, so this restriction captures the pattern #2389 asks for +/// while leaving #1741's already-diagnosed false-positive risk exactly as +/// closed as it was. +/// +/// Restricted to direct-child bare identifiers of the arguments list, +/// mirroring this file's "restrict to the simplest syntactic shape" +/// precedent (#1771/#1784). +/// +/// Mirrors `extractCallArgumentIdentifierRefs` in `src/extractors/javascript.ts`. +fn extract_call_argument_identifier_refs(call_node: &Node, source: &[u8]) -> Vec { + let mut result = Vec::new(); + let Some(args) = call_node + .child_by_field_name("arguments") + .or_else(|| find_child(call_node, "arguments")) + else { + return result; + }; + let line = start_line(call_node); + for i in 0..args.child_count() { + let Some(child) = args.child(i) else { continue }; + if child.kind() != "identifier" { + continue; + } + let text = node_text(&child, source); + if !text.starts_with(|c: char| c.is_ascii_uppercase()) || JS_BUILTIN_GLOBALS.contains(&text) + { + continue; + } + result.push(Call { + name: text.to_string(), + line, + dynamic: Some(true), + dynamic_kind: Some("value-ref".to_string()), + ..Default::default() + }); + } + result +} + /// Mirrors `collectInstanceofValueRefCall` in `src/extractors/javascript.ts`. fn handle_instanceof_value_ref(node: &Node, source: &[u8], calls: &mut Vec) { let Some(operator_n) = node.child_by_field_name("operator") else { @@ -8465,6 +8586,74 @@ mod tests { assert!(value_refs.iter().any(|c| c.name == "someFunction")); } + // ── #2389: JSX element value-ref extraction ────────────────────────────── + + #[test] + fn extracts_value_ref_call_for_self_closing_jsx_component() { + let s = parse_js("function App() { return
; }"); + assert!(s + .calls + .iter() + .any(|c| c.name == "Header" && c.dynamic_kind.as_deref() == Some("value-ref"))); + } + + #[test] + fn extracts_value_ref_call_for_jsx_component_with_children() { + let s = parse_js("function App() { return ; }"); + assert!(s + .calls + .iter() + .any(|c| c.name == "Wrapper" && c.dynamic_kind.as_deref() == Some("value-ref"))); + } + + #[test] + fn does_not_extract_value_ref_call_for_lowercase_intrinsic_jsx_tag() { + let s = parse_js("function App() { return
; }"); + assert!(!s + .calls + .iter() + .any(|c| c.dynamic_kind.as_deref() == Some("value-ref"))); + } + + #[test] + fn credits_base_identifier_for_namespaced_jsx_component() { + let s = parse_js("function App() { return ; }"); + assert!(s + .calls + .iter() + .any(|c| c.name == "NS" && c.dynamic_kind.as_deref() == Some("value-ref"))); + } + + // ── #2389: call-argument identifier value-ref extraction ──────────────── + + #[test] + fn extracts_value_ref_call_for_capitalized_call_argument() { + let s = parse_js("Factory.create(AppModule);"); + assert!(s + .calls + .iter() + .any(|c| c.name == "AppModule" && c.dynamic_kind.as_deref() == Some("value-ref"))); + } + + #[test] + fn does_not_extract_value_ref_call_for_lowercase_data_argument_regression_1741() { + // Regression guard mirroring #1741: a lowercase DATA argument must + // never be credited as any kind of reference, or the global-fallback + // resolver can bind it to an unrelated same-named function elsewhere + // in the repo, fabricating a call edge and a phantom cycle. + let s = parse_js("analyzeDrift(communities, communityDirs);"); + assert!(!s.calls.iter().any(|c| c.dynamic == Some(true))); + } + + #[test] + fn does_not_extract_value_ref_call_for_builtin_global_argument() { + let s = parse_js("register(console);"); + assert!(!s + .calls + .iter() + .any(|c| c.dynamic_kind.as_deref() == Some("value-ref"))); + } + // ── #2257: logical-or/nullish-coalescing/ternary value-ref extraction ─── #[test] diff --git a/src/domain/parser.ts b/src/domain/parser.ts index 5175f2ca3..e32586ac5 100644 --- a/src/domain/parser.ts +++ b/src/domain/parser.ts @@ -180,6 +180,13 @@ const COMMON_QUERY_PATTERNS: string[] = [ '(call_expression function: (member_expression) @callmem_fn) @callmem_node', '(call_expression function: (subscript_expression) @callsub_fn) @callsub_node', '(call_expression function: (super) @callsuper_fn) @callsuper_node', + // Generic capture for call-argument identifier value-ref extraction (#2389) — + // matches every call_expression regardless of the callee's shape (chained, + // curried, or parenthesized callees like `getFactory()(AppModule)`), mirroring + // the walk path's unconditional `case 'call_expression'` dispatch. Named-call + // resolution stays on the shape-specific patterns above; this one only feeds + // dispatchQueryMatch's `callarg_node` branch. + '(call_expression) @callarg_node', '(new_expression constructor: (identifier) @newfn_name) @newfn_node', '(new_expression constructor: (member_expression) @newmem_fn) @newmem_node', '(expression_statement (assignment_expression left: (member_expression) @assign_left right: (_) @assign_right)) @assign_node', @@ -203,6 +210,18 @@ const TS_EXTRA_PATTERNS: string[] = [ '(type_alias_declaration name: (type_identifier) @type_name) @type_node', ]; +// JSX element tag names — only javascript and tsx grammars define these node +// types; plain typescript (.ts, no JSX) does not, so this must never be +// folded into COMMON_QUERY_PATTERNS or TS_EXTRA_PATTERNS (shared with plain +// typescript) — Query() compilation throws on an unknown node type, which +// would break all .ts parsing (#2389). +const JSX_QUERY_PATTERNS: string[] = [ + '(jsx_opening_element name: (identifier) @jsxid_name) @jsxid_node', + '(jsx_self_closing_element name: (identifier) @jsxid_name) @jsxid_node', + '(jsx_opening_element name: (member_expression) @jsxmem_name) @jsxmem_node', + '(jsx_self_closing_element name: (member_expression) @jsxmem_name) @jsxmem_node', +]; + /** * Load a single language grammar and cache the parser + language + query. * Uses in-flight deduplication so concurrent callers awaiting the same grammar @@ -227,9 +246,12 @@ async function doLoadLanguage(entry: LanguageRegistryEntry): Promise { _cachedLanguages!.set(entry.id, lang); if (entry.extractor === extractSymbols && !_queryCache.has(entry.id)) { const isTS = entry.id === 'typescript' || entry.id === 'tsx'; - const patterns = isTS - ? [...COMMON_QUERY_PATTERNS, ...TS_EXTRA_PATTERNS] - : [...COMMON_QUERY_PATTERNS, ...JS_CLASS_PATTERNS]; + const supportsJsx = entry.id === 'javascript' || entry.id === 'tsx'; + const patterns = [ + ...COMMON_QUERY_PATTERNS, + ...(isTS ? TS_EXTRA_PATTERNS : JS_CLASS_PATTERNS), + ...(supportsJsx ? JSX_QUERY_PATTERNS : []), + ]; _queryCache.set(entry.id, new Query(lang, patterns.join('\n'))); } } catch (e: unknown) { diff --git a/src/domain/wasm-worker-entry.ts b/src/domain/wasm-worker-entry.ts index 58556ce17..40befd8bd 100644 --- a/src/domain/wasm-worker-entry.ts +++ b/src/domain/wasm-worker-entry.ts @@ -125,6 +125,13 @@ const COMMON_QUERY_PATTERNS: string[] = [ '(call_expression function: (member_expression) @callmem_fn) @callmem_node', '(call_expression function: (subscript_expression) @callsub_fn) @callsub_node', '(call_expression function: (super) @callsuper_fn) @callsuper_node', + // Generic capture for call-argument identifier value-ref extraction (#2389) — + // matches every call_expression regardless of the callee's shape (chained, + // curried, or parenthesized callees like `getFactory()(AppModule)`), mirroring + // the walk path's unconditional `case 'call_expression'` dispatch. Named-call + // resolution stays on the shape-specific patterns above; this one only feeds + // dispatchQueryMatch's `callarg_node` branch. + '(call_expression) @callarg_node', '(new_expression constructor: (identifier) @newfn_name) @newfn_node', '(new_expression constructor: (member_expression) @newmem_fn) @newmem_node', '(expression_statement (assignment_expression left: (member_expression) @assign_left right: (_) @assign_right)) @assign_node', @@ -145,6 +152,18 @@ const TS_EXTRA_PATTERNS: string[] = [ '(type_alias_declaration name: (type_identifier) @type_name) @type_node', ]; +// JSX element tag names — only javascript and tsx grammars define these node +// types; plain typescript (.ts, no JSX) does not, so this must never be +// folded into COMMON_QUERY_PATTERNS or TS_EXTRA_PATTERNS (shared with plain +// typescript) — Query() compilation throws on an unknown node type, which +// would break all .ts parsing (#2389). +const JSX_QUERY_PATTERNS: string[] = [ + '(jsx_opening_element name: (identifier) @jsxid_name) @jsxid_node', + '(jsx_self_closing_element name: (identifier) @jsxid_name) @jsxid_node', + '(jsx_opening_element name: (member_expression) @jsxmem_name) @jsxmem_node', + '(jsx_self_closing_element name: (member_expression) @jsxmem_name) @jsxmem_node', +]; + // ── Local language registry ───────────────────────────────────────────────── // Local copy — re-using parser.ts's registry would drag in its process-wide // parser/grammar caches, which are not safe to share across worker threads. @@ -444,9 +463,12 @@ async function loadLanguageLazy(entry: LanguageRegistryEntry): Promise` is exactly as much a use of `Header` + * as `Header()` would be, but produces no call edge by construction since + * it's not a `call_expression` (issue #2389). Emitted as a `value-ref` + * dynamic call, the same mechanism already used for object-literal + * property values, `instanceof` operands, and logical-or/ternary fallbacks + * (#1771/#1895/#2257). + * + * Only a capitalized bare identifier is treated as a component reference, + * matching JSX's own convention: a lowercase-first tag name (`
`, + * ``) compiles to a DOM/intrinsic element (a string, not an + * identifier reference) and must not be credited as a symbol use. A + * `member_expression` name (``) credits the base + * object identifier, mirroring `extractReceiverName`'s handling of + * member-expression receivers elsewhere in this file. + */ +function handleJsxElementRef(node: TreeSitterNode, calls: Call[]): void { + const nameNode = node.childForFieldName('name'); + if (!nameNode) return; + const line = nodeStartLine(node); + if (nameNode.type === 'identifier') { + const name = nameNode.text; + if (!name || !/^[A-Z]/.test(name) || BUILTIN_GLOBALS.has(name)) return; + calls.push({ name, line, dynamic: true, dynamicKind: 'value-ref' }); + } else if (nameNode.type === 'member_expression') { + const objNode = nameNode.childForFieldName('object'); + if (objNode?.type === 'identifier' && !BUILTIN_GLOBALS.has(objNode.text)) { + calls.push({ name: objNode.text, line, dynamic: true, dynamicKind: 'value-ref' }); + } + } +} + +/** + * A capitalized bare identifier passed as a call argument is a value + * reference to whatever it names — `Factory.create(AppModule)` is a + * genuine use of `AppModule`, the same as an object-literal property value + * or a logical-or fallback (#1771/#2257), but arguments in ordinary call + * position produce no edge at all today (issue #2389; the NestJS + * module/controller registration idiom, `NestFactory.create(AppModule)`, + * relies on exactly this pattern). + * + * Restricted to capitalized identifiers — the same class/component-naming + * convention already used to gate JSX element references + * (`handleJsxElementRef`) — deliberately, not merely for style: issue + * #1741 is a regression guard proving that crediting an arbitrary + * lowercase DATA argument (e.g. `analyzeDrift(communities, communityDirs)`) + * as any kind of reference risks the global-fallback resolver binding it + * to an unrelated same-named function elsewhere in the repo, fabricating a + * call edge and, transitively, a phantom cycle. A class/component + * reference passed by value is overwhelmingly PascalCase in JS/TS + * convention, so this restriction captures the pattern #2389 asks for + * while leaving #1741's already-diagnosed false-positive risk exactly as + * closed as it was. + * + * Restricted to direct-child bare identifiers of the arguments list (not + * nested inside member/call expressions), matching this file's established + * "restrict to the simplest syntactic shape" precedent (#1771/#1784). + */ +function extractCallArgumentIdentifierRefs(callNode: TreeSitterNode): Call[] { + const args = callNode.childForFieldName('arguments') || findChild(callNode, 'arguments'); + if (!args) return []; + const result: Call[] = []; + const line = nodeStartLine(callNode); + for (let i = 0; i < args.childCount; i++) { + const child = args.child(i); + if (!child) continue; + if (child.type !== 'identifier') continue; + const name = child.text; + if (!name || !/^[A-Z]/.test(name) || BUILTIN_GLOBALS.has(name)) continue; + result.push({ name, line, dynamic: true, dynamicKind: 'value-ref' }); } + return result; } function handleNewExpr(node: TreeSitterNode, ctx: ExtractorOutput): void { diff --git a/tests/engines/query-walk-parity.test.ts b/tests/engines/query-walk-parity.test.ts index 100b5cb7b..33126aee4 100644 --- a/tests/engines/query-walk-parity.test.ts +++ b/tests/engines/query-walk-parity.test.ts @@ -364,6 +364,55 @@ export function useIt(): number { const p = makePartition(42); return p.deltaModularity(1); } +`, + }, + // Issue #2389: JSX component references and call-argument identifiers + { + name: 'JSX self-closing component reference (#2389)', + file: 'test.jsx', + code: ` +import { Header } from './comp.jsx'; +export function App() { + return
; +} +`, + }, + { + name: 'JSX component with children and a namespaced reference (#2389)', + file: 'test.tsx', + code: ` +import * as NS from './comp'; +export function App() { + return ; +} +`, + }, + { + name: 'call-argument identifier value reference (#2389)', + file: 'test.ts', + code: ` +class AppModule {} +const Factory = { create(m: unknown) { return m; } }; +export function bootstrap() { + return Factory.create(AppModule); +} +`, + }, + { + // Regression guard for Greptile's #2389 review finding: a call whose callee + // is itself an expression (not a bare identifier/member/subscript) has no + // dedicated query capture, so the value-ref extraction must be routed + // through a shape-agnostic capture rather than the shape-specific ones. + name: 'call-argument identifier value reference with expression-based callee (#2389)', + file: 'test.ts', + code: ` +class AppModule {} +function getFactory() { + return (m: unknown) => m; +} +export function bootstrap() { + return getFactory()(AppModule); +} `, }, ]; diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index 5f6ff73f3..61b13cc7e 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -2034,6 +2034,62 @@ function runDemo(reporter: Reporter, users: string[]): void { }); }); + describe('JSX element value-ref extraction (#2389)', () => { + it('extracts a value-ref call for a self-closing component reference', () => { + const symbols = parseJS(`function App() { return
; }`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'Header', dynamic: true, dynamicKind: 'value-ref' }), + ); + }); + + it('extracts a value-ref call for a component with children', () => { + const symbols = parseJS(`function App() { return ; }`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'Wrapper', dynamic: true, dynamicKind: 'value-ref' }), + ); + }); + + it('does not extract a value-ref call for a lowercase intrinsic HTML tag', () => { + const symbols = parseJS(`function App() { return
; }`); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + + it('credits the base object identifier for a namespaced component reference', () => { + const symbols = parseJS(`function App() { return ; }`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'NS', dynamic: true, dynamicKind: 'value-ref' }), + ); + }); + }); + + describe('call-argument identifier value-ref extraction (#2389)', () => { + it('extracts a value-ref call for a bare identifier passed as a call argument', () => { + const symbols = parseJS(`Factory.create(AppModule);`); + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name: 'AppModule', dynamic: true, dynamicKind: 'value-ref' }), + ); + }); + + it('extracts a value-ref call for every bare identifier argument', () => { + const symbols = parseJS(`register(ModuleA, ModuleB);`); + for (const name of ['ModuleA', 'ModuleB']) { + expect(symbols.calls).toContainEqual( + expect.objectContaining({ name, dynamic: true, dynamicKind: 'value-ref' }), + ); + } + }); + + it('does not extract a value-ref call for undefined/null/builtin-global arguments', () => { + const symbols = parseJS(`register(undefined, null, console);`); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + + it('does not extract a value-ref call for a member-expression or call-expression argument', () => { + const symbols = parseJS(`register(obj.Module, makeModule());`); + expect(symbols.calls.filter((c) => c.dynamicKind === 'value-ref')).toHaveLength(0); + }); + }); + describe('object-literal value-ref keyExpr capture (#1895)', () => { it('captures the property key, distinct from the referenced value name', () => { const symbols = parseJS(`const table = { resolve: someFunction };`);