Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): eslint/max-params #2749

Merged
merged 6 commits into from Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
143 changes: 143 additions & 0 deletions 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<MaxParamsConfig>);

#[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) => {
mysteryven marked this conversation as resolved.
Show resolved Hide resolved
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();
}
74 changes: 74 additions & 0 deletions 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.