mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-29 06:32:28 +00:00
Extract resolveFreeCall from resolveCallTarget (SM-13) (#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 #756 review findings on resolveFreeCall 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: https://github.com/abhigyanpatwari/GitNexus/pull/756#issuecomment-4213879002 * refactor(SM-13): extract dedupSwiftExtensionCandidates shared helper 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: https://github.com/abhigyanpatwari/GitNexus/pull/756 * docs(SM-13): address PR #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 #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: https://github.com/abhigyanpatwari/GitNexus/pull/756#issuecomment-4215739052 * test(SM-13): cover ownerless-Constructor retry + PHP free function 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: https://github.com/abhigyanpatwari/GitNexus/pull/756#issuecomment-4215739052 --------- 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>
This commit is contained in:
parent
81bba52e98
commit
fd428963c3
2 changed files with 632 additions and 54 deletions
|
|
@ -1317,6 +1317,44 @@ const tryOverloadDisambiguation = (
|
|||
return matchCandidatesByArgTypes(candidates, argTypes);
|
||||
};
|
||||
|
||||
/**
|
||||
* Collapse Swift-extension duplicate Class/Struct candidates to the primary
|
||||
* definition, preferring the shortest file path.
|
||||
*
|
||||
* Swift extensions (`extension User { ... }` in a separate file) create
|
||||
* multiple `Class` nodes sharing the same symbol name — one for the primary
|
||||
* declaration and one per extension file. When overload disambiguation and
|
||||
* receiver narrowing both fail to converge on a single candidate, this
|
||||
* heuristic picks the primary definition based on the assumption that it
|
||||
* lives at the shortest file path (e.g. `User.swift` over `UserExtensions.swift`).
|
||||
*
|
||||
* Intentionally narrower than {@link INSTANTIABLE_CLASS_TYPES}: only `Class`
|
||||
* and `Struct` are considered, not `Record`. Swift extensions only produce
|
||||
* `Class` duplicates in practice, and C#/Kotlin records do not exhibit the
|
||||
* same multi-file-definition pattern, so widening this set risks accidental
|
||||
* dedup of legitimately distinct record types.
|
||||
*
|
||||
* Returns a `ResolveResult` when the heuristic fires, `null` when the
|
||||
* candidate pool does not match the shape (mixed types, non-Class/Struct
|
||||
* kinds, or `length <= 1`). Callers should fall through to their own null
|
||||
* return when this helper returns `null`.
|
||||
*
|
||||
* Shared between `resolveCallTarget` and `resolveFreeCall` — SM-13 originally
|
||||
* duplicated this block into both functions. Having a single source of truth
|
||||
* prevents the two copies from drifting if the heuristic is ever tuned.
|
||||
*/
|
||||
const dedupSwiftExtensionCandidates = (
|
||||
candidates: readonly SymbolDefinition[],
|
||||
tier: ResolutionTier,
|
||||
): ResolveResult | null => {
|
||||
if (candidates.length <= 1) return null;
|
||||
const allSameType = candidates.every((c) => c.type === candidates[0].type);
|
||||
if (!allSameType) return null;
|
||||
if (candidates[0].type !== 'Class' && candidates[0].type !== 'Struct') return null;
|
||||
const sorted = [...candidates].sort((a, b) => a.filePath.length - b.filePath.length);
|
||||
return toResolveResult(sorted[0], tier);
|
||||
};
|
||||
|
||||
/**
|
||||
* Resolve a function call to its target node ID using priority strategy:
|
||||
* A. Narrow candidates by scope tier via ctx.resolve()
|
||||
|
|
@ -1370,6 +1408,20 @@ const resolveCallTarget = (
|
|||
const tiered = ctx.resolve(call.calledName, currentFile);
|
||||
if (!tiered) return null;
|
||||
|
||||
// SM-13: Free function calls route through resolveFreeCall.
|
||||
// Handles pure free calls (foo()) and Swift/Kotlin implicit constructors (User()).
|
||||
if (call.callForm === 'free') {
|
||||
return resolveFreeCall(
|
||||
call.calledName,
|
||||
currentFile,
|
||||
ctx,
|
||||
call.argCount,
|
||||
tiered,
|
||||
overloadHints,
|
||||
preComputedArgTypes,
|
||||
);
|
||||
}
|
||||
|
||||
let filteredCandidates = filterCallableCandidates(
|
||||
tiered.candidates,
|
||||
call.argCount,
|
||||
|
|
@ -1377,13 +1429,10 @@ const resolveCallTarget = (
|
|||
);
|
||||
|
||||
// S0. Constructor/static fast path (SM-12): O(1) class + constructor lookup
|
||||
// via lookupClassByName + lookupMethodByOwner before falling back to the
|
||||
// existing filtering + fuzzy-widening path. Falls back to the class node
|
||||
// itself when no Constructor symbol is indexed for the type.
|
||||
//
|
||||
// Handles:
|
||||
// (a) callForm === 'constructor' — explicit `new User()` in Java/TS/C#/etc.
|
||||
// (b) callForm === 'free' with class target — implicit `User()` in Swift/Kotlin
|
||||
// via lookupClassByName + lookupMethodByOwner.
|
||||
// Handles callForm === 'constructor' — explicit `new User()` in Java/TS/C#/etc.
|
||||
// Free-form class targets (Swift/Kotlin `User()`) are handled by
|
||||
// resolveFreeCall above (SM-13).
|
||||
//
|
||||
// Known gaps (handled by the existing tail fallback at the bottom of
|
||||
// this function, not S0):
|
||||
|
|
@ -1392,21 +1441,7 @@ const resolveCallTarget = (
|
|||
// S0 to cover them would require threading receiver-type resolution
|
||||
// through the module-alias logic; revisit if it shows up as a hot
|
||||
// spot.
|
||||
//
|
||||
// The `.some()` trigger below must stay aligned with
|
||||
// `INSTANTIABLE_CLASS_TYPES` — any type admitted here that is not in
|
||||
// that set will cause S0 → `resolveStaticCall` to run and return null,
|
||||
// wasting two lookup passes per call. `Enum` is deliberately excluded
|
||||
// (same rationale as `INSTANTIABLE_CLASS_TYPES`); `Record` is included
|
||||
// so C# records and Kotlin data classes reach the fast path.
|
||||
const freeFormHasClassTarget =
|
||||
call.callForm === 'free' &&
|
||||
filteredCandidates.length === 0 &&
|
||||
tiered.candidates.some((c) => c.type === 'Class' || c.type === 'Struct' || c.type === 'Record');
|
||||
if (call.callForm === 'constructor' || freeFormHasClassTarget) {
|
||||
// Reuse the pre-computed `tiered` result — resolveStaticCall's class name
|
||||
// is identical to `call.calledName` here, so re-running ctx.resolve would
|
||||
// duplicate the tiered-lookup work performed at the top of this function.
|
||||
if (call.callForm === 'constructor') {
|
||||
const staticResult = resolveStaticCall(
|
||||
call.calledName,
|
||||
currentFile,
|
||||
|
|
@ -1417,22 +1452,6 @@ const resolveCallTarget = (
|
|||
if (staticResult) return staticResult;
|
||||
}
|
||||
|
||||
// Swift/Kotlin: constructor calls look like free function calls (no `new` keyword).
|
||||
// If free-form filtering found no callable candidates but the symbol resolves to a
|
||||
// Class/Struct, retry with constructor form so CONSTRUCTOR_TARGET_TYPES applies.
|
||||
if (filteredCandidates.length === 0 && call.callForm === 'free') {
|
||||
// `freeFormHasClassTarget` was already computed for the S0 fast path
|
||||
// above under the same `callForm === 'free' && filteredCandidates.length === 0`
|
||||
// precondition. Reuse it to avoid a second `.some()` scan on the same pool.
|
||||
if (freeFormHasClassTarget) {
|
||||
filteredCandidates = filterCallableCandidates(
|
||||
tiered.candidates,
|
||||
call.argCount,
|
||||
'constructor',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Module-qualified constructor pattern: e.g. Python `import models; models.User()`.
|
||||
// The attribute access gives callForm='member', but the callee may be a Class — a valid
|
||||
// constructor target. Re-try with constructor-form filtering so that `module.ClassName()`
|
||||
|
|
@ -1610,22 +1629,11 @@ const resolveCallTarget = (
|
|||
}
|
||||
|
||||
if (filteredCandidates.length !== 1) {
|
||||
// Deduplicate: Swift extensions create multiple Class nodes with the same name.
|
||||
// When all candidates share the same type and differ only by file (extension vs
|
||||
// primary definition), they represent the same symbol. Prefer the primary
|
||||
// definition (shortest file path: Product.swift over ProductExtension.swift).
|
||||
if (filteredCandidates.length > 1) {
|
||||
const allSameType = filteredCandidates.every((c) => c.type === filteredCandidates[0].type);
|
||||
if (
|
||||
allSameType &&
|
||||
(filteredCandidates[0].type === 'Class' || filteredCandidates[0].type === 'Struct')
|
||||
) {
|
||||
const sorted = [...filteredCandidates].sort(
|
||||
(a, b) => a.filePath.length - b.filePath.length,
|
||||
);
|
||||
return toResolveResult(sorted[0], tiered.tier);
|
||||
}
|
||||
}
|
||||
// See `dedupSwiftExtensionCandidates` — returns non-null only when the
|
||||
// Swift-extension same-name collision heuristic applies. Otherwise null-
|
||||
// route (ambiguous candidates should not produce a wrong edge).
|
||||
const deduped = dedupSwiftExtensionCandidates(filteredCandidates, tiered.tier);
|
||||
if (deduped) return deduped;
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -1929,6 +1937,135 @@ export const resolveMemberCall = (
|
|||
return toResolveResult(resolved.def, resolved.tier);
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// SM-13: Free-function call resolution
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Resolve a free-function call using `lookupExact` (same-file) + import-scoped
|
||||
* resolution via `ctx.resolve()`.
|
||||
*
|
||||
* Used for `foo()`, `doStuff()` — unqualified calls with no receiver.
|
||||
* Also handles Swift/Kotlin implicit constructors (`User()` without `new`)
|
||||
* by delegating to {@link resolveStaticCall} when the tiered pool contains
|
||||
* class-like targets.
|
||||
*
|
||||
* {@link resolveCallTarget} delegates here for `callForm === 'free'` before
|
||||
* processing constructor and member calls.
|
||||
*
|
||||
* **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.
|
||||
*
|
||||
* **Asymmetry vs `resolveCallTarget`:** `resolveFreeCall` intentionally does
|
||||
* NOT take a `widenCache` parameter and does NOT run a D2 fuzzy-widening
|
||||
* pass. Member calls (`resolveCallTarget`'s main body) widen via
|
||||
* `lookupFuzzy` to reach parent-class methods defined in different files;
|
||||
* free calls have no receiver type and rely exclusively on the tiered pool
|
||||
* from `ctx.resolve()`. Phase 5 will revisit whether free calls need a
|
||||
* scoped widening pass once `lookupFuzzy` is retired.
|
||||
*
|
||||
* @param calledName - The called function name (e.g. 'doStuff')
|
||||
* @param filePath - File path of the call site
|
||||
* @param ctx - Resolution context
|
||||
* @param argCount - Optional argument count for arity filtering
|
||||
* @param tieredOverride - Pre-computed tiered candidates from an upstream
|
||||
* `ctx.resolve` call. When provided, skips the redundant
|
||||
* lookup inside this function.
|
||||
* @param overloadHints - Optional AST-based overload disambiguation hints
|
||||
* @param preComputedArgTypes - Optional pre-computed argument types (worker path)
|
||||
*/
|
||||
export const resolveFreeCall = (
|
||||
calledName: string,
|
||||
filePath: string,
|
||||
ctx: ResolutionContext,
|
||||
argCount?: number,
|
||||
tieredOverride?: TieredCandidates,
|
||||
overloadHints?: OverloadHints,
|
||||
preComputedArgTypes?: (string | undefined)[],
|
||||
): ResolveResult | null => {
|
||||
const tiered = tieredOverride ?? ctx.resolve(calledName, filePath);
|
||||
if (!tiered) return null;
|
||||
|
||||
let filteredCandidates = filterCallableCandidates(tiered.candidates, argCount, 'free');
|
||||
|
||||
// Class-target fast path: Swift/Kotlin `User()` — free-form call targeting a
|
||||
// class. Delegates to resolveStaticCall for O(1) class + constructor lookup.
|
||||
// The `.some()` trigger must stay aligned with `INSTANTIABLE_CLASS_TYPES` —
|
||||
// any type admitted here that is not in that set will cause resolveStaticCall
|
||||
// to return null, wasting two lookup passes per call. `Enum` is deliberately
|
||||
// excluded; `Record` is included so C# records and Kotlin data classes reach
|
||||
// the fast path.
|
||||
// Align with INSTANTIABLE_CLASS_TYPES by reusing the set directly rather
|
||||
// than enumerating literal strings. This converts an invariant that was
|
||||
// previously enforced by a comment ("keep this list aligned with
|
||||
// INSTANTIABLE_CLASS_TYPES") into one enforced structurally — any future
|
||||
// extension of the set (e.g. Kotlin `object`) propagates here automatically.
|
||||
// The `dedupSwiftExtensionCandidates` helper used in the tail of this
|
||||
// function deliberately uses a narrower literal `'Class' | 'Struct'` check
|
||||
// — Swift extensions only produce Class duplicates in practice, so Record
|
||||
// is excluded there by design. Do not collapse that helper into
|
||||
// INSTANTIABLE_CLASS_TYPES.
|
||||
const hasClassTarget =
|
||||
filteredCandidates.length === 0 &&
|
||||
tiered.candidates.some((c) => INSTANTIABLE_CLASS_TYPES.has(c.type));
|
||||
if (hasClassTarget) {
|
||||
const staticResult = resolveStaticCall(calledName, filePath, ctx, argCount, tiered);
|
||||
if (staticResult) return staticResult;
|
||||
// Retry with constructor form: Swift/Kotlin constructor calls look like
|
||||
// free function calls (no `new` keyword). If resolveStaticCall didn't
|
||||
// match, re-filter with constructor form so CONSTRUCTOR_TARGET_TYPES
|
||||
// applies.
|
||||
//
|
||||
// The retry fires for every null return from `resolveStaticCall`, which
|
||||
// can happen for three distinct reasons — all three are handled below:
|
||||
//
|
||||
// (a) No explicit `Constructor` node found and zero instantiable
|
||||
// class candidates (e.g. Interface/Trait/Impl only — the SM-12
|
||||
// null-route contract). `filterCallableCandidates` with
|
||||
// `'constructor'` form will also return nothing → we fall
|
||||
// through to the final null return. Correct.
|
||||
//
|
||||
// (b) Homonym ambiguity — two or more instantiable class candidates
|
||||
// share the name (e.g. `User` in two files, same tier). The
|
||||
// retry repopulates `filteredCandidates` with both Classes and
|
||||
// they flow into `dedupSwiftExtensionCandidates` below, which
|
||||
// either picks the shortest-path primary or null-routes.
|
||||
// Covered by the R7 Swift-extension dedup test.
|
||||
//
|
||||
// (c) `resolveStaticCall` step 4 bailed because the tiered pool
|
||||
// contains ownerless `Constructor` nodes (some extractors emit
|
||||
// constructors without `ownerId`). Those `Constructor` nodes
|
||||
// survive the constructor-form filter below and reach overload
|
||||
// disambiguation, giving the existing filter path a chance to
|
||||
// pick the right one. Correct but currently uncovered by a
|
||||
// dedicated test — the R5 `preComputedArgTypes` path exercises
|
||||
// overload disambiguation for Functions, which is structurally
|
||||
// the same code.
|
||||
filteredCandidates = filterCallableCandidates(tiered.candidates, argCount, 'constructor');
|
||||
}
|
||||
|
||||
// E. Overload disambiguation
|
||||
if (filteredCandidates.length > 1) {
|
||||
const disambiguated = overloadHints
|
||||
? tryOverloadDisambiguation(filteredCandidates, overloadHints)
|
||||
: preComputedArgTypes
|
||||
? matchCandidatesByArgTypes(filteredCandidates, preComputedArgTypes)
|
||||
: null;
|
||||
if (disambiguated) return toResolveResult(disambiguated, tiered.tier);
|
||||
}
|
||||
|
||||
if (filteredCandidates.length !== 1) {
|
||||
// See `dedupSwiftExtensionCandidates` — shared helper, single source of
|
||||
// truth for the Swift-extension same-name collision heuristic.
|
||||
const deduped = dedupSwiftExtensionCandidates(filteredCandidates, tiered.tier);
|
||||
if (deduped) return deduped;
|
||||
return null;
|
||||
}
|
||||
|
||||
return toResolveResult(filteredCandidates[0], tiered.tier);
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// SM-12: Constructor/static call resolution (no fuzzy lookup)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -1417,6 +1417,7 @@ describe('lookupMethodByOwnerWithMRO', () => {
|
|||
import {
|
||||
_resolveCallTargetForTesting,
|
||||
resolveMemberCall,
|
||||
resolveFreeCall,
|
||||
type OverloadHints,
|
||||
} from '../../src/core/ingestion/call-processor.js';
|
||||
|
||||
|
|
@ -2329,3 +2330,443 @@ describe('resolveStaticCall', () => {
|
|||
expect(result!.nodeId).toBe('ctor:User:2');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// resolveFreeCall — SM-13: free-function call resolution
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('resolveFreeCall', () => {
|
||||
let ctx: ResolutionContext;
|
||||
|
||||
beforeEach(() => {
|
||||
ctx = createResolutionContext();
|
||||
});
|
||||
|
||||
it('resolves a free function call via import-scoped resolution', () => {
|
||||
ctx.symbols.add('src/utils.ts', 'doStuff', 'func:doStuff', 'Function');
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/utils.ts']));
|
||||
|
||||
const result = resolveFreeCall('doStuff', 'src/app.ts', ctx);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:doStuff');
|
||||
expect(result!.confidence).toBe(0.9); // import-scoped tier
|
||||
expect(result!.reason).toBe('import-resolved');
|
||||
});
|
||||
|
||||
it('resolves a free function call via same-file resolution', () => {
|
||||
ctx.symbols.add('src/app.ts', 'helper', 'func:helper', 'Function');
|
||||
|
||||
const result = resolveFreeCall('helper', 'src/app.ts', ctx);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:helper');
|
||||
expect(result!.confidence).toBe(0.95); // same-file tier
|
||||
expect(result!.reason).toBe('same-file');
|
||||
});
|
||||
|
||||
it('returns null when no candidates exist', () => {
|
||||
const result = resolveFreeCall('nonexistent', 'src/app.ts', ctx);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for ambiguous free function calls (multiple candidates)', () => {
|
||||
ctx.symbols.add('src/a.ts', 'doStuff', 'func:a:doStuff', 'Function');
|
||||
ctx.symbols.add('src/b.ts', 'doStuff', 'func:b:doStuff', 'Function');
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/a.ts', 'src/b.ts']));
|
||||
|
||||
const result = resolveFreeCall('doStuff', 'src/app.ts', ctx);
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('delegates to resolveStaticCall for free-form class targets (Swift/Kotlin)', () => {
|
||||
ctx.symbols.add('src/user.swift', 'User', 'class:User', 'Class');
|
||||
ctx.importMap.set('src/app.swift', new Set(['src/user.swift']));
|
||||
|
||||
const result = resolveFreeCall('User', 'src/app.swift', ctx);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('class:User');
|
||||
});
|
||||
|
||||
it('delegates to resolveStaticCall for Record free-form targets (C#/Kotlin)', () => {
|
||||
ctx.symbols.add('src/User.cs', 'User', 'record:cs:User', 'Record');
|
||||
ctx.importMap.set('src/App.cs', new Set(['src/User.cs']));
|
||||
|
||||
const result = resolveFreeCall('User', 'src/App.cs', ctx);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('record:cs:User');
|
||||
});
|
||||
|
||||
it('null-routes Trait free-form calls via resolveStaticCall', () => {
|
||||
ctx.symbols.add('src/timestamps.php', 'HasTimestamps', 'trait:HasTimestamps', 'Trait');
|
||||
ctx.importMap.set('src/model.php', new Set(['src/timestamps.php']));
|
||||
|
||||
const result = resolveFreeCall('HasTimestamps', 'src/model.php', ctx);
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('uses tieredOverride when provided', () => {
|
||||
ctx.symbols.add('src/utils.ts', 'doStuff', 'func:doStuff', 'Function');
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/utils.ts']));
|
||||
|
||||
const tiered = ctx.resolve('doStuff', 'src/app.ts');
|
||||
expect(tiered).not.toBeNull();
|
||||
|
||||
// Spy on ctx.resolve to verify it is NOT called again
|
||||
const originalResolve = ctx.resolve.bind(ctx);
|
||||
let resolveCallCount = 0;
|
||||
ctx.resolve = ((name: string, fromFile: string) => {
|
||||
resolveCallCount++;
|
||||
return originalResolve(name, fromFile);
|
||||
}) as typeof ctx.resolve;
|
||||
|
||||
const result = resolveFreeCall('doStuff', 'src/app.ts', ctx, undefined, tiered!);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:doStuff');
|
||||
expect(resolveCallCount).toBe(0);
|
||||
});
|
||||
|
||||
it('routes through resolveCallTarget for free-form calls', () => {
|
||||
ctx.symbols.add('src/utils.ts', 'doStuff', 'func:doStuff', 'Function');
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/utils.ts']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{
|
||||
calledName: 'doStuff',
|
||||
callForm: 'free',
|
||||
},
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:doStuff');
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// PR #756 review follow-up (plan 2026-04-09-003): language coverage,
|
||||
// arity threading, Tier 3 resolution, preComputedArgTypes worker path,
|
||||
// Enum null-route, and Swift extension dedup guard.
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
// R2 — Language coverage: Go, Python, Rust, Java, JavaScript free-function
|
||||
// dispatch through _resolveCallTargetForTesting. resolveFreeCall has no
|
||||
// file-extension branching; these guard the dispatch chain per language.
|
||||
|
||||
it('resolves a Go free function (doStuff())', () => {
|
||||
ctx.symbols.add('src/helper.go', 'doStuff', 'func:go:doStuff', 'Function');
|
||||
ctx.importMap.set('src/main.go', new Set(['src/helper.go']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'doStuff', callForm: 'free' },
|
||||
'src/main.go',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:go:doStuff');
|
||||
});
|
||||
|
||||
it('resolves a Python free function (def helper(): ... helper())', () => {
|
||||
ctx.symbols.add('helpers.py', 'helper', 'func:py:helper', 'Function');
|
||||
ctx.importMap.set('app.py', new Set(['helpers.py']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'helper', callForm: 'free' },
|
||||
'app.py',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:py:helper');
|
||||
});
|
||||
|
||||
it('resolves a Rust free function outside any impl block (free_fn())', () => {
|
||||
ctx.symbols.add('src/helpers.rs', 'free_fn', 'func:rs:free_fn', 'Function');
|
||||
ctx.importMap.set('src/main.rs', new Set(['src/helpers.rs']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'free_fn', callForm: 'free' },
|
||||
'src/main.rs',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:rs:free_fn');
|
||||
});
|
||||
|
||||
it('resolves a Java statically-imported function (doStuff() after import static Utils.doStuff)', () => {
|
||||
// Note: this simulates the extractor output post static import by
|
||||
// indexing the function directly in its declaring file. The test guards
|
||||
// the dispatch chain for .java files, not the extractor's handling of
|
||||
// static imports specifically.
|
||||
ctx.symbols.add('src/Utils.java', 'doStuff', 'func:java:doStuff', 'Function');
|
||||
ctx.importMap.set('src/App.java', new Set(['src/Utils.java']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'doStuff', callForm: 'free' },
|
||||
'src/App.java',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:java:doStuff');
|
||||
});
|
||||
|
||||
it('resolves a JavaScript module-level function (moduleFn())', () => {
|
||||
ctx.symbols.add('src/helpers.js', 'moduleFn', 'func:js:moduleFn', 'Function');
|
||||
ctx.importMap.set('src/app.js', new Set(['src/helpers.js']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'moduleFn', callForm: 'free' },
|
||||
'src/app.js',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:js:moduleFn');
|
||||
});
|
||||
|
||||
// R3 — Arity filtering: call.argCount must narrow overloaded free functions
|
||||
// differing only in parameter count.
|
||||
|
||||
it('narrows overloaded free functions by argCount (2-arg overload selected)', () => {
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:0', 'Function', {
|
||||
parameterCount: 0,
|
||||
});
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:2', 'Function', {
|
||||
parameterCount: 2,
|
||||
});
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/utils.ts']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'helper', callForm: 'free', argCount: 2 },
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:helper:2');
|
||||
});
|
||||
|
||||
it('narrows overloaded free functions by argCount (0-arg overload selected)', () => {
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:0', 'Function', {
|
||||
parameterCount: 0,
|
||||
});
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:2', 'Function', {
|
||||
parameterCount: 2,
|
||||
});
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/utils.ts']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'helper', callForm: 'free', argCount: 0 },
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:helper:0');
|
||||
});
|
||||
|
||||
// R4 — Tier 3 (global) resolution: function globally visible but not
|
||||
// imported. Locks in TIER_CONFIDENCE.global === 0.5 and reason === 'global'
|
||||
// so a silent tier-table refactor surfaces here.
|
||||
|
||||
it('resolves a globally-visible free function via Tier 3 with global confidence', () => {
|
||||
ctx.symbols.add('lib/global.ts', 'helper', 'func:global:helper', 'Function');
|
||||
// No importMap entry — must fall through to Tier 3 (global).
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'helper', callForm: 'free' },
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:global:helper');
|
||||
expect(result!.confidence).toBe(0.5); // TIER_CONFIDENCE.global
|
||||
expect(result!.reason).toBe('global');
|
||||
});
|
||||
|
||||
// R5 — preComputedArgTypes worker path: when parse-worker pre-computes
|
||||
// argument types, the disambiguation routes through matchCandidatesByArgTypes.
|
||||
// Preconditions (verified at feasibility review):
|
||||
// 1. filteredCandidates.length > 1 — both overloads must survive arity
|
||||
// filtering, so argCount left unset here.
|
||||
// 2. overloadHints must be undefined — it takes precedence over
|
||||
// preComputedArgTypes at the disambiguation site.
|
||||
|
||||
it('disambiguates overloads via preComputedArgTypes (String overload matched)', () => {
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:str', 'Function', {
|
||||
parameterCount: 1,
|
||||
parameterTypes: ['String'],
|
||||
});
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:int', 'Function', {
|
||||
parameterCount: 1,
|
||||
parameterTypes: ['Int'],
|
||||
});
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/utils.ts']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'helper', callForm: 'free', argCount: 1 },
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
{ preComputedArgTypes: ['String'] },
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:helper:str');
|
||||
});
|
||||
|
||||
it('disambiguates overloads via preComputedArgTypes (Int overload matched)', () => {
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:str', 'Function', {
|
||||
parameterCount: 1,
|
||||
parameterTypes: ['String'],
|
||||
});
|
||||
ctx.symbols.add('src/utils.ts', 'helper', 'func:helper:int', 'Function', {
|
||||
parameterCount: 1,
|
||||
parameterTypes: ['Int'],
|
||||
});
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/utils.ts']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'helper', callForm: 'free', argCount: 1 },
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
// `Int` is normalized to `int` on the stored side via normalizeJvmTypeName
|
||||
// (matchCandidatesByArgTypes:1287). Real parse-worker-emitted argTypes are
|
||||
// already lowercase primitive names inferred from literals, so this
|
||||
// mirrors production call-site shape.
|
||||
{ preComputedArgTypes: ['int'] },
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:helper:int');
|
||||
});
|
||||
|
||||
// R6 — Enum free-form null-route: locks in the current behavior that
|
||||
// `Color()`-style calls on Enum types return null because Enum is
|
||||
// deliberately excluded from INSTANTIABLE_CLASS_TYPES. This is intentional
|
||||
// per PR #754 round 1 (see `call-processor.ts` INSTANTIABLE_CLASS_TYPES
|
||||
// JSDoc — "Enum excluded pending language-specific support with motivating
|
||||
// test fixtures"). If a future extension adds Enum to the set, this test
|
||||
// will need to be updated alongside that work — that is the correct signal.
|
||||
|
||||
it('null-routes Enum free-form calls (Color() — no instantiable fallback)', () => {
|
||||
ctx.symbols.add('src/color.ts', 'Color', 'enum:Color', 'Enum');
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/color.ts']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'Color', callForm: 'free' },
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
);
|
||||
|
||||
// Enum not in INSTANTIABLE_CLASS_TYPES → hasClassTarget is false →
|
||||
// resolveStaticCall is not called → tail dedup also doesn't fire →
|
||||
// falls through to the final null return.
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
// R7 — Swift extension dedup `filePath.length` heuristic guard:
|
||||
// Two same-name Class entries at different path lengths. The free-form
|
||||
// dispatch chain goes:
|
||||
// 1. filterCallableCandidates(tiered, argCount, 'free') strips Class →
|
||||
// filteredCandidates.length === 0
|
||||
// 2. hasClassTarget is true (both are Class)
|
||||
// 3. resolveStaticCall runs, has 2 homonym Class candidates →
|
||||
// instantiableCandidates.length > 1 → returns null (SM-12 round-1
|
||||
// null-route contract)
|
||||
// 4. Constructor-form retry: filterCallableCandidates(tiered, argCount,
|
||||
// 'constructor') keeps Class entries → filteredCandidates.length === 2
|
||||
// 5. Falls through to the Swift extension dedup block → sorts by
|
||||
// filePath.length → returns the shortest path.
|
||||
|
||||
it('dedupes Swift extension candidates by shortest file path (free-form retry path)', () => {
|
||||
// Two same-name Class entries, different path lengths.
|
||||
ctx.symbols.add('src/User.swift', 'User', 'class:User:primary', 'Class');
|
||||
ctx.symbols.add('src/Extensions/UserExtensions.swift', 'User', 'class:User:extension', 'Class');
|
||||
ctx.importMap.set(
|
||||
'src/App.swift',
|
||||
new Set(['src/User.swift', 'src/Extensions/UserExtensions.swift']),
|
||||
);
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'User', callForm: 'free' },
|
||||
'src/App.swift',
|
||||
ctx,
|
||||
);
|
||||
|
||||
// The shortest file path wins per the existing heuristic. This is a
|
||||
// behavior guard for finding #4 in the PR #756 review — if the dedup
|
||||
// heuristic changes, this test surfaces that intent.
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('class:User:primary');
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// PR #756 final review follow-up (comment 4215739052):
|
||||
// - Finding #3 low: ownerless-Constructor retry path (previously covered
|
||||
// by comment only) — adds the concrete test the reviewer asked for.
|
||||
// - Low-severity coverage gap: PHP free function (from the language
|
||||
// coverage table in the same review).
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
it('routes through resolveStaticCall retry when tiered pool contains an ownerless Constructor (free-form)', () => {
|
||||
// This exercises the third null-return reason documented in the retry
|
||||
// comment inside resolveFreeCall: resolveStaticCall's step-4 bailout when
|
||||
// the tiered pool contains Constructor nodes that lack ownerId (common in
|
||||
// some extractors). In that case:
|
||||
// 1. resolveStaticCall step 3 walks classCandidates via lookupMethodByOwner
|
||||
// — the ownerless Constructor is NOT in methodByOwner, so nothing found.
|
||||
// 2. Step 4 detects the Constructor in the tiered pool and bails out
|
||||
// with null so filterCallableCandidates can handle Constructor-vs-
|
||||
// Class preference correctly.
|
||||
// 3. resolveFreeCall's retry re-runs filterCallableCandidates with
|
||||
// 'constructor' form, which — per CONSTRUCTOR_TARGET_TYPES — prefers
|
||||
// the Constructor node over the Class node.
|
||||
// 4. Single survivor → returned as the call target.
|
||||
ctx.symbols.add('src/user.ts', 'User', 'class:User', 'Class');
|
||||
ctx.symbols.add('src/user.ts', 'User', 'ctor:User:ownerless', 'Constructor', {
|
||||
parameterCount: 0,
|
||||
// No ownerId — this is the pathological extractor output the retry path
|
||||
// exists to handle.
|
||||
});
|
||||
ctx.importMap.set('src/app.ts', new Set(['src/user.ts']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'User', callForm: 'free' },
|
||||
'src/app.ts',
|
||||
ctx,
|
||||
);
|
||||
|
||||
// The Constructor survives filterCallableCandidates's 'constructor' form
|
||||
// filter and is preferred over the Class (CONSTRUCTOR_TARGET_TYPES puts
|
||||
// Constructor first). Guards the (c) case in the retry-reasons comment.
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('ctor:User:ownerless');
|
||||
});
|
||||
|
||||
it('resolves a PHP free function (top-level helper())', () => {
|
||||
// PHP allows top-level function definitions outside any class. The
|
||||
// language coverage table in PR #756 review flagged this as uncovered;
|
||||
// this test exercises the `.php` dispatch path for free calls. Matches
|
||||
// the shape of the existing Go/Python/Rust/Java/JS language tests above.
|
||||
ctx.symbols.add('src/helpers.php', 'helper', 'func:php:helper', 'Function');
|
||||
ctx.importMap.set('src/app.php', new Set(['src/helpers.php']));
|
||||
|
||||
const result = _resolveCallTargetForTesting(
|
||||
{ calledName: 'helper', callForm: 'free' },
|
||||
'src/app.php',
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.nodeId).toBe('func:php:helper');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue