Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions packages/agent-bundle/tests/check-test-remove-tree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 (<Remove>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([]);
});
26 changes: 14 additions & 12 deletions scripts/check-test-remove-tree.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) => {
Expand Down
Loading