diff --git a/docs/languages/r-provider.md b/docs/languages/r-provider.md index 758a2bd89..3ca36597c 100644 --- a/docs/languages/r-provider.md +++ b/docs/languages/r-provider.md @@ -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). diff --git a/gitnexus/src/core/ingestion/languages/r/rename.ts b/gitnexus/src/core/ingestion/languages/r/rename.ts index befb9a5df..bca9efc1b 100644 --- a/gitnexus/src/core/ingestion/languages/r/rename.ts +++ b/gitnexus/src/core/ingestion/languages/r/rename.ts @@ -98,7 +98,11 @@ export function registersS3Method(newName: string, defined: ReadonlySet) 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 = new Set([ 'float', 'integer', @@ -106,7 +110,6 @@ const CONSTANT_DEFAULTS: ReadonlySet = new Set([ 'string', 'true', 'false', - 'null', 'na', 'inf', 'nan', diff --git a/gitnexus/test/fixtures/lang-resolution/r-formal-shadows/R/a.R b/gitnexus/test/fixtures/lang-resolution/r-formal-shadows/R/a.R index a30c1647d..a926d0f04 100644 --- a/gitnexus/test/fixtures/lang-resolution/r-formal-shadows/R/a.R +++ b/gitnexus/test/fixtures/lang-resolution/r-formal-shadows/R/a.R @@ -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) diff --git a/gitnexus/test/integration/resolvers/r-formal-shadows.test.ts b/gitnexus/test/integration/resolvers/r-formal-shadows.test.ts index c1588e3f5..5176bc349 100644 --- a/gitnexus/test/integration/resolvers/r-formal-shadows.test.ts +++ b/gitnexus/test/integration/resolvers/r-formal-shadows.test.ts @@ -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([]); }); diff --git a/gitnexus/test/unit/r-rename-parameter-shadow.test.ts b/gitnexus/test/unit/r-rename-parameter-shadow.test.ts index efb77fa7a..d1c2173bc 100644 --- a/gitnexus/test/unit/r-rename-parameter-shadow.test.ts +++ b/gitnexus/test/unit/r-rename-parameter-shadow.test.ts @@ -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', diff --git a/gitnexus/test/unit/scope-resolution/r/r-lexical-names.test.ts b/gitnexus/test/unit/scope-resolution/r/r-lexical-names.test.ts index 4d593a2a8..588eb2e2e 100644 --- a/gitnexus/test/unit/scope-resolution/r/r-lexical-names.test.ts +++ b/gitnexus/test/unit/scope-resolution/r/r-lexical-names.test.ts @@ -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.