From e752f70ffbb0b1f800db659fafc4b4bcd0197fcd Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 14:21:38 -0700 Subject: [PATCH 1/7] Add remove unnecessary await fix --- src/compiler/checker.ts | 6 +++- src/compiler/diagnosticMessages.json | 13 ++++++++ .../codefixes/removeUnnecessaryAwait.ts | 32 +++++++++++++++++++ src/services/tsconfig.json | 1 + .../codeFixRemoveUnnecessaryAwait.ts | 25 +++++++++++++++ 5 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 src/services/codefixes/removeUnnecessaryAwait.ts create mode 100644 tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b761606fcb3ef..e7655f8962edd 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -26403,7 +26403,11 @@ namespace ts { * The runtime behavior of the `await` keyword. */ function checkAwaitedType(type: Type, errorNode: Node, diagnosticMessage: DiagnosticMessage, arg0?: string | number): Type { - return getAwaitedType(type, errorNode, diagnosticMessage, arg0) || errorType; + const awaitedType = getAwaitedType(type, errorNode, diagnosticMessage, arg0); + if (awaitedType === type) { + addErrorOrSuggestion(/*isError*/ false, createDiagnosticForNode(errorNode, Diagnostics.await_has_no_effect_on_the_type_of_this_expression)); + } + return awaitedType || errorType; } function getAwaitedType(type: Type, errorNode?: Node, diagnosticMessage?: DiagnosticMessage, arg0?: string | number): Type | undefined { diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 7fc2fdd4237e1..7a6b798e2aee3 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4635,6 +4635,11 @@ "category": "Suggestion", "code": 80006 }, + "'await' has no effect on the type of this expression.": { + "category": "Suggestion", + "code": 80007 + }, + "Add missing 'super()' call": { "category": "Message", "code": 90001 @@ -5095,6 +5100,14 @@ "category": "Message", "code": 95085 }, + "Remove unnecessary 'await'": { + "category": "Message", + "code": 95086 + }, + "Remove all unnecessary uses of 'await'": { + "category": "Message", + "code": 95087 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/codefixes/removeUnnecessaryAwait.ts b/src/services/codefixes/removeUnnecessaryAwait.ts new file mode 100644 index 0000000000000..fb6cc6f4bd260 --- /dev/null +++ b/src/services/codefixes/removeUnnecessaryAwait.ts @@ -0,0 +1,32 @@ +/* @internal */ +namespace ts.codefix { + const fixId = "removeUnnecessaryAwait"; + const errorCodes = [ + Diagnostics.await_has_no_effect_on_the_type_of_this_expression.code, + ]; + + registerCodeFix({ + errorCodes, + getCodeActions: (context) => { + const changes = textChanges.ChangeTracker.with(context, t => makeChange(t, context.sourceFile, context.span)); + if (changes.length > 0) { + return [createCodeFixAction(fixId, changes, Diagnostics.Remove_unnecessary_await, fixId, Diagnostics.Remove_all_unnecessary_uses_of_await)]; + } + }, + fixIds: [fixId], + getAllCodeActions: context => { + return codeFixAll(context, errorCodes, (changes, diag) => makeChange(changes, diag.file, diag)); + }, + }); + + function makeChange(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, span: TextSpan) { + const awaitKeyword = tryCast(getTokenAtPosition(sourceFile, span.start), (node): node is AwaitKeywordToken => node.kind === SyntaxKind.AwaitKeyword); + const awaitExpression = awaitKeyword && tryCast(awaitKeyword.parent, isAwaitExpression); + if (!awaitExpression) { + return; + } + + const parenthesizedExpression = awaitExpression && tryCast(awaitExpression.parent, isParenthesizedExpression); + changeTracker.replaceNode(sourceFile, parenthesizedExpression || awaitExpression, awaitExpression.expression); + } +} diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 05c1b0b482ea0..c2a6946b5ed77 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -80,6 +80,7 @@ "codefixes/useDefaultImport.ts", "codefixes/fixAddModuleReferTypeMissingTypeof.ts", "codefixes/convertToMappedObjectType.ts", + "codefixes/removeUnnecessaryAwait.ts", "refactors/convertExport.ts", "refactors/convertImport.ts", "refactors/extractSymbol.ts", diff --git a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts new file mode 100644 index 0000000000000..743d98afb0952 --- /dev/null +++ b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts @@ -0,0 +1,25 @@ +/// +////async function f() { +//// await ""; +//// await 0; +////} + +verify.codeFix({ + description: ts.Diagnostics.Remove_unnecessary_await.message, + index: 0, + newFileContent: +`async function f() { + ""; + await 0; +}` +}); + +verify.codeFixAll({ + fixAllDescription: ts.Diagnostics.Remove_all_unnecessary_uses_of_await.message, + fixId: "removeUnnecessaryAwait", + newFileContent: +`async function f() { + ""; + 0; +}` +}); From 38247e0f89c14edfe8fc0ecf4647daedcc7f16f2 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 14:32:02 -0700 Subject: [PATCH 2/7] Add test for removing unnecessary parens after await is gone --- tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts index 743d98afb0952..9dadda3c77d04 100644 --- a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts +++ b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts @@ -2,6 +2,7 @@ ////async function f() { //// await ""; //// await 0; +//// (await "").toLowerCase(); ////} verify.codeFix({ @@ -11,6 +12,7 @@ verify.codeFix({ `async function f() { ""; await 0; + (await "").toLowerCase(); }` }); @@ -21,5 +23,6 @@ verify.codeFixAll({ `async function f() { ""; 0; + "".toLowerCase(); }` }); From e0a781e20c69c445701f702b42d9b5d3856a4b66 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 16:15:01 -0700 Subject: [PATCH 3/7] Fix handling of numbers in property access expressions --- src/services/codefixes/removeUnnecessaryAwait.ts | 10 ++++++++-- src/services/utilities.ts | 4 ++++ tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts | 6 ++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/services/codefixes/removeUnnecessaryAwait.ts b/src/services/codefixes/removeUnnecessaryAwait.ts index fb6cc6f4bd260..03707a0928cbc 100644 --- a/src/services/codefixes/removeUnnecessaryAwait.ts +++ b/src/services/codefixes/removeUnnecessaryAwait.ts @@ -26,7 +26,13 @@ namespace ts.codefix { return; } - const parenthesizedExpression = awaitExpression && tryCast(awaitExpression.parent, isParenthesizedExpression); - changeTracker.replaceNode(sourceFile, parenthesizedExpression || awaitExpression, awaitExpression.expression); + const parenthesizedExpression = tryCast(awaitExpression.parent, isParenthesizedExpression); + // (await 0).toFixed() should keep its parens (or add an extra dot for 0..toFixed()) + if (parenthesizedExpression && isPropertyAccessExpression(parenthesizedExpression.parent) && isDecimalIntegerLiteral(awaitExpression.expression)) { + changeTracker.replaceNode(sourceFile, awaitExpression, awaitExpression.expression); + } + else { + changeTracker.replaceNode(sourceFile, parenthesizedExpression || awaitExpression, awaitExpression.expression); + } } } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index cbadfbc9b53a8..5a39e79c08a6d 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1992,6 +1992,10 @@ namespace ts { return typeIsAccessible ? res : undefined; } + export function isDecimalIntegerLiteral(node: Node): node is NumericLiteral { + return isNumericLiteral(node) && /^\d+$/.test(node.text); + } + export function syntaxUsuallyHasTrailingSemicolon(kind: SyntaxKind) { return kind === SyntaxKind.VariableStatement || kind === SyntaxKind.ExpressionStatement diff --git a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts index 9dadda3c77d04..93c87a594350d 100644 --- a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts +++ b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts @@ -3,6 +3,8 @@ //// await ""; //// await 0; //// (await "").toLowerCase(); +//// (await 0).toFixed(); +//// (await 3.2).toFixed(); ////} verify.codeFix({ @@ -13,6 +15,8 @@ verify.codeFix({ ""; await 0; (await "").toLowerCase(); + (await 0).toFixed(); + (await 3.2).toFixed(); }` }); @@ -24,5 +28,7 @@ verify.codeFixAll({ ""; 0; "".toLowerCase(); + (0).toFixed(); + 3.2.toFixed(); }` }); From 58d960e3efc4eeeaa587daffb4b9a5e0c4dd6655 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 16:24:08 -0700 Subject: [PATCH 4/7] =?UTF-8?q?Don=E2=80=99t=20offer=20suggestion=20when?= =?UTF-8?q?=20awaited=20type=20is=20any/unknown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e7655f8962edd..a79a02cba0796 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -26404,7 +26404,7 @@ namespace ts { */ function checkAwaitedType(type: Type, errorNode: Node, diagnosticMessage: DiagnosticMessage, arg0?: string | number): Type { const awaitedType = getAwaitedType(type, errorNode, diagnosticMessage, arg0); - if (awaitedType === type) { + if (awaitedType === type && !(type.flags & TypeFlags.AnyOrUnknown)) { addErrorOrSuggestion(/*isError*/ false, createDiagnosticForNode(errorNode, Diagnostics.await_has_no_effect_on_the_type_of_this_expression)); } return awaitedType || errorType; From a54ed2d4b1b8bcb4490fa96a821873ad2e3dd13b Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 11 Jul 2019 16:38:49 -0700 Subject: [PATCH 5/7] Fix random other test --- .../fourslash/convertFunctionToEs6Class_asyncMethods.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts b/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts index fb3b46763c878..bb8f8c229621f 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class_asyncMethods.ts @@ -2,13 +2,14 @@ // @allowNonTsExtensions: true // @Filename: test123.js +// @lib: es5 ////export function /**/MyClass() { ////} ////MyClass.prototype.foo = async function() { -//// await 2; +//// await Promise.resolve(); ////} ////MyClass.bar = async function() { -//// await 3; +//// await Promise.resolve(); ////} verify.codeFix({ @@ -18,10 +19,10 @@ verify.codeFix({ constructor() { } async foo() { - await 2; + await Promise.resolve(); } static async bar() { - await 3; + await Promise.resolve(); } } `, From e27d260117dda461a759d4767f9e76dea23a4b9d Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 12 Jul 2019 08:35:37 -0700 Subject: [PATCH 6/7] Fix new expression edge cases --- .../codefixes/removeUnnecessaryAwait.ts | 26 +++++++++++++++++-- src/services/utilities.ts | 4 +-- .../codeFixRemoveUnnecessaryAwait.ts | 16 ++++++++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/services/codefixes/removeUnnecessaryAwait.ts b/src/services/codefixes/removeUnnecessaryAwait.ts index 03707a0928cbc..93e76198a5341 100644 --- a/src/services/codefixes/removeUnnecessaryAwait.ts +++ b/src/services/codefixes/removeUnnecessaryAwait.ts @@ -27,12 +27,34 @@ namespace ts.codefix { } const parenthesizedExpression = tryCast(awaitExpression.parent, isParenthesizedExpression); - // (await 0).toFixed() should keep its parens (or add an extra dot for 0..toFixed()) - if (parenthesizedExpression && isPropertyAccessExpression(parenthesizedExpression.parent) && isDecimalIntegerLiteral(awaitExpression.expression)) { + const preserveParens = parenthesizedExpression && ( + // (await 0).toFixed() should keep its parens (or add an extra dot for 0..toFixed()) + isPropertyAccessExpression(parenthesizedExpression.parent) && isDecimalIntegerLiteral(awaitExpression.expression, sourceFile) || + // new (await c).Class() + isPropertyAccessExpressionInNewExpression(parenthesizedExpression.parent) || + // (await new C).foo + isNewExpressionWithoutParens(awaitExpression.expression) + ); + + if (preserveParens) { changeTracker.replaceNode(sourceFile, awaitExpression, awaitExpression.expression); } else { changeTracker.replaceNode(sourceFile, parenthesizedExpression || awaitExpression, awaitExpression.expression); } } + + function isPropertyAccessExpressionInNewExpression(expression: Node) { + return isPropertyAccessExpression(expression) && !!findAncestor(expression, ancestor => { + return isPropertyAccessExpression(ancestor) + ? false + : isNewExpression(ancestor) + ? true + : "quit"; + }); + } + + function isNewExpressionWithoutParens(expression: Node) { + return isNewExpression(expression) && expression.getLastToken() === expression.expression; + } } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 5a39e79c08a6d..0dd15bcbab581 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1992,8 +1992,8 @@ namespace ts { return typeIsAccessible ? res : undefined; } - export function isDecimalIntegerLiteral(node: Node): node is NumericLiteral { - return isNumericLiteral(node) && /^\d+$/.test(node.text); + export function isDecimalIntegerLiteral(node: Node, sourceFile: SourceFile): node is NumericLiteral { + return isNumericLiteral(node) && /^\d+$/.test(node.getText(sourceFile)); } export function syntaxUsuallyHasTrailingSemicolon(kind: SyntaxKind) { diff --git a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts index 93c87a594350d..021a8f2a67608 100644 --- a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts +++ b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts @@ -1,22 +1,30 @@ /// +////declare class C { foo(): void } +////declare function getC(): { foo: { Class: C } } ////async function f() { //// await ""; //// await 0; //// (await "").toLowerCase(); //// (await 0).toFixed(); //// (await 3.2).toFixed(); +//// (await new C).foo(); +//// new (await getC()).foo.Class(); ////} verify.codeFix({ description: ts.Diagnostics.Remove_unnecessary_await.message, index: 0, newFileContent: -`async function f() { +`declare class C { foo(): void } +declare function getC(): { foo: { Class: C } } +async function f() { ""; await 0; (await "").toLowerCase(); (await 0).toFixed(); (await 3.2).toFixed(); + (await new C).foo(); + new (await getC()).foo.Class(); }` }); @@ -24,11 +32,15 @@ verify.codeFixAll({ fixAllDescription: ts.Diagnostics.Remove_all_unnecessary_uses_of_await.message, fixId: "removeUnnecessaryAwait", newFileContent: -`async function f() { +`declare class C { foo(): void } +declare function getC(): { foo: { Class: C } } +async function f() { ""; 0; "".toLowerCase(); (0).toFixed(); 3.2.toFixed(); + (new C).foo(); + new (getC()).foo.Class(); }` }); From f22dc9d6c23fab2c190e23c27f81ab4efee4163b Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 12 Jul 2019 09:15:56 -0700 Subject: [PATCH 7/7] Only remove parens for identifiers and call expressions --- .../codefixes/removeUnnecessaryAwait.ts | 31 ++----------------- src/services/utilities.ts | 4 --- .../codeFixRemoveUnnecessaryAwait.ts | 18 ++++------- 3 files changed, 8 insertions(+), 45 deletions(-) diff --git a/src/services/codefixes/removeUnnecessaryAwait.ts b/src/services/codefixes/removeUnnecessaryAwait.ts index 93e76198a5341..c235d028efbce 100644 --- a/src/services/codefixes/removeUnnecessaryAwait.ts +++ b/src/services/codefixes/removeUnnecessaryAwait.ts @@ -27,34 +27,7 @@ namespace ts.codefix { } const parenthesizedExpression = tryCast(awaitExpression.parent, isParenthesizedExpression); - const preserveParens = parenthesizedExpression && ( - // (await 0).toFixed() should keep its parens (or add an extra dot for 0..toFixed()) - isPropertyAccessExpression(parenthesizedExpression.parent) && isDecimalIntegerLiteral(awaitExpression.expression, sourceFile) || - // new (await c).Class() - isPropertyAccessExpressionInNewExpression(parenthesizedExpression.parent) || - // (await new C).foo - isNewExpressionWithoutParens(awaitExpression.expression) - ); - - if (preserveParens) { - changeTracker.replaceNode(sourceFile, awaitExpression, awaitExpression.expression); - } - else { - changeTracker.replaceNode(sourceFile, parenthesizedExpression || awaitExpression, awaitExpression.expression); - } - } - - function isPropertyAccessExpressionInNewExpression(expression: Node) { - return isPropertyAccessExpression(expression) && !!findAncestor(expression, ancestor => { - return isPropertyAccessExpression(ancestor) - ? false - : isNewExpression(ancestor) - ? true - : "quit"; - }); - } - - function isNewExpressionWithoutParens(expression: Node) { - return isNewExpression(expression) && expression.getLastToken() === expression.expression; + const removeParens = parenthesizedExpression && (isIdentifier(awaitExpression.expression) || isCallExpression(awaitExpression.expression)); + changeTracker.replaceNode(sourceFile, removeParens ? parenthesizedExpression || awaitExpression : awaitExpression, awaitExpression.expression); } } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 0dd15bcbab581..cbadfbc9b53a8 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1992,10 +1992,6 @@ namespace ts { return typeIsAccessible ? res : undefined; } - export function isDecimalIntegerLiteral(node: Node, sourceFile: SourceFile): node is NumericLiteral { - return isNumericLiteral(node) && /^\d+$/.test(node.getText(sourceFile)); - } - export function syntaxUsuallyHasTrailingSemicolon(kind: SyntaxKind) { return kind === SyntaxKind.VariableStatement || kind === SyntaxKind.ExpressionStatement diff --git a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts index 021a8f2a67608..b8e1de4605e27 100644 --- a/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts +++ b/tests/cases/fourslash/codeFixRemoveUnnecessaryAwait.ts @@ -1,14 +1,12 @@ /// ////declare class C { foo(): void } -////declare function getC(): { foo: { Class: C } } +////declare function foo(): string; ////async function f() { //// await ""; //// await 0; -//// (await "").toLowerCase(); +//// (await foo()).toLowerCase(); //// (await 0).toFixed(); -//// (await 3.2).toFixed(); //// (await new C).foo(); -//// new (await getC()).foo.Class(); ////} verify.codeFix({ @@ -16,15 +14,13 @@ verify.codeFix({ index: 0, newFileContent: `declare class C { foo(): void } -declare function getC(): { foo: { Class: C } } +declare function foo(): string; async function f() { ""; await 0; - (await "").toLowerCase(); + (await foo()).toLowerCase(); (await 0).toFixed(); - (await 3.2).toFixed(); (await new C).foo(); - new (await getC()).foo.Class(); }` }); @@ -33,14 +29,12 @@ verify.codeFixAll({ fixId: "removeUnnecessaryAwait", newFileContent: `declare class C { foo(): void } -declare function getC(): { foo: { Class: C } } +declare function foo(): string; async function f() { ""; 0; - "".toLowerCase(); + foo().toLowerCase(); (0).toFixed(); - 3.2.toFixed(); (new C).foo(); - new (getC()).foo.Class(); }` });