Skip to content

Commit 7d47aac

Browse files
committed
feat: Add timeoutsErr option to no-callback-in-promise rule (#15178)
1 parent 5fa6bb8 commit 7d47aac

File tree

2 files changed

+177
-26
lines changed

2 files changed

+177
-26
lines changed

crates/oxc_linter/src/rules/promise/no_callback_in_promise.rs

Lines changed: 110 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use oxc_diagnostics::OxcDiagnostic;
66
use oxc_macros::declare_oxc_lint;
77
use oxc_span::{CompactStr, GetSpan, Span};
88
use schemars::JsonSchema;
9+
use serde::Deserialize;
910

1011
use crate::{AstNode, context::LintContext, rule::Rule};
1112

@@ -18,20 +19,23 @@ fn no_callback_in_promise_diagnostic(span: Span) -> OxcDiagnostic {
1819
#[derive(Debug, Default, Clone)]
1920
pub struct NoCallbackInPromise(Box<NoCallbackInPromiseConfig>);
2021

21-
#[derive(Debug, Clone, JsonSchema)]
22+
#[derive(Debug, Clone, JsonSchema, Deserialize)]
2223
#[serde(rename_all = "camelCase", default)]
2324
pub struct NoCallbackInPromiseConfig {
2425
/// List of callback function names to check for within Promise `then` and `catch` methods.
2526
callbacks: Vec<CompactStr>,
2627
/// List of callback function names to allow within Promise `then` and `catch` methods.
2728
exceptions: Vec<CompactStr>,
29+
/// Boolean as to whether callbacks in timeout functions like `setTimeout` will err.
30+
timeouts_err: bool,
2831
}
2932

3033
impl Default for NoCallbackInPromiseConfig {
3134
fn default() -> Self {
3235
Self {
3336
callbacks: vec!["callback".into(), "cb".into(), "done".into(), "next".into()],
3437
exceptions: Vec::new(),
38+
timeouts_err: false,
3539
}
3640
}
3741
}
@@ -83,22 +87,18 @@ declare_oxc_lint!(
8387
config = NoCallbackInPromiseConfig,
8488
);
8589

90+
const TIMEOUT_WHITELIST: [&str; 4] =
91+
["setImmediate", "setTimeout", "requestAnimationFrame", "nextTick"];
92+
8693
impl Rule for NoCallbackInPromise {
8794
fn from_configuration(value: serde_json::Value) -> Self {
88-
let mut default_config = NoCallbackInPromiseConfig::default();
89-
90-
let exceptions: Vec<String> = value
91-
.get(0)
92-
.and_then(|v| v.get("exceptions"))
93-
.and_then(serde_json::Value::as_array)
94-
.map(|v| {
95-
v.iter().filter_map(serde_json::Value::as_str).map(ToString::to_string).collect()
96-
})
95+
let mut config: NoCallbackInPromiseConfig = value
96+
.as_array()
97+
.and_then(|arr| arr.first())
98+
.and_then(|value| serde_json::from_value(value.clone()).ok())
9799
.unwrap_or_default();
98-
99-
default_config.callbacks.retain(|item| !exceptions.contains(&item.to_string()));
100-
101-
Self(Box::new(default_config))
100+
config.callbacks.retain(|item| !config.exceptions.contains(&item));
101+
Self(Box::new(config))
102102
}
103103

104104
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
@@ -112,20 +112,33 @@ impl Rule for NoCallbackInPromise {
112112
.is_none_or(|id| self.callbacks.binary_search(&id.name.as_str().into()).is_err());
113113

114114
if is_not_callback {
115+
let Some(id) = call_expr
116+
.arguments
117+
.first()
118+
.and_then(|arg| arg.as_expression().and_then(Expression::get_identifier_reference))
119+
else {
120+
return;
121+
};
122+
let name = id.name.as_str();
123+
if self.callbacks.binary_search(&name.into()).is_err() {
124+
return;
125+
}
115126
if Self::has_promise_callback(call_expr) {
116-
let Some(id) = call_expr.arguments.first().and_then(|arg| {
117-
arg.as_expression().and_then(Expression::get_identifier_reference)
118-
}) else {
119-
return;
120-
};
121-
122-
let name = id.name.as_str();
123-
if self.callbacks.binary_search(&name.into()).is_ok() {
124-
ctx.diagnostic(no_callback_in_promise_diagnostic(id.span));
125-
}
127+
ctx.diagnostic(no_callback_in_promise_diagnostic(id.span));
128+
return;
129+
} else if !self.timeouts_err && Self::is_inside_timeout(node) {
130+
return;
131+
}
132+
}
133+
let ancestors = ctx.nodes().ancestors(node.id());
134+
for ancestor in ancestors {
135+
if !self.timeouts_err && Self::is_inside_timeout(ancestor) {
136+
break;
137+
}
138+
if Self::is_inside_promise(ancestor, ctx) {
139+
ctx.diagnostic(no_callback_in_promise_diagnostic(node.span()));
140+
break;
126141
}
127-
} else if ctx.nodes().ancestors(node.id()).any(|node| Self::is_inside_promise(node, ctx)) {
128-
ctx.diagnostic(no_callback_in_promise_diagnostic(node.span()));
129142
}
130143
}
131144
}
@@ -152,6 +165,23 @@ impl NoCallbackInPromise {
152165
Some("then" | "catch")
153166
)
154167
}
168+
169+
fn is_inside_timeout(node: &AstNode) -> bool {
170+
let Some(call_expr) = node.kind().as_call_expression() else {
171+
return false;
172+
};
173+
match &call_expr.callee {
174+
Expression::Identifier(ident) => {
175+
return TIMEOUT_WHITELIST.contains(&ident.name.as_str());
176+
}
177+
Expression::StaticMemberExpression(static_member_expr) => {
178+
return TIMEOUT_WHITELIST.contains(&static_member_expr.property.name.as_str());
179+
}
180+
_ => {
181+
return false;
182+
}
183+
}
184+
}
155185
}
156186

157187
#[test]
@@ -163,6 +193,17 @@ fn test() {
163193
("doSomething(function(err) { cb(err) })", None),
164194
("function thing(callback) { callback() }", None),
165195
("doSomething(function(err) { callback(err) })", None),
196+
("a.then(doSomething)", None),
197+
("a.then(() => doSomething())", None),
198+
("a.then(function(err) { doSomething(err) })", None),
199+
("a.then(function(data) { doSomething(data) }, function(err) { doSomething(err) })", None),
200+
("a.catch(function(err) { doSomething(err) })", None),
201+
("whatever.then((err) => { process.nextTick(() => cb()) })", None),
202+
("whatever.then((err) => { setImmediate(() => cb()) })", None),
203+
("whatever.then((err) => setImmediate(() => cb()))", None),
204+
("whatever.then((err) => process.nextTick(() => cb()))", None),
205+
("whatever.then((err) => process.nextTick(cb))", None),
206+
("whatever.then((err) => setImmediate(cb))", None),
166207
("let thing = (cb) => cb()", None),
167208
("doSomething(err => cb(err))", None),
168209
("a.then(() => next())", Some(serde_json::json!([{ "exceptions": ["next"] }]))),
@@ -185,6 +226,49 @@ fn test() {
185226
("a.then(function(err) { callback(err) })", None),
186227
("a.then(function(data) { callback(data) }, function(err) { callback(err) })", None),
187228
("a.catch(function(err) { callback(err) })", None),
229+
("a.then(() => doSometing(cb))", None),
230+
(
231+
"function wait (callback) {
232+
return Promise.resolve()
233+
.then(() => {
234+
setTimeout(callback);
235+
});
236+
}",
237+
Some(serde_json::json!([{ "timeoutsErr": true }])),
238+
),
239+
(
240+
"function wait (callback) {
241+
return Promise.resolve()
242+
.then(() => {
243+
setTimeout(() => callback());
244+
});
245+
}",
246+
Some(serde_json::json!([{ "timeoutsErr": true }])),
247+
),
248+
(
249+
"whatever.then((err) => { process.nextTick(() => cb()) })",
250+
Some(serde_json::json!([{ "timeoutsErr": true }])),
251+
),
252+
(
253+
"whatever.then((err) => { setImmediate(() => cb()) })",
254+
Some(serde_json::json!([{ "timeoutsErr": true }])),
255+
),
256+
(
257+
"whatever.then((err) => setImmediate(() => cb()))",
258+
Some(serde_json::json!([{ "timeoutsErr": true }])),
259+
),
260+
(
261+
"whatever.then((err) => process.nextTick(() => cb()))",
262+
Some(serde_json::json!([{ "timeoutsErr": true }])),
263+
),
264+
(
265+
"whatever.then((err) => process.nextTick(cb))",
266+
Some(serde_json::json!([{ "timeoutsErr": true }])),
267+
),
268+
(
269+
"whatever.then((err) => setImmediate(cb))",
270+
Some(serde_json::json!([{ "timeoutsErr": true }])),
271+
),
188272
];
189273

190274
Tester::new(NoCallbackInPromise::NAME, NoCallbackInPromise::PLUGIN, pass, fail)

crates/oxc_linter/src/snapshots/promise_no_callback_in_promise.snap

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,70 @@ source: crates/oxc_linter/src/tester.rs
8484
· ─────────────
8585
╰────
8686
help: Use `then` and `catch` directly
87+
88+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
89+
╭─[no_callback_in_promise.tsx:1:14]
90+
1a.then(() => doSometing(cb))
91+
· ──────────────
92+
╰────
93+
help: Use `then` and `catch` directly
94+
95+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
96+
╭─[no_callback_in_promise.tsx:4:13]
97+
3 │ .then(() => {
98+
4setTimeout(callback);
99+
· ────────────────────
100+
5 │ });
101+
╰────
102+
help: Use `then` and `catch` directly
103+
104+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
105+
╭─[no_callback_in_promise.tsx:4:30]
106+
3 │ .then(() => {
107+
4setTimeout(() => callback());
108+
· ──────────
109+
5 │ });
110+
╰────
111+
help: Use `then` and `catch` directly
112+
113+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
114+
╭─[no_callback_in_promise.tsx:1:49]
115+
1whatever.then((err) => { process.nextTick(() => cb()) })
116+
· ────
117+
╰────
118+
help: Use `then` and `catch` directly
119+
120+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
121+
╭─[no_callback_in_promise.tsx:1:45]
122+
1whatever.then((err) => { setImmediate(() => cb()) })
123+
· ────
124+
╰────
125+
help: Use `then` and `catch` directly
126+
127+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
128+
╭─[no_callback_in_promise.tsx:1:43]
129+
1whatever.then((err) => setImmediate(() => cb()))
130+
· ────
131+
╰────
132+
help: Use `then` and `catch` directly
133+
134+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
135+
╭─[no_callback_in_promise.tsx:1:47]
136+
1whatever.then((err) => process.nextTick(() => cb()))
137+
· ────
138+
╰────
139+
help: Use `then` and `catch` directly
140+
141+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
142+
╭─[no_callback_in_promise.tsx:1:24]
143+
1whatever.then((err) => process.nextTick(cb))
144+
· ────────────────────
145+
╰────
146+
help: Use `then` and `catch` directly
147+
148+
eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
149+
╭─[no_callback_in_promise.tsx:1:24]
150+
1whatever.then((err) => setImmediate(cb))
151+
· ────────────────
152+
╰────
153+
help: Use `then` and `catch` directly

0 commit comments

Comments
 (0)