From 286049b17be8aee856c2b196b25a9e9e97c00f8d Mon Sep 17 00:00:00 2001 From: Devin-Yeung <53309384+Devin-Yeung@users.noreply.github.com> Date: Mon, 4 Sep 2023 23:04:51 +0800 Subject: [PATCH] feat(linter): implement unicorn/no-unnecessary-await (#856) related to #684 --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/unicorn/no_unnecessary_await.rs | 162 ++++++++++++ .../src/snapshots/no_unnecessary_await.snap | 243 ++++++++++++++++++ 3 files changed, 407 insertions(+) create mode 100644 crates/oxc_linter/src/rules/unicorn/no_unnecessary_await.rs create mode 100644 crates/oxc_linter/src/snapshots/no_unnecessary_await.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index e8763d8b42aed..6bc13033ba473 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -103,6 +103,7 @@ mod jest { mod unicorn { pub mod no_instanceof_array; + pub mod no_unnecessary_await; } oxc_macros::declare_all_lint_rules! { @@ -191,4 +192,5 @@ oxc_macros::declare_all_lint_rules! { jest::no_alias_methods, jest::no_conditional_expect, unicorn::no_instanceof_array, + unicorn::no_unnecessary_await } diff --git a/crates/oxc_linter/src/rules/unicorn/no_unnecessary_await.rs b/crates/oxc_linter/src/rules/unicorn/no_unnecessary_await.rs new file mode 100644 index 0000000000000..a785cd8013f2d --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/no_unnecessary_await.rs @@ -0,0 +1,162 @@ +use oxc_ast::{ast::Expression, AstKind}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_formatter::Gen; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode, Fix}; + +#[derive(Debug, Error, Diagnostic)] +#[error("Disallow awaiting non-promise values.")] +#[diagnostic(severity(warning), help("consider to remove the `await`"))] +struct NoUnnecessaryAwaitDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoUnnecessaryAwait; + +declare_oxc_lint!( + /// ### What it does + /// Disallow awaiting on non-promise values. + /// + /// ### Why is this bad? + /// The `await` operator should only be used on `Promise` values. + /// + /// ### Example + /// ```javascript + /// await await promise; + /// ``` + NoUnnecessaryAwait, + correctness +); + +impl Rule for NoUnnecessaryAwait { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if let AstKind::AwaitExpression(expr) = node.kind() { + if !not_promise(&expr.argument) { + return; + } + if { + // Removing `await` may change them to a declaration, if there is no `id` will cause SyntaxError + matches!(expr.argument, Expression::FunctionExpression(_)) + || matches!(expr.argument, Expression::ClassExpression(_)) + } || { + // `+await +1` -> `++1` + ctx.nodes().parent_node(node.id()).map_or(false, |parent| { + if let ( + AstKind::UnaryExpression(parent_unary), + Expression::UnaryExpression(inner_unary), + ) = (parent.kind(), &expr.argument) + { + parent_unary.operator == inner_unary.operator + } else { + false + } + }) + } { + ctx.diagnostic(NoUnnecessaryAwaitDiagnostic(expr.span)); + } else { + ctx.diagnostic_with_fix(NoUnnecessaryAwaitDiagnostic(expr.span), || { + let mut formatter = ctx.formatter(); + expr.argument.gen(&mut formatter); + Fix::new(formatter.into_code(), expr.span) + }); + }; + } + } +} + +fn not_promise(expr: &Expression) -> bool { + match expr { + Expression::ArrayExpression(_) + | Expression::ArrowExpression(_) + | Expression::AwaitExpression(_) + | Expression::BinaryExpression(_) + | Expression::ClassExpression(_) + | Expression::FunctionExpression(_) + | Expression::JSXElement(_) + | Expression::JSXFragment(_) + | Expression::BooleanLiteral(_) + | Expression::NullLiteral(_) + | Expression::NumberLiteral(_) + | Expression::BigintLiteral(_) + | Expression::RegExpLiteral(_) + | Expression::StringLiteral(_) + | Expression::TemplateLiteral(_) + | Expression::UnaryExpression(_) + | Expression::UpdateExpression(_) => true, + Expression::SequenceExpression(expr) => not_promise(expr.expressions.last().unwrap()), + Expression::ParenthesizedExpression(expr) => not_promise(&expr.expression), + _ => false, + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("await {then}", None), + ("await a ? b : c", None), + ("await a || b", None), + ("await a && b", None), + ("await a ?? b", None), + ("await new Foo()", None), + ("await tagged``", None), + ("class A { async foo() { await this }}", None), + ("async function * foo() {await (yield bar);}", None), + ("await (1, Promise.resolve())", None), + ]; + + let fail = vec![ + ("await []", None), + ("await [Promise.resolve()]", None), + ("await (() => {})", None), + ("await (() => Promise.resolve())", None), + ("await (a === b)", None), + ("await (a instanceof Promise)", None), + ("await (a > b)", None), + ("await class {}", None), + ("await class extends Promise {}", None), + ("await function() {}", None), + ("await function name() {}", None), + ("await function() { return Promise.resolve() }", None), + ("await (<>)", None), + ("await ()", None), + ("await 0", None), + ("await 1", None), + ("await \"\"", None), + ("await \"string\"", None), + ("await true", None), + ("await false", None), + ("await null", None), + ("await 0n", None), + ("await 1n", None), + ("await `${Promise.resolve()}`", None), + ("await !Promise.resolve()", None), + ("await void Promise.resolve()", None), + ("await +Promise.resolve()", None), + ("await ~1", None), + ("await ++foo", None), + ("await foo--", None), + ("await (Promise.resolve(), 1)", None), + ("async function foo() {+await +1}", None), + ("async function foo() {-await-1}", None), + ("async function foo() {+await -1}", None), + ]; + + let fix = vec![ + ("await []", "[]", None), + ("await (a == b)", "(a == b)", None), + ("+await -1", "+ -1", None), + ("-await +1", "- +1", None), + ("await function() {}", "await function() {}", None), // no autofix + ("await class {}", "await class {}", None), // no autofix + ("+await +1", "+await +1", None), // no autofix + ("-await -1", "-await -1", None), // no autofix + ]; + + Tester::new(NoUnnecessaryAwait::NAME, pass, fail).expect_fix(fix).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_unnecessary_await.snap b/crates/oxc_linter/src/snapshots/no_unnecessary_await.snap new file mode 100644 index 0000000000000..202ecf21e161e --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_unnecessary_await.snap @@ -0,0 +1,243 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_unnecessary_await +--- + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await [] + · ──────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await [Promise.resolve()] + · ───────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await (() => {}) + · ──────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await (() => Promise.resolve()) + · ─────────────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await (a === b) + · ─────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await (a instanceof Promise) + · ──────────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await (a > b) + · ───────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await class {} + · ────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await class extends Promise {} + · ────────────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await function() {} + · ─────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await function name() {} + · ──────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await function() { return Promise.resolve() } + · ───────────────────────────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await (<>) + · ───────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await () + · ─────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await 0 + · ─────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await 1 + · ─────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await "" + · ──────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await "string" + · ────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await true + · ────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await false + · ─────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await null + · ────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await 0n + · ──────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await 1n + · ──────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await `${Promise.resolve()}` + · ──────────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await !Promise.resolve() + · ──────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await void Promise.resolve() + · ──────────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await +Promise.resolve() + · ──────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await ~1 + · ──────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await ++foo + · ─────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await foo-- + · ─────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ await (Promise.resolve(), 1) + · ──────────────────────────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ async function foo() {+await +1} + · ──────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ async function foo() {-await-1} + · ─────── + ╰──── + help: consider to remove the `await` + + ⚠ Disallow awaiting non-promise values. + ╭─[no_unnecessary_await.tsx:1:1] + 1 │ async function foo() {+await -1} + · ──────── + ╰──── + help: consider to remove the `await` + +