From 13cdf4347a5d1a44b057fd3b175a00a386311476 Mon Sep 17 00:00:00 2001 From: Mick Cooney Date: Sat, 17 Oct 2026 17:20:40 +0100 Subject: [PATCH] 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. --- .../src/core/ingestion/method-extractors/r.ts | 23 ++++++++- .../r-s4-signature-owners/R/shapes.R | 10 ++++ .../resolvers/r-s4-anchors.test.ts | 5 +- .../resolvers/r-s4-signature-owners.test.ts | 47 +++++++++++++++++++ .../unit/r-method-extractor-owner.test.ts | 25 ++++++++-- 5 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/r-s4-signature-owners/R/shapes.R create mode 100644 gitnexus/test/integration/resolvers/r-s4-signature-owners.test.ts diff --git a/gitnexus/src/core/ingestion/method-extractors/r.ts b/gitnexus/src/core/ingestion/method-extractors/r.ts index 9f16b1604..37e2deea6 100644 --- a/gitnexus/src/core/ingestion/method-extractors/r.ts +++ b/gitnexus/src/core/ingestion/method-extractors/r.ts @@ -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); } // --------------------------------------------------------------------------- diff --git a/gitnexus/test/fixtures/lang-resolution/r-s4-signature-owners/R/shapes.R b/gitnexus/test/fixtures/lang-resolution/r-s4-signature-owners/R/shapes.R new file mode 100644 index 000000000..52aa59f17 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/r-s4-signature-owners/R/shapes.R @@ -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) diff --git a/gitnexus/test/integration/resolvers/r-s4-anchors.test.ts b/gitnexus/test/integration/resolvers/r-s4-anchors.test.ts index 96406b28c..80e6e2532 100644 --- a/gitnexus/test/integration/resolvers/r-s4-anchors.test.ts +++ b/gitnexus/test/integration/resolvers/r-s4-anchors.test.ts @@ -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', () => { diff --git a/gitnexus/test/integration/resolvers/r-s4-signature-owners.test.ts b/gitnexus/test/integration/resolvers/r-s4-signature-owners.test.ts new file mode 100644 index 000000000..ca490163e --- /dev/null +++ b/gitnexus/test/integration/resolvers/r-s4-signature-owners.test.ts @@ -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']); + }); +}); diff --git a/gitnexus/test/unit/r-method-extractor-owner.test.ts b/gitnexus/test/unit/r-method-extractor-owner.test.ts index a9bf56220..85a84552a 100644 --- a/gitnexus/test/unit/r-method-extractor-owner.test.ts +++ b/gitnexus/test/unit/r-method-extractor-owner.test.ts @@ -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(); }); });