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.

Sign up for GitHub

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): implement no-lone-blocks rule #8145

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+608 −0
Open

feat(linter): implement no-lone-blocks rule #8145

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f4c5c8
feat(linter): implement no-lone-blocks
baseballyama Dec 27, 2024
bd52e6e
perf
baseballyama Dec 27, 2024
51cc55f
fix lint
baseballyama Dec 27, 2024
62f0f8b
Merge branch 'main' into feat/lint/no-lone-blocks
baseballyama Dec 27, 2024
d272fe5
split no_lone_blocks_diagnostic
baseballyama Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ mod eslint {
pub mod no_iterator;
pub mod no_label_var;
pub mod no_labels;
pub mod no_lone_blocks;
pub mod no_loss_of_precision;
pub mod no_magic_numbers;
pub mod no_multi_str;
Expand Down Expand Up @@ -536,6 +537,7 @@ oxc_macros::declare_all_lint_rules! {
eslint::max_lines,
eslint::max_params,
eslint::no_labels,
eslint::no_lone_blocks,
eslint::no_restricted_imports,
eslint::no_object_constructor,
eslint::no_duplicate_imports,
Expand Down
374 changes: 374 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_lone_blocks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,374 @@
use oxc_ast::ast::{Declaration, 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 <https://oxc.rs/docs/contribute/linter/adding-rules.html#diagnostics> for details
OxcDiagnostic::warn(message).with_label(span)
}
camc314 marked this conversation as resolved.
Show resolved Hide resolved

#[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);
}

for child in &stmt.body {
match child.as_declaration() {
Some(Declaration::VariableDeclaration(decl))
if decl.kind != VariableDeclarationKind::Var =>
{
mark_lone_block(node, &mut lone_blocks);
}
Some(Declaration::ClassDeclaration(_) | Declaration::FunctionDeclaration(_)) => {
mark_lone_block(node, &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();
}
Loading
Loading