fix(parity): infer var-typed local types from new-expression initializers in C##1424
Conversation
…ion guard (#1407) Adds class-scope.js to the JS resolution benchmark. The fixture contains a class with a bare flush() call inside run() and a Processor.flush method that must NOT be the target of that call — JS bare calls are lexically scoped to the module, never the class. The 1.0 precision floor for the JS fixture acts as the enforcement mechanism: if the call.receiver guard in resolveByMethodOrGlobal is ever removed, the resolver would emit Processor.run → Processor.flush as a false positive, immediately failing CI. Closes #1407
…alizers in C# Both the WASM and native C# extractors were skipping variable declarations with `var`/`implicit_type` type nodes entirely, so `var service = new UserService(repo)` never added `service → UserService` to the typeMap. The call-edge resolver therefore could not resolve `service.AddUser()` or `service.GetUser()` to the qualified methods on `UserService`. The dist copy of the WASM extractor already had the fix (extractVarInitType), but the source had drifted out of sync. This commit re-introduces the logic in both engines: WASM (TypeScript): add `extractVarInitType(declarator)` that walks the variable_declarator children looking for an `object_creation_expression` (or one inside an `equals_value_clause`), then reads the `type` field via `extractCSharpTypeName`. `handleCSharpVarDecl` now sets `isVar` for `implicit_type | var_keyword` and calls `extractVarInitType` in that branch. Native (Rust): mirror the same logic in `extract_var_init_type` and update `match_csharp_type_map` to drive it when the type node is `var_keyword` or `implicit_type`. Infrastructure: add CODEGRAPH_NATIVE_ADDON_PATH env-var override to `loadNative()` for local development workflows where the published npm binary can't be loaded (its dylib install-name points to the CI build path). Tests now pass with CODEGRAPH_NATIVE_ADDON_PATH set to the locally rebuilt binary. Fixes: #1418 (build-parity test failure — native vs WASM C# edge/role divergence on `var`-typed local variables instantiated with `new`).
Greptile SummaryThis commit resolves the stale PR reference comments that were flagged in the previous review round —
Confidence Score: 5/5Safe to merge — both changed files contain only comment corrections with no functional code impact. Both changes are single-line comment fixes updating stale PR reference numbers. No logic, test assertions, or runtime behaviour is modified. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["PR #1424 — comment-ref fix commit"] --> B["tests/benchmarks/resolution/\nresolution-benchmark.test.ts"]
A --> C["tests/unit/\ncall-resolver.test.ts"]
B --> D["#1407 → #1422\n(bare-call guard PR reference)"]
C --> E["#1407 → #1422/#1424\n(dual-PR attribution)"]
Reviews (11): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // define-property.js and accessorGetter→accessorTarget.accessMethod in define-property-accessor.js, | ||
| // total expected now 35. | ||
| // total expected now 35. multi-class.js adds 4 class-scoped typeMap edges → 39. | ||
| // #1407 adds class-scope.js (bare-call guard), +1 → total 40. |
There was a problem hiding this comment.
class-scope.js to PR #1407, but that file is introduced in this PR (#1424). The reference pattern used elsewhere in this comment block (e.g. #1335) refers to PR numbers, so this looks like a copy-paste from a draft. Suggest updating to the actual PR number so the history trail stays accurate.
| // #1407 adds class-scope.js (bare-call guard), +1 → total 40. | |
| // #1424 adds class-scope.js (bare-call guard), +1 → total 40. |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| } catch (err) { | ||
| _loadError = err as Error; | ||
| _cached = null; | ||
| return null; | ||
| } |
There was a problem hiding this comment.
null with no diagnostic output. Every other load-failure path in this file sets _loadError, and callers surface the message through getNative(). Here the error is recorded but there's no debug() call, which means a developer who misspells the path or points to a stale binary will see native silently unavailable with no log hint — even when DEBUG is enabled.
| } catch (err) { | |
| _loadError = err as Error; | |
| _cached = null; | |
| return null; | |
| } | |
| } catch (err) { | |
| _loadError = err as Error; | |
| debug(`loadNative: CODEGRAPH_NATIVE_ADDON_PATH=${devOverride} failed: ${toErrorMessage(err as Error)}`); | |
| _cached = null; | |
| return null; | |
| } |
There was a problem hiding this comment.
Fixed — the merge conflict resolution adopted main's version of native.ts (PR #1389), which already includes a warn() call on the failure path (more visible than debug()). The original PR's simpler CODEGRAPH_NATIVE_ADDON_PATH approach was superseded by the more complete implementation from main that uses NAPI_RS_NATIVE_LIBRARY_PATH + automatic local binary detection.
Codegraph Impact Analysis10 functions changed → 22 callers affected across 8 files
|
In JS/TS, bare foo() calls inside a class method are module-scoped — there is no implicit class binding. The same-class fallback in resolveByMethodOrGlobal was incorrectly emitting class-scoped edges for bare calls with no module-level match (e.g. flush() inside Processor.run resolving to Processor.flush). Gate the fallback: skip for bare calls in JS/TS, keep for this.method() calls and for C#/Java where bare calls are class-scoped.
1 participant
Summary
var/implicit_typevariable declarations entirely.var service = new UserService(repo)never addedservice → UserServiceto the typeMap, soservice.AddUser()couldn't be resolved toUserService.AddUser.src/extractors/csharp.ts): AddextractVarInitTypethat infers the constructor type fromvar x = new Foo()initializers — syncs source with pre-existing dist logic that had drifted out of sync.crates/codegraph-core/src/extractors/csharp.rs): Mirror the same logic, enabling the Rust engine to emit the samecalls/receiveredges as WASM.src/infrastructure/native.ts): AddCODEGRAPH_NATIVE_ADDON_PATHenv-var override toloadNative()for local development workflows where the published npm binary cannot be loaded (its dylib install-name points to the CI build path).Closes #1418.
Test plan
CODEGRAPH_NATIVE_ADDON_PATH=... npx vitest run tests/integration/build-parity.test.ts— 8/8 pass (previously 2 failed with edge/role divergence)npx vitest run tests/parsers/csharp.test.ts— 13/13 passTo run parity tests locally, first rebuild the native addon:
Before: native 68 edges vs WASM 78 edges — instance-method calls via
var-typed locals missingAfter: native 74 edges vs WASM 78 edges — parity on
calls/receiveredges for instance method dispatch throughvar-typed locals