From 4477fb74ae8beb8b5fda778ea8ef46ef332d55f6 Mon Sep 17 00:00:00 2001 From: islandryu Date: Fri, 7 Jan 2022 17:04:39 +0900 Subject: [PATCH 1/3] fix(eslint-plugin): return-await in binary expression --- .../eslint-plugin/src/rules/return-await.ts | 35 ++++++++- .../tests/rules/return-await.test.ts | 76 +++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index 93b863757a0..67595e47b09 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -4,6 +4,7 @@ import { TSESTree, } from '@typescript-eslint/experimental-utils'; import * as tsutils from 'tsutils'; +import { isBinaryExpression } from 'tsutils'; import * as ts from 'typescript'; import * as util from '../util'; @@ -154,6 +155,12 @@ export default util.createRule({ fixer: TSESLint.RuleFixer, node: TSESTree.Expression, ): TSESLint.RuleFix | TSESLint.RuleFix[] { + if (node.parent?.type === AST_NODE_TYPES.LogicalExpression) { + return [ + fixer.insertTextBefore(node, '(await '), + fixer.insertTextAfter(node, ')'), + ]; + } if (node.type !== AST_NODE_TYPES.TSAsExpression) { return fixer.insertTextBefore(node, 'await '); } @@ -259,6 +266,22 @@ export default util.createRule({ } } + function testLogicalExpression( + node: TSESTree.LogicalExpression, + tsNode: ts.BinaryExpression, + ): void { + const left = node.left; + const tsLeft = tsNode.left; + const right = node.right; + const tsRight = tsNode.right; + test(right, tsRight); + if (isLogicalExpression(left) && isBinaryExpression(tsLeft)) { + testLogicalExpression(left, tsLeft); + } else { + test(left, tsLeft); + } + } + function findPossiblyReturnedNodes( node: TSESTree.Expression, ): TSESTree.Expression[] { @@ -298,9 +321,19 @@ export default util.createRule({ } findPossiblyReturnedNodes(node.argument).forEach(node => { const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); - test(node, tsNode); + if (isLogicalExpression(node) && isBinaryExpression(tsNode)) { + testLogicalExpression(node, tsNode); + } else { + test(node, tsNode); + } }); }, }; }, }); + +function isLogicalExpression( + node: TSESTree.Node, +): node is TSESTree.LogicalExpression { + return node.type === AST_NODE_TYPES.LogicalExpression; +} diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index 1c0a9cdd2a1..83b27e0e69d 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -909,5 +909,81 @@ async function test(): Promise { }, ], }, + { + code: ` + async function bar() {} + async function foo() { + try { + return undefined || bar(); + } catch {} + } + `, + output: ` + async function bar() {} + async function foo() { + try { + return undefined || (await bar()); + } catch {} + } + `, + errors: [ + { + line: 5, + messageId: 'requiredPromiseAwait', + }, + ], + }, + { + code: ` + async function bar() {} + async function foo() { + try { + return bar() || undefined || bar(); + } catch {} + } + `, + output: ` + async function bar() {} + async function foo() { + try { + return (await bar()) || undefined || (await bar()); + } catch {} + } + `, + errors: [ + { + line: 5, + messageId: 'requiredPromiseAwait', + }, + { + line: 5, + messageId: 'requiredPromiseAwait', + }, + ], + }, + { + code: ` + async function bar() {} + async function foo() { + try { + return window.foo ?? bar(); + } catch {} + } + `, + output: ` + async function bar() {} + async function foo() { + try { + return window.foo ?? (await bar()); + } catch {} + } + `, + errors: [ + { + line: 5, + messageId: 'requiredPromiseAwait', + }, + ], + }, ], }); From 9c309159b38e4c5be9b83de7dc5d6136179ebf7b Mon Sep 17 00:00:00 2001 From: islandryu Date: Tue, 18 Jan 2022 20:27:30 +0900 Subject: [PATCH 2/3] fix(eslint-plugin): [return-await] add ( ) to Expressions with lower precedence than awaitExpressions --- .../eslint-plugin/src/rules/return-await.ts | 60 ++- .../src/util/getOperatorPrecedence.ts | 347 ++++++++++++++++++ .../tests/rules/return-await.test.ts | 12 +- 3 files changed, 373 insertions(+), 46 deletions(-) create mode 100644 packages/eslint-plugin/src/util/getOperatorPrecedence.ts diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index 67595e47b09..450d6305eae 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -7,6 +7,7 @@ import * as tsutils from 'tsutils'; import { isBinaryExpression } from 'tsutils'; import * as ts from 'typescript'; import * as util from '../util'; +import { getOperatorPrecedence } from '../util/getOperatorPrecedence'; type FunctionNode = | TSESTree.FunctionDeclaration @@ -154,21 +155,28 @@ export default util.createRule({ function insertAwait( fixer: TSESLint.RuleFixer, node: TSESTree.Expression, + isHighPrecendence: boolean, ): TSESLint.RuleFix | TSESLint.RuleFix[] { - if (node.parent?.type === AST_NODE_TYPES.LogicalExpression) { + if (isHighPrecendence) { + return fixer.insertTextBefore(node, 'await '); + } else { return [ - fixer.insertTextBefore(node, '(await '), + fixer.insertTextBefore(node, 'await ('), fixer.insertTextAfter(node, ')'), ]; } - if (node.type !== AST_NODE_TYPES.TSAsExpression) { - return fixer.insertTextBefore(node, 'await '); - } + } - return [ - fixer.insertTextBefore(node, 'await ('), - fixer.insertTextAfter(node, ')'), - ]; + function isHigherPrecedenceThanAwait(node: ts.Node): boolean { + const operator = isBinaryExpression(node) + ? node.operatorToken.kind + : ts.SyntaxKind.Unknown; + const nodePrecedence = getOperatorPrecedence(node.kind, operator); + const awaitPrecedence = getOperatorPrecedence( + ts.SyntaxKind.AwaitExpression, + ts.SyntaxKind.Unknown, + ); + return nodePrecedence > awaitPrecedence; } function test(node: TSESTree.Expression, expression: ts.Node): void { @@ -219,7 +227,8 @@ export default util.createRule({ context.report({ messageId: 'requiredPromiseAwait', node, - fix: fixer => insertAwait(fixer, node), + fix: fixer => + insertAwait(fixer, node, isHigherPrecedenceThanAwait(expression)), }); } @@ -258,7 +267,8 @@ export default util.createRule({ context.report({ messageId: 'requiredPromiseAwait', node, - fix: fixer => insertAwait(fixer, node), + fix: fixer => + insertAwait(fixer, node, isHigherPrecedenceThanAwait(expression)), }); } @@ -266,22 +276,6 @@ export default util.createRule({ } } - function testLogicalExpression( - node: TSESTree.LogicalExpression, - tsNode: ts.BinaryExpression, - ): void { - const left = node.left; - const tsLeft = tsNode.left; - const right = node.right; - const tsRight = tsNode.right; - test(right, tsRight); - if (isLogicalExpression(left) && isBinaryExpression(tsLeft)) { - testLogicalExpression(left, tsLeft); - } else { - test(left, tsLeft); - } - } - function findPossiblyReturnedNodes( node: TSESTree.Expression, ): TSESTree.Expression[] { @@ -321,19 +315,9 @@ export default util.createRule({ } findPossiblyReturnedNodes(node.argument).forEach(node => { const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); - if (isLogicalExpression(node) && isBinaryExpression(tsNode)) { - testLogicalExpression(node, tsNode); - } else { - test(node, tsNode); - } + test(node, tsNode); }); }, }; }, }); - -function isLogicalExpression( - node: TSESTree.Node, -): node is TSESTree.LogicalExpression { - return node.type === AST_NODE_TYPES.LogicalExpression; -} diff --git a/packages/eslint-plugin/src/util/getOperatorPrecedence.ts b/packages/eslint-plugin/src/util/getOperatorPrecedence.ts new file mode 100644 index 00000000000..b7a9d75fb15 --- /dev/null +++ b/packages/eslint-plugin/src/util/getOperatorPrecedence.ts @@ -0,0 +1,347 @@ +import { SyntaxKind } from 'typescript'; + +export enum OperatorPrecedence { + // Expression: + // AssignmentExpression + // Expression `,` AssignmentExpression + Comma, + + // NOTE: `Spread` is higher than `Comma` due to how it is parsed in |ElementList| + // SpreadElement: + // `...` AssignmentExpression + Spread, + + // AssignmentExpression: + // ConditionalExpression + // YieldExpression + // ArrowFunction + // AsyncArrowFunction + // LeftHandSideExpression `=` AssignmentExpression + // LeftHandSideExpression AssignmentOperator AssignmentExpression + // + // NOTE: AssignmentExpression is broken down into several precedences due to the requirements + // of the parenthesize rules. + + // AssignmentExpression: YieldExpression + // YieldExpression: + // `yield` + // `yield` AssignmentExpression + // `yield` `*` AssignmentExpression + Yield, + + // AssignmentExpression: LeftHandSideExpression `=` AssignmentExpression + // AssignmentExpression: LeftHandSideExpression AssignmentOperator AssignmentExpression + // AssignmentOperator: one of + // `*=` `/=` `%=` `+=` `-=` `<<=` `>>=` `>>>=` `&=` `^=` `|=` `**=` + Assignment, + + // NOTE: `Conditional` is considered higher than `Assignment` here, but in reality they have + // the same precedence. + // AssignmentExpression: ConditionalExpression + // ConditionalExpression: + // ShortCircuitExpression + // ShortCircuitExpression `?` AssignmentExpression `:` AssignmentExpression + // ShortCircuitExpression: + // LogicalORExpression + // CoalesceExpression + Conditional, + + // CoalesceExpression: + // CoalesceExpressionHead `??` BitwiseORExpression + // CoalesceExpressionHead: + // CoalesceExpression + // BitwiseORExpression + Coalesce = Conditional, // NOTE: This is wrong + + // LogicalORExpression: + // LogicalANDExpression + // LogicalORExpression `||` LogicalANDExpression + LogicalOR, + + // LogicalANDExpression: + // BitwiseORExpression + // LogicalANDExpression `&&` BitwiseORExpression + LogicalAND, + + // BitwiseORExpression: + // BitwiseXORExpression + // BitwiseORExpression `^` BitwiseXORExpression + BitwiseOR, + + // BitwiseXORExpression: + // BitwiseANDExpression + // BitwiseXORExpression `^` BitwiseANDExpression + BitwiseXOR, + + // BitwiseANDExpression: + // EqualityExpression + // BitwiseANDExpression `^` EqualityExpression + BitwiseAND, + + // EqualityExpression: + // RelationalExpression + // EqualityExpression `==` RelationalExpression + // EqualityExpression `!=` RelationalExpression + // EqualityExpression `===` RelationalExpression + // EqualityExpression `!==` RelationalExpression + Equality, + + // RelationalExpression: + // ShiftExpression + // RelationalExpression `<` ShiftExpression + // RelationalExpression `>` ShiftExpression + // RelationalExpression `<=` ShiftExpression + // RelationalExpression `>=` ShiftExpression + // RelationalExpression `instanceof` ShiftExpression + // RelationalExpression `in` ShiftExpression + // [+TypeScript] RelationalExpression `as` Type + Relational, + + // ShiftExpression: + // AdditiveExpression + // ShiftExpression `<<` AdditiveExpression + // ShiftExpression `>>` AdditiveExpression + // ShiftExpression `>>>` AdditiveExpression + Shift, + + // AdditiveExpression: + // MultiplicativeExpression + // AdditiveExpression `+` MultiplicativeExpression + // AdditiveExpression `-` MultiplicativeExpression + Additive, + + // MultiplicativeExpression: + // ExponentiationExpression + // MultiplicativeExpression MultiplicativeOperator ExponentiationExpression + // MultiplicativeOperator: one of `*`, `/`, `%` + Multiplicative, + + // ExponentiationExpression: + // UnaryExpression + // UpdateExpression `**` ExponentiationExpression + Exponentiation, + + // UnaryExpression: + // UpdateExpression + // `delete` UnaryExpression + // `void` UnaryExpression + // `typeof` UnaryExpression + // `+` UnaryExpression + // `-` UnaryExpression + // `~` UnaryExpression + // `!` UnaryExpression + // AwaitExpression + // UpdateExpression: // TODO: Do we need to investigate the precedence here? + // `++` UnaryExpression + // `--` UnaryExpression + Unary, + + // UpdateExpression: + // LeftHandSideExpression + // LeftHandSideExpression `++` + // LeftHandSideExpression `--` + Update, + + // LeftHandSideExpression: + // NewExpression + // CallExpression + // NewExpression: + // MemberExpression + // `new` NewExpression + LeftHandSide, + + // CallExpression: + // CoverCallExpressionAndAsyncArrowHead + // SuperCall + // ImportCall + // CallExpression Arguments + // CallExpression `[` Expression `]` + // CallExpression `.` IdentifierName + // CallExpression TemplateLiteral + // MemberExpression: + // PrimaryExpression + // MemberExpression `[` Expression `]` + // MemberExpression `.` IdentifierName + // MemberExpression TemplateLiteral + // SuperProperty + // MetaProperty + // `new` MemberExpression Arguments + Member, + + // TODO: JSXElement? + // PrimaryExpression: + // `this` + // IdentifierReference + // Literal + // ArrayLiteral + // ObjectLiteral + // FunctionExpression + // ClassExpression + // GeneratorExpression + // AsyncFunctionExpression + // AsyncGeneratorExpression + // RegularExpressionLiteral + // TemplateLiteral + // CoverParenthesizedExpressionAndArrowParameterList + Primary, + + Highest = Primary, + Lowest = Comma, + // -1 is lower than all other precedences. Returning it will cause binary expression + // parsing to stop. + Invalid = -1, +} + +export function getOperatorPrecedence( + nodeKind: SyntaxKind, + operatorKind: SyntaxKind, + hasArguments?: boolean, +): OperatorPrecedence { + switch (nodeKind) { + case SyntaxKind.CommaListExpression: + return OperatorPrecedence.Comma; + + case SyntaxKind.SpreadElement: + return OperatorPrecedence.Spread; + + case SyntaxKind.YieldExpression: + return OperatorPrecedence.Yield; + + case SyntaxKind.ConditionalExpression: + return OperatorPrecedence.Conditional; + + case SyntaxKind.BinaryExpression: + switch (operatorKind) { + case SyntaxKind.CommaToken: + return OperatorPrecedence.Comma; + + case SyntaxKind.EqualsToken: + case SyntaxKind.PlusEqualsToken: + case SyntaxKind.MinusEqualsToken: + case SyntaxKind.AsteriskAsteriskEqualsToken: + case SyntaxKind.AsteriskEqualsToken: + case SyntaxKind.SlashEqualsToken: + case SyntaxKind.PercentEqualsToken: + case SyntaxKind.LessThanLessThanEqualsToken: + case SyntaxKind.GreaterThanGreaterThanEqualsToken: + case SyntaxKind.GreaterThanGreaterThanGreaterThanEqualsToken: + case SyntaxKind.AmpersandEqualsToken: + case SyntaxKind.CaretEqualsToken: + case SyntaxKind.BarEqualsToken: + case SyntaxKind.BarBarEqualsToken: + case SyntaxKind.AmpersandAmpersandEqualsToken: + case SyntaxKind.QuestionQuestionEqualsToken: + return OperatorPrecedence.Assignment; + + default: + return getBinaryOperatorPrecedence(operatorKind); + } + + // TODO: Should prefix `++` and `--` be moved to the `Update` precedence? + case SyntaxKind.TypeAssertionExpression: + case SyntaxKind.NonNullExpression: + case SyntaxKind.PrefixUnaryExpression: + case SyntaxKind.TypeOfExpression: + case SyntaxKind.VoidExpression: + case SyntaxKind.DeleteExpression: + case SyntaxKind.AwaitExpression: + return OperatorPrecedence.Unary; + + case SyntaxKind.PostfixUnaryExpression: + return OperatorPrecedence.Update; + + case SyntaxKind.CallExpression: + return OperatorPrecedence.LeftHandSide; + + case SyntaxKind.NewExpression: + return hasArguments + ? OperatorPrecedence.Member + : OperatorPrecedence.LeftHandSide; + + case SyntaxKind.TaggedTemplateExpression: + case SyntaxKind.PropertyAccessExpression: + case SyntaxKind.ElementAccessExpression: + case SyntaxKind.MetaProperty: + return OperatorPrecedence.Member; + + case SyntaxKind.AsExpression: + return OperatorPrecedence.Relational; + + case SyntaxKind.ThisKeyword: + case SyntaxKind.SuperKeyword: + case SyntaxKind.Identifier: + case SyntaxKind.PrivateIdentifier: + case SyntaxKind.NullKeyword: + case SyntaxKind.TrueKeyword: + case SyntaxKind.FalseKeyword: + case SyntaxKind.NumericLiteral: + case SyntaxKind.BigIntLiteral: + case SyntaxKind.StringLiteral: + case SyntaxKind.ArrayLiteralExpression: + case SyntaxKind.ObjectLiteralExpression: + case SyntaxKind.FunctionExpression: + case SyntaxKind.ArrowFunction: + case SyntaxKind.ClassExpression: + case SyntaxKind.RegularExpressionLiteral: + case SyntaxKind.NoSubstitutionTemplateLiteral: + case SyntaxKind.TemplateExpression: + case SyntaxKind.ParenthesizedExpression: + case SyntaxKind.OmittedExpression: + case SyntaxKind.JsxElement: + case SyntaxKind.JsxSelfClosingElement: + case SyntaxKind.JsxFragment: + return OperatorPrecedence.Primary; + + default: + return OperatorPrecedence.Invalid; + } +} + +export function getBinaryOperatorPrecedence( + kind: SyntaxKind, +): OperatorPrecedence { + switch (kind) { + case SyntaxKind.QuestionQuestionToken: + return OperatorPrecedence.Coalesce; + case SyntaxKind.BarBarToken: + return OperatorPrecedence.LogicalOR; + case SyntaxKind.AmpersandAmpersandToken: + return OperatorPrecedence.LogicalAND; + case SyntaxKind.BarToken: + return OperatorPrecedence.BitwiseOR; + case SyntaxKind.CaretToken: + return OperatorPrecedence.BitwiseXOR; + case SyntaxKind.AmpersandToken: + return OperatorPrecedence.BitwiseAND; + case SyntaxKind.EqualsEqualsToken: + case SyntaxKind.ExclamationEqualsToken: + case SyntaxKind.EqualsEqualsEqualsToken: + case SyntaxKind.ExclamationEqualsEqualsToken: + return OperatorPrecedence.Equality; + case SyntaxKind.LessThanToken: + case SyntaxKind.GreaterThanToken: + case SyntaxKind.LessThanEqualsToken: + case SyntaxKind.GreaterThanEqualsToken: + case SyntaxKind.InstanceOfKeyword: + case SyntaxKind.InKeyword: + case SyntaxKind.AsKeyword: + return OperatorPrecedence.Relational; + case SyntaxKind.LessThanLessThanToken: + case SyntaxKind.GreaterThanGreaterThanToken: + case SyntaxKind.GreaterThanGreaterThanGreaterThanToken: + return OperatorPrecedence.Shift; + case SyntaxKind.PlusToken: + case SyntaxKind.MinusToken: + return OperatorPrecedence.Additive; + case SyntaxKind.AsteriskToken: + case SyntaxKind.SlashToken: + case SyntaxKind.PercentToken: + return OperatorPrecedence.Multiplicative; + case SyntaxKind.AsteriskAsteriskToken: + return OperatorPrecedence.Exponentiation; + } + + // -1 is lower than all other precedences. Returning it will cause binary expression + // parsing to stop. + return -1; +} diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index 83b27e0e69d..b1b78a7ae0b 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -922,7 +922,7 @@ async function test(): Promise { async function bar() {} async function foo() { try { - return undefined || (await bar()); + return await (undefined || bar()); } catch {} } `, @@ -946,7 +946,7 @@ async function test(): Promise { async function bar() {} async function foo() { try { - return (await bar()) || undefined || (await bar()); + return await (bar() || undefined || bar()); } catch {} } `, @@ -955,10 +955,6 @@ async function test(): Promise { line: 5, messageId: 'requiredPromiseAwait', }, - { - line: 5, - messageId: 'requiredPromiseAwait', - }, ], }, { @@ -966,7 +962,7 @@ async function test(): Promise { async function bar() {} async function foo() { try { - return window.foo ?? bar(); + return null ?? bar(); } catch {} } `, @@ -974,7 +970,7 @@ async function test(): Promise { async function bar() {} async function foo() { try { - return window.foo ?? (await bar()); + return await (null ?? bar()); } catch {} } `, From 6402c6f31b63d9dfd25315494ebc2c5a33cd3dd6 Mon Sep 17 00:00:00 2001 From: islandryu Date: Sun, 30 Jan 2022 16:19:13 +0900 Subject: [PATCH 3/3] fix(eslint-plugin): [return-await] add test --- .../tests/rules/return-await.test.ts | 70 ++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index b1b78a7ae0b..43db87ef819 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -960,25 +960,91 @@ async function test(): Promise { { code: ` async function bar() {} - async function foo() { + async function func1() { try { return null ?? bar(); } catch {} } + async function func2() { + try { + return 1 && bar(); + } catch {} + } + const foo = { + bar: async function () {}, + }; + async function func3() { + try { + return foo.bar(); + } catch {} + } `, output: ` async function bar() {} - async function foo() { + async function func1() { try { return await (null ?? bar()); } catch {} } + async function func2() { + try { + return await (1 && bar()); + } catch {} + } + const foo = { + bar: async function () {}, + }; + async function func3() { + try { + return await foo.bar(); + } catch {} + } `, errors: [ { line: 5, messageId: 'requiredPromiseAwait', }, + { + line: 10, + messageId: 'requiredPromiseAwait', + }, + { + line: 18, + messageId: 'requiredPromiseAwait', + }, + ], + }, + { + code: ` + class X { + async bar() { + return; + } + async func2() { + try { + return this.bar(); + } catch {} + } + } + `, + output: ` + class X { + async bar() { + return; + } + async func2() { + try { + return await this.bar(); + } catch {} + } + } + `, + errors: [ + { + line: 8, + messageId: 'requiredPromiseAwait', + }, ], }, ],