fix(r): say what is wrong with a rejected rename name

validateName let ... and ..N through to a misleading name_collision. Reject
them as reserved, say in the message that new names are ASCII-only by design,
and report an old name made only of non-ASCII characters as an unsupported
identifier rather than an infix function or operator.
This commit is contained in:
Mick Cooney 2026-10-18 13:19:39 +01:00
parent 587bf6ce85
commit 3cfd55a340
3 changed files with 80 additions and 1 deletions

View file

@ -158,7 +158,7 @@ Not modelled:
- a function that is also assigned with `<<-` or `->>` anywhere in the indexed R sources (`ambiguous_reference`, as the top-level form already was): the global re-assignment is a second definition that a rename would leave behind;
- a package function under `R/` that the index did not prove private, because `NAMESPACE` is not an indexed source and its entry would go stale. The rename tool reads the indexed `isExported` flag, which is false only for a function that a `NAMESPACE` file beside the package leaves out of `export()`, `exportPattern()` and `S3method()` (the third argument of `S3method(generic, class, method)` counts). A private package function is renamed; an exported one is refused; so is a function in a package with no `NAMESPACE` file, or in an `R/` folder that belongs to no package, where the index keeps the default public flag.
- **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).
- **Invalid names (`invalid_name`):** anything that is not a nonreserved R identifier. Backtick-quoting a new name is never offered: a name that needs it is refused rather than quoted at every use.
- **Invalid names (`invalid_name`):** anything that is not a nonreserved R identifier: the reserved words, `...` and `..1`, `..2`, ..., and any name with a non-ASCII character, because new names are limited to ASCII letters, digits, `.` and `_` (conservative by design; the message says so). An old name made only of non-ASCII characters (`λ`) is refused as not a supported identifier. Backtick-quoting a new name is never offered: a name that needs it is refused rather than quoted at every use.
- **Not proven:** unqualified calls to a function defined in another file (including the package `R/` layout) fail with `ambiguous_reference`, because R's global-name fallback is not a provable binding. Renaming across files works only where the call is qualified or otherwise resolved.
## Type annotations

View file

@ -350,6 +350,19 @@ const parseNames = createTreeSitterRenameSource({
export const rRenameProvider: RenameProvider = {
canonicalName: rBareName,
validateName(name) {
// `...`, `..1`, `..2`, ... refer to the arguments passed down to a function; R reserves them.
if (name === '...' || /^\.\.[0-9]+$/.test(name))
return {
code: 'invalid_name',
message:
'Choose a valid nonreserved R identifier: `...` and `..1`, `..2`, ... are reserved.',
};
if (/[^\x00-\x7f]/.test(name))
return {
code: 'invalid_name',
message:
'Choose a valid nonreserved R identifier. New names are limited to ASCII letters, digits, `.` and `_` (conservative by design).',
};
if (!/^(?:[A-Za-z]|\.(?![0-9]))[A-Za-z0-9._]*$/.test(name) || RESERVED.has(name))
return { code: 'invalid_name', message: 'Choose a valid nonreserved R identifier.' };
},
@ -394,6 +407,12 @@ export const rRenameProvider: RenameProvider = {
return unsupported(
'A replacement function (`name<-`) is called as `name(x) <- value`; those calls are not renamed.',
);
// A name made only of non-ASCII characters (`λ`) is an identifier in a UTF-8 session, not an
// operator; the provider reads ASCII names only, so it says that rather than "operator".
if (/^[^\x00-\x7f]+$/.test(name))
return unsupported(
'Only ASCII identifiers are renamed; this name is not a supported identifier.',
);
if (isOperatorName(name) || isOperatorName(newName))
return unsupported(
'An infix function or operator (`%op%`, `+`) is also written between its operands; those uses are not renamed.',

View file

@ -0,0 +1,60 @@
/**
* The R rename provider's name checks say what is wrong with a name. `...` and `..1` are reserved
* in R, so they are not "a collision"; a new name with a non-ASCII character is refused because
* the provider reads ASCII identifiers only (conservative by design); an old name made only of
* non-ASCII characters (`λ`) is an identifier the provider does not support, not an operator.
*/
import { describe, expect, it } from 'vitest';
import { rRenameProvider } from '../../src/core/ingestion/languages/r/rename.js';
import { rIssue } from '../helpers/r-rename-verdict.js';
const check = (name: string): { code: string; message: string } | undefined => {
const issue = rRenameProvider.validateName?.(name);
return issue ? { code: issue.code, message: issue.message } : undefined;
};
describe('R rename: validateName', () => {
it.each(['...', '..1', '..2', '..10', '..0'])('rejects the reserved name %s', (name) => {
const issue = check(name);
expect(issue?.code).toBe('invalid_name');
expect(issue?.message).toMatch(/reserved/);
});
it.each(['..', '.x', '..x', 'x..1', '.a1', 'helper'])('still accepts %s', (name) => {
expect(check(name)).toBeUndefined();
});
it.each(['λ', 'café', 'aé', 'x_δ'])('rejects the non-ASCII name %s and says why', (name) => {
const issue = check(name);
expect(issue?.code).toBe('invalid_name');
expect(issue?.message).toMatch(/ASCII/);
expect(issue?.message).toMatch(/conservative/);
});
it.each(['1x', '_x', 'a b', 'if', 'NULL', '.1', ''])(
'keeps the general message for %s',
(name) => {
const issue = check(name);
expect(issue?.code).toBe('invalid_name');
expect(issue?.message).toBe('Choose a valid nonreserved R identifier.');
},
);
});
describe('R rename: an old name of non-ASCII characters only', () => {
it('is reported as an unsupported identifier, not as an operator', () => {
const issue = rIssue({ files: { 'a.R': 'λ <- function(v) v\n' }, target: 'λ', newName: 'lam' });
expect(issue?.code).toBe('unsupported_occurrence');
expect(issue?.message).toMatch(/not a supported identifier/);
expect(issue?.message).not.toMatch(/operator/);
});
it('still reports an operator as an operator', () => {
const issue = rIssue({
files: { 'a.R': '`+` <- function(a, b) a\n' },
target: '`+`',
newName: 'f',
});
expect(issue?.message).toMatch(/operator/);
});
});