Skip to content

Commit

Permalink
perf: use Set instead of iterating, and deduplicate a function (#1278)
Browse files Browse the repository at this point in the history
* perf: use `Set` instead of iterating

* test: add case for default import

* test: add case for async function call

* test: add case for `TSImportEqualsDeclaration` import

* refactor: replace `scopeHasLocalReference` with `resolveScope`
  • Loading branch information
G-Rath authored Nov 10, 2022
1 parent ad04fcc commit 0e048f1
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 44 deletions.
4 changes: 2 additions & 2 deletions src/rules/no-disabled-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
createRule,
getAccessorValue,
parseJestFnCall,
scopeHasLocalReference,
resolveScope,
} from './utils';

export default createRule({
Expand Down Expand Up @@ -80,7 +80,7 @@ export default createRule({
}
},
'CallExpression[callee.name="pending"]'(node) {
if (scopeHasLocalReference(context.getScope(), 'pending')) {
if (resolveScope(context.getScope(), 'pending')) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/rules/no-jasmine-globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
createRule,
getNodeName,
isSupportedAccessor,
scopeHasLocalReference,
resolveScope,
} from './utils';

export default createRule({
Expand Down Expand Up @@ -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;
}
Expand Down
25 changes: 25 additions & 0 deletions src/rules/utils/__tests__/parseJestFnCall.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,23 @@ ruleTester.run('esm', rule, {
`,
parserOptions: { sourceType: 'module' },
},
{
code: dedent`
import ByDefault from './myfile';
ByDefault.sayHello();
`,
parserOptions: { sourceType: 'module' },
},
{
code: dedent`
async function doSomething() {
const build = await rollup(config);
build.generate();
}
`,
parserOptions: { sourceType: 'module', ecmaVersion: 2017 },
},
],
invalid: [],
});
Expand Down Expand Up @@ -782,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: [
{
Expand Down
48 changes: 8 additions & 40 deletions src/rules/utils/parseJestFnCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ const findImportSourceNode = (
): 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;
return node.argument.source;
}

return null;
Expand Down Expand Up @@ -514,15 +514,16 @@ 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) {
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);
Expand All @@ -531,9 +532,7 @@ const resolveScope = (scope: TSESLint.Scope.Scope, identifier: string) => {
return importDetails;
}

if (ref.name === identifier) {
return 'local';
}
return 'local';
}

currentScope = currentScope.upper;
Expand Down Expand Up @@ -580,34 +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) {
for (const ref of currentScope.variables) {
if (ref.defs.length === 0) {
continue;
}

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;
};

0 comments on commit 0e048f1

Please sign in to comment.