From 06994e474a1bb49adaed44c28df13f026268d29d Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 19 Mar 2026 21:06:05 +0000 Subject: [PATCH] =?UTF-8?q?fix(type-resolution):=20address=20PR=20#387=20r?= =?UTF-8?q?eview=20=E2=80=94=20dead=20code,=20nullable=5Ftype,=20scope=20b?= =?UTF-8?q?oundaries=20+=20integration=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove dead replayPendingItems array and inert if-block in type-env.ts - Add nullable_type fallback in extractKotlinDeclaration for val x: User? local vars - Tighten isCSharpNullableDecl to avoid substring false positives on type names - Add missing scope boundaries: function_expression (TS), constructor_declaration/ local_function_statement/lambda_expression (C#) in null-check narrowing walkers - Extend null-check narrowing fixtures and add 4 integration tests covering: Kotlin local variable nullable, C# constructor + lambda, TS function expression --- gitnexus/src/core/ingestion/type-env.ts | 10 +------ .../core/ingestion/type-extractors/csharp.ts | 9 ++++--- .../src/core/ingestion/type-extractors/jvm.ts | 3 ++- .../ingestion/type-extractors/typescript.ts | 4 +-- .../Services/App.cs | 21 +++++++++++++++ .../models/User.kt | 4 +++ .../services/App.kt | 8 ++++++ .../ts-null-check-narrowing/src/app.ts | 6 +++++ .../test/integration/resolvers/csharp.test.ts | 16 ++++++++++++ .../test/integration/resolvers/kotlin.test.ts | 8 ++++++ .../integration/resolvers/typescript.test.ts | 8 ++++++ type-resolution-roadmap.md | 26 +++++++++++++++++-- 12 files changed, 105 insertions(+), 18 deletions(-) diff --git a/gitnexus/src/core/ingestion/type-env.ts b/gitnexus/src/core/ingestion/type-env.ts index 345f6233f..121e7d01f 100644 --- a/gitnexus/src/core/ingestion/type-env.ts +++ b/gitnexus/src/core/ingestion/type-env.ts @@ -894,20 +894,12 @@ export const buildTypeEnv = ( // - fixpoint: users → User[] // - replay: users now typed → u → User if (pendingForLoops.length > 0 && config.extractForLoopBinding) { - const replayPendingItems: Array<{ scope: string } & PendingAssignment> = []; for (const { node, scope } of pendingForLoops) { if (!env.has(scope)) env.set(scope, new Map()); const scopeEnv = env.get(scope)!; config.extractForLoopBinding(node, { scopeEnv, declarationTypeNodes, scope, returnTypeLookup }); } - // Collect any new pending items from replay-produced variables. - // Re-walk the for-loop bodies to pick up field/method chains on the now-typed loop vars. - // For simplicity, run a mini-fixpoint on any pending items that were already collected - // but couldn't resolve because they depended on the loop variable. - if (replayPendingItems.length > 0) { - resolveFixpointBindings(replayPendingItems, env, returnTypeLookup, symbolTable); - } - // Also re-run the main fixpoint to resolve items that depended on loop variables. + // Re-run the main fixpoint to resolve items that depended on loop variables. // Only needed if replay actually produced new bindings. const unresolvedBefore = pendingItems.filter((item) => { const scopeEnv = env.get(item.scope); diff --git a/gitnexus/src/core/ingestion/type-extractors/csharp.ts b/gitnexus/src/core/ingestion/type-extractors/csharp.ts index 2af9df5df..6071c3366 100644 --- a/gitnexus/src/core/ingestion/type-extractors/csharp.ts +++ b/gitnexus/src/core/ingestion/type-extractors/csharp.ts @@ -296,18 +296,19 @@ const findCSharpIfConsequenceBlock = (expr: SyntaxNode): SyntaxNode | undefined } return undefined; } - if (current.type === 'block' || current.type === 'method_declaration') return undefined; + if (current.type === 'block' || current.type === 'method_declaration' + || current.type === 'constructor_declaration' || current.type === 'local_function_statement' + || current.type === 'lambda_expression') return undefined; current = current.parent; } return undefined; }; /** Check if a C# declaration type node represents a nullable type. - * Checks for nullable_type node or text containing '?' or 'null'. */ + * Checks for nullable_type AST node or '?' in the type text (e.g., User?). */ const isCSharpNullableDecl = (declTypeNode: SyntaxNode): boolean => { if (declTypeNode.type === 'nullable_type') return true; - const text = declTypeNode.text; - return text.includes('?') || text.includes('null'); + return declTypeNode.text.includes('?'); }; const extractPatternBinding: PatternBindingExtractor = (node, scopeEnv, declarationTypeNodes, scope) => { diff --git a/gitnexus/src/core/ingestion/type-extractors/jvm.ts b/gitnexus/src/core/ingestion/type-extractors/jvm.ts index 3604b50eb..0ad51ffb0 100644 --- a/gitnexus/src/core/ingestion/type-extractors/jvm.ts +++ b/gitnexus/src/core/ingestion/type-extractors/jvm.ts @@ -295,7 +295,8 @@ const extractKotlinDeclaration: TypeBindingExtractor = (node: SyntaxNode, env: M const varDecl = findChildByType(node, 'variable_declaration'); if (varDecl) { const nameNode = findChildByType(varDecl, 'simple_identifier'); - const typeNode = findChildByType(varDecl, 'user_type'); + const typeNode = findChildByType(varDecl, 'user_type') + ?? findChildByType(varDecl, 'nullable_type'); if (!nameNode || !typeNode) return; const varName = extractVarName(nameNode); const typeName = extractSimpleTypeName(typeNode); diff --git a/gitnexus/src/core/ingestion/type-extractors/typescript.ts b/gitnexus/src/core/ingestion/type-extractors/typescript.ts index 778631875..d193e8399 100644 --- a/gitnexus/src/core/ingestion/type-extractors/typescript.ts +++ b/gitnexus/src/core/ingestion/type-extractors/typescript.ts @@ -529,8 +529,8 @@ const findIfConsequenceBlock = (binaryExpr: SyntaxNode): SyntaxNode | undefined return undefined; } // Stop climbing at function/block boundaries — don't cross scope - if (current.type === 'function_declaration' || current.type === 'arrow_function' - || current.type === 'method_definition') return undefined; + if (current.type === 'function_declaration' || current.type === 'function_expression' + || current.type === 'arrow_function' || current.type === 'method_definition') return undefined; current = current.parent; } return undefined; diff --git a/gitnexus/test/fixtures/lang-resolution/csharp-null-check-narrowing/Services/App.cs b/gitnexus/test/fixtures/lang-resolution/csharp-null-check-narrowing/Services/App.cs index 84b2fe0f1..0bc217252 100644 --- a/gitnexus/test/fixtures/lang-resolution/csharp-null-check-narrowing/Services/App.cs +++ b/gitnexus/test/fixtures/lang-resolution/csharp-null-check-narrowing/Services/App.cs @@ -1,9 +1,18 @@ using NullCheck.Models; +using System; namespace NullCheck.Services { public class App { + public App(User? x) + { + if (x != null) + { + x.Save(); + } + } + public void ProcessInequality(User x) { if (x != null) @@ -19,5 +28,17 @@ namespace NullCheck.Services x.Save(); } } + + public void ProcessInLambda(User? x) + { + Action act = () => + { + if (x != null) + { + x.Save(); + } + }; + act(); + } } } diff --git a/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/models/User.kt b/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/models/User.kt index 494376e42..77912abe3 100644 --- a/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/models/User.kt +++ b/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/models/User.kt @@ -3,3 +3,7 @@ package models class User { fun save() {} } + +fun findUser(): User? { + return null +} diff --git a/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/services/App.kt b/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/services/App.kt index 4578c07f5..393833b6f 100644 --- a/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/services/App.kt +++ b/gitnexus/test/fixtures/lang-resolution/kotlin-null-check-narrowing/services/App.kt @@ -1,9 +1,17 @@ package services import models.User +import models.findUser fun processNullable(x: User?) { if (x != null) { x.save() } } + +fun processLocalNullable() { + val x: User? = findUser() + if (x != null) { + x.save() + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/ts-null-check-narrowing/src/app.ts b/gitnexus/test/fixtures/lang-resolution/ts-null-check-narrowing/src/app.ts index b73cb15dc..2242d32df 100644 --- a/gitnexus/test/fixtures/lang-resolution/ts-null-check-narrowing/src/app.ts +++ b/gitnexus/test/fixtures/lang-resolution/ts-null-check-narrowing/src/app.ts @@ -17,3 +17,9 @@ function processUndefined(x: User | undefined) { x.save(); } } + +const processFuncExpr = function(x: User | null) { + if (x !== null) { + x.save(); + } +}; diff --git a/gitnexus/test/integration/resolvers/csharp.test.ts b/gitnexus/test/integration/resolvers/csharp.test.ts index 5a5f6caa6..049590524 100644 --- a/gitnexus/test/integration/resolvers/csharp.test.ts +++ b/gitnexus/test/integration/resolvers/csharp.test.ts @@ -1452,4 +1452,20 @@ describe('C# null-check narrowing resolution (Phase C)', () => { ); expect(wrongCall).toBeUndefined(); }); + + it('resolves x.Save() inside constructor via null-check narrowing', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find(c => + c.target === 'Save' && c.source === 'App' && c.targetFilePath.includes('User'), + ); + expect(saveCall).toBeDefined(); + }); + + it('resolves x.Save() inside lambda via null-check narrowing', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find(c => + c.target === 'Save' && c.source === 'ProcessInLambda' && c.targetFilePath.includes('User'), + ); + expect(saveCall).toBeDefined(); + }); }); diff --git a/gitnexus/test/integration/resolvers/kotlin.test.ts b/gitnexus/test/integration/resolvers/kotlin.test.ts index b8442d39d..4db02d4dd 100644 --- a/gitnexus/test/integration/resolvers/kotlin.test.ts +++ b/gitnexus/test/integration/resolvers/kotlin.test.ts @@ -1523,4 +1523,12 @@ describe('Kotlin null-check narrowing resolution (Phase C)', () => { ); expect(wrongCall).toBeUndefined(); }); + + it('resolves x.save() from local variable val x: User? via null-check narrowing', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find(c => + c.target === 'save' && c.source === 'processLocalNullable' && c.targetFilePath.includes('User'), + ); + expect(saveCall).toBeDefined(); + }); }); diff --git a/gitnexus/test/integration/resolvers/typescript.test.ts b/gitnexus/test/integration/resolvers/typescript.test.ts index ce4f30240..6352d4024 100644 --- a/gitnexus/test/integration/resolvers/typescript.test.ts +++ b/gitnexus/test/integration/resolvers/typescript.test.ts @@ -2213,4 +2213,12 @@ describe('TypeScript null-check narrowing resolution (Phase C)', () => { ); expect(saveCall).toBeDefined(); }); + + it('resolves x.save() inside function expression null-check (processFuncExpr)', () => { + const calls = getRelationships(result, 'CALLS'); + const saveCall = calls.find(c => + c.target === 'save' && c.source === 'processFuncExpr' && c.targetFilePath.includes('models'), + ); + expect(saveCall).toBeDefined(); + }); }); diff --git a/type-resolution-roadmap.md b/type-resolution-roadmap.md index 2284d8a5c..3837e0b18 100644 --- a/type-resolution-roadmap.md +++ b/type-resolution-roadmap.md @@ -96,6 +96,22 @@ The goal is not to build a compiler. The goal is to support high-value static an ## Open Phases +### Phase P: Polymorphism & Overloading + +**Plan:** `docs/plans/2026-03-19-feat-polymorphism-overloading-type-resolution-plan.md` + +Four incremental phases: +1. **Parameter type metadata** — extend `SymbolDefinition` with `parameterTypes: string[]` extracted during parsing +2. **Overload disambiguation** — filter overloaded methods by argument literal types at call sites +3. **Constructor-visible virtual dispatch** — `Base b = new Derived(); b.method()` resolves to `Derived#method` when constructor type is a known subclass +4. **Covariant return type awareness** — prefer child's return type over inherited definition + +Languages benefiting: Java, Kotlin, C#, C++, TypeScript (overloading). All OOP languages (virtual dispatch). + +**Impact: High | Effort: High** + +--- + ### Phase S: Swift Parity **Blocked on** tree-sitter-swift Node 22 compatibility. @@ -145,10 +161,12 @@ config.validate(); // missed ``` Milestone D (Phases A, B, C) ✅ ──┐ ├──→ Phase 14 (cross-file) +Phase P (polymorphism) ───────────┤ + │ Phase S (Swift parity) ───────────┘ -Phase S is independent of Phase 14. -Phase 14 depends on Milestone D being stable. +Phase P and Phase S are independent of each other and Phase 14. +Phase 14 benefits from Phase P (better per-file resolution = fewer cross-file gaps). ``` --- @@ -187,6 +205,10 @@ Consolidated Phases 10–13 into 3 balanced phases. Loop-fixpoint bridge, MRO-aw Export-type index, cross-file binding propagation. +### Milestone P — Polymorphism & Overloading (Phase P) + +Parameter type metadata, overload disambiguation, constructor-visible virtual dispatch, covariant return types. + ### Milestone S — Swift Parity (Phase S) For-loop binding, assignment chains, `guard let` narrowing. Blocked on tree-sitter-swift Node 22.