Skip to content

Commit 069ef2d

Browse files
refactor(linter): improve promise/no-nesting (#9544)
- Use the `utils::is_promise` function where possible instead of duplicating this logic in this rule. - Condense some of the doc examples. - Add missing test cases related to `.catch`, which are cited in the eslint-plugin-promise documentation [here ](https://github.com/eslint-community/eslint-plugin-promise/blob/main/docs/rules/no-nesting.md).
1 parent ffd485c commit 069ef2d

File tree

2 files changed

+32
-33
lines changed

2 files changed

+32
-33
lines changed

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

+18-33
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
use oxc_ast::{
22
AstKind,
3-
ast::{CallExpression, Expression, MemberExpression},
3+
ast::{CallExpression, Expression},
44
};
55
use oxc_diagnostics::OxcDiagnostic;
66
use oxc_macros::declare_oxc_lint;
77
use oxc_semantic::ScopeId;
88
use oxc_span::{GetSpan, Span};
99

10-
use crate::{AstNode, context::LintContext, rule::Rule};
10+
use crate::{AstNode, context::LintContext, rule::Rule, utils::is_promise};
1111

1212
fn no_nesting_diagnostic(span: Span) -> OxcDiagnostic {
1313
OxcDiagnostic::warn("Avoid nesting promises.")
@@ -32,26 +32,20 @@ declare_oxc_lint!(
3232
/// Examples of **incorrect** code for this rule:
3333
/// ```javascript
3434
/// doThing().then(() => a.then())
35-
/// ```
3635
///
37-
/// ```javascript
3836
/// doThing().then(function() { a.then() })
39-
/// ```
4037
///
41-
/// ```javascript
4238
/// doThing().then(() => { b.catch() })
39+
///
40+
/// doThing().catch((val) => doSomething(val).catch(errors))
4341
/// ```
4442
///
4543
/// Examples of **correct** code for this rule:
4644
/// ```javascript
4745
/// doThing().then(() => 4)
48-
/// ```
4946
///
50-
/// ```javascript
5147
/// doThing().then(function() { return 4 })
52-
/// ```
5348
///
54-
/// ```javascript
5549
/// doThing().catch(() => 4)
5650
/// ```
5751
///
@@ -82,10 +76,11 @@ fn is_inside_promise(node: &AstNode, ctx: &LintContext) -> bool {
8276
return false;
8377
}
8478

85-
ctx.nodes()
86-
.ancestors(node.id())
87-
.nth(2)
88-
.is_some_and(|node| node.kind().as_call_expression().is_some_and(has_promise_callback))
79+
ctx.nodes().ancestors(node.id()).nth(2).is_some_and(|node| {
80+
node.kind().as_call_expression().is_some_and(|a| {
81+
is_promise(a).is_some_and(|prop_name| prop_name == "then" || prop_name == "catch")
82+
})
83+
})
8984
}
9085

9186
/// Gets the closest promise callback function of the nested promise.
@@ -96,27 +91,13 @@ fn closest_promise_cb<'a, 'b>(
9691
ctx.nodes()
9792
.ancestors(node.id())
9893
.filter_map(|node| node.kind().as_call_expression())
99-
.filter(|a| has_promise_callback(a))
94+
.filter(|ancestor| {
95+
is_promise(ancestor)
96+
.is_some_and(|prop_name| prop_name == "then" || prop_name == "catch")
97+
})
10098
.nth(1)
10199
}
102100

103-
fn has_promise_callback(call_expr: &CallExpression) -> bool {
104-
matches!(
105-
call_expr.callee.as_member_expression().and_then(MemberExpression::static_property_name),
106-
Some("then" | "catch")
107-
)
108-
}
109-
110-
fn is_promise_then_or_catch(call_expr: &CallExpression) -> bool {
111-
let Some(member_expr) = call_expr.callee.get_member_expr() else {
112-
return false;
113-
};
114-
115-
member_expr
116-
.static_property_name()
117-
.map_or_else(|| false, |prop_name| matches!(prop_name, "then" | "catch"))
118-
}
119-
120101
/// Checks if we can safely unnest the promise callback.
121102
///
122103
/// 1. This function gets variable bindings defined in closest parent promise callback function
@@ -205,7 +186,9 @@ impl Rule for NoNesting {
205186
return;
206187
};
207188

208-
if !is_promise_then_or_catch(call_expr) {
189+
if !is_promise(call_expr)
190+
.is_some_and(|prop_name| prop_name == "then" || prop_name == "catch")
191+
{
209192
return;
210193
};
211194

@@ -284,6 +267,8 @@ fn test() {
284267
"doThing().then(() => { b.catch() })",
285268
"doThing().then(() => a.then())",
286269
"doThing().then(() => b.catch())",
270+
"doThing().then((val) => doSomething(val).catch(errors))",
271+
"doThing().catch((val) => doSomething(val).catch(errors))",
287272
"doThing()
288273
.then(() =>
289274
a.then(() => Promise.resolve(1)))",

crates/oxc_linter/src/snapshots/promise_no_nesting.snap

+14
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,20 @@ source: crates/oxc_linter/src/tester.rs
5757
╰────
5858
help: Refactor so that promises are chained in a flat manner.
5959

60+
eslint-plugin-promise(no-nesting): Avoid nesting promises.
61+
╭─[no_nesting.tsx:1:25]
62+
1doThing().then((val) => doSomething(val).catch(errors))
63+
· ──────────────────────
64+
╰────
65+
help: Refactor so that promises are chained in a flat manner.
66+
67+
eslint-plugin-promise(no-nesting): Avoid nesting promises.
68+
╭─[no_nesting.tsx:1:26]
69+
1doThing().catch((val) => doSomething(val).catch(errors))
70+
· ──────────────────────
71+
╰────
72+
help: Refactor so that promises are chained in a flat manner.
73+
6074
eslint-plugin-promise(no-nesting): Avoid nesting promises.
6175
╭─[no_nesting.tsx:3:13]
6276
2 │ .then(() =>

0 commit comments

Comments
 (0)