fix(r): refuse a new name that the sources already read as a value

The engine protects the call form of a new name, but R's reference sites are calls,
so a plain read was invisible: sapply(x, paste), Reduce(max, x), x <- letters or a
default f = assist all kept compiling and silently started to mean the renamed
global.

The provider now refuses any identifier read of the new name that is not bound
locally: a parameter of an enclosing function covers a read, and covers a call only
when it has no default. Bound names, argument names, members ($, @) and qualified
names are not reads. Four fixtures and unit tests cover values, callees, formulas
and the allowed spellings.

gitnexus/src/core/ingestion/languages/r/rename.ts

@@ -248,6 +248,43 @@ function insideFunction(node: Parser.SyntaxNode): boolean {
   return false;
 }
 
+/**
+ * Whether `node`, an identifier spelled like the new name, is used to READ a binding: a value
+ * (`sapply(x, paste)`, `x <- letters`, a default `f = assist`) or a callee. After the rename
+ * the renamed function is the global of that name, so a read that reached something else
+ * (a base or attached object, another package's function) would be captured by it. Not reads:
+ * a bound name (`assist <- 3`, a parameter, a `for` variable), an argument name (`f(assist = 1)`),
+ * a member (`x$assist`, `x@assist`) and a qualified name (`pkg::assist`).
+ */
+function readsNewName(node: Parser.SyntaxNode, newName: string): boolean {
+  const parent = node.parent;
+  if (parent === null) return true;
+  switch (parent.type) {
+    case 'argument':
+      if (parent.childForFieldName('name')?.id === node.id) return false;
+      break;
+    case 'parameter':
+      if (parent.childForFieldName('name')?.id === node.id) return false;
+      break;
+    case 'extract_operator':
+      if (parent.childForFieldName('rhs')?.id === node.id) return false;
+      break;
+    case 'namespace_operator':
+      return false;
+    case 'for_statement':
+      if (parent.childForFieldName('variable')?.id === node.id) return false;
+      break;
+    case 'binary_operator':
+      if (assignmentTarget(parent)?.id === node.id) return false;
+      break;
+  }
+  // A parameter of the enclosing function binds the name locally: a read of it, or a call of it
+  // when it can only be a function (no default), never reaches the global.
+  const local = innermostParameterKind(node, newName);
+  if (local === null) return true;
+  return isCallee(node) && local !== 'function';
+}
+
 /**
  * A name R also writes between its operands: an infix function (`%+%`, `%in%`) or an operator
  * (`+`, `[[`). Its uses are operator tokens, not `identifier` nodes, so a rename would edit the
@@ -371,6 +408,14 @@ export const rRenameProvider: RenameProvider = {
           superassignedName(node) === newName
         )
           return collision('The new name is already assigned as a variable at top level.');
+        if (
+          node.type === 'identifier' &&
+          rBareName(node.text) === newName &&
+          readsNewName(node, newName)
+        )
+          return collision(
+            'The new name is already read here as a value or called; the renamed function would capture it.',
+          );
         // `name <<- value` (inside a function or not) is a global re-definition that the
         // scope graph keeps as a separate declaration, so a rename would leave it behind.
         if (node.type === 'identifier' && isCallee(node) && rBareName(node.text) === name) {

gitnexus/test/fixtures/rename-integration/r/value-reference-constant/after/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+alphabet <- letters

gitnexus/test/fixtures/rename-integration/r/value-reference-constant/before/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+alphabet <- letters

gitnexus/test/fixtures/rename-integration/r/value-reference-constant/case.json

@@ -0,0 +1,9 @@
+{
+  "language": "r",
+  "description": "Refuse a new name that the sources read as a base constant (x <- letters): the renamed function would shadow it.",
+  "categories": ["name-collision", "value-reference", "negative", "unchanged-on-error"],
+  "target": { "name": "helper", "kind": "Function", "file": "ref.R", "startLine": 0 },
+  "newName": "letters",
+  "expect": { "status": "error", "code": "name_collision", "planningStatus": "blocked" },
+  "edges": []
+}

gitnexus/test/fixtures/rename-integration/r/value-reference-default/after/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+use <- function(f = assist) f(1)

gitnexus/test/fixtures/rename-integration/r/value-reference-default/before/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+use <- function(f = assist) f(1)

gitnexus/test/fixtures/rename-integration/r/value-reference-default/case.json

@@ -0,0 +1,9 @@
+{
+  "language": "r",
+  "description": "Refuse a new name that a parameter default reads (f = assist): the default would start to mean the renamed function.",
+  "categories": ["name-collision", "value-reference", "negative", "unchanged-on-error"],
+  "target": { "name": "helper", "kind": "Function", "file": "ref.R", "startLine": 0 },
+  "newName": "assist",
+  "expect": { "status": "error", "code": "name_collision", "planningStatus": "blocked" },
+  "edges": []
+}

gitnexus/test/fixtures/rename-integration/r/value-reference-formula/after/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+fit <- function(d) lm(y ~ assist, data = d)

gitnexus/test/fixtures/rename-integration/r/value-reference-formula/before/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+fit <- function(d) lm(y ~ assist, data = d)

gitnexus/test/fixtures/rename-integration/r/value-reference-formula/case.json

@@ -0,0 +1,9 @@
+{
+  "language": "r",
+  "description": "Refuse a new name that a formula spells as a variable.",
+  "categories": ["name-collision", "value-reference", "negative", "unchanged-on-error"],
+  "target": { "name": "helper", "kind": "Function", "file": "ref.R", "startLine": 0 },
+  "newName": "assist",
+  "expect": { "status": "error", "code": "name_collision", "planningStatus": "blocked" },
+  "edges": []
+}

gitnexus/test/fixtures/rename-integration/r/value-reference-function-argument/after/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+squares <- sapply(1:3, paste)

gitnexus/test/fixtures/rename-integration/r/value-reference-function-argument/before/ref.R

@@ -0,0 +1,3 @@
+helper <- function(v) v + 1
+
+squares <- sapply(1:3, paste)

gitnexus/test/fixtures/rename-integration/r/value-reference-function-argument/case.json

@@ -0,0 +1,9 @@
+{
+  "language": "r",
+  "description": "Refuse a new name that the sources pass as a function value (sapply(x, paste)): the renamed global would capture the base function.",
+  "categories": ["name-collision", "value-reference", "negative", "unchanged-on-error"],
+  "target": { "name": "helper", "kind": "Function", "file": "ref.R", "startLine": 0 },
+  "newName": "paste",
+  "expect": { "status": "error", "code": "name_collision", "planningStatus": "blocked" },
+  "edges": []
+}

gitnexus/test/unit/r-rename-collisions.test.ts

@@ -65,3 +65,61 @@ describe('R rename: a new name that already exists', () => {
     }
   });
 });
+
+describe('R rename: a new name that the sources already read', () => {
+  it('refuses a value, a function argument or a callee of the new name', () => {
+    for (const source of [
+      'r <- sapply(1:3, assist)\n',
+      'r <- Reduce(assist, 1:3)\n',
+      'x <- assist\n',
+      'x <- assist[1]\n',
+      'x <- assist$a\n',
+      'f <- function(f = assist) f(1)\n',
+      'r <- assist(1)\n',
+      'r <- 1 |> assist()\n',
+      'r <- c(1, 2) |> sapply(assist)\n',
+      'r <- y ~ assist\n',
+      'print(`assist`)\n',
+      'g <- function() { assist <- 3; assist }\n',
+    ]) {
+      expect(verdict(source), source).toBe('name_collision');
+    }
+  });
+
+  it('allows a name that is not a read: argument name, member, qualified name, parameter', () => {
+    for (const source of [
+      'r <- f(assist = 1)\n',
+      'r <- x$assist\n',
+      'r <- x@assist\n',
+      'r <- pkg::assist\n',
+      'r <- assist::f(1)\n',
+      'r <- pkg:::assist(1)\n',
+      'f <- function(assist) assist * 2\n',
+      'f <- function(assist, n = assist) n\n',
+      'f <- function(assist) { g <- function() assist; g() }\n',
+      'f <- function(assist) assist(1)\n',
+      '# assist\n',
+    ]) {
+      expect(verdict(source), source).toBe('ok');
+    }
+  });
+
+  it('still refuses a call of a parameter that may not hold a function', () => {
+    for (const source of [
+      'f <- function(assist = 2) assist(1)\n',
+      'f <- function(assist = g()) assist(1)\n',
+      'f <- function(assist) { assist <- 3; assist(1) }\n',
+    ]) {
+      expect(verdict(source), source).toBe('name_collision');
+    }
+  });
+
+  it('reads the new name through backticks and across files', () => {
+    expect(
+      rVerdict({
+        files: { 'a.R': HELPER, 'b.R': 'x <- sapply(1:3, `assist`)\n' },
+        newName: 'assist',
+      }),
+    ).toBe('name_collision');
+  });
+});