From 96a0c368dbcb27392284e630143d10c61342a552 Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Wed, 6 Jun 2018 17:34:11 +0300 Subject: [PATCH 01/10] add noAsyncWithoutAwait rule --- src/configs/all.ts | 1 + .../noAsyncWithoutAwait.examples.ts | 47 ++++++++++ src/rules/noAsyncWithoutAwaitRule.ts | 92 +++++++++++++++++++ .../rules/no-async-without-await/test.ts.lint | 74 +++++++++++++++ test/rules/no-async-without-await/tslint.json | 5 + 5 files changed, 219 insertions(+) create mode 100644 src/rules/code-examples/noAsyncWithoutAwait.examples.ts create mode 100644 src/rules/noAsyncWithoutAwaitRule.ts create mode 100644 test/rules/no-async-without-await/test.ts.lint create mode 100644 test/rules/no-async-without-await/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index 31d6a37375f..1778df3c492 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -95,6 +95,7 @@ export const rules = { // "import-blacklist": no sensible default "label-position": true, "no-arg": true, + "no-async-without-await": true, "no-bitwise": true, "no-conditional-assignment": true, "no-console": true, diff --git a/src/rules/code-examples/noAsyncWithoutAwait.examples.ts b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts new file mode 100644 index 00000000000..5368926e1c7 --- /dev/null +++ b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts @@ -0,0 +1,47 @@ +/** + * @license + * Copyright 2018 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as Lint from "../../index"; + +export const codeExamples = [ + { + config: Lint.Utils.dedent` + "rules": { "no-async-without-await": true } + `, + description: "Do not use the async in case it is not actually needed", + fail: Lint.Utils.dedent` + async function f() { + fetch(); + } + + async function f() { + async function g() { + await h(); + } + } + `, + pass: Lint.Utils.dedent` + async function f() { + await fetch(); + } + + const f = async () => { + await fetch(); + }; + `, + }, +]; diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts new file mode 100644 index 00000000000..8df3bbaca75 --- /dev/null +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -0,0 +1,92 @@ +/** + * @license + * Copyright 2018 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as ts from "typescript"; +import * as Lint from "../index"; +import { codeExamples } from "./code-examples/noAsyncWithoutAwait.examples"; + +export class Rule extends Lint.Rules.AbstractRule { + public static metadata: Lint.IRuleMetadata = { + codeExamples, + description: "Do not write async functions that do not have an await statement in them", + hasFix: false, + optionExamples: [true], + options: null, + optionsDescription: "Not configurable.", + rationale: "Cleaner code, can possibly reduce transpiled output", + ruleName: "no-async-without-await", + type: "functionality", + typescriptOnly: false, + }; + + public static FAILURE_STRING = "Async functions with no await are not allowed."; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new Walk(sourceFile, this.getOptions())); + } +} + +class Walk extends Lint.RuleWalker { + protected visitFunctionDeclaration(node: ts.FunctionDeclaration) { + this.addFailureIfAsyncFunctionHasNoAwait(node); + super.visitFunctionDeclaration(node); + } + + protected visitArrowFunction(node: ts.ArrowFunction) { + this.addFailureIfAsyncFunctionHasNoAwait(node); + super.visitArrowFunction(node); + } + + protected visitMethodDeclaration(node: ts.MethodDeclaration) { + this.addFailureIfAsyncFunctionHasNoAwait(node); + super.visitMethodDeclaration(node); + } + + private isAsyncFunction(node: ts.Node): boolean { + return Boolean(this.getAsyncModifier(node)); + } + + private getAsyncModifier(node: ts.Node) { + return node.modifiers && node.modifiers.find((modifier) => modifier.kind === ts.SyntaxKind.AsyncKeyword); + } + + private isAwait(node: ts.Node): boolean { + return node.kind === ts.SyntaxKind.AwaitKeyword; + } + + private functionBlockHasAwait(node: ts.Node) { + if (this.isAwait(node)) { + return true; + } + + if (node.kind === ts.SyntaxKind.ArrowFunction || node.kind === ts.SyntaxKind.FunctionDeclaration) { + return false; + } + + const awaitInChildren: boolean[] = node.getChildren().map(this.functionBlockHasAwait.bind(this)); + return awaitInChildren.some(Boolean); + } + + private addFailureIfAsyncFunctionHasNoAwait(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { + if (this.isAsyncFunction(node) && !this.functionBlockHasAwait(node.body as ts.Node)) { + const asyncModifier = this.getAsyncModifier(node); + if (asyncModifier) { + this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); + } + } + } +} diff --git a/test/rules/no-async-without-await/test.ts.lint b/test/rules/no-async-without-await/test.ts.lint new file mode 100644 index 00000000000..46e5df62a2b --- /dev/null +++ b/test/rules/no-async-without-await/test.ts.lint @@ -0,0 +1,74 @@ +async function a(){ +~~~~~ [0] + let b = 1; + console.log(b); +} + +async function a(){ + let b = 1; + await console.log(b); +} + +async function a(){ + let b = 1; + console.log(await b()); +} + +async function a(){ +~~~~~ [0] + let b = 1; + let c = async () => { + await fetch(); + }; +} + +async function a(){ +~~~~~ [Async functions with no await are not allowed.] + let b = 1; + async function f() { + await fetch(); + }; +} + +function a(){ + let b = 1; + async function f() { + ~~~~~ [Async functions with no await are not allowed.] + fetch(); + }; +} + +const a = async () => { + ~~~~~ [Async functions with no await are not allowed.] + let b = 1; + console.log(b); +} + + +const a = async () => 1; + ~~~~~ [Async functions with no await are not allowed.] + +class A { + async b() { + ~~~~~ [Async functions with no await are not allowed.] + console.log(1); + } +} + +class A { + async b() { + await b(); + } +} + +async () => { + await a(); + class A { + async b() { + ~~~~~ [Async functions with no await are not allowed.] + console.log(1); + } + } +}; + +[0]: Async functions with no await are not allowed. diff --git a/test/rules/no-async-without-await/tslint.json b/test/rules/no-async-without-await/tslint.json new file mode 100644 index 00000000000..140ce628454 --- /dev/null +++ b/test/rules/no-async-without-await/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-async-without-await": true + } +} From 5b4e3c3c3f13b6467172c4d69917ecb5db8ebc37 Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Fri, 8 Jun 2018 17:57:19 +0300 Subject: [PATCH 02/10] fix lint --- src/rules/noAsyncWithoutAwaitRule.ts | 15 +++++++++++---- src/runner.ts | 2 ++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index 8df3bbaca75..b6572e3d26c 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -61,7 +61,11 @@ class Walk extends Lint.RuleWalker { } private getAsyncModifier(node: ts.Node) { - return node.modifiers && node.modifiers.find((modifier) => modifier.kind === ts.SyntaxKind.AsyncKeyword); + if (node.modifiers !== undefined) { + return node.modifiers.find((modifier) => modifier.kind === ts.SyntaxKind.AsyncKeyword); + } + + return undefined; } private isAwait(node: ts.Node): boolean { @@ -73,18 +77,21 @@ class Walk extends Lint.RuleWalker { return true; } - if (node.kind === ts.SyntaxKind.ArrowFunction || node.kind === ts.SyntaxKind.FunctionDeclaration) { + // tslint:disable-next-line:no-unsafe-any + if (node.kind === ts.SyntaxKind.ArrowFunction + || node.kind === ts.SyntaxKind.FunctionDeclaration) { return false; } - const awaitInChildren: boolean[] = node.getChildren().map(this.functionBlockHasAwait.bind(this)); + const awaitInChildren: boolean[] = node.getChildren() + .map((functionBlockNode: ts.Node) => this.functionBlockHasAwait(functionBlockNode)); return awaitInChildren.some(Boolean); } private addFailureIfAsyncFunctionHasNoAwait(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { if (this.isAsyncFunction(node) && !this.functionBlockHasAwait(node.body as ts.Node)) { const asyncModifier = this.getAsyncModifier(node); - if (asyncModifier) { + if (asyncModifier !== undefined) { this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); } } diff --git a/src/runner.ts b/src/runner.ts index 1e65dcd348b..244d8a28d34 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -156,6 +156,7 @@ async function runWorker(options: Options, logger: Logger): Promise { return options.force || errorCount === 0 ? Status.Ok : Status.LintError; } +// tslint:disable-next-line:no-async-without-await async function runLinter(options: Options, logger: Logger): Promise { const { files, program } = resolveFilesAndProgram(options, logger); // if type checking, run the type checker @@ -287,6 +288,7 @@ async function doLinting(options: Options, files: string[], program: ts.Program } /** Read a file, but return undefined if it is an MPEG '.ts' file. */ +// tslint:disable-next-line:no-async-without-await async function tryReadFile(filename: string, logger: Logger): Promise { if (!fs.existsSync(filename)) { throw new FatalError(`Unable to open file: ${filename}`); From 7f6d5c7d0eeb1794a1ce91e0abbb85f955a3db68 Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Mon, 18 Jun 2018 01:34:37 +0300 Subject: [PATCH 03/10] code review updates --- .../code-examples/noAsyncWithoutAwait.examples.ts | 2 +- src/rules/noAsyncWithoutAwaitRule.ts | 12 ++++++------ src/runner.ts | 4 ++-- test/rules/no-async-without-await/test.ts.lint | 14 +++++++------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/rules/code-examples/noAsyncWithoutAwait.examples.ts b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts index 5368926e1c7..35828204b74 100644 --- a/src/rules/code-examples/noAsyncWithoutAwait.examples.ts +++ b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts @@ -22,7 +22,7 @@ export const codeExamples = [ config: Lint.Utils.dedent` "rules": { "no-async-without-await": true } `, - description: "Do not use the async in case it is not actually needed", + description: "Do not use the async keyword in case it is not actually needed", fail: Lint.Utils.dedent` async function f() { fetch(); diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index b6572e3d26c..404047becd8 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -20,9 +20,11 @@ import * as Lint from "../index"; import { codeExamples } from "./code-examples/noAsyncWithoutAwait.examples"; export class Rule extends Lint.Rules.AbstractRule { + public static FAILURE_STRING = "Functions marked async must contain an await statement."; + public static metadata: Lint.IRuleMetadata = { codeExamples, - description: "Do not write async functions that do not have an await statement in them", + description: Rule.FAILURE_STRING, hasFix: false, optionExamples: [true], options: null, @@ -33,8 +35,6 @@ export class Rule extends Lint.Rules.AbstractRule { typescriptOnly: false, }; - public static FAILURE_STRING = "Async functions with no await are not allowed."; - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithWalker(new Walk(sourceFile, this.getOptions())); } @@ -57,7 +57,7 @@ class Walk extends Lint.RuleWalker { } private isAsyncFunction(node: ts.Node): boolean { - return Boolean(this.getAsyncModifier(node)); + return this.getAsyncModifier(node) !== undefined; } private getAsyncModifier(node: ts.Node) { @@ -72,7 +72,7 @@ class Walk extends Lint.RuleWalker { return node.kind === ts.SyntaxKind.AwaitKeyword; } - private functionBlockHasAwait(node: ts.Node) { + private functionBlockHasAwait(node: ts.Node): boolean { if (this.isAwait(node)) { return true; } @@ -83,7 +83,7 @@ class Walk extends Lint.RuleWalker { return false; } - const awaitInChildren: boolean[] = node.getChildren() + const awaitInChildren = node.getChildren() .map((functionBlockNode: ts.Node) => this.functionBlockHasAwait(functionBlockNode)); return awaitInChildren.some(Boolean); } diff --git a/src/runner.ts b/src/runner.ts index 244d8a28d34..ccf4966152d 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -156,8 +156,8 @@ async function runWorker(options: Options, logger: Logger): Promise { return options.force || errorCount === 0 ? Status.Ok : Status.LintError; } -// tslint:disable-next-line:no-async-without-await -async function runLinter(options: Options, logger: Logger): Promise { +// tslint:disable-next-line:promise-function-async +function runLinter(options: Options, logger: Logger): Promise { const { files, program } = resolveFilesAndProgram(options, logger); // if type checking, run the type checker if (program && options.typeCheck) { diff --git a/test/rules/no-async-without-await/test.ts.lint b/test/rules/no-async-without-await/test.ts.lint index 46e5df62a2b..b679fc0151c 100644 --- a/test/rules/no-async-without-await/test.ts.lint +++ b/test/rules/no-async-without-await/test.ts.lint @@ -23,7 +23,7 @@ async function a(){ } async function a(){ -~~~~~ [Async functions with no await are not allowed.] +~~~~~ [Functions marked async must contain an await statement.] let b = 1; async function f() { await fetch(); @@ -33,24 +33,24 @@ async function a(){ function a(){ let b = 1; async function f() { - ~~~~~ [Async functions with no await are not allowed.] + ~~~~~ [Functions marked async must contain an await statement.] fetch(); }; } const a = async () => { - ~~~~~ [Async functions with no await are not allowed.] + ~~~~~ [Functions marked async must contain an await statement.] let b = 1; console.log(b); } const a = async () => 1; - ~~~~~ [Async functions with no await are not allowed.] + ~~~~~ [Functions marked async must contain an await statement.] class A { async b() { - ~~~~~ [Async functions with no await are not allowed.] + ~~~~~ [Functions marked async must contain an await statement.] console.log(1); } } @@ -65,10 +65,10 @@ async () => { await a(); class A { async b() { - ~~~~~ [Async functions with no await are not allowed.] + ~~~~~ [Functions marked async must contain an await statement.] console.log(1); } } }; -[0]: Async functions with no await are not allowed. +[0]: Functions marked async must contain an await statement. From f4680efe6180d8ce2d2c5f9681bfe880eac31517 Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Fri, 6 Jul 2018 17:40:52 +0300 Subject: [PATCH 04/10] allow return as well as await --- .../noAsyncWithoutAwait.examples.ts | 4 +++ src/rules/noAsyncWithoutAwaitRule.ts | 31 +++++++++++++++++-- .../rules/no-async-without-await/test.ts.lint | 31 +++++++++++++------ 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/rules/code-examples/noAsyncWithoutAwait.examples.ts b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts index 35828204b74..e5f2aa74c6d 100644 --- a/src/rules/code-examples/noAsyncWithoutAwait.examples.ts +++ b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts @@ -42,6 +42,10 @@ export const codeExamples = [ const f = async () => { await fetch(); }; + + const f = async () => { + return 'value'; + }; `, }, ]; diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index 404047becd8..9d2e8245e6d 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -20,7 +20,7 @@ import * as Lint from "../index"; import { codeExamples } from "./code-examples/noAsyncWithoutAwait.examples"; export class Rule extends Lint.Rules.AbstractRule { - public static FAILURE_STRING = "Functions marked async must contain an await statement."; + public static FAILURE_STRING = "Functions marked async must contain an await or return statement."; public static metadata: Lint.IRuleMetadata = { codeExamples, @@ -72,6 +72,10 @@ class Walk extends Lint.RuleWalker { return node.kind === ts.SyntaxKind.AwaitKeyword; } + private isReturn(node: ts.Node): boolean { + return node.kind === ts.SyntaxKind.ReturnKeyword; + } + private functionBlockHasAwait(node: ts.Node): boolean { if (this.isAwait(node)) { return true; @@ -88,8 +92,31 @@ class Walk extends Lint.RuleWalker { return awaitInChildren.some(Boolean); } + private functionBlockHasReturn(node: ts.Node): boolean { + if (this.isReturn(node)) { + return true; + } + + // tslint:disable-next-line:no-unsafe-any + if (node.kind === ts.SyntaxKind.ArrowFunction + || node.kind === ts.SyntaxKind.FunctionDeclaration) { + return false; + } + + const returnInChildren = node.getChildren() + .map((functionBlockNode: ts.Node) => this.functionBlockHasReturn(functionBlockNode)); + return returnInChildren.some(Boolean); + } + + private isShortArrowReturn(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { + return node.kind === ts.SyntaxKind.ArrowFunction && node.body.kind !== ts.SyntaxKind.Block; + } + private addFailureIfAsyncFunctionHasNoAwait(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { - if (this.isAsyncFunction(node) && !this.functionBlockHasAwait(node.body as ts.Node)) { + if (this.isAsyncFunction(node) + && !this.functionBlockHasAwait(node.body as ts.Node) + && !this.functionBlockHasReturn(node.body as ts.Node) + && !this.isShortArrowReturn(node)) { const asyncModifier = this.getAsyncModifier(node); if (asyncModifier !== undefined) { this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); diff --git a/test/rules/no-async-without-await/test.ts.lint b/test/rules/no-async-without-await/test.ts.lint index b679fc0151c..779e00e97a0 100644 --- a/test/rules/no-async-without-await/test.ts.lint +++ b/test/rules/no-async-without-await/test.ts.lint @@ -23,7 +23,7 @@ async function a(){ } async function a(){ -~~~~~ [Functions marked async must contain an await statement.] +~~~~~ [Functions marked async must contain an await or return statement.] let b = 1; async function f() { await fetch(); @@ -33,24 +33,20 @@ async function a(){ function a(){ let b = 1; async function f() { - ~~~~~ [Functions marked async must contain an await statement.] + ~~~~~ [Functions marked async must contain an await or return statement.] fetch(); }; } const a = async () => { - ~~~~~ [Functions marked async must contain an await statement.] + ~~~~~ [Functions marked async must contain an await or return statement.] let b = 1; console.log(b); } - -const a = async () => 1; - ~~~~~ [Functions marked async must contain an await statement.] - class A { async b() { - ~~~~~ [Functions marked async must contain an await statement.] + ~~~~~ [Functions marked async must contain an await or return statement.] console.log(1); } } @@ -65,10 +61,25 @@ async () => { await a(); class A { async b() { - ~~~~~ [Functions marked async must contain an await statement.] + ~~~~~ [Functions marked async must contain an await or return statement.] console.log(1); } } }; -[0]: Functions marked async must contain an await statement. +async function a() { + let b = 1; + return b; +} + +let a = async () => 1; + +async function a() { +~~~~~ [Functions marked async must contain an await or return statement.] + let b = 1; + let a = () => { + return 1; + } +} + +[0]: Functions marked async must contain an await or return statement. From d28b2d3c6e330fef0bda7baa2c0a04ab7d0144ce Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Fri, 6 Jul 2018 17:42:50 +0300 Subject: [PATCH 05/10] remove unneeded lint ignore --- src/runner.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/runner.ts b/src/runner.ts index ccf4966152d..0dabb65c34b 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -156,8 +156,7 @@ async function runWorker(options: Options, logger: Logger): Promise { return options.force || errorCount === 0 ? Status.Ok : Status.LintError; } -// tslint:disable-next-line:promise-function-async -function runLinter(options: Options, logger: Logger): Promise { +async function runLinter(options: Options, logger: Logger): Promise { const { files, program } = resolveFilesAndProgram(options, logger); // if type checking, run the type checker if (program && options.typeCheck) { From bfa4c915ac25af08522fc55fa3d0b97a50be8121 Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Mon, 19 Nov 2018 20:02:36 +0200 Subject: [PATCH 06/10] improve rationale & fix performance issue --- src/rules/noAsyncWithoutAwaitRule.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index 9d2e8245e6d..48910fe6770 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -29,7 +29,12 @@ export class Rule extends Lint.Rules.AbstractRule { optionExamples: [true], options: null, optionsDescription: "Not configurable.", - rationale: "Cleaner code, can possibly reduce transpiled output", + /* tslint:disable:max-line-length */ + rationale: Lint.Utils.dedent` + Marking a function as \`async\` without using \`await\` or returning a value inside it can lead to an unintended promise return and a larger transpiled output. + Often the function can be synchronous and the \`async\` keyword is there by mistake. + Return statements are allowed has sometimes it is desirable to wrap the returned value in promise`, + /* tslint:enable:max-line-length */ ruleName: "no-async-without-await", type: "functionality", typescriptOnly: false, @@ -113,10 +118,11 @@ class Walk extends Lint.RuleWalker { } private addFailureIfAsyncFunctionHasNoAwait(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { - if (this.isAsyncFunction(node) + if (!this.isShortArrowReturn(node) + && this.isAsyncFunction(node) && !this.functionBlockHasAwait(node.body as ts.Node) && !this.functionBlockHasReturn(node.body as ts.Node) - && !this.isShortArrowReturn(node)) { + ) { const asyncModifier = this.getAsyncModifier(node); if (asyncModifier !== undefined) { this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); From 37aa7b5a09023245b71075ed498556a30898b72c Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Thu, 27 Dec 2018 01:47:17 +0200 Subject: [PATCH 07/10] fixes according to review --- src/rules/noAsyncWithoutAwaitRule.ts | 25 +++++++++++-------- .../rules/no-async-without-await/test.ts.lint | 3 +++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index 48910fe6770..e9d0f14b21f 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -33,7 +33,7 @@ export class Rule extends Lint.Rules.AbstractRule { rationale: Lint.Utils.dedent` Marking a function as \`async\` without using \`await\` or returning a value inside it can lead to an unintended promise return and a larger transpiled output. Often the function can be synchronous and the \`async\` keyword is there by mistake. - Return statements are allowed has sometimes it is desirable to wrap the returned value in promise`, + Return statements are allowed as sometimes it is desirable to wrap the returned value in a Promise`, /* tslint:enable:max-line-length */ ruleName: "no-async-without-await", type: "functionality", @@ -86,15 +86,13 @@ class Walk extends Lint.RuleWalker { return true; } - // tslint:disable-next-line:no-unsafe-any if (node.kind === ts.SyntaxKind.ArrowFunction || node.kind === ts.SyntaxKind.FunctionDeclaration) { return false; } - const awaitInChildren = node.getChildren() - .map((functionBlockNode: ts.Node) => this.functionBlockHasAwait(functionBlockNode)); - return awaitInChildren.some(Boolean); + // tslint:disable-next-line:no-unsafe-any + return node.getChildren().some(this.functionBlockHasAwait.bind(this)); } private functionBlockHasReturn(node: ts.Node): boolean { @@ -102,7 +100,6 @@ class Walk extends Lint.RuleWalker { return true; } - // tslint:disable-next-line:no-unsafe-any if (node.kind === ts.SyntaxKind.ArrowFunction || node.kind === ts.SyntaxKind.FunctionDeclaration) { return false; @@ -117,16 +114,24 @@ class Walk extends Lint.RuleWalker { return node.kind === ts.SyntaxKind.ArrowFunction && node.body.kind !== ts.SyntaxKind.Block; } + private reportFailure(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { + const asyncModifier = this.getAsyncModifier(node); + if (asyncModifier !== undefined) { + this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); + } + } + private addFailureIfAsyncFunctionHasNoAwait(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { + if (node.body === undefined) { + this.reportFailure(node); + return; + } if (!this.isShortArrowReturn(node) && this.isAsyncFunction(node) && !this.functionBlockHasAwait(node.body as ts.Node) && !this.functionBlockHasReturn(node.body as ts.Node) ) { - const asyncModifier = this.getAsyncModifier(node); - if (asyncModifier !== undefined) { - this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); - } + this.reportFailure(node); } } } diff --git a/test/rules/no-async-without-await/test.ts.lint b/test/rules/no-async-without-await/test.ts.lint index 779e00e97a0..897229a8ae2 100644 --- a/test/rules/no-async-without-await/test.ts.lint +++ b/test/rules/no-async-without-await/test.ts.lint @@ -82,4 +82,7 @@ async function a() { } } +async function foo; +~~~~~ [Functions marked async must contain an await or return statement.] + [0]: Functions marked async must contain an await or return statement. From 0d58f6889b204e5b0792823457ba68701e7edbc8 Mon Sep 17 00:00:00 2001 From: Eran Shabi Date: Thu, 27 Dec 2018 02:27:09 +0200 Subject: [PATCH 08/10] finish fixing according to review --- src/rules/noAsyncWithoutAwaitRule.ts | 18 +++++----- .../rules/no-async-without-await/test.ts.lint | 34 +++++++++++++++++++ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index e9d0f14b21f..2534f2daed6 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -19,6 +19,8 @@ import * as ts from "typescript"; import * as Lint from "../index"; import { codeExamples } from "./code-examples/noAsyncWithoutAwait.examples"; +type FunctionNodeType = ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration | ts.FunctionExpression; + export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "Functions marked async must contain an await or return statement."; @@ -61,8 +63,9 @@ class Walk extends Lint.RuleWalker { super.visitMethodDeclaration(node); } - private isAsyncFunction(node: ts.Node): boolean { - return this.getAsyncModifier(node) !== undefined; + protected visitFunctionExpression(node: ts.FunctionExpression) { + this.addFailureIfAsyncFunctionHasNoAwait(node); + super.visitFunctionExpression(node); } private getAsyncModifier(node: ts.Node) { @@ -110,28 +113,27 @@ class Walk extends Lint.RuleWalker { return returnInChildren.some(Boolean); } - private isShortArrowReturn(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { + private isShortArrowReturn(node: FunctionNodeType) { return node.kind === ts.SyntaxKind.ArrowFunction && node.body.kind !== ts.SyntaxKind.Block; } - private reportFailure(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { + private reportFailureIfAsyncFunction(node: FunctionNodeType) { const asyncModifier = this.getAsyncModifier(node); if (asyncModifier !== undefined) { this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); } } - private addFailureIfAsyncFunctionHasNoAwait(node: ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration) { + private addFailureIfAsyncFunctionHasNoAwait(node: FunctionNodeType) { if (node.body === undefined) { - this.reportFailure(node); + this.reportFailureIfAsyncFunction(node); return; } if (!this.isShortArrowReturn(node) - && this.isAsyncFunction(node) && !this.functionBlockHasAwait(node.body as ts.Node) && !this.functionBlockHasReturn(node.body as ts.Node) ) { - this.reportFailure(node); + this.reportFailureIfAsyncFunction(node); } } } diff --git a/test/rules/no-async-without-await/test.ts.lint b/test/rules/no-async-without-await/test.ts.lint index 897229a8ae2..452278893ac 100644 --- a/test/rules/no-async-without-await/test.ts.lint +++ b/test/rules/no-async-without-await/test.ts.lint @@ -57,6 +57,36 @@ class A { } } +class A { + public a = async function b() { + await b(); + } + } + +class A { + public a = async function b() { + ~~~~~ [Functions marked async must contain an await or return statement.] + b(); + } + } + +class A { + public a = async () => { + await b(); + } + } + +class A { + public a = async () => { + ~~~~~ [Functions marked async must contain an await or return statement.] + b(); + } + } + +class A { + public a = async () => 1; + } + async () => { await a(); class A { @@ -85,4 +115,8 @@ async function a() { async function foo; ~~~~~ [Functions marked async must contain an await or return statement.] +function * foo() { + return 1; +} + [0]: Functions marked async must contain an await or return statement. From 96f39ed8250469824e30ab20baefd6f8ca986b12 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 15 Jun 2019 21:15:39 -0400 Subject: [PATCH 09/10] Initial feedback cleanups --- .../noAsyncWithoutAwait.examples.ts | 2 +- src/rules/noAsyncWithoutAwaitRule.ts | 58 +++++++++++-------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/rules/code-examples/noAsyncWithoutAwait.examples.ts b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts index e5f2aa74c6d..bf2d6c8b289 100644 --- a/src/rules/code-examples/noAsyncWithoutAwait.examples.ts +++ b/src/rules/code-examples/noAsyncWithoutAwait.examples.ts @@ -22,7 +22,7 @@ export const codeExamples = [ config: Lint.Utils.dedent` "rules": { "no-async-without-await": true } `, - description: "Do not use the async keyword in case it is not actually needed", + description: "Do not use the async keyword if it is not needed", fail: Lint.Utils.dedent` async function f() { fetch(); diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index 2534f2daed6..171c68fc5d2 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -15,14 +15,22 @@ * limitations under the License. */ +import * as tsutils from "tsutils"; import * as ts from "typescript"; + import * as Lint from "../index"; + import { codeExamples } from "./code-examples/noAsyncWithoutAwait.examples"; -type FunctionNodeType = ts.ArrowFunction | ts.FunctionDeclaration | ts.MethodDeclaration | ts.FunctionExpression; +type FunctionNodeType = + | ts.ArrowFunction + | ts.FunctionDeclaration + | ts.MethodDeclaration + | ts.FunctionExpression; export class Rule extends Lint.Rules.AbstractRule { - public static FAILURE_STRING = "Functions marked async must contain an await or return statement."; + public static FAILURE_STRING = + "Functions marked async must contain an await or return statement."; public static metadata: Lint.IRuleMetadata = { codeExamples, @@ -35,7 +43,7 @@ export class Rule extends Lint.Rules.AbstractRule { rationale: Lint.Utils.dedent` Marking a function as \`async\` without using \`await\` or returning a value inside it can lead to an unintended promise return and a larger transpiled output. Often the function can be synchronous and the \`async\` keyword is there by mistake. - Return statements are allowed as sometimes it is desirable to wrap the returned value in a Promise`, + Return statements are allowed as sometimes it is desirable to wrap the returned value in a Promise.`, /* tslint:enable:max-line-length */ ruleName: "no-async-without-await", type: "functionality", @@ -47,6 +55,7 @@ export class Rule extends Lint.Rules.AbstractRule { } } +// tslint:disable-next-line: deprecation class Walk extends Lint.RuleWalker { protected visitFunctionDeclaration(node: ts.FunctionDeclaration) { this.addFailureIfAsyncFunctionHasNoAwait(node); @@ -70,27 +79,25 @@ class Walk extends Lint.RuleWalker { private getAsyncModifier(node: ts.Node) { if (node.modifiers !== undefined) { - return node.modifiers.find((modifier) => modifier.kind === ts.SyntaxKind.AsyncKeyword); + return node.modifiers.find(modifier => modifier.kind === ts.SyntaxKind.AsyncKeyword); } return undefined; } - private isAwait(node: ts.Node): boolean { - return node.kind === ts.SyntaxKind.AwaitKeyword; - } - private isReturn(node: ts.Node): boolean { return node.kind === ts.SyntaxKind.ReturnKeyword; } private functionBlockHasAwait(node: ts.Node): boolean { - if (this.isAwait(node)) { + if (tsutils.isAwaitExpression(node)) { return true; } - if (node.kind === ts.SyntaxKind.ArrowFunction - || node.kind === ts.SyntaxKind.FunctionDeclaration) { + if ( + node.kind === ts.SyntaxKind.ArrowFunction || + node.kind === ts.SyntaxKind.FunctionDeclaration + ) { return false; } @@ -98,20 +105,20 @@ class Walk extends Lint.RuleWalker { return node.getChildren().some(this.functionBlockHasAwait.bind(this)); } - private functionBlockHasReturn(node: ts.Node): boolean { + private readonly functionBlockHasReturn = (node: ts.Node): boolean => { if (this.isReturn(node)) { return true; } - if (node.kind === ts.SyntaxKind.ArrowFunction - || node.kind === ts.SyntaxKind.FunctionDeclaration) { + if ( + node.kind === ts.SyntaxKind.ArrowFunction || + node.kind === ts.SyntaxKind.FunctionDeclaration + ) { return false; } - const returnInChildren = node.getChildren() - .map((functionBlockNode: ts.Node) => this.functionBlockHasReturn(functionBlockNode)); - return returnInChildren.some(Boolean); - } + return node.getChildren().some(this.functionBlockHasReturn); + }; private isShortArrowReturn(node: FunctionNodeType) { return node.kind === ts.SyntaxKind.ArrowFunction && node.body.kind !== ts.SyntaxKind.Block; @@ -120,7 +127,11 @@ class Walk extends Lint.RuleWalker { private reportFailureIfAsyncFunction(node: FunctionNodeType) { const asyncModifier = this.getAsyncModifier(node); if (asyncModifier !== undefined) { - this.addFailureAt(asyncModifier.getStart(), asyncModifier.getEnd() - asyncModifier.getStart(), Rule.FAILURE_STRING); + this.addFailureAt( + asyncModifier.getStart(), + asyncModifier.getEnd() - asyncModifier.getStart(), + Rule.FAILURE_STRING, + ); } } @@ -129,10 +140,11 @@ class Walk extends Lint.RuleWalker { this.reportFailureIfAsyncFunction(node); return; } - if (!this.isShortArrowReturn(node) - && !this.functionBlockHasAwait(node.body as ts.Node) - && !this.functionBlockHasReturn(node.body as ts.Node) - ) { + if ( + !this.isShortArrowReturn(node) && + !this.functionBlockHasAwait(node.body) && + !this.functionBlockHasReturn(node.body) + ) { this.reportFailureIfAsyncFunction(node); } } From 65e10f5927e4cf7a58f2264e3671727503a66ba4 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 15 Jun 2019 21:27:25 -0400 Subject: [PATCH 10/10] Refactored to walk function --- src/rules/noAsyncWithoutAwaitRule.ts | 141 ++++++++++++--------------- 1 file changed, 65 insertions(+), 76 deletions(-) diff --git a/src/rules/noAsyncWithoutAwaitRule.ts b/src/rules/noAsyncWithoutAwaitRule.ts index 171c68fc5d2..220c4888aed 100644 --- a/src/rules/noAsyncWithoutAwaitRule.ts +++ b/src/rules/noAsyncWithoutAwaitRule.ts @@ -51,101 +51,90 @@ export class Rule extends Lint.Rules.AbstractRule { }; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new Walk(sourceFile, this.getOptions())); + return this.applyWithFunction(sourceFile, walk); } } -// tslint:disable-next-line: deprecation -class Walk extends Lint.RuleWalker { - protected visitFunctionDeclaration(node: ts.FunctionDeclaration) { - this.addFailureIfAsyncFunctionHasNoAwait(node); - super.visitFunctionDeclaration(node); - } - - protected visitArrowFunction(node: ts.ArrowFunction) { - this.addFailureIfAsyncFunctionHasNoAwait(node); - super.visitArrowFunction(node); - } - - protected visitMethodDeclaration(node: ts.MethodDeclaration) { - this.addFailureIfAsyncFunctionHasNoAwait(node); - super.visitMethodDeclaration(node); - } - - protected visitFunctionExpression(node: ts.FunctionExpression) { - this.addFailureIfAsyncFunctionHasNoAwait(node); - super.visitFunctionExpression(node); - } - - private getAsyncModifier(node: ts.Node) { - if (node.modifiers !== undefined) { - return node.modifiers.find(modifier => modifier.kind === ts.SyntaxKind.AsyncKeyword); +function walk(context: Lint.WalkContext) { + const reportFailureIfAsyncFunction = (node: FunctionNodeType) => { + const asyncModifier = getAsyncModifier(node); + if (asyncModifier !== undefined) { + context.addFailureAt( + asyncModifier.getStart(), + asyncModifier.getEnd() - asyncModifier.getStart(), + Rule.FAILURE_STRING, + ); } + }; - return undefined; - } - - private isReturn(node: ts.Node): boolean { - return node.kind === ts.SyntaxKind.ReturnKeyword; - } + const addFailureIfAsyncFunctionHasNoAwait = (node: FunctionNodeType) => { + if (node.body === undefined) { + reportFailureIfAsyncFunction(node); + return; + } - private functionBlockHasAwait(node: ts.Node): boolean { - if (tsutils.isAwaitExpression(node)) { - return true; + if ( + !isShortArrowReturn(node) && + !functionBlockHasAwait(node.body) && + !functionBlockHasReturn(node.body) + ) { + reportFailureIfAsyncFunction(node); } + }; + return ts.forEachChild(context.sourceFile, function visitNode(node): void { if ( - node.kind === ts.SyntaxKind.ArrowFunction || - node.kind === ts.SyntaxKind.FunctionDeclaration + tsutils.isArrowFunction(node) || + tsutils.isFunctionDeclaration(node) || + tsutils.isFunctionExpression(node) || + tsutils.isMethodDeclaration(node) ) { - return false; + addFailureIfAsyncFunctionHasNoAwait(node); } - // tslint:disable-next-line:no-unsafe-any - return node.getChildren().some(this.functionBlockHasAwait.bind(this)); + return ts.forEachChild(node, visitNode); + }); +} + +const getAsyncModifier = (node: ts.Node) => { + if (node.modifiers !== undefined) { + return node.modifiers.find(modifier => modifier.kind === ts.SyntaxKind.AsyncKeyword); } - private readonly functionBlockHasReturn = (node: ts.Node): boolean => { - if (this.isReturn(node)) { - return true; - } + return undefined; +}; - if ( - node.kind === ts.SyntaxKind.ArrowFunction || - node.kind === ts.SyntaxKind.FunctionDeclaration - ) { - return false; - } +const isReturn = (node: ts.Node): boolean => node.kind === ts.SyntaxKind.ReturnKeyword; - return node.getChildren().some(this.functionBlockHasReturn); - }; +const functionBlockHasAwait = (node: ts.Node): boolean => { + if (tsutils.isAwaitExpression(node)) { + return true; + } - private isShortArrowReturn(node: FunctionNodeType) { - return node.kind === ts.SyntaxKind.ArrowFunction && node.body.kind !== ts.SyntaxKind.Block; + if ( + node.kind === ts.SyntaxKind.ArrowFunction || + node.kind === ts.SyntaxKind.FunctionDeclaration + ) { + return false; } - private reportFailureIfAsyncFunction(node: FunctionNodeType) { - const asyncModifier = this.getAsyncModifier(node); - if (asyncModifier !== undefined) { - this.addFailureAt( - asyncModifier.getStart(), - asyncModifier.getEnd() - asyncModifier.getStart(), - Rule.FAILURE_STRING, - ); - } + return node.getChildren().some(functionBlockHasAwait); +}; + +const functionBlockHasReturn = (node: ts.Node): boolean => { + if (isReturn(node)) { + return true; } - private addFailureIfAsyncFunctionHasNoAwait(node: FunctionNodeType) { - if (node.body === undefined) { - this.reportFailureIfAsyncFunction(node); - return; - } - if ( - !this.isShortArrowReturn(node) && - !this.functionBlockHasAwait(node.body) && - !this.functionBlockHasReturn(node.body) - ) { - this.reportFailureIfAsyncFunction(node); - } + if ( + node.kind === ts.SyntaxKind.ArrowFunction || + node.kind === ts.SyntaxKind.FunctionDeclaration + ) { + return false; } -} + + return node.getChildren().some(functionBlockHasReturn); +}; + +const isShortArrowReturn = (node: FunctionNodeType) => + node.kind === ts.SyntaxKind.ArrowFunction && node.body.kind !== ts.SyntaxKind.Block;