feat(linter): add promise/no-nesting

crates/oxc_linter/src/rules.rs

@@ -516,6 +516,7 @@ mod promise {
     pub mod avoid_new;
     pub mod catch_or_return;
     pub mod no_callback_in_promise;
+    pub mod no_nesting;
     pub mod no_new_statics;
     pub mod no_promise_in_callback;
     pub mod no_return_in_finally;
@@ -846,6 +847,7 @@ oxc_macros::declare_all_lint_rules! {
     oxc::uninvoked_array_callback,
     promise::avoid_new,
     promise::catch_or_return,
+    promise::no_nesting,
     promise::no_promise_in_callback,
     promise::no_callback_in_promise,
     promise::no_new_statics,

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

@@ -0,0 +1,303 @@
+use oxc_ast::{
+    AstKind,
+    ast::{CallExpression, Expression, MemberExpression},
+};
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_semantic::ScopeId;
+use oxc_span::{GetSpan, Span};
+
+use crate::{AstNode, context::LintContext, rule::Rule};
+
+fn no_nesting_diagnostic(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::warn("Avoid nesting promises.")
+        .with_help("Refactor so that promises are chained in a flat manner.")
+        .with_label(span)
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct NoNesting;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Disallow nested then() or catch() statements.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Nesting promises makes code harder to read and understand.
+    ///
+    /// ### Examples
+    ///
+    /// Examples of **incorrect** code for this rule:
+    /// ```javascript
+    /// doThing().then(() => a.then())
+    /// ```
+    ///
+    /// ```javascript
+    /// doThing().then(function() { a.then() })
+    /// ```
+    ///
+    /// ```javascript
+    /// doThing().then(() => { b.catch() })
+    /// ```
+    ///
+    /// Examples of **correct** code for this rule:
+    /// ```javascript
+    /// doThing().then(() => 4)
+    /// ```
+    ///
+    /// ```javascript
+    /// doThing().then(function() { return 4 })
+    /// ```
+    ///
+    /// ```javascript
+    /// doThing().catch(() => 4)
+    /// ```
+    ///
+    /// ```javascript
+    /// doThing()
+    ///   .then(() => Promise.resolve(1))
+    ///   .then(() => Promise.resolve(2))
+    /// ```
+    ///
+    /// This example is not a rule violation as unnesting here would
+    /// result in `a` being undefined in the expression `getC(a, b)`.
+    /// ```javascript
+    /// doThing()
+    ///	  .then(a => getB(a)
+    ///      .then(b => getC(a, b))
+    ///    )
+    /// ```
+    NoNesting,
+    promise,
+    style,
+    pending
+);
+
+fn is_inside_promise(node: &AstNode, ctx: &LintContext) -> bool {
+    if !matches!(node.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_))
+        || !matches!(ctx.nodes().parent_kind(node.id()), Some(AstKind::Argument(_)))
+    {
+        return false;
+    }
+
+    ctx.nodes()
+        .ancestors(node.id())
+        .nth(2)
+        .is_some_and(|node| node.kind().as_call_expression().is_some_and(has_promise_callback))
+}
+
+/// Gets the closest promise callback function of the nested promise.
+fn closest_promise_cb<'a, 'b>(
+    node: &'a AstNode<'b>,
+    ctx: &'a LintContext<'b>,
+) -> Option<&'a CallExpression<'b>> {
+    ctx.nodes()
+        .ancestors(node.id())
+        .filter_map(|node| node.kind().as_call_expression())
+        .filter(|a| has_promise_callback(a))
+        .nth(1)
+}
+
+fn has_promise_callback(call_expr: &CallExpression) -> bool {
+    matches!(
+        call_expr.callee.as_member_expression().and_then(MemberExpression::static_property_name),
+        Some("then" | "catch")
+    )
+}
+
+fn is_promise_then_or_catch(call_expr: &CallExpression) -> bool {
+    let Some(member_expr) = call_expr.callee.get_member_expr() else {
+        return false;
+    };
+
+    member_expr
+        .static_property_name()
+        .map_or_else(|| false, |prop_name| matches!(prop_name, "then" | "catch"))
+}
+
+/// Checks if we can safely unnest the promise callback.
+///
+/// 1. This function gets variable bindings defined in closest parent promise callback function
+///    scope.
+///
+/// 2. Checks if the argument callback of the nested promise call uses any of these variables
+///    and if so returns `false` to denote that the promises cannot be safely unnested.
+///
+/// Here is an example of a nested promise which isn't safe to nest without further refactoring.
+/// ```javascript
+/// doThing()
+///  .then(a => getB(a) <---- 1. Get this scopes bound variables
+///    .then(b => getC(a, b)) <--- 2. Check for references to the bound variables from 1.
+///  )
+/// ```
+///
+/// In this case unnesting is not safe as doing so would result in `a` being undefined in the
+/// expression `getC(a, b)`, as seen below in the unnested version of the example:
+/// ```javascript
+/// doThing()
+///  .then(a => getB(a))
+///  .then(b => getC(a, b))
+/// ```
+fn can_safely_unnest(
+    cb_call_expr: &CallExpression,
+    closest: &CallExpression,
+    ctx: &LintContext,
+) -> bool {
+    let mut safe_to_unnest: bool = true;
+
+    if let Some(cb_span) = cb_call_expr.arguments.first().map(GetSpan::span) {
+        closest.arguments.iter().for_each(|new_expr| {
+            if let Some(arg_expr) = new_expr.as_expression() {
+                match arg_expr {
+                    Expression::ArrowFunctionExpression(arrow_expr) => {
+                        let scope = arrow_expr.scope_id();
+                        if uses_closest_cb_vars(scope, cb_span, ctx) {
+                            safe_to_unnest = false;
+                        }
+                    }
+                    Expression::FunctionExpression(func_expr) => {
+                        let scope = func_expr.scope_id();
+                        if uses_closest_cb_vars(scope, cb_span, ctx) {
+                            safe_to_unnest = false;
+                        }
+                    }
+                    _ => {}
+                }
+            };
+        });
+    };
+
+    safe_to_unnest
+}
+
+/// Check for references in cb_span to variables defined in the closest parent cb scope
+/// and returns true if the nested promise callback uses references that are bound in
+/// the closest parent callback scope.
+///
+/// In the given example we would loop through all bindings in the closest
+/// parent scope a,b,c,d.
+///
+///  .then((a,b,c) => { // closest_cb_scope_id
+///    const d = 5;
+///    getB(a).then(d => getC(a, b)) });
+///                // ^^^^^^^^^^^^^^ <- `cb_span`
+fn uses_closest_cb_vars(closest_cb_scope_id: ScopeId, cb_span: Span, ctx: &LintContext) -> bool {
+    for (_, binding_symbol_id) in ctx.scopes().get_bindings(closest_cb_scope_id) {
+        for usage in ctx.semantic().symbol_references(*binding_symbol_id) {
+            let usage_span: Span = ctx.reference_span(usage);
+            if cb_span.contains_inclusive(usage_span) {
+                // Cannot unnest this nested promise as the nested cb refers to a variable
+                // defined in the parent promise callback scope. Unnesting would result in
+                // reference to an undefined variable.
+                return true;
+            };
+        }
+    }
+
+    false
+}
+
+impl Rule for NoNesting {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        let AstKind::CallExpression(call_expr) = node.kind() else {
+            return;
+        };
+
+        if !is_promise_then_or_catch(call_expr) {
+            return;
+        };
+
+        let mut ancestors = ctx.nodes().ancestors(node.id());
+        if ancestors.any(|node| is_inside_promise(node, ctx)) {
+            match closest_promise_cb(node, ctx) {
+                Some(closest) => {
+                    if can_safely_unnest(call_expr, closest, ctx) {
+                        ctx.diagnostic(no_nesting_diagnostic(call_expr.callee.span()));
+                    }
+                }
+                None => ctx.diagnostic(no_nesting_diagnostic(call_expr.callee.span())),
+            }
+        };
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        "Promise.resolve(4).then(function(x) { return x })",
+        "Promise.reject(4).then(function(x) { return x })",
+        "Promise.resolve(4).then(function() {})",
+        "Promise.reject(4).then(function() {})",
+        "doThing().then(function() { return 4 })",
+        "doThing().then(function() { throw 4 })",
+        "doThing().then(null, function() { return 4 })",
+        "doThing().then(null, function() { throw 4 })",
+        "doThing().catch(null, function() { return 4 })",
+        "doThing().catch(null, function() { throw 4 })",
+        "doThing().then(() => 4)",
+        "doThing().then(() => { throw 4 })",
+        "doThing().then(()=>{}, () => 4)",
+        "doThing().then(()=>{}, () => { throw 4 })",
+        "doThing().catch(() => 4)",
+        "doThing().catch(() => { throw 4 })",
+        "var x = function() { return Promise.resolve(4) }",
+        "function y() { return Promise.resolve(4) }",
+        "function then() { return Promise.reject() }",
+        "doThing(function(x) { return Promise.reject(x) })",
+        "doThing().then(function() { return Promise.all([a,b,c]) })",
+        "doThing().then(function() { return Promise.resolve(4) })",
+        "doThing().then(() => Promise.resolve(4))",
+        "doThing()
+          .then(() => Promise.resolve(1))
+          .then(() => Promise.resolve(2))",
+        "doThing().then(() => Promise.all([a]))",
+        "doThing()
+		  .then(a => getB(a)
+			.then(b => getC(a, b))
+		  )",
+        "doThing()
+		  .then(a => getB(a)
+		    .then(function(b) { getC(a, b) })
+		  )",
+        "doThing()
+          .then(a => {
+            const c = a * 2;
+            return getB(c).then(b => getC(c, b))
+          })",
+        "doThing()
+          .then(function (a) {
+            const c = a * 2;
+            return getB(c).then(function () { getC(c, b) } )
+          })",
+    ];
+
+    let fail = vec![
+        "doThing().then(function() { a.then() })",
+        "doThing().then(function() { b.catch() })",
+        "doThing().then(function() { return a.then() })",
+        "doThing().then(function() { return b.catch() })",
+        "doThing().then(() => { a.then() })",
+        "doThing().then(() => { b.catch() })",
+        "doThing().then(() => a.then())",
+        "doThing().then(() => b.catch())",
+        "doThing()
+          .then(() =>
+            a.then(() => Promise.resolve(1)))",
+        "doThing()
+		  .then(a => getB(a)
+		    .then(b => getC(b))
+		  )",
+        "doThing()
+		  .then(a => getB(a)
+		    .then(b => getC(a, b)
+		      .then(c => getD(a, c))
+			)
+		  )",
+    ];
+
+    Tester::new(NoNesting::NAME, NoNesting::PLUGIN, pass, fail).test_and_snapshot();
+}