diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 84482610c6259..f459a64f89168 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -471,6 +471,7 @@ mod promise { pub mod no_new_statics; pub mod no_return_in_finally; pub mod param_names; + pub mod prefer_await_to_callbacks; pub mod prefer_await_to_then; pub mod spec_only; pub mod valid_params; @@ -760,6 +761,7 @@ oxc_macros::declare_all_lint_rules! { promise::no_new_statics, promise::no_return_in_finally, promise::param_names, + promise::prefer_await_to_callbacks, promise::prefer_await_to_then, promise::spec_only, promise::valid_params, diff --git a/crates/oxc_linter/src/rules/promise/prefer_await_to_callbacks.rs b/crates/oxc_linter/src/rules/promise/prefer_await_to_callbacks.rs new file mode 100644 index 0000000000000..7dd655e32a36a --- /dev/null +++ b/crates/oxc_linter/src/rules/promise/prefer_await_to_callbacks.rs @@ -0,0 +1,185 @@ +use oxc_ast::{ + ast::{Argument, Expression, FormalParameters, MemberExpression}, + AstKind, +}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::NodeId; +use oxc_span::{GetSpan, Span}; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn prefer_await_to_callbacks(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Prefer `async`/`await` to the callback pattern").with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct PreferAwaitToCallbacks; + +declare_oxc_lint!( + /// ### What it does + /// + /// The rule encourages the use of `async/await` for handling asynchronous code + /// instead of traditional callback functions. `async/await`, introduced in ES2017, + /// provides a clearer and more concise syntax for writing asynchronous code, + /// making it easier to read and maintain. + /// + /// ### Why is this bad? + /// + /// Using callbacks can lead to complex, nested structures known as "callback hell," + /// which make code difficult to read and maintain. Additionally, error handling can + /// become cumbersome with callbacks, whereas `async/await` allows for more straightforward + /// try/catch blocks for managing errors. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// cb() + /// callback() + /// doSomething(arg, (err) => {}) + /// function doSomethingElse(cb) {} + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// await doSomething(arg) + /// async function doSomethingElse() {} + /// function* generator() { + /// yield yieldValue(err => {}) + /// } + /// eventEmitter.on('error', err => {}) + /// ``` + PreferAwaitToCallbacks, + style, +); + +impl Rule for PreferAwaitToCallbacks { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::CallExpression(expr) => { + let callee_name = expr.callee.get_identifier_reference().map(|id| id.name.as_str()); + if matches!(callee_name, Some("callback" | "cb")) { + ctx.diagnostic(prefer_await_to_callbacks(expr.span)); + return; + } + + if let Some(last_arg) = expr.arguments.last() { + let args = match last_arg { + Argument::FunctionExpression(expr) => &expr.params, + Argument::ArrowFunctionExpression(expr) => &expr.params, + _ => return, + }; + + let callee_property_name = expr + .callee + .as_member_expression() + .and_then(MemberExpression::static_property_name); + + if matches!(callee_property_name, Some("on" | "once")) { + return; + } + + let is_lodash = expr.callee.as_member_expression().map_or(false, |mem_expr| { + matches!(mem_expr.object(), Expression::Identifier(id) if matches!(id.name.as_str(), "_" | "lodash" | "underscore")) + }); + + let calls_array_method = callee_property_name + .is_some_and(Self::is_array_method) + && (expr.arguments.len() == 1 || (expr.arguments.len() == 2 && is_lodash)); + + let is_array_method = + callee_name.is_some_and(Self::is_array_method) && expr.arguments.len() == 2; + + if calls_array_method || is_array_method { + return; + } + + let Some(param) = args.items.first() else { + return; + }; + + if matches!(param.pattern.get_identifier().as_deref(), Some("err" | "error")) + && !Self::is_inside_yield_or_await(node.id(), ctx) + { + ctx.diagnostic(prefer_await_to_callbacks(last_arg.span())); + } + } + } + AstKind::Function(func) => { + Self::check_last_params_for_callback(&func.params, ctx); + } + AstKind::ArrowFunctionExpression(expr) => { + Self::check_last_params_for_callback(&expr.params, ctx); + } + _ => {} + } + } +} + +impl PreferAwaitToCallbacks { + fn check_last_params_for_callback(params: &FormalParameters, ctx: &LintContext) { + let Some(param) = params.items.last() else { + return; + }; + + let id = param.pattern.get_identifier(); + if matches!(id.as_deref(), Some("callback" | "cb")) { + ctx.diagnostic(prefer_await_to_callbacks(param.span)); + } + } + + fn is_inside_yield_or_await(id: NodeId, ctx: &LintContext) -> bool { + ctx.nodes().iter_parents(id).skip(1).any(|parent| { + matches!(parent.kind(), AstKind::AwaitExpression(_) | AstKind::YieldExpression(_)) + }) + } + + fn is_array_method(name: &str) -> bool { + ["map", "every", "forEach", "some", "find", "filter"].contains(&name) + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "async function hi() { await thing().catch(err => console.log(err)) }", + "async function hi() { await thing().then() }", + "async function hi() { await thing().catch() }", + r#"dbConn.on("error", err => { console.error(err) })"#, + r#"dbConn.once("error", err => { console.error(err) })"#, + "heart(something => {})", + "getErrors().map(error => responseTo(error))", + "errors.filter(err => err.status === 402)", + r#"errors.some(err => err.message.includes("Yo"))"#, + "errors.every(err => err.status === 402)", + "errors.filter(err => console.log(err))", + r#"const error = errors.find(err => err.stack.includes("file.js"))"#, + "this.myErrors.forEach(function(error) { log(error); })", + r#"find(errors, function(err) { return err.type === "CoolError" })"#, + r#"map(errors, function(error) { return err.type === "CoolError" })"#, + r#"_.find(errors, function(error) { return err.type === "CoolError" })"#, + r#"_.map(errors, function(err) { return err.type === "CoolError" })"#, + ]; + + let fail = vec![ + "heart(function(err) {})", + "heart(err => {})", + r#"heart("ball", function(err) {})"#, + "function getData(id, callback) {}", + "const getData = (cb) => {}", + "var x = function (x, cb) {}", + "cb()", + "callback()", + "heart(error => {})", + "async.map(files, fs.stat, function(err, results) { if (err) throw err; });", + "_.map(files, fs.stat, function(err, results) { if (err) throw err; });", + "map(files, fs.stat, function(err, results) { if (err) throw err; });", + "map(function(err, results) { if (err) throw err; });", + "customMap(errors, (err) => err.message)", + ]; + + Tester::new(PreferAwaitToCallbacks::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/prefer_await_to_callbacks.snap b/crates/oxc_linter/src/snapshots/prefer_await_to_callbacks.snap new file mode 100644 index 0000000000000..60286d06438f4 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/prefer_await_to_callbacks.snap @@ -0,0 +1,86 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:7] + 1 │ heart(function(err) {}) + · ──────────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:7] + 1 │ heart(err => {}) + · ───────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:15] + 1 │ heart("ball", function(err) {}) + · ──────────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:22] + 1 │ function getData(id, callback) {} + · ──────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:18] + 1 │ const getData = (cb) => {} + · ── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:22] + 1 │ var x = function (x, cb) {} + · ── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:1] + 1 │ cb() + · ──── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:1] + 1 │ callback() + · ────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:7] + 1 │ heart(error => {}) + · ─────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:27] + 1 │ async.map(files, fs.stat, function(err, results) { if (err) throw err; }); + · ────────────────────────────────────────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:23] + 1 │ _.map(files, fs.stat, function(err, results) { if (err) throw err; }); + · ────────────────────────────────────────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:21] + 1 │ map(files, fs.stat, function(err, results) { if (err) throw err; }); + · ────────────────────────────────────────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:5] + 1 │ map(function(err, results) { if (err) throw err; }); + · ────────────────────────────────────────────── + ╰──── + + ⚠ eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern + ╭─[prefer_await_to_callbacks.tsx:1:19] + 1 │ customMap(errors, (err) => err.message) + · ──────────────────── + ╰────