fix(r): own a setMethod() written with signature() or c() by its first class

setMethod("show", signature("A"), ...) and c("A", "B") signatures named no
owner, so the methods of one generic on different classes collapsed into one
node and none had a HAS_METHOD edge.

The owner is the string in the first argument of a signature() or c() value,
named or not. A multi-class signature therefore belongs to its first class; a
first argument that is not a string still names no owner.
This commit is contained in:
Mick Cooney 2026-10-17 17:20:40 +01:00
parent b107810fcc
commit 13cdf4347a
5 changed files with 103 additions and 7 deletions

View file

@ -56,7 +56,28 @@ export function getRTopLevelMethodOwnerName(node: SyntaxNode): string | null {
// The owner is the `signature` argument, as R matches it (named first, then by position).
const signature = getRMatchedArgument(args, SET_METHOD_FORMALS, 'signature');
return extractQuotedString(signature?.childForFieldName('value') ?? null);
const value = signature?.childForFieldName('value') ?? null;
return extractQuotedString(value) ?? extractFirstSignatureClass(value);
}
/**
* The class a `signature("A", "B")`, `signature(object = "A")` or `c("A", "B")` value dispatches
* on first: the string of its first argument, named or not. A method on several classes (multiple
* dispatch) belongs to the first one, so methods of one generic on different first classes keep
* distinct ids and an owner; two methods that differ only in a later class share an id and the
* first written wins. A first argument that is not a string (a variable, a call) names no owner.
*/
function extractFirstSignatureClass(value: SyntaxNode | null): string | null {
if (!value || value.type !== 'call') return null;
const fn = value.childForFieldName('function');
const callee = fn?.type === 'namespace_operator' ? fn.childForFieldName('rhs') : fn;
if (callee?.type !== 'identifier' || (callee.text !== 'signature' && callee.text !== 'c')) {
return null;
}
const first = value
.childForFieldName('arguments')
?.namedChildren.find((c) => c.type === 'argument');
return extractQuotedString(first?.childForFieldName('value') ?? null);
}
// ---------------------------------------------------------------------------

View file

@ -0,0 +1,10 @@
setClass("A", slots = c(x = "numeric"))
setClass("B", slots = c(y = "numeric"))
setClass("P", slots = c(p = "numeric"))
setMethod("show", signature("A"), function(object) cat("A"))
setMethod("show", signature(object = "B"), function(object) cat("B"))
setMethod("show", c("P", "A"), function(object) cat("P"))
setMethod("combine", signature("A", "A"), function(x, y) 1)
setMethod("combine", signature("B", "A"), function(x, y) 2)

View file

@ -75,9 +75,8 @@ describe('R S4/RefClass definitions are named by their first argument only', ()
expect(methods.filter((n) => n === 'describe')).toHaveLength(1);
expect(methods).not.toContain('Derived');
const hasMethod = getRelationships(result, 'HAS_METHOD').filter((e) => e.source === 'Derived');
// `signature("Derived")` (a call, not a string) names no owner today; that is
// unchanged by the anchor, so only the string-signature `show` is owned.
expect(hasMethod.map((e) => e.target)).toEqual(['show']);
// `signature("Derived")` names its first class, as the string signature does.
expect(hasMethod.map((e) => e.target).sort()).toEqual(['describe', 'show']);
});
it('keeps slot, field and representation properties', () => {

View file

@ -0,0 +1,47 @@
/**
* R: `setMethod("show", signature("A"), ...)` and `c("A", "B")` signatures named no owner, so
* every method of one generic collapsed into a single node. The first class of the signature is
* the owner, which keeps the ids of methods on different first classes apart.
*/
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';
// 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());
const FILE = 'R/shapes.R';
describe('R setMethod() with a signature() or c() signature', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(path.join(FIXTURES, 'r-s4-signature-owners'), () => {});
}, 60000);
const methodIds = (name: string): string[] => {
const ids: string[] = [];
result.graph.forEachNode((n) => {
if (n.label === 'Method' && n.properties.name === name) ids.push(n.id);
});
return ids.sort();
};
it('mints one node per first class of the signature', () => {
expect(methodIds('show')).toEqual([
`Method:${FILE}:A.show`,
`Method:${FILE}:B.show`,
`Method:${FILE}:P.show`,
]);
expect(methodIds('combine')).toEqual([`Method:${FILE}:A.combine`, `Method:${FILE}:B.combine`]);
});
it('attaches each method to its first class', () => {
const owned = getRelationships(result, 'HAS_METHOD')
.map((e) => `${e.source}->${e.target}`)
.sort();
expect(owned).toEqual(['A->combine', 'A->show', 'B->combine', 'B->show', 'P->show']);
});
});

View file

@ -67,9 +67,28 @@ describe('R method extractor: setMethod owner is the signature argument', () =>
expect(methodOwner(source)).toBe('Foo');
});
it('names no owner when the signature is not a string', () => {
expect(methodOwner('setMethod("show", c("A", "B"), function(object) 1)')).toBeNull();
expect(methodOwner('setMethod("show", signature("A"), function(object) 1)')).toBeNull();
it.each([
['signature("A")', 'setMethod("show", signature("A"), function(object) 1)', 'A'],
['signature(name = "B")', 'setMethod("show", signature(object = "B"), function(o) 1)', 'B'],
[
'signature(x, y) uses the first class',
'setMethod("f", signature("A", "B"), function(x, y) 1)',
'A',
],
['c("A", "B")', 'setMethod("show", c("P2", "Q"), function(object) 1)', 'P2'],
['c(object = "A")', 'setMethod("show", c(object = "A"), function(object) 1)', 'A'],
['named signature = signature(...)', 'setMethod("show", signature = signature("A"))', 'A'],
['methods::signature(...)', 'setMethod("show", methods::signature("A"), function(o) 1)', 'A'],
])('takes the first class of a multi-class signature: %s', (_label, source, owner) => {
expect(methodOwner(source)).toBe(owner);
});
it('names no owner when the signature is not a string or a literal signature call', () => {
expect(methodOwner('setMethod("show", sig, function(object) 1)')).toBeNull();
expect(methodOwner('setMethod("show", signature(cls), function(object) 1)')).toBeNull();
expect(methodOwner('setMethod("show", signature(cls, "B"), function(object) 1)')).toBeNull();
expect(methodOwner('setMethod("show", list("A"), function(object) 1)')).toBeNull();
expect(methodOwner('setMethod("show", signature(), function(object) 1)')).toBeNull();
expect(methodOwner('setMethod("show", where = "pkg")')).toBeNull();
});
});