mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-29 06:32:28 +00:00
503de8bed8
* Initial plan * feat(SM-12): extract resolveStaticCall from resolveCallTarget - Add resolveStaticCall(className, methodName, currentFile, ctx, argCount?) using lookupClassByName + lookupMethodByOwner for O(1) constructor/static resolution - Add S0 fast path in resolveCallTarget for constructor/free-form class calls - Export resolveStaticCall from call-processor.ts - Add 11 unit tests covering constructor resolution, confidence tiers, arity disambiguation, and resolveCallTarget delegation Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c9471ca9-57ff-4dae-956e-e7ffdc326bc4 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * chore: revert unrelated package-lock.json change Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c9471ca9-57ff-4dae-956e-e7ffdc326bc4 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor: shorten verbose test name per code review feedback Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c9471ca9-57ff-4dae-956e-e7ffdc326bc4 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(SM-12): address PR #754 review findings Addresses Claude's review comments on PR #754: Performance - Pass pre-computed `tiered` result into `resolveStaticCall` as optional `tieredOverride` parameter, eliminating the duplicate `ctx.resolve(className, currentFile)` on every constructor call path. - Cache `freeFormHasClassTarget` in `resolveCallTarget` so the S0 fast path and the free-form constructor retry share a single `.some()` scan. Architecture - Reconcile `CLASS_LIKE_TYPES` (call-processor) with `CLASS_TYPES` (symbol-table): `CLASS_LIKE_TYPES = [...CLASS_TYPES, 'Impl']`. This makes the relationship explicit — the call resolver's set is a strict superset of the heritage-index set, guaranteeing anything reachable via `lookupClassByName` also passes the resolver filter. Trait is now included (harmless: traits have no Constructor nodes, so step-3 returns undefined and step-5 still returns the class-like node when unique). Documented the Interface inclusion rationale (static methods + MRO walker). - Collapse `resolveStaticCall`'s `methodName` parameter into `className` — all call sites passed identical values. Named constructors (Dart `User.fromJson()`) arrive as member calls and go through `resolveMemberCall`. Documented the reserved path for when a language surfaces a static-method-shaped call with a distinct member name. - Document the known gap: `callForm === 'member'` constructor patterns (e.g. Python `models.User()`) are handled by the tail fallback, not S0. Tests - Add tiered-override test asserting `ctx.resolve` is not re-invoked when a pre-computed result is passed in. - Add language-specific `_resolveCallTargetForTesting` integration tests for Java (`new User()`), Python (`User()`), and Kotlin (`User()`). Verification: 3031 unit + 1766 integration tests pass, zero regressions. * fix(SM-12): restrict resolveStaticCall fallback to instantiable kinds Addresses the high-severity finding from the Codex adversarial review of PR #754: `resolveStaticCall`'s step-5 "return the class itself when no Constructor node is found" fallback reused `CLASS_LIKE_TYPES`, which — after SM-11 and PR #754's reconciliation — now includes `Interface`, `Trait`, and `Impl`. That is the method-dispatch set, not the instantiable set, so constructor-shaped calls could resolve to non-instantiable nodes and emit false `CALLS` edges. Concrete failure: Rust same-file `impl User { ... }` alongside `struct User { ... }` — both land at same-file tier, the Impl is not filtered out, and the step-5 fallback produces a `CALLS` edge to the `Impl` block instead of the `Struct`. The same widening exposed Interface / Trait targets in Java / C# / PHP / Scala. Fix - Introduce `INSTANTIABLE_CLASS_TYPES = {'Class', 'Struct', 'Record'}` as a sibling to `CLASS_LIKE_TYPES`, documenting the contract explicitly and cross-referencing `CONSTRUCTOR_TARGET_TYPES`. - Update `CLASS_LIKE_TYPES` JSDoc to clarify it is the method-dispatch set and add an anti-pattern warning against reusing it for constructor-fallback filtering. - Tighten `resolveStaticCall` step 5: filter `classCandidates` through `INSTANTIABLE_CLASS_TYPES` before the `length === 1` check. This strips `Impl` from the Rust shadowing scenario (leaving `Struct` as the sole instantiable target) and null-routes Interface / Trait / `Impl`-alone scenarios, matching the SM-10 R3 null-route precedent. - Step 3 (explicit Constructor lookup via `lookupMethodByOwner`) is intentionally unchanged — its `def.type === 'Constructor'` check is the correct contract, and legitimate Constructor nodes attached to `Impl` owners still resolve correctly. Tests (+10 regression scenarios) - Positive guards: Struct, Record fallback paths. - Null-route: Interface (Java/C#/TS), PHP Trait, Rust Trait. - Rust same-file shadowing: Struct wins over Impl. - Rust Impl-alone: null-routes (no Struct present). - Step-3 preservation: Constructor owned by Impl still resolves to the Constructor node, proving step-5 tightening doesn't leak into step 3. - Full cascade via `_resolveCallTargetForTesting` for Interface and Trait — confirms no downstream path silently re-introduces the edge. Verification - `tsc --noEmit` clean - 3041 unit tests pass (+10) - 1766 integration tests pass - Zero regressions Plan: docs/plans/2026-04-09-002-fix-sm12-constructor-fallback-instantiable-only-plan.md Codex review job: review-mnrao7fr-nv9y0e * fix(SM-12): address PR #754 second review round Addresses the 9 findings from the follow-up review on PR #754. Performance - Align `freeFormHasClassTarget` with `INSTANTIABLE_CLASS_TYPES`: drop `Enum` (S0 would always return null for it — wasted lookup work) and add `Record` (C# records and Kotlin data classes were bypassing S0 entirely). The trigger set and the fallback filter set now agree by construction, documented inline. Documentation - Remove stale single-line JSDoc on `CLASS_LIKE_TYPES` (line 57) that duplicated the full multi-line block immediately below it — tooling picks up the first block so the old one-liner was shadowing the current explanation. - Rewrite the `resolveStaticCall` JSDoc step list to match the actual step boundaries in the implementation (steps 3, 4, 5 were blurred in the old description). - Add inline comment on step 3 documenting the same-name lookup assumption (`${candidate.nodeId}\0${className}`) and the symmetric miss case for Python `__init__`-style constructors. - Add inline comment on step 4 documenting that it also catches the ambiguous-step-3 case, and warning against removing the check without handling that path explicitly. - Add inline comment on step 5 enumerating the three length outcomes (0 / 1 / >1) so future readers see the dominant null-route case. - Document Ruby `User.new` as a known gap alongside Python `models.User()` in the S0 header comment. Tests (+2 scenarios) - Record free-form constructor call via `_resolveCallTargetForTesting` exercises the aligned `freeFormHasClassTarget` trigger end-to-end, closing the gap where the direct `resolveStaticCall` test passed but the integration path was silently bypassing S0. - Arity threading via `_resolveCallTargetForTesting` asserts that `call.argCount` flows through resolveCallTarget → S0 → resolveStaticCall → lookupMethodByOwner, catching any future regression where the argCount is dropped at the S0 call site. Verification - `tsc --noEmit` clean - 3043 unit tests pass (+2) - 1766 integration tests pass - Zero regressions Plan: docs/plans/2026-04-09-002-fix-sm12-constructor-fallback-instantiable-only-plan.md Review: https://github.com/abhigyanpatwari/GitNexus/pull/754#issuecomment-4213536094 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> |
||
|---|---|---|
| .. | ||
| fixtures | ||
| helpers | ||
| integration | ||
| unit | ||
| utils | ||
| global-setup.ts | ||
| vitest.d.ts | ||