From e5c755a7a6eb7ef1b48fd98e670803daf9cf0086 Mon Sep 17 00:00:00 2001 From: Jelle van der Waa Date: Sat, 31 Aug 2024 17:38:47 +0200 Subject: [PATCH] feat(linter/promise): add `spec-only` rule (#5124) Rule Detail: [link](https://github.com/eslint-community/eslint-plugin-promise/blob/main/docs/rules/spec-only.md) Co-authored-by: Don Isaac --- crates/oxc_linter/src/rules.rs | 2 + .../oxc_linter/src/rules/promise/spec_only.rs | 135 ++++++++++++++++++ .../oxc_linter/src/snapshots/spec_only.snap | 36 +++++ 3 files changed, 173 insertions(+) create mode 100644 crates/oxc_linter/src/rules/promise/spec_only.rs create mode 100644 crates/oxc_linter/src/snapshots/spec_only.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index a33ff7a39de79..8c3aef2e2ae36 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -454,6 +454,7 @@ mod promise { pub mod no_return_in_finally; pub mod param_names; pub mod prefer_await_to_then; + pub mod spec_only; pub mod valid_params; } @@ -878,6 +879,7 @@ oxc_macros::declare_all_lint_rules! { promise::valid_params, promise::no_return_in_finally, promise::prefer_await_to_then, + promise::spec_only, vitest::no_import_node_test, vitest::prefer_each, vitest::prefer_to_be_falsy, diff --git a/crates/oxc_linter/src/rules/promise/spec_only.rs b/crates/oxc_linter/src/rules/promise/spec_only.rs new file mode 100644 index 0000000000000..0192075f0315d --- /dev/null +++ b/crates/oxc_linter/src/rules/promise/spec_only.rs @@ -0,0 +1,135 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::GetSpan; +use rustc_hash::FxHashSet; + +use crate::{context::LintContext, rule::Rule, utils::PROMISE_STATIC_METHODS, AstNode}; + +#[derive(Debug, Default, Clone)] +pub struct SpecOnly(Box); + +#[derive(Debug, Default, Clone)] +pub struct SpecOnlyConfig { + allowed_methods: Option>, +} + +impl std::ops::Deref for SpecOnly { + type Target = SpecOnlyConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow use of non-standard Promise static methods. + /// + /// ### Why is this bad? + /// + /// Non-standard Promises may cost more maintenance work. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// Promise.done() + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// Promise.resolve() + /// ``` + SpecOnly, + restriction, +); + +impl Rule for SpecOnly { + fn from_configuration(value: serde_json::Value) -> Self { + let allowed_methods = value + .get(0) + .and_then(|v| v.get("allowedMethods")) + .and_then(serde_json::Value::as_array) + .map(|v| { + v.iter().filter_map(serde_json::Value::as_str).map(ToString::to_string).collect() + }); + + Self(Box::new(SpecOnlyConfig { allowed_methods })) + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::MemberExpression(member_expr) = node.kind() else { + return; + }; + + if !member_expr.object().is_specific_id("Promise") { + return; + } + + let Some(prop_name) = member_expr.static_property_name() else { + return; + }; + + if PROMISE_STATIC_METHODS.contains(prop_name) { + return; + } + + if let Some(allowed_methods) = &self.allowed_methods { + if allowed_methods.contains(prop_name) { + return; + } + } + + ctx.diagnostic( + OxcDiagnostic::warn(format!("Avoid using non-standard `Promise.{prop_name}`")) + .with_label(member_expr.span()), + ); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("Promise.resolve()", None), + ("Promise.reject()", None), + ("Promise.all()", None), + ("Promise.race()", None), + ("Promise.withResolvers()", None), + ("new Promise(function (resolve, reject) {})", None), + ("SomeClass.resolve()", None), + ("doSomething(Promise.all)", None), + ( + "Promise.permittedMethod()", + Some(serde_json::json!([ { "allowedMethods": ["permittedMethod"], }, ])), + ), + ]; + + let fail = vec![ + ("Promise.done()", None), + ("Promise.something()", None), + ("new Promise.done()", None), + ( + " + function foo() { + var a = getA() + return Promise.done(a) + } + ", + None, + ), + ( + " + function foo() { + getA(Promise.done) + } + ", + None, + ), + ]; + + Tester::new(SpecOnly::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/spec_only.snap b/crates/oxc_linter/src/snapshots/spec_only.snap new file mode 100644 index 0000000000000..db7a3e002482f --- /dev/null +++ b/crates/oxc_linter/src/snapshots/spec_only.snap @@ -0,0 +1,36 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-promise(spec-only): Avoid using non-standard `Promise.done` + ╭─[spec_only.tsx:1:1] + 1 │ Promise.done() + · ──────────── + ╰──── + + ⚠ eslint-plugin-promise(spec-only): Avoid using non-standard `Promise.something` + ╭─[spec_only.tsx:1:1] + 1 │ Promise.something() + · ───────────────── + ╰──── + + ⚠ eslint-plugin-promise(spec-only): Avoid using non-standard `Promise.done` + ╭─[spec_only.tsx:1:5] + 1 │ new Promise.done() + · ──────────── + ╰──── + + ⚠ eslint-plugin-promise(spec-only): Avoid using non-standard `Promise.done` + ╭─[spec_only.tsx:4:22] + 3 │ var a = getA() + 4 │ return Promise.done(a) + · ──────────── + 5 │ } + ╰──── + + ⚠ eslint-plugin-promise(spec-only): Avoid using non-standard `Promise.done` + ╭─[spec_only.tsx:3:20] + 2 │ function foo() { + 3 │ getA(Promise.done) + · ──────────── + 4 │ } + ╰────