fix: detect ambiguity in findInheritedMethod BFS

When multiple EXTENDS ancestors provide the same method matching an
interface contract, findInheritedMethod now collects all matches across
the full BFS and returns null (ambiguous) when 2+ distinct methodIds
are found. Diamond paths are deduplicated by methodId so the same
method reachable via two paths is not treated as ambiguous.

2 new tests: multi-parent ambiguity (returns null), diamond dedup
(returns the single shared method).
This commit is contained in:
Gergo Magyar 2026-04-03 19:58:34 +01:00
parent 4205261c92
commit 7149001877
2 changed files with 59 additions and 2 deletions

View file

@ -632,6 +632,11 @@ function findInheritedMethod(
}
}
// Collect all matches across the full BFS, then check for ambiguity.
// If the same methodId is reachable via multiple paths (diamond), it's not
// ambiguous. Only distinct methodIds count as separate matches.
const matches = new Map<string, { methodId: string; parameterTypes: string[] }>();
while (queue.length > 0) {
const ancestorId = queue.shift()!;
if (visited.has(ancestorId)) continue;
@ -647,7 +652,7 @@ function findInheritedMethod(
const mParamTypes = (mNode.properties.parameterTypes as string[] | undefined) ?? [];
const mParamCount = mNode.properties.parameterCount as number | undefined;
if (parameterTypesMatch(mParamTypes, targetParamTypes, mParamCount, targetParamCount)) {
return { methodId: mid, parameterTypes: mParamTypes };
matches.set(mid, { methodId: mid, parameterTypes: mParamTypes });
}
}
@ -666,7 +671,9 @@ function findInheritedMethod(
}
}
return null;
// Ambiguous: 2+ distinct methods from different ancestors
if (matches.size === 1) return matches.values().next().value!;
return null; // 0 matches or ambiguous (2+)
}
/**

View file

@ -1090,4 +1090,54 @@ describe('computeMRO', () => {
});
});
});
// ---- findInheritedMethod ambiguity detection ------------------------------
describe('findInheritedMethod ambiguity', () => {
it('returns null when two EXTENDS parents both provide matching method', () => {
// I { foo() }, B { foo() }, M { foo() }, C extends B + M, C implements I
const graph = createKnowledgeGraph();
addClass(graph, 'I', 'cpp', 'Interface');
addClass(graph, 'B', 'cpp');
addClass(graph, 'M', 'cpp');
addClass(graph, 'C', 'cpp');
addImplements(graph, 'C', 'I');
addExtends(graph, 'C', 'B');
addExtends(graph, 'C', 'M');
addMethod(graph, 'I', 'foo', 'Interface');
addMethod(graph, 'B', 'foo');
addMethod(graph, 'M', 'foo');
// C has NO own foo — must walk EXTENDS chain
const result = computeMRO(graph);
// Ambiguous: B.foo and M.foo both match — no METHOD_IMPLEMENTS edge
const mi = graph.relationships.filter((r) => r.type === 'METHOD_IMPLEMENTS');
const fooEdges = mi.filter((e) => graph.getNode(e.targetId)?.properties.name === 'foo');
expect(fooEdges).toHaveLength(0);
});
it('diamond dedup: same method via two paths is NOT ambiguous', () => {
// I { foo() }, GrandBase { foo() }, B extends GrandBase, M extends GrandBase
// C extends B + M, C implements I
const graph = createKnowledgeGraph();
addClass(graph, 'I', 'cpp', 'Interface');
addClass(graph, 'GrandBase', 'cpp');
addClass(graph, 'B', 'cpp');
addClass(graph, 'M', 'cpp');
addClass(graph, 'C', 'cpp');
addImplements(graph, 'C', 'I');
addExtends(graph, 'C', 'B');
addExtends(graph, 'C', 'M');
addExtends(graph, 'B', 'GrandBase');
addExtends(graph, 'M', 'GrandBase');
addMethod(graph, 'I', 'foo', 'Interface');
const gbFoo = addMethod(graph, 'GrandBase', 'foo');
// B and M have NO own foo — both inherit from GrandBase
const result = computeMRO(graph);
// Not ambiguous: same GrandBase.foo via both paths
const mi = graph.relationships.filter((r) => r.type === 'METHOD_IMPLEMENTS');
const fooEdge = mi.find((e) => e.sourceId === gbFoo);
expect(fooEdge).toBeDefined();
});
});
});