From 606418925dcf79a63cec38b28f4beca3e5e764ed Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 6 Nov 2022 07:56:55 +1300 Subject: [PATCH 1/5] perf: use `Set` instead of iterating --- src/rules/utils/parseJestFnCall.ts | 35 ++++++++++++++++-------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/rules/utils/parseJestFnCall.ts b/src/rules/utils/parseJestFnCall.ts index 0430b29d7..618a01610 100644 --- a/src/rules/utils/parseJestFnCall.ts +++ b/src/rules/utils/parseJestFnCall.ts @@ -415,12 +415,18 @@ interface ImportDetails { const describeImportDefAsImport = ( def: TSESLint.Scope.Definitions.ImportBindingDefinition, ): ImportDetails | null => { + /* istanbul ignore if */ if (def.parent.type === AST_NODE_TYPES.TSImportEqualsDeclaration) { - return null; + throw new Error( + `Did not expect a ${def.parent.type} here - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, + ); } + /* istanbul ignore if */ if (def.node.type !== AST_NODE_TYPES.ImportSpecifier) { - return null; + throw new Error( + `Did not expect a ${def.node.type} here - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, + ); } // we only care about value imports @@ -446,11 +452,14 @@ const findImportSourceNode = ( node: TSESTree.Expression, ): TSESTree.Node | null => { if (node.type === AST_NODE_TYPES.AwaitExpression) { - if (node.argument.type === AST_NODE_TYPES.ImportExpression) { - return (node.argument as TSESTree.ImportExpression).source; + /* istanbul ignore if */ + if (node.argument.type !== AST_NODE_TYPES.ImportExpression) { + throw new Error( + `Did not expect a ${node.argument.type} here - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, + ); } - return null; + return node.argument.source; } if ( @@ -518,11 +527,9 @@ const resolveScope = (scope: TSESLint.Scope.Scope, identifier: string) => { let currentScope: TSESLint.Scope.Scope | null = scope; while (currentScope !== null) { - for (const ref of currentScope.variables) { - if (ref.defs.length === 0) { - continue; - } + const ref = currentScope.set.get(identifier); + if (ref && ref.defs.length > 0) { const def = ref.defs[ref.defs.length - 1]; const importDetails = describePossibleImportDef(def); @@ -531,9 +538,7 @@ const resolveScope = (scope: TSESLint.Scope.Scope, identifier: string) => { return importDetails; } - if (ref.name === identifier) { - return 'local'; - } + return 'local'; } currentScope = currentScope.upper; @@ -588,11 +593,9 @@ export const scopeHasLocalReference = ( let currentScope: TSESLint.Scope.Scope | null = scope; while (currentScope !== null) { - for (const ref of currentScope.variables) { - if (ref.defs.length === 0) { - continue; - } + const ref = currentScope.set.get(referenceName); + if (ref && ref.defs.length > 0) { const def = ref.defs[ref.defs.length - 1]; const importDetails = describePossibleImportDef(def); From 29e111dd282cb9b9f34f04b593c368f8bc0472d3 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 6 Nov 2022 08:56:46 +1300 Subject: [PATCH 2/5] test: add case for default import --- src/rules/utils/__tests__/parseJestFnCall.test.ts | 8 ++++++++ src/rules/utils/parseJestFnCall.ts | 5 +---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/rules/utils/__tests__/parseJestFnCall.test.ts b/src/rules/utils/__tests__/parseJestFnCall.test.ts index e4526b791..9e9bf2815 100644 --- a/src/rules/utils/__tests__/parseJestFnCall.test.ts +++ b/src/rules/utils/__tests__/parseJestFnCall.test.ts @@ -441,6 +441,14 @@ ruleTester.run('esm', rule, { `, parserOptions: { sourceType: 'module' }, }, + { + code: dedent` + import ByDefault from './myfile'; + + ByDefault.sayHello(); + `, + parserOptions: { sourceType: 'module' }, + }, ], invalid: [], }); diff --git a/src/rules/utils/parseJestFnCall.ts b/src/rules/utils/parseJestFnCall.ts index 618a01610..88e92031d 100644 --- a/src/rules/utils/parseJestFnCall.ts +++ b/src/rules/utils/parseJestFnCall.ts @@ -422,11 +422,8 @@ const describeImportDefAsImport = ( ); } - /* istanbul ignore if */ if (def.node.type !== AST_NODE_TYPES.ImportSpecifier) { - throw new Error( - `Did not expect a ${def.node.type} here - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, - ); + return null; } // we only care about value imports From 0b4d6a2ed31ffa3bf246026816a4b910966736b7 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 6 Nov 2022 09:14:59 +1300 Subject: [PATCH 3/5] test: add case for async function call --- src/rules/utils/__tests__/parseJestFnCall.test.ts | 9 +++++++++ src/rules/utils/parseJestFnCall.ts | 9 +++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/rules/utils/__tests__/parseJestFnCall.test.ts b/src/rules/utils/__tests__/parseJestFnCall.test.ts index 9e9bf2815..f9b8d8113 100644 --- a/src/rules/utils/__tests__/parseJestFnCall.test.ts +++ b/src/rules/utils/__tests__/parseJestFnCall.test.ts @@ -449,6 +449,15 @@ ruleTester.run('esm', rule, { `, parserOptions: { sourceType: 'module' }, }, + { + code: dedent` + async function doSomething() { + const build = await rollup(config); + build.generate(); + } + `, + parserOptions: { sourceType: 'module', ecmaVersion: 2017 }, + }, ], invalid: [], }); diff --git a/src/rules/utils/parseJestFnCall.ts b/src/rules/utils/parseJestFnCall.ts index 88e92031d..cb670a482 100644 --- a/src/rules/utils/parseJestFnCall.ts +++ b/src/rules/utils/parseJestFnCall.ts @@ -449,14 +449,11 @@ const findImportSourceNode = ( node: TSESTree.Expression, ): TSESTree.Node | null => { if (node.type === AST_NODE_TYPES.AwaitExpression) { - /* istanbul ignore if */ - if (node.argument.type !== AST_NODE_TYPES.ImportExpression) { - throw new Error( - `Did not expect a ${node.argument.type} here - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, - ); + if (node.argument.type === AST_NODE_TYPES.ImportExpression) { + return node.argument.source; } - return node.argument.source; + return null; } if ( From d13568a1f3f05d472dfc2f2e01b02b6d0fba86b8 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 6 Nov 2022 09:18:28 +1300 Subject: [PATCH 4/5] test: add case for `TSImportEqualsDeclaration` import --- src/rules/utils/__tests__/parseJestFnCall.test.ts | 8 ++++++++ src/rules/utils/parseJestFnCall.ts | 5 +---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/rules/utils/__tests__/parseJestFnCall.test.ts b/src/rules/utils/__tests__/parseJestFnCall.test.ts index f9b8d8113..05a15ba0c 100644 --- a/src/rules/utils/__tests__/parseJestFnCall.test.ts +++ b/src/rules/utils/__tests__/parseJestFnCall.test.ts @@ -799,6 +799,14 @@ ruleTester.run('typescript', rule, { parser: require.resolve('@typescript-eslint/parser'), parserOptions: { sourceType: 'module' }, }, + { + code: dedent` + import dedent = require('dedent'); + + dedent(); + `, + parser: require.resolve('@typescript-eslint/parser'), + }, ], invalid: [ { diff --git a/src/rules/utils/parseJestFnCall.ts b/src/rules/utils/parseJestFnCall.ts index cb670a482..c0d49f751 100644 --- a/src/rules/utils/parseJestFnCall.ts +++ b/src/rules/utils/parseJestFnCall.ts @@ -415,11 +415,8 @@ interface ImportDetails { const describeImportDefAsImport = ( def: TSESLint.Scope.Definitions.ImportBindingDefinition, ): ImportDetails | null => { - /* istanbul ignore if */ if (def.parent.type === AST_NODE_TYPES.TSImportEqualsDeclaration) { - throw new Error( - `Did not expect a ${def.parent.type} here - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, - ); + return null; } if (def.node.type !== AST_NODE_TYPES.ImportSpecifier) { From 16cf7210506d4a2bbc8ca1a0993e887d8511b0f4 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 6 Nov 2022 09:27:02 +1300 Subject: [PATCH 5/5] refactor: replace `scopeHasLocalReference` with `resolveScope` --- src/rules/no-disabled-tests.ts | 4 ++-- src/rules/no-jasmine-globals.ts | 4 ++-- src/rules/utils/parseJestFnCall.ts | 34 ++++-------------------------- 3 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/rules/no-disabled-tests.ts b/src/rules/no-disabled-tests.ts index f4300659e..cb8d0c047 100644 --- a/src/rules/no-disabled-tests.ts +++ b/src/rules/no-disabled-tests.ts @@ -2,7 +2,7 @@ import { createRule, getAccessorValue, parseJestFnCall, - scopeHasLocalReference, + resolveScope, } from './utils'; export default createRule({ @@ -80,7 +80,7 @@ export default createRule({ } }, 'CallExpression[callee.name="pending"]'(node) { - if (scopeHasLocalReference(context.getScope(), 'pending')) { + if (resolveScope(context.getScope(), 'pending')) { return; } diff --git a/src/rules/no-jasmine-globals.ts b/src/rules/no-jasmine-globals.ts index f5206ec49..adf34604a 100644 --- a/src/rules/no-jasmine-globals.ts +++ b/src/rules/no-jasmine-globals.ts @@ -3,7 +3,7 @@ import { createRule, getNodeName, isSupportedAccessor, - scopeHasLocalReference, + resolveScope, } from './utils'; export default createRule({ @@ -46,7 +46,7 @@ export default createRule({ calleeName === 'fail' || calleeName === 'pending' ) { - if (scopeHasLocalReference(context.getScope(), calleeName)) { + if (resolveScope(context.getScope(), calleeName)) { // It's a local variable, not a jasmine global. return; } diff --git a/src/rules/utils/parseJestFnCall.ts b/src/rules/utils/parseJestFnCall.ts index c0d49f751..dffe4ae5f 100644 --- a/src/rules/utils/parseJestFnCall.ts +++ b/src/rules/utils/parseJestFnCall.ts @@ -514,7 +514,10 @@ const describePossibleImportDef = (def: TSESLint.Scope.Definition) => { return null; }; -const resolveScope = (scope: TSESLint.Scope.Scope, identifier: string) => { +export const resolveScope = ( + scope: TSESLint.Scope.Scope, + identifier: string, +): ImportDetails | 'local' | null => { let currentScope: TSESLint.Scope.Scope | null = scope; while (currentScope !== null) { @@ -576,32 +579,3 @@ const resolveToJestFn = ( type: 'global', }; }; - -export const scopeHasLocalReference = ( - scope: TSESLint.Scope.Scope, - referenceName: string, -) => { - let currentScope: TSESLint.Scope.Scope | null = scope; - - while (currentScope !== null) { - const ref = currentScope.set.get(referenceName); - - if (ref && ref.defs.length > 0) { - const def = ref.defs[ref.defs.length - 1]; - - const importDetails = describePossibleImportDef(def); - - // referenceName was found as an imported identifier - if (importDetails?.local === referenceName) { - return true; - } - - // referenceName was found as a local variable or function declaration. - return ref.name === referenceName; - } - - currentScope = currentScope.upper; - } - - return false; -};