From aeb8c3f1c3e7e8e1d097f7afa84b8985503d142c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 17 Sep 2026 05:15:21 +0000 Subject: [PATCH 1/3] fix(test): close remove-tree lint AST false positives and negatives Recognize shorthand maxRetries, shadowed local rm, named promises rebinds from fs/node:fs, and assertion/paren/satisfies option wrappers. Co-authored-by: Zack Jackson --- .../tests/check-test-remove-tree.test.ts | 81 ++++++++++++ scripts/check-test-remove-tree.d.mts | 2 +- scripts/check-test-remove-tree.mjs | 121 ++++++++++++++++-- 3 files changed, 192 insertions(+), 12 deletions(-) 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..93654527e 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,84 @@ 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([]); + + expect(recursiveRmCalls(sample([ + "import { rm } from 'node:fs';", + 'const rm = async () => undefined;', + `await rm(root, { ${recursiveTrue} });`, + ]))).toEqual([]); + expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([ + "import { rm } from 'node:fs';", + 'const rm = async () => undefined;', + `await rm(root, { ${recursiveTrue} });`, + ]))).toEqual([]); + + expect(recursiveRmCalls(sample([ + "import { rm } from 'node:fs';", + `await rm(root, { ${recursiveTrue} });`, + ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 2 })]); +}); + +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(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([ + "import { rm } from 'node:fs/promises';", + `await rm(root, { ${recursiveTrue}, maxRetries: 5 } as const);`, + ]))).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..42fdd6f4b 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,102 @@ const propertyName = (name) => { return undefined; }; +const unwrapExpression = (node) => { + let current = node; + while (current !== undefined) { + if ( + ts.isParenthesizedExpression(current) + || ts.isAsExpression(current) + || ts.isSatisfiesExpression(current) + || ts.isTypeAssertionExpression(current) + ) { + current = current.expression; + continue; + } + break; + } + 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) => { + if (ts.isOmittedExpression(element) || ts.isIdentifier(element)) { + return ts.isIdentifier(element) && element.text === name; + } + return ts.isBindingElement(element) && declarationNameIs(element.name, name); + }); + } + return false; +}; + +const statementDeclares = (statement, name) => { + if (ts.isVariableStatement(statement)) { + return statement.declarationList.declarations.some((decl) => declarationNameIs(decl.name, name)); + } + if (ts.isFunctionDeclaration(statement) || ts.isClassDeclaration(statement)) { + return statement.name !== undefined && statement.name.text === name; + } + return false; +}; + +const functionLikeDeclares = (node, name) => { + if ( + !( + ts.isFunctionDeclaration(node) + || ts.isFunctionExpression(node) + || ts.isArrowFunction(node) + || ts.isMethodDeclaration(node) + || ts.isConstructorDeclaration(node) + ) + ) { + return false; + } + if (ts.isFunctionDeclaration(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`. */ +const identifierIsLocallyShadowed = (identifier) => { + const name = identifier.text; + let current = identifier.parent; + while (current !== undefined) { + if (ts.isSourceFile(current) || ts.isBlock(current) || ts.isModuleBlock(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 (functionLikeDeclares(current, name)) return true; + if ( + ts.isCatchClause(current) + && current.variableDeclaration !== undefined + && declarationNameIs(current.variableDeclaration.name, name) + ) { + return true; + } + 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 +223,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; }; From 06f98d927bf432a3214da82691eefdcc48ee5b96 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Thu, 24 Sep 2026 23:30:30 +0000 Subject: [PATCH 2/3] fix(test): treat loop, case-clause, and named function expression bindings as remove-tree shadows --- .../tests/check-test-remove-tree.test.ts | 22 ++++++++ scripts/check-test-remove-tree.mjs | 50 ++++++++++++------- 2 files changed, 55 insertions(+), 17 deletions(-) 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 93654527e..9705f961b 100644 --- a/packages/agent-bundle/tests/check-test-remove-tree.test.ts +++ b/packages/agent-bundle/tests/check-test-remove-tree.test.ts @@ -270,6 +270,28 @@ it('accepts shorthand maxRetries and ignores a shadowed local rm', () => { ]))).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} });`, + `await fs.rm(root, { ${recursiveTrue} });`, + ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 5 })]); + + expect(recursiveRmCalls(sample([ + "import { rm } from 'node:fs/promises';", + 'switch (mode) {', + ' case 1:', + ' const rm = mockRm;', + ` 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: 9 })]); +}); + it('flags promises-namespace and asserted options without maxRetries', () => { expect(removalBindings(`import { promises as fs } from 'node:fs';`)).toEqual({ bareNames: new Set(), diff --git a/scripts/check-test-remove-tree.mjs b/scripts/check-test-remove-tree.mjs index 42fdd6f4b..a80ebaf3a 100644 --- a/scripts/check-test-remove-tree.mjs +++ b/scripts/check-test-remove-tree.mjs @@ -118,17 +118,16 @@ const propertyName = (name) => { const unwrapExpression = (node) => { let current = node; - while (current !== undefined) { - if ( + while ( + current !== undefined + && ( ts.isParenthesizedExpression(current) || ts.isAsExpression(current) || ts.isSatisfiesExpression(current) || ts.isTypeAssertionExpression(current) - ) { - current = current.expression; - continue; - } - break; + ) + ) { + current = current.expression; } return current; }; @@ -136,19 +135,18 @@ const unwrapExpression = (node) => { const declarationNameIs = (nameNode, name) => { if (ts.isIdentifier(nameNode)) return nameNode.text === name; if (ts.isObjectBindingPattern(nameNode) || ts.isArrayBindingPattern(nameNode)) { - return nameNode.elements.some((element) => { - if (ts.isOmittedExpression(element) || ts.isIdentifier(element)) { - return ts.isIdentifier(element) && element.text === name; - } - return ts.isBindingElement(element) && declarationNameIs(element.name, name); - }); + 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 statement.declarationList.declarations.some((decl) => declarationNameIs(decl.name, name)); + return declarationListDeclares(statement.declarationList, name); } if (ts.isFunctionDeclaration(statement) || ts.isClassDeclaration(statement)) { return statement.name !== undefined && statement.name.text === name; @@ -168,7 +166,11 @@ const functionLikeDeclares = (node, name) => { ) { return false; } - if (ts.isFunctionDeclaration(node) && node.name !== undefined && node.name.text === name) { + 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)); @@ -179,13 +181,27 @@ const identifierIsLocallyShadowed = (identifier) => { const name = identifier.text; let current = identifier.parent; while (current !== undefined) { - if (ts.isSourceFile(current) || ts.isBlock(current) || ts.isModuleBlock(current)) { + 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, name)) return true; if ( ts.isCatchClause(current) @@ -246,7 +262,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, From 7fde28243074bdb71734df8ad8c2608617d8acff Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Thu, 24 Sep 2026 23:39:52 +0000 Subject: [PATCH 3/3] fix(test): keep computed method keys and decorators outside parameter shadow scope --- .../tests/check-test-remove-tree.test.ts | 67 ++++++++++++++++--- scripts/check-test-remove-tree.mjs | 15 ++++- 2 files changed, 68 insertions(+), 14 deletions(-) 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 9705f961b..4f0d02119 100644 --- a/packages/agent-bundle/tests/check-test-remove-tree.test.ts +++ b/packages/agent-bundle/tests/check-test-remove-tree.test.ts @@ -253,16 +253,15 @@ it('accepts shorthand maxRetries and ignores a shadowed local rm', () => { `await rm(root, { ${recursiveTrue}, maxRetries });`, ]))).toEqual([]); - expect(recursiveRmCalls(sample([ - "import { rm } from 'node:fs';", - 'const rm = async () => undefined;', - `await rm(root, { ${recursiveTrue} });`, - ]))).toEqual([]); - expect(bareRecursiveRmFailures('packages/agent-bundle/tests/example.test.ts', sample([ + const shadowed = sample([ "import { rm } from 'node:fs';", - 'const rm = async () => undefined;', - `await rm(root, { ${recursiveTrue} });`, - ]))).toEqual([]); + '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';", @@ -276,8 +275,9 @@ it('ignores loop, switch-case, and named function expression shadows only in the `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: 5 })]); + ]))).toEqual([expect.objectContaining({ hasRetries: false, line: 6 })]); expect(recursiveRmCalls(sample([ "import { rm } from 'node:fs/promises';", @@ -285,11 +285,49 @@ it('ignores loop, switch-case, and named function expression shadows only in the ' 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: 9 })]); + ]))).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', () => { @@ -338,8 +376,15 @@ it('flags promises-namespace and asserted options without maxRetries', () => { "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.mjs b/scripts/check-test-remove-tree.mjs index a80ebaf3a..32bebb86c 100644 --- a/scripts/check-test-remove-tree.mjs +++ b/scripts/check-test-remove-tree.mjs @@ -154,7 +154,8 @@ const statementDeclares = (statement, name) => { return false; }; -const functionLikeDeclares = (node, name) => { +/** `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) @@ -166,6 +167,7 @@ const functionLikeDeclares = (node, name) => { ) { return false; } + if (from !== node.body && !node.parameters.includes(from)) return false; if ( (ts.isFunctionDeclaration(node) || ts.isFunctionExpression(node)) && node.name !== undefined @@ -176,9 +178,15 @@ const functionLikeDeclares = (node, name) => { return node.parameters.some((parameter) => declarationNameIs(parameter.name, name)); }; -/** True when a later/inner local binding hides the Node fs import of `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 ( @@ -202,7 +210,7 @@ const identifierIsLocallyShadowed = (identifier) => { ) { return true; } - if (functionLikeDeclares(current, name)) return true; + if (functionLikeDeclares(current, from, name)) return true; if ( ts.isCatchClause(current) && current.variableDeclaration !== undefined @@ -210,6 +218,7 @@ const identifierIsLocallyShadowed = (identifier) => { ) { return true; } + from = current; current = current.parent; } return false;