mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-30 06:42:43 +00:00
fix(r): refuse a rename target name that registers an S3 method at any dot
The guard looked only at the text before the first dot, so to.json.foo (a method of the dotted generic to.json) passed, and base generics such as print, format, summary and plot were never checked unless the sources defined them. Every dot-separated prefix of the new name is now tested against the functions the indexed sources define and a small list of base generics.
This commit is contained in:
parent
154b7d4ebb
commit
85fb8aeb55
11 changed files with 155 additions and 3 deletions
|
|
@ -123,7 +123,7 @@ Not modelled:
|
|||
- **Refused (`unsupported_occurrence`, planning status `unsupported`):**
|
||||
- any target that is not a plain `Function` (S4 classes, R6 classes, members);
|
||||
- a function whose old or new name is the whole value of a string literal anywhere in the indexed R sources (`UseMethod("area")`, `do.call("f", ...)`, `match.fun`, `get`, `setGeneric`, ...), because string lookup is not resolved. The literal is decoded first, so escapes (`"f\x6fo"`, `"\u{66}oo"`, octal) and raw strings (`r"(foo)"`) count. A string that only contains the name (`"foo(1)"`), a name built at run time (`paste0("fo", "o")`), and strings in files that are not indexed R sources (R Markdown, `NAMESPACE`) are not detected;
|
||||
- a new name `generic.class` whose prefix is an existing function, which would register an S3 method;
|
||||
- a new name `generic.class` where any dot-separated prefix is a function defined in the indexed R sources or a common base generic (`print`, `format`, `summary`, `plot`, `as.character`, ...), which would register an S3 method (`to.json.foo` for a defined `to.json`, `print.foo`);
|
||||
- a package function under `R/` that `NAMESPACE` does not mark private (`isExported` is not false), because `NAMESPACE` is not an indexed source and its `export()` or `S3method()` entry would go stale.
|
||||
- **Refused by the shared engine (fail-closed, not provider code):** S3 methods and any dotted function name (`area.circle` reads as an owner-qualified member, so `stale_graph`); S4 class names (`unsupported_occurrence`, no exact source-name span); R6 methods (`stale_graph`).
|
||||
- **Invalid names (`invalid_name`):** anything that is not a nonreserved R identifier.
|
||||
|
|
|
|||
|
|
@ -47,6 +47,40 @@ function rStringValue(text: string): string | null {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Generics of base R and its default packages that an S3 method name `generic.class` can register
|
||||
* against, whether or not the indexed sources define them. Not exhaustive: it covers the generics
|
||||
* a project is most likely to meet by accident, and the repository's own functions are checked
|
||||
* separately.
|
||||
*/
|
||||
const BASE_GENERICS: ReadonlySet<string> = new Set(
|
||||
(
|
||||
'print format summary plot lines points text image hist barplot boxplot pairs persp ' +
|
||||
'as.character as.data.frame as.list as.vector as.numeric as.integer as.logical as.matrix ' +
|
||||
'as.Date as.POSIXct as.POSIXlt as.factor as.table as.function as.environment as.double ' +
|
||||
'length names dim dimnames levels rev sort unique duplicated anyDuplicated rep seq ' +
|
||||
'mean median quantile range max min round diff scale cut split merge subset transform ' +
|
||||
'with within head tail t c cbind rbind toString all.equal is.na labels str ' +
|
||||
'update predict residuals coef fitted anova simulate terms formula family weights ' +
|
||||
'vcov logLik AIC BIC nobs deviance confint profile model.frame row.names open close ' +
|
||||
'by aggregate droplevels relevel reorder xtfrm solve determinant julian months weekdays'
|
||||
).split(' '),
|
||||
);
|
||||
|
||||
/**
|
||||
* Whether `newName` reads as an S3 method `generic.class`: some dot-separated prefix of it is a
|
||||
* function the indexed sources define or a known base generic. Every dot is a possible split
|
||||
* (`to.json.foo` is a method of `to.json`; `as.data.frame.foo` of `as.data.frame`).
|
||||
*/
|
||||
export function registersS3Method(newName: string, defined: ReadonlySet<string>): boolean {
|
||||
for (let dot = newName.indexOf('.'); dot >= 0; dot = newName.indexOf('.', dot + 1)) {
|
||||
if (dot === 0) continue;
|
||||
const prefix = newName.slice(0, dot);
|
||||
if (defined.has(prefix) || BASE_GENERICS.has(prefix)) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
const unsupported = (message: string): RenameIssue => ({
|
||||
code: 'unsupported_occurrence',
|
||||
status: 'unsupported',
|
||||
|
|
@ -114,8 +148,10 @@ export const rRenameProvider: RenameProvider = {
|
|||
),
|
||||
),
|
||||
);
|
||||
if (newName.includes('.') && defined.has(newName.slice(0, newName.indexOf('.'))))
|
||||
return unsupported('The new name would register an S3 method of an existing function.');
|
||||
if (registersS3Method(newName, defined))
|
||||
return unsupported(
|
||||
'The new name would register an S3 method of an existing function or base generic.',
|
||||
);
|
||||
for (const source of files.values()) {
|
||||
if (!source.tree) continue;
|
||||
for (const node of walkNamedNodes(source.tree.rootNode)) {
|
||||
|
|
|
|||
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-base-generic/after/helper.R
vendored
Normal file
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-base-generic/after/helper.R
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
helper <- function(v) v + 1
|
||||
|
||||
use_helper <- function(v) {
|
||||
helper(v)
|
||||
}
|
||||
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-base-generic/before/helper.R
vendored
Normal file
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-base-generic/before/helper.R
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
helper <- function(v) v + 1
|
||||
|
||||
use_helper <- function(v) {
|
||||
helper(v)
|
||||
}
|
||||
9
gitnexus/test/fixtures/rename-integration/r/s3-new-method-base-generic/case.json
vendored
Normal file
9
gitnexus/test/fixtures/rename-integration/r/s3-new-method-base-generic/case.json
vendored
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
{
|
||||
"language": "r",
|
||||
"description": "Refuse a new name that would register a method of a base generic (print.foo), although the repository defines no print().",
|
||||
"categories": ["s3", "name-collision", "negative", "unchanged-on-error"],
|
||||
"target": { "name": "helper", "kind": "Function", "file": "helper.R", "startLine": 0 },
|
||||
"newName": "print.foo",
|
||||
"expect": { "status": "error", "code": "unsupported_occurrence", "planningStatus": "unsupported" },
|
||||
"edges": []
|
||||
}
|
||||
1
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/after/generics.R
vendored
Normal file
1
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/after/generics.R
vendored
Normal file
|
|
@ -0,0 +1 @@
|
|||
to.json <- function(x) UseMethod("to.json")
|
||||
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/after/helper.R
vendored
Normal file
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/after/helper.R
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
helper <- function(v) v + 1
|
||||
|
||||
use_helper <- function(v) {
|
||||
helper(v)
|
||||
}
|
||||
1
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/before/generics.R
vendored
Normal file
1
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/before/generics.R
vendored
Normal file
|
|
@ -0,0 +1 @@
|
|||
to.json <- function(x) UseMethod("to.json")
|
||||
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/before/helper.R
vendored
Normal file
5
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/before/helper.R
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
helper <- function(v) v + 1
|
||||
|
||||
use_helper <- function(v) {
|
||||
helper(v)
|
||||
}
|
||||
9
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/case.json
vendored
Normal file
9
gitnexus/test/fixtures/rename-integration/r/s3-new-method-dotted-generic/case.json
vendored
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
{
|
||||
"language": "r",
|
||||
"description": "Refuse a new name whose second dot split names an existing dotted generic (to.json.foo registers a method of to.json).",
|
||||
"categories": ["s3", "name-collision", "negative", "unchanged-on-error"],
|
||||
"target": { "name": "helper", "kind": "Function", "file": "helper.R", "startLine": 0 },
|
||||
"newName": "to.json.foo",
|
||||
"expect": { "status": "error", "code": "unsupported_occurrence", "planningStatus": "unsupported" },
|
||||
"edges": []
|
||||
}
|
||||
76
gitnexus/test/unit/r-rename-s3-guard.test.ts
Normal file
76
gitnexus/test/unit/r-rename-s3-guard.test.ts
Normal file
|
|
@ -0,0 +1,76 @@
|
|||
/**
|
||||
* The R rename provider refuses a new name `generic.class` that would silently register an S3
|
||||
* method. Every dot is a possible split, so `to.json.foo` is a method of `to.json`, and a base
|
||||
* generic counts although no indexed source defines it (`print.foo`). Each refusal is paired with
|
||||
* a name that only looks similar.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import Parser from 'tree-sitter';
|
||||
import R from '@eagleoutice/tree-sitter-r';
|
||||
import { registersS3Method, rRenameProvider } from '../../src/core/ingestion/languages/r/rename.js';
|
||||
|
||||
const parser = new Parser();
|
||||
parser.setLanguage(R as Parameters<Parser['setLanguage']>[0]);
|
||||
|
||||
/** Run the provider's safety check for renaming `helper` to `newName`, with these functions defined. */
|
||||
function verdict(newName: string, defined: readonly string[]): string {
|
||||
const source = defined.map((name) => `${name} <- function(x) x`).join('\n');
|
||||
const files = new Map([
|
||||
[
|
||||
'a.R',
|
||||
{
|
||||
tree: parser.parse(source),
|
||||
parsed: { localDefs: defined.map((name) => ({ qualifiedName: name })) },
|
||||
},
|
||||
],
|
||||
]);
|
||||
const issue = rRenameProvider.checkSafety?.({
|
||||
target: { type: 'Function', qualifiedName: 'helper', filePath: 'a.R' },
|
||||
newName,
|
||||
files,
|
||||
graph: { forEachNode: () => undefined },
|
||||
} as never);
|
||||
return issue ? issue.code : 'ok';
|
||||
}
|
||||
|
||||
describe('R rename: the S3 registration guard', () => {
|
||||
it('refuses a name whose first dot split is a defined function', () => {
|
||||
expect(verdict('area.circle', ['area'])).toBe('unsupported_occurrence');
|
||||
});
|
||||
|
||||
it('refuses a name whose later dot split is a defined dotted generic', () => {
|
||||
expect(verdict('to.json.foo', ['to.json'])).toBe('unsupported_occurrence');
|
||||
expect(verdict('as.data.frame.foo', ['as.data.frame'])).toBe('unsupported_occurrence');
|
||||
});
|
||||
|
||||
it('refuses a name with one dot after a defined function of a shorter name', () => {
|
||||
expect(verdict('to.json', ['to'])).toBe('unsupported_occurrence');
|
||||
});
|
||||
|
||||
it('refuses a method of a base generic that the sources do not define', () => {
|
||||
for (const name of ['print.foo', 'format.foo', 'summary.foo', 'plot.foo', 'as.character.foo']) {
|
||||
expect(verdict(name, [])).toBe('unsupported_occurrence');
|
||||
}
|
||||
});
|
||||
|
||||
it('allows a dotted name when no prefix is a function or a base generic', () => {
|
||||
expect(verdict('my.helper', [])).toBe('ok');
|
||||
expect(verdict('to.json.foo', [])).toBe('ok');
|
||||
expect(verdict('toJson.foo', ['to.json'])).toBe('ok');
|
||||
});
|
||||
|
||||
it('allows a name with no dot, and a name that merely starts with a dot', () => {
|
||||
expect(verdict('print_foo', [])).toBe('ok');
|
||||
expect(verdict('.print', [])).toBe('ok');
|
||||
expect(verdict('.print.x', [])).toBe('ok');
|
||||
});
|
||||
|
||||
it('tests every prefix of the name', () => {
|
||||
const defined = new Set(['a.b']);
|
||||
expect([
|
||||
registersS3Method('a.b.c.d', defined),
|
||||
registersS3Method('a.x.b', defined),
|
||||
registersS3Method('a.b', defined),
|
||||
]).toEqual([true, false, false]);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue