From 7271e4556e54bd0520451804f9242fefd4a5aec4 Mon Sep 17 00:00:00 2001 From: eight04 Date: Thu, 22 Oct 2020 09:31:36 +0800 Subject: [PATCH 1/3] fix(pluginutils): attach scope object to for-loop --- packages/pluginutils/src/attachScopes.ts | 18 +++--- packages/pluginutils/test/attachScopes.ts | 73 +++++++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/packages/pluginutils/src/attachScopes.ts b/packages/pluginutils/src/attachScopes.ts index d48b90a39..e1dc15515 100755 --- a/packages/pluginutils/src/attachScopes.ts +++ b/packages/pluginutils/src/attachScopes.ts @@ -72,13 +72,9 @@ 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; @@ -99,6 +95,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({ diff --git a/packages/pluginutils/test/attachScopes.ts b/packages/pluginutils/test/attachScopes.ts index a67cbcf06..f71a36aab 100755 --- a/packages/pluginutils/test/attachScopes.ts +++ b/packages/pluginutils/test/attachScopes.ts @@ -70,3 +70,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 import('estree').Program; + + const scope = attachScopes(ast, 'scope'); + t.falsy(scope.contains('a')); + t.falsy(scope.contains('b')); + + const forLoop = ast.body[0] as import('estree').ForStatement & Record; + + t.truthy(forLoop.scope.contains('a')); + t.falsy(forLoop.scope.contains('b')); + + const forBody = forLoop.body as import('estree').BlockStatement & Record; + 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 import('estree').Program; + + const scope = attachScopes(ast, 'scope'); + t.falsy(scope.contains('a')); + t.falsy(scope.contains('b')); + + const forLoop = ast.body[0] as import('estree').ForOfStatement & Record; + t.truthy(forLoop.scope.contains('a')); + t.falsy(forLoop.scope.contains('b')); + + const forBody = forLoop.body as import('estree').BlockStatement & Record; + 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 import('estree').Program; + + const scope = attachScopes(ast, 'scope'); + t.falsy(scope.contains('a')); + t.falsy(scope.contains('b')); + + const forLoop = ast.body[0] as import('estree').ForInStatement & Record; + t.truthy(forLoop.scope.contains('a')); + t.falsy(forLoop.scope.contains('b')); + + const forBody = forLoop.body as import('estree').BlockStatement & Record; + t.truthy(forBody.scope.contains('a')); + t.truthy(forBody.scope.contains('b')); +}); From 0932f32efbc240edd346916d4bb71f9279d1cef6 Mon Sep 17 00:00:00 2001 From: eight04 Date: Thu, 22 Oct 2020 18:55:23 +0800 Subject: [PATCH 2/3] fix: types, add estree to devdependencies --- packages/pluginutils/package.json | 2 +- packages/pluginutils/test/attachScopes.ts | 23 +++++++++++++---------- pnpm-lock.yaml | 2 +- 3 files changed, 15 insertions(+), 12 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/test/attachScopes.ts b/packages/pluginutils/test/attachScopes.ts index f71a36aab..2671d7394 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 { Program, ForStatement, ForOfStatement, ForInStatement, BlockStatement } 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' }); @@ -80,18 +83,18 @@ test('supports ForStatement', (t) => { } `, { ecmaVersion: 2020, sourceType: 'module' } - ) as unknown) as import('estree').Program; + ) as unknown) as Program; const scope = attachScopes(ast, 'scope'); t.falsy(scope.contains('a')); t.falsy(scope.contains('b')); - const forLoop = ast.body[0] as import('estree').ForStatement & Record; + const forLoop = ast.body[0] as ForStatement & { scope: AttachedScope }; t.truthy(forLoop.scope.contains('a')); t.falsy(forLoop.scope.contains('b')); - const forBody = forLoop.body as import('estree').BlockStatement & Record; + const forBody = forLoop.body as BlockStatement & { scope: AttachedScope }; t.truthy(forBody.scope.contains('a')); t.truthy(forBody.scope.contains('b')); }); @@ -105,17 +108,17 @@ test('supports ForOfStatement', (t) => { } `, { ecmaVersion: 2020, sourceType: 'module' } - ) as unknown) as import('estree').Program; + ) as unknown) as Program; const scope = attachScopes(ast, 'scope'); t.falsy(scope.contains('a')); t.falsy(scope.contains('b')); - const forLoop = ast.body[0] as import('estree').ForOfStatement & Record; + const forLoop = ast.body[0] as ForOfStatement & { scope: AttachedScope }; t.truthy(forLoop.scope.contains('a')); t.falsy(forLoop.scope.contains('b')); - const forBody = forLoop.body as import('estree').BlockStatement & Record; + const forBody = forLoop.body as BlockStatement & { scope: AttachedScope }; t.truthy(forBody.scope.contains('a')); t.truthy(forBody.scope.contains('b')); }); @@ -129,17 +132,17 @@ test('supports ForInStatement', (t) => { } `, { ecmaVersion: 2020, sourceType: 'module' } - ) as unknown) as import('estree').Program; + ) as unknown) as Program; const scope = attachScopes(ast, 'scope'); t.falsy(scope.contains('a')); t.falsy(scope.contains('b')); - const forLoop = ast.body[0] as import('estree').ForInStatement & Record; + const forLoop = ast.body[0] as ForInStatement & { scope: AttachedScope }; t.truthy(forLoop.scope.contains('a')); t.falsy(forLoop.scope.contains('b')); - const forBody = forLoop.body as import('estree').BlockStatement & Record; + const forBody = forLoop.body as 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 From cc8f511e9b3b2fb32dbfe84ef00fc0c3b768fb05 Mon Sep 17 00:00:00 2001 From: eight04 Date: Thu, 22 Oct 2020 20:50:26 +0800 Subject: [PATCH 3/3] refactor: move estree import to the top --- packages/pluginutils/src/attachScopes.ts | 13 ++++++++----- packages/pluginutils/test/attachScopes.ts | 20 ++++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/pluginutils/src/attachScopes.ts b/packages/pluginutils/src/attachScopes.ts index e1dc15515..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)) { @@ -81,7 +84,7 @@ const attachScopes: AttachScopes = function attachScopes(ast, propertyName = 'sc // 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, @@ -130,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 2671d7394..8cb621fe2 100755 --- a/packages/pluginutils/test/attachScopes.ts +++ b/packages/pluginutils/test/attachScopes.ts @@ -1,5 +1,5 @@ // eslint-disable-next-line import/no-unresolved -import { Program, ForStatement, ForOfStatement, ForInStatement, BlockStatement } from 'estree'; +import * as estree from 'estree'; import test from 'ava'; import { parse } from 'acorn'; @@ -83,18 +83,18 @@ test('supports ForStatement', (t) => { } `, { ecmaVersion: 2020, sourceType: 'module' } - ) as unknown) as Program; + ) 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 ForStatement & { scope: AttachedScope }; + 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 BlockStatement & { scope: AttachedScope }; + const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope }; t.truthy(forBody.scope.contains('a')); t.truthy(forBody.scope.contains('b')); }); @@ -108,17 +108,17 @@ test('supports ForOfStatement', (t) => { } `, { ecmaVersion: 2020, sourceType: 'module' } - ) as unknown) as Program; + ) 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 ForOfStatement & { scope: AttachedScope }; + 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 BlockStatement & { scope: AttachedScope }; + const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope }; t.truthy(forBody.scope.contains('a')); t.truthy(forBody.scope.contains('b')); }); @@ -132,17 +132,17 @@ test('supports ForInStatement', (t) => { } `, { ecmaVersion: 2020, sourceType: 'module' } - ) as unknown) as Program; + ) 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 ForInStatement & { scope: AttachedScope }; + 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 BlockStatement & { scope: AttachedScope }; + const forBody = forLoop.body as estree.BlockStatement & { scope: AttachedScope }; t.truthy(forBody.scope.contains('a')); t.truthy(forBody.scope.contains('b')); });