fix: position-indexed when/is bindings, Kotlin param extraction, HashMap.values for-loop

Three root causes for failing Kotlin integration tests:

1. When/is multi-arm resolution: flat scopeEnv stored only the last arm's
   type (last-writer-wins). Added PatternOverrides with AST range indexing
   so each when arm resolves to its narrowed type independently.

2. HashMap.values for-loop: navigation_expression without call_suffix was
   classified as bare property access (iterableName='values' instead of
   'data'). Now tries object-as-iterable + property-as-method first, with
   fallback to property-as-iterable for this.users patterns.

3. Kotlin parameter extraction: tree-sitter-kotlin parameter nodes use
   positional children (simple_identifier, user_type) not named fields
   (name, type). Added fallback to findChildByType in both
   extractKotlinParameter and extractTypeBinding.

Integration tests added for .keys/.values/Set/MutableMap iteration,
3-arm when/is, multi-call within arms, and when+else branch.
This commit is contained in:
Gergo Magyar 2026-03-17 10:14:03 +00:00
parent 294bfddaf2
commit 08902f8a18
9 changed files with 366 additions and 15 deletions

View file

@ -48,11 +48,42 @@ export interface TypeEnvironment {
readonly env: TypeEnv;
}
/**
* Position-indexed pattern binding: active only within a specific AST range.
* Used for smart-cast narrowing in mutually exclusive branches (e.g., Kotlin when arms).
*/
interface PatternOverride {
rangeStart: number;
rangeEnd: number;
typeName: string;
}
/** scope → varName → overrides (checked in order, first range match wins) */
type PatternOverrides = Map<string, Map<string, PatternOverride[]>>;
/** AST node types that represent mutually exclusive branch containers for pattern bindings. */
const PATTERN_BRANCH_TYPES = new Set([
'when_entry', // Kotlin when
'switch_block_label', // Java switch (enhanced)
]);
/** Walk up the AST from a pattern node to find the enclosing branch container. */
const findPatternBranchScope = (node: SyntaxNode): SyntaxNode | undefined => {
let current = node.parent;
while (current) {
if (PATTERN_BRANCH_TYPES.has(current.type)) return current;
if (FUNCTION_NODE_TYPES.has(current.type)) return undefined;
current = current.parent;
}
return undefined;
};
/** Implementation of the lookup logic — shared between TypeEnvironment and the legacy export. */
const lookupInEnv = (
env: TypeEnv,
varName: string,
callNode: SyntaxNode,
patternOverrides?: PatternOverrides,
): string | undefined => {
// Self/this receiver: resolve to enclosing class name via AST walk
if (varName === 'self' || varName === 'this' || varName === '$this') {
@ -68,6 +99,20 @@ const lookupInEnv = (
// Determine the enclosing function scope for the call
const scopeKey = findEnclosingScopeKey(callNode);
// Check position-indexed pattern overrides first (e.g., Kotlin when/is smart casts).
// These take priority over flat scopeEnv because they represent per-branch narrowing.
if (scopeKey && patternOverrides) {
const varOverrides = patternOverrides.get(scopeKey)?.get(varName);
if (varOverrides) {
const pos = callNode.startIndex;
for (const override of varOverrides) {
if (pos >= override.rangeStart && pos <= override.rangeEnd) {
return stripNullable(override.typeName);
}
}
}
}
// Try function-local scope first
if (scopeKey) {
const scopeEnv = env.get(scopeKey);
@ -289,6 +334,7 @@ export const buildTypeEnv = (
symbolTable?: SymbolTable,
): TypeEnvironment => {
const env: TypeEnv = new Map();
const patternOverrides: PatternOverrides = new Map();
const localClassNames = new Set<string>();
const classNames = createClassNameLookup(localClassNames, symbolTable);
const config = typeConfigs[language];
@ -319,7 +365,9 @@ export const buildTypeEnv = (
if (TYPED_PARAMETER_TYPES.has(node.type)) {
// Capture the raw type annotation BEFORE extractParameter.
// Most languages use 'name' field; Rust uses 'pattern'; TS uses 'pattern' for some param types.
const typeNode = node.childForFieldName('type');
// Kotlin `parameter` nodes use positional children instead of named fields,
// so we fall back to scanning children by type when childForFieldName returns null.
let typeNode = node.childForFieldName('type');
if (typeNode) {
const nameNode = node.childForFieldName('name')
?? node.childForFieldName('pattern');
@ -329,6 +377,27 @@ export const buildTypeEnv = (
declarationTypeNodes.set(`${scope}\0${varName}`, typeNode);
}
}
} else {
// Fallback: positional children (Kotlin `parameter` → simple_identifier + user_type)
let fallbackName: SyntaxNode | null = null;
let fallbackType: SyntaxNode | null = null;
for (let i = 0; i < node.namedChildCount; i++) {
const child = node.namedChild(i);
if (!child) continue;
if (!fallbackName && (child.type === 'simple_identifier' || child.type === 'identifier')) {
fallbackName = child;
}
if (!fallbackType && (child.type === 'user_type' || child.type === 'type_identifier'
|| child.type === 'generic_type' || child.type === 'parameterized_type')) {
fallbackType = child;
}
}
if (fallbackName && fallbackType) {
const varName = extractVarName(fallbackName);
if (varName && !declarationTypeNodes.has(`${scope}\0${varName}`)) {
declarationTypeNodes.set(`${scope}\0${varName}`, fallbackType);
}
}
}
config.extractParameter(node, scopeEnv);
return;
@ -409,9 +478,29 @@ export const buildTypeEnv = (
// Conservative: extractor returns undefined when source type is unknown.
if (config.extractPatternBinding && (!config.patternBindingNodeTypes || config.patternBindingNodeTypes.has(node.type))) {
const patternBinding = config.extractPatternBinding(node, scopeEnv, declarationTypeNodes, scope);
// Allow overwrite for languages with smart-cast narrowing (e.g., Kotlin when)
if (patternBinding && (!scopeEnv.has(patternBinding.varName) || config.allowPatternBindingOverwrite)) {
scopeEnv.set(patternBinding.varName, patternBinding.typeName);
if (patternBinding) {
if (config.allowPatternBindingOverwrite) {
// Position-indexed: store per-branch binding for smart-cast narrowing.
// Each when arm / switch case gets its own type for the variable,
// preventing cross-arm contamination (e.g., Kotlin when/is).
const branchNode = findPatternBranchScope(node);
if (branchNode) {
if (!patternOverrides.has(scope)) patternOverrides.set(scope, new Map());
const varMap = patternOverrides.get(scope)!;
if (!varMap.has(patternBinding.varName)) varMap.set(patternBinding.varName, []);
varMap.get(patternBinding.varName)!.push({
rangeStart: branchNode.startIndex,
rangeEnd: branchNode.endIndex,
typeName: patternBinding.typeName,
});
}
// Also store in flat scopeEnv as fallback (last arm wins — same as before
// for code that doesn't use position-indexed lookup).
scopeEnv.set(patternBinding.varName, patternBinding.typeName);
} else if (!scopeEnv.has(patternBinding.varName)) {
// First-writer-wins for languages without smart-cast overwrite (Java instanceof, etc.)
scopeEnv.set(patternBinding.varName, patternBinding.typeName);
}
}
}
@ -459,7 +548,7 @@ export const buildTypeEnv = (
}
return {
lookup: (varName, callNode) => lookupInEnv(env, varName, callNode),
lookup: (varName, callNode) => lookupInEnv(env, varName, callNode, patternOverrides),
constructorBindings: bindings,
env,
};

View file

@ -287,7 +287,10 @@ const extractKotlinDeclaration: TypeBindingExtractor = (node: SyntaxNode, env: M
}
};
/** Kotlin: formal_parameter → type name */
/** Kotlin: parameter / formal_parameter → type name.
* Kotlin's tree-sitter grammar uses positional children (simple_identifier, user_type)
* rather than named fields (name, type) on `parameter` nodes, so we fall back to
* findChildByType when childForFieldName returns null. */
const extractKotlinParameter: ParameterExtractor = (node: SyntaxNode, env: Map<string, string>): void => {
let nameNode: SyntaxNode | null = null;
let typeNode: SyntaxNode | null = null;
@ -300,6 +303,10 @@ const extractKotlinParameter: ParameterExtractor = (node: SyntaxNode, env: Map<s
typeNode = node.childForFieldName('type');
}
// Fallback: Kotlin `parameter` nodes use positional children, not named fields
if (!nameNode) nameNode = findChildByType(node, 'simple_identifier');
if (!typeNode) typeNode = findChildByType(node, 'user_type');
if (!nameNode || !typeNode) return;
const varName = extractVarName(nameNode);
const typeName = extractSimpleTypeName(typeNode);
@ -444,6 +451,7 @@ const extractKotlinForLoopBinding: ForLoopExtractor = (
// The iterable is the second named child of the for_statement (after variable_declaration)
let iterableName: string | undefined;
let methodName: string | undefined;
let fallbackIterableName: string | undefined;
let foundVarDecl = false;
for (let i = 0; i < node.namedChildCount; i++) {
const child = node.namedChild(i);
@ -458,15 +466,14 @@ const extractKotlinForLoopBinding: ForLoopExtractor = (
const obj = child.firstNamedChild;
const suffix = findChildByType(child, 'navigation_suffix');
const prop = suffix ? findChildByType(suffix, 'simple_identifier') : null;
// If the suffix has a call_suffix, it's a method call on a container (e.g., data.keys()).
// Otherwise it's a bare property access (e.g., self.users) — use the property as iterableName.
const hasCallSuffix = suffix ? findChildByType(suffix, 'call_suffix') !== null : false;
if (hasCallSuffix || !prop) {
if (obj?.type === 'simple_identifier') iterableName = obj.text;
if (prop) methodName = prop.text;
} else {
// Bare property access: self.users, repo.users → use property as iterable name
iterableName = prop.text;
// Always try object as iterable + property as method first (handles data.values, data.keys).
// For bare property access without call_suffix, also save property as fallback
// (handles this.users, repo.items where the property IS the iterable).
if (obj?.type === 'simple_identifier') iterableName = obj.text;
if (prop) methodName = prop.text;
if (!hasCallSuffix && prop) {
fallbackIterableName = prop.text;
}
break;
}
@ -487,7 +494,14 @@ const extractKotlinForLoopBinding: ForLoopExtractor = (
}
if (!iterableName) return;
const containerTypeName = scopeEnv.get(iterableName);
let containerTypeName = scopeEnv.get(iterableName);
// Fallback: if object has no type in scope, try the property as the iterable name.
// Handles patterns like this.users where the property itself is the iterable variable.
if (!containerTypeName && fallbackIterableName) {
iterableName = fallbackIterableName;
methodName = undefined;
containerTypeName = scopeEnv.get(iterableName);
}
const typeArgPos = methodToTypeArgPosition(methodName, containerTypeName);
const elementType = resolveIterableElementType(
iterableName, node, scopeEnv, declarationTypeNodes, scope,

View file

@ -4,8 +4,26 @@ fun processValues(data: HashMap<String, User>) {
}
}
fun processKeys(data: HashMap<User, Repo>) {
for (user in data.keys) {
user.save()
}
}
fun processMutableMapValues(data: MutableMap<String, Repo>) {
for (repo in data.values) {
repo.save()
}
}
fun processList(users: List<User>) {
for (user in users) {
user.save()
}
}
fun processSet(repos: Set<Repo>) {
for (repo in repos) {
repo.save()
}
}

View file

@ -0,0 +1,34 @@
import models.User
import models.Repo
import models.Admin
// Three-arm when: each arm should resolve obj to its narrowed type
fun processThreeArms(obj: Any) {
when (obj) {
is User -> obj.save()
is Repo -> obj.save()
is Admin -> obj.save()
}
}
// Multiple method calls within a single when arm
fun processMultiCall(obj: Any) {
when (obj) {
is User -> {
obj.validate()
obj.save()
}
is Repo -> {
obj.validate()
obj.save()
}
}
}
// when with else branch — else should NOT narrow the type
fun processWithElse(obj: Any) {
when (obj) {
is User -> obj.save()
else -> println(obj)
}
}

View file

@ -0,0 +1,5 @@
package models
class Admin {
fun save() {}
}

View file

@ -0,0 +1,6 @@
package models
class Repo {
fun save() {}
fun validate() {}
}

View file

@ -0,0 +1,6 @@
package models
class User {
fun save() {}
fun validate() {}
}

View file

@ -1009,4 +1009,157 @@ describe('Kotlin HashMap .values for-loop resolution', () => {
);
expect(userSave).toBeDefined();
});
it('resolves user.save() via HashMap.keys to User#save (first type arg)', () => {
const calls = getRelationships(result, 'CALLS');
const userSave = calls.find(c =>
c.target === 'save' && c.source === 'processKeys' && c.targetFilePath?.includes('User'),
);
expect(userSave).toBeDefined();
});
it('does NOT resolve HashMap.keys iteration to Repo#save (negative)', () => {
const calls = getRelationships(result, 'CALLS');
const wrong = calls.find(c =>
c.target === 'save' && c.source === 'processKeys' && c.targetFilePath?.includes('Repo'),
);
expect(wrong).toBeUndefined();
});
it('resolves repo.save() via MutableMap.values to Repo#save', () => {
const calls = getRelationships(result, 'CALLS');
const repoSave = calls.find(c =>
c.target === 'save' && c.source === 'processMutableMapValues' && c.targetFilePath?.includes('Repo'),
);
expect(repoSave).toBeDefined();
});
it('resolves repo.save() via Set iteration to Repo#save', () => {
const calls = getRelationships(result, 'CALLS');
const repoSave = calls.find(c =>
c.target === 'save' && c.source === 'processSet' && c.targetFilePath?.includes('Repo'),
);
expect(repoSave).toBeDefined();
});
});
// ---------------------------------------------------------------------------
// Kotlin when/is complex patterns: 3+ arms, multi-call, else branch
// ---------------------------------------------------------------------------
describe('Kotlin when/is complex pattern binding', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'kotlin-when-complex'),
() => {},
);
}, 60000);
it('detects User, Repo, and Admin classes', () => {
const classes = getNodesByLabel(result, 'Class');
expect(classes).toContain('User');
expect(classes).toContain('Repo');
expect(classes).toContain('Admin');
});
// --- Three-arm when: each arm resolves obj to the correct narrowed type ---
it('resolves obj.save() in 3-arm when/is User to User#save', () => {
const calls = getRelationships(result, 'CALLS');
const userSave = calls.find(c =>
c.target === 'save' && c.source === 'processThreeArms' && c.targetFilePath === 'models/User.kt',
);
expect(userSave).toBeDefined();
});
it('resolves obj.save() in 3-arm when/is Repo to Repo#save', () => {
const calls = getRelationships(result, 'CALLS');
const repoSave = calls.find(c =>
c.target === 'save' && c.source === 'processThreeArms' && c.targetFilePath === 'models/Repo.kt',
);
expect(repoSave).toBeDefined();
});
it('resolves obj.save() in 3-arm when/is Admin to Admin#save', () => {
const calls = getRelationships(result, 'CALLS');
const adminSave = calls.find(c =>
c.target === 'save' && c.source === 'processThreeArms' && c.targetFilePath === 'models/Admin.kt',
);
expect(adminSave).toBeDefined();
});
// --- Multiple method calls within a single when arm ---
it('resolves obj.validate() in when/is User arm to User#validate', () => {
const calls = getRelationships(result, 'CALLS');
const userValidate = calls.find(c =>
c.target === 'validate' && c.source === 'processMultiCall' && c.targetFilePath === 'models/User.kt',
);
expect(userValidate).toBeDefined();
});
it('resolves obj.save() in when/is User arm to User#save (multi-call)', () => {
const calls = getRelationships(result, 'CALLS');
const userSave = calls.find(c =>
c.target === 'save' && c.source === 'processMultiCall' && c.targetFilePath === 'models/User.kt',
);
expect(userSave).toBeDefined();
});
it('resolves obj.validate() in when/is Repo arm to Repo#validate', () => {
const calls = getRelationships(result, 'CALLS');
const repoValidate = calls.find(c =>
c.target === 'validate' && c.source === 'processMultiCall' && c.targetFilePath === 'models/Repo.kt',
);
expect(repoValidate).toBeDefined();
});
it('resolves obj.save() in when/is Repo arm to Repo#save (multi-call)', () => {
const calls = getRelationships(result, 'CALLS');
const repoSave = calls.find(c =>
c.target === 'save' && c.source === 'processMultiCall' && c.targetFilePath === 'models/Repo.kt',
);
expect(repoSave).toBeDefined();
});
// --- Cross-resolution negatives: User arm does NOT resolve to Repo ---
it('does NOT resolve processMultiCall when/is User arm validate() to Repo', () => {
const calls = getRelationships(result, 'CALLS');
const wrong = calls.find(c =>
c.target === 'validate' && c.source === 'processMultiCall' && c.targetFilePath === 'models/Repo.kt',
);
// Both User and Repo have validate(), so the Repo arm DOES resolve here.
// But processMultiCall should NOT have a cross-arm leak.
// We test that the User arm doesn't produce a Repo edge by checking save count.
const userSaves = calls.filter(c =>
c.target === 'save' && c.source === 'processMultiCall',
);
// Exactly 2 save() CALLS edges (one per arm, not duplicated)
expect(userSaves.length).toBe(2);
});
// --- when with else: is User arm narrows, else does not ---
it('resolves obj.save() in when/is User + else to User#save', () => {
const calls = getRelationships(result, 'CALLS');
const userSave = calls.find(c =>
c.target === 'save' && c.source === 'processWithElse' && c.targetFilePath === 'models/User.kt',
);
expect(userSave).toBeDefined();
});
it('does NOT resolve processWithElse to Repo#save or Admin#save', () => {
const calls = getRelationships(result, 'CALLS');
const wrongRepo = calls.find(c =>
c.target === 'save' && c.source === 'processWithElse' && c.targetFilePath === 'models/Repo.kt',
);
const wrongAdmin = calls.find(c =>
c.target === 'save' && c.source === 'processWithElse' && c.targetFilePath === 'models/Admin.kt',
);
expect(wrongRepo).toBeUndefined();
expect(wrongAdmin).toBeUndefined();
});
});

View file

@ -3240,6 +3240,32 @@ fun process() {
});
});
describe('Kotlin for-loop HashMap.values resolution (Phase 6)', () => {
it('for (user in data.values) binds user to User via HashMap<String, User>', () => {
const tree = parse(`
fun processValues(data: HashMap<String, User>) {
for (user in data.values) {
user.save()
}
}
`, Kotlin);
const { env } = buildTypeEnv(tree, 'kotlin');
expect(flatGet(env, 'user')).toBe('User');
});
it('for (user in users) binds user to User via List<User> param', () => {
const tree = parse(`
fun processList(users: List<User>) {
for (user in users) {
user.save()
}
}
`, Kotlin);
const { env } = buildTypeEnv(tree, 'kotlin');
expect(flatGet(env, 'user')).toBe('User');
});
});
describe('Java switch pattern variable (Phase 6)', () => {
it('switch (obj) { case User u -> } binds u to User', () => {
const tree = parse(`