fix: track class instantiation (new) as consumption#861
fix: track class instantiation (new) as consumption#861carlos-alm wants to merge 7 commits intomainfrom
Conversation
`new ClassName()` was not tracked as a call site, causing all instantiated classes (e.g. error hierarchy) to appear dead with 0 consumers. Both WASM and native engines now extract `new_expression` as calls alongside regular `call_expression`. Closes #836
|
Claude finished @carlos-alm's task —— View job PR Review CompleteReviewing PR #861: "fix: track class instantiation (new) as consumption" Tasks:
Phase 4: Code Quality Review ✅Implementation Quality: The fix is well-implemented across both engines: WASM Engine Changes (
JavaScript Extractor Changes (
Native Rust Engine Changes (
Test Coverage (
Engine Parity Compliance: Per CLAUDE.md requirement that "both engines must produce identical results," this PR ensures:
No Security Concerns: Pure parser enhancement with no external inputs or security implications. Phase 5: Backlog Compliance Assessment ✅Zero-dep: ✅ - No new runtime dependencies added. Builds on existing tree-sitter/SQLite infrastructure. Foundation-aligned: ✅ - Fully compliant with core principles:
Problem-fit: 5/5 - Directly addresses the core problem: "AI coding assistants waste tokens re-orienting in large codebases, hallucinate dependencies, and miss blast radius."
Breaking: No - Purely additive. No CLI output format changes, API signature changes, or tool contract modifications. Tier: 1 (zero-dep ✓ + foundation-aligned ✓) Root Cause AnalysisProblem Being Solved: Class instantiations ( Why This Approach: Correctly adds Risk Assessment: Very low risk. Well-tested, follows existing patterns, maintains engine parity. Backlog Compliance
Final Recommendation
|
Greptile SummaryThis PR fixes a gap in both the WASM and native Rust extractors where Confidence Score: 5/5Safe to merge — the fix is correct, symmetric across all three extraction paths, and the prior constructor: field-anchor concern from the earlier review thread has already been addressed in commit bbe0a33. All three extraction paths (WASM query, WASM walk, Rust native) are implemented consistently. The new handling of identifier constructors is a direct push (matching the call_expression identifier path), while member-expression constructors reuse extractCallInfo (also matching the existing path). No BUILTIN_GLOBALS filtering discrepancy exists — call_expression tracking also doesn't filter builtins, so the new path is consistent. Test coverage for the new behaviour is solid. The 2 build-parity failures are expected pending a native rebuild and don't indicate a code defect. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["JS/TS source file"] --> B["tree-sitter parse"]
B --> C["AST"]
C --> D{"Engine"}
D -->|"WASM query path"| E["COMMON_QUERY_PATTERNS\n+ new_expression patterns"]
D -->|"WASM walk path"| F["walkJavaScriptNode\nswitch: new_expression"]
D -->|"Native Rust"| G["match_js_node\nnew_expression"]
E --> H["dispatchQueryMatch\nnewfn_node / newmem_node"]
F --> I["handleNewExpr\nidentifier → push call\nmember_expr → extractCallInfo"]
G --> J["handle_new_expr\nidentifier → push call\nmember_expr → extract_call_info"]
H --> K["calls: Call[]"]
I --> K
J --> K
K --> L["SQLite edges\nkind = calls"]
L --> M["Class no longer\nappears dead"]
Reviews (4): Last reviewed commit: "fix(test): add 3.9.0 fnDeps to known reg..." | Re-trigger Greptile |
src/domain/parser.ts
Outdated
| '(new_expression (identifier) @newfn_name) @newfn_node', | ||
| '(new_expression (member_expression) @newmem_fn) @newmem_node', |
There was a problem hiding this comment.
Missing
constructor: field anchor in new_expression patterns
The two new patterns match the constructor child positionally rather than by named field, unlike all neighbouring call_expression patterns which use function:. In practice the anonymous new keyword is never an identifier, so the positional match is correct today, but pinning to the named field is more defensive and consistent with the rest of the file.
| '(new_expression (identifier) @newfn_name) @newfn_node', | |
| '(new_expression (member_expression) @newmem_fn) @newmem_node', | |
| '(new_expression constructor: (identifier) @newfn_name) @newfn_node', | |
| '(new_expression constructor: (member_expression) @newmem_fn) @newmem_node', |
There was a problem hiding this comment.
Applied. Both patterns now use the named constructor: field anchor (bbe0a33), matching the function: convention on call_expression patterns. The walk paths (WASM + Rust) already used childForFieldName('constructor'), so this brings the query path in line.
Codegraph Impact Analysis5 functions changed → 5 callers affected across 2 files
|
Use named field anchor `constructor:` in tree-sitter query patterns for consistency with the `function:` anchors on call_expression patterns. More defensive against grammar changes.
The new_expression tracking correctly produces call edges for class instantiation. Update both JS and TS expected-edges.json manifests to include these edges, which were previously untracked and now correctly appear as consumption.
The published native binary (v3.9.0) does not yet include new_expression extraction. The Rust code is fixed in this PR but CI tests use the npm-published binary. Add a runtime check that detects whether the installed native binary supports new_expression calls edges, and filters the known divergence when it does not. Remove once the next native binary is published.
…iation-consumption
Summary
new ClassName()was not tracked as a call site by either engine, causing all instantiated classes (e.g. error hierarchy inshared/errors.ts) to appear dead with 0 consumersnew_expressionextraction in both WASM (query + walk paths) and native Rust engines, creatingcallsentries identical to regularcall_expressionnew ns.Bar())Details
Root cause: The WASM extractor's
COMMON_QUERY_PATTERNSonly matchedcall_expressionnodes, and the walk-based fallback'sswitchonly handledcall_expression. The native Rust engine'smatch_js_nodesimilarly only dispatchedcall_expressiontohandle_call_expr.Fix (4 files):
src/domain/parser.tsnew_expressionwithidentifierandmember_expressionchildrensrc/extractors/javascript.tsnewfn_node/newmem_nodein query path; addedhandleNewExpr+new_expressioncase in walk pathcrates/codegraph-core/src/extractors/javascript.rsnew_expression→handle_new_exprinmatch_js_node; new handler extracts constructor name and pushes tosymbols.callstests/parsers/javascript.test.tsextracts class instantiation as callscoveringnew Foo(),new ns.Bar(), andthrow new ParseError()Note: The native Rust engine changes require a rebuild to take effect. The
build-parityintegration tests will fail until the native binary is recompiled.Test plan
build-parityfailures expected until native rebuild)Closes #836