mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-08 03:08:13 +00:00
fix(scope-resolution): keep unique-name property inference inside one language
The pass indexed `Property` nodes from the whole shared graph. Per-language gating decides whether it RUNS for a language; it never restricted which nodes could be TARGETS. So the only carrier of a name could be in another language entirely, and a read here resolved to it on name uniqueness alone — no owner, no file, no call path. Reproduced: a Java class declaring `private int loyaltyPointsBalance` and a JS `cfg.loyaltyPointsBalance` on an untyped parameter produced an ACCESSES edge from the JS function to the Java private field. Confidence does not mitigate it, because `minConfidence` defaults to 0 — the tier is only a filter for consumers who ask for one. Candidates are now restricted to files in the language's own `parsedFiles`, which is a precise restriction rather than a heuristic and needs no new node property. Every other fixture in the suite is single-language, so this could not be caught anywhere by construction. The new fixture is deliberately polyglot and asserts both halves: no cross-language edge, and a same-language unique name still resolves. Known and not addressed here: the index is still O(total graph nodes) and is rebuilt once per qualifying language, the per-language whole-graph-scan pattern `phase.ts` hoisted out for `sharedNodeLookup`. Hoisting it belongs with that machinery rather than in this fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6df6fb8501
commit
3c5eadc705
4 changed files with 85 additions and 5 deletions
|
|
@ -140,6 +140,7 @@ export interface UniqueNamePropertyStats {
|
|||
*/
|
||||
function indexPropertyNodesByName(
|
||||
graph: KnowledgeGraph,
|
||||
ownFilePaths: ReadonlySet<string>,
|
||||
): ReadonlyMap<string, readonly PropertyCandidate[] | null> {
|
||||
const byName = new Map<string, PropertyCandidate[] | null>();
|
||||
for (const node of graph.iterNodes()) {
|
||||
|
|
@ -147,12 +148,21 @@ function indexPropertyNodesByName(
|
|||
const name = node.properties.name;
|
||||
if (typeof name !== 'string' || name.length === 0) continue;
|
||||
const filePath = node.properties.filePath;
|
||||
// SAME LANGUAGE ONLY. The graph is shared across every language in the
|
||||
// repo, and `fieldFallbackOnMethodLookup` only decides whether this pass
|
||||
// RUNS for a language — it never restricted which nodes could be TARGETS.
|
||||
// So a Java backend declaring `private int loyaltyPoints` was the unique
|
||||
// carrier of that name, and a JS frontend writing `cfg.loyaltyPoints` on an
|
||||
// untyped parameter got an edge to it: no owner, file, or language evidence,
|
||||
// and inference across a language boundary that has no call path at all.
|
||||
// The confidence tier does not save it, since `minConfidence` defaults to 0.
|
||||
//
|
||||
// `parsedFiles` is exactly this language's file set, so matching on it is a
|
||||
// precise restriction rather than a heuristic — no node property needed.
|
||||
if (typeof filePath !== 'string' || !ownFilePaths.has(filePath)) continue;
|
||||
const existing = byName.get(name);
|
||||
if (existing === OVERSATURATED) continue;
|
||||
const candidate: PropertyCandidate = {
|
||||
id: node.id,
|
||||
filePath: typeof filePath === 'string' ? filePath : '',
|
||||
};
|
||||
const candidate: PropertyCandidate = { id: node.id, filePath };
|
||||
if (existing === undefined) {
|
||||
byName.set(name, [candidate]);
|
||||
continue;
|
||||
|
|
@ -245,7 +255,7 @@ export function emitUniqueNamePropertyAccesses(
|
|||
/** Finalized import graph; narrows a name carried by several definitions. */
|
||||
finalized?: FinalizedImportView,
|
||||
): UniqueNamePropertyStats {
|
||||
const byName = indexPropertyNodesByName(graph);
|
||||
const byName = indexPropertyNodesByName(graph, new Set(parsedFiles.map((p) => p.filePath)));
|
||||
if (byName.size === 0) {
|
||||
return { emitted: 0, ambiguous: 0, narrowed: 0, ambiguousNames: [] };
|
||||
}
|
||||
|
|
|
|||
11
gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/Loyalty.java
vendored
Normal file
11
gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/Loyalty.java
vendored
Normal file
|
|
@ -0,0 +1,11 @@
|
|||
// RV-5: the ONLY declaration of `loyaltyPointsBalance` in the workspace, and it
|
||||
// is Java. Nothing in the JS file below can call into it.
|
||||
package shop;
|
||||
|
||||
public class Loyalty {
|
||||
private int loyaltyPointsBalance;
|
||||
|
||||
public int read() {
|
||||
return loyaltyPointsBalance;
|
||||
}
|
||||
}
|
||||
16
gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/settings.js
vendored
Normal file
16
gitnexus/test/fixtures/lang-resolution/polyglot-property-isolation/settings.js
vendored
Normal file
|
|
@ -0,0 +1,16 @@
|
|||
// A JS read of the same name through an untyped receiver. Workspace-wide the
|
||||
// name is unique, so unique-name inference resolved it — to a Java private
|
||||
// field, across a language boundary with no call path.
|
||||
export function renderLoyalty(cfg) {
|
||||
return cfg.loyaltyPointsBalance;
|
||||
}
|
||||
|
||||
// CONTROL: a same-language target the pass SHOULD still reach, so the fix is
|
||||
// shown to restrict by language rather than to disable the pass.
|
||||
export const jsConfig = {
|
||||
jsOnlyThreshold: 10,
|
||||
};
|
||||
|
||||
export function readsJsOnly(bag) {
|
||||
return bag.jsOnlyThreshold;
|
||||
}
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
/**
|
||||
* RV-5 — unique-name property inference must not cross a language boundary.
|
||||
*
|
||||
* The pass indexed `Property` nodes from the whole shared graph. Per-language
|
||||
* gating (`fieldFallbackOnMethodLookup`) decides whether the pass RUNS for a
|
||||
* language; it never restricted which nodes could be TARGETS. So the only
|
||||
* carrier of a name might be in another language entirely, and a read here
|
||||
* resolved to it on name uniqueness alone — no owner, no file, no call path.
|
||||
*
|
||||
* Confidence does not mitigate it: `minConfidence` defaults to 0, so a consumer
|
||||
* gets the edge unless it opts out explicitly.
|
||||
*
|
||||
* Every other fixture is single-language, so this could not be caught by
|
||||
* construction anywhere in the suite.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll } from 'vitest';
|
||||
import path from 'path';
|
||||
import { FIXTURES, getRelationships, runPipelineFromRepo, type PipelineResult } from './helpers.js';
|
||||
|
||||
describe('cross-language property inference (RV-5)', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
result = await runPipelineFromRepo(
|
||||
path.join(FIXTURES, 'polyglot-property-isolation'),
|
||||
() => {},
|
||||
);
|
||||
}, 60000);
|
||||
|
||||
const readersOf = (field: string): string[] =>
|
||||
getRelationships(result, 'ACCESSES')
|
||||
.filter((e) => e.target === field)
|
||||
.map((e) => e.source);
|
||||
|
||||
it('does not link a JS read to a Java field of the same name', () => {
|
||||
expect(readersOf('loyaltyPointsBalance')).not.toContain('renderLoyalty');
|
||||
});
|
||||
|
||||
// The other half: restricting by language must not disable the pass.
|
||||
it('still resolves a same-language unique name', () => {
|
||||
expect(readersOf('jsOnlyThreshold')).toContain('readsJsOnly');
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue