feat(linter): implement prefer-await-to-callbacks (#6153)

This commit is contained in:
dalaoshu 2024-09-29 10:30:30 +08:00 committed by GitHub
parent f50fdcd0f9
commit 183739ff47
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 273 additions and 0 deletions

View file

@ -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,

View file

@ -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();
}

View file

@ -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)
· ────────────────────
╰────