mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-07 02:58:02 +00:00
fix(ingestion): resolve same-tail Ruby mixin/attr_accessor owners to the correct qualified node (#1982)
emitRubyMixinEdges keyed its owner map by the SIMPLE tail (def.qualifiedName split-popped) with last-wins, and the __heritage__/__property__ markers carried only the immediate owner name — so `module Outer; class Inner` and `module Other; class Inner` collapsed onto one `Inner` key and cross-wired their include/attr_accessor edges onto whichever Inner was processed last. Fix (lockstep, full-qualified): - ruby/captures.ts: build the marker owner from the FULL enclosing class/module chain (buildEnclosingQualifiedName walks all ancestors, normalizing the compact `class Outer::Inner` scope_resolution form via the shared splitQualifiedName) so the marker owner byte-matches the resolution def's qualifiedName. - ruby/scope-resolver.ts: key graphIdByName by the full def.qualifiedName instead of the simple tail. Top-level owners/mixins are unchanged (full == simple). Registry-primary ruby.test.ts 142/142 incl. a new worker-path block (the deferred note's duplicate-edge concern: markers survive worker serialization, exactly one HAS_PROPERTY per attr). Legacy leg unaffected (136 pass / 6 skip) — new assertions registry-primary-only via helpers.ts. tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
16883f2d79
commit
bb84ccb27e
5 changed files with 111 additions and 6 deletions
|
|
@ -12,6 +12,7 @@ import { recordRubyCacheHit, recordRubyCacheMiss } from './cache-stats.js';
|
|||
import { synthesizeRubyReceiverBinding, findEnclosingClassOrModule } from './receiver-binding.js';
|
||||
import { getTreeSitterBufferSize } from '../../constants.js';
|
||||
import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js';
|
||||
import { splitQualifiedName } from '../../utils/qualified-name.js';
|
||||
|
||||
const FUNCTION_NODE_TYPES = ['method', 'singleton_method'] as const;
|
||||
const HERITAGE_CALL_NAMES: ReadonlySet<string> = new Set(['include', 'extend', 'prepend']);
|
||||
|
|
@ -21,6 +22,30 @@ const ATTR_CALL_NAMES: ReadonlySet<string> = new Set([
|
|||
'attr_writer',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Build the full `.`-joined qualified owner name for a heritage/attr call by
|
||||
* walking ALL enclosing class/module ancestors (not just the immediate one),
|
||||
* so a same-tail nested owner (`module Outer; class Inner`) is keyed by its
|
||||
* full path `Outer.Inner` instead of the bare tail `Inner` — which otherwise
|
||||
* collapses both same-tail owners onto one `__heritage__`/`__property__` marker
|
||||
* key (last-wins) and cross-wires their mixin / attr_accessor edges (#1982).
|
||||
* Handles the compact `class Outer::Inner` form (name is a `scope_resolution`)
|
||||
* via the shared normalizer, so the marker owner byte-matches the resolution
|
||||
* def's `qualifiedName`. Returns undefined when there is no enclosing class/module.
|
||||
*/
|
||||
function buildEnclosingQualifiedName(callNode: SyntaxNode): string | undefined {
|
||||
const segments: string[] = [];
|
||||
let current: SyntaxNode | null = callNode.parent;
|
||||
while (current !== null) {
|
||||
if (current.type === 'class' || current.type === 'module') {
|
||||
const nameNode = current.childForFieldName('name');
|
||||
if (nameNode !== null) segments.unshift(...splitQualifiedName(nameNode.text));
|
||||
}
|
||||
current = current.parent;
|
||||
}
|
||||
return segments.length > 0 ? segments.join('.') : undefined;
|
||||
}
|
||||
|
||||
export function emitRubyScopeCaptures(
|
||||
sourceText: string,
|
||||
_filePath: string,
|
||||
|
|
@ -144,8 +169,7 @@ export function emitRubyScopeCaptures(
|
|||
if (HERITAGE_CALL_NAMES.has(callName)) {
|
||||
const callNode = nodeIfType(nodeMap['@reference.call.free'], 'call');
|
||||
if (callNode !== null) {
|
||||
const enclosing = findEnclosingClassOrModule(callNode);
|
||||
const ownerName = enclosing?.childForFieldName('name')?.text;
|
||||
const ownerName = buildEnclosingQualifiedName(callNode);
|
||||
if (ownerName) {
|
||||
const argList = callNode.childForFieldName('arguments');
|
||||
if (argList !== null) {
|
||||
|
|
@ -178,8 +202,7 @@ export function emitRubyScopeCaptures(
|
|||
if (ATTR_CALL_NAMES.has(callName)) {
|
||||
const callNode = nodeIfType(nodeMap['@reference.call.free'], 'call');
|
||||
if (callNode !== null) {
|
||||
const enclosing = findEnclosingClassOrModule(callNode);
|
||||
const ownerName = enclosing?.childForFieldName('name')?.text;
|
||||
const ownerName = buildEnclosingQualifiedName(callNode);
|
||||
if (ownerName) {
|
||||
const argList = callNode.childForFieldName('arguments');
|
||||
if (argList !== null) {
|
||||
|
|
|
|||
|
|
@ -23,8 +23,13 @@ function emitRubyMixinEdges(
|
|||
if (!isClassLike(def.type)) continue;
|
||||
const graphId = resolveDefGraphId(parsed.filePath, def, nodeLookup);
|
||||
if (graphId !== undefined) {
|
||||
const simpleName = def.qualifiedName?.split('.').pop() ?? def.qualifiedName ?? '';
|
||||
graphIdByName.set(simpleName, graphId);
|
||||
// Key by the FULL qualified name (`Outer.Inner`), NOT the simple tail.
|
||||
// Same-tail nested classes (`Outer::Inner` + `Other::Inner`) otherwise
|
||||
// collapse onto one `Inner` key (last-wins) and cross-wire their mixin /
|
||||
// attr_accessor owners (#1982). The `__heritage__`/`__property__` markers
|
||||
// carry the full qualified owner name in lockstep (see ruby/captures.ts).
|
||||
const fullName = def.qualifiedName ?? '';
|
||||
if (fullName.length > 0) graphIdByName.set(fullName, graphId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,10 +1,16 @@
|
|||
module OuterMix; end
|
||||
module OtherMix; end
|
||||
module Outer
|
||||
class Inner
|
||||
include OuterMix
|
||||
attr_accessor :outer_attr
|
||||
def from_outer; end
|
||||
end
|
||||
end
|
||||
module Other
|
||||
class Inner
|
||||
include OtherMix
|
||||
attr_accessor :other_attr
|
||||
def from_other; end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -264,6 +264,14 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly<Record<string, Readonly
|
|||
// to the new node-identity behavior.
|
||||
'owns from_outer / from_other through distinct Outer.Inner / Other.Inner nodes (R7)',
|
||||
'owns radius (attr_accessor) under the qualified Shapes.Circle node, no dangling (R7)',
|
||||
// #1982 RESOLUTION-side same-tail owner identity. The registry-primary
|
||||
// emitRubyMixinEdges bridge keys its owner map by full qualifiedName and the
|
||||
// captures emit the full enclosing-scope owner; the legacy DAG does not use
|
||||
// that bridge, so these are registry-primary-only by design.
|
||||
'owns outer_attr / other_attr under their OWN qualified Inner node (same-tail attr_accessor, R7)',
|
||||
'routes include OuterMix / OtherMix to their OWN qualified Inner owner (same-tail mixin, R7)',
|
||||
'genuinely used the worker pool for the same-tail Ruby fixture',
|
||||
'owns outer_attr / other_attr under their OWN qualified Inner node on the worker path (no duplicate, R7)',
|
||||
]),
|
||||
swift: new Set<string>([
|
||||
// Swift scope-resolution achieves 77/77 baseline parity. The tests
|
||||
|
|
|
|||
|
|
@ -1563,4 +1563,67 @@ describe('Ruby inline module-nested same-tail collision — distinct nodes (issu
|
|||
expect(result.graph.getNode(e!.rel.sourceId)?.properties.qualifiedName).toBe('Shapes.Circle');
|
||||
},
|
||||
);
|
||||
|
||||
// #1982 resolution-side: SAME-TAIL routed-property owner identity. The
|
||||
// pre-fix emitRubyMixinEdges keys its owner map by simple tail (last-wins),
|
||||
// so outer_attr / other_attr both attach to whichever `Inner` was processed
|
||||
// last. Asserts each routes to its OWN qualified node by qualifiedName, with
|
||||
// exactly one (non-duplicated) edge. Registry-primary only.
|
||||
pit('owns outer_attr / other_attr under their OWN qualified Inner node (same-tail attr_accessor, R7)', () => {
|
||||
const hp = getRelationships(result, 'HAS_PROPERTY');
|
||||
const ownerQnOf = (prop: string) => {
|
||||
const e = hp.find((x) => x.target === prop);
|
||||
expect(e, `HAS_PROPERTY -> ${prop}`).toBeDefined();
|
||||
return result.graph.getNode(e!.rel.sourceId)?.properties.qualifiedName;
|
||||
};
|
||||
expect(ownerQnOf('outer_attr')).toBe('Outer.Inner');
|
||||
expect(ownerQnOf('other_attr')).toBe('Other.Inner');
|
||||
expect(hp.filter((x) => x.target === 'outer_attr')).toHaveLength(1);
|
||||
expect(hp.filter((x) => x.target === 'other_attr')).toHaveLength(1);
|
||||
});
|
||||
|
||||
// #1982 resolution-side: SAME-TAIL mixin owner identity (IMPLEMENTS).
|
||||
pit('routes include OuterMix / OtherMix to their OWN qualified Inner owner (same-tail mixin, R7)', () => {
|
||||
const impl = getRelationships(result, 'IMPLEMENTS');
|
||||
const ownerQnOfMixin = (mixinName: string) => {
|
||||
const e = impl.find((x) => x.target === mixinName);
|
||||
expect(e, `IMPLEMENTS -> ${mixinName}`).toBeDefined();
|
||||
return result.graph.getNode(e!.rel.sourceId)?.properties.qualifiedName;
|
||||
};
|
||||
expect(ownerQnOfMixin('OuterMix')).toBe('Outer.Inner');
|
||||
expect(ownerQnOfMixin('OtherMix')).toBe('Other.Inner');
|
||||
});
|
||||
});
|
||||
|
||||
// Same fixture through the WORKER pool. The deferred note flagged that the worker
|
||||
// path could emit a DUPLICATE cross-wired same-tail owner edge (the worker emits
|
||||
// the __property__/__heritage__ markers, which must now carry the full qualified
|
||||
// owner). Asserts worker == sequential: each attr owns its OWN qualified node with
|
||||
// exactly one edge (#1982 R7). Registry-primary only.
|
||||
describe('Ruby inline module-nested same-tail collision — worker path parity (issue #1982)', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
result = await runPipelineFromRepo(path.join(FIXTURES, 'ruby-nested-tail-collision'), () => {}, {
|
||||
workerThresholdsForTest: { minFiles: 1, minBytes: 1 },
|
||||
workerPoolSize: 2,
|
||||
});
|
||||
}, 120000);
|
||||
|
||||
pit('genuinely used the worker pool for the same-tail Ruby fixture', () => {
|
||||
expect(result.usedWorkerPool).toBe(true);
|
||||
});
|
||||
|
||||
pit('owns outer_attr / other_attr under their OWN qualified Inner node on the worker path (no duplicate, R7)', () => {
|
||||
const hp = getRelationships(result, 'HAS_PROPERTY');
|
||||
const ownerQnOf = (prop: string) => {
|
||||
const e = hp.find((x) => x.target === prop);
|
||||
expect(e, `HAS_PROPERTY -> ${prop}`).toBeDefined();
|
||||
return result.graph.getNode(e!.rel.sourceId)?.properties.qualifiedName;
|
||||
};
|
||||
expect(ownerQnOf('outer_attr')).toBe('Outer.Inner');
|
||||
expect(ownerQnOf('other_attr')).toBe('Other.Inner');
|
||||
expect(hp.filter((x) => x.target === 'outer_attr')).toHaveLength(1);
|
||||
expect(hp.filter((x) => x.target === 'other_attr')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue