From 23b6bafb81cc7e25e49fc10bb057ab37ce4245bc Mon Sep 17 00:00:00 2001 From: ikkz Date: Mon, 17 Feb 2025 17:37:14 +0800 Subject: [PATCH 1/2] feat(linter): add eslint/max-depth --- crates/oxc_linter/src/rules.rs | 2 + .../oxc_linter/src/rules/eslint/max_depth.rs | 214 ++++++++++++++++++ .../src/snapshots/eslint_max_depth.snap | 107 +++++++++ 3 files changed, 323 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/max_depth.rs create mode 100644 crates/oxc_linter/src/snapshots/eslint_max_depth.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 89ce665f7d68f..c8a35c14b9f53 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -45,6 +45,7 @@ mod eslint { pub mod guard_for_in; pub mod init_declarations; pub mod max_classes_per_file; + pub mod max_depth; pub mod max_lines; pub mod max_nested_callbacks; pub mod max_params; @@ -554,6 +555,7 @@ oxc_macros::declare_all_lint_rules! { eslint::init_declarations, eslint::max_nested_callbacks, eslint::max_classes_per_file, + eslint::max_depth, eslint::max_lines, eslint::max_params, eslint::new_cap, diff --git a/crates/oxc_linter/src/rules/eslint/max_depth.rs b/crates/oxc_linter/src/rules/eslint/max_depth.rs new file mode 100644 index 0000000000000..9eb1f44e1c6a4 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/max_depth.rs @@ -0,0 +1,214 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::AstNodes; +use oxc_span::GetSpan; +use oxc_span::Span; +use serde_json::Value; + +use crate::{ast_util::is_function_node, context::LintContext, rule::Rule, AstNode}; + +fn max_depth_diagnostic(num: usize, max: usize, span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn(format!("Blocks are nested too deeply ({num}). Maximum allowed is {max}.")) + .with_help("Consider refactoring your code.") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct MaxDepth { + max: usize, +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Enforce a maximum depth that blocks can be nested + /// + /// ### Why is this bad? + /// + /// Many developers consider code difficult to read if blocks are nested beyond a + /// certain depth + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule with the default `{ "max": 4 }` + /// option: + /// ```js + /// function foo() { + /// for (;;) { // Nested 1 deep + /// while (true) { // Nested 2 deep + /// if (true) { // Nested 3 deep + /// if (true) { // Nested 4 deep + /// if (true) { // Nested 5 deep + /// } + /// } + /// } + /// } + /// } + /// } + /// ``` + /// + /// Examples of **correct** code for this rule with the default `{ "max": 4 }` option: + /// ```js + /// function foo() { + /// for (;;) { // Nested 1 deep + /// while (true) { // Nested 2 deep + /// if (true) { // Nested 3 deep + /// if (true) { // Nested 4 deep + /// } + /// } + /// } + /// } + /// } + /// ``` + /// + /// Note that class static blocks do not count as nested blocks, and that the depth in + /// them is calculated separately from the enclosing context. + /// + /// Examples of **incorrect** code for this rule with `{ "max": 2 }` option: + /// ```js + /// function foo() { + /// if (true) { // Nested 1 deep + /// class C { + /// static { + /// if (true) { // Nested 1 deep + /// if (true) { // Nested 2 deep + /// if (true) { // Nested 3 deep + /// } + /// } + /// } + /// } + /// } + /// } + /// } + /// ``` + /// + /// Examples of **correct** code for this rule with `{ "max": 2 }` option: + /// ```js + /// function foo() { + /// if (true) { // Nested 1 deep + /// class C { + /// static { + /// if (true) { // Nested 1 deep + /// if (true) { // Nested 2 deep + /// } + /// } + /// } + /// } + /// } + /// } + /// ``` + /// + /// ### Options + /// + /// #### max + /// + /// `{ type: number, default: 4 }` + /// + /// The `max` enforces a maximum depth that blocks can be nested + /// + /// Example: + /// + /// ```json + /// "eslint/max-depth": ["error", 4] + /// + /// "eslint/max-depth": [ + /// "error", + /// { + /// max: 4 + /// } + /// ] + /// ``` + MaxDepth, + eslint, + pedantic + +); + +impl Rule for MaxDepth { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if should_count(node, ctx.semantic().nodes()) { + let depth = ctx + .semantic() + .nodes() + .ancestors(node.id()) + .take_while(|node| !should_stop(node)) + .filter(|node| should_count(node, ctx.semantic().nodes())) + .count(); + if depth > self.max { + ctx.diagnostic(max_depth_diagnostic(depth, self.max, node.span())); + } + } + } + + fn from_configuration(value: serde_json::Value) -> Self { + let config = value.get(0); + let max = 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()) + { + max + } else { + config + .and_then(|config| config.get("max")) + .and_then(Value::as_number) + .and_then(serde_json::Number::as_u64) + .map_or(4, |v| usize::try_from(v).unwrap_or(4)) + }; + Self { max } + } +} + +fn should_count(node: &AstNode<'_>, nodes: &AstNodes<'_>) -> bool { + matches!(node.kind(), AstKind::IfStatement(_) if !matches!(nodes.parent_kind(node.id()), Some(AstKind::IfStatement(_)))) + || matches!(node.kind(), |AstKind::SwitchStatement(_)| AstKind::TryStatement(_) + | AstKind::DoWhileStatement(_) + | AstKind::WhileStatement(_) + | AstKind::WithStatement(_) + | AstKind::ForStatement(_) + | AstKind::ForInStatement(_) + | AstKind::ForOfStatement(_)) +} + +fn should_stop(node: &AstNode<'_>) -> bool { + is_function_node(node) || matches!(node.kind(), AstKind::Program(_) | AstKind::StaticBlock(_)) +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("function foo() { if (true) { if (false) { if (true) { } } } }", Some(serde_json::json!([3]))), + ("function foo() { if (true) { } else if (false) { } else if (true) { } else if (false) {} }", Some(serde_json::json!([3]))), + ("var foo = () => { if (true) { if (false) { if (true) { } } } }", Some(serde_json::json!([3]))), // { "ecmaVersion": 6 }, + ("function foo() { if (true) { if (false) { if (true) { } } } }", None), + ("function foo() { if (true) { if (false) { if (true) { } } } }", Some(serde_json::json!([{ "max": 3 }]))), + ("class C { static { if (1) { if (2) {} } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("class C { static { if (1) { if (2) {} } if (1) { if (2) {} } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("class C { static { if (1) { if (2) {} } } static { if (1) { if (2) {} } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("if (1) { class C { static { if (1) { if (2) {} } } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("function foo() { if (1) { class C { static { if (1) { if (2) {} } } } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("function foo() { if (1) { if (2) { class C { static { if (1) { if (2) {} } if (1) { if (2) {} } } } } } if (1) { if (2) {} } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 } + ]; + + let fail = vec![ + ("function foo() { if (true) { if (false) { if (true) { } } } }", Some(serde_json::json!([2]))), + ("var foo = () => { if (true) { if (false) { if (true) { } } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 6 }, + ("function foo() { if (true) {} else { for(;;) {} } }", Some(serde_json::json!([1]))), + ("function foo() { while (true) { if (true) {} } }", Some(serde_json::json!([1]))), + ("function foo() { for (let x of foo) { if (true) {} } }", Some(serde_json::json!([1]))), // { "ecmaVersion": 6 }, + ("function foo() { while (true) { if (true) { if (false) { } } } }", Some(serde_json::json!([1]))), + ("function foo() { if (true) { if (false) { if (true) { if (false) { if (true) { } } } } } }", None), + ("function foo() { if (true) { if (false) { if (true) { } } } }", Some(serde_json::json!([{ "max": 2 }]))), + ("function foo() { if (a) { if (b) { if (c) { if (d) { if (e) {} } } } } }", Some(serde_json::json!([{}]))), + ("function foo() { if (true) {} }", Some(serde_json::json!([{ "max": 0 }]))), + ("class C { static { if (1) { if (2) { if (3) {} } } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("if (1) { class C { static { if (1) { if (2) { if (3) {} } } } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("function foo() { if (1) { class C { static { if (1) { if (2) { if (3) {} } } } } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 }, + ("function foo() { if (1) { class C { static { if (1) { if (2) {} } } } if (2) { if (3) {} } } }", Some(serde_json::json!([2]))), // { "ecmaVersion": 2022 } + ]; + + Tester::new(MaxDepth::NAME, MaxDepth::PLUGIN, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_max_depth.snap b/crates/oxc_linter/src/snapshots/eslint_max_depth.snap new file mode 100644 index 0000000000000..2ab8eaf69d412 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_max_depth.snap @@ -0,0 +1,107 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 2. + ╭─[max_depth.tsx:1:43] + 1 │ function foo() { if (true) { if (false) { if (true) { } } } } + · ───────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 2. + ╭─[max_depth.tsx:1:44] + 1 │ var foo = () => { if (true) { if (false) { if (true) { } } } } + · ───────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (2). Maximum allowed is 1. + ╭─[max_depth.tsx:1:38] + 1 │ function foo() { if (true) {} else { for(;;) {} } } + · ────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (2). Maximum allowed is 1. + ╭─[max_depth.tsx:1:33] + 1 │ function foo() { while (true) { if (true) {} } } + · ──────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (2). Maximum allowed is 1. + ╭─[max_depth.tsx:1:39] + 1 │ function foo() { for (let x of foo) { if (true) {} } } + · ──────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (2). Maximum allowed is 1. + ╭─[max_depth.tsx:1:33] + 1 │ function foo() { while (true) { if (true) { if (false) { } } } } + · ──────────────────────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 1. + ╭─[max_depth.tsx:1:45] + 1 │ function foo() { while (true) { if (true) { if (false) { } } } } + · ────────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (5). Maximum allowed is 4. + ╭─[max_depth.tsx:1:68] + 1 │ function foo() { if (true) { if (false) { if (true) { if (false) { if (true) { } } } } } } + · ───────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 2. + ╭─[max_depth.tsx:1:43] + 1 │ function foo() { if (true) { if (false) { if (true) { } } } } + · ───────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (5). Maximum allowed is 4. + ╭─[max_depth.tsx:1:54] + 1 │ function foo() { if (a) { if (b) { if (c) { if (d) { if (e) {} } } } } } + · ───────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (1). Maximum allowed is 0. + ╭─[max_depth.tsx:1:18] + 1 │ function foo() { if (true) {} } + · ──────────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 2. + ╭─[max_depth.tsx:1:38] + 1 │ class C { static { if (1) { if (2) { if (3) {} } } } } + · ───────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 2. + ╭─[max_depth.tsx:1:47] + 1 │ if (1) { class C { static { if (1) { if (2) { if (3) {} } } } } } + · ───────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 2. + ╭─[max_depth.tsx:1:64] + 1 │ function foo() { if (1) { class C { static { if (1) { if (2) { if (3) {} } } } } } } + · ───────── + ╰──── + help: Consider refactoring your code. + + ⚠ eslint(max-depth): Blocks are nested too deeply (3). Maximum allowed is 2. + ╭─[max_depth.tsx:1:80] + 1 │ function foo() { if (1) { class C { static { if (1) { if (2) {} } } } if (2) { if (3) {} } } } + · ───────── + ╰──── + help: Consider refactoring your code. From e836078239fcdeddb14b237699c92c0922e402c3 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Thu, 20 Feb 2025 23:34:58 +0800 Subject: [PATCH 2/2] chore: improve the documentation --- .../oxc_linter/src/rules/eslint/max_depth.rs | 81 +++++++------------ 1 file changed, 29 insertions(+), 52 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/max_depth.rs b/crates/oxc_linter/src/rules/eslint/max_depth.rs index 9eb1f44e1c6a4..7132f78c77a0c 100644 --- a/crates/oxc_linter/src/rules/eslint/max_depth.rs +++ b/crates/oxc_linter/src/rules/eslint/max_depth.rs @@ -22,80 +22,58 @@ pub struct MaxDepth { declare_oxc_lint!( /// ### What it does /// - /// Enforce a maximum depth that blocks can be nested + /// Enforce a maximum depth that blocks can be nested. This rule helps to limit the complexity + /// of nested blocks, improving readability and maintainability by ensuring that code does not + /// become too deeply nested. /// /// ### Why is this bad? /// - /// Many developers consider code difficult to read if blocks are nested beyond a - /// certain depth + /// Many developers consider code difficult to read if blocks are nested beyond a certain depth. + /// Excessive nesting can make it harder to follow the flow of the code, increasing cognitive load + /// and making maintenance more error-prone. By enforcing a maximum block depth, this rule encourages + /// cleaner, more readable code. /// /// ### Examples /// - /// Examples of **incorrect** code for this rule with the default `{ "max": 4 }` - /// option: + /// Examples of **incorrect** code for this rule with the default `{ "max": 3 }` option: /// ```js /// function foo() { - /// for (;;) { // Nested 1 deep - /// while (true) { // Nested 2 deep - /// if (true) { // Nested 3 deep - /// if (true) { // Nested 4 deep - /// if (true) { // Nested 5 deep - /// } - /// } - /// } - /// } + /// for (;;) { // Nested 1 deep + /// while (true) { // Nested 2 deep + /// if (true) { // Nested 3 deep + /// if (true) { // Nested 4 deep } + /// } /// } + /// } /// } /// ``` /// - /// Examples of **correct** code for this rule with the default `{ "max": 4 }` option: + /// Examples of **correct** code for this rule with the default `{ "max": 3 }` option: /// ```js /// function foo() { - /// for (;;) { // Nested 1 deep - /// while (true) { // Nested 2 deep - /// if (true) { // Nested 3 deep - /// if (true) { // Nested 4 deep - /// } - /// } - /// } + /// for (;;) { // Nested 1 deep + /// while (true) { // Nested 2 deep + /// if (true) { // Nested 3 deep } /// } + /// } /// } /// ``` /// /// Note that class static blocks do not count as nested blocks, and that the depth in /// them is calculated separately from the enclosing context. /// - /// Examples of **incorrect** code for this rule with `{ "max": 2 }` option: - /// ```js - /// function foo() { - /// if (true) { // Nested 1 deep - /// class C { - /// static { - /// if (true) { // Nested 1 deep - /// if (true) { // Nested 2 deep - /// if (true) { // Nested 3 deep - /// } - /// } - /// } - /// } - /// } - /// } - /// } - /// ``` - /// - /// Examples of **correct** code for this rule with `{ "max": 2 }` option: + /// Example: /// ```js /// function foo() { - /// if (true) { // Nested 1 deep - /// class C { - /// static { - /// if (true) { // Nested 1 deep - /// if (true) { // Nested 2 deep - /// } - /// } - /// } + /// if (true) { // Nested 1 deep + /// class C { + /// static { + /// if (true) { // Nested 1 deep + /// if (true) { // Nested 2 deep } /// } + /// } /// } + /// } /// } /// ``` /// @@ -127,13 +105,12 @@ declare_oxc_lint!( impl Rule for MaxDepth { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - if should_count(node, ctx.semantic().nodes()) { + if should_count(node, ctx.nodes()) { let depth = ctx - .semantic() .nodes() .ancestors(node.id()) .take_while(|node| !should_stop(node)) - .filter(|node| should_count(node, ctx.semantic().nodes())) + .filter(|node| should_count(node, ctx.nodes())) .count(); if depth > self.max { ctx.diagnostic(max_depth_diagnostic(depth, self.max, node.span()));