From 896b27c4b539acd15599b85690f089007d2dc334 Mon Sep 17 00:00:00 2001 From: "j.buendia" Date: Sun, 17 Mar 2024 20:04:02 +0100 Subject: [PATCH 1/3] feat(linter): add eslint/max-params --- crates/oxc_linter/src/rules.rs | 2 + .../oxc_linter/src/rules/eslint/max_params.rs | 143 ++++++++++++++++++ .../oxc_linter/src/snapshots/max_params.snap | 74 +++++++++ 3 files changed, 219 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/max_params.rs create mode 100644 crates/oxc_linter/src/snapshots/max_params.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index f7d2b63bd66c..c21314b49c44 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -44,6 +44,7 @@ mod eslint { pub mod for_direction; pub mod getter_return; pub mod max_lines; + pub mod max_params; pub mod no_array_constructor; pub mod no_async_promise_executor; pub mod no_bitwise; @@ -361,6 +362,7 @@ oxc_macros::declare_all_lint_rules! { eslint::for_direction, eslint::getter_return, eslint::max_lines, + eslint::max_params, eslint::no_ternary, eslint::no_this_before_super, eslint::no_array_constructor, diff --git a/crates/oxc_linter/src/rules/eslint/max_params.rs b/crates/oxc_linter/src/rules/eslint/max_params.rs new file mode 100644 index 000000000000..4f73b3568ea4 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/max_params.rs @@ -0,0 +1,143 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::{CompactStr, Span}; +use serde_json::Value; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(max-params): {0:?}")] +#[diagnostic( + severity(warning), + help("This rule enforces a maximum number of parameters allowed in function definitions.") +)] +struct MaxParamsDiagnostic(CompactStr, #[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct MaxParams(Box); + +#[derive(Debug, Clone)] +pub struct MaxParamsConfig { + max: usize, +} + +impl std::ops::Deref for MaxParams { + type Target = MaxParamsConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl Default for MaxParamsConfig { + fn default() -> Self { + Self { max: 3 } + } +} + +declare_oxc_lint!( + /// ### What it does + /// Enforce a maximum number of parameters in function definitions + /// + /// ### Why is this bad? + /// Functions that take numerous parameters can be difficult to read and write because it requires the memorization of what each parameter is, its type, and the order they should appear in. As a result, many coders adhere to a convention that caps the number of parameters a function can take. + /// + /// ### Example + /// ```javascript + /// function foo (bar, baz, qux, qxx) { + /// doSomething(); + /// } + /// ``` + MaxParams, + style +); + +impl Rule for MaxParams { + fn from_configuration(value: Value) -> Self { + let config = value.get(0); + if let Some(max) = config + .and_then(Value::as_number) + .and_then(serde_json::Number::as_u64) + .and_then(|v| usize::try_from(v).ok()) + { + Self(Box::new(MaxParamsConfig { max })) + } else { + let max = config + .and_then(|config| config.get("max")) + .and_then(Value::as_number) + .and_then(serde_json::Number::as_u64) + .map_or(3, |v| usize::try_from(v).unwrap_or(3)); + + Self(Box::new(MaxParamsConfig { max })) + } + } + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::Function(function) => { + if !function.is_declaration() & !function.is_expression() { + return; + } + if function.params.items.len() > self.max { + let error_msg = format!( + "Function has too many parameters ({}). Maximum allowed is {}.", + function.params.items.len(), + self.max + ); + let error = CompactStr::from(error_msg); + let span = function.params.span; + ctx.diagnostic(MaxParamsDiagnostic(error, Span::new(span.start, span.end))); + } + } + AstKind::ArrowFunctionExpression(function) => { + if function.params.items.len() > self.max { + let error_msg = format!( + "Function has too many parameters ({}). Maximum allowed is {}.", + function.params.items.len(), + self.max + ); + let error = CompactStr::from(error_msg); + let span = function.params.span; + ctx.diagnostic(MaxParamsDiagnostic(error, Span::new(span.start, span.end))); + } + } + _ => {} + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("function test(d, e, f) {}", None), + ("var test = function(a, b, c) {};", Some(serde_json::json!([3]))), + ("var test = (a, b, c) => {};", Some(serde_json::json!([3]))), + ("var test = function test(a, b, c) {};", Some(serde_json::json!([3]))), + ("var test = function(a, b, c) {};", Some(serde_json::json!([{ "max": 3 }]))), + ]; + + let fail = vec![ + ("function test(a, b, c) {}", Some(serde_json::json!([2]))), + ("function test(a, b, c, d) {}", None), + ("var test = function(a, b, c, d) {};", Some(serde_json::json!([3]))), + ("var test = (a, b, c, d) => {};", Some(serde_json::json!([3]))), + ("(function(a, b, c, d) {});", Some(serde_json::json!([3]))), + ("var test = function test(a, b, c) {};", Some(serde_json::json!([1]))), + ("function test(a, b, c) {}", Some(serde_json::json!([{ "max": 2 }]))), + ("function test(a, b, c, d) {}", Some(serde_json::json!([{}]))), + ("function test(a) {}", Some(serde_json::json!([{ "max": 0 }]))), + ( + "function test(a, b, c) { + // Just to make it longer + }", + Some(serde_json::json!([{ "max": 2 }])), + ), + ]; + + Tester::new(MaxParams::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/max_params.snap b/crates/oxc_linter/src/snapshots/max_params.snap new file mode 100644 index 000000000000..d06f0a384da5 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/max_params.snap @@ -0,0 +1,74 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: max_params +--- + ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 2." + ╭─[max_params.tsx:1:14] + 1 │ function test(a, b, c) {} + · ───────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ╭─[max_params.tsx:1:14] + 1 │ function test(a, b, c, d) {} + · ──────────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ╭─[max_params.tsx:1:20] + 1 │ var test = function(a, b, c, d) {}; + · ──────────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ╭─[max_params.tsx:1:12] + 1 │ var test = (a, b, c, d) => {}; + · ──────────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ╭─[max_params.tsx:1:10] + 1 │ (function(a, b, c, d) {}); + · ──────────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 1." + ╭─[max_params.tsx:1:25] + 1 │ var test = function test(a, b, c) {}; + · ───────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 2." + ╭─[max_params.tsx:1:14] + 1 │ function test(a, b, c) {} + · ───────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ╭─[max_params.tsx:1:14] + 1 │ function test(a, b, c, d) {} + · ──────────── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (1). Maximum allowed is 0." + ╭─[max_params.tsx:1:14] + 1 │ function test(a) {} + · ─── + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. + + ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 2." + ╭─[max_params.tsx:1:14] + 1 │ function test(a, b, c) { + · ───────── + 2 │ // Just to make it longer + ╰──── + help: This rule enforces a maximum number of parameters allowed in function definitions. From e85524636cabdcbba52941c44fb19ee9c64e8c69 Mon Sep 17 00:00:00 2001 From: "j.buendia" Date: Mon, 18 Mar 2024 16:28:17 +0100 Subject: [PATCH 2/3] feat(linter): improve snapshots --- .../oxc_linter/src/rules/eslint/max_params.rs | 34 ++++++++++++++----- .../oxc_linter/src/snapshots/max_params.snap | 16 ++++----- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/max_params.rs b/crates/oxc_linter/src/rules/eslint/max_params.rs index 4f73b3568ea4..0fac7db49496 100644 --- a/crates/oxc_linter/src/rules/eslint/max_params.rs +++ b/crates/oxc_linter/src/rules/eslint/max_params.rs @@ -81,21 +81,37 @@ impl Rule for MaxParams { if !function.is_declaration() & !function.is_expression() { return; } + if function.params.items.len() > self.max { - let error_msg = format!( - "Function has too many parameters ({}). Maximum allowed is {}.", - function.params.items.len(), - self.max - ); - let error = CompactStr::from(error_msg); - let span = function.params.span; - ctx.diagnostic(MaxParamsDiagnostic(error, Span::new(span.start, span.end))); + if let Some(id) = &function.id { + let function_name = id.name.as_str(); + let error_msg = format!( + "Function '{}' has too many parameters ({}). Maximum allowed is {}.", + function_name, + function.params.items.len(), + self.max + ); + let error = CompactStr::from(error_msg); + let span = function.params.span; + ctx.diagnostic(MaxParamsDiagnostic(error, Span::new(span.start, span.end))); + } else { + let error_msg = format!( + "Function has too many parameters ({}). Maximum allowed is {}.", + function.params.items.len(), + self.max + ); + let error = CompactStr::from(error_msg); + let span = function.params.span; + ctx.diagnostic(MaxParamsDiagnostic(error, Span::new(span.start, span.end))); + } } + + } AstKind::ArrowFunctionExpression(function) => { if function.params.items.len() > self.max { let error_msg = format!( - "Function has too many parameters ({}). Maximum allowed is {}.", + "Arrow function has too many parameters ({}). Maximum allowed is {}.", function.params.items.len(), self.max ); diff --git a/crates/oxc_linter/src/snapshots/max_params.snap b/crates/oxc_linter/src/snapshots/max_params.snap index d06f0a384da5..2a518f1a2e0a 100644 --- a/crates/oxc_linter/src/snapshots/max_params.snap +++ b/crates/oxc_linter/src/snapshots/max_params.snap @@ -2,14 +2,14 @@ source: crates/oxc_linter/src/tester.rs expression: max_params --- - ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 2." + ⚠ eslint(max-params): "Function 'test' has too many parameters (3). Maximum allowed is 2." ╭─[max_params.tsx:1:14] 1 │ function test(a, b, c) {} · ───────── ╰──── help: This rule enforces a maximum number of parameters allowed in function definitions. - ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ⚠ eslint(max-params): "Function 'test' has too many parameters (4). Maximum allowed is 3." ╭─[max_params.tsx:1:14] 1 │ function test(a, b, c, d) {} · ──────────── @@ -23,7 +23,7 @@ expression: max_params ╰──── help: This rule enforces a maximum number of parameters allowed in function definitions. - ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ⚠ eslint(max-params): "Arrow function has too many parameters (4). Maximum allowed is 3." ╭─[max_params.tsx:1:12] 1 │ var test = (a, b, c, d) => {}; · ──────────── @@ -37,35 +37,35 @@ expression: max_params ╰──── help: This rule enforces a maximum number of parameters allowed in function definitions. - ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 1." + ⚠ eslint(max-params): "Function 'test' has too many parameters (3). Maximum allowed is 1." ╭─[max_params.tsx:1:25] 1 │ var test = function test(a, b, c) {}; · ───────── ╰──── help: This rule enforces a maximum number of parameters allowed in function definitions. - ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 2." + ⚠ eslint(max-params): "Function 'test' has too many parameters (3). Maximum allowed is 2." ╭─[max_params.tsx:1:14] 1 │ function test(a, b, c) {} · ───────── ╰──── help: This rule enforces a maximum number of parameters allowed in function definitions. - ⚠ eslint(max-params): "Function has too many parameters (4). Maximum allowed is 3." + ⚠ eslint(max-params): "Function 'test' has too many parameters (4). Maximum allowed is 3." ╭─[max_params.tsx:1:14] 1 │ function test(a, b, c, d) {} · ──────────── ╰──── help: This rule enforces a maximum number of parameters allowed in function definitions. - ⚠ eslint(max-params): "Function has too many parameters (1). Maximum allowed is 0." + ⚠ eslint(max-params): "Function 'test' has too many parameters (1). Maximum allowed is 0." ╭─[max_params.tsx:1:14] 1 │ function test(a) {} · ─── ╰──── help: This rule enforces a maximum number of parameters allowed in function definitions. - ⚠ eslint(max-params): "Function has too many parameters (3). Maximum allowed is 2." + ⚠ eslint(max-params): "Function 'test' has too many parameters (3). Maximum allowed is 2." ╭─[max_params.tsx:1:14] 1 │ function test(a, b, c) { · ───────── From 10b78475a14e2037c4c58235253f32173b240f11 Mon Sep 17 00:00:00 2001 From: "j.buendia" Date: Mon, 18 Mar 2024 16:30:35 +0100 Subject: [PATCH 3/3] feat(linter): fix format --- crates/oxc_linter/src/rules/eslint/max_params.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/max_params.rs b/crates/oxc_linter/src/rules/eslint/max_params.rs index 0fac7db49496..0993a4898a4f 100644 --- a/crates/oxc_linter/src/rules/eslint/max_params.rs +++ b/crates/oxc_linter/src/rules/eslint/max_params.rs @@ -105,8 +105,6 @@ impl Rule for MaxParams { ctx.diagnostic(MaxParamsDiagnostic(error, Span::new(span.start, span.end))); } } - - } AstKind::ArrowFunctionExpression(function) => { if function.params.items.len() > self.max {