From 8f4c5c8f37584ebade3f9ffee39cc7643de86834 Mon Sep 17 00:00:00 2001 From: baseballyama Date: Fri, 27 Dec 2024 21:12:05 +0900 Subject: [PATCH] feat(linter): implement no-lone-blocks --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/eslint/no_lone_blocks.rs | 386 ++++++++++++++++++ .../src/snapshots/eslint_no_lone_blocks.snap | 231 +++++++++++ 3 files changed, 619 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/no_lone_blocks.rs create mode 100644 crates/oxc_linter/src/snapshots/eslint_no_lone_blocks.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 4245090ed683e..f290a4f51cf8d 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -89,6 +89,7 @@ mod eslint { pub mod no_irregular_whitespace; pub mod no_iterator; pub mod no_label_var; + pub mod no_lone_blocks; pub mod no_loss_of_precision; pub mod no_magic_numbers; pub mod no_multi_str; @@ -534,6 +535,7 @@ oxc_macros::declare_all_lint_rules! { eslint::max_classes_per_file, eslint::max_lines, eslint::max_params, + eslint::no_lone_blocks, eslint::no_restricted_imports, eslint::no_object_constructor, eslint::no_duplicate_imports, diff --git a/crates/oxc_linter/src/rules/eslint/no_lone_blocks.rs b/crates/oxc_linter/src/rules/eslint/no_lone_blocks.rs new file mode 100644 index 0000000000000..0ebad6396f673 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_lone_blocks.rs @@ -0,0 +1,386 @@ +use oxc_ast::ast::VariableDeclarationKind; +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::{GetSpan, Span}; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn no_lone_blocks_diagnostic(span: Span, is_nested_block: bool) -> OxcDiagnostic { + let message = + if is_nested_block { "Nested block is redundant." } else { "Block is unnecessary." }; + // See for details + OxcDiagnostic::warn(message).with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoLoneBlocks; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows unnecessary standalone block statements. + /// + /// ### Why is this bad? + /// + /// Standalone blocks can be confusing as they do not provide any meaningful purpose when used unnecessarily. + /// They may introduce extra nesting, reducing code readability, and can mislead readers about scope or intent. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// { + /// var x = 1; + /// } + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// if (condition) { + /// var x = 1; + /// } + /// + /// { + /// let x = 1; // Used to create a valid block scope. + /// } + /// ``` + NoLoneBlocks, + style, +); + +impl Rule for NoLoneBlocks { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::BlockStatement(stmt) = node.kind() else { + return; + }; + + let Some(parent_node) = ctx.nodes().parent_node(node.id()) else { + return; + }; + + let body = &stmt.body; + if body.is_empty() { + report(ctx, node, parent_node); + return; + } + + if body.len() == 1 + && matches!(parent_node.kind(), AstKind::FunctionBody(parent) if parent.statements.len() == 1) + { + report(ctx, node, parent_node); + } + + let mut lone_blocks = Vec::new(); + if is_lone_block(node, parent_node) { + lone_blocks.push(node); + } + + let span = stmt.span(); + let children: Vec<_> = ctx + .nodes() + .iter() + .filter(|n| { + let n_span = n.span(); + span.start < n_span.start && n_span.end < span.end + }) + .collect(); + + for child in children { + if let Some(parent_of_child) = ctx.nodes().parent_node(child.id()) { + match child.kind() { + AstKind::VariableDeclaration(var_decl) + if var_decl.kind != VariableDeclarationKind::Var => + { + mark_lone_block(parent_of_child, &mut lone_blocks); + } + AstKind::Function(_) | AstKind::Class(_) => { + mark_lone_block(parent_of_child, &mut lone_blocks); + } + _ => {} + } + } + } + + if let Some(last) = lone_blocks.last() { + if last.id() == node.id() { + lone_blocks.pop(); + report(ctx, node, parent_node); + } + } else { + match parent_node.kind() { + AstKind::BlockStatement(parent_statement) => { + if parent_statement.body.len() == 1 { + report(ctx, node, parent_node); + } + } + AstKind::StaticBlock(parent_statement) => { + if parent_statement.body.len() == 1 { + report(ctx, node, parent_node); + } + } + _ => {} + } + } + } +} + +fn report(ctx: &LintContext, node: &AstNode, parent_node: &AstNode) { + match parent_node.kind() { + AstKind::BlockStatement(_) | AstKind::StaticBlock(_) => { + ctx.diagnostic(no_lone_blocks_diagnostic(node.span(), true)); + } + _ => ctx.diagnostic(no_lone_blocks_diagnostic(node.span(), false)), + }; +} + +fn is_lone_block(node: &AstNode, parent_node: &AstNode) -> bool { + match parent_node.kind() { + AstKind::BlockStatement(_) | AstKind::StaticBlock(_) | AstKind::Program(_) => true, + AstKind::SwitchCase(parent_node) => { + let consequent = &parent_node.consequent; + if consequent.len() != 1 { + return true; + } + let node_span = node.span(); + let consequent_span = consequent[0].span(); + node_span.start != consequent_span.start || node_span.end != consequent_span.end + } + _ => false, + } +} + +fn mark_lone_block(parent_node: &AstNode, lone_blocks: &mut Vec<&AstNode>) { + if lone_blocks.is_empty() { + return; + } + + if lone_blocks.last().map_or(false, |last| last.id() == parent_node.id()) { + lone_blocks.pop(); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "if (foo) { if (bar) { baz(); } }", + "do { bar(); } while (foo)", + "function foo() { while (bar) { baz() } }", + "{ let x = 1; }", // { "ecmaVersion": 6 }, + "{ const x = 1; }", // { "ecmaVersion": 6 }, + "'use strict'; { function bar() {} }", // { "ecmaVersion": 6 }, + "{ function bar() {} }", // { "ecmaVersion": 6, "parserOptions": { "ecmaFeatures": { "impliedStrict": true } } }, + "{ class Bar {} }", // { "ecmaVersion": 6 }, + "{ {let y = 1;} let x = 1; }", // { "ecmaVersion": 6 }, + " + switch (foo) { + case bar: { + baz; + } + } + ", + " + switch (foo) { + case bar: { + baz; + } + case qux: { + boop; + } + } + ", + " + switch (foo) { + case bar: + { + baz; + } + } + ", + "function foo() { { const x = 4 } const x = 3 }", // { "ecmaVersion": 6 }, + "class C { static {} }", // { "ecmaVersion": 2022 }, + "class C { static { foo; } }", // { "ecmaVersion": 2022 }, + "class C { static { if (foo) { block; } } }", // { "ecmaVersion": 2022 }, + "class C { static { lbl: { block; } } }", // { "ecmaVersion": 2022 }, + "class C { static { { let block; } something; } }", // { "ecmaVersion": 2022 }, + "class C { static { something; { const block = 1; } } }", // { "ecmaVersion": 2022 }, + "class C { static { { function block(){} } something; } }", // { "ecmaVersion": 2022 }, + "class C { static { something; { class block {} } } }", // { "ecmaVersion": 2022 }, + " + { + using x = makeDisposable(); + }", // { "parser": require(parser("typescript-parsers/no-lone-blocks/using")), "ecmaVersion": 2022 }, + " + { + await using x = makeDisposable(); + }", // { "parser": require(parser("typescript-parsers/no-lone-blocks/await-using")), "ecmaVersion": 2022 } + ]; + + let fail = vec![ + "{}", + "{var x = 1;}", + "foo(); {} bar();", + "if (foo) { bar(); {} baz(); }", + "{ + { } }", + "function foo() { bar(); {} baz(); }", + "while (foo) { {} }", + // MEMO: Currently, this rule always analyzes in strict mode (as it cannot retrieve ecmaFeatures). + // "{ function bar() {} }", // { "ecmaVersion": 6 }, + "{var x = 1;}", // { "ecmaVersion": 6 }, + "{ + {var x = 1;} + let y = 2; } {let z = 1;}", // { "ecmaVersion": 6 }, + "{ + {let x = 1;} + var y = 2; } {let z = 1;}", // { "ecmaVersion": 6 }, + "{ + {var x = 1;} + var y = 2; } + {var z = 1;}", // { "ecmaVersion": 6 }, + " + switch (foo) { + case 1: + foo(); + { + bar; + } + } + ", + " + switch (foo) { + case 1: + { + bar; + } + foo(); + } + ", + " + function foo () { + { + const x = 4; + } + } + ", // { "ecmaVersion": 6 }, + " + function foo () { + { + var x = 4; + } + } + ", + " + class C { + static { + if (foo) { + { + let block; + } + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + if (foo) { + { + block; + } + something; + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + { + block; + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + { + let block; + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + { + const block = 1; + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + { + function block() {} + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + { + class block {} + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + { + var block; + } + something; + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + something; + { + var block; + } + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + { + block; + } + something; + } + } + ", // { "ecmaVersion": 2022 }, + " + class C { + static { + something; + { + block; + } + } + } + ", // { "ecmaVersion": 2022 } + ]; + + Tester::new(NoLoneBlocks::NAME, NoLoneBlocks::CATEGORY, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_no_lone_blocks.snap b/crates/oxc_linter/src/snapshots/eslint_no_lone_blocks.snap new file mode 100644 index 0000000000000..a1677b9f013d7 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_lone_blocks.snap @@ -0,0 +1,231 @@ +--- +source: crates/oxc_linter/src/tester.rs +snapshot_kind: text +--- + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:1] + 1 │ {} + · ── + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:1] + 1 │ {var x = 1;} + · ──────────── + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:8] + 1 │ foo(); {} bar(); + · ── + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:1:19] + 1 │ if (foo) { bar(); {} baz(); } + · ── + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:1] + 1 │ ╭─▶ { + 2 │ ╰─▶ { } } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:2:4] + 1 │ { + 2 │ { } } + · ─── + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:25] + 1 │ function foo() { bar(); {} baz(); } + · ── + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:1:15] + 1 │ while (foo) { {} } + · ── + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:1] + 1 │ {var x = 1;} + · ──────────── + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:2:4] + 1 │ { + 2 │ {var x = 1;} + · ──────────── + 3 │ let y = 2; } {let z = 1;} + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:1] + 1 │ ╭─▶ { + 2 │ │ {let x = 1;} + 3 │ ╰─▶ var y = 2; } {let z = 1;} + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:1:1] + 1 │ ╭─▶ { + 2 │ │ {var x = 1;} + 3 │ ╰─▶ var y = 2; } + 4 │ {var z = 1;} + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:2:4] + 1 │ { + 2 │ {var x = 1;} + · ──────────── + 3 │ var y = 2; } + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:4:5] + 3 │ var y = 2; } + 4 │ {var z = 1;} + · ──────────── + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:5:24] + 4 │ foo(); + 5 │ ╭─▶ { + 6 │ │ bar; + 7 │ ╰─▶ } + 8 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:4:20] + 3 │ case 1: + 4 │ ╭─▶ { + 5 │ │ bar; + 6 │ ╰─▶ } + 7 │ foo(); + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:3:20] + 2 │ function foo () { + 3 │ ╭─▶ { + 4 │ │ const x = 4; + 5 │ ╰─▶ } + 6 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Block is unnecessary. + ╭─[no_lone_blocks.tsx:3:20] + 2 │ function foo () { + 3 │ ╭─▶ { + 4 │ │ var x = 4; + 5 │ ╰─▶ } + 6 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:5:24] + 4 │ if (foo) { + 5 │ ╭─▶ { + 6 │ │ let block; + 7 │ ╰─▶ } + 8 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:5:24] + 4 │ if (foo) { + 5 │ ╭─▶ { + 6 │ │ block; + 7 │ ╰─▶ } + 8 │ something; + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:4:22] + 3 │ static { + 4 │ ╭─▶ { + 5 │ │ block; + 6 │ ╰─▶ } + 7 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:4:22] + 3 │ static { + 4 │ ╭─▶ { + 5 │ │ let block; + 6 │ ╰─▶ } + 7 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:4:22] + 3 │ static { + 4 │ ╭─▶ { + 5 │ │ const block = 1; + 6 │ ╰─▶ } + 7 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:4:22] + 3 │ static { + 4 │ ╭─▶ { + 5 │ │ function block() {} + 6 │ ╰─▶ } + 7 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:4:22] + 3 │ static { + 4 │ ╭─▶ { + 5 │ │ class block {} + 6 │ ╰─▶ } + 7 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:4:22] + 3 │ static { + 4 │ ╭─▶ { + 5 │ │ var block; + 6 │ ╰─▶ } + 7 │ something; + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:5:22] + 4 │ something; + 5 │ ╭─▶ { + 6 │ │ var block; + 7 │ ╰─▶ } + 8 │ } + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:4:22] + 3 │ static { + 4 │ ╭─▶ { + 5 │ │ block; + 6 │ ╰─▶ } + 7 │ something; + ╰──── + + ⚠ eslint(no-lone-blocks): Nested block is redundant. + ╭─[no_lone_blocks.tsx:5:22] + 4 │ something; + 5 │ ╭─▶ { + 6 │ │ block; + 7 │ ╰─▶ } + 8 │ } + ╰────