Skip to content

Commit 183739f

Browse files
authoredSep 29, 2024··
feat(linter): implement prefer-await-to-callbacks (#6153)
1 parent f50fdcd commit 183739f

File tree

3 files changed

+273
-0
lines changed

3 files changed

+273
-0
lines changed
 

‎crates/oxc_linter/src/rules.rs

+2
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ mod promise {
471471
pub mod no_new_statics;
472472
pub mod no_return_in_finally;
473473
pub mod param_names;
474+
pub mod prefer_await_to_callbacks;
474475
pub mod prefer_await_to_then;
475476
pub mod spec_only;
476477
pub mod valid_params;
@@ -760,6 +761,7 @@ oxc_macros::declare_all_lint_rules! {
760761
promise::no_new_statics,
761762
promise::no_return_in_finally,
762763
promise::param_names,
764+
promise::prefer_await_to_callbacks,
763765
promise::prefer_await_to_then,
764766
promise::spec_only,
765767
promise::valid_params,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
use oxc_ast::{
2+
ast::{Argument, Expression, FormalParameters, MemberExpression},
3+
AstKind,
4+
};
5+
use oxc_diagnostics::OxcDiagnostic;
6+
use oxc_macros::declare_oxc_lint;
7+
use oxc_semantic::NodeId;
8+
use oxc_span::{GetSpan, Span};
9+
10+
use crate::{context::LintContext, rule::Rule, AstNode};
11+
12+
fn prefer_await_to_callbacks(span: Span) -> OxcDiagnostic {
13+
OxcDiagnostic::warn("Prefer `async`/`await` to the callback pattern").with_label(span)
14+
}
15+
16+
#[derive(Debug, Default, Clone)]
17+
pub struct PreferAwaitToCallbacks;
18+
19+
declare_oxc_lint!(
20+
/// ### What it does
21+
///
22+
/// The rule encourages the use of `async/await` for handling asynchronous code
23+
/// instead of traditional callback functions. `async/await`, introduced in ES2017,
24+
/// provides a clearer and more concise syntax for writing asynchronous code,
25+
/// making it easier to read and maintain.
26+
///
27+
/// ### Why is this bad?
28+
///
29+
/// Using callbacks can lead to complex, nested structures known as "callback hell,"
30+
/// which make code difficult to read and maintain. Additionally, error handling can
31+
/// become cumbersome with callbacks, whereas `async/await` allows for more straightforward
32+
/// try/catch blocks for managing errors.
33+
///
34+
/// ### Examples
35+
///
36+
/// Examples of **incorrect** code for this rule:
37+
/// ```js
38+
/// cb()
39+
/// callback()
40+
/// doSomething(arg, (err) => {})
41+
/// function doSomethingElse(cb) {}
42+
/// ```
43+
///
44+
/// Examples of **correct** code for this rule:
45+
/// ```js
46+
/// await doSomething(arg)
47+
/// async function doSomethingElse() {}
48+
/// function* generator() {
49+
/// yield yieldValue(err => {})
50+
/// }
51+
/// eventEmitter.on('error', err => {})
52+
/// ```
53+
PreferAwaitToCallbacks,
54+
style,
55+
);
56+
57+
impl Rule for PreferAwaitToCallbacks {
58+
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
59+
match node.kind() {
60+
AstKind::CallExpression(expr) => {
61+
let callee_name = expr.callee.get_identifier_reference().map(|id| id.name.as_str());
62+
if matches!(callee_name, Some("callback" | "cb")) {
63+
ctx.diagnostic(prefer_await_to_callbacks(expr.span));
64+
return;
65+
}
66+
67+
if let Some(last_arg) = expr.arguments.last() {
68+
let args = match last_arg {
69+
Argument::FunctionExpression(expr) => &expr.params,
70+
Argument::ArrowFunctionExpression(expr) => &expr.params,
71+
_ => return,
72+
};
73+
74+
let callee_property_name = expr
75+
.callee
76+
.as_member_expression()
77+
.and_then(MemberExpression::static_property_name);
78+
79+
if matches!(callee_property_name, Some("on" | "once")) {
80+
return;
81+
}
82+
83+
let is_lodash = expr.callee.as_member_expression().map_or(false, |mem_expr| {
84+
matches!(mem_expr.object(), Expression::Identifier(id) if matches!(id.name.as_str(), "_" | "lodash" | "underscore"))
85+
});
86+
87+
let calls_array_method = callee_property_name
88+
.is_some_and(Self::is_array_method)
89+
&& (expr.arguments.len() == 1 || (expr.arguments.len() == 2 && is_lodash));
90+
91+
let is_array_method =
92+
callee_name.is_some_and(Self::is_array_method) && expr.arguments.len() == 2;
93+
94+
if calls_array_method || is_array_method {
95+
return;
96+
}
97+
98+
let Some(param) = args.items.first() else {
99+
return;
100+
};
101+
102+
if matches!(param.pattern.get_identifier().as_deref(), Some("err" | "error"))
103+
&& !Self::is_inside_yield_or_await(node.id(), ctx)
104+
{
105+
ctx.diagnostic(prefer_await_to_callbacks(last_arg.span()));
106+
}
107+
}
108+
}
109+
AstKind::Function(func) => {
110+
Self::check_last_params_for_callback(&func.params, ctx);
111+
}
112+
AstKind::ArrowFunctionExpression(expr) => {
113+
Self::check_last_params_for_callback(&expr.params, ctx);
114+
}
115+
_ => {}
116+
}
117+
}
118+
}
119+
120+
impl PreferAwaitToCallbacks {
121+
fn check_last_params_for_callback(params: &FormalParameters, ctx: &LintContext) {
122+
let Some(param) = params.items.last() else {
123+
return;
124+
};
125+
126+
let id = param.pattern.get_identifier();
127+
if matches!(id.as_deref(), Some("callback" | "cb")) {
128+
ctx.diagnostic(prefer_await_to_callbacks(param.span));
129+
}
130+
}
131+
132+
fn is_inside_yield_or_await(id: NodeId, ctx: &LintContext) -> bool {
133+
ctx.nodes().iter_parents(id).skip(1).any(|parent| {
134+
matches!(parent.kind(), AstKind::AwaitExpression(_) | AstKind::YieldExpression(_))
135+
})
136+
}
137+
138+
fn is_array_method(name: &str) -> bool {
139+
["map", "every", "forEach", "some", "find", "filter"].contains(&name)
140+
}
141+
}
142+
143+
#[test]
144+
fn test() {
145+
use crate::tester::Tester;
146+
147+
let pass = vec![
148+
"async function hi() { await thing().catch(err => console.log(err)) }",
149+
"async function hi() { await thing().then() }",
150+
"async function hi() { await thing().catch() }",
151+
r#"dbConn.on("error", err => { console.error(err) })"#,
152+
r#"dbConn.once("error", err => { console.error(err) })"#,
153+
"heart(something => {})",
154+
"getErrors().map(error => responseTo(error))",
155+
"errors.filter(err => err.status === 402)",
156+
r#"errors.some(err => err.message.includes("Yo"))"#,
157+
"errors.every(err => err.status === 402)",
158+
"errors.filter(err => console.log(err))",
159+
r#"const error = errors.find(err => err.stack.includes("file.js"))"#,
160+
"this.myErrors.forEach(function(error) { log(error); })",
161+
r#"find(errors, function(err) { return err.type === "CoolError" })"#,
162+
r#"map(errors, function(error) { return err.type === "CoolError" })"#,
163+
r#"_.find(errors, function(error) { return err.type === "CoolError" })"#,
164+
r#"_.map(errors, function(err) { return err.type === "CoolError" })"#,
165+
];
166+
167+
let fail = vec![
168+
"heart(function(err) {})",
169+
"heart(err => {})",
170+
r#"heart("ball", function(err) {})"#,
171+
"function getData(id, callback) {}",
172+
"const getData = (cb) => {}",
173+
"var x = function (x, cb) {}",
174+
"cb()",
175+
"callback()",
176+
"heart(error => {})",
177+
"async.map(files, fs.stat, function(err, results) { if (err) throw err; });",
178+
"_.map(files, fs.stat, function(err, results) { if (err) throw err; });",
179+
"map(files, fs.stat, function(err, results) { if (err) throw err; });",
180+
"map(function(err, results) { if (err) throw err; });",
181+
"customMap(errors, (err) => err.message)",
182+
];
183+
184+
Tester::new(PreferAwaitToCallbacks::NAME, pass, fail).test_and_snapshot();
185+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
---
2+
source: crates/oxc_linter/src/tester.rs
3+
---
4+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
5+
╭─[prefer_await_to_callbacks.tsx:1:7]
6+
1heart(function(err) {})
7+
· ────────────────
8+
╰────
9+
10+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
11+
╭─[prefer_await_to_callbacks.tsx:1:7]
12+
1heart(err => {})
13+
· ─────────
14+
╰────
15+
16+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
17+
╭─[prefer_await_to_callbacks.tsx:1:15]
18+
1heart("ball", function(err) {})
19+
· ────────────────
20+
╰────
21+
22+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
23+
╭─[prefer_await_to_callbacks.tsx:1:22]
24+
1function getData(id, callback) {}
25+
· ────────
26+
╰────
27+
28+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
29+
╭─[prefer_await_to_callbacks.tsx:1:18]
30+
1const getData = (cb) => {}
31+
· ──
32+
╰────
33+
34+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
35+
╭─[prefer_await_to_callbacks.tsx:1:22]
36+
1var x = function (x, cb) {}
37+
· ──
38+
╰────
39+
40+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
41+
╭─[prefer_await_to_callbacks.tsx:1:1]
42+
1cb()
43+
· ────
44+
╰────
45+
46+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
47+
╭─[prefer_await_to_callbacks.tsx:1:1]
48+
1callback()
49+
· ──────────
50+
╰────
51+
52+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
53+
╭─[prefer_await_to_callbacks.tsx:1:7]
54+
1heart(error => {})
55+
· ───────────
56+
╰────
57+
58+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
59+
╭─[prefer_await_to_callbacks.tsx:1:27]
60+
1async.map(files, fs.stat, function(err, results) { if (err) throw err; });
61+
· ──────────────────────────────────────────────
62+
╰────
63+
64+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
65+
╭─[prefer_await_to_callbacks.tsx:1:23]
66+
1_.map(files, fs.stat, function(err, results) { if (err) throw err; });
67+
· ──────────────────────────────────────────────
68+
╰────
69+
70+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
71+
╭─[prefer_await_to_callbacks.tsx:1:21]
72+
1map(files, fs.stat, function(err, results) { if (err) throw err; });
73+
· ──────────────────────────────────────────────
74+
╰────
75+
76+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
77+
╭─[prefer_await_to_callbacks.tsx:1:5]
78+
1map(function(err, results) { if (err) throw err; });
79+
· ──────────────────────────────────────────────
80+
╰────
81+
82+
eslint-plugin-promise(prefer-await-to-callbacks): Prefer `async`/`await` to the callback pattern
83+
╭─[prefer_await_to_callbacks.tsx:1:19]
84+
1customMap(errors, (err) => err.message)
85+
· ────────────────────
86+
╰────

0 commit comments

Comments
 (0)
Please sign in to comment.