mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-06 02:49:56 +00:00
fix(swift): move duplicate type ordering into provider
Keep Swift extension candidate ordering behind the LanguageProvider contract and cover the Swift 0.7 init scanner path so parser runtime changes do not leak language-specific logic into shared resolution. Made-with: Cursor
This commit is contained in:
parent
cf3df472b1
commit
50d9b69860
6 changed files with 116 additions and 77 deletions
|
|
@ -12,7 +12,7 @@ type ReceiverSource = ReceiverEnriched['receiverSource'];
|
|||
* DAG stage 4 fallback: used when `selectDispatch` is absent or returns null.
|
||||
* Preserves pre-DAG dispatch semantics:
|
||||
* - 'constructor' → constructor branch
|
||||
* - 'free' → free branch (admits Swift/Kotlin class-target fast path)
|
||||
* - 'free' → free branch (admits class-target fast path)
|
||||
* - 'member' or undefined → owner-scoped branch
|
||||
*
|
||||
* `undefined` callForm MUST route through owner-scoped (not free) so bare
|
||||
|
|
@ -1595,49 +1595,30 @@ const disambiguateByOverloadOrArgTypes = (
|
|||
return null;
|
||||
};
|
||||
|
||||
/**
|
||||
* 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`.
|
||||
*
|
||||
* Used by free-call, constructor, and member-call paths. Having a single
|
||||
* source of truth prevents duplication if the heuristic is ever tuned.
|
||||
*/
|
||||
const dedupSwiftExtensionCandidates = (
|
||||
const orderProviderSameNameTypeCandidates = (
|
||||
candidates: readonly SymbolDefinition[],
|
||||
tier: ResolutionTier,
|
||||
): ResolveResult | null => {
|
||||
const sorted = orderSwiftExtensionCandidates(candidates);
|
||||
return sorted ? toResolveResult(sorted[0], tier) : null;
|
||||
typeName: string,
|
||||
filePath: string,
|
||||
): readonly SymbolDefinition[] | null => {
|
||||
const language = getLanguageFromFilename(filePath);
|
||||
if (language == null) return null;
|
||||
return (
|
||||
getProvider(language).orderSameNameTypeCandidates?.({
|
||||
typeName,
|
||||
callSiteFilePath: filePath,
|
||||
candidates,
|
||||
}) ?? null
|
||||
);
|
||||
};
|
||||
|
||||
const orderSwiftExtensionCandidates = (
|
||||
const resolveProviderPrimaryTypeCandidate = (
|
||||
candidates: readonly SymbolDefinition[],
|
||||
): SymbolDefinition[] | 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;
|
||||
return [...candidates].sort(
|
||||
(a, b) => a.filePath.length - b.filePath.length || a.filePath.localeCompare(b.filePath),
|
||||
);
|
||||
tier: ResolutionTier,
|
||||
typeName: string,
|
||||
filePath: string,
|
||||
): ResolveResult | null => {
|
||||
const ordered = orderProviderSameNameTypeCandidates(candidates, typeName, filePath);
|
||||
return ordered && ordered.length > 0 ? toResolveResult(ordered[0], tier) : null;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -2232,11 +2213,13 @@ const resolveMethodByOwner = (
|
|||
}
|
||||
|
||||
if (!firstDef && !ambiguous) {
|
||||
const extensionCandidates = orderSwiftExtensionCandidates(
|
||||
const orderedTypeCandidates = orderProviderSameNameTypeCandidates(
|
||||
ctx.model.types.lookupClassByName(receiverTypeName),
|
||||
receiverTypeName,
|
||||
filePath,
|
||||
);
|
||||
if (extensionCandidates) {
|
||||
for (const candidate of extensionCandidates) {
|
||||
if (orderedTypeCandidates) {
|
||||
for (const candidate of orderedTypeCandidates) {
|
||||
const def = canWalkMRO
|
||||
? lookupMethodByOwnerWithMRO(
|
||||
candidate.nodeId,
|
||||
|
|
@ -2325,9 +2308,9 @@ export const resolveMemberCall = (
|
|||
* 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.
|
||||
* Also handles 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'`.
|
||||
*
|
||||
|
|
@ -2359,33 +2342,30 @@ export const resolveFreeCall = (
|
|||
|
||||
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.
|
||||
// Class-target fast path: 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.
|
||||
// excluded; `Record` is included so record-like class targets 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.
|
||||
// extension of the set propagates here automatically.
|
||||
// Language providers can still choose a primary same-name type candidate in
|
||||
// the tail of this function when their grammars index one logical type
|
||||
// multiple times.
|
||||
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.
|
||||
// Retry with constructor form for languages whose constructor calls look
|
||||
// like free function calls. 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:
|
||||
|
|
@ -2399,9 +2379,8 @@ export const resolveFreeCall = (
|
|||
// (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.
|
||||
// they flow into the provider same-name candidate hook below, which
|
||||
// can pick a primary definition or null-route.
|
||||
//
|
||||
// (c) `resolveStaticCall` step 4 bailed because the tiered pool
|
||||
// contains ownerless `Constructor` nodes (some extractors emit
|
||||
|
|
@ -2426,10 +2405,13 @@ export const resolveFreeCall = (
|
|||
}
|
||||
|
||||
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;
|
||||
const primary = resolveProviderPrimaryTypeCandidate(
|
||||
filteredCandidates,
|
||||
tiered.tier,
|
||||
calledName,
|
||||
filePath,
|
||||
);
|
||||
if (primary) return primary;
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
@ -2594,12 +2576,17 @@ export const resolveStaticCall = (
|
|||
// Interface / Trait / Impl). Null-route via the fall-through `return
|
||||
// null` — this is the dominant Codex-fix case.
|
||||
// length === 1 → a single instantiable candidate remains, return it.
|
||||
// length > 1 → two or more instantiable classes share the name (e.g.
|
||||
// homonym classes across files with no import narrowing). Fall through
|
||||
// to `return null` so the caller null-routes rather than guess.
|
||||
// length > 1 → let the call-site provider choose a primary when it can
|
||||
// prove the candidates are one logical type; otherwise null-route.
|
||||
const primary = resolveProviderPrimaryTypeCandidate(
|
||||
instantiableCandidates,
|
||||
typeResolved.tier,
|
||||
className,
|
||||
currentFile,
|
||||
);
|
||||
if (primary) return primary;
|
||||
|
||||
if (instantiableCandidates.length === 1) {
|
||||
const primary = orderSwiftExtensionCandidates(allClasses);
|
||||
if (primary) return toResolveResult(primary[0], typeResolved.tier);
|
||||
return toResolveResult(instantiableCandidates[0], typeResolved.tier);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -498,6 +498,15 @@ interface LanguageProviderConfig {
|
|||
|
||||
// ── Resolution phase (RFC §4v2) ────────────────────────────────────
|
||||
|
||||
/** Order same-name type candidates when a language can index multiple
|
||||
* definitions for one logical type. Return null to keep shared ambiguity
|
||||
* handling. */
|
||||
readonly orderSameNameTypeCandidates?: (params: {
|
||||
readonly typeName: string;
|
||||
readonly callSiteFilePath: string;
|
||||
readonly candidates: readonly SymbolDefinition[];
|
||||
}) => readonly SymbolDefinition[] | null;
|
||||
|
||||
/**
|
||||
* Is this callable definition compatible with the given call-site arity?
|
||||
* Language-specific rules: Python `*args`/`**kwargs`/defaults, JS default
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@
|
|||
*/
|
||||
|
||||
import { SupportedLanguages } from 'gitnexus-shared';
|
||||
import type { NodeLabel } from 'gitnexus-shared';
|
||||
import type { NodeLabel, SymbolDefinition } from 'gitnexus-shared';
|
||||
import { createClassExtractor } from '../class-extractors/generic.js';
|
||||
import { swiftClassConfig } from '../class-extractors/configs/swift.js';
|
||||
import { defineLanguage } from '../language-provider.js';
|
||||
|
|
@ -128,6 +128,22 @@ const swiftExtractFunctionName = (
|
|||
return null; // fall through to generic
|
||||
};
|
||||
|
||||
const orderSwiftSameNameTypeCandidates = ({
|
||||
candidates,
|
||||
}: {
|
||||
readonly typeName: string;
|
||||
readonly callSiteFilePath: string;
|
||||
readonly candidates: readonly SymbolDefinition[];
|
||||
}): readonly SymbolDefinition[] | null => {
|
||||
if (candidates.length <= 1) return null;
|
||||
if (!candidates.every((c) => c.type === candidates[0].type)) return null;
|
||||
if (candidates[0].type !== 'Class' && candidates[0].type !== 'Struct') return null;
|
||||
if (!candidates.every((c) => c.filePath.endsWith('.swift'))) return null;
|
||||
return [...candidates].sort(
|
||||
(a, b) => a.filePath.length - b.filePath.length || a.filePath.localeCompare(b.filePath),
|
||||
);
|
||||
};
|
||||
|
||||
const BUILT_INS: ReadonlySet<string> = new Set([
|
||||
'print',
|
||||
'debugPrint',
|
||||
|
|
@ -257,5 +273,6 @@ export const swiftProvider = defineLanguage({
|
|||
classExtractor: createClassExtractor(swiftClassConfig),
|
||||
heritageExtractor: createHeritageExtractor(SupportedLanguages.Swift),
|
||||
implicitImportWirer: wireSwiftImplicitImports,
|
||||
orderSameNameTypeCandidates: orderSwiftSameNameTypeCandidates,
|
||||
builtInNames: BUILT_INS,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -41,6 +41,10 @@ function unwrapSwiftExpression(node: SyntaxNode): SyntaxNode {
|
|||
return node;
|
||||
}
|
||||
|
||||
function swiftNavigationSuffixName(node: SyntaxNode | null): string | undefined {
|
||||
return node?.type === 'navigation_suffix' ? node.lastNamedChild?.text : node?.text;
|
||||
}
|
||||
|
||||
/** Swift: let x: Foo = ... */
|
||||
const extractDeclaration: TypeBindingExtractor = (
|
||||
node: SyntaxNode,
|
||||
|
|
@ -119,8 +123,10 @@ const extractInitializer: InitializerExtractor = (
|
|||
// Explicit init: User.init(name: "alice") — navigation_expression with .init suffix
|
||||
if (callee.type === 'navigation_expression') {
|
||||
const receiver = callee.firstNamedChild;
|
||||
const suffix = callee.lastNamedChild;
|
||||
if (receiver?.type === 'simple_identifier' && suffix?.text === 'init') {
|
||||
if (
|
||||
receiver?.type === 'simple_identifier' &&
|
||||
swiftNavigationSuffixName(callee.lastNamedChild) === 'init'
|
||||
) {
|
||||
const calleeName = receiver.text;
|
||||
if (calleeName && classNames.has(calleeName)) {
|
||||
env.set(varName, calleeName);
|
||||
|
|
@ -133,7 +139,7 @@ const extractInitializer: InitializerExtractor = (
|
|||
const scanConstructorBinding: ConstructorBindingScanner = (node) => {
|
||||
if (node.type !== 'property_declaration') return undefined;
|
||||
if (hasTypeAnnotation(node)) return undefined;
|
||||
const pattern = node.childForFieldName('pattern');
|
||||
const pattern = node.childForFieldName('pattern') ?? findChild(node, 'pattern');
|
||||
if (!pattern) return undefined;
|
||||
const varName = pattern.text;
|
||||
if (!varName) return undefined;
|
||||
|
|
@ -162,7 +168,7 @@ const scanConstructorBinding: ConstructorBindingScanner = (node) => {
|
|||
if (callee.type === 'navigation_expression') {
|
||||
const receiver = callee.firstNamedChild;
|
||||
const suffix = callee.lastNamedChild;
|
||||
if (receiver?.type === 'simple_identifier' && suffix?.text === 'init') {
|
||||
if (receiver?.type === 'simple_identifier' && swiftNavigationSuffixName(suffix) === 'init') {
|
||||
return { varName, calleeName: receiver.text };
|
||||
}
|
||||
// General qualified call: service.getUser() → extract method name.
|
||||
|
|
|
|||
|
|
@ -4385,12 +4385,14 @@ class Foo {
|
|||
expect(m.parameters[0]).toEqual({
|
||||
name: 'name',
|
||||
type: 'String',
|
||||
rawType: 'String',
|
||||
isOptional: false,
|
||||
isVariadic: false,
|
||||
});
|
||||
expect(m.parameters[1]).toEqual({
|
||||
name: 'age',
|
||||
type: 'Int',
|
||||
rawType: 'Int',
|
||||
isOptional: true,
|
||||
isVariadic: false,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2895,6 +2895,24 @@ class User : BaseModel<string> {
|
|||
expect(typeEnv.constructorBindings.find((b) => b.varName === 'user')).toBeUndefined();
|
||||
});
|
||||
|
||||
describeSwift('Swift constructor binding scanner', () => {
|
||||
it('returns constructor binding for explicit User.init(...) calls', () => {
|
||||
const tree = parseSwift(`
|
||||
func run() {
|
||||
let user = User.init(name: "alice")
|
||||
}
|
||||
`);
|
||||
const typeEnv = buildTypeEnv(tree, 'swift');
|
||||
expect(flatGet(typeEnv, 'user')).toBeUndefined();
|
||||
expect(typeEnv.constructorBindings).toEqual([
|
||||
expect.objectContaining({
|
||||
varName: 'user',
|
||||
calleeName: 'User',
|
||||
}),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
it('returns constructor bindings for Python x = UnknownClass()', () => {
|
||||
const tree = parse(
|
||||
`
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue