Skip to content

Commit

Permalink
feat(linter/eslint-plugin-promise): implement param-names
Browse files Browse the repository at this point in the history
  • Loading branch information
jelly committed Jul 16, 2024
1 parent 3b55fd7 commit 63947bc
Show file tree
Hide file tree
Showing 3 changed files with 231 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 @@ -435,6 +435,7 @@ mod tree_shaking {
mod promise {
pub mod avoid_new;
pub mod no_new_statics;
pub mod param_names;
}

oxc_macros::declare_all_lint_rules! {
Expand Down Expand Up @@ -829,4 +830,5 @@ oxc_macros::declare_all_lint_rules! {
tree_shaking::no_side_effects_in_initialization,
promise::avoid_new,
promise::no_new_statics,
promise::param_names,
}
179 changes: 179 additions & 0 deletions crates/oxc_linter/src/rules/promise/param_names.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
use oxc_ast::{
ast::{BindingPatternKind, Expression, FormalParameter, FormalParameters},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use regex::Regex;

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

fn param_names_diagnostic(span0: Span, x0: &str) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `{x0}`")).with_label(span0)
}

#[derive(Debug, Default, Clone)]
pub struct ParamNames(Box<ParamNamesConfig>);

#[derive(Debug, Default, Clone)]
pub struct ParamNamesConfig {
resolve_pattern: Option<Regex>,
reject_pattern: Option<Regex>,
}

impl std::ops::Deref for ParamNames {
type Target = ParamNamesConfig;

fn deref(&self) -> &Self::Target {
&self.0
}
}

enum ParamType {
Resolve,
Reject,
}

declare_oxc_lint!(
/// ### What it does
///
/// Enforce standard parameter names for Promise constructors.
///
/// ### Why is this bad?
///
/// Ensures that new Promise() is instantiated with the parameter names resolve, reject to
/// avoid confusion with order such as reject, resolve. The Promise constructor uses the
/// RevealingConstructor pattern. Using the same parameter names as the language specification
/// makes code more uniform and easier to understand.
///
/// ### Example
/// ```javascript
/// new Promise(function (reject, resolve) { ... }) // incorrect order
/// new Promise(function (ok, fail) { ... }) // non-standard parameter names
/// ```
ParamNames,
style,
);

impl Rule for ParamNames {
fn from_configuration(value: serde_json::Value) -> Self {
let mut cfg = ParamNamesConfig::default();

if let Some(config) = value.get(0) {
if let Some(val) = config.get("resolvePattern").and_then(serde_json::Value::as_str) {
cfg.resolve_pattern = Regex::new(val).ok();
}
if let Some(val) = config.get("rejectPattern").and_then(serde_json::Value::as_str) {
cfg.reject_pattern = Regex::new(val).ok();
}
}

Self(Box::new(cfg))
}

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

if !new_expr.callee.is_specific_id("Promise") || new_expr.arguments.len() != 1 {
return;
}

for argument in &new_expr.arguments {
let Some(arg_expr) = argument.as_expression() else {
continue;
};
match arg_expr {
Expression::ArrowFunctionExpression(arrow_expr) => {
self.check_parameter_names(&arrow_expr.params, ctx);
}
Expression::FunctionExpression(func_expr) => {
self.check_parameter_names(&func_expr.params, ctx);
}
_ => continue,
}
}
}
}

impl ParamNames {
fn check_parameter_names(&self, params: &FormalParameters, ctx: &LintContext) {
if params.items.is_empty() {
return;
}

self.check_parameter(&params.items[0], &ParamType::Resolve, ctx);

if params.items.len() > 1 {
self.check_parameter(&params.items[1], &ParamType::Reject, ctx);
}
}

fn check_parameter(&self, param: &FormalParameter, param_type: &ParamType, ctx: &LintContext) {
let BindingPatternKind::BindingIdentifier(param_ident) = &param.pattern.kind else {
return;
};

let param_pattern = if matches!(param_type, ParamType::Reject) {
&self.reject_pattern
} else {
&self.resolve_pattern
};

match param_pattern {
Some(pattern) => {
if !pattern.is_match(param_ident.name.as_str()) {
ctx.diagnostic(param_names_diagnostic(param_ident.span, pattern.as_str()));
}
}
None => {
if matches!(param_type, ParamType::Resolve)
&& !matches!(param_ident.name.as_str(), "_resolve" | "resolve")
{
ctx.diagnostic(param_names_diagnostic(param_ident.span, "^_?resolve$"));
} else if matches!(param_type, ParamType::Reject)
&& !matches!(param_ident.name.as_str(), "_reject" | "reject")
{
ctx.diagnostic(param_names_diagnostic(param_ident.span, "^_?reject$"));
}
}
}
}
}

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

let pass = vec![
("new Promise(function(resolve, reject) {})", None),
("new Promise(function(resolve, _reject) {})", None),
("new Promise(function(_resolve, reject) {})", None),
("new Promise(function(_resolve, _reject) {})", None),
("new Promise(function(resolve) {})", None),
("new Promise(function(_resolve) {})", None),
("new Promise(resolve => {})", None),
("new Promise((resolve, reject) => {})", None),
("new Promise(() => {})", None),
("new NonPromise()", None),
(
"new Promise((yes, no) => {})",
Some(serde_json::json!([{ "resolvePattern": "^yes$", "rejectPattern": "^no$" }])),
),
];

let fail = vec![
("new Promise(function(reject, resolve) {})", None),
("new Promise(function(resolve, rej) {})", None),
("new Promise(yes => {})", None),
("new Promise((yes, no) => {})", None),
(
"new Promise(function(resolve, reject) { config(); })",
Some(serde_json::json!([{ "resolvePattern": "^yes$", "rejectPattern": "^no$" }])),
),
];

Tester::new(ParamNames::NAME, pass, fail).test_and_snapshot();
}
50 changes: 50 additions & 0 deletions crates/oxc_linter/src/snapshots/param_names.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
source: crates/oxc_linter/src/tester.rs
---
eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^_?resolve$`
╭─[param_names.tsx:1:22]
1new Promise(function(reject, resolve) {})
· ──────
╰────

eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^_?reject$`
╭─[param_names.tsx:1:30]
1new Promise(function(reject, resolve) {})
· ───────
╰────

eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^_?reject$`
╭─[param_names.tsx:1:31]
1new Promise(function(resolve, rej) {})
· ───
╰────

eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^_?resolve$`
╭─[param_names.tsx:1:13]
1new Promise(yes => {})
· ───
╰────

eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^_?resolve$`
╭─[param_names.tsx:1:14]
1new Promise((yes, no) => {})
· ───
╰────

eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^_?reject$`
╭─[param_names.tsx:1:19]
1new Promise((yes, no) => {})
· ──
╰────

eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^yes$`
╭─[param_names.tsx:1:22]
1new Promise(function(resolve, reject) { config(); })
· ───────
╰────

eslint-plugin-promise(param-names): Promise constructor parameters must be named to match `^no$`
╭─[param_names.tsx:1:31]
1new Promise(function(resolve, reject) { config(); })
· ──────
╰────

0 comments on commit 63947bc

Please sign in to comment.