From f36ec87b7112abd4e2b612db4da5d0a65a8edeb1 Mon Sep 17 00:00:00 2001 From: thomasmichaelwallace Date: Tue, 13 Oct 2020 16:00:38 +0100 Subject: [PATCH 1/4] fix(eslint-plugin): [return-await] do not auto-fix against any --- .../eslint-plugin/src/rules/return-await.ts | 64 +++++++++++++++++-- .../tests/rules/return-await.test.ts | 23 ++++++- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index edbfcb593ed..10b6f848558 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -1,7 +1,7 @@ import { AST_NODE_TYPES, - TSESTree, TSESLint, + TSESTree, } from '@typescript-eslint/experimental-utils'; import * as tsutils from 'tsutils'; import * as ts from 'typescript'; @@ -169,11 +169,27 @@ export default util.createRule({ return; } + // any could be thenable; do not auto-fix + const useAutoFix = !util.isTypeAnyType(type); + const removeFix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix | null => + removeAwait(fixer, node); + const insertFix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix | null => + insertAwait(fixer, node); + if (isAwait && !isThenable) { context.report({ messageId: 'nonPromiseAwait', node, - fix: fixer => removeAwait(fixer, node), + ...(useAutoFix + ? { fix: removeFix } + : { + suggest: [ + { + messageId: 'nonPromiseAwait', + fix: removeFix, + }, + ], + }), }); return; } @@ -183,7 +199,16 @@ export default util.createRule({ context.report({ messageId: 'requiredPromiseAwait', node, - fix: fixer => insertAwait(fixer, node), + ...(useAutoFix + ? { fix: insertFix } + : { + suggest: [ + { + messageId: 'requiredPromiseAwait', + fix: insertFix, + }, + ], + }), }); } @@ -195,7 +220,16 @@ export default util.createRule({ context.report({ messageId: 'disallowedPromiseAwait', node, - fix: fixer => removeAwait(fixer, node), + ...(useAutoFix + ? { fix: removeFix } + : { + suggest: [ + { + messageId: 'disallowedPromiseAwait', + fix: removeFix, + }, + ], + }), }); } @@ -208,7 +242,16 @@ export default util.createRule({ context.report({ messageId: 'disallowedPromiseAwait', node, - fix: fixer => removeAwait(fixer, node), + ...(useAutoFix + ? { fix: removeFix } + : { + suggest: [ + { + messageId: 'disallowedPromiseAwait', + fix: removeFix, + }, + ], + }), }); } else if (!isAwait && isInTryCatch) { if (inCatch(expression) && !hasFinallyBlock(expression)) { @@ -222,7 +265,16 @@ export default util.createRule({ context.report({ messageId: 'requiredPromiseAwait', node, - fix: fixer => insertAwait(fixer, node), + ...(useAutoFix + ? { fix: insertFix } + : { + suggest: [ + { + messageId: 'requiredPromiseAwait', + fix: insertFix, + }, + ], + }), }); } diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index 887768a6106..4453db6609d 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -1,5 +1,5 @@ import rule from '../../src/rules/return-await'; -import { getFixturesRootDir, RuleTester, noFormat } from '../RuleTester'; +import { getFixturesRootDir, noFormat, RuleTester } from '../RuleTester'; const rootDir = getFixturesRootDir(); @@ -312,6 +312,27 @@ ruleTester.run('return-await', rule, { }, ], }, + { + code: ` + const fn = (): any => null; + async function test() { + return await fn(); + } + `, + // output matches input because return type any is suggestion only + output: ` + const fn = (): any => null; + async function test() { + return await fn(); + } + `, + errors: [ + { + line: 4, + messageId: 'nonPromiseAwait', + }, + ], + }, { code: 'const test = async () => await Promise.resolve(1);', output: 'const test = async () => Promise.resolve(1);', From 832392b1ea97d2b0b81b2020a06ab90607d2e40e Mon Sep 17 00:00:00 2001 From: thomasmichaelwallace Date: Tue, 13 Oct 2020 16:58:13 +0100 Subject: [PATCH 2/4] fix: auto-fix branch is only required on non-promise-await --- .../eslint-plugin/src/rules/return-await.ts | 47 ++----------------- 1 file changed, 4 insertions(+), 43 deletions(-) diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index 10b6f848558..3229c3a4487 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -173,9 +173,6 @@ export default util.createRule({ const useAutoFix = !util.isTypeAnyType(type); const removeFix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix | null => removeAwait(fixer, node); - const insertFix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix | null => - insertAwait(fixer, node); - if (isAwait && !isThenable) { context.report({ messageId: 'nonPromiseAwait', @@ -199,16 +196,7 @@ export default util.createRule({ context.report({ messageId: 'requiredPromiseAwait', node, - ...(useAutoFix - ? { fix: insertFix } - : { - suggest: [ - { - messageId: 'requiredPromiseAwait', - fix: insertFix, - }, - ], - }), + fix: fixer => insertAwait(fixer, node), }); } @@ -220,16 +208,7 @@ export default util.createRule({ context.report({ messageId: 'disallowedPromiseAwait', node, - ...(useAutoFix - ? { fix: removeFix } - : { - suggest: [ - { - messageId: 'disallowedPromiseAwait', - fix: removeFix, - }, - ], - }), + fix: fixer => removeAwait(fixer, node), }); } @@ -242,16 +221,7 @@ export default util.createRule({ context.report({ messageId: 'disallowedPromiseAwait', node, - ...(useAutoFix - ? { fix: removeFix } - : { - suggest: [ - { - messageId: 'disallowedPromiseAwait', - fix: removeFix, - }, - ], - }), + fix: fixer => removeAwait(fixer, node), }); } else if (!isAwait && isInTryCatch) { if (inCatch(expression) && !hasFinallyBlock(expression)) { @@ -265,16 +235,7 @@ export default util.createRule({ context.report({ messageId: 'requiredPromiseAwait', node, - ...(useAutoFix - ? { fix: insertFix } - : { - suggest: [ - { - messageId: 'requiredPromiseAwait', - fix: insertFix, - }, - ], - }), + fix: fixer => insertAwait(fixer, node), }); } From 9970d26bea4be759c6a8712c57b4993bf7089134 Mon Sep 17 00:00:00 2001 From: thomasmichaelwallace Date: Tue, 13 Oct 2020 17:36:16 +0100 Subject: [PATCH 3/4] fix: suggest on unknown as well as any --- .../eslint-plugin/src/rules/return-await.ts | 15 +++++++------ .../tests/rules/return-await.test.ts | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index 3229c3a4487..c8e7f102b09 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -169,21 +169,24 @@ export default util.createRule({ return; } - // any could be thenable; do not auto-fix - const useAutoFix = !util.isTypeAnyType(type); - const removeFix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix | null => - removeAwait(fixer, node); if (isAwait && !isThenable) { + // any/unknown could be thenable; do not auto-fix + const useAutoFix = !( + util.isTypeAnyType(type) || util.isTypeUnknownType(type) + ); + const fix = (fixer: TSESLint.RuleFixer): TSESLint.RuleFix | null => + removeAwait(fixer, node); + context.report({ messageId: 'nonPromiseAwait', node, ...(useAutoFix - ? { fix: removeFix } + ? { fix } : { suggest: [ { messageId: 'nonPromiseAwait', - fix: removeFix, + fix, }, ], }), diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index 4453db6609d..ab65b972e18 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -333,6 +333,27 @@ ruleTester.run('return-await', rule, { }, ], }, + { + code: ` + const fn = (): unknown => null; + async function test() { + return await fn(); + } + `, + // output matches input because return type unknown is suggestion only + output: ` + const fn = (): unknown => null; + async function test() { + return await fn(); + } + `, + errors: [ + { + line: 4, + messageId: 'nonPromiseAwait', + }, + ], + }, { code: 'const test = async () => await Promise.resolve(1);', output: 'const test = async () => Promise.resolve(1);', From ae8a8882f14eba4f7951f94b303912b2e0008b3e Mon Sep 17 00:00:00 2001 From: thomasmichaelwallace Date: Tue, 13 Oct 2020 17:52:29 +0100 Subject: [PATCH 4/4] refactor: use explicit suggestion test --- .../tests/rules/return-await.test.ts | 56 +++++++++++-------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index ab65b972e18..73056f6091c 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -314,43 +314,51 @@ ruleTester.run('return-await', rule, { }, { code: ` - const fn = (): any => null; - async function test() { - return await fn(); - } - `, - // output matches input because return type any is suggestion only - output: ` - const fn = (): any => null; - async function test() { - return await fn(); - } - `, +const fn = (): any => null; +async function test() { + return await fn(); +} + `.trimRight(), errors: [ { line: 4, messageId: 'nonPromiseAwait', + suggestions: [ + { + messageId: 'nonPromiseAwait', + output: ` +const fn = (): any => null; +async function test() { + return fn(); +} + `.trimRight(), + }, + ], }, ], }, { code: ` - const fn = (): unknown => null; - async function test() { - return await fn(); - } - `, - // output matches input because return type unknown is suggestion only - output: ` - const fn = (): unknown => null; - async function test() { - return await fn(); - } - `, +const fn = (): unknown => null; +async function test() { + return await fn(); +} + `.trimRight(), errors: [ { line: 4, messageId: 'nonPromiseAwait', + suggestions: [ + { + messageId: 'nonPromiseAwait', + output: ` +const fn = (): unknown => null; +async function test() { + return fn(); +} + `.trimRight(), + }, + ], }, ], },