From ce0652ddd9894c37ec47e1e04a291a77af5ce0ac Mon Sep 17 00:00:00 2001 From: eight Date: Fri, 23 Oct 2020 04:52:55 +0800 Subject: [PATCH] fix(pluginutils): attach scope object to for-loop (#616) * fix(pluginutils): attach scope object to for-loop * fix: types, add estree to devdependencies * refactor: move estree import to the top --- packages/pluginutils/package.json | 2 +- packages/pluginutils/src/attachScopes.ts | 31 +++++---- packages/pluginutils/test/attachScopes.ts | 78 ++++++++++++++++++++++- pnpm-lock.yaml | 2 +- 4 files changed, 98 insertions(+), 15 deletions(-) diff --git a/packages/pluginutils/package.json b/packages/pluginutils/package.json index 3e2a4c01c..662c3111d 100644 --- a/packages/pluginutils/package.json +++ b/packages/pluginutils/package.json @@ -48,7 +48,6 @@ "rollup": "^1.20.0||^2.0.0" }, "dependencies": { - "@types/estree": "0.0.45", "estree-walker": "^2.0.1", "picomatch": "^2.2.2" }, @@ -56,6 +55,7 @@ "@rollup/plugin-commonjs": "^14.0.0", "@rollup/plugin-node-resolve": "^8.4.0", "@rollup/plugin-typescript": "^5.0.2", + "@types/estree": "0.0.45", "@types/node": "^14.0.26", "@types/picomatch": "^2.2.1", "acorn": "^8.0.4", diff --git a/packages/pluginutils/src/attachScopes.ts b/packages/pluginutils/src/attachScopes.ts index d48b90a39..d5a962964 100755 --- a/packages/pluginutils/src/attachScopes.ts +++ b/packages/pluginutils/src/attachScopes.ts @@ -1,3 +1,6 @@ +// eslint-disable-next-line import/no-unresolved +import * as estree from 'estree'; + import { walk } from 'estree-walker'; import { AttachedScope, AttachScopes } from '../types'; @@ -16,7 +19,7 @@ const blockDeclarations: BlockDeclaration = { interface ScopeOptions { parent?: AttachedScope; block?: boolean; - params?: import('estree').Node[]; + params?: estree.Node[]; } class Scope implements AttachedScope { @@ -39,7 +42,7 @@ class Scope implements AttachedScope { } } - addDeclaration(node: import('estree').Node, isBlockDeclaration: boolean, isVar: boolean): void { + addDeclaration(node: estree.Node, isBlockDeclaration: boolean, isVar: boolean): void { if (!isBlockDeclaration && this.isBlockScope) { // it's a `var` or function node, and this // is a block scope, so we need to go up @@ -61,7 +64,7 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc walk(ast, { enter(n, parent) { - const node = n as import('estree').Node; + const node = n as estree.Node; // function foo () {...} // class Foo {...} if (/(Function|Class)Declaration/.test(node.type)) { @@ -72,20 +75,16 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc if (node.type === 'VariableDeclaration') { const { kind } = node; const isBlockDeclaration = blockDeclarations[kind]; - // don't add const/let declarations in the body of a for loop #113 - const parentType = parent ? parent.type : ''; - if (!(isBlockDeclaration && /ForOfStatement/.test(parentType))) { - node.declarations.forEach((declaration) => { - scope.addDeclaration(declaration, isBlockDeclaration, true); - }); - } + node.declarations.forEach((declaration) => { + scope.addDeclaration(declaration, isBlockDeclaration, true); + }); } let newScope: AttachedScope | undefined; // create new function scope if (/Function/.test(node.type)) { - const func = node as import('estree').Function; + const func = node as estree.Function; newScope = new Scope({ parent: scope, block: false, @@ -99,6 +98,14 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc } } + // create new for scope + if (/For(In|Of)?Statement/.test(node.type)) { + newScope = new Scope({ + parent: scope, + block: true + }); + } + // create new block scope if (node.type === 'BlockStatement' && !/Function/.test(parent.type)) { newScope = new Scope({ @@ -126,7 +133,7 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc } }, leave(n) { - const node = n as import('estree').Node & Record; + const node = n as estree.Node & Record; if (node[propertyName]) scope = scope.parent!; } }); diff --git a/packages/pluginutils/test/attachScopes.ts b/packages/pluginutils/test/attachScopes.ts index a67cbcf06..8cb621fe2 100755 --- a/packages/pluginutils/test/attachScopes.ts +++ b/packages/pluginutils/test/attachScopes.ts @@ -1,7 +1,10 @@ +// eslint-disable-next-line import/no-unresolved +import * as estree from 'estree'; + import test from 'ava'; import { parse } from 'acorn'; -import { attachScopes } from '../'; +import { attachScopes, AttachedScope } from '../'; test('attaches a scope to the top level', (t) => { const ast = parse('var foo;', { ecmaVersion: 2020, sourceType: 'module' }); @@ -70,3 +73,76 @@ test('supports catch without a parameter', (t) => { attachScopes(ast, 'scope'); }); }); + +test('supports ForStatement', (t) => { + const ast = (parse( + ` + for (let a = 0; a < 10; a++) { + console.log(a); + let b = 10; + } + `, + { ecmaVersion: 2020, sourceType: 'module' } + ) as unknown) as estree.Program; + + const scope = attachScopes(ast, 'scope'); + t.falsy(scope.contains('a')); + t.falsy(scope.contains('b')); + + const forLoop = ast.body[0] as estree.ForStatement & { scope: AttachedScope }; + + t.truthy(forLoop.scope.contains('a')); + t.falsy(forLoop.scope.contains('b')); + + const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope }; + t.truthy(forBody.scope.contains('a')); + t.truthy(forBody.scope.contains('b')); +}); + +test('supports ForOfStatement', (t) => { + const ast = (parse( + ` + for (const a of [1, 2, 3]) { + console.log(a); + let b = 10; + } + `, + { ecmaVersion: 2020, sourceType: 'module' } + ) as unknown) as estree.Program; + + const scope = attachScopes(ast, 'scope'); + t.falsy(scope.contains('a')); + t.falsy(scope.contains('b')); + + const forLoop = ast.body[0] as estree.ForOfStatement & { scope: AttachedScope }; + t.truthy(forLoop.scope.contains('a')); + t.falsy(forLoop.scope.contains('b')); + + const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope }; + t.truthy(forBody.scope.contains('a')); + t.truthy(forBody.scope.contains('b')); +}); + +test('supports ForInStatement', (t) => { + const ast = (parse( + ` + for (let a in [1, 2, 3]) { + console.log(a); + let b = 10; + } + `, + { ecmaVersion: 2020, sourceType: 'module' } + ) as unknown) as estree.Program; + + const scope = attachScopes(ast, 'scope'); + t.falsy(scope.contains('a')); + t.falsy(scope.contains('b')); + + const forLoop = ast.body[0] as estree.ForInStatement & { scope: AttachedScope }; + t.truthy(forLoop.scope.contains('a')); + t.falsy(forLoop.scope.contains('b')); + + const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope }; + t.truthy(forBody.scope.contains('a')); + t.truthy(forBody.scope.contains('b')); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0a6af048d..cbebb4906 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -322,13 +322,13 @@ importers: string-capitalize: ^1.0.1 packages/pluginutils: dependencies: - '@types/estree': 0.0.45 estree-walker: 2.0.1 picomatch: 2.2.2 devDependencies: '@rollup/plugin-commonjs': 14.0.0_rollup@2.23.0 '@rollup/plugin-node-resolve': 8.4.0_rollup@2.23.0 '@rollup/plugin-typescript': 5.0.2_rollup@2.23.0 + '@types/estree': 0.0.45 '@types/node': 14.0.26 '@types/picomatch': 2.2.1 acorn: 8.0.4