Skip to content

Commit ccccd79

Browse files
fix(test): flag fs.promises.rm and wrapped or optional-chained rm callees (#848)
* fix(test): flag fs.promises.rm and wrapped or optional-chained rm callees * docs(test): tighten remove-tree gate header wording
1 parent 3b667c8 commit ccccd79

2 files changed

Lines changed: 59 additions & 12 deletions

File tree

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,3 +439,48 @@ it('unwraps non-null asserted options, alone and nested in other wrappers', () =
439439
`await rm(root, ({ ${recursiveTrue}, maxRetries: 5 } as const)!);`,
440440
]))).toEqual([]);
441441
});
442+
443+
it('flags fs.promises.rm and wrapped callees or callee objects', () => {
444+
const flaggedLines = (count: number) => Array.from(
445+
{ length: count },
446+
(_, index) => expect.objectContaining({ hasRetries: false, line: index + 4 }),
447+
);
448+
449+
expect(recursiveRmCalls(sample([
450+
"import fs from 'node:fs';",
451+
"import * as nodeFs from 'node:fs';",
452+
"import fsp from 'node:fs/promises';",
453+
`await fs.promises.rm(root, { ${recursiveTrue} });`,
454+
`await nodeFs.promises.rm(root, { ${recursiveTrue} });`,
455+
`await fs.promises!.rm(root, { ${recursiveTrue} });`,
456+
`await (fs.promises as typeof fsp).rm(root, { ${recursiveTrue} });`,
457+
`await fsp.rm(root, { ${recursiveTrue} });`,
458+
]))).toEqual(flaggedLines(5));
459+
460+
expect(recursiveRmCalls(sample([
461+
"import { rm } from 'node:fs/promises';",
462+
"import * as fs from 'node:fs/promises';",
463+
'',
464+
`await (rm)(root, { ${recursiveTrue} });`,
465+
`await rm!(root, { ${recursiveTrue} });`,
466+
`await (rm as typeof rm)(root, { ${recursiveTrue} });`,
467+
`await (rm satisfies Remove)(root, { ${recursiveTrue} });`,
468+
`await (<Remove>rm)(root, { ${recursiveTrue} });`,
469+
`await ((rm)!)(root, { ${recursiveTrue} });`,
470+
`await fs!.rm(root, { ${recursiveTrue} });`,
471+
`await (fs as typeof fs).rm(root, { ${recursiveTrue} });`,
472+
`await (fs.rm)(root, { ${recursiveTrue} });`,
473+
`await fs?.rm(root, { ${recursiveTrue} });`,
474+
`await rm?.(root, { ${recursiveTrue} });`,
475+
]))).toEqual(flaggedLines(11));
476+
477+
expect(recursiveRmCalls(sample([
478+
"import fs from 'node:fs';",
479+
"import { rm } from 'node:fs/promises';",
480+
`function cleanup(fs) { return fs.promises.rm(root, { ${recursiveTrue} }); }`,
481+
`{ const rm = mockRm; await (rm)!(root, { ${recursiveTrue} }); }`,
482+
`const reset = (fs) => (fs as Mock)!.rm(root, { ${recursiveTrue} });`,
483+
`await other.promises.rm(root, { ${recursiveTrue} });`,
484+
`await fs.other.rm(root, { ${recursiveTrue} });`,
485+
]))).toEqual([]);
486+
});

‎scripts/check-test-remove-tree.mjs‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
* Test teardown that deletes a tree calls `removeTree`. A bare `rm` with
33
* `recursive: true` and no `maxRetries` races a late writer and flakes with ENOTEMPTY.
44
*
5-
* Catches bare `rm(`, aliased `import { rm as remove }` calls, and `ns.rm(` when
6-
* `ns` is a namespace/default import from node:fs, fs, or their /promises forms.
5+
* Catches bare `rm(`, aliased `import { rm as remove }` calls, and `ns.rm(` or
6+
* `ns.promises.rm(` when `ns` is a namespace/default import from node:fs, fs, or
7+
* their /promises forms. The same wrappers as the options argument are unwrapped
8+
* around the callee and its object, and `?.` member access counts.
79
*
810
* Call, option, and import-binding detection is parser-backed (typescript-5):
911
* only real node:fs(/promises) ImportDeclaration bindings count, only Node-bound
@@ -257,20 +259,20 @@ const optionsFlags = (optionsArg) => {
257259
return { recursive, hasRetries };
258260
};
259261

260-
const isNodeBoundRmCall = (expression, bareNames, namespaceNames) => {
262+
/** `fs.promises.rm` counts for any fs namespace; on a /promises namespace it is only an extra flag. */
263+
const isNodeBoundRmCall = (callee, bareNames, namespaceNames) => {
264+
const expression = unwrapExpression(callee);
261265
if (ts.isIdentifier(expression)) {
262266
return bareNames.has(expression.text) && !identifierIsLocallyShadowed(expression);
263267
}
264-
if (
265-
ts.isPropertyAccessExpression(expression)
266-
&& !expression.questionDotToken
267-
&& expression.name.text === 'rm'
268-
&& ts.isIdentifier(expression.expression)
269-
) {
270-
return namespaceNames.has(expression.expression.text)
271-
&& !identifierIsLocallyShadowed(expression.expression);
268+
if (!ts.isPropertyAccessExpression(expression) || expression.name.text !== 'rm') return false;
269+
let object = unwrapExpression(expression.expression);
270+
if (ts.isPropertyAccessExpression(object) && object.name.text === 'promises') {
271+
object = unwrapExpression(object.expression);
272272
}
273-
return false;
273+
return ts.isIdentifier(object)
274+
&& namespaceNames.has(object.text)
275+
&& !identifierIsLocallyShadowed(object);
274276
};
275277

276278
const scriptKindFor = (fileName) => {

0 commit comments

Comments
 (0)