fix(r): attach R members only to classes defined in R

attachDeferredROwners resolved the owner hint through the class registry,
which every language shares. A Python class Circle next to an R Circle made
the name ambiguous, so the R methods and slots lost their HAS_METHOD and
HAS_PROPERTY edges; and with no R Circle, an R setMethod attached to the
Python class.

Keep only candidates whose graph node is an R class before the uniqueness
test. The change is inside R's postParse hook, so shared code is untouched.
This commit is contained in:
Mick Cooney 2026-10-16 15:49:23 +01:00
parent 0698fa1420
commit 0476330429
5 changed files with 106 additions and 1 deletions

View file

@ -63,7 +63,11 @@ export const attachDeferredROwners = (
typeof node.properties.ownerNameHint === 'string' ? node.properties.ownerNameHint : null;
if (!ownerNameHint) return;
const classDefs = model.types.lookupClassByName(ownerNameHint);
// The class registry is shared by every language, so a Python `Circle` next to an R
// `Circle` must not make the name ambiguous, nor stand in for an R class that is absent.
const classDefs = model.types
.lookupClassByName(ownerNameHint)
.filter((def) => graph.getNode(def.nodeId)?.properties.language === SupportedLanguages.R);
if (classDefs.length !== 1) return; // ambiguous or not found — leave hint for debugging
const ownerId = classDefs[0].nodeId;

View file

@ -0,0 +1,7 @@
# `Circle` is also a Python class. Its R members still belong to the R class.
setClass("Circle", slots = list(radius = "numeric"))
setMethod("area", "Circle", function(shape) 1)
# `Square` is defined only in Python here (the R class would come from another
# package), so this method has no R owner and must not attach to the Python class.
setMethod("perimeter", "Square", function(shape) 4)

View file

@ -0,0 +1,6 @@
class Circle:
pass
class Square:
pass

View file

@ -0,0 +1,51 @@
/**
* R: a member names its class by string (`setMethod("area", "Circle", ...)`) or by the
* call that defines it, and the owner is found through the class registry, which every
* language shares. A Python `class Circle` must neither hide the R `Circle` (making the
* name ambiguous) nor adopt an R member when no R class of that name exists.
*/
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
import path from 'path';
import { FIXTURES, getRelationships, runPipelineFromRepo } from './helpers.js';
import type { PipelineResult } from './helpers.js';
// Cold worker-pool startups otherwise flake against the 5s default ready budget
// on a loaded runner, failing for reasons unrelated to R parsing or resolution.
// Safe under `pool: 'forks'`: the env stub is process-global, but each file gets its own process.
beforeAll(() => vi.stubEnv('GITNEXUS_WORKER_READY_TIMEOUT_MS', '60000'));
afterAll(() => vi.unstubAllEnvs());
describe('R owner attachment next to same-named classes of another language', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(path.join(FIXTURES, 'r-mixed-language-owners'), () => {});
}, 60000);
const edges = (type: 'HAS_METHOD' | 'HAS_PROPERTY'): string[] =>
getRelationships(result, type)
.map((e) => `${e.sourceFilePath}:${e.source}->${e.targetFilePath}:${e.target}`)
.sort();
it('indexes both Circle classes (positive control)', () => {
const circles: string[] = [];
result.graph.forEachNode((n) => {
if (n.label === 'Class' && n.properties.name === 'Circle') {
circles.push(n.properties.filePath as string);
}
});
expect(circles.sort()).toEqual(['R/shapes.R', 'py/shapes.py']);
});
it('attaches the R method and slot to the R Circle despite the Python Circle', () => {
expect(edges('HAS_METHOD')).toEqual(['R/shapes.R:Circle->R/shapes.R:area']);
expect(edges('HAS_PROPERTY')).toEqual(['R/shapes.R:Circle->R/shapes.R:radius']);
});
it('does not attach an R method to a Python class when no R class has the name', () => {
expect(edges('HAS_METHOD').filter((e) => e.includes('perimeter'))).toEqual([]);
const perimeter = result.graph.getNode('Method:R/shapes.R:Square.perimeter');
expect(perimeter).toBeDefined();
expect(perimeter?.properties.ownerId).toBeUndefined();
});
});

View file

@ -166,6 +166,43 @@ describe('attachDeferredROwners', () => {
expect(edges('HAS_METHOD')).toHaveLength(0);
});
it('(f) ignores a same-named class of another language when picking the owner', () => {
const { graph, model, addClass, addMember, edges } = setup();
const ownerId = addClass('Foo');
const pyId = 'Class:src/a.py:Foo';
graph.addNode({
id: pyId,
label: 'Class',
properties: { name: 'Foo', filePath: 'src/a.py', language: SupportedLanguages.Python },
});
model.symbols.add('src/a.py', 'Foo', pyId, 'Class');
const nodeId = addMember('Method', 'run', { ownerNameHint: 'Foo' });
attachDeferredROwners(graph, model, 'Method', 'HAS_METHOD');
expect(graph.getNode(nodeId)!.properties.ownerId).toBe(ownerId);
expect(edges('HAS_METHOD').map((r) => r.sourceId)).toEqual([ownerId]);
});
it('(g) leaves the hint when the only class of that name is in another language', () => {
const { graph, model, addMember, edges } = setup();
const pyId = 'Class:src/a.py:Foo';
graph.addNode({
id: pyId,
label: 'Class',
properties: { name: 'Foo', filePath: 'src/a.py', language: SupportedLanguages.Python },
});
model.symbols.add('src/a.py', 'Foo', pyId, 'Class');
const nodeId = addMember('Method', 'run', { ownerNameHint: 'Foo' });
attachDeferredROwners(graph, model, 'Method', 'HAS_METHOD');
const node = graph.getNode(nodeId)!;
expect(node.properties.ownerNameHint).toBe('Foo');
expect(node.properties).not.toHaveProperty('ownerId');
expect(edges('HAS_METHOD')).toHaveLength(0);
});
it('(e) an already-owned node drops its hint and is never re-attached or double-registered', () => {
const { graph, model, addClass, addMember, edges } = setup();
const ownerId = addClass('Foo');