From f330e0651548d55163ddc3233c90fd3cbe37c9c0 Mon Sep 17 00:00:00 2001 From: SHIMA RYUHEI <65934663+islandryu@users.noreply.github.com> Date: Sat, 4 Feb 2023 03:39:29 +0900 Subject: [PATCH] feat(eslint-plugin): [no-floating-promises] error on logical expression (#6356) * feat(eslint-plugin) [no-floating-promises] Error on logical expression * Update packages/eslint-plugin/src/rules/no-floating-promises.ts Co-authored-by: Josh Goldberg * remove sourceCode Variable --------- Co-authored-by: Josh Goldberg --- .../src/rules/no-floating-promises.ts | 36 +- .../tests/rules/no-floating-promises.test.ts | 310 ++++++++++++++++++ 2 files changed, 334 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 61829743e81..c4ce3db8e1c 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -4,6 +4,7 @@ import * as tsutils from 'tsutils'; import * as ts from 'typescript'; import * as util from '../util'; +import { OperatorPrecedence } from '../util'; type Options = [ { @@ -66,7 +67,6 @@ export default util.createRule({ create(context, [options]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); - const sourceCode = context.getSourceCode(); return { ExpressionStatement(node): void { @@ -88,10 +88,21 @@ export default util.createRule({ suggest: [ { messageId: 'floatingFixVoid', - fix(fixer): TSESLint.RuleFix { - let code = sourceCode.getText(node); - code = `void ${code}`; - return fixer.replaceText(node, code); + fix(fixer): TSESLint.RuleFix | TSESLint.RuleFix[] { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get( + node.expression, + ); + if (isHigherPrecedenceThanUnary(tsNode)) { + return fixer.insertTextBefore(node, 'void '); + } else { + return [ + fixer.insertTextBefore(node, 'void ('), + fixer.insertTextAfterRange( + [expression.range[1], expression.range[1]], + ')', + ), + ]; + } }, }, ], @@ -116,7 +127,7 @@ export default util.createRule({ const tsNode = parserServices.esTreeNodeToTSNodeMap.get( node.expression, ); - if (isHigherPrecedenceThanAwait(tsNode)) { + if (isHigherPrecedenceThanUnary(tsNode)) { return fixer.insertTextBefore(node, 'await '); } else { return [ @@ -136,16 +147,12 @@ export default util.createRule({ }, }; - function isHigherPrecedenceThanAwait(node: ts.Node): boolean { + function isHigherPrecedenceThanUnary(node: ts.Node): boolean { const operator = tsutils.isBinaryExpression(node) ? node.operatorToken.kind : ts.SyntaxKind.Unknown; const nodePrecedence = util.getOperatorPrecedence(node.kind, operator); - const awaitPrecedence = util.getOperatorPrecedence( - ts.SyntaxKind.AwaitExpression, - ts.SyntaxKind.Unknown, - ); - return nodePrecedence > awaitPrecedence; + return nodePrecedence > OperatorPrecedence.Unary; } function isAsyncIife(node: TSESTree.ExpressionStatement): boolean { @@ -214,6 +221,11 @@ export default util.createRule({ // `new Promise()`), the promise is not handled because it doesn't have the // necessary then/catch call at the end of the chain. return true; + } else if (node.type === AST_NODE_TYPES.LogicalExpression) { + return ( + isUnhandledPromise(checker, node.left) || + isUnhandledPromise(checker, node.right) + ); } // We conservatively return false for all other types of expressions because diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index a80acab989a..070cef91e9a 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -405,6 +405,57 @@ void doSomething(); `, options: [{ ignoreIIFE: true }], }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + void (condition && myPromise()); +} + `, + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + await (condition && myPromise()); +} + `, + options: [{ ignoreVoid: false }], + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + condition && void myPromise(); +} + `, + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + condition && (await myPromise()); +} + `, + options: [{ ignoreVoid: false }], + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + let condition = false; + condition && myPromise(); + condition = true; + condition || myPromise(); + condition ?? myPromise(); +} + `, + options: [{ ignoreVoid: false }], + }, ], invalid: [ @@ -1117,5 +1168,264 @@ async function test() { }, ], }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + + void condition || myPromise(); +} + `, + errors: [ + { + line: 6, + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + + void (void condition || myPromise()); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + + (await condition) && myPromise(); +} + `, + options: [{ ignoreVoid: false }], + errors: [ + { + line: 6, + messageId: 'floating', + suggestions: [ + { + messageId: 'floatingFixAwait', + output: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + + await ((await condition) && myPromise()); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + + condition && myPromise(); +} + `, + errors: [ + { + line: 6, + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +async function foo() { + const myPromise = async () => void 0; + const condition = true; + + void (condition && myPromise()); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = false; + + condition || myPromise(); +} + `, + errors: [ + { + line: 6, + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +async function foo() { + const myPromise = async () => void 0; + const condition = false; + + void (condition || myPromise()); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = null; + + condition ?? myPromise(); +} + `, + errors: [ + { + line: 6, + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +async function foo() { + const myPromise = async () => void 0; + const condition = null; + + void (condition ?? myPromise()); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = Promise.resolve(true); + let condition = true; + condition && myPromise; +} + `, + options: [{ ignoreVoid: false }], + errors: [ + { + line: 5, + messageId: 'floating', + suggestions: [ + { + messageId: 'floatingFixAwait', + output: ` +async function foo() { + const myPromise = Promise.resolve(true); + let condition = true; + await (condition && myPromise); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = Promise.resolve(true); + let condition = false; + condition || myPromise; +} + `, + options: [{ ignoreVoid: false }], + errors: [ + { + line: 5, + messageId: 'floating', + suggestions: [ + { + messageId: 'floatingFixAwait', + output: ` +async function foo() { + const myPromise = Promise.resolve(true); + let condition = false; + await (condition || myPromise); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = Promise.resolve(true); + let condition = null; + condition ?? myPromise; +} + `, + options: [{ ignoreVoid: false }], + errors: [ + { + line: 5, + messageId: 'floating', + suggestions: [ + { + messageId: 'floatingFixAwait', + output: ` +async function foo() { + const myPromise = Promise.resolve(true); + let condition = null; + await (condition ?? myPromise); +} + `, + }, + ], + }, + ], + }, + { + code: ` +async function foo() { + const myPromise = async () => void 0; + const condition = false; + + condition || condition || myPromise(); +} + `, + errors: [ + { + line: 6, + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +async function foo() { + const myPromise = async () => void 0; + const condition = false; + + void (condition || condition || myPromise()); +} + `, + }, + ], + }, + ], + }, ], });