Skip to content

Commit

Permalink
feat(linter/eslint-plugin-promise): implement valid-params (#4598)
Browse files Browse the repository at this point in the history
  • Loading branch information
jelly committed Aug 9, 2024
1 parent b259f47 commit c3c5766
Show file tree
Hide file tree
Showing 6 changed files with 396 additions and 7 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 2 additions & 6 deletions crates/oxc_linter/src/rules/promise/no_new_statics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
189 changes: 189 additions & 0 deletions crates/oxc_linter/src/rules/promise/valid_params.rs
Original file line number Diff line number Diff line change
@@ -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();
}
170 changes: 170 additions & 0 deletions crates/oxc_linter/src/snapshots/valid_params.snap
Original file line number Diff line number Diff line change
@@ -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]
1Promise.resolve(1, 2)
· ─────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.resolve() requires 0 or 1 arguments, but received 5
╭─[valid_params.tsx:1:1]
1Promise.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]
1Promise.reject(1, 2, 3)
· ───────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.reject() requires 0 or 1 arguments, but received 4
╭─[valid_params.tsx:1:1]
1Promise.reject({}, function() {}, 1, 2)
· ───────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.race() requires 1 argument, but received 2
╭─[valid_params.tsx:1:1]
1Promise.race(1, 2)
· ──────────────────
╰────

eslint-plugin-promise(valid-params): Promise.race() requires 1 argument, but received 5
╭─[valid_params.tsx:1:1]
1Promise.race({}, function() {}, 1, 2, 3)
· ────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.all() requires 1 argument, but received 3
╭─[valid_params.tsx:1:1]
1Promise.all(1, 2, 3)
· ────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.all() requires 1 argument, but received 4
╭─[valid_params.tsx:1:1]
1Promise.all({}, function() {}, 1, 2)
· ────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.allSettled() requires 1 argument, but received 3
╭─[valid_params.tsx:1:1]
1Promise.allSettled(1, 2, 3)
· ───────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.allSettled() requires 1 argument, but received 4
╭─[valid_params.tsx:1:1]
1Promise.allSettled({}, function() {}, 1, 2)
· ───────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.any() requires 1 argument, but received 3
╭─[valid_params.tsx:1:1]
1Promise.any(1, 2, 3)
· ────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.any() requires 1 argument, but received 4
╭─[valid_params.tsx:1:1]
1Promise.any({}, function() {}, 1, 2)
· ────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
╭─[valid_params.tsx:1:1]
1somePromise().then()
· ────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
╭─[valid_params.tsx:1:1]
1somePromise().then()
· ────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
╭─[valid_params.tsx:1:1]
1somePromise().then(() => {}, () => {}, () => {})
· ────────────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
╭─[valid_params.tsx:1:1]
1somePromise().then(() => {}, () => {}, () => {})
· ────────────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
╭─[valid_params.tsx:1:1]
1promiseReference.then()
· ───────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 0
╭─[valid_params.tsx:1:1]
1promiseReference.then()
· ───────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
╭─[valid_params.tsx:1:1]
1promiseReference.then(() => {}, () => {}, () => {})
· ───────────────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.then() requires 1 or 2 arguments, but received 3
╭─[valid_params.tsx:1:1]
1promiseReference.then(() => {}, () => {}, () => {})
· ───────────────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 0
╭─[valid_params.tsx:1:1]
1somePromise().catch()
· ─────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 2
╭─[valid_params.tsx:1:1]
1somePromise().catch(() => {}, () => {})
· ───────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 0
╭─[valid_params.tsx:1:1]
1promiseReference.catch()
· ────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.catch() requires 1 argument, but received 2
╭─[valid_params.tsx:1:1]
1promiseReference.catch(() => {}, () => {})
· ──────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 0
╭─[valid_params.tsx:1:1]
1somePromise().finally()
· ───────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 2
╭─[valid_params.tsx:1:1]
1somePromise().finally(() => {}, () => {})
· ─────────────────────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 0
╭─[valid_params.tsx:1:1]
1promiseReference.finally()
· ──────────────────────────
╰────

eslint-plugin-promise(valid-params): Promise.finally() requires 1 argument, but received 2
╭─[valid_params.tsx:1:1]
1promiseReference.finally(() => {}, () => {})
· ────────────────────────────────────────────
╰────
Loading

0 comments on commit c3c5766

Please sign in to comment.