diff --git a/packages/agent-bundle/tests/check-test-remove-tree.test.ts b/packages/agent-bundle/tests/check-test-remove-tree.test.ts
index 2b6458b8e..4f0d02119 100644
--- a/packages/agent-bundle/tests/check-test-remove-tree.test.ts
+++ b/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 =
;',
+ `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, { ${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, { ${recursiveTrue}, maxRetries: 5 });`,
+ ]))).toEqual([]);
+});
diff --git a/scripts/check-test-remove-tree.d.mts b/scripts/check-test-remove-tree.d.mts
index d7056e395..883389100 100644
--- a/scripts/check-test-remove-tree.d.mts
+++ b/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;
- /** Namespace and default import names whose `.rm` is node's. */
+ /** Namespace, default, and `promises` rebind names whose `.rm` is node's. */
readonly namespaceNames: ReadonlySet;
}
diff --git a/scripts/check-test-remove-tree.mjs b/scripts/check-test-remove-tree.mjs
index a63b1a9e3..32bebb86c 100644
--- a/scripts/check-test-remove-tree.mjs
+++ b/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,