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 e4ae99fd6..21665c6c9 100644 --- a/packages/agent-bundle/tests/check-test-remove-tree.test.ts +++ b/packages/agent-bundle/tests/check-test-remove-tree.test.ts @@ -439,3 +439,48 @@ it('unwraps non-null asserted options, alone and nested in other wrappers', () = `await rm(root, ({ ${recursiveTrue}, maxRetries: 5 } as const)!);`, ]))).toEqual([]); }); + +it('flags fs.promises.rm and wrapped callees or callee objects', () => { + const flaggedLines = (count: number) => Array.from( + { length: count }, + (_, index) => expect.objectContaining({ hasRetries: false, line: index + 4 }), + ); + + expect(recursiveRmCalls(sample([ + "import fs from 'node:fs';", + "import * as nodeFs from 'node:fs';", + "import fsp from 'node:fs/promises';", + `await fs.promises.rm(root, { ${recursiveTrue} });`, + `await nodeFs.promises.rm(root, { ${recursiveTrue} });`, + `await fs.promises!.rm(root, { ${recursiveTrue} });`, + `await (fs.promises as typeof fsp).rm(root, { ${recursiveTrue} });`, + `await fsp.rm(root, { ${recursiveTrue} });`, + ]))).toEqual(flaggedLines(5)); + + expect(recursiveRmCalls(sample([ + "import { rm } from 'node:fs/promises';", + "import * as fs from 'node:fs/promises';", + '', + `await (rm)(root, { ${recursiveTrue} });`, + `await rm!(root, { ${recursiveTrue} });`, + `await (rm as typeof rm)(root, { ${recursiveTrue} });`, + `await (rm satisfies Remove)(root, { ${recursiveTrue} });`, + `await (rm)(root, { ${recursiveTrue} });`, + `await ((rm)!)(root, { ${recursiveTrue} });`, + `await fs!.rm(root, { ${recursiveTrue} });`, + `await (fs as typeof fs).rm(root, { ${recursiveTrue} });`, + `await (fs.rm)(root, { ${recursiveTrue} });`, + `await fs?.rm(root, { ${recursiveTrue} });`, + `await rm?.(root, { ${recursiveTrue} });`, + ]))).toEqual(flaggedLines(11)); + + expect(recursiveRmCalls(sample([ + "import fs from 'node:fs';", + "import { rm } from 'node:fs/promises';", + `function cleanup(fs) { return fs.promises.rm(root, { ${recursiveTrue} }); }`, + `{ const rm = mockRm; await (rm)!(root, { ${recursiveTrue} }); }`, + `const reset = (fs) => (fs as Mock)!.rm(root, { ${recursiveTrue} });`, + `await other.promises.rm(root, { ${recursiveTrue} });`, + `await fs.other.rm(root, { ${recursiveTrue} });`, + ]))).toEqual([]); +}); diff --git a/scripts/check-test-remove-tree.mjs b/scripts/check-test-remove-tree.mjs index ecfc4f2f5..d6db27b84 100644 --- a/scripts/check-test-remove-tree.mjs +++ b/scripts/check-test-remove-tree.mjs @@ -2,8 +2,10 @@ * Test teardown that deletes a tree calls `removeTree`. A bare `rm` with * `recursive: true` and no `maxRetries` races a late writer and flakes with ENOTEMPTY. * - * Catches bare `rm(`, aliased `import { rm as remove }` calls, and `ns.rm(` when - * `ns` is a namespace/default import from node:fs, fs, or their /promises forms. + * Catches bare `rm(`, aliased `import { rm as remove }` calls, and `ns.rm(` or + * `ns.promises.rm(` when `ns` is a namespace/default import from node:fs, fs, or + * their /promises forms. The same wrappers as the options argument are unwrapped + * around the callee and its object, and `?.` member access counts. * * Call, option, and import-binding detection is parser-backed (typescript-5): * only real node:fs(/promises) ImportDeclaration bindings count, only Node-bound @@ -257,20 +259,20 @@ const optionsFlags = (optionsArg) => { return { recursive, hasRetries }; }; -const isNodeBoundRmCall = (expression, bareNames, namespaceNames) => { +/** `fs.promises.rm` counts for any fs namespace; on a /promises namespace it is only an extra flag. */ +const isNodeBoundRmCall = (callee, bareNames, namespaceNames) => { + const expression = unwrapExpression(callee); 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) - && !identifierIsLocallyShadowed(expression.expression); + if (!ts.isPropertyAccessExpression(expression) || expression.name.text !== 'rm') return false; + let object = unwrapExpression(expression.expression); + if (ts.isPropertyAccessExpression(object) && object.name.text === 'promises') { + object = unwrapExpression(object.expression); } - return false; + return ts.isIdentifier(object) + && namespaceNames.has(object.text) + && !identifierIsLocallyShadowed(object); }; const scriptKindFor = (fileName) => {