Skip to content

Commit 4ad26b9

Browse files
no-yanno-yan
authored
feat(linter): add no-promise-in-callback (#7307)
related: #4655 This PR implements a rule to detect Promises inside error-first callbacks, preventing the mixed usage of callbacks and Promises. Example of problematic code: ```javascript a(function(err) { doThing().then(a) }); ^^^^^^^^^^^^^^ ``` [Original implementation](https://github.com/eslint-community/eslint-plugin-promise/blob/266ddbb03076c05c362a6daecb9382b80cdd7108/rules/no-promise-in-callback.js)
1 parent 9002e97 commit 4ad26b9

3 files changed

Lines changed: 250 additions & 0 deletions

File tree

‎crates/oxc_linter/src/rules.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,7 @@ mod promise {
485485
pub mod catch_or_return;
486486
pub mod no_callback_in_promise;
487487
pub mod no_new_statics;
488+
pub mod no_promise_in_callback;
488489
pub mod no_return_in_finally;
489490
pub mod param_names;
490491
pub mod prefer_await_to_callbacks;
@@ -784,6 +785,7 @@ oxc_macros::declare_all_lint_rules! {
784785
oxc::uninvoked_array_callback,
785786
promise::avoid_new,
786787
promise::catch_or_return,
788+
promise::no_promise_in_callback,
787789
promise::no_callback_in_promise,
788790
promise::no_new_statics,
789791
promise::no_return_in_finally,

‎crates/oxc_linter/src/rules/promise/no_promise_in_callback.rs‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
use oxc_ast::ast::FormalParameters;
2+
use oxc_ast::AstKind;
3+
use oxc_diagnostics::OxcDiagnostic;
4+
use oxc_macros::declare_oxc_lint;
5+
use oxc_span::{GetSpan, Span};
6+
7+
use crate::utils::is_promise;
8+
use crate::{context::LintContext, rule::Rule, AstNode};
9+
10+
fn no_promise_in_callback_diagnostic(span: Span) -> OxcDiagnostic {
11+
OxcDiagnostic::warn("Avoid using promises inside of callbacks.")
12+
.with_help("Use either promises or callbacks exclusively for handling asynchronous code.")
13+
.with_label(span)
14+
}
15+
16+
#[derive(Debug, Default, Clone)]
17+
pub struct NoPromiseInCallback;
18+
19+
declare_oxc_lint!(
20+
/// ### What it does
21+
/// Disallows the use of Promises within error-first callback functions.
22+
///
23+
/// ### Why is this bad?
24+
/// Mixing Promises and callbacks can lead to unclear and inconsistent code.
25+
/// Promises and callbacks are different patterns for handling asynchronous code.
26+
/// Mixing them makes the logic flow harder to follow and complicates error handling,
27+
/// as callbacks rely on an error-first pattern, while Promises use `catch`.
28+
///
29+
/// ### Examples
30+
///
31+
/// Examples of **incorrect** code for this rule:
32+
/// ```js
33+
/// doSomething((err, val) => {
34+
/// if (err) console.error(err)
35+
/// else doSomethingElse(val).then(console.log)
36+
/// })
37+
/// ```
38+
///
39+
/// Examples of **correct** code for this rule:
40+
/// ```js
41+
/// promisify(doSomething)()
42+
/// .then(doSomethingElse)
43+
/// .then(console.log)
44+
/// .catch(console.error)
45+
/// ```
46+
NoPromiseInCallback,
47+
suspicious,
48+
);
49+
50+
impl Rule for NoPromiseInCallback {
51+
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
52+
let AstKind::CallExpression(call_expr) = node.kind() else {
53+
return;
54+
};
55+
56+
if is_promise(call_expr).is_none() {
57+
return;
58+
}
59+
60+
// When a Promise is returned in a ReturnStatement, the function is most likely
61+
// being used as part of a Promise chain rather than as a callback function.
62+
// To avoid false positives, this case is intentionally excluded from the scope of this rule.
63+
if let Some(AstKind::ReturnStatement(_)) = ctx.nodes().parent_kind(node.id()) {
64+
return;
65+
};
66+
67+
let mut ancestors = ctx.nodes().ancestors(node.id());
68+
if ancestors.any(|node| is_callback_function(node, ctx)) {
69+
ctx.diagnostic(no_promise_in_callback_diagnostic(call_expr.callee.span()));
70+
}
71+
}
72+
}
73+
74+
fn is_callback_function<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool {
75+
if !node.kind().is_function_like() {
76+
return false;
77+
}
78+
79+
if is_within_promise_handler(node, ctx) {
80+
return false;
81+
}
82+
83+
match node.kind() {
84+
AstKind::Function(function) => is_error_first_callback(&function.params),
85+
AstKind::ArrowFunctionExpression(arrow_function) => {
86+
is_error_first_callback(&arrow_function.params)
87+
}
88+
_ => false,
89+
}
90+
}
91+
92+
fn is_error_first_callback(params: &FormalParameters) -> bool {
93+
let Some(first_parameter) = params.items.first() else {
94+
return false;
95+
};
96+
97+
let Some(ident) = first_parameter.pattern.get_binding_identifier() else {
98+
return false;
99+
};
100+
101+
matches!(ident.name.as_str(), "err" | "error")
102+
}
103+
104+
fn is_within_promise_handler<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool {
105+
if !matches!(node.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) {
106+
return false;
107+
}
108+
109+
let Some(parent) = ctx.nodes().parent_node(node.id()) else {
110+
return false;
111+
};
112+
if !matches!(ctx.nodes().kind(parent.id()), AstKind::Argument(_)) {
113+
return false;
114+
};
115+
116+
let Some(AstKind::CallExpression(call_expr)) = ctx.nodes().parent_kind(parent.id()) else {
117+
return false;
118+
};
119+
120+
matches!(call_expr.callee_name(), Some("then" | "catch"))
121+
}
122+
123+
#[test]
124+
fn test() {
125+
use crate::tester::Tester;
126+
127+
// The following test cases are based on the original
128+
// implementation from eslint-plugin-promise and are licensed under the ISC License.
129+
//
130+
// Copyright (c) 2020, Jamund Ferguson
131+
// https://github.com/eslint-community/eslint-plugin-promise/blob/266ddbb03076c05c362a6daecb9382b80cdd7108/__tests__/no-promise-in-callback.js
132+
let pass = vec![
133+
"go(function() { return Promise.resolve(4) })",
134+
"go(function() { return a.then(b) })",
135+
"go(function() { b.catch(c) })",
136+
"go(function() { b.then(c, d) })",
137+
"go(() => Promise.resolve(4))",
138+
"go((errrr) => a.then(b))",
139+
"go((helpers) => { b.catch(c) })",
140+
"go((e) => { b.then(c, d) })",
141+
"a.catch((err) => { b.then(c, d) })",
142+
"var x = function() { return Promise.resolve(4) }",
143+
"function y() { return Promise.resolve(4) }",
144+
"function then() { return Promise.reject() }",
145+
"doThing(function(x) { return Promise.reject(x) })",
146+
"doThing().then(function() { return Promise.all([a,b,c]) })",
147+
"doThing().then(function() { return Promise.resolve(4) })",
148+
"doThing().then(() => Promise.resolve(4))",
149+
"doThing().then(() => Promise.all([a]))",
150+
"a(function(err) { return doThing().then(a) })",
151+
];
152+
153+
let fail = vec![
154+
"a(function(err) { doThing().then(a) })",
155+
"a(function(error, zup, supa) { doThing().then(a) })",
156+
"a(function(error) { doThing().then(a) })",
157+
"a((error) => { doThing().then(a) })",
158+
"a((error) => doThing().then(a))",
159+
"a((err, data) => { doThing().then(a) })",
160+
"a((err, data) => doThing().then(a))",
161+
"function x(err) { Promise.all() }",
162+
"function x(err) { Promise.allSettled() }",
163+
"function x(err) { Promise.any() }",
164+
"let x = (err) => doThingWith(err).then(a)",
165+
];
166+
167+
Tester::new(NoPromiseInCallback::NAME, pass, fail).test_and_snapshot();
168+
}

‎crates/oxc_linter/src/snapshots/no_promise_in_callback.snap‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
---
2+
source: crates/oxc_linter/src/tester.rs
3+
snapshot_kind: text
4+
---
5+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
6+
╭─[no_promise_in_callback.tsx:1:19]
7+
1 │ a(function(err) { doThing().then(a) })
8+
· ──────────────
9+
╰────
10+
help: Use either promises or callbacks exclusively for handling asynchronous code.
11+
12+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
13+
╭─[no_promise_in_callback.tsx:1:32]
14+
1 │ a(function(error, zup, supa) { doThing().then(a) })
15+
· ──────────────
16+
╰────
17+
help: Use either promises or callbacks exclusively for handling asynchronous code.
18+
19+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
20+
╭─[no_promise_in_callback.tsx:1:21]
21+
1 │ a(function(error) { doThing().then(a) })
22+
· ──────────────
23+
╰────
24+
help: Use either promises or callbacks exclusively for handling asynchronous code.
25+
26+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
27+
╭─[no_promise_in_callback.tsx:1:16]
28+
1 │ a((error) => { doThing().then(a) })
29+
· ──────────────
30+
╰────
31+
help: Use either promises or callbacks exclusively for handling asynchronous code.
32+
33+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
34+
╭─[no_promise_in_callback.tsx:1:14]
35+
1 │ a((error) => doThing().then(a))
36+
· ──────────────
37+
╰────
38+
help: Use either promises or callbacks exclusively for handling asynchronous code.
39+
40+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
41+
╭─[no_promise_in_callback.tsx:1:20]
42+
1 │ a((err, data) => { doThing().then(a) })
43+
· ──────────────
44+
╰────
45+
help: Use either promises or callbacks exclusively for handling asynchronous code.
46+
47+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
48+
╭─[no_promise_in_callback.tsx:1:18]
49+
1 │ a((err, data) => doThing().then(a))
50+
· ──────────────
51+
╰────
52+
help: Use either promises or callbacks exclusively for handling asynchronous code.
53+
54+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
55+
╭─[no_promise_in_callback.tsx:1:19]
56+
1 │ function x(err) { Promise.all() }
57+
· ───────────
58+
╰────
59+
help: Use either promises or callbacks exclusively for handling asynchronous code.
60+
61+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
62+
╭─[no_promise_in_callback.tsx:1:19]
63+
1 │ function x(err) { Promise.allSettled() }
64+
· ──────────────────
65+
╰────
66+
help: Use either promises or callbacks exclusively for handling asynchronous code.
67+
68+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
69+
╭─[no_promise_in_callback.tsx:1:19]
70+
1 │ function x(err) { Promise.any() }
71+
· ───────────
72+
╰────
73+
help: Use either promises or callbacks exclusively for handling asynchronous code.
74+
75+
⚠ eslint-plugin-promise(no-promise-in-callback): Avoid using promises inside of callbacks.
76+
╭─[no_promise_in_callback.tsx:1:18]
77+
1 │ let x = (err) => doThingWith(err).then(a)
78+
· ─────────────────────
79+
╰────
80+
help: Use either promises or callbacks exclusively for handling asynchronous code.

0 commit comments

Comments
 (0)