Skip to content

Extract resolveFreeCall from resolveCallTarget (SM-13)#756

Merged
magyargergo merged 7 commits into
mainfrom
copilot/sm-13-extract-resolvefreecall
Apr 9, 2026
Merged

Extract resolveFreeCall from resolveCallTarget (SM-13)#756
magyargergo merged 7 commits into
mainfrom
copilot/sm-13-extract-resolvefreecall

Conversation

Copilot AI commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Extracts the free-function call resolution path into a standalone resolveFreeCall(calledName, filePath, ctx), following the same pattern as resolveMemberCall (SM-11) and resolveStaticCall (SM-12).

resolveFreeCall

New exported function handling all callForm === 'free' calls:

  • Uses ctx.resolve() for tiered lookup (lookupExact → import-scoped → global)
  • Class-target fast path delegates to resolveStaticCall for Swift/Kotlin implicit constructors (User())
  • Constructor-form retry when resolveStaticCall misses
  • Overload disambiguation + Swift extension dedup preserved
export const resolveFreeCall = (
  calledName: string,
  filePath: string,
  ctx: ResolutionContext,
  argCount?: number,
  tieredOverride?: TieredCandidates,
  overloadHints?: OverloadHints,
  preComputedArgTypes?: (string | undefined)[],
): ResolveResult | null => { ... }

resolveCallTarget simplification

  • Dispatches callForm === 'free' to resolveFreeCall before any other logic
  • Removes freeFormHasClassTarget variable and the Swift/Kotlin constructor retry block
  • S0 block narrowed to callForm === 'constructor' only

Design note

resolveFreeCall still falls through to Tier 3 (global) via ctx.resolve(). Fuzzy remains on this path until Phase 5 replaces lookupFuzzy.

Tests

9 new unit tests: import-scoped, same-file, null-routing (unknown/ambiguous/Trait), class-target delegation, Record targets, tieredOverride short-circuit, and resolveCallTarget dispatch verification.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • extension.ladybugdb.com
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/GitNexus/GitNexus/gitnexus/node_modules/.bin/vitest run test/unit/symbol-table.test.ts --reporter=verbose (dns block)
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/GitNexus/GitNexus/gitnexus/node_modules/.bin/vitest run test/unit/symbol-table.test.ts --reporter=verbose ite(sys.executable.encode('utf-8-I (dns block)
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/GitNexus/GitNexus/gitnexus/node_modules/.bin/vitest run --reporter=verbose xus/�� # We remove slashes and replace spaces with new lines; (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@vercel

vercel Bot commented Apr 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gitnexus Ready Ready Preview, Comment Apr 9, 2026 4:31pm

Request Review

Extract the free-function call resolution path into a dedicated
`resolveFreeCall(calledName, filePath, ctx)` function that uses
`lookupExact` + import-scoped resolution via `ctx.resolve()`.

- Free function calls (foo()) now route through `resolveFreeCall`
- Swift/Kotlin implicit constructors (User()) delegate to
  `resolveStaticCall` within `resolveFreeCall`
- `resolveCallTarget` dispatches `callForm === 'free'` early,
  removing the inline freeFormHasClassTarget logic
- S0 block simplified to only handle `callForm === 'constructor'`
- Global (Tier 3) fallthrough preserved via ctx.resolve() until Phase 5
- 9 new unit tests for resolveFreeCall
- All 163 unit tests pass, all 1199 integration resolver tests pass

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c5f2e73a-259a-438c-b5c8-286b82e3c215

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>
Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c5f2e73a-259a-438c-b5c8-286b82e3c215

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>
Copilot AI changed the title [WIP] Extract resolveFreeCall from resolveCallTarget Extract resolveFreeCall from resolveCallTarget (SM-13) Apr 9, 2026
Copilot AI requested a review from magyargergo April 9, 2026 11:31
@magyargergo

Copy link
Copy Markdown
Collaborator

@claude review these changes along with the plan on the issue. You need to look for gaps and performance issues. You are also tasked with reviewing the architectureal fit. Also we need to ensure we have coverage for these per languages. Also include other comments in your evaluation.

@github-actions

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 3m 3s —— View job


PR Review: Extract resolveFreeCall from resolveCallTarget (SM-13)

Tasks:

  • Gather context and read changed files
  • Review resolveFreeCall implementation (gaps, correctness)
  • Review architectural fit vs SM-11/SM-12 pattern
  • Review performance characteristics
  • Review per-language coverage in tests
  • Review test quality and gaps
  • Post final review

Overall Assessment

The extraction is clean and architecturally consistent with SM-11/SM-12. The early-dispatch pattern in resolveCallTarget, the tieredOverride short-circuit, and the JSDoc all follow established conventions. No regressions expected. However, there are several gaps worth addressing before merge.


Gaps & Issues

1. INSTANTIABLE_CLASS_TYPES not reused in hasClassTarget check — maintenance hazard

call-processor.ts:1968–1970

const hasClassTarget =
  filteredCandidates.length === 0 &&
  tiered.candidates.some((c) => c.type === 'Class' || c.type === 'Struct' || c.type === 'Record');

The comment at line 1963–1966 already warns about drift, but the fix is right there: INSTANTIABLE_CLASS_TYPES.has(c.type). Using the Set directly would make this self-enforcing. As-is, a future addition to INSTANTIABLE_CLASS_TYPES (e.g., Kotlin object) would silently not apply to the fast path in resolveFreeCall.

Fix this →


2. Language coverage gaps in tests

The 9 new tests cover Swift/Kotlin (class delegation), C# Records, PHP Traits, and TypeScript-flavoured paths. Missing coverage:

Language Free-call pattern Covered?
Go doStuff(), fmt.Println() ❌ No test
Python def helper()helper() ❌ No test
Rust Free functions outside impl ❌ No test
Java Static imports (import static Utils.doStuff) ❌ No test
Kotlin User() implicit constructor Only tested via proxy (same path as Swift)
JavaScript Module-level function call ❌ No test

Go is a particularly important gap — Go codebases make heavy use of free functions and have no method-dispatch syntax, so all calls flow through resolveFreeCall.


3. No test for arity filtering (argCount) in resolveFreeCall

The argCount parameter is threaded through to filterCallableCandidates, but none of the 9 new tests exercise it — every test either has a single candidate or uses the ambiguous (multi-candidate) null-routing test. A test with two overloaded functions differing only in parameter count would verify that argCount actually narrows the result.


4. Swift extension deduplication uses file-path length as a proxy for "primary definition"

call-processor.ts:2004–2008

const sorted = [...filteredCandidates].sort(
  (a, b) => a.filePath.length - b.filePath.length,
);
return toResolveResult(sorted[0], tiered.tier);

This heuristic assumes the primary definition lives at the shorter path. That assumption breaks for monorepos where the canonical definition is at e.g. packages/core/src/components/Button/Button.swift (long path) and the extension is at src/ext.swift (short path). The same dedup logic existed before SM-13, but there's still no test that validates the shortest-path-wins assumption. Consider adding one to document and guard the behaviour.


5. No test for global (Tier 3) resolution path

The design note at line 1929–1931 explicitly says the function "still falls through to Tier 3 (global) via ctx.resolve()". The existing tests rely on import-scoped (Tier 2, confidence 0.9) and same-file (Tier 1, confidence 0.95) resolution. A test where a function is globally visible but not imported would exercise the Tier 3 path and document its confidence level (expected to be lower, e.g. 0.7 or 0.5 depending on TIER_CONFIDENCE values).


6. No test for preComputedArgTypes disambiguation path (worker path)

The preComputedArgTypes parameter is present in the signature and correctly wired through to matchCandidatesByArgTypes, but it is not tested in any of the 9 new tests. This is the worker path (pre-computed in parse-worker before the main thread resolves). A missing test here means any regression in that branch won't be caught.


7. Enum free-form calls are silently null-routed

Enum is excluded from INSTANTIABLE_CLASS_TYPES (by design, documented at line 94–95), so Kotlin.Color() or Java Color.RED free-call patterns will produce filteredCandidates.length === 0 AND hasClassTarget === false, returning null. This is the stated "pending" position, but there's no test asserting that Enum calls are null-routed with a comment linking back to the tracking work. Worth adding a skipped/pending test to make the gap visible.


Architectural Fit

The SM-13 extraction fits cleanly into the established pattern:

  • resolveCallTarget dispatches on callForm === 'free' before any other logic (same dispatch order as SM-12 for 'constructor')
  • tieredOverride avoids a redundant ctx.resolve() call — the one at resolveCallTarget:1370 is pre-computed and passed in
  • _resolveCallTargetForTesting correctly exposes the dispatch path
  • freeFormHasClassTarget variable cleanly removed from resolveCallTarget
  • ✅ S0 block now correctly narrowed to callForm === 'constructor' only

One subtle design point worth noting: when hasClassTarget is true and resolveStaticCall returns null, the constructor-form retry at lines 1977–1981 re-filters tiered.candidates with 'constructor' form. The result may include Constructor nodes that were excluded by the earlier 'free' filter. This is intentional and correct, but the path from "free call with a class target that has no constructor" → "constructor-form retry" → "Swift extension dedup" is not explicitly tested end-to-end.


Performance

No regressions introduced. The hasClassTarget .some() short-circuits on the first matching type; for typical candidate pools (1–5 entries) this is negligible. The main throughput concern remains the pre-existing ctx.resolve() call in resolveCallTarget for all call forms, which is unchanged and justified by the need to pass tiered as tieredOverride.


Minor

  • The test file name is symbol-table.test.ts but now contains describe('resolveFreeCall', ...) at line 2338. This is following the same convention as SM-11 and SM-12 tests (which also live here), but a future rename or split into call-processor.test.ts would improve discoverability.
  • The // SM-13: Free function calls route through resolveFreeCall. comment at resolveCallTarget:1373 is consistent with the SM-11/SM-12 comments — good.

Addresses all 7 findings from the PR #756 review comment.

Code (R1, finding #1)
- Replace the literal `'Class' | 'Struct' | 'Record'` check in
  `hasClassTarget` with `INSTANTIABLE_CLASS_TYPES.has(c.type)`. Converts
  an invariant that was previously comment-enforced ("keep this list
  aligned with INSTANTIABLE_CLASS_TYPES") into one enforced structurally.
  Any future extension of the set propagates here automatically. The
  narrower Swift extension dedup block below still uses literal
  `'Class' | 'Struct'` by design — Swift extensions only produce Class
  duplicates in practice, Record is deliberately excluded there, and
  the inline comment now documents that asymmetry.

Tests (+12 regression scenarios)

Finding #2 — language coverage
- Go free function (doStuff())
- Python free function (def helper(): ... helper())
- Rust free function outside any impl block
- Java statically-imported function
- JavaScript module-level function
Each exercises `_resolveCallTargetForTesting` with `callForm='free'`
and the language-specific file extension. `resolveFreeCall` has no
file-extension branching, so these guard the dispatch chain per
language without assuming extractor-specific symbol shapes.

Finding #3 — argCount threading
- 2-arg overload selected when argCount=2
- 0-arg overload selected when argCount=0

Finding #5 — Tier 3 (global) resolution
- Function globally visible but not imported. Asserts exact
  `TIER_CONFIDENCE.global === 0.5` and `reason === 'global'` to catch
  silent drift if the tier table is ever refactored.

Finding #6 — preComputedArgTypes worker path
- String overload matched via preComputedArgTypes=['String']
- Int overload matched via preComputedArgTypes=['int'] (lowercase,
  mirroring the parse-worker's inferred-literal shape; stored 'Int' is
  normalized via normalizeJvmTypeName at comparison time)

Finding #7 — Enum null-route documentation
- Enum-only free call asserts `toBeNull()` with an explanatory comment
  linking to the INSTANTIABLE_CLASS_TYPES rationale. NOT marked skipped
  — current behavior is intentional, not broken.

Finding #4 — Swift extension dedup guard
- Two same-name Class entries at different path lengths; exercises the
  full dispatch chain:
    1. filterCallableCandidates with 'free' strips Class → length 0
    2. hasClassTarget triggers resolveStaticCall
    3. Homonym ambiguity null-routes per SM-12 round-1 contract
    4. Constructor-form retry repopulates with both Classes
    5. Dedup block sorts by filePath.length → shortest path wins

Verification
- `tsc --noEmit` clean
- 3064 unit tests pass (+12)
- 1766 integration tests pass
- Zero regressions

Plan: docs/plans/2026-04-09-003-fix-sm13-resolve-free-call-review-findings-plan.md
Review: #756 (comment)
@magyargergo

Copy link
Copy Markdown
Collaborator

Review findings addressed (06ed1c8)

All 7 findings from the review. One code fix + 12 new regression tests.

Finding-by-finding disposition

# Finding Disposition
1 hasClassTarget literal drift Fixed. Replaced ('Class' || 'Struct' || 'Record') with INSTANTIABLE_CLASS_TYPES.has(c.type). Invariant is now enforced by the Set, not by a comment. The inline comment now also documents why the Swift extension dedup block below keeps its narrower literal 'Class' || 'Struct' (Swift extensions only produce Class duplicates in practice, Record excluded by design).
2 Language coverage gaps Fixed. Added 5 new tests covering Go (doStuff()), Python (def helper()), Rust free function outside any impl, Java statically-imported function, JavaScript module-level function. Each exercises _resolveCallTargetForTesting with callForm='free' and the language-specific file extension. resolveFreeCall has zero file-extension branching, so these guard the dispatch chain per language without assuming extractor-specific symbol shapes.
3 No arity filtering test Fixed. Two new tests narrow the same overloaded helper by argCount: 2 and argCount: 0.
4 Swift extension dedup unguarded Fixed. New test locks in the filePath.length heuristic via the full dispatch chain: two same-name Class entries → callForm='free'filterCallableCandidates strips ClasshasClassTarget triggers resolveStaticCall → homonym ambiguity null-routes per SM-12 → constructor-form retry repopulates with both Classes → dedup sorts by path length → shortest wins. The feasibility pass identified this exact chain up-front so the test wouldn't silently pass on a different branch.
5 No Tier 3 global test Fixed. New test: function in lib/global.ts, no importMap entry, resolves via Tier 3 with confidence === 0.5 and reason === 'global' — exact TIER_CONFIDENCE.global values locked in so a silent tier-table refactor surfaces here.
6 No preComputedArgTypes worker-path test Fixed. Two new tests using opts.preComputedArgTypes on _resolveCallTargetForTesting. The Int overload test surfaced a gotcha during execution: matchCandidatesByArgTypes (line 1287) normalizes the STORED parameterTypes via normalizeJvmTypeName (Kotlin Int → int) but compares against the RAW argTypes. Real parse-worker output is already lowercase primitive names inferred from literals, so the test now queries ['int'] to mirror production shape. Inline comment documents this.
7 Enum null-route undocumented Fixed. New test asserts Color()-style free calls on Enum types return null because Enum is excluded from INSTANTIABLE_CLASS_TYPES. NOT marked skipped — current behavior is intentional per PR #754 round 1, and the test comment links to the INSTANTIABLE_CLASS_TYPES JSDoc rationale. If a future extension adds Enum to the set, this test will need to be updated alongside that work — the correct living-documentation signal.

Deepening pass

The plan ran through a feasibility review before implementation, which caught three things that would have been debug-time surprises:

  • Exact TIER_CONFIDENCE.global = 0.5 / reason = 'global' at resolution-context.ts:34 — locked into R5 assertions up front.
  • _resolveCallTargetForTesting helper already threads preComputedArgTypes via opts — no direct-call workaround needed.
  • R7 branch reachability — the naive "two Class entries with callForm='free'" setup would fall through at different points depending on exactly how filterCallableCandidates and resolveStaticCall interact. The feasibility pass traced the 5-step chain ahead of time so the test was written to exercise the intended block on the first try, not by trial and error.

Scope discipline

  • freeFormHasClassTarget at call-processor.ts:1394-1400 (resolveCallTarget's own free-form probe) still uses the literal list with an alignment comment — same drift pattern as R1. Intentionally out of scope for this PR to keep it focused on resolveFreeCall. Flagged as a future consideration in the plan; worth a small follow-up commit.
  • Swift extension dedup duplication between resolveCallTarget (lines 1594-1607) and resolveFreeCall (lines 1994-2008) — flagged as a future-consideration refactor (extract a shared helper). Out of scope for a review-response PR.

Verification

  • tsc --noEmit clean
  • 3064 unit tests pass (+12 new)
  • 1766 integration tests pass
  • Zero regressions across the full suite

Plan

docs/plans/2026-04-09-003-fix-sm13-resolve-free-call-review-findings-plan.md (Standard depth, feasibility-deepened).

2 files changed, 273 insertions(+), 11 deletions(-).

Follow-up to the PR #756 review fix. SM-13 duplicated the Swift
extension same-name collision dedup block between `resolveCallTarget`
and `resolveFreeCall` — two copies of identical 15-line logic with the
same heuristic (`filePath.length` sort, Class/Struct-only, `length > 1`
guard). Extract a single shared helper so the two sites cannot drift.

Changes
- New `dedupSwiftExtensionCandidates(candidates, tier)` helper defined
  alongside `tryOverloadDisambiguation`, with JSDoc documenting:
  - The Swift extension scenario it addresses
  - Why it is intentionally narrower than INSTANTIABLE_CLASS_TYPES
    (Class/Struct only, not Record — C#/Kotlin records don't exhibit
    the multi-file definition pattern, widening risks accidental
    dedup of legitimately distinct record types)
  - The return-null-on-no-match contract so callers can fall through
- `resolveCallTarget` tail dedup (was lines 1593-1610): replaced with
  a single `dedupSwiftExtensionCandidates` call
- `resolveFreeCall` tail dedup (was lines 1994-2012): same replacement
- Net line count: -32 insertions, -9 deletions in the consumer sites,
  +36 for the shared helper + JSDoc

Verification
- `tsc --noEmit` clean
- 3064 unit tests pass (including the R7 Swift dedup guard test added
  in the previous commit that exercises the full free-form retry
  chain through this helper)
- 1766 integration tests pass
- Zero regressions

Follows-up on: #756
@magyargergo

Copy link
Copy Markdown
Collaborator

Deferred items closed out (4726cfc)

The two items I originally flagged as "future considerations" have been handled in this PR rather than split off.

freeFormHasClassTarget upgrade — already moot

Verified against current state: SM-13 removed freeFormHasClassTarget from resolveCallTarget when the entire free-form path was delegated to resolveFreeCall. The hasClassTarget site inside resolveFreeCall (line 1968) was the ONLY surviving literal-list instance, and that was already upgraded to INSTANTIABLE_CLASS_TYPES.has() in the previous commit. No additional change needed — the "deferred" label was based on an outdated mental model of where the literals lived.

Swift extension dedup duplication — extracted to shared helper

Added dedupSwiftExtensionCandidates(candidates, tier) next to tryOverloadDisambiguation. Both resolveCallTarget's tail dedup (was lines 1593-1610) and resolveFreeCall's duplicate (was lines 1994-2012) now call the shared helper.

The JSDoc on the new helper documents:

  • The Swift extension scenario it addresses (same-name Class nodes from extension User { ... } in separate files)
  • Why it is intentionally narrower than INSTANTIABLE_CLASS_TYPES: only Class and Struct are handled, not Record. C#/Kotlin records don't exhibit the multi-file definition pattern, and widening would risk accidental dedup of legitimately distinct record types. Future maintainers will see this rationale inline and not "fix" the asymmetry.
  • The return-null-on-no-match contract so callers can fall through cleanly.

Net effect: -32 lines at the two consumer sites, +36 for the helper + JSDoc. The diff is roughly flat on line count but the single source of truth means the two sites cannot silently drift if the heuristic is ever tuned — which was the original concern in finding #4.

Verification

  • tsc --noEmit clean
  • 3064 unit tests pass (including the R7 Swift dedup guard from the previous commit that now exercises the full free-form retry chain through the shared helper)
  • 1766 integration tests pass
  • Zero regressions

1 file changed, 47 insertions(+), 32 deletions(-).

@magyargergo magyargergo marked this pull request as ready for review April 9, 2026 14:14
@github-actions

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
5776 5679 0 97 215s

✅ All 5679 tests passed

97 test(s) skipped — expand for details
  • Swift MethodExtractor > isTypeDeclaration > recognizes class_declaration
  • Swift MethodExtractor > isTypeDeclaration > recognizes protocol_declaration
  • Swift MethodExtractor > isTypeDeclaration > rejects import_declaration
  • Swift MethodExtractor > visibility > extracts public method
  • Swift MethodExtractor > visibility > extracts private method
  • Swift MethodExtractor > visibility > defaults to internal when no modifier
  • Swift MethodExtractor > protocol methods > marks protocol method as abstract
  • Swift MethodExtractor > static and class methods > detects static func as isStatic
  • Swift MethodExtractor > static and class methods > detects class func as isStatic
  • Swift MethodExtractor > parameters > extracts parameters with types and default values
  • Swift MethodExtractor > return type > extracts return type from -> annotation
  • Swift MethodExtractor > annotations > extracts @objc attribute
  • Swift MethodExtractor > isFinal > detects final func
  • Swift MethodExtractor > isFinal > is false when not final
  • Swift MethodExtractor > isAsync > detects async func
  • Swift MethodExtractor > isOverride > detects override method
  • buildTypeEnv > constructor inference (Tier 1 fallback) > lookupClassByName regression coverage > Swift lookupClassByName regression coverage > Swift cross-file constructor inference uses lookupClassByName
  • buildTypeEnv > constructor inference (Tier 1 fallback) > lookupClassByName regression coverage > Swift lookupClassByName regression coverage > Swift explicit init inference uses lookupClassByName
  • buildTypeEnv > constructor inference (Tier 1 fallback) > lookupClassByName regression coverage > Swift lookupClassByName regression coverage > Swift cross-file constructor inference does not bind plain functions
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature
  • Swift constructor-inferred type resolution > detects User and Repo classes, both with save methods
  • Swift constructor-inferred type resolution > resolves user.save() to Models/User.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > resolves repo.save() to Models/Repo.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > emits exactly 2 save() CALLS edges (one per receiver type)
  • Swift self resolution > detects User and Repo classes, each with a save function
  • Swift self resolution > resolves self.save() inside User.process to User.save, not Repo.save
  • Swift parent resolution > detects BaseModel and User classes plus Serializable protocol
  • Swift parent resolution > emits EXTENDS edge: User → BaseModel
  • Swift parent resolution > emits IMPLEMENTS edge: User → Serializable (protocol conformance)
  • Swift cross-file User.init() inference > resolves user.save() via User.init(name:) inference
  • Swift cross-file User.init() inference > resolves user.greet() via User.init(name:) inference
  • Swift return type inference > detects User class and getUser function
  • Swift return type inference > detects save function on User (Swift class methods are Function nodes)
  • Swift return type inference > resolves user.save() to User#save via return type of getUser() -> User
  • Swift return-type inference via function return type > resolves user.save() to User#save via return type of getUser()
  • Swift return-type inference via function return type > user.save() does NOT resolve to Repo#save
  • Swift return-type inference via function return type > resolves repo.save() to Repo#save via return type of getRepo()
  • Swift implicit imports (cross-file visibility) > detects UserService class in Models.swift
  • Swift implicit imports (cross-file visibility) > resolves UserService() constructor call across files (no explicit import)
  • Swift implicit imports (cross-file visibility) > resolves service.fetchUser() member call across files
  • Swift implicit imports (cross-file visibility) > creates IMPORTS edges between files in the same module
  • Swift extension deduplication > detects Product class
  • Swift extension deduplication > resolves Product() constructor despite extension creating duplicate class node
  • Swift extension deduplication > resolves product.save() to Product.swift (primary definition)
  • Swift constructor call fallback (no new keyword) > resolves OCRService() as constructor call across files
  • Swift constructor call fallback (no new keyword) > resolves ocr.recognize() member call via constructor-inferred type
  • Swift export visibility (internal vs private) > resolves PublicService() constructor across files
  • Swift export visibility (internal vs private) > resolves internalHelper() across files (internal = module-scoped)
  • Swift if let / guard let binding resolution > detects User and Repo classes
  • Swift if let / guard let binding resolution > resolves user.save() inside if-let to User#save
  • Swift if let / guard let binding resolution > resolves repo.save() inside guard-let to Repo#save
  • Swift if let / guard let binding resolution > user.save() in if-let does NOT resolve to Repo#save
  • Swift await / try expression unwrapping > resolves user.save() via await fetchUser() return type
  • Swift await / try expression unwrapping > resolves repo.save() via try parseRepo() return type
  • Swift await / try expression unwrapping > detects fetchUser and parseRepo as functions
  • Swift for-in loop element type inference > detects User and Repo classes
  • Swift for-in loop element type inference > creates implicit import edges between files
  • Swift field-type resolution > detects classes and their properties
  • Swift field-type resolution > emits HAS_PROPERTY edges from class to field
  • Swift field-type resolution > resolves field-chain call user.address.save() → Address#save
  • Swift field-type resolution > emits ACCESSES edges for field reads in chains
  • Swift field-type resolution > populates field metadata (visibility, declaredType) on Property nodes
  • Swift call-result binding > resolves call-result-bound method call user.save() → User#save
  • Swift call-result binding > getUser() is present as a defined function
  • Swift call-result binding > emits processUser -> getUser CALLS edge for let-assigned free function call
  • Swift method enrichment > detects Animal protocol and Dog class
  • Swift method enrichment > emits IMPLEMENTS edge Dog -> Animal
  • Swift method enrichment > emits HAS_METHOD edges for Dog methods
  • Swift method enrichment > marks protocol Animal.speak as isAbstract
  • Swift method enrichment > marks Dog.speak as NOT isAbstract
  • Swift method enrichment > marks breathe as isFinal
  • Swift method enrichment > marks classify as isStatic
  • Swift method enrichment > captures @objc annotation on breathe
  • Swift method enrichment > populates parameterTypes for classify(_ name: String)
  • Swift method enrichment > records parameterCount for classify
  • Swift method enrichment > records returnType for speak
  • Swift method enrichment > resolves dog.speak() CALLS edge
  • Swift method enrichment > resolves Dog.classify("dog") CALLS edge
  • Swift abstract dispatch > detects Repository protocol and SqlRepository class
  • Swift abstract dispatch > emits IMPLEMENTS edge SqlRepository -> Repository
  • Swift abstract dispatch > emits HAS_METHOD edges for Repository.find and Repository.save
  • Swift abstract dispatch > emits HAS_METHOD edges for SqlRepository.find and SqlRepository.save
  • Swift abstract dispatch > marks base Repository.find as isAbstract
  • Swift abstract dispatch > marks base Repository.save as isAbstract
  • Swift abstract dispatch > marks concrete SqlRepository.find as NOT isAbstract
  • Swift abstract dispatch > resolves repo.find(id: 42) CALLS edge
  • Swift abstract dispatch > resolves repo.save(entity: user) CALLS edge
  • Swift abstract dispatch > populates parameterTypes for Repository.find
  • Swift abstract dispatch > populates parameterTypes for Repository.save
  • Swift abstract dispatch > records returnType for SqlRepository.find
  • Swift abstract dispatch > emits METHOD_IMPLEMENTS edges from SqlRepository methods → Repository protocol methods
  • Swift overloaded method disambiguation > detects 2 distinct find Method nodes on SqlRepository
  • Swift overloaded method disambiguation > emits METHOD_IMPLEMENTS edges for both find overloads
  • Swift overloaded method disambiguation > emits METHOD_IMPLEMENTS edge for save
  • Swift overloaded method disambiguation > emits exactly 3 METHOD_IMPLEMENTS edges total
  • Swift Child extends Parent — inherited method resolution (SM-9) > detects Parent and Child classes
  • Swift Child extends Parent — inherited method resolution (SM-9) > resolves c.parentMethod() to Parent.parentMethod via first-wins MRO walk

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 71.99% 15345/21314 71.96% 📈 +0.0 🟢 ██████████████░░░░░░
Branches 61.16% 10024/16388 61.14% 📈 +0.0 🟢 ████████████░░░░░░░░
Functions 76.5% 1410/1843 76.48% 📈 +0.0 🟢 ███████████████░░░░░
Lines 74.15% 13889/18729 74.13% 📈 +0.0 🟢 ██████████████░░░░░░

📋 View full run · Generated by CI

@magyargergo

Copy link
Copy Markdown
Collaborator

@claude review these changes along with the plan on the issue. You need to look for gaps and performance issues. You are also tasked with reviewing the architectureal fit. Also we need to ensure we have coverage for these per languages. Also include other comments in your evaluation.

@github-actions

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 2m 52s —— View job


PR Review: Extract resolveFreeCall from resolveCallTarget (SM-13)

Tasks:

  • Gather context and read PR description
  • Read changed files (call-processor.ts, symbol-table.test.ts)
  • Review architectural fit vs SM-11/SM-12 pattern
  • Review resolveFreeCall implementation for gaps
  • Review performance characteristics
  • Review per-language test coverage
  • Check test quality and completeness
  • Post final review

Summary

This is a mature, well-executed extraction that has gone through a full review cycle (7 findings identified + addressed, 2 deferred items resolved). The final state is clean. I've reviewed the code in its current form and found 3 minor issues plus a few observations. No blockers.


Findings

1. Stale line-number comment in resolveFreeCall — misleading reference

call-processor.ts:1996

// The Swift extension dedup block below (lines ~1994-2008) deliberately
// uses a narrower literal `'Class' | 'Struct'` check

The original lines 1994–2008 were the inlined dedup block in the pre-SM-13 version of resolveFreeCall. That block was extracted to dedupSwiftExtensionCandidates in the final commit (4726cfc). The "lines ~1994-2008" reference now points at unrelated code — the dedup call actually lives at lines 2022–2026. A reader following the comment will be confused about what "the Swift extension dedup block" refers to.

Suggested fix: Replace "lines ~1994-2008" with a reference to the helper name.

// The `dedupSwiftExtensionCandidates` call below deliberately
// uses a narrower literal `'Class' | 'Struct'` check — Swift extensions

Fix this →


2. Fuzzy-widening asymmetry not documented in resolveFreeCall JSDoc

resolveFreeCall has no widenCache parameter and no D2 fuzzy-widening path. This is intentional — the design note at line 1929 says Phase 5 will address it. But the current JSDoc doesn't make this explicit: a caller comparing the signature to resolveCallTarget might not realize free calls silently skip the fuzzy path. This is an architectural asymmetry worth one sentence in the JSDoc.

 * **Design note (SM-13):** This path still falls through to Tier 3 (global)
 * via `ctx.resolve()`. Fuzzy global resolution remains until Phase 5 replaces
 * `lookupFuzzy` with a scoped data source.

Consider adding: No fuzzy-widening pass (no widenCache) — free calls rely on exact tier resolution only. Phase 5 will address this.

This doesn't affect correctness but would help future maintainers understand why free calls and constructor calls have different resolution depths.


3. Constructor-form retry fires unconditionally after resolveStaticCall returns null — no guard for "no-constructor" reason

call-processor.ts:2003–2010

if (hasClassTarget) {
  const staticResult = resolveStaticCall(calledName, filePath, ctx, argCount, tiered);
  if (staticResult) return staticResult;
  // Retry with constructor form...
  filteredCandidates = filterCallableCandidates(tiered.candidates, argCount, 'constructor');
}

resolveStaticCall returns null for at least 3 distinct reasons (no class found, ambiguous homonym classes, no matching constructor). The retry unconditionally re-filters as 'constructor' regardless of which reason fired. In the homonym case (two Class entries, same tier), this sets filteredCandidates to both Classes, which then falls through to dedupSwiftExtensionCandidates — the R7 scenario, intentional.

However, in the case where resolveStaticCall bailed out because tiered.candidates.some(c => c.type === 'Constructor') (step 4, line 2150), those Constructor nodes will also survive filterCallableCandidates(..., 'constructor') and be passed to overload disambiguation. This is likely correct behavior, but no test exercises this path (a free call User() where tiered candidates contain both a Class node AND a Constructor node indexed without an ownerId). Consider adding a comment explaining the retry covers all three null-return reasons from resolveStaticCall.


Architectural Fit

Consistent with SM-11/SM-12. The dispatch ordering in resolveCallTarget is now freeconstructor → everything else, which mirrors the natural priority of the fast paths. tieredOverride ensures ctx.resolve() is called exactly once per resolveCallTarget invocation regardless of which sub-resolver handles the call — no redundant lookup. dedupSwiftExtensionCandidates is the right outcome of the R4 dedup finding: single source of truth for the heuristic, well-scoped to Class|Struct only, with JSDoc explaining the intentional asymmetry vs INSTANTIABLE_CLASS_TYPES.


Performance

No regressions. The hasClassTarget .some() short-circuits on first match, negligible for typical 1–5 candidate pools. The pre-computed tieredOverride short-circuit test (R1 test with resolve spy) correctly verifies the double-lookup is prevented. No allocations introduced beyond what SM-11/SM-12 already established.


Per-Language Coverage

Language Basic free call Constructor (User()) Null-route case
TypeScript ✅ import-scoped + same-file
Swift ✅ (via R2 test shape) ✅ R7 dedup
Kotlin ✅ (via R2 test shape) via Swift proxy
Go ✅ R2
Python ✅ R2
Rust ✅ R2
Java ✅ R2 (static import proxy)
JavaScript ✅ R2
C# ✅ Record target
PHP ✅ Trait null-route
PHP free fn
Ruby
C/C++

The R2 tests for Go/Python/Rust/Java/JS exercise the dispatch chain with language-appropriate file extensions, which is the right strategy since resolveFreeCall has no file-extension branching. PHP, Ruby, and C/C++ plain free functions are uncovered, but those are consistent with the broader test gap profile in this file (not a regression from this PR).


Minor / Nits

  • The Java R2 test comment accurately notes it guards dispatch routing, not static-import extractor behavior — good self-documentation.
  • The preComputedArgTypes test comment explaining Int → int JVM normalization is valuable and should be preserved.
  • dedupSwiftExtensionCandidates being private (unexported) is correct — it's an implementation detail of the call-processor module.
  • The test file name (symbol-table.test.ts) continues to house resolveFreeCall tests, matching the SM-11/SM-12 precedent. Acceptable for now; a future call-processor.test.ts split would improve discoverability.

Verdict

Approve with minor cleanup. Finding #1 (stale line numbers) is a one-line fix. Findings #2 and #3 are documentation gaps, not bugs. All previous review findings were addressed thoroughly, CI is clean, and coverage delta is positive.

Three documentation-only findings from the approval review. No
behavior change, no new tests, no code path modifications.

Finding #1 — stale line-number comment
- The comment inside `resolveFreeCall` at the `hasClassTarget` site
  referenced "lines ~1994-2008" for the Swift extension dedup block.
  Those lines were the inlined pre-SM-13 version; the block has since
  been extracted to `dedupSwiftExtensionCandidates`. Replaced the line
  reference with the helper name so future readers don't chase dead
  line numbers.

Finding #2 — fuzzy-widening asymmetry undocumented
- `resolveFreeCall` intentionally has no `widenCache` parameter and no
  D2 fuzzy-widening pass (unlike `resolveCallTarget`'s member-call
  path). Added an explicit "Asymmetry vs `resolveCallTarget`" paragraph
  to the JSDoc so a caller comparing the two signatures knows the
  skipped pass is deliberate and tied to Phase 5.

Finding #3 — constructor-form retry reasons undocumented
- `resolveStaticCall` can return null for three distinct reasons
  (empty instantiable pool, homonym ambiguity, ownerless Constructor
  nodes). The retry below it unconditionally re-filters with
  `'constructor'` form, which is correct for all three but not
  obvious. Added a structured three-case comment enumerating each
  reason and linking (a) to the SM-12 null-route contract, (b) to
  the R7 dedup test, and (c) to the currently-uncovered ownerless-
  Constructor path (noted as a future test candidate).

Verification
- `tsc --noEmit` clean
- 175 `resolveFreeCall` + `resolveStaticCall` + sibling tests pass
  (sanity check — no behavior change expected)
- No regressions

Follows-up on: #756 (comment)
@magyargergo

Copy link
Copy Markdown
Collaborator

Final review findings addressed (112fa7c)

All 3 documentation-only findings from the approval review. No behavior change, no new tests.

# Finding Fix
1 Stale "lines ~1994-2008" comment inside resolveFreeCall — those lines were the inlined pre-SM-13 dedup block, which was extracted to dedupSwiftExtensionCandidates in the previous commit Replaced the line reference with the helper name so future readers don't chase dead coordinates
2 resolveFreeCall JSDoc didn't flag the architectural asymmetry — no widenCache parameter, no D2 fuzzy-widening pass (unlike resolveCallTarget's member path) Added an "Asymmetry vs resolveCallTarget" paragraph explaining the skipped pass is deliberate and tied to Phase 5
3 Constructor-form retry after resolveStaticCall returns null wasn't documented — resolveStaticCall can null-return for three distinct reasons and the retry unconditionally re-filters as 'constructor' Added a structured three-case comment enumerating each reason: (a) null-route contract from SM-12, (b) homonym ambiguity covered by R7 dedup test, (c) ownerless-Constructor path currently uncovered by a dedicated test — flagged as a future candidate

Verification

  • tsc --noEmit clean
  • 175 resolveFreeCall / resolveStaticCall / sibling tests pass (sanity check — no behavior change expected)
  • No regressions

1 file changed, 42 insertions(+), 6 deletions(-).

Two low-severity test gaps from PR #756 review comment 4215739052 —
previously addressed doc-only, now have concrete test coverage.

Finding #3 low — ownerless-Constructor retry path (previously comment-only)
- The retry after resolveStaticCall returns null handles three distinct
  null-return reasons. Cases (a) and (b) were already tested (Interface/
  Trait null-route from SM-12, Swift shadowing dedup from R7). Case (c) —
  resolveStaticCall step-4 bailout when the tiered pool contains
  ownerless Constructor nodes — was only covered by a comment.
- New test: Class + ownerless Constructor in tiered pool, callForm='free'.
  Exercises the full chain:
    1. resolveStaticCall step 3 walks classCandidates via
       lookupMethodByOwner — ownerless Constructor not in methodByOwner,
       nothing found.
    2. Step 4 detects Constructor in tiered pool, bails with null.
    3. resolveFreeCall retry re-runs filterCallableCandidates with
       'constructor' form, which prefers Constructor over Class per
       CONSTRUCTOR_TARGET_TYPES ordering.
    4. Single survivor returned.
- Asserts the Constructor node (not the Class) is the resolved target.

Low — PHP free function coverage gap
- The language coverage table in the same review flagged PHP free
  functions (top-level `function helper()` outside any class) as
  uncovered. Added a test mirroring the existing Go/Python/Rust/Java/
  JS language tests — exercises the `.php` dispatch path for free
  calls. Ruby and C/C++ remain uncovered; deferred to a future round
  since those languages also have other gaps in the broader test file.

Verification
- `tsc --noEmit` clean
- 3066 unit tests pass (+2 new regression tests)
- 1766 integration tests pass
- Zero regressions

Follows-up on: #756 (comment)
@magyargergo

Copy link
Copy Markdown
Collaborator

Two low-severity test gaps addressed (d56b85d)

Previous commit addressed finding #3 with a three-reason comment but left case (c) uncovered by a concrete test. This commit adds the two remaining low-priority test coverage items from comment 4215739052.

Finding #3 low — ownerless-Constructor retry path

The retry comment enumerated three null-return reasons from resolveStaticCall. Cases (a) and (b) were already tested:

  • (a) null-route contract — covered by SM-12 Interface/Trait tests
  • (b) homonym ambiguity — covered by the R7 Swift-extension dedup test

Case (c)resolveStaticCall step-4 bailout when the tiered pool contains ownerless Constructor nodes (pathological extractor output some languages still emit) — was only covered by a comment. Now exercised by a concrete test:

Class + ownerless Constructor in tiered pool, callForm='free'
  ↓
  resolveStaticCall step 3: lookupMethodByOwner finds nothing (no ownerId)
  ↓
  step 4: detects Constructor in tiered pool → bails with null
  ↓
  resolveFreeCall retry: filterCallableCandidates('constructor') prefers
    Constructor over Class per CONSTRUCTOR_TARGET_TYPES ordering
  ↓
  returns the Constructor node

Asserts the Constructor is the resolved target (not the Class), which is the exact branch the retry comment describes.

Low — PHP free function coverage gap

The language coverage table in the review flagged PHP free fn, Ruby, and C/C++ as uncovered. This commit adds the PHP test (top-level function helper() outside any class), mirroring the existing Go/Python/Rust/Java/JavaScript language tests. Ruby and C/C++ remain uncovered — deferred to a future round since those languages also have broader gaps in the test file that are out of scope for a single finding-response commit.

Verification

  • tsc --noEmit clean
  • 3066 unit tests pass (+2 new regression tests — 3064 → 3066)
  • 1766 integration tests pass
  • Zero regressions

1 file changed, 61 insertions(+).

@magyargergo magyargergo merged commit d090789 into main Apr 9, 2026
13 checks passed
@magyargergo magyargergo deleted the copilot/sm-13-extract-resolvefreecall branch April 9, 2026 16:41
github714801013 pushed a commit to github714801013/GitNexus that referenced this pull request Apr 28, 2026
…atwari#756)

* Initial plan

* feat(SM-13): extract resolveFreeCall from resolveCallTarget

Extract the free-function call resolution path into a dedicated
`resolveFreeCall(calledName, filePath, ctx)` function that uses
`lookupExact` + import-scoped resolution via `ctx.resolve()`.

- Free function calls (foo()) now route through `resolveFreeCall`
- Swift/Kotlin implicit constructors (User()) delegate to
  `resolveStaticCall` within `resolveFreeCall`
- `resolveCallTarget` dispatches `callForm === 'free'` early,
  removing the inline freeFormHasClassTarget logic
- S0 block simplified to only handle `callForm === 'constructor'`
- Global (Tier 3) fallthrough preserved via ctx.resolve() until Phase 5
- 9 new unit tests for resolveFreeCall
- All 163 unit tests pass, all 1199 integration resolver tests pass

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c5f2e73a-259a-438c-b5c8-286b82e3c215

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/c5f2e73a-259a-438c-b5c8-286b82e3c215

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* fix(SM-13): address PR abhigyanpatwari#756 review findings on resolveFreeCall

Addresses all 7 findings from the PR abhigyanpatwari#756 review comment.

Code (R1, finding abhigyanpatwari#1)
- Replace the literal `'Class' | 'Struct' | 'Record'` check in
  `hasClassTarget` with `INSTANTIABLE_CLASS_TYPES.has(c.type)`. Converts
  an invariant that was previously comment-enforced ("keep this list
  aligned with INSTANTIABLE_CLASS_TYPES") into one enforced structurally.
  Any future extension of the set propagates here automatically. The
  narrower Swift extension dedup block below still uses literal
  `'Class' | 'Struct'` by design — Swift extensions only produce Class
  duplicates in practice, Record is deliberately excluded there, and
  the inline comment now documents that asymmetry.

Tests (+12 regression scenarios)

Finding abhigyanpatwari#2 — language coverage
- Go free function (doStuff())
- Python free function (def helper(): ... helper())
- Rust free function outside any impl block
- Java statically-imported function
- JavaScript module-level function
Each exercises `_resolveCallTargetForTesting` with `callForm='free'`
and the language-specific file extension. `resolveFreeCall` has no
file-extension branching, so these guard the dispatch chain per
language without assuming extractor-specific symbol shapes.

Finding abhigyanpatwari#3 — argCount threading
- 2-arg overload selected when argCount=2
- 0-arg overload selected when argCount=0

Finding abhigyanpatwari#5 — Tier 3 (global) resolution
- Function globally visible but not imported. Asserts exact
  `TIER_CONFIDENCE.global === 0.5` and `reason === 'global'` to catch
  silent drift if the tier table is ever refactored.

Finding abhigyanpatwari#6 — preComputedArgTypes worker path
- String overload matched via preComputedArgTypes=['String']
- Int overload matched via preComputedArgTypes=['int'] (lowercase,
  mirroring the parse-worker's inferred-literal shape; stored 'Int' is
  normalized via normalizeJvmTypeName at comparison time)

Finding abhigyanpatwari#7 — Enum null-route documentation
- Enum-only free call asserts `toBeNull()` with an explanatory comment
  linking to the INSTANTIABLE_CLASS_TYPES rationale. NOT marked skipped
  — current behavior is intentional, not broken.

Finding abhigyanpatwari#4 — Swift extension dedup guard
- Two same-name Class entries at different path lengths; exercises the
  full dispatch chain:
    1. filterCallableCandidates with 'free' strips Class → length 0
    2. hasClassTarget triggers resolveStaticCall
    3. Homonym ambiguity null-routes per SM-12 round-1 contract
    4. Constructor-form retry repopulates with both Classes
    5. Dedup block sorts by filePath.length → shortest path wins

Verification
- `tsc --noEmit` clean
- 3064 unit tests pass (+12)
- 1766 integration tests pass
- Zero regressions

Plan: docs/plans/2026-04-09-003-fix-sm13-resolve-free-call-review-findings-plan.md
Review: abhigyanpatwari#756 (comment)

* refactor(SM-13): extract dedupSwiftExtensionCandidates shared helper

Follow-up to the PR abhigyanpatwari#756 review fix. SM-13 duplicated the Swift
extension same-name collision dedup block between `resolveCallTarget`
and `resolveFreeCall` — two copies of identical 15-line logic with the
same heuristic (`filePath.length` sort, Class/Struct-only, `length > 1`
guard). Extract a single shared helper so the two sites cannot drift.

Changes
- New `dedupSwiftExtensionCandidates(candidates, tier)` helper defined
  alongside `tryOverloadDisambiguation`, with JSDoc documenting:
  - The Swift extension scenario it addresses
  - Why it is intentionally narrower than INSTANTIABLE_CLASS_TYPES
    (Class/Struct only, not Record — C#/Kotlin records don't exhibit
    the multi-file definition pattern, widening risks accidental
    dedup of legitimately distinct record types)
  - The return-null-on-no-match contract so callers can fall through
- `resolveCallTarget` tail dedup (was lines 1593-1610): replaced with
  a single `dedupSwiftExtensionCandidates` call
- `resolveFreeCall` tail dedup (was lines 1994-2012): same replacement
- Net line count: -32 insertions, -9 deletions in the consumer sites,
  +36 for the shared helper + JSDoc

Verification
- `tsc --noEmit` clean
- 3064 unit tests pass (including the R7 Swift dedup guard test added
  in the previous commit that exercises the full free-form retry
  chain through this helper)
- 1766 integration tests pass
- Zero regressions

Follows-up on: abhigyanpatwari#756

* docs(SM-13): address PR abhigyanpatwari#756 final review — comment cleanup only

Three documentation-only findings from the approval review. No
behavior change, no new tests, no code path modifications.

Finding abhigyanpatwari#1 — stale line-number comment
- The comment inside `resolveFreeCall` at the `hasClassTarget` site
  referenced "lines ~1994-2008" for the Swift extension dedup block.
  Those lines were the inlined pre-SM-13 version; the block has since
  been extracted to `dedupSwiftExtensionCandidates`. Replaced the line
  reference with the helper name so future readers don't chase dead
  line numbers.

Finding abhigyanpatwari#2 — fuzzy-widening asymmetry undocumented
- `resolveFreeCall` intentionally has no `widenCache` parameter and no
  D2 fuzzy-widening pass (unlike `resolveCallTarget`'s member-call
  path). Added an explicit "Asymmetry vs `resolveCallTarget`" paragraph
  to the JSDoc so a caller comparing the two signatures knows the
  skipped pass is deliberate and tied to Phase 5.

Finding abhigyanpatwari#3 — constructor-form retry reasons undocumented
- `resolveStaticCall` can return null for three distinct reasons
  (empty instantiable pool, homonym ambiguity, ownerless Constructor
  nodes). The retry below it unconditionally re-filters with
  `'constructor'` form, which is correct for all three but not
  obvious. Added a structured three-case comment enumerating each
  reason and linking (a) to the SM-12 null-route contract, (b) to
  the R7 dedup test, and (c) to the currently-uncovered ownerless-
  Constructor path (noted as a future test candidate).

Verification
- `tsc --noEmit` clean
- 175 `resolveFreeCall` + `resolveStaticCall` + sibling tests pass
  (sanity check — no behavior change expected)
- No regressions

Follows-up on: abhigyanpatwari#756 (comment)

* test(SM-13): cover ownerless-Constructor retry + PHP free function

Two low-severity test gaps from PR abhigyanpatwari#756 review comment 4215739052 —
previously addressed doc-only, now have concrete test coverage.

Finding abhigyanpatwari#3 low — ownerless-Constructor retry path (previously comment-only)
- The retry after resolveStaticCall returns null handles three distinct
  null-return reasons. Cases (a) and (b) were already tested (Interface/
  Trait null-route from SM-12, Swift shadowing dedup from R7). Case (c) —
  resolveStaticCall step-4 bailout when the tiered pool contains
  ownerless Constructor nodes — was only covered by a comment.
- New test: Class + ownerless Constructor in tiered pool, callForm='free'.
  Exercises the full chain:
    1. resolveStaticCall step 3 walks classCandidates via
       lookupMethodByOwner — ownerless Constructor not in methodByOwner,
       nothing found.
    2. Step 4 detects Constructor in tiered pool, bails with null.
    3. resolveFreeCall retry re-runs filterCallableCandidates with
       'constructor' form, which prefers Constructor over Class per
       CONSTRUCTOR_TARGET_TYPES ordering.
    4. Single survivor returned.
- Asserts the Constructor node (not the Class) is the resolved target.

Low — PHP free function coverage gap
- The language coverage table in the same review flagged PHP free
  functions (top-level `function helper()` outside any class) as
  uncovered. Added a test mirroring the existing Go/Python/Rust/Java/
  JS language tests — exercises the `.php` dispatch path for free
  calls. Ruby and C/C++ remain uncovered; deferred to a future round
  since those languages also have other gaps in the broader test file.

Verification
- `tsc --noEmit` clean
- 3066 unit tests pass (+2 new regression tests)
- 1766 integration tests pass
- Zero regressions

Follows-up on: abhigyanpatwari#756 (comment)

---------

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SM-13] Extract resolveFreeCall from resolveCallTarget

2 participants