From c4c0550c95336c8acf530c8c90ef0f57a5eb7340 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 29 Jul 2024 09:40:45 +0930 Subject: [PATCH 1/2] fix(eslint-plugin): [no-misused-promises] perf: avoid getting types of variables/functions if the annotated type is obviously not a function --- eslint.config.mjs | 8 +- .../src/rules/no-misused-promises.ts | 105 +++++++++++++++++- .../tests/rules/no-misused-promises.test.ts | 8 ++ 3 files changed, 115 insertions(+), 6 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 67bab36e44bb..60e9c46ae008 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -27,6 +27,7 @@ export default tseslint.config( // register all of the plugins up-front { // note - intentionally uses computed syntax to make it easy to sort the keys + /* eslint-disable no-useless-computed-key */ plugins: { ['@typescript-eslint']: tseslint.plugin, ['@typescript-eslint/internal']: tseslintInternalPlugin, @@ -47,10 +48,13 @@ export default tseslint.config( ['simple-import-sort']: simpleImportSortPlugin, ['unicorn']: unicornPlugin, }, + /* eslint-enable no-useless-computed-key */ }, { // config with just ignores is the replacement for `.eslintignore` ignores: [ + '.nx/', + '.yarn', '**/jest.config.js', '**/node_modules/**', '**/dist/**', @@ -62,9 +66,9 @@ export default tseslint.config( // Files copied as part of the build 'packages/types/src/generated/**/*.ts', // Playground types downloaded from the web - 'packages/website/src/vendor', + 'packages/website/src/vendor/', // see the file header in eslint-base.test.js for more info - 'packages/rule-tester/tests/eslint-base', + 'packages/rule-tester/tests/eslint-base/', ], }, diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 1a58d884dc7c..46d3d9759b1c 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -6,7 +6,10 @@ import * as ts from 'typescript'; import { createRule, getParserServices, + isFunction, isRestParameterDeclaration, + nullThrows, + NullThrowsReasons, } from '../util'; type Options = [ @@ -170,9 +173,77 @@ export default createRule({ SpreadElement: checkSpread, }; - function checkTestConditional(node: { - test: TSESTree.Expression | null; - }): void { + /** + * A syntactic check to see if an annotated type is maybe a function type. + * This is a perf optimization to help avoid requesting types where possible + */ + function isPossiblyFunctionType(node: TSESTree.TSTypeAnnotation): boolean { + switch (node.typeAnnotation.type) { + case AST_NODE_TYPES.TSConditionalType: + case AST_NODE_TYPES.TSConstructorType: + case AST_NODE_TYPES.TSFunctionType: + case AST_NODE_TYPES.TSImportType: + case AST_NODE_TYPES.TSIndexedAccessType: + case AST_NODE_TYPES.TSInferType: + case AST_NODE_TYPES.TSIntersectionType: + case AST_NODE_TYPES.TSQualifiedName: + case AST_NODE_TYPES.TSThisType: + case AST_NODE_TYPES.TSTypeOperator: + case AST_NODE_TYPES.TSTypeQuery: + case AST_NODE_TYPES.TSTypeReference: + case AST_NODE_TYPES.TSUnionType: + return true; + + case AST_NODE_TYPES.TSTypeLiteral: + return node.typeAnnotation.members.some( + member => + member.type === AST_NODE_TYPES.TSCallSignatureDeclaration || + member.type === AST_NODE_TYPES.TSConstructSignatureDeclaration, + ); + + case AST_NODE_TYPES.TSAbstractKeyword: + case AST_NODE_TYPES.TSAnyKeyword: + case AST_NODE_TYPES.TSArrayType: + case AST_NODE_TYPES.TSAsyncKeyword: + case AST_NODE_TYPES.TSBigIntKeyword: + case AST_NODE_TYPES.TSBooleanKeyword: + case AST_NODE_TYPES.TSDeclareKeyword: + case AST_NODE_TYPES.TSExportKeyword: + case AST_NODE_TYPES.TSIntrinsicKeyword: + case AST_NODE_TYPES.TSLiteralType: + case AST_NODE_TYPES.TSMappedType: + case AST_NODE_TYPES.TSNamedTupleMember: + case AST_NODE_TYPES.TSNeverKeyword: + case AST_NODE_TYPES.TSNullKeyword: + case AST_NODE_TYPES.TSNumberKeyword: + case AST_NODE_TYPES.TSObjectKeyword: + case AST_NODE_TYPES.TSOptionalType: + case AST_NODE_TYPES.TSPrivateKeyword: + case AST_NODE_TYPES.TSProtectedKeyword: + case AST_NODE_TYPES.TSPublicKeyword: + case AST_NODE_TYPES.TSReadonlyKeyword: + case AST_NODE_TYPES.TSRestType: + case AST_NODE_TYPES.TSStaticKeyword: + case AST_NODE_TYPES.TSStringKeyword: + case AST_NODE_TYPES.TSSymbolKeyword: + case AST_NODE_TYPES.TSTemplateLiteralType: + case AST_NODE_TYPES.TSTupleType: + case AST_NODE_TYPES.TSTypePredicate: + case AST_NODE_TYPES.TSUndefinedKeyword: + case AST_NODE_TYPES.TSUnknownKeyword: + case AST_NODE_TYPES.TSVoidKeyword: + return false; + } + } + + function checkTestConditional( + node: + | TSESTree.ConditionalExpression + | TSESTree.DoWhileStatement + | TSESTree.ForStatement + | TSESTree.IfStatement + | TSESTree.WhileStatement, + ): void { if (node.test) { checkConditional(node.test, true); } @@ -255,9 +326,19 @@ export default createRule({ function checkVariableDeclaration(node: TSESTree.VariableDeclarator): void { const tsNode = services.esTreeNodeToTSNodeMap.get(node); - if (tsNode.initializer === undefined || node.init == null) { + if ( + tsNode.initializer === undefined || + node.init == null || + node.id.typeAnnotation == null + ) { return; } + + // syntactically ignore some known-good cases to avoid touching type info + if (!isPossiblyFunctionType(node.id.typeAnnotation)) { + return; + } + const varType = services.getTypeAtLocation(node.id); if (!isVoidReturningFunctionType(checker, tsNode.initializer, varType)) { return; @@ -352,6 +433,22 @@ export default createRule({ if (tsNode.expression === undefined || node.argument == null) { return; } + + // syntactically ignore some known-good cases to avoid touching type info + const functionNode = (() => { + let current: TSESTree.Node | undefined = node.parent; + while (current && !isFunction(current)) { + current = current.parent; + } + return nullThrows(current, NullThrowsReasons.MissingParent); + })(); + if ( + functionNode.returnType && + !isPossiblyFunctionType(functionNode.returnType) + ) { + return; + } + const contextualType = checker.getContextualType(tsNode.expression); if ( contextualType !== undefined && diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index 45f608f4a8d5..ce7f5c0eef90 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -508,6 +508,14 @@ foo(bar); }, options: [{ checksVoidReturn: { attributes: true } }], }, + ` + const notAFn1: string = ''; + const notAFn2: number = 1; + const notAFn3: boolean = true; + const notAFn4: { prop: 1 } = { prop: 1 }; + const notAFn5: {} = {}; + const notAFn5: {} = {}; + `, ], invalid: [ From 37b85c74041dd602f92bcbc17db3b7afd079656a Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 29 Jul 2024 10:41:18 +0930 Subject: [PATCH 2/2] Update eslint.config.mjs --- eslint.config.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index e4419dfbabbb..7cb84f5cd214 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -54,7 +54,7 @@ export default tseslint.config( // config with just ignores is the replacement for `.eslintignore` ignores: [ '.nx/', - '.yarn', + '.yarn/', '**/jest.config.js', '**/node_modules/**', '**/dist/**',