fix(r): read a NULL default as a formal that may hold a function

A formal defaulting to NULL is the usual placeholder of an optional
callback, so classing it as constant let a rename edit the call of the
parameter and drew a false edge to a same-named top-level function. Treat it
as 'maybe': the rename refuses the call and the scope publishes the formal
as a lexical name. Update the provider guide and pin both paths.
This commit is contained in:
Mick Cooney 2026-10-18 13:18:37 +01:00
parent af4ee3d9b4
commit 587bf6ce85
6 changed files with 27 additions and 24 deletions

View file

@ -35,7 +35,7 @@ Imports:
Calls:
- Free calls, `obj$method()` calls (receiver typed as above), R6 `self$method()` calls, and calls inside native pipe (`|>`) chains.
- A call through a name that a function's formal or one of its `for` variables binds (`function(data, transform) transform(data)`) gets no edge, not even to a top-level function of that name. The scope publishes those names as lexical names: they stop the lookup and the unique-name fallback. A formal with a constant default (`n = 2`, `x = NULL`, `s = "a"`) that the body never assigns is left out, because R skips a value that is not a function when it looks up a callee, so that call still reaches the function of that name (the rename provider reads it the same way); a formal that may hold a function, and a `for` variable, are not. A `for` loop outside every function binds nothing, and a variable assigned by `<-` in the body (`helper <- identity; helper(x)`) still does not shadow.
- A call through a name that a function's formal or one of its `for` variables binds (`function(data, transform) transform(data)`) gets no edge, not even to a top-level function of that name. The scope publishes those names as lexical names: they stop the lookup and the unique-name fallback. A formal with a constant default (`n = 2`, `s = "a"`, `b = TRUE`) that the body never assigns is left out, because R skips a value that is not a function when it looks up a callee, so that call still reaches the function of that name (the rename provider reads it the same way); a formal that may hold a function, and a `for` variable, are not. A `NULL` default is not constant: it is the usual placeholder of an optional callback (`function(x, helper = NULL) helper(x)`), so that formal counts as one that may hold a function. A `for` loop outside every function binds nothing, and a variable assigned by `<-` in the body (`helper <- identity; helper(x)`) still does not shadow.
- Named nested functions (closures) are indexed, and calls inside attribute to the nested function, which can shadow a top-level name. Top-level script calls outside any function are attributed to the File node.
- `pkg::fn()` and `pkg:::fn()`, decided by the qualifier (see [Resolution model](#resolution-model)).
@ -154,7 +154,7 @@ Not modelled:
- a replacement function and its getter (`unsupported_occurrence`): a call of the name anywhere in an assignment target (`helper(z) <- 5`, `helper(z)$a <- 1`, `names(helper(z)) <- v`, the `->` forms), because R then calls `` `helper<-` ``; an old or new name ending in `<-`; and a string-defined setter (`"helper<-" <- function(...)`) of either name;
- a new name that is already a global (`name_collision`): a top-level variable or function of that name in any indexed file (`assist <- 3`, a `for` variable, `3 -> assist`), a `<<-` or `->>` to it anywhere, or a function of that name in another file. Same-file function collisions were already refused by the shared engine; local variables, parameters and members of that name are allowed;
- a new name that the sources already read as a value or call (`name_collision`): `sapply(x, paste)`, `Reduce(max, x)`, `x <- letters`, a default `f = assist`, a formula term, `assist[1]`, `assist(1)`. After the rename the function is the global of that name and would capture the read. A parameter of an enclosing function of that name covers a read, and covers a call only when it has no default. Not reads: bound names, argument names (`f(assist = 1)`), members (`x$assist`, `x@assist`) and qualified names (`pkg::assist`);
- a call of the name under a same-named parameter that may hold a function (`ambiguous_reference`): a function or computed default, or an assignment to the parameter in the body, because the call may reach the parameter or the function being renamed;
- a call of the name under a same-named parameter that may hold a function (`ambiguous_reference`): a function, `NULL` or computed default, or an assignment to the parameter in the body, because the call may reach the parameter or the function being renamed;
- 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).

View file

@ -98,7 +98,11 @@ export function registersS3Method(newName: string, defined: ReadonlySet<string>)
return false;
}
/** Default values that can never be a function. */
/**
* Default values that can never be a function. `NULL` is not one of them: it is the usual
* placeholder of an optional callback (`function(x, helper = NULL) helper(x)`), so the formal
* may hold a function when the caller supplies one.
*/
const CONSTANT_DEFAULTS: ReadonlySet<string> = new Set([
'float',
'integer',
@ -106,7 +110,6 @@ const CONSTANT_DEFAULTS: ReadonlySet<string> = new Set([
'string',
'true',
'false',
'null',
'na',
'inf',
'nan',

View file

@ -17,4 +17,6 @@ plain <- function(data) transform(data)
constant_default <- function(data, transform = 2) transform(data)
null_default <- function(data, transform = NULL) transform(data)
function_default <- function(data, validate = function(x) x) validate(data)

View file

@ -42,6 +42,10 @@ describe('R calls through a formal or a loop variable', () => {
expect(targets('constant_default')).toEqual(['transform']);
});
it('does not bind a call through a formal that defaults to NULL: it can hold a callback', () => {
expect(targets('null_default')).toEqual([]);
});
it('does not bind a call through a formal that defaults to a function', () => {
expect(targets('function_default')).toEqual([]);
});

View file

@ -48,24 +48,14 @@ describe('rParameterKind', () => {
expect(kindOf('use <- function(x, helper) helper(x)')).toBe('function');
});
it.each([
'2',
'2L',
'1i',
'"s"',
'TRUE',
'FALSE',
'NULL',
'NA',
'NA_integer_',
'Inf',
'NaN',
'-1',
])('treats the constant default %s as not a function', (constant) => {
expect(kindOf(`use <- function(x, helper = ${constant}) helper(x)`)).toBe('constant');
});
it.each(['2', '2L', '1i', '"s"', 'TRUE', 'FALSE', 'NA', 'NA_integer_', 'Inf', 'NaN', '-1'])(
'treats the constant default %s as not a function',
(constant) => {
expect(kindOf(`use <- function(x, helper = ${constant}) helper(x)`)).toBe('constant');
},
);
it.each(['function(z) z', '\\(z) z', 'identity', 'c(1, 2)', 'x + 1', 'list()'])(
it.each(['function(z) z', '\\(z) z', 'NULL', 'identity', 'c(1, 2)', 'x + 1', 'list()'])(
'treats the default %s as possibly a function',
(expression) => {
expect(kindOf(`use <- function(x, helper = ${expression}) helper(x)`)).toBe('maybe');
@ -107,6 +97,10 @@ describe('R rename: a call that a same-named parameter may or may not shadow', (
);
});
it('refuses a call under a parameter that defaults to NULL: it can hold a callback', () => {
expect(verdict('use <- function(x, helper = NULL) { helper(x) }')).toBe('ambiguous_reference');
});
it('refuses a call under a parameter with a computed default', () => {
expect(verdict('use <- function(x, helper = pick()) { helper(x) }')).toBe(
'ambiguous_reference',

View file

@ -31,9 +31,9 @@ describe('rFunctionLexicalNames', () => {
});
it('skips a formal with a constant default that the body never assigns', () => {
expect(namesOf('f <- function(a, n = 2, s = "x", b = TRUE, z = NULL, m = -1) 1')).toEqual([
'a',
]);
expect(namesOf('f <- function(a, n = 2, s = "x", b = TRUE, m = -1) 1')).toEqual(['a']);
// NULL is the usual placeholder of an optional callback, so that formal may hold a function.
expect(namesOf('f <- function(a, z = NULL) 1')).toEqual(['a', 'z']);
// Assigned in the body, the formal may hold a function again.
expect(namesOf('f <- function(a, n = 2) { n <- identity; n(a) }')).toEqual(['a', 'n']);
// A constant-default formal that a loop also binds is a loop variable.