feat(linter/eslint-plugin-promise): implement valid-params (#4598)

crates/oxc_linter/src/rules.rs

@@ -445,6 +445,7 @@ mod promise {
     pub mod avoid_new;
     pub mod no_new_statics;
     pub mod param_names;
+    pub mod valid_params;
 }
 
 mod vitest {
@@ -855,6 +856,7 @@ oxc_macros::declare_all_lint_rules! {
     promise::avoid_new,
     promise::no_new_statics,
     promise::param_names,
+    promise::valid_params,
     vitest::no_import_node_test,
     vitest::prefer_to_be_falsy,
     vitest::prefer_to_be_truthy,

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

@@ -3,7 +3,7 @@ use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_span::Span;
 
-use crate::{context::LintContext, rule::Rule, AstNode};
+use crate::{context::LintContext, rule::Rule, utils::PROMISE_STATIC_METHODS, AstNode};
 
 fn static_promise_diagnostic(x0: &str, span0: Span) -> OxcDiagnostic {
     OxcDiagnostic::warn(format!("Disallow calling `new` on a `Promise.{x0}`")).with_label(span0)
@@ -52,11 +52,7 @@ impl Rule for NoNewStatics {
             return;
         };
 
-        // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise
-        if matches!(
-            prop_name,
-            "resolve" | "reject" | "all" | "allSettled" | "race" | "any" | "withResolvers"
-        ) {
+        if PROMISE_STATIC_METHODS.contains(prop_name) {
             ctx.diagnostic_with_fix(
                 static_promise_diagnostic(
                     prop_name,

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

@@ -0,0 +1,189 @@
+use oxc_ast::AstKind;
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_span::Span;
+
+use crate::{context::LintContext, rule::Rule, utils::is_promise, AstNode};
+
+fn zero_or_one_argument_required_diagnostic(
+    span0: Span,
+    prop_name: &str,
+    args_len: usize,
+) -> OxcDiagnostic {
+    OxcDiagnostic::warn(format!(
+        "Promise.{prop_name}() requires 0 or 1 arguments, but received {args_len}"
+    ))
+    .with_label(span0)
+}
+
+fn one_or_two_argument_required_diagnostic(
+    span0: Span,
+    prop_name: &str,
+    args_len: usize,
+) -> OxcDiagnostic {
+    OxcDiagnostic::warn(format!(
+        "Promise.{prop_name}() requires 1 or 2 arguments, but received {args_len}"
+    ))
+    .with_label(span0)
+}
+
+fn one_argument_required_diagnostic(
+    span0: Span,
+    prop_name: &str,
+    args_len: usize,
+) -> OxcDiagnostic {
+    OxcDiagnostic::warn(format!(
+        "Promise.{prop_name}() requires 1 argument, but received {args_len}"
+    ))
+    .with_label(span0)
+}
+
+fn valid_params_diagnostic(span0: Span, x0: &str) -> OxcDiagnostic {
+    OxcDiagnostic::warn(x0.to_string()).with_label(span0)
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct ValidParams;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Enforces the proper number of arguments are passed to Promise functions.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Calling a Promise function with the incorrect number of arguments can lead to unexpected
+    /// behavior or hard to spot bugs.
+    ///
+    /// ### Example
+    /// ```javascript
+    /// Promise.resolve(1, 2)
+    /// ```
+    ValidParams,
+    correctness,
+);
+
+impl Rule for ValidParams {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        let AstKind::CallExpression(call_expr) = node.kind() else {
+            return;
+        };
+
+        let Some(prop_name) = is_promise(call_expr) else {
+            return;
+        };
+
+        let args_len = call_expr.arguments.len();
+
+        match prop_name.as_str() {
+            "resolve" | "reject" => {
+                if args_len > 1 {
+                    ctx.diagnostic(zero_or_one_argument_required_diagnostic(
+                        call_expr.span,
+                        &prop_name,
+                        args_len,
+                    ));
+                }
+            }
+            "then" => {
+                if args_len != 1 && args_len != 2 {
+                    ctx.diagnostic(one_or_two_argument_required_diagnostic(
+                        call_expr.span,
+                        &prop_name,
+                        args_len,
+                    ));
+                    ctx.diagnostic(valid_params_diagnostic(call_expr.span, &format!("Promise.{prop_name}() requires 1 or 2 arguments, but received {args_len}")));
+                }
+            }
+            "race" | "all" | "allSettled" | "any" | "catch" | "finally" => {
+                if args_len != 1 {
+                    ctx.diagnostic(one_argument_required_diagnostic(
+                        call_expr.span,
+                        &prop_name,
+                        args_len,
+                    ));
+                }
+            }
+            _ => {}
+        }
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        "Promise.resolve()",
+        "Promise.resolve(1)",
+        "Promise.resolve({})",
+        "Promise.resolve(referenceToSomething)",
+        "Promise.reject()",
+        "Promise.reject(1)",
+        "Promise.reject({})",
+        "Promise.reject(referenceToSomething)",
+        "Promise.reject(Error())",
+        "Promise.race([])",
+        "Promise.race(iterable)",
+        "Promise.race([one, two, three])",
+        "Promise.all([])",
+        "Promise.all(iterable)",
+        "Promise.all([one, two, three])",
+        "Promise.allSettled([])",
+        "Promise.allSettled(iterable)",
+        "Promise.allSettled([one, two, three])",
+        "Promise.any([])",
+        "Promise.any(iterable)",
+        "Promise.any([one, two, three])",
+        "somePromise().then(success)",
+        "somePromise().then(success, failure)",
+        "promiseReference.then(() => {})",
+        "promiseReference.then(() => {}, () => {})",
+        "somePromise().catch(callback)",
+        "somePromise().catch(err => {})",
+        "promiseReference.catch(callback)",
+        "promiseReference.catch(err => {})",
+        "somePromise().finally(callback)",
+        "somePromise().finally(() => {})",
+        "promiseReference.finally(callback)",
+        "promiseReference.finally(() => {})",
+        "Promise.all([
+			  Promise.resolve(1),
+			  Promise.resolve(2),
+			  Promise.reject(Error()),
+			])
+			  .then(console.log)
+			  .catch(console.error)
+			  .finally(console.log)
+			",
+    ];
+
+    let fail = vec![
+        "Promise.resolve(1, 2)",
+        "Promise.resolve({}, function() {}, 1, 2, 3)",
+        "Promise.reject(1, 2, 3)",
+        "Promise.reject({}, function() {}, 1, 2)",
+        "Promise.race(1, 2)",
+        "Promise.race({}, function() {}, 1, 2, 3)",
+        "Promise.all(1, 2, 3)",
+        "Promise.all({}, function() {}, 1, 2)",
+        "Promise.allSettled(1, 2, 3)",
+        "Promise.allSettled({}, function() {}, 1, 2)",
+        "Promise.any(1, 2, 3)",
+        "Promise.any({}, function() {}, 1, 2)",
+        "somePromise().then()",
+        "somePromise().then(() => {}, () => {}, () => {})",
+        "promiseReference.then()",
+        "promiseReference.then(() => {}, () => {}, () => {})",
+        "somePromise().catch()",
+        "somePromise().catch(() => {}, () => {})",
+        "promiseReference.catch()",
+        "promiseReference.catch(() => {}, () => {})",
+        "somePromise().finally()",
+        "somePromise().finally(() => {}, () => {})",
+        "promiseReference.finally()",
+        "promiseReference.finally(() => {}, () => {})",
+    ];
+
+    Tester::new(ValidParams::NAME, pass, fail).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/valid_params.snap

@@ -0,0 +1,170 @@
+---
+source: crates/oxc_linter/src/tester.rs
+---
+  ⚠ eslint-plugin-promise(valid-params): Promise.resolve() requires 0 or 1 arguments, but received 2
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.resolve(1, 2)
+   · ─────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.resolve() requires 0 or 1 arguments, but received 5
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.resolve({}, function() {}, 1, 2, 3)
+   · ───────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.reject() requires 0 or 1 arguments, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.reject(1, 2, 3)
+   · ───────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.reject() requires 0 or 1 arguments, but received 4
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.reject({}, function() {}, 1, 2)
+   · ───────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.race() requires 1 argument, but received 2
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.race(1, 2)
+   · ──────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.race() requires 1 argument, but received 5
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.race({}, function() {}, 1, 2, 3)
+   · ────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.all() requires 1 argument, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.all(1, 2, 3)
+   · ────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.all() requires 1 argument, but received 4
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.all({}, function() {}, 1, 2)
+   · ────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.allSettled() requires 1 argument, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.allSettled(1, 2, 3)
+   · ───────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.allSettled() requires 1 argument, but received 4
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.allSettled({}, function() {}, 1, 2)
+   · ───────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.any() requires 1 argument, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.any(1, 2, 3)
+   · ────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.any() requires 1 argument, but received 4
+   ╭─[valid_params.tsx:1:1]
+ 1 │ Promise.any({}, function() {}, 1, 2)
+   · ────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().then()
+   · ────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().then()
+   · ────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().then(() => {}, () => {}, () => {})
+   · ────────────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().then(() => {}, () => {}, () => {})
+   · ────────────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.then()
+   · ───────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.then()
+   · ───────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.then(() => {}, () => {}, () => {})
+   · ───────────────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.then(() => {}, () => {}, () => {})
+   · ───────────────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().catch()
+   · ─────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 2
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().catch(() => {}, () => {})
+   · ───────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.catch()
+   · ────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 2
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.catch(() => {}, () => {})
+   · ──────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().finally()
+   · ───────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 2
+   ╭─[valid_params.tsx:1:1]
+ 1 │ somePromise().finally(() => {}, () => {})
+   · ─────────────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 0
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.finally()
+   · ──────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 2
+   ╭─[valid_params.tsx:1:1]
+ 1 │ promiseReference.finally(() => {}, () => {})
+   · ────────────────────────────────────────────
+   ╰────

crates/oxc_linter/src/utils/mod.rs

@@ -1,6 +1,7 @@
 mod jest;
 mod jsdoc;
 mod nextjs;
+mod promise;
 mod react;
 mod react_perf;
 mod tree_shaking;
@@ -8,7 +9,8 @@ mod unicorn;
 mod vitest;
 
 pub use self::{
-    jest::*, jsdoc::*, nextjs::*, react::*, react_perf::*, tree_shaking::*, unicorn::*, vitest::*,
+    jest::*, jsdoc::*, nextjs::*, promise::*, react::*, react_perf::*, tree_shaking::*, unicorn::*,
+    vitest::*,
 };
 
 /// Check if the Jest rule is adapted to Vitest.

crates/oxc_linter/src/utils/promise.rs

@@ -0,0 +1,30 @@
+use oxc_ast::ast::CallExpression;
+use phf::{phf_set, Set};
+
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise
+pub const PROMISE_STATIC_METHODS: Set<&'static str> = phf_set! {
+    "resolve",
+    "reject",
+    "all",
+    "allSettled",
+    "race",
+    "any",
+    "withResolvers",
+};
+
+pub fn is_promise(call_expr: &CallExpression) -> Option<String> {
+    let member_expr = call_expr.callee.get_member_expr()?;
+    let prop_name = member_expr.static_property_name()?;
+
+    // hello.then(), hello.catch(), hello.finally()
+    if matches!(prop_name, "then" | "catch" | "finally") {
+        return Some(prop_name.into());
+    }
+
+    if member_expr.object().is_specific_id("Promise") && PROMISE_STATIC_METHODS.contains(prop_name)
+    {
+        return Some(prop_name.into());
+    }
+
+    None
+}