feat(linter): implement no-callback-in-promise

crates/oxc_linter/src/rules.rs

@@ -468,6 +468,7 @@ mod tree_shaking {
 mod promise {
     pub mod avoid_new;
     pub mod catch_or_return;
+    pub mod no_callback_in_promise;
     pub mod no_new_statics;
     pub mod no_return_in_finally;
     pub mod param_names;
@@ -758,6 +759,7 @@ oxc_macros::declare_all_lint_rules! {
     oxc::uninvoked_array_callback,
     promise::avoid_new,
     promise::catch_or_return,
+    promise::no_callback_in_promise,
     promise::no_new_statics,
     promise::no_return_in_finally,
     promise::param_names,

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

@@ -0,0 +1,193 @@
+use oxc_ast::{
+    ast::{Expression, MemberExpression},
+    AstKind,
+};
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_span::{GetSpan, Span};
+
+use crate::{context::LintContext, rule::Rule, AstNode};
+
+static CALLBACKS: [&str; 4] = ["done", "cb", "callback", "next"];
+
+fn no_callback_in_promise_diagnostic(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::warn("Avoid calling back inside of a promise").with_label(span)
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct NoCallbackInPromise(Box<NoCallbackInPromiseConfig>);
+
+#[derive(Debug, Default, Clone)]
+pub struct NoCallbackInPromiseConfig {
+    exceptions: Vec<String>,
+}
+
+impl std::ops::Deref for NoCallbackInPromise {
+    type Target = NoCallbackInPromiseConfig;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Disallows calling a callback function (`cb()`) inside a `Promise.prototype.then()`
+    /// or `Promise.prototype.catch()`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Directly invoking a callback inside a `then()` or `catch()` method can lead to
+    /// unexpected behavior, such as the callback being called multiple times. Additionally,
+    /// mixing the callback and promise paradigms in this way can make the code confusing
+    /// and harder to maintain.
+    ///
+    /// ### Examples
+    ///
+    /// Examples of **incorrect** code for this rule:
+    /// ```js
+    /// function callback(err, data) {
+    ///     console.log('Callback got called with:', err, data)
+    ///     throw new Error('My error')
+    ///   }
+    ///
+    /// Promise.resolve()
+    ///   .then(() => callback(null, 'data'))
+    ///   .catch((err) => callback(err.message, null))
+    /// ```
+    ///
+    /// Examples of **correct** code for this rule:
+    /// ```js
+    /// Promise.resolve()
+    ///   .then(() => setTimeout(() => callback(null, 'data'), 0))
+    ///   .catch((err) => setTimeout(() => callback(err.message, null), 0))
+    /// ```
+    NoCallbackInPromise,
+    correctness,
+);
+
+impl Rule for NoCallbackInPromise {
+    fn from_configuration(value: serde_json::Value) -> Self {
+        Self(Box::new(NoCallbackInPromiseConfig {
+            exceptions: value
+                .get(0)
+                .and_then(|v| v.get("exceptions"))
+                .and_then(serde_json::Value::as_array)
+                .map(|v| {
+                    v.iter()
+                        .filter_map(serde_json::Value::as_str)
+                        .map(ToString::to_string)
+                        .collect()
+                })
+                .unwrap_or_default(),
+        }))
+    }
+
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        if !self.is_callback(node) {
+            if Self::has_promise_callback(node) {
+                let Some(call_expr) = node.kind().as_call_expression() else {
+                    return;
+                };
+
+                let Some(id) = call_expr.arguments.first().and_then(|arg| {
+                    arg.as_expression().and_then(Expression::get_identifier_reference)
+                }) else {
+                    return;
+                };
+
+                let name = id.name.as_str();
+                if !self.exceptions.iter().any(|exception| exception == name)
+                    && CALLBACKS.contains(&name)
+                {
+                    ctx.diagnostic(no_callback_in_promise_diagnostic(id.span));
+                }
+            }
+        } else if ctx
+            .nodes()
+            .iter_parents(node.id())
+            .skip(1)
+            .any(|node| Self::is_inside_promise(node, ctx))
+        {
+            ctx.diagnostic(no_callback_in_promise_diagnostic(node.span()));
+        }
+    }
+}
+
+impl NoCallbackInPromise {
+    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().iter_parents(node.id()).nth(2).is_some_and(Self::has_promise_callback)
+    }
+
+    fn has_promise_callback(node: &AstNode) -> bool {
+        let AstKind::CallExpression(call_expr) = node.kind() else {
+            return false;
+        };
+
+        matches!(
+            call_expr
+                .callee
+                .as_member_expression()
+                .and_then(MemberExpression::static_property_name),
+            Some("then" | "catch")
+        )
+    }
+
+    fn is_callback(&self, node: &AstNode) -> bool {
+        let AstKind::CallExpression(call_expr) = node.kind() else {
+            return false;
+        };
+
+        call_expr.callee.get_identifier_reference().is_some_and(|id| {
+            let callbacks: Vec<_> = CALLBACKS
+                .iter()
+                .filter(|&&item| !self.exceptions.iter().any(|exception| exception == item))
+                .collect();
+
+            callbacks.contains(&&id.name.as_str())
+        })
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        ("function thing(cb) { cb() }", None),
+        ("doSomething(function(err) { cb(err) })", None),
+        ("function thing(callback) { callback() }", None),
+        ("doSomething(function(err) { callback(err) })", None),
+        ("let thing = (cb) => cb()", None),
+        ("doSomething(err => cb(err))", None),
+        ("a.then(() => next())", Some(serde_json::json!([{ "exceptions": ["next"] }]))),
+        (
+            "a.then(() => next()).catch((err) => next(err))",
+            Some(serde_json::json!([{ "exceptions": ["next"] }])),
+        ),
+        ("a.then(next)", Some(serde_json::json!([{ "exceptions": ["next"] }]))),
+        ("a.then(next).catch(next)", Some(serde_json::json!([{ "exceptions": ["next"] }]))),
+    ];
+
+    let fail = vec![
+        ("a.then(cb)", None),
+        ("a.then(() => cb())", None),
+        ("a.then(function(err) { cb(err) })", None),
+        ("a.then(function(data) { cb(data) }, function(err) { cb(err) })", None),
+        ("a.catch(function(err) { cb(err) })", None),
+        ("a.then(callback)", None),
+        ("a.then(() => callback())", None),
+        ("a.then(function(err) { callback(err) })", None),
+        ("a.then(function(data) { callback(data) }, function(err) { callback(err) })", None),
+        ("a.catch(function(err) { callback(err) })", None),
+    ];
+
+    Tester::new(NoCallbackInPromise::NAME, pass, fail).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/no_callback_in_promise.snap

@@ -0,0 +1,74 @@
+---
+source: crates/oxc_linter/src/tester.rs
+---
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:8]
+ 1 │ a.then(cb)
+   ·        ──
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:14]
+ 1 │ a.then(() => cb())
+   ·              ────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:24]
+ 1 │ a.then(function(err) { cb(err) })
+   ·                        ───────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:25]
+ 1 │ a.then(function(data) { cb(data) }, function(err) { cb(err) })
+   ·                         ────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:53]
+ 1 │ a.then(function(data) { cb(data) }, function(err) { cb(err) })
+   ·                                                     ───────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:25]
+ 1 │ a.catch(function(err) { cb(err) })
+   ·                         ───────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:8]
+ 1 │ a.then(callback)
+   ·        ────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:14]
+ 1 │ a.then(() => callback())
+   ·              ──────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:24]
+ 1 │ a.then(function(err) { callback(err) })
+   ·                        ─────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:25]
+ 1 │ a.then(function(data) { callback(data) }, function(err) { callback(err) })
+   ·                         ──────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:59]
+ 1 │ a.then(function(data) { callback(data) }, function(err) { callback(err) })
+   ·                                                           ─────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(no-callback-in-promise): Avoid calling back inside of a promise
+   ╭─[no_callback_in_promise.tsx:1:25]
+ 1 │ a.catch(function(err) { callback(err) })
+   ·                         ─────────────
+   ╰────