Skip to content

Commit

Permalink
feat(linter/eslint-plugin-promise): implement valid-params
Browse files Browse the repository at this point in the history
  • Loading branch information
jelly committed Aug 1, 2024
1 parent e0b03f8 commit 49495ee
Show file tree
Hide file tree
Showing 3 changed files with 325 additions and 0 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 @@ -443,6 +443,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 @@ -849,5 +850,6 @@ oxc_macros::declare_all_lint_rules! {
promise::avoid_new,
promise::no_new_statics,
promise::param_names,
promise::valid_params,
vitest::no_import_node_test,
}
177 changes: 177 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,177 @@
use oxc_ast::{ast::CallExpression, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};

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,
);

fn is_promise(call_expr: &CallExpression) -> bool {
let Some(member_expr) = call_expr.callee.get_member_expr() else {
return false;
};

let Some(prop_name) = member_expr.static_property_name() else {
return false;
};

// hello.then(), hello.catch(), hello.finally()
if matches!(prop_name, "then" | "catch" | "finally") {
return true;
}

member_expr.object().is_specific_id("Promise")
&& matches!(
prop_name,
"resolve" | "reject" | "all" | "allSettled" | "race" | "any" | "withResolvers"
)
}

impl Rule for ValidParams {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};

if !is_promise(call_expr) {
return;
}

let Some(member_expr) = call_expr.callee.get_member_expr() else {
return;
};

let Some(prop_name) = member_expr.static_property_name() else {
return;
};

let args_len = call_expr.arguments.len();

match prop_name {
"resolve" | "reject" => {
if args_len > 1 {
ctx.diagnostic(valid_params_diagnostic(call_expr.span, &format!("Promise.{prop_name}() requires 0 or 1 arguments, but received {args_len}")));
}
}
"then" => {
if !(1..=2).contains(&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(valid_params_diagnostic(
call_expr.span,
&format!(
"Promise.{prop_name}() requires 1 argument, but received {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();
}
146 changes: 146 additions & 0 deletions crates/oxc_linter/src/snapshots/valid_params.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
---
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 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 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(() => {}, () => {})
· ────────────────────────────────────────────
╰────

0 comments on commit 49495ee

Please sign in to comment.