Skip to content

Commit

Permalink
feat(linter): Add eslint-plugin-promise rules: avoid-new, no-new-stat…
Browse files Browse the repository at this point in the history
…ics, params-names (#4293)

This introduces the `eslint-plugin-promise` plugin and implements three
relatively simple rules.

Split off from #4252
  • Loading branch information
jelly authored Jul 17, 2024
1 parent fc0b17d commit 1f8968a
Show file tree
Hide file tree
Showing 15 changed files with 512 additions and 1 deletion.
4 changes: 4 additions & 0 deletions apps/oxlint/src/command/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ pub struct EnablePlugins {
/// Enable the React performance plugin and detect rendering performance problems
#[bpaf(switch, hide_usage)]
pub react_perf_plugin: bool,

/// Enable the promise plugin and detect promise usage problems
#[bpaf(switch, hide_usage)]
pub promise_plugin: bool,
}

#[cfg(test)]
Expand Down
3 changes: 2 additions & 1 deletion apps/oxlint/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ impl Runner for LintRunner {
.with_vitest_plugin(enable_plugins.vitest_plugin)
.with_jsx_a11y_plugin(enable_plugins.jsx_a11y_plugin)
.with_nextjs_plugin(enable_plugins.nextjs_plugin)
.with_react_perf_plugin(enable_plugins.react_perf_plugin);
.with_react_perf_plugin(enable_plugins.react_perf_plugin)
.with_promise_plugin(enable_plugins.promise_plugin);

let linter = match Linter::from_options(lint_options) {
Ok(lint_service) => lint_service,
Expand Down
9 changes: 9 additions & 0 deletions crates/oxc_linter/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub struct LintOptions {
pub jsx_a11y_plugin: bool,
pub nextjs_plugin: bool,
pub react_perf_plugin: bool,
pub promise_plugin: bool,
}

impl Default for LintOptions {
Expand All @@ -48,6 +49,7 @@ impl Default for LintOptions {
jsx_a11y_plugin: false,
nextjs_plugin: false,
react_perf_plugin: false,
promise_plugin: false,
}
}
}
Expand Down Expand Up @@ -138,6 +140,12 @@ impl LintOptions {
self.react_perf_plugin = yes;
self
}

#[must_use]
pub fn with_promise_plugin(mut self, yes: bool) -> Self {
self.promise_plugin = yes;
self
}
}

#[derive(Debug, Clone, Copy, Eq, PartialEq)]
Expand Down Expand Up @@ -342,6 +350,7 @@ impl LintOptions {
"react_perf" => self.react_perf_plugin,
"oxc" => self.oxc_plugin,
"eslint" | "tree_shaking" => true,
"promise" => self.promise_plugin,
name => panic!("Unhandled plugin: {name}"),
})
.cloned()
Expand Down
9 changes: 9 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,12 @@ mod tree_shaking {
pub mod no_side_effects_in_initialization;
}

mod promise {
pub mod avoid_new;
pub mod no_new_statics;
pub mod param_names;
}

oxc_macros::declare_all_lint_rules! {
eslint::array_callback_return,
eslint::constructor_super,
Expand Down Expand Up @@ -822,4 +828,7 @@ oxc_macros::declare_all_lint_rules! {
jsdoc::require_returns_type,
jsdoc::require_yields,
tree_shaking::no_side_effects_in_initialization,
promise::avoid_new,
promise::no_new_statics,
promise::param_names,
}
69 changes: 69 additions & 0 deletions crates/oxc_linter/src/rules/promise/avoid_new.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

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

fn avoid_new_promise_diagnostic(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("eslint-plugin-promise(avoid-new): Avoid creating new promises")
.with_label(span0)
}

#[derive(Debug, Default, Clone)]
pub struct AvoidNew;

declare_oxc_lint!(
/// ### What it does
///
/// Disallow creating new promises outside of utility libs.
///
/// ### Why is this bad?
///
/// If you dislike the new promise style promises.
///
/// ### Example
/// ```javascript
/// new Promise((resolve, reject) => { ... });
/// ```
AvoidNew,
restriction,
);

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

let Expression::Identifier(ident) = &expr.callee else {
return;
};

if ident.name == "Promise" && ctx.semantic().is_reference_to_global_variable(ident) {
ctx.diagnostic(avoid_new_promise_diagnostic(expr.span));
}
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
"Promise.resolve()",
"Promise.reject()",
"Promise.all()",
"new Horse()",
"new PromiseLikeThing()",
"new Promise.resolve()",
];

let fail = vec![
"var x = new Promise(function (x, y) {})",
"new Promise()",
"Thing(new Promise(() => {}))",
];

Tester::new(AvoidNew::NAME, pass, fail).test_and_snapshot();
}
110 changes: 110 additions & 0 deletions crates/oxc_linter/src/rules/promise/no_new_statics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

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

fn static_promise_diagnostic(x0: &str, span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!(
"eslint-plugin-promise(no-new-statics): Disallow calling `new` on a `Promise.{x0}`"
))
.with_label(span0)
}

#[derive(Debug, Default, Clone)]
pub struct NoNewStatics;

declare_oxc_lint!(
/// ### What it does
///
/// Disallow calling new on a Promise static method.
///
/// ### Why is this bad?
///
/// Calling a Promise static method with new is invalid, resulting in a TypeError at runtime.
///
/// ### Example
/// ```javascript
/// new Promise.resolve(value);
/// ```
NoNewStatics,
correctness
);

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

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

let Expression::Identifier(ident) = &member_expr.object() else {
return;
};

if ident.name != "Promise" || !ctx.semantic().is_reference_to_global_variable(ident) {
return;
}

let Some(prop_name) = member_expr.static_property_name() else {
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"
) {
ctx.diagnostic_with_fix(
static_promise_diagnostic(
prop_name,
Span::new(new_expr.span.start, ident.span.start - 1),
),
|fixer| fixer.delete_range(Span::new(new_expr.span.start, ident.span.start)),
);
}
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
"Promise.resolve()",
"Promise.reject()",
"Promise.all()",
"Promise.race()",
"new Promise(function (resolve, reject) {})",
"new SomeClass()",
"SomeClass.resolve()",
"new SomeClass.resolve()",
];

let fail = vec![
"new Promise.resolve()",
"new Promise.reject()",
"new Promise.all()",
"new Promise.allSettled()",
"new Promise.any()",
"new Promise.race()",
"function foo() {
var a = getA()
return new Promise.resolve(a)
}",
];

let fix = vec![
("new Promise.resolve()", "Promise.resolve()", None),
("new Promise.reject()", "Promise.reject()", None),
("new Promise.all()", "Promise.all()", None),
("new Promise.allSettled()", "Promise.allSettled()", None),
("new Promise.any()", "Promise.any()", None),
("new Promise.race()", "Promise.race()", None),
];
Tester::new(NoNewStatics::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
}
Loading

0 comments on commit 1f8968a

Please sign in to comment.