fix(test): close remove-tree lint AST false positives and negatives

packages/agent-bundle/tests/check-test-remove-tree.test.ts

@@ -240,3 +240,151 @@ it('still gates aliased and namespace Node fs.rm without maxRetries', () => {
     `await fs.rm(path, { ${recursiveTrue}, maxRetries: 5 });`,
   ]))).toEqual([]);
 });
+
+it('accepts shorthand maxRetries and ignores a shadowed local rm', () => {
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    'const maxRetries = 5;',
+    `await rm(root, { ${recursiveTrue}, maxRetries });`,
+  ]))).toEqual([expect.objectContaining({ hasRetries: true, line: 3 })]);
+  expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([
+    "import { rm } from 'node:fs/promises';",
+    'const maxRetries = 5;',
+    `await rm(root, { ${recursiveTrue}, maxRetries });`,
+  ]))).toEqual([]);
+
+  const shadowed = sample([
+    "import { rm } from 'node:fs';",
+    'const cleanup = async () => {',
+    '  const rm = async () => undefined;',
+    `  await rm(root, { ${recursiveTrue} });`,
+    '};',
+  ]);
+  expect(recursiveRmCalls(shadowed)).toEqual([]);
+  expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', shadowed)).toEqual([]);
+
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs';",
+    `await rm(root, { ${recursiveTrue} });`,
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 2 })]);
+});
+
+it('ignores loop, switch-case, and named function expression shadows only in their scope', () => {
+  expect(recursiveRmCalls(sample([
+    "import { promises as fs } from 'node:fs';",
+    `for (const fs of mockFilesystems) await fs.rm(root, { ${recursiveTrue} });`,
+    `for (const [, fs] of entries) { await fs.rm(root, { ${recursiveTrue} }); }`,
+    `for (let fs = mock; fs; fs = undefined) await fs.rm(root, { ${recursiveTrue} });`,
+    `for (const fs in mocks) await fs.rm(root, { ${recursiveTrue} });`,
+    `await fs.rm(root, { ${recursiveTrue} });`,
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 6 })]);
+
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    'switch (mode) {',
+    '  case 1:',
+    '    const rm = mockRm;',
+    `    await rm(root, { ${recursiveTrue} });`,
+    '    break;',
+    '  default:',
+    '    const [rm2, rm] = mocks;',
+    `    await rm(root, { ${recursiveTrue} });`,
+    '}',
+    `const again = async function rm() { await rm(root, { ${recursiveTrue} }); };`,
+    `const inner = () => { const rm = mockRm; return rm(root, { ${recursiveTrue} }); };`,
+    `await rm(root, { ${recursiveTrue} });`,
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 13 })]);
+});
+
+it('does not let a parameter shadow a computed method key or decorator', () => {
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    'const hooks = {',
+    `  [rm(root, { ${recursiveTrue} })](rm) {},`,
+    `  async cleanup(rm) { await rm(root, { ${recursiveTrue} }); },`,
+    '};',
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 3 })]);
+
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    'class Suite {',
+    `  @hook(rm(root, { ${recursiveTrue} })) run(rm) {}`,
+    '}',
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 3 })]);
+});
+
+it('parses TSX and JS test files with their own script kind', () => {
+  const tsx = sample([
+    "import { rm } from 'node:fs/promises';",
+    'const view = <div className="root" />;',
+    `await rm(root, { ${recursiveTrue} });`,
+  ]);
+  expect(bareRecursiveRmFailures('packages/workbench/tests/view.test.tsx', tsx)).toEqual([
+    'packages/workbench/tests/view.test.tsx:3 bare recursive rm. Use removeTree.',
+  ]);
+  expect(bareRecursiveRmFailures('packages/agent-bundle/tests/fixture.mjs', sample([
+    "import * as fs from 'node:fs/promises';",
+    `await fs.rm(root, { ${recursiveTrue} });`,
+  ]))).toEqual([
+    'packages/agent-bundle/tests/fixture.mjs:2 bare recursive rm. Use removeTree.',
+  ]);
+});
+
+it('flags promises-namespace and asserted options without maxRetries', () => {
+  expect(removalBindings(`import { promises as fs } from 'node:fs';`)).toEqual({
+    bareNames: new Set(),
+    namespaceNames: new Set(['fs']),
+  });
+  expect(removalBindings(`import { promises as fs } from 'fs';`)).toEqual({
+    bareNames: new Set(),
+    namespaceNames: new Set(['fs']),
+  });
+
+  const promisesNs = recursiveRmCalls(sample([
+    "import { promises as fs } from 'node:fs';",
+    `await fs.rm(path, { ${recursiveTrue} });`,
+  ]));
+  expect(promisesNs).toEqual([expect.objectContaining({ hasRetries: false, line: 2 })]);
+  expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([
+    "import { promises as fs } from 'node:fs';",
+    `await fs.rm(path, { ${recursiveTrue} });`,
+  ]))).toEqual([
+    'packages/agent-bundle/tests/example.test.ts:2 bare recursive rm. Use removeTree.',
+  ]);
+  expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([
+    "import { promises as fs } from 'node:fs';",
+    `await fs.rm(path, { ${recursiveTrue}, maxRetries: 5 });`,
+  ]))).toEqual([]);
+
+  const asserted = recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    `await rm(root, { ${recursiveTrue} } as const);`,
+  ]));
+  expect(asserted).toEqual([expect.objectContaining({ hasRetries: false, line: 2 })]);
+  expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([
+    "import { rm } from 'node:fs/promises';",
+    `await rm(root, { ${recursiveTrue} } as const);`,
+  ]))).toEqual([
+    'packages/agent-bundle/tests/example.test.ts:2 bare recursive rm. Use removeTree.',
+  ]);
+
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    `await rm(root, ({ ${recursiveTrue} }));`,
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 2 })]);
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    `await rm(root, { ${recursiveTrue} } satisfies Options);`,
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 2 })]);
+  expect(recursiveRmCalls(sample([
+    "import { rm } from 'node:fs/promises';",
+    `await rm(root, <const>{ ${recursiveTrue} });`,
+  ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 2 })]);
+  expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([
+    "import { rm } from 'node:fs/promises';",
+    `await rm(root, { ${recursiveTrue}, maxRetries: 5 } as const);`,
+    `await rm(root, ({ ${recursiveTrue}, maxRetries: 5 }));`,
+    `await rm(root, { ${recursiveTrue}, maxRetries: 5 } satisfies Options);`,
+    `await rm(root, <const>{ ${recursiveTrue}, maxRetries: 5 });`,
+  ]))).toEqual([]);
+});

scripts/check-test-remove-tree.d.mts

@@ -8,7 +8,7 @@ export interface RecursiveRmCall {
 export interface RemovalBindings {
   /** Local names bound to `rm` from node:fs or node:fs/promises, including aliases. */
   readonly bareNames: ReadonlySet<string>;
-  /** Namespace and default import names whose `.rm` is node's. */
+  /** Namespace, default, and `promises` rebind names whose `.rm` is node's. */
   readonly namespaceNames: ReadonlySet<string>;
 }
 

scripts/check-test-remove-tree.mjs

@@ -8,9 +8,12 @@
  * Call, option, and import-binding detection is parser-backed (typescript-5):
  * only real node:fs(/promises) ImportDeclaration bindings count, only Node-bound
  * call expressions are considered, and `recursive` / `maxRetries` are read from
- * the second argument's object-literal properties (including quoted keys). Nested
+ * the second argument's object-literal properties (including quoted keys,
+ * shorthand `maxRetries`, and Parenthesized / As / Satisfies wrappers). Nested
  * objects in the path argument, member calls, comments, strings, regexes, and
  * template substitutions are handled by the AST rather than text masking.
+ * Named `promises` rebinds from `fs` / `node:fs` count as `.rm` carriers.
+ * A later local binding that shadows an import is not treated as Node-bound.
  */
 import { createRequire } from 'node:module';
 import { readdir, readFile } from 'node:fs/promises';
@@ -49,7 +52,7 @@ const walk = async (directory, files) => {
 };
 
 /**
- * Named/aliased rm bindings and namespace/default bindings that expose .rm.
+ * Named/aliased rm bindings and namespace/default/`promises` bindings that expose .rm.
  * Import bindings are collected from the TypeScript AST so comments and local
  * identifiers cannot forge Node fs.rm bindings.
  */
@@ -87,15 +90,20 @@ export const removalBindings = (text, fileName = 'bindings.ts') => {
     }
 
     if (!ts.isNamedImports(bindings)) continue;
+    const isFsRoot = /^(?:node:)?fs$/u.test(statement.moduleSpecifier.text);
     for (const element of bindings.elements) {
       if (element.isTypeOnly) continue;
-      if (element.propertyName !== undefined) {
-        if (element.propertyName.text !== 'rm') continue;
-        bareNames.add(element.name.text);
+      const importedName = element.propertyName === undefined
+        ? element.name.text
+        : element.propertyName.text;
+      const localName = element.name.text;
+      if (importedName === 'rm') {
+        bareNames.add(localName);
         continue;
       }
-      if (element.name.text !== 'rm') continue;
-      bareNames.add('rm');
+      if (importedName === 'promises' && isFsRoot) {
+        namespaceNames.add(localName);
+      }
     }
   }
 
@@ -108,14 +116,127 @@ const propertyName = (name) => {
   return undefined;
 };
 
+const unwrapExpression = (node) => {
+  let current = node;
+  while (
+    current !== undefined
+    && (
+      ts.isParenthesizedExpression(current)
+      || ts.isAsExpression(current)
+      || ts.isSatisfiesExpression(current)
+      || ts.isTypeAssertionExpression(current)
+    )
+  ) {
+    current = current.expression;
+  }
+  return current;
+};
+
+const declarationNameIs = (nameNode, name) => {
+  if (ts.isIdentifier(nameNode)) return nameNode.text === name;
+  if (ts.isObjectBindingPattern(nameNode) || ts.isArrayBindingPattern(nameNode)) {
+    return nameNode.elements.some((element) =>
+      ts.isBindingElement(element) && declarationNameIs(element.name, name));
+  }
+  return false;
+};
+
+const declarationListDeclares = (list, name) =>
+  list.declarations.some((decl) => declarationNameIs(decl.name, name));
+
+const statementDeclares = (statement, name) => {
+  if (ts.isVariableStatement(statement)) {
+    return declarationListDeclares(statement.declarationList, name);
+  }
+  if (ts.isFunctionDeclaration(statement) || ts.isClassDeclaration(statement)) {
+    return statement.name !== undefined && statement.name.text === name;
+  }
+  return false;
+};
+
+/** `from` is the child the walk came up through; names, computed keys, and decorators sit outside the function scope. */
+const functionLikeDeclares = (node, from, name) => {
+  if (
+    !(
+      ts.isFunctionDeclaration(node)
+      || ts.isFunctionExpression(node)
+      || ts.isArrowFunction(node)
+      || ts.isMethodDeclaration(node)
+      || ts.isConstructorDeclaration(node)
+    )
+  ) {
+    return false;
+  }
+  if (from !== node.body && !node.parameters.includes(from)) return false;
+  if (
+    (ts.isFunctionDeclaration(node) || ts.isFunctionExpression(node))
+    && node.name !== undefined
+    && node.name.text === name
+  ) {
+    return true;
+  }
+  return node.parameters.some((parameter) => declarationNameIs(parameter.name, name));
+};
+
+/**
+ * True when a later/inner local binding hides the Node fs import of `name`.
+ * ponytail: `var` is treated as block-scoped, so a `var` hoisted out of a nested
+ * block is missed and its call still fails the gate (false positive, never a
+ * false negative). Track function-scoped `var` if that ever bites.
+ */
+const identifierIsLocallyShadowed = (identifier) => {
+  const name = identifier.text;
+  let from = identifier;
+  let current = identifier.parent;
+  while (current !== undefined) {
+    if (
+      ts.isSourceFile(current)
+      || ts.isBlock(current)
+      || ts.isModuleBlock(current)
+      || ts.isCaseClause(current)
+      || ts.isDefaultClause(current)
+    ) {
+      const shadowed = current.statements.some((statement) => {
+        if (ts.isSourceFile(current) && ts.isImportDeclaration(statement)) return false;
+        return statementDeclares(statement, name);
+      });
+      if (shadowed) return true;
+    }
+    if (
+      (ts.isForStatement(current) || ts.isForOfStatement(current) || ts.isForInStatement(current))
+      && current.initializer !== undefined
+      && ts.isVariableDeclarationList(current.initializer)
+      && declarationListDeclares(current.initializer, name)
+    ) {
+      return true;
+    }
+    if (functionLikeDeclares(current, from, name)) return true;
+    if (
+      ts.isCatchClause(current)
+      && current.variableDeclaration !== undefined
+      && declarationNameIs(current.variableDeclaration.name, name)
+    ) {
+      return true;
+    }
+    from = current;
+    current = current.parent;
+  }
+  return false;
+};
+
 /** Options flags from a call's second-argument object literal only. */
 const optionsFlags = (optionsArg) => {
-  if (optionsArg === undefined || !ts.isObjectLiteralExpression(optionsArg)) {
+  const unwrapped = unwrapExpression(optionsArg);
+  if (unwrapped === undefined || !ts.isObjectLiteralExpression(unwrapped)) {
     return { recursive: false, hasRetries: false };
   }
   let recursive = false;
   let hasRetries = false;
-  for (const property of optionsArg.properties) {
+  for (const property of unwrapped.properties) {
+    if (ts.isShorthandPropertyAssignment(property)) {
+      if (property.name.text === 'maxRetries') hasRetries = true;
+      continue;
+    }
     if (!ts.isPropertyAssignment(property)) continue;
     const key = propertyName(property.name);
     if (key === 'recursive' && property.initializer.kind === ts.SyntaxKind.TrueKeyword) {
@@ -127,14 +248,17 @@ const optionsFlags = (optionsArg) => {
 };
 
 const isNodeBoundRmCall = (expression, bareNames, namespaceNames) => {
-  if (ts.isIdentifier(expression)) return bareNames.has(expression.text);
+  if (ts.isIdentifier(expression)) {
+    return bareNames.has(expression.text) && !identifierIsLocallyShadowed(expression);
+  }
   if (
     ts.isPropertyAccessExpression(expression)
     && !expression.questionDotToken
     && expression.name.text === 'rm'
     && ts.isIdentifier(expression.expression)
   ) {
-    return namespaceNames.has(expression.expression.text);
+    return namespaceNames.has(expression.expression.text)
+      && !identifierIsLocallyShadowed(expression.expression);
   }
   return false;
 };
@@ -147,7 +271,7 @@ const scriptKindFor = (fileName) => {
 };
 
 export const recursiveRmCalls = (text, fileName = 'check.ts') => {
-  const { bareNames, namespaceNames } = removalBindings(text);
+  const { bareNames, namespaceNames } = removalBindings(text, fileName);
   const sourceFile = ts.createSourceFile(
     fileName,
     text,