mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-30 06:42:43 +00:00
fix(r): qualify R6, RefClass and S4 member ids with their class
Member ids carried no owner, so a second class in one file that shared a member name with an earlier one (push, items, a setMethod generic, a slot) minted the same id as the first and lost the member, along with its HAS_METHOD or HAS_PROPERTY edge. Register resolveDetachedOwnerName for R. It names the class declared by an ancestor R6Class assignment, setClass or setRefClass call, or the signature class of a setMethod call, so ids become Method:<file>:Stack.push#n and Property:<file>:Stack.items. The owner edge is still attached by postParse through ownerNameHint. The change is R-only: the hook is optional and undefined for every other language. The r-packages characterisation counts now include Child.greet, the member the id collision used to drop.
This commit is contained in:
parent
c6c86a247e
commit
fc32e53967
8 changed files with 238 additions and 7 deletions
|
|
@ -8,7 +8,7 @@ The provider is covered by unit and integration tests, listed under [Tests](#tes
|
|||
|
||||
GitNexus indexes `.r` and `.R` files with the `@eagleoutice/tree-sitter-r` grammar and resolves them through the scope-resolution pipeline (see ARCHITECTURE.md, Scope-Resolution Pipeline). R-specific logic lives in `gitnexus/src/core/ingestion/languages/r/`, with the per-language extractor and import-resolver files beside those of the other languages. Apart from the registrations every language has (the language enum, the file-extension and classification tables, the export-checker table, the grammar loader and parse-worker grammar map, and the query, provider and scope-resolver registries), shared ingestion code does not name R; the provider plugs in through `LanguageProvider` and `ScopeResolver` hooks.
|
||||
|
||||
R has no class syntax: classes are function calls (`R6Class(...)`, `setClass(...)`, `setRefClass(...)`) and package exports live in a NAMESPACE file rather than in the source. That is why the provider uses two optional hooks, `postParse` (attach each member to its owner and refine export status once every file is parsed) and `resolveMemberOwnerNode` (name the owner of a member from a call rather than an enclosing node).
|
||||
R has no class syntax: classes are function calls (`R6Class(...)`, `setClass(...)`, `setRefClass(...)`) and package exports live in a NAMESPACE file rather than in the source. That is why the provider uses three optional hooks: `postParse` (attach each member to its owner and refine export status once every file is parsed), `resolveMemberOwnerNode` (find the owner of a member from a call rather than an enclosing node) and `resolveDetachedOwnerName` (qualify a member's id with its class, so two classes in one file that share a member name stay distinct nodes: `Method:R/a.R:Stack.push#0`, `Property:R/a.R:Stack.items`).
|
||||
|
||||
## Supported constructs
|
||||
|
||||
|
|
|
|||
|
|
@ -190,6 +190,17 @@ export function findRFieldOwnerNode(node: SyntaxNode): SyntaxNode | null {
|
|||
export function getRTopLevelPropertyOwnerName(node: SyntaxNode): string | null {
|
||||
const ownerNode = findRFieldOwnerNode(node);
|
||||
if (!ownerNode) return null;
|
||||
return getRClassNameOfOwnerNode(ownerNode);
|
||||
}
|
||||
|
||||
/**
|
||||
* The class name an R6 / S4 / RefClass owner node declares: the binding name of
|
||||
* `Foo <- R6Class(...)`, or the `Class` argument of `setClass(...)` /
|
||||
* `setRefClass(...)`. Null when `ownerNode` is not such a node. Looks at this node
|
||||
* only (no ancestor walk), so a caller visiting every ancestor stays linear.
|
||||
*/
|
||||
export function getRClassNameOfOwnerNode(ownerNode: SyntaxNode): string | null {
|
||||
if (!isR6Class(ownerNode) && !isS4Class(ownerNode)) return null;
|
||||
|
||||
if (ownerNode.type === 'binary_operator') {
|
||||
return ownerNode.childForFieldName('lhs')?.text ?? null;
|
||||
|
|
|
|||
|
|
@ -17,7 +17,11 @@ import { rMethodExtractor } from '../method-extractors/r.js';
|
|||
import { emitRScopeCaptures } from './r/captures.js';
|
||||
import { interpretRImport, interpretRTypeBinding } from './r/interpret.js';
|
||||
import { rPostParse } from './r/post-parse.js';
|
||||
import { rDefinitionProperties, rResolveMemberOwnerNode } from './r/owner-hooks.js';
|
||||
import {
|
||||
rDefinitionProperties,
|
||||
rResolveDetachedOwnerName,
|
||||
rResolveMemberOwnerNode,
|
||||
} from './r/owner-hooks.js';
|
||||
import { isRNonNamingArgumentCapture } from './r/naming-argument.js';
|
||||
import { createRCfgVisitor } from '../cfg/visitors/r.js';
|
||||
import { rRenameProvider } from './r/rename.js';
|
||||
|
|
@ -190,6 +194,7 @@ export const rProvider = defineLanguage({
|
|||
interpretTypeBinding: interpretRTypeBinding,
|
||||
postParse: rPostParse,
|
||||
resolveMemberOwnerNode: rResolveMemberOwnerNode,
|
||||
resolveDetachedOwnerName: rResolveDetachedOwnerName,
|
||||
definitionPropertiesExtractor: rDefinitionProperties,
|
||||
// R_QUERIES captures every string argument of setClass/setGeneric/setMethod;
|
||||
// only the one that names the definition is kept.
|
||||
|
|
|
|||
|
|
@ -12,13 +12,29 @@
|
|||
|
||||
import type { SyntaxNode } from '../../utils/ast-helpers.js';
|
||||
import type { DefinitionPropertiesContext } from '../../language-provider.js';
|
||||
import { findRFieldOwnerNode, getRTopLevelPropertyOwnerName } from '../../field-extractors/r.js';
|
||||
import {
|
||||
findRFieldOwnerNode,
|
||||
getRClassNameOfOwnerNode,
|
||||
getRTopLevelPropertyOwnerName,
|
||||
} from '../../field-extractors/r.js';
|
||||
import { getRTopLevelMethodOwnerName } from '../../method-extractors/r.js';
|
||||
|
||||
/** `LanguageProvider.resolveMemberOwnerNode` for R. */
|
||||
export const rResolveMemberOwnerNode = (definitionNode: SyntaxNode): SyntaxNode | null =>
|
||||
findRFieldOwnerNode(definitionNode);
|
||||
|
||||
/**
|
||||
* `LanguageProvider.resolveDetachedOwnerName` for R: the class name an ancestor
|
||||
* call or assignment declares (`Foo <- R6Class(...)`, `setClass("Foo", ...)`,
|
||||
* `setRefClass("Foo", ...)`) or the class a `setMethod("m", "Foo", ...)` call
|
||||
* names in its signature. It qualifies the member's id (`Method:<file>:Foo.m`,
|
||||
* `Property:<file>:Foo.x`), so two classes in one file that share a member name
|
||||
* stay distinct nodes. It carries no graph id: the owner edge is still attached
|
||||
* by `postParse` through `ownerNameHint`.
|
||||
*/
|
||||
export const rResolveDetachedOwnerName = (ancestor: SyntaxNode): string | undefined =>
|
||||
getRClassNameOfOwnerNode(ancestor) ?? getRTopLevelMethodOwnerName(ancestor) ?? undefined;
|
||||
|
||||
/**
|
||||
* `LanguageProvider.definitionPropertiesExtractor` for R: the deferred owner
|
||||
* name (`ownerNameHint`) of a `Method`/`Property` whose owning class is named by
|
||||
|
|
|
|||
33
gitnexus/test/fixtures/lang-resolution/r-class-member-ids/R/classes.R
vendored
Normal file
33
gitnexus/test/fixtures/lang-resolution/r-class-member-ids/R/classes.R
vendored
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
# Several classes in one file whose members share names. Each member must be its own
|
||||
# node, owned by its own class.
|
||||
|
||||
Stack <- R6::R6Class("Stack", public = list(
|
||||
items = list(),
|
||||
initialize = function() { self$items <- list() },
|
||||
push = function(x) { self$items <- c(self$items, x); invisible(self) },
|
||||
twice = function(x) { self$push(x); self$push(x) }
|
||||
))
|
||||
|
||||
Queue <- R6::R6Class("Queue", public = list(
|
||||
items = list(),
|
||||
initialize = function() { self$items <- list() },
|
||||
push = function(x) { self$items <- c(self$items, x); invisible(self) },
|
||||
twice = function(x) { self$push(x); self$push(x) }
|
||||
))
|
||||
|
||||
Account <- setRefClass("Account",
|
||||
fields = list(balance = "numeric"),
|
||||
methods = list(deposit = function(x) { balance <<- balance + x })
|
||||
)
|
||||
|
||||
Wallet <- setRefClass("Wallet",
|
||||
fields = list(balance = "numeric"),
|
||||
methods = list(deposit = function(x) { balance <<- balance + x })
|
||||
)
|
||||
|
||||
setClass("Circle", slots = list(size = "numeric"))
|
||||
setClass("Square", slots = list(size = "numeric"))
|
||||
|
||||
setGeneric("area", function(shape) standardGeneric("area"))
|
||||
setMethod("area", "Circle", function(shape) 1)
|
||||
setMethod("area", "Square", function(shape) 2)
|
||||
121
gitnexus/test/integration/resolvers/r-class-member-ids.test.ts
Normal file
121
gitnexus/test/integration/resolvers/r-class-member-ids.test.ts
Normal file
|
|
@ -0,0 +1,121 @@
|
|||
/**
|
||||
* R: classes are calls, so a member's id used to carry no owner. Two classes in one
|
||||
* file that shared a member name (`push`, `items`, `deposit`, `balance`, a `setMethod`
|
||||
* generic, an S4 slot) minted the same id, and the second class lost the member. The
|
||||
* id now carries the owning class.
|
||||
*/
|
||||
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());
|
||||
|
||||
const FILE = 'R/classes.R';
|
||||
|
||||
describe('R members of several classes in one file', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
result = await runPipelineFromRepo(path.join(FIXTURES, 'r-class-member-ids'), () => {});
|
||||
}, 60000);
|
||||
|
||||
/** Ids of the nodes with this label and name, sorted. */
|
||||
const idsOf = (label: string, name: string): string[] => {
|
||||
const ids: string[] = [];
|
||||
result.graph.forEachNode((n) => {
|
||||
if (n.label === label && n.properties.name === name && n.properties.filePath === FILE) {
|
||||
ids.push(n.id);
|
||||
}
|
||||
});
|
||||
return ids.sort();
|
||||
};
|
||||
|
||||
/** `ownerName -> memberName` for every edge of this type, sorted. */
|
||||
const owned = (type: 'HAS_METHOD' | 'HAS_PROPERTY'): string[] =>
|
||||
getRelationships(result, type)
|
||||
.map((e) => `${e.source}->${e.target}`)
|
||||
.sort();
|
||||
|
||||
it('keeps one node per class for a shared R6 method name', () => {
|
||||
expect(idsOf('Method', 'push').map((id) => id.replace(/#\d+$/, ''))).toEqual([
|
||||
`Method:${FILE}:Queue.push`,
|
||||
`Method:${FILE}:Stack.push`,
|
||||
]);
|
||||
expect(idsOf('Method', 'initialize')).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('keeps one node per class for a shared R6 field name', () => {
|
||||
expect(idsOf('Property', 'items')).toEqual([
|
||||
`Property:${FILE}:Queue.items`,
|
||||
`Property:${FILE}:Stack.items`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps one node per class for a shared RefClass method and field name', () => {
|
||||
expect(idsOf('Method', 'deposit').map((id) => id.replace(/#\d+$/, ''))).toEqual([
|
||||
`Method:${FILE}:Account.deposit`,
|
||||
`Method:${FILE}:Wallet.deposit`,
|
||||
]);
|
||||
expect(idsOf('Property', 'balance')).toEqual([
|
||||
`Property:${FILE}:Account.balance`,
|
||||
`Property:${FILE}:Wallet.balance`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps one node per class for a shared S4 slot and setMethod generic', () => {
|
||||
expect(idsOf('Property', 'size')).toEqual([
|
||||
`Property:${FILE}:Circle.size`,
|
||||
`Property:${FILE}:Square.size`,
|
||||
]);
|
||||
expect(idsOf('Method', 'area').map((id) => id.replace(/#\d+$/, ''))).toEqual([
|
||||
`Method:${FILE}:Circle.area`,
|
||||
`Method:${FILE}:Square.area`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('gives every class its own HAS_METHOD edges', () => {
|
||||
const methods = owned('HAS_METHOD');
|
||||
for (const owner of ['Stack', 'Queue']) {
|
||||
for (const m of ['initialize', 'push', 'twice']) expect(methods).toContain(`${owner}->${m}`);
|
||||
}
|
||||
for (const owner of ['Account', 'Wallet']) expect(methods).toContain(`${owner}->deposit`);
|
||||
for (const owner of ['Circle', 'Square']) expect(methods).toContain(`${owner}->area`);
|
||||
expect(methods).toHaveLength(10);
|
||||
});
|
||||
|
||||
it('gives every class its own HAS_PROPERTY edges', () => {
|
||||
expect(owned('HAS_PROPERTY')).toEqual([
|
||||
'Account->balance',
|
||||
'Circle->size',
|
||||
'Queue->items',
|
||||
'Square->size',
|
||||
'Stack->items',
|
||||
'Wallet->balance',
|
||||
]);
|
||||
});
|
||||
|
||||
it('points each owner edge at its own class member node', () => {
|
||||
const edges = getRelationships(result, 'HAS_METHOD').filter((e) => e.target === 'push');
|
||||
const pairs = edges.map((e) => `${e.rel.sourceId}->${e.rel.targetId.replace(/#\d+$/, '')}`);
|
||||
expect(pairs.sort()).toEqual([
|
||||
`Class:${FILE}:Queue->Method:${FILE}:Queue.push`,
|
||||
`Class:${FILE}:Stack->Method:${FILE}:Stack.push`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('resolves self$push() inside each class to that class own push', () => {
|
||||
const calls = getRelationships(result, 'CALLS').filter((e) => e.target === 'push');
|
||||
const pairs = calls.map(
|
||||
(e) => `${e.rel.sourceId.replace(/#\d+$/, '')}->${e.rel.targetId.replace(/#\d+$/, '')}`,
|
||||
);
|
||||
expect([...new Set(pairs)].sort()).toEqual([
|
||||
`Method:${FILE}:Queue.twice->Method:${FILE}:Queue.push`,
|
||||
`Method:${FILE}:Stack.twice->Method:${FILE}:Stack.push`,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -1402,6 +1402,9 @@ describe('R deferred-owner attach (r-packages)', () => {
|
|||
'BareR6|value|HAS_PROPERTY',
|
||||
'BaseA|a_field|HAS_PROPERTY',
|
||||
'BaseB|b_field|HAS_PROPERTY',
|
||||
// Child and Parent share a file and both define `greet`; member ids carry the owner, so
|
||||
// Child keeps its own node and edge (the id used to collide and drop it).
|
||||
'Child|greet|HAS_METHOD',
|
||||
'DataModel|name|HAS_PROPERTY',
|
||||
'DataModel|validate|HAS_METHOD',
|
||||
'DataModel|value|HAS_PROPERTY',
|
||||
|
|
@ -1414,8 +1417,8 @@ describe('R deferred-owner attach (r-packages)', () => {
|
|||
'ResultSet|items|HAS_PROPERTY',
|
||||
];
|
||||
|
||||
it('emits exactly 10 HAS_METHOD and 12 HAS_PROPERTY edges', () => {
|
||||
expect(getRelationships(result, 'HAS_METHOD')).toHaveLength(10);
|
||||
it('emits exactly 11 HAS_METHOD and 12 HAS_PROPERTY edges', () => {
|
||||
expect(getRelationships(result, 'HAS_METHOD')).toHaveLength(11);
|
||||
expect(getRelationships(result, 'HAS_PROPERTY')).toHaveLength(12);
|
||||
});
|
||||
|
||||
|
|
@ -1429,7 +1432,7 @@ describe('R deferred-owner attach (r-packages)', () => {
|
|||
expect(triples).toEqual(ownedTriples);
|
||||
});
|
||||
|
||||
it('gives exactly the 22 attached R Method/Property nodes a string ownerId', () => {
|
||||
it('gives exactly the 23 attached R Method/Property nodes a string ownerId', () => {
|
||||
const owned: string[] = [];
|
||||
result.graph.forEachNode((n) => {
|
||||
if (
|
||||
|
|
@ -1440,7 +1443,7 @@ describe('R deferred-owner attach (r-packages)', () => {
|
|||
owned.push(n.id);
|
||||
}
|
||||
});
|
||||
expect(owned).toHaveLength(22);
|
||||
expect(owned).toHaveLength(23);
|
||||
});
|
||||
|
||||
it('leaves no R node with an unresolved ownerNameHint', () => {
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { createParserForLanguage } from '../../src/core/tree-sitter/parser-loade
|
|||
import type { SyntaxNode } from '../../src/core/ingestion/utils/ast-helpers.js';
|
||||
import {
|
||||
rDefinitionProperties,
|
||||
rResolveDetachedOwnerName,
|
||||
rResolveMemberOwnerNode,
|
||||
} from '../../src/core/ingestion/languages/r/owner-hooks.js';
|
||||
import type { DefinitionPropertiesContext } from '../../src/core/ingestion/language-provider.js';
|
||||
|
|
@ -99,6 +100,47 @@ describe('resolveMemberOwnerNode provider wiring', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('rResolveDetachedOwnerName (class-qualified member ids)', () => {
|
||||
const callTo = (fn: string): SyntaxNode =>
|
||||
find((n) => n.type === 'call' && n.childForFieldName('function')?.text === fn);
|
||||
|
||||
it('names the binding of an `R6Class(...)` assignment', () => {
|
||||
const r6 = find(
|
||||
(n) => n.type === 'binary_operator' && n.childForFieldName('lhs')?.text === 'Foo',
|
||||
);
|
||||
expect(rResolveDetachedOwnerName(r6)).toBe('Foo');
|
||||
});
|
||||
|
||||
it('names the Class argument of a `setRefClass(...)` call', () => {
|
||||
expect(rResolveDetachedOwnerName(callTo('setRefClass'))).toBe('Acc');
|
||||
});
|
||||
|
||||
it('names the signature class of a `setMethod(...)` call', () => {
|
||||
expect(rResolveDetachedOwnerName(callTo('setMethod'))).toBe('Circle');
|
||||
});
|
||||
|
||||
it('is undefined for ancestors that declare no class', () => {
|
||||
const plain = find(
|
||||
(n) => n.type === 'binary_operator' && n.childForFieldName('lhs')?.text === 'plain',
|
||||
);
|
||||
expect(rResolveDetachedOwnerName(plain)).toBeUndefined();
|
||||
expect(rResolveDetachedOwnerName(fnDefinitionOn(3))).toBeUndefined();
|
||||
expect(rResolveDetachedOwnerName(callTo('list'))).toBeUndefined();
|
||||
expect(rResolveDetachedOwnerName(root)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('is registered on the R provider only', () => {
|
||||
expect(rProvider.resolveDetachedOwnerName).toBe(rResolveDetachedOwnerName);
|
||||
for (const lang of [
|
||||
SupportedLanguages.Python,
|
||||
SupportedLanguages.TypeScript,
|
||||
SupportedLanguages.Java,
|
||||
]) {
|
||||
expect(getProvider(lang).resolveDetachedOwnerName).not.toBe(rResolveDetachedOwnerName);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('rDefinitionProperties (ownerNameHint)', () => {
|
||||
const ctxFor = (
|
||||
nodeLabel: DefinitionPropertiesContext['nodeLabel'],
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue