From 1a901124954579bd278909271072d29c62fc6dc7 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Mon, 16 Mar 2026 21:47:46 +0000 Subject: [PATCH] =?UTF-8?q?refactor:=20Phase=202=20architecture=20?= =?UTF-8?q?=E2=80=94=20shared=20helper,=20required=20params,=20decoupled?= =?UTF-8?q?=20type=20nodes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract resolveIterableElementType shared helper in shared.ts implementing 3-strategy fallback (declarationTypeNodes → scopeEnv string → AST walk) - Refactor TS, Python, Go extractors to use shared helper (eliminates 3x duplication) - Make ForLoopExtractor params required (aligned with PatternBindingExtractor) - Update Java, Kotlin, C# extractor signatures to accept required params - Decouple declarationTypeNodes from scopeEnv — capture raw type annotation nodes BEFORE extractDeclaration for container types (User[], []User, List[User]) - Hybrid approach: direct name extraction + keysBefore fallback for multi-declarator - Document declarationTypeNodes invariant change (superset of scopeEnv) --- gitnexus/src/core/ingestion/type-env.ts | 45 ++++++++++++++----- .../core/ingestion/type-extractors/csharp.ts | 7 ++- .../src/core/ingestion/type-extractors/go.ts | 31 +++---------- .../src/core/ingestion/type-extractors/jvm.ts | 14 +++++- .../core/ingestion/type-extractors/python.ts | 31 +++---------- .../core/ingestion/type-extractors/shared.ts | 38 ++++++++++++++++ .../core/ingestion/type-extractors/types.ts | 8 ++-- .../ingestion/type-extractors/typescript.ts | 31 +++---------- 8 files changed, 115 insertions(+), 90 deletions(-) diff --git a/gitnexus/src/core/ingestion/type-env.ts b/gitnexus/src/core/ingestion/type-env.ts index c56f18b04..62cb79712 100644 --- a/gitnexus/src/core/ingestion/type-env.ts +++ b/gitnexus/src/core/ingestion/type-env.ts @@ -3,7 +3,7 @@ import { FUNCTION_NODE_TYPES, extractFunctionName, CLASS_CONTAINER_TYPES } from import { SupportedLanguages } from '../../config/supported-languages.js'; import { typeConfigs, TYPED_PARAMETER_TYPES } from './type-extractors/index.js'; import type { ClassNameLookup } from './type-extractors/types.js'; -import { extractSimpleTypeName, stripNullable } from './type-extractors/shared.js'; +import { extractSimpleTypeName, extractVarName, stripNullable } from './type-extractors/shared.js'; import type { SymbolTable } from './symbol-table.js'; /** @@ -297,6 +297,9 @@ export const buildTypeEnv = ( // Maps `scope\0varName` → the type annotation AST node from the original declaration. // Allows pattern extractors to navigate back to the declaration's generic type arguments // (e.g., to extract T from Result for `if let Ok(x) = res`). + // NOTE: This is a SUPERSET of scopeEnv — entries exist even when extractSimpleTypeName + // returns undefined for container types (User[], []User, List[User]). This is intentional: + // for-loop Strategy 1 needs the raw AST type node for exactly those container types. const declarationTypeNodes = new Map(); /** @@ -314,17 +317,19 @@ export const buildTypeEnv = ( const extractTypeBinding = (node: SyntaxNode, scopeEnv: Map, scope: string): void => { // This guard eliminates 90%+ of calls before any language dispatch. if (TYPED_PARAMETER_TYPES.has(node.type)) { - const keysBefore = new Set(scopeEnv.keys()); - config.extractParameter(node, scopeEnv); - // Capture the type node for newly introduced parameter bindings + // Capture the raw type annotation BEFORE extractParameter — parameters + // consistently expose 'name' and 'type' fields across all languages. const typeNode = node.childForFieldName('type'); if (typeNode) { - for (const varName of scopeEnv.keys()) { - if (!keysBefore.has(varName)) { + const nameNode = node.childForFieldName('name'); + if (nameNode) { + const varName = extractVarName(nameNode); + if (varName && !declarationTypeNodes.has(`${scope}\0${varName}`)) { declarationTypeNodes.set(`${scope}\0${varName}`, typeNode); } } } + config.extractParameter(node, scopeEnv); return; } // For-each loop variable bindings (Java/C#/Kotlin): explicit element types in the AST. @@ -334,15 +339,31 @@ export const buildTypeEnv = ( return; } if (config.declarationNodeTypes.has(node.type)) { - const keysBefore = new Set(scopeEnv.keys()); - config.extractDeclaration(node, scopeEnv); - // Capture the type annotation AST node for newly introduced bindings. - // Only declarations with an explicit 'type' field are recorded — constructor - // inferences (Tier 1) don't have a type annotation node to preserve. + // Capture the raw type annotation AST node BEFORE extractDeclaration. + // This decouples type node capture from scopeEnv success — container types + // (User[], []User, List[User]) that fail extractSimpleTypeName still get + // their AST type node recorded for Strategy 1 for-loop resolution. + // Try direct extraction first (works for Go var_spec, Python assignment, Rust let_declaration). const typeNode = node.childForFieldName('type'); + if (typeNode) { + const nameNode = node.childForFieldName('name') + ?? node.childForFieldName('left') + ?? node.childForFieldName('pattern'); + if (nameNode) { + const varName = extractVarName(nameNode); + if (varName && !declarationTypeNodes.has(`${scope}\0${varName}`)) { + declarationTypeNodes.set(`${scope}\0${varName}`, typeNode); + } + } + } + // Run the language-specific declaration extractor (may or may not add to scopeEnv). + const keysBefore = new Set(scopeEnv.keys()); + config.extractDeclaration(node, scopeEnv); + // Fallback: for multi-declarator languages (TS, C#, Java) where the type field + // is on variable_declarator children, capture via keysBefore/keysAfter diff. if (typeNode) { for (const varName of scopeEnv.keys()) { - if (!keysBefore.has(varName)) { + if (!keysBefore.has(varName) && !declarationTypeNodes.has(`${scope}\0${varName}`)) { declarationTypeNodes.set(`${scope}\0${varName}`, typeNode); } } diff --git a/gitnexus/src/core/ingestion/type-extractors/csharp.ts b/gitnexus/src/core/ingestion/type-extractors/csharp.ts index e6dc993b3..2bab31dd4 100644 --- a/gitnexus/src/core/ingestion/type-extractors/csharp.ts +++ b/gitnexus/src/core/ingestion/type-extractors/csharp.ts @@ -132,7 +132,12 @@ const FOR_LOOP_NODE_TYPES: ReadonlySet = new Set([ ]); /** C#: foreach (User user in users) — extract loop variable binding */ -const extractForLoopBinding: ForLoopExtractor = (node: SyntaxNode, scopeEnv: Map): void => { +const extractForLoopBinding: ForLoopExtractor = ( + node: SyntaxNode, + scopeEnv: Map, + _declarationTypeNodes: ReadonlyMap, + _scope: string, +): void => { const typeNode = node.childForFieldName('type'); // The loop variable name is in the 'left' field in tree-sitter-c-sharp const nameNode = node.childForFieldName('left'); diff --git a/gitnexus/src/core/ingestion/type-extractors/go.ts b/gitnexus/src/core/ingestion/type-extractors/go.ts index fbbfd408a..cccc04ac9 100644 --- a/gitnexus/src/core/ingestion/type-extractors/go.ts +++ b/gitnexus/src/core/ingestion/type-extractors/go.ts @@ -1,6 +1,6 @@ import type { SyntaxNode } from '../utils.js'; import type { ConstructorBindingScanner, ForLoopExtractor, LanguageTypeConfig, ParameterExtractor, TypeBindingExtractor, PendingAssignmentExtractor } from './types.js'; -import { extractSimpleTypeName, extractVarName, extractElementTypeFromString, findChildByType } from './shared.js'; +import { extractSimpleTypeName, extractVarName, extractElementTypeFromString, findChildByType, resolveIterableElementType } from './shared.js'; const DECLARATION_NODE_TYPES: ReadonlySet = new Set([ 'var_declaration', @@ -283,8 +283,8 @@ const findGoParamElementType = (iterableName: string, startNode: SyntaxNode): st const extractForLoopBinding: ForLoopExtractor = ( node: SyntaxNode, scopeEnv: Map, - declarationTypeNodes?: ReadonlyMap, - scope?: string, + declarationTypeNodes: ReadonlyMap, + scope: string, ): void => { if (node.type !== 'for_statement') return; @@ -304,27 +304,10 @@ const extractForLoopBinding: ForLoopExtractor = ( if (!rightNode || rightNode.type !== 'identifier') return; const iterableName = rightNode.text; - let elementType: string | undefined; - - // Strategy 1: declarationTypeNodes — raw type annotation node (covers var decls with known types) - if (!elementType && declarationTypeNodes && scope) { - const typeAnnotationNode = declarationTypeNodes.get(`${scope}\0${iterableName}`); - if (typeAnnotationNode) { - elementType = extractGoElementTypeFromTypeNode(typeAnnotationNode); - } - } - - // Strategy 2: scopeEnv string — for locally declared vars where the type was stored - if (!elementType) { - const iterableType = scopeEnv.get(iterableName); - if (iterableType) elementType = extractElementTypeFromString(iterableType); - } - - // Strategy 3: AST walk — for []User parameters where extractSimpleTypeName returned undefined - if (!elementType) { - elementType = findGoParamElementType(iterableName, node); - } - + const elementType = resolveIterableElementType( + iterableName, node, scopeEnv, declarationTypeNodes, scope, + extractGoElementTypeFromTypeNode, findGoParamElementType, + ); if (!elementType) return; // The loop variable(s) are in the `left` field. diff --git a/gitnexus/src/core/ingestion/type-extractors/jvm.ts b/gitnexus/src/core/ingestion/type-extractors/jvm.ts index d4b7f9a7c..646d1a228 100644 --- a/gitnexus/src/core/ingestion/type-extractors/jvm.ts +++ b/gitnexus/src/core/ingestion/type-extractors/jvm.ts @@ -90,7 +90,12 @@ const JAVA_FOR_LOOP_NODE_TYPES: ReadonlySet = new Set([ ]); /** Java: for (User user : users) — extract loop variable binding */ -const extractJavaForLoopBinding: ForLoopExtractor = (node: SyntaxNode, scopeEnv: Map): void => { +const extractJavaForLoopBinding: ForLoopExtractor = ( + node: SyntaxNode, + scopeEnv: Map, + _declarationTypeNodes: ReadonlyMap, + _scope: string, +): void => { const typeNode = node.childForFieldName('type'); const nameNode = node.childForFieldName('name'); if (!typeNode || !nameNode) return; @@ -281,7 +286,12 @@ const KOTLIN_FOR_LOOP_NODE_TYPES: ReadonlySet = new Set([ ]); /** Kotlin: for (user: User in users) — extract loop variable binding when explicit type annotation exists */ -const extractKotlinForLoopBinding: ForLoopExtractor = (node: SyntaxNode, scopeEnv: Map): void => { +const extractKotlinForLoopBinding: ForLoopExtractor = ( + node: SyntaxNode, + scopeEnv: Map, + _declarationTypeNodes: ReadonlyMap, + _scope: string, +): void => { // Kotlin loop variable: variable_declaration child with optional user_type annotation const varDecl = findChildByType(node, 'variable_declaration'); if (!varDecl) return; diff --git a/gitnexus/src/core/ingestion/type-extractors/python.ts b/gitnexus/src/core/ingestion/type-extractors/python.ts index 8fbad67ad..e0e0c535b 100644 --- a/gitnexus/src/core/ingestion/type-extractors/python.ts +++ b/gitnexus/src/core/ingestion/type-extractors/python.ts @@ -1,6 +1,6 @@ import type { SyntaxNode } from '../utils.js'; import type { LanguageTypeConfig, ParameterExtractor, TypeBindingExtractor, InitializerExtractor, ClassNameLookup, ConstructorBindingScanner, PendingAssignmentExtractor, PatternBindingExtractor, ForLoopExtractor } from './types.js'; -import { extractSimpleTypeName, extractVarName, extractElementTypeFromString, extractGenericTypeArgs } from './shared.js'; +import { extractSimpleTypeName, extractVarName, extractElementTypeFromString, extractGenericTypeArgs, resolveIterableElementType } from './shared.js'; const DECLARATION_NODE_TYPES: ReadonlySet = new Set([ 'assignment', @@ -215,8 +215,8 @@ const findPyParamElementType = (iterableName: string, startNode: SyntaxNode): st const extractForLoopBinding: ForLoopExtractor = ( node: SyntaxNode, scopeEnv: Map, - declarationTypeNodes?: ReadonlyMap, - scope?: string, + declarationTypeNodes: ReadonlyMap, + scope: string, ): void => { if (node.type !== 'for_statement') return; @@ -225,27 +225,10 @@ const extractForLoopBinding: ForLoopExtractor = ( if (!rightNode || rightNode.type !== 'identifier') return; const iterableName = rightNode.text; - let elementType: string | undefined; - - // Strategy 1: declarationTypeNodes — raw type annotation node - if (!elementType && declarationTypeNodes && scope) { - const typeAnnotationNode = declarationTypeNodes.get(`${scope}\0${iterableName}`); - if (typeAnnotationNode) { - elementType = extractPyElementTypeFromAnnotation(typeAnnotationNode); - } - } - - // Strategy 2: scopeEnv string — for locally declared vars with container type strings - if (!elementType) { - const iterableType = scopeEnv.get(iterableName); - if (iterableType) elementType = extractElementTypeFromString(iterableType); - } - - // Strategy 3: AST walk — for List[User] parameters where extractSimpleTypeName returned undefined - if (!elementType) { - elementType = findPyParamElementType(iterableName, node); - } - + const elementType = resolveIterableElementType( + iterableName, node, scopeEnv, declarationTypeNodes, scope, + extractPyElementTypeFromAnnotation, findPyParamElementType, + ); if (!elementType) return; // The loop variable is the `left` field — a plain identifier. diff --git a/gitnexus/src/core/ingestion/type-extractors/shared.ts b/gitnexus/src/core/ingestion/type-extractors/shared.ts index 8925775a3..b01d001e5 100644 --- a/gitnexus/src/core/ingestion/type-extractors/shared.ts +++ b/gitnexus/src/core/ingestion/type-extractors/shared.ts @@ -1,5 +1,43 @@ import type { SyntaxNode } from '../utils.js'; +/** + * Shared 3-strategy fallback for resolving the element type of a container variable. + * Used by all for-loop extractors to resolve the loop variable's type from the iterable. + * + * Strategy 1: declarationTypeNodes — raw AST type annotation node (handles container types + * where extractSimpleTypeName returned undefined, e.g., User[], List[User]) + * Strategy 2: scopeEnv string — extractElementTypeFromString on the stored type string + * Strategy 3: AST walk — language-specific upward walk to enclosing function parameters + * + * @param extractFromTypeNode Language-specific function to extract element type from AST node + * @param findParamElementType Optional language-specific AST walk to find parameter type + */ +export function resolveIterableElementType( + iterableName: string, + node: SyntaxNode, + scopeEnv: ReadonlyMap, + declarationTypeNodes: ReadonlyMap, + scope: string, + extractFromTypeNode: (typeNode: SyntaxNode) => string | undefined, + findParamElementType?: (name: string, startNode: SyntaxNode) => string | undefined, +): string | undefined { + // Strategy 1: declarationTypeNodes AST node + const typeNode = declarationTypeNodes.get(`${scope}\0${iterableName}`); + if (typeNode) { + const t = extractFromTypeNode(typeNode); + if (t) return t; + } + // Strategy 2: scopeEnv string → extractElementTypeFromString + const iterableType = scopeEnv.get(iterableName); + if (iterableType) { + const el = extractElementTypeFromString(iterableType); + if (el) return el; + } + // Strategy 3: AST walk to function parameters + if (findParamElementType) return findParamElementType(iterableName, node); + return undefined; +} + /** Known single-arg nullable wrapper types that unwrap to their inner type * for receiver resolution. Optional → "User", Option → "User". * Only nullable wrappers — NOT containers (List, Vec) or async wrappers (Promise, Future). diff --git a/gitnexus/src/core/ingestion/type-extractors/types.ts b/gitnexus/src/core/ingestion/type-extractors/types.ts index 704e0fb22..07fb22591 100644 --- a/gitnexus/src/core/ingestion/type-extractors/types.ts +++ b/gitnexus/src/core/ingestion/type-extractors/types.ts @@ -24,12 +24,14 @@ export type ConstructorBindingScanner = (node: SyntaxNode) => { varName: string; * rather than in AST fields. Returns undefined if no return type can be determined. */ export type ReturnTypeExtractor = (node: SyntaxNode) => string | undefined; -/** Extracts loop variable type binding from a for-each statement. */ +/** Extracts loop variable type binding from a for-each statement. + * All parameters are required (aligned with PatternBindingExtractor convention) + * to prevent new extractors from silently ignoring declarationTypeNodes/scope. */ export type ForLoopExtractor = ( node: SyntaxNode, scopeEnv: Map, - declarationTypeNodes?: ReadonlyMap, - scope?: string, + declarationTypeNodes: ReadonlyMap, + scope: string, ) => void; /** Extracts a plain-identifier assignment for Tier 2 propagation. diff --git a/gitnexus/src/core/ingestion/type-extractors/typescript.ts b/gitnexus/src/core/ingestion/type-extractors/typescript.ts index dfcffafb3..8ec93a894 100644 --- a/gitnexus/src/core/ingestion/type-extractors/typescript.ts +++ b/gitnexus/src/core/ingestion/type-extractors/typescript.ts @@ -1,6 +1,6 @@ import type { SyntaxNode } from '../utils.js'; import type { LanguageTypeConfig, ParameterExtractor, TypeBindingExtractor, InitializerExtractor, ClassNameLookup, ConstructorBindingScanner, ReturnTypeExtractor, PendingAssignmentExtractor, ForLoopExtractor } from './types.js'; -import { extractSimpleTypeName, extractVarName, hasTypeAnnotation, unwrapAwait, extractCalleeName, extractElementTypeFromString, extractGenericTypeArgs } from './shared.js'; +import { extractSimpleTypeName, extractVarName, hasTypeAnnotation, unwrapAwait, extractCalleeName, extractElementTypeFromString, extractGenericTypeArgs, resolveIterableElementType } from './shared.js'; const DECLARATION_NODE_TYPES: ReadonlySet = new Set([ 'lexical_declaration', @@ -314,8 +314,8 @@ const findTsIterableElementType = (iterableName: string, startNode: SyntaxNode): const extractForLoopBinding: ForLoopExtractor = ( node: SyntaxNode, scopeEnv: Map, - declarationTypeNodes?: ReadonlyMap, - scope?: string, + declarationTypeNodes: ReadonlyMap, + scope: string, ): void => { if (node.type !== 'for_in_statement') return; @@ -335,27 +335,10 @@ const extractForLoopBinding: ForLoopExtractor = ( if (!rightNode || rightNode.type !== 'identifier') return; const iterableName = rightNode.text; - let elementType: string | undefined; - - // Strategy 1: declarationTypeNodes — raw type annotation node (avoids extractSimpleTypeName stripping) - if (!elementType && declarationTypeNodes && scope) { - const typeAnnotationNode = declarationTypeNodes.get(`${scope}\0${iterableName}`); - if (typeAnnotationNode) { - elementType = extractTsElementTypeFromAnnotation(typeAnnotationNode); - } - } - - // Strategy 2: scopeEnv string — for locally declared vars with container types like Array - if (!elementType) { - const iterableType = scopeEnv.get(iterableName); - if (iterableType) elementType = extractElementTypeFromString(iterableType); - } - - // Strategy 3: AST walk — for User[] parameters/locals where extractSimpleTypeName returned undefined - if (!elementType) { - elementType = findTsIterableElementType(iterableName, node); - } - + const elementType = resolveIterableElementType( + iterableName, node, scopeEnv, declarationTypeNodes, scope, + extractTsElementTypeFromAnnotation, findTsIterableElementType, + ); if (!elementType) return; // The loop variable is the `left` field. It may be wrapped in a variable_declarator.