From 9598b4dd4f0c26204b512eea7de8b89cb7bf9968 Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 14:47:36 +0900 Subject: [PATCH 01/16] refactor(eslint-plugin): create isVoidReturningFunctionType --- .../src/rules/no-misused-promises.ts | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 6e42e547436..1eda89a97c4 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -219,7 +219,6 @@ function voidFunctionParams( node: ts.CallExpression | ts.NewExpression, ): Set { const voidReturnIndices = new Set(); - const thenableReturnIndices = new Set(); const type = checker.getTypeAtLocation(node.expression); for (const subType of tsutils.unionTypeParts(type)) { @@ -233,29 +232,37 @@ function voidFunctionParams( parameter, node.expression, ); - for (const subType of tsutils.unionTypeParts(type)) { - for (const signature of subType.getCallSignatures()) { - const returnType = signature.getReturnType(); - if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) { - voidReturnIndices.add(index); - } else if ( - tsutils.isThenableType(checker, node.expression, returnType) - ) { - thenableReturnIndices.add(index); - } - } + if (isVoidReturningFunctionType(checker, node.expression, type)) { + voidReturnIndices.add(index); } } } } + return voidReturnIndices; +} + +// Returns true if given type is a void-returning function. +function isVoidReturningFunctionType( + checker: ts.TypeChecker, + node: ts.Node, + type: ts.Type, +): boolean { + let hasVoidReturningFunction = false; + let hasThenableReturningFunction = false; + for (const subType of tsutils.unionTypeParts(type)) { + for (const signature of subType.getCallSignatures()) { + const returnType = signature.getReturnType(); + if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) { + hasVoidReturningFunction = true; + } else if (tsutils.isThenableType(checker, node, returnType)) { + hasThenableReturningFunction = true; + } + } + } // If a certain positional argument accepts both thenable and void returns, // a promise-returning function is valid - for (const thenable of thenableReturnIndices) { - voidReturnIndices.delete(thenable); - } - - return voidReturnIndices; + return hasVoidReturningFunction && !hasThenableReturningFunction; } // Returns true if the expression is a function that returns a thenable From 2d0f9d85db5cbe388f7176d9874ce88562fcd3bd Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 15:27:27 +0900 Subject: [PATCH 02/16] feat(eslint-plugin): add checkAssignments and checkVariableDeclaration --- .../src/rules/no-misused-promises.ts | 41 +++++++++++++++- .../tests/rules/no-misused-promises.test.ts | 49 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 1eda89a97c4..d43c484e67f 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -11,7 +11,9 @@ type Options = [ }, ]; -export default util.createRule({ +type MessageId = 'conditional' | 'voidReturn' | 'voidReturnVariable'; + +export default util.createRule({ name: 'no-misused-promises', meta: { docs: { @@ -22,6 +24,8 @@ export default util.createRule({ messages: { voidReturn: 'Promise returned in function argument where a void return was expected.', + voidReturnVariable: + 'Promise returned in variable where a void return was expected.', conditional: 'Expected non-Promise value in a boolean conditional.', }, schema: [ @@ -67,6 +71,8 @@ export default util.createRule({ const voidReturnChecks: TSESLint.RuleListener = { CallExpression: checkArguments, NewExpression: checkArguments, + AssignmentExpression: checkAssignments, + VariableDeclarator: checkVariableDeclaration, }; function checkTestConditional(node: { @@ -137,6 +143,39 @@ export default util.createRule({ } } + function checkAssignments(node: TSESTree.AssignmentExpression): void { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + const varType = checker.getTypeAtLocation(tsNode.left); + if (!isVoidReturningFunctionType(checker, tsNode.left, varType)) { + return; + } + + if (returnsThenable(checker, tsNode.right)) { + context.report({ + messageId: 'voidReturnVariable', + node: node.right, + }); + } + } + + function checkVariableDeclaration(node: TSESTree.VariableDeclarator): void { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + if (tsNode.initializer === undefined || node.init === null) { + return; + } + const varType = checker.getTypeAtLocation(tsNode.name); + if (!isVoidReturningFunctionType(checker, tsNode.initializer, varType)) { + return; + } + + if (returnsThenable(checker, tsNode.initializer)) { + context.report({ + messageId: 'voidReturnVariable', + node: node.init, + }); + } + } + return { ...(checksConditionals ? conditionalChecks : {}), ...(checksVoidReturn ? voidReturnChecks : {}), 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 63e429000c0..df4d58fbe6f 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -432,5 +432,54 @@ function test(p: Promise | undefined) { }, ], }, + { + code: ` +let f: () => void; +f = async () => { + return 3; +}; + `, + errors: [ + { + line: 3, + messageId: 'voidReturnVariable', + }, + ], + }, + { + code: ` +const f: () => void = async () => { + return 0; +}; +const g = async () => 1, + h: () => void = async () => {}; + `, + errors: [ + { + line: 2, + messageId: 'voidReturnVariable', + }, + { + line: 6, + messageId: 'voidReturnVariable', + }, + ], + }, + { + code: ` +const obj: { + f?: () => void; +} = {}; +obj.f = async () => { + return 0; +}; + `, + errors: [ + { + line: 5, + messageId: 'voidReturnVariable', + }, + ], + }, ], }); From 96ae88d2a0fc28dd97cd19add74c1855398706bd Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 15:30:42 +0900 Subject: [PATCH 03/16] test(eslint-plugin): add valid test cases --- .../tests/rules/no-misused-promises.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 df4d58fbe6f..2ed04f853bf 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -166,6 +166,16 @@ async function test(p: Promise | undefined) { } } `, + ` +let f; +f = async () => 10; + `, + ` +let f: () => Promise; +f = async () => 10; +const g = async () => 0; +const h: () => Promise = async () => 10; + `, ], invalid: [ From 5d39edd45332e2874ff3db3edd86f687b108bad2 Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 16:46:20 +0900 Subject: [PATCH 04/16] feat(eslint-plugin): add object property checks --- .../src/rules/no-misused-promises.ts | 93 ++++++++++++++++++- .../tests/rules/no-misused-promises.test.ts | 92 ++++++++++++++++++ 2 files changed, 180 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index d43c484e67f..1821a63c545 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -11,7 +11,11 @@ type Options = [ }, ]; -type MessageId = 'conditional' | 'voidReturn' | 'voidReturnVariable'; +type MessageId = + | 'conditional' + | 'voidReturn' + | 'voidReturnVariable' + | 'voidReturnProperty'; export default util.createRule({ name: 'no-misused-promises', @@ -26,6 +30,8 @@ export default util.createRule({ 'Promise returned in function argument where a void return was expected.', voidReturnVariable: 'Promise returned in variable where a void return was expected.', + voidReturnProperty: + 'Promise returned in property where a void return was expected.', conditional: 'Expected non-Promise value in a boolean conditional.', }, schema: [ @@ -73,6 +79,7 @@ export default util.createRule({ NewExpression: checkArguments, AssignmentExpression: checkAssignments, VariableDeclarator: checkVariableDeclaration, + Property: checkProperties, }; function checkTestConditional(node: { @@ -176,6 +183,85 @@ export default util.createRule({ } } + function checkProperties(node: TSESTree.Property): void { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + if (ts.isPropertyAssignment(tsNode)) { + const contextualType = checker.getContextualType(tsNode.initializer); + if (contextualType === undefined) { + return; + } + if ( + !isVoidReturningFunctionType( + checker, + tsNode.initializer, + contextualType, + ) + ) { + return; + } + if (returnsThenable(checker, tsNode.initializer)) { + context.report({ + messageId: 'voidReturnProperty', + node: node.value, + }); + } + } else if (ts.isShorthandPropertyAssignment(tsNode)) { + const contextualType = checker.getContextualType(tsNode.name); + if (contextualType === undefined) { + return; + } + if ( + !isVoidReturningFunctionType(checker, tsNode.name, contextualType) + ) { + return; + } + if (returnsThenable(checker, tsNode.name)) { + context.report({ + messageId: 'voidReturnProperty', + node: node.value, + }); + } + } else if (ts.isMethodDeclaration(tsNode)) { + if (ts.isComputedPropertyName(tsNode.name)) { + return; + } + const obj = tsNode.parent; + if (!ts.isObjectLiteralExpression(obj)) { + return; + } + const objType = + checker.getContextualType(obj) ?? checker.getTypeAtLocation(obj); + const propertySymbol = checker.getPropertyOfType( + objType, + tsNode.name.text, + ); + if (propertySymbol === undefined) { + return; + } + + const contextualType = checker.getTypeOfSymbolAtLocation( + propertySymbol, + tsNode.name, + ); + + if (contextualType === undefined) { + return; + } + if ( + !isVoidReturningFunctionType(checker, tsNode.name, contextualType) + ) { + return; + } + if (returnsThenable(checker, tsNode)) { + context.report({ + messageId: 'voidReturnProperty', + node: node.value, + }); + } + return; + } + } + return { ...(checksConditionals ? conditionalChecks : {}), ...(checksVoidReturn ? voidReturnChecks : {}), @@ -305,10 +391,7 @@ function isVoidReturningFunctionType( } // Returns true if the expression is a function that returns a thenable -function returnsThenable( - checker: ts.TypeChecker, - node: ts.Expression, -): boolean { +function returnsThenable(checker: ts.TypeChecker, node: ts.Node): boolean { const type = checker.getApparentType(checker.getTypeAtLocation(node)); for (const subType of tsutils.unionTypeParts(type)) { 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 2ed04f853bf..61e161da548 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -176,6 +176,24 @@ f = async () => 10; const g = async () => 0; const h: () => Promise = async () => 10; `, + ` +const obj = { + f: async () => 10, +}; + `, + ` +const f = async () => 123; +const obj = { + f, +}; + `, + ` +const obj = { + async f() { + return 0; + }, +}; + `, ], invalid: [ @@ -491,5 +509,79 @@ obj.f = async () => { }, ], }, + { + code: ` +type O = { f: () => void }; +const obj: O = { + f: async () => 'foo', +}; + `, + errors: [ + { + line: 4, + messageId: 'voidReturnProperty', + }, + ], + }, + { + code: ` +type O = { f: () => void }; +const f = async () => 0; +const obj: O = { + f, +}; + `, + errors: [ + { + line: 5, + messageId: 'voidReturnProperty', + }, + ], + }, + { + code: ` +type O = { f: () => void }; +const obj: O = { + async f() { + return 0; + }, +}; + `, + errors: [ + { + line: 4, + messageId: 'voidReturnProperty', + }, + ], + }, + { + code: ` +type O = { f: () => void; g: () => void; h: () => void }; +function f(): O { + const h = async () => 0; + return { + async f() { + return 123; + }, + g: async () => 0, + h, + }; +} + `, + errors: [ + { + line: 6, + messageId: 'voidReturnProperty', + }, + { + line: 9, + messageId: 'voidReturnProperty', + }, + { + line: 10, + messageId: 'voidReturnProperty', + }, + ], + }, ], }); From 18811b2167cd85ae0760edbb045a60a3b986fab9 Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 16:51:43 +0900 Subject: [PATCH 05/16] feat(eslint-plugin): add checkReturnStatements --- .../src/rules/no-misused-promises.ts | 28 ++++++++++++++++++- .../tests/rules/no-misused-promises.test.ts | 21 ++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 1821a63c545..0f012c9afc6 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -15,7 +15,8 @@ type MessageId = | 'conditional' | 'voidReturn' | 'voidReturnVariable' - | 'voidReturnProperty'; + | 'voidReturnProperty' + | 'voidReturnReturnValue'; export default util.createRule({ name: 'no-misused-promises', @@ -32,6 +33,8 @@ export default util.createRule({ 'Promise returned in variable where a void return was expected.', voidReturnProperty: 'Promise returned in property where a void return was expected.', + voidReturnReturnValue: + 'Promise returned in return value where a void return was expected.', conditional: 'Expected non-Promise value in a boolean conditional.', }, schema: [ @@ -80,6 +83,7 @@ export default util.createRule({ AssignmentExpression: checkAssignments, VariableDeclarator: checkVariableDeclaration, Property: checkProperties, + ReturnStatement: checkReturnStatements, }; function checkTestConditional(node: { @@ -262,6 +266,28 @@ export default util.createRule({ } } + function checkReturnStatements(node: TSESTree.ReturnStatement): void { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + if (tsNode.expression === undefined || node.argument === null) { + return; + } + const contextualType = checker.getContextualType(tsNode.expression); + if (contextualType === undefined) { + return; + } + if ( + !isVoidReturningFunctionType(checker, tsNode.expression, contextualType) + ) { + return; + } + if (returnsThenable(checker, tsNode.expression)) { + context.report({ + messageId: 'voidReturnReturnValue', + node: node.argument, + }); + } + } + return { ...(checksConditionals ? conditionalChecks : {}), ...(checksVoidReturn ? voidReturnChecks : {}), 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 61e161da548..9468f798e90 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -194,6 +194,14 @@ const obj = { }, }; `, + ` +function f() { + return async () => 0; +} +function g() { + return; +} + `, ], invalid: [ @@ -583,5 +591,18 @@ function f(): O { }, ], }, + { + code: ` +function f(): () => void { + return async () => 0; +} + `, + errors: [ + { + line: 3, + messageId: 'voidReturnReturnValue', + }, + ], + }, ], }); From 733ad0fa33ec7a19076378fb48f259bcd539619c Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 17:20:28 +0900 Subject: [PATCH 06/16] feat(eslint-plugin): add checkJSXAttributes --- .../src/rules/no-misused-promises.ts | 33 ++++++++++++++++++- .../tests/rules/no-misused-promises.test.ts | 33 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 0f012c9afc6..20624f519cb 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -16,7 +16,8 @@ type MessageId = | 'voidReturn' | 'voidReturnVariable' | 'voidReturnProperty' - | 'voidReturnReturnValue'; + | 'voidReturnReturnValue' + | 'voidReturnAttribute'; export default util.createRule({ name: 'no-misused-promises', @@ -35,6 +36,8 @@ export default util.createRule({ 'Promise returned in property where a void return was expected.', voidReturnReturnValue: 'Promise returned in return value where a void return was expected.', + voidReturnAttribute: + 'Promise returned in attribute where a void return was expected.', conditional: 'Expected non-Promise value in a boolean conditional.', }, schema: [ @@ -84,6 +87,7 @@ export default util.createRule({ VariableDeclarator: checkVariableDeclaration, Property: checkProperties, ReturnStatement: checkReturnStatements, + JSXAttribute: checkJSXAttributes, }; function checkTestConditional(node: { @@ -288,6 +292,33 @@ export default util.createRule({ } } + function checkJSXAttributes(node: TSESTree.JSXAttribute): void { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + const value = tsNode.initializer; + if ( + node.value === null || + value === undefined || + !ts.isJsxExpression(value) || + value.expression === undefined + ) { + return; + } + const contextualType = checker.getContextualType(value); + if (contextualType === undefined) { + return; + } + if (!isVoidReturningFunctionType(checker, value, contextualType)) { + return; + } + + if (returnsThenable(checker, value.expression)) { + context.report({ + messageId: 'voidReturnAttribute', + node: node.value, + }); + } + } + return { ...(checksConditionals ? conditionalChecks : {}), ...(checksVoidReturn ? voidReturnChecks : {}), 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 9468f798e90..0fea7fded98 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -604,5 +604,38 @@ function f(): () => void { }, ], }, + { + code: ` +type O = { + func: () => void; +}; +const Component = (obj: O) => null; + 0} />; + `, + filename: 'react.tsx', + errors: [ + { + line: 6, + messageId: 'voidReturnAttribute', + }, + ], + }, + { + code: ` +type O = { + func: () => void; +}; +const g = async () => 'foo'; +const Component = (obj: O) => null; +; + `, + filename: 'react.tsx', + errors: [ + { + line: 7, + messageId: 'voidReturnAttribute', + }, + ], + }, ], }); From 3585e63399946e006c18ec033a6685b28ae2d9d0 Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 17:50:08 +0900 Subject: [PATCH 07/16] fix(website): resolve newly introduced lint errors --- .../website/src/components/OptionsSelector.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/website/src/components/OptionsSelector.tsx b/packages/website/src/components/OptionsSelector.tsx index 83715fc3ae5..86c84296507 100644 --- a/packages/website/src/components/OptionsSelector.tsx +++ b/packages/website/src/components/OptionsSelector.tsx @@ -74,17 +74,21 @@ function OptionsSelector({ [setState], ); - const copyLinkToClipboard = useCallback(async () => { - await navigator.clipboard.writeText(document.location.toString()); - setCopyLink(true); + const copyLinkToClipboard = useCallback(() => { + void navigator.clipboard + .writeText(document.location.toString()) + .then(() => { + setCopyLink(true); + }); }, []); - const copyMarkdownToClipboard = useCallback(async () => { + const copyMarkdownToClipboard = useCallback(() => { if (isLoading) { return; } - await navigator.clipboard.writeText(createMarkdown(state)); - setCopyMarkdown(true); + void navigator.clipboard.writeText(createMarkdown(state)).then(() => { + setCopyMarkdown(true); + }); }, [state, isLoading]); const openIssue = useCallback(() => { From 88fff213242256263c77766bcdc43fbf15a7a40c Mon Sep 17 00:00:00 2001 From: uhyo Date: Sat, 12 Feb 2022 22:27:27 +0900 Subject: [PATCH 08/16] test(eslint-plugin): worship code coverage --- .../src/rules/no-misused-promises.ts | 3 -- .../tests/rules/no-misused-promises.test.ts | 43 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 20624f519cb..27d83c04127 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -252,9 +252,6 @@ export default util.createRule({ tsNode.name, ); - if (contextualType === undefined) { - return; - } if ( !isVoidReturningFunctionType(checker, tsNode.name, contextualType) ) { 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 0fea7fded98..afe14590aca 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -195,6 +195,31 @@ const obj = { }; `, ` +type O = { f: () => Promise; g: () => Promise }; +const g = async () => 0; +const obj: O = { + f: async () => 10, + g, +}; + `, + ` +type O = { f: () => Promise }; +const name = 'f'; +const obj: O = { + async [name]() { + return 10; + }, +}; + `, + ` +const obj = { + f: async () => 'foo', + async g() { + return 0; + }, +}; + `, + ` function f() { return async () => 0; } @@ -202,6 +227,24 @@ function g() { return; } `, + { + code: ` +type O = { + bool: boolean; + func: () => Promise; +}; +const Component = (obj: O) => null; + 10} />; + `, + filename: 'react.tsx', + }, + { + code: ` +const Component: any = () => null; + 10} />; + `, + filename: 'react.tsx', + }, ], invalid: [ From e82b964ac922b71075d3ed00e62a77e739eb1f52 Mon Sep 17 00:00:00 2001 From: uhyo Date: Thu, 24 Feb 2022 09:42:45 +0900 Subject: [PATCH 09/16] Apply suggestions from code review Co-authored-by: Josh Goldberg --- packages/eslint-plugin/src/rules/no-misused-promises.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 27d83c04127..9aa41108784 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -31,7 +31,7 @@ export default util.createRule({ voidReturn: 'Promise returned in function argument where a void return was expected.', voidReturnVariable: - 'Promise returned in variable where a void return was expected.', + 'Promise-returning function provided to variable where a void return was expected.', voidReturnProperty: 'Promise returned in property where a void return was expected.', voidReturnReturnValue: @@ -158,7 +158,7 @@ export default util.createRule({ } } - function checkAssignments(node: TSESTree.AssignmentExpression): void { + function checkAssignment(node: TSESTree.AssignmentExpression): void { const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); const varType = checker.getTypeAtLocation(tsNode.left); if (!isVoidReturningFunctionType(checker, tsNode.left, varType)) { From 9c4bcfd8e954116c0e9cb8de80a0dd25927327a4 Mon Sep 17 00:00:00 2001 From: uhyo Date: Thu, 24 Feb 2022 09:46:05 +0900 Subject: [PATCH 10/16] fix(eslint-plugin): rename checker functions to singular --- .../eslint-plugin/src/rules/no-misused-promises.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 9aa41108784..f11980b3286 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -83,11 +83,11 @@ export default util.createRule({ const voidReturnChecks: TSESLint.RuleListener = { CallExpression: checkArguments, NewExpression: checkArguments, - AssignmentExpression: checkAssignments, + AssignmentExpression: checkAssignment, VariableDeclarator: checkVariableDeclaration, - Property: checkProperties, - ReturnStatement: checkReturnStatements, - JSXAttribute: checkJSXAttributes, + Property: checkProperty, + ReturnStatement: checkReturnStatement, + JSXAttribute: checkJSXAttribute, }; function checkTestConditional(node: { @@ -191,7 +191,7 @@ export default util.createRule({ } } - function checkProperties(node: TSESTree.Property): void { + function checkProperty(node: TSESTree.Property): void { const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); if (ts.isPropertyAssignment(tsNode)) { const contextualType = checker.getContextualType(tsNode.initializer); @@ -267,7 +267,7 @@ export default util.createRule({ } } - function checkReturnStatements(node: TSESTree.ReturnStatement): void { + function checkReturnStatement(node: TSESTree.ReturnStatement): void { const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); if (tsNode.expression === undefined || node.argument === null) { return; @@ -289,7 +289,7 @@ export default util.createRule({ } } - function checkJSXAttributes(node: TSESTree.JSXAttribute): void { + function checkJSXAttribute(node: TSESTree.JSXAttribute): void { const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); const value = tsNode.initializer; if ( From dc452e359f6acbca6848390a86466f6d1befab10 Mon Sep 17 00:00:00 2001 From: uhyo Date: Thu, 24 Feb 2022 09:47:37 +0900 Subject: [PATCH 11/16] fix(eslint-plugin): align error messages --- packages/eslint-plugin/src/rules/no-misused-promises.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index f11980b3286..79a4ee5e084 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -33,11 +33,11 @@ export default util.createRule({ voidReturnVariable: 'Promise-returning function provided to variable where a void return was expected.', voidReturnProperty: - 'Promise returned in property where a void return was expected.', + 'Promise-returning function provided to property where a void return was expected.', voidReturnReturnValue: - 'Promise returned in return value where a void return was expected.', + 'Promise-returning function provided to return value where a void return was expected.', voidReturnAttribute: - 'Promise returned in attribute where a void return was expected.', + 'Promise-returning function provided to attribute where a void return was expected.', conditional: 'Expected non-Promise value in a boolean conditional.', }, schema: [ From b59abc494a764c5379b8472b383a5e157b1f0e49 Mon Sep 17 00:00:00 2001 From: uhyo Date: Thu, 24 Feb 2022 09:51:21 +0900 Subject: [PATCH 12/16] refactor(eslint-plugin): change MessageId --- .../eslint-plugin/src/rules/no-misused-promises.ts | 6 +++--- .../tests/rules/no-misused-promises.test.ts | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 79a4ee5e084..1983e6313b8 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -13,7 +13,7 @@ type Options = [ type MessageId = | 'conditional' - | 'voidReturn' + | 'voidReturnArgument' | 'voidReturnVariable' | 'voidReturnProperty' | 'voidReturnReturnValue' @@ -28,7 +28,7 @@ export default util.createRule({ requiresTypeChecking: true, }, messages: { - voidReturn: + voidReturnArgument: 'Promise returned in function argument where a void return was expected.', voidReturnVariable: 'Promise-returning function provided to variable where a void return was expected.', @@ -151,7 +151,7 @@ export default util.createRule({ const tsNode = parserServices.esTreeNodeToTSNodeMap.get(argument); if (returnsThenable(checker, tsNode as ts.Expression)) { context.report({ - messageId: 'voidReturn', + messageId: 'voidReturnArgument', node: argument, }); } 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 afe14590aca..97b7905203e 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -344,7 +344,7 @@ if (!Promise.resolve()) { errors: [ { line: 2, - messageId: 'voidReturn', + messageId: 'voidReturnArgument', }, ], }, @@ -358,7 +358,7 @@ new Promise(async (resolve, reject) => { errors: [ { line: 2, - messageId: 'voidReturn', + messageId: 'voidReturnArgument', }, ], }, @@ -375,7 +375,7 @@ fnWithCallback('val', async (err, res) => { errors: [ { line: 6, - messageId: 'voidReturn', + messageId: 'voidReturnArgument', }, ], }, @@ -390,7 +390,7 @@ fnWithCallback('val', (err, res) => Promise.resolve(res)); errors: [ { line: 6, - messageId: 'voidReturn', + messageId: 'voidReturnArgument', }, ], }, @@ -411,7 +411,7 @@ fnWithCallback('val', (err, res) => { errors: [ { line: 6, - messageId: 'voidReturn', + messageId: 'voidReturnArgument', }, ], }, @@ -428,7 +428,7 @@ fnWithCallback?.('val', (err, res) => Promise.resolve(res)); errors: [ { line: 8, - messageId: 'voidReturn', + messageId: 'voidReturnArgument', }, ], }, @@ -451,7 +451,7 @@ fnWithCallback('val', (err, res) => { errors: [ { line: 8, - messageId: 'voidReturn', + messageId: 'voidReturnArgument', }, ], }, From deae75592cd2a61bb396350e751474210a2f8bd8 Mon Sep 17 00:00:00 2001 From: uhyo Date: Thu, 24 Feb 2022 09:55:38 +0900 Subject: [PATCH 13/16] refactor(eslint-plugin): fix if statements style --- .../src/rules/no-misused-promises.ts | 55 +++++++------------ 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 1983e6313b8..2ac07a43843 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -195,19 +195,15 @@ export default util.createRule({ const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); if (ts.isPropertyAssignment(tsNode)) { const contextualType = checker.getContextualType(tsNode.initializer); - if (contextualType === undefined) { - return; - } if ( - !isVoidReturningFunctionType( + contextualType !== undefined && + isVoidReturningFunctionType( checker, tsNode.initializer, contextualType, - ) + ) && + returnsThenable(checker, tsNode.initializer) ) { - return; - } - if (returnsThenable(checker, tsNode.initializer)) { context.report({ messageId: 'voidReturnProperty', node: node.value, @@ -215,15 +211,11 @@ export default util.createRule({ } } else if (ts.isShorthandPropertyAssignment(tsNode)) { const contextualType = checker.getContextualType(tsNode.name); - if (contextualType === undefined) { - return; - } if ( - !isVoidReturningFunctionType(checker, tsNode.name, contextualType) + contextualType !== undefined && + isVoidReturningFunctionType(checker, tsNode.name, contextualType) && + returnsThenable(checker, tsNode.name) ) { - return; - } - if (returnsThenable(checker, tsNode.name)) { context.report({ messageId: 'voidReturnProperty', node: node.value, @@ -253,11 +245,9 @@ export default util.createRule({ ); if ( - !isVoidReturningFunctionType(checker, tsNode.name, contextualType) + isVoidReturningFunctionType(checker, tsNode.name, contextualType) && + returnsThenable(checker, tsNode) ) { - return; - } - if (returnsThenable(checker, tsNode)) { context.report({ messageId: 'voidReturnProperty', node: node.value, @@ -273,15 +263,15 @@ export default util.createRule({ return; } const contextualType = checker.getContextualType(tsNode.expression); - if (contextualType === undefined) { - return; - } if ( - !isVoidReturningFunctionType(checker, tsNode.expression, contextualType) + contextualType !== undefined && + isVoidReturningFunctionType( + checker, + tsNode.expression, + contextualType, + ) && + returnsThenable(checker, tsNode.expression) ) { - return; - } - if (returnsThenable(checker, tsNode.expression)) { context.report({ messageId: 'voidReturnReturnValue', node: node.argument, @@ -301,14 +291,11 @@ export default util.createRule({ return; } const contextualType = checker.getContextualType(value); - if (contextualType === undefined) { - return; - } - if (!isVoidReturningFunctionType(checker, value, contextualType)) { - return; - } - - if (returnsThenable(checker, value.expression)) { + if ( + contextualType !== undefined && + isVoidReturningFunctionType(checker, value, contextualType) && + returnsThenable(checker, value.expression) + ) { context.report({ messageId: 'voidReturnAttribute', node: node.value, From 691cc2c228fb6497a2e9962156a0247e5291a531 Mon Sep 17 00:00:00 2001 From: uhyo Date: Thu, 24 Feb 2022 09:59:41 +0900 Subject: [PATCH 14/16] refactor(eslint-plugin): no need of getTypeAtLocation --- packages/eslint-plugin/src/rules/no-misused-promises.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 2ac07a43843..ebdb1e6eee9 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -229,8 +229,10 @@ export default util.createRule({ if (!ts.isObjectLiteralExpression(obj)) { return; } - const objType = - checker.getContextualType(obj) ?? checker.getTypeAtLocation(obj); + const objType = checker.getContextualType(obj); + if (objType === undefined) { + return; + } const propertySymbol = checker.getPropertyOfType( objType, tsNode.name.text, From f277f831ae59514d52f1dd2cf55fe893ae4a427a Mon Sep 17 00:00:00 2001 From: uhyo Date: Thu, 24 Feb 2022 10:29:04 +0900 Subject: [PATCH 15/16] refactor(eslint-plugin): make coverage 100% --- packages/eslint-plugin/src/rules/no-misused-promises.ts | 5 ++++- .../eslint-plugin/tests/rules/no-misused-promises.test.ts | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index ebdb1e6eee9..d199ca120bb 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -226,7 +226,10 @@ export default util.createRule({ return; } const obj = tsNode.parent; - if (!ts.isObjectLiteralExpression(obj)) { + + // This condition isn't satisfied but needed for type checking. + // The fundamental reason is that mapping between ESLint and TypeScript ASTs is not 1:1. + /* istanbul ignore if */ if (!ts.isObjectLiteralExpression(obj)) { return; } const objType = checker.getContextualType(obj); 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 97b7905203e..961a21d4c89 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -212,6 +212,13 @@ const obj: O = { }; `, ` +const obj: number = { + g() { + return 10; + }, +}; + `, + ` const obj = { f: async () => 'foo', async g() { From eadaaa8704449d7e3aff3fb0303f7acb18c2a45e Mon Sep 17 00:00:00 2001 From: uhyo Date: Fri, 25 Feb 2022 01:04:05 +0900 Subject: [PATCH 16/16] refactor(eslint-plugin): update comment --- .../eslint-plugin/src/rules/no-misused-promises.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index d199ca120bb..7f08402334c 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -227,11 +227,16 @@ export default util.createRule({ } const obj = tsNode.parent; - // This condition isn't satisfied but needed for type checking. - // The fundamental reason is that mapping between ESLint and TypeScript ASTs is not 1:1. - /* istanbul ignore if */ if (!ts.isObjectLiteralExpression(obj)) { + // Below condition isn't satisfied unless something goes wrong, + // but is needed for type checking. + // 'node' does not include class method declaration so 'obj' is + // always an object literal expression, but after converting 'node' + // to TypeScript AST, its type includes MethodDeclaration which + // does include the case of class method declaration. + if (!ts.isObjectLiteralExpression(obj)) { return; } + const objType = checker.getContextualType(obj); if (objType === undefined) { return;