Skip to content

Commit

Permalink
feat(lint/noUselessBlockStatements): add rule (#499)
Browse files Browse the repository at this point in the history
Co-authored-by: Eddy Brown <eddy@devbrown.com>
  • Loading branch information
emab and Eddy Brown authored Oct 14, 2023
1 parent ee2e2ab commit 0a688a9
Show file tree
Hide file tree
Showing 22 changed files with 999 additions and 30 deletions.
1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ define_categories! {
"lint/nursery/noMisrefactoredShorthandAssign": "https://biomejs.dev/lint/rules/no-misrefactored-shorthand-assign",
"lint/nursery/noUnusedImports": "https://biomejs.dev/lint/rules/no-unused-imports",
"lint/nursery/noUselessElse": "https://biomejs.dev/lint/rules/no-useless-else",
"lint/nursery/noUselessLoneBlockStatements": "https://biomejs.dev/lint/rules/no-useless-lone-block-statements",
"lint/nursery/noVoid": "https://biomejs.dev/linter/rules/no-void",
"lint/nursery/useArrowFunction": "https://biomejs.dev/linter/rules/use-arrow-function",
"lint/nursery/useAsConstAssertion": "https://biomejs.dev/lint/rules/use-as-const-assertion",
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
use crate::semantic_services::Semantic;
use crate::JsRuleAction;
use biome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_diagnostics::Applicability;
use biome_js_factory::make;
use biome_js_syntax::{
AnyJsStatement, JsBlockStatement, JsFileSource, JsLabeledStatement, JsStatementList,
JsVariableStatement,
};
use biome_rowan::{AstNode, AstNodeList, BatchMutationExt};

declare_rule! {
/// Disallow unnecessary nested block statements.
///
/// > In JavaScript, prior to ES6, standalone code blocks delimited by curly braces do not create a new scope and have no use.
/// > In ES6, code blocks may create a new scope if a block-level binding (let and const), a class declaration or a function declaration (in strict mode) are present. A block is not considered redundant in these cases.
///
/// Source: https://eslint.org/docs/latest/rules/no-lone-blocks
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// {}
/// ```
///
/// ```js,expect_diagnostic
/// if (foo) {
/// bar();
/// {
/// baz();
/// }
/// }
/// ```
///
/// ## Valid
///
/// ```js
/// while (foo) {
/// bar();
/// }
/// ```
///
pub(crate) NoUselessLoneBlockStatements {
version: "next",
name: "noUselessLoneBlockStatements",
recommended: false,
}
}

impl Rule for NoUselessLoneBlockStatements {
type Query = Semantic<JsBlockStatement>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let block = ctx.query();
let is_module = ctx.source_type::<JsFileSource>().is_module();

if JsLabeledStatement::can_cast(block.syntax().parent()?.kind()) {
return None;
}

if in_control_structure(block) {
return None;
}

if block.statements().is_empty() {
return Some(());
}

if block
.statements()
.iter()
.any(|statement| statement_has_block_level_declaration(&statement, is_module))
{
return None;
}

Some(())
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
let block = ctx.query();
let block_range = block.range();
Some(
RuleDiagnostic::new(
rule_category!(),
block_range,
markup! {
"This block statement doesn't serve any purpose and can be safely removed."
},
)
.note(markup! {
"Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code."
}),
)
}

fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> {
let block = ctx.query();
let stmts_list = block.parent::<JsStatementList>()?;
let block_pos = stmts_list
.iter()
.position(|x| x.syntax() == block.syntax())?;

let inner_statements: Vec<AnyJsStatement> = block.statements().iter().collect();

let prev_stmts = stmts_list.iter().take(block_pos);

let next_stmts = stmts_list.iter().skip(block_pos + 1);

let new_stmts: Vec<_> = prev_stmts
.chain(inner_statements)
.chain(next_stmts)
.collect();

let new_stmts_list = make::js_statement_list(new_stmts);

let mut mutation = ctx.root().begin();
mutation.replace_node_discard_trivia(stmts_list, new_stmts_list);

return Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::Always,
message: markup! { "Remove redundant block." }.to_owned(),
mutation,
});
}
}

fn statement_has_block_level_declaration(statement: &AnyJsStatement, is_module: bool) -> bool {
match statement {
AnyJsStatement::JsVariableStatement(variable) => is_not_var_declaration(variable),
AnyJsStatement::JsFunctionDeclaration(_) => is_module,
AnyJsStatement::JsClassDeclaration(_) => true,
_ => false,
}
}

fn in_control_structure(block: &JsBlockStatement) -> bool {
matches!(
block.parent(),
Some(
AnyJsStatement::JsIfStatement(_)
| AnyJsStatement::JsWhileStatement(_)
| AnyJsStatement::JsDoWhileStatement(_)
| AnyJsStatement::JsForStatement(_)
| AnyJsStatement::JsForInStatement(_)
| AnyJsStatement::JsForOfStatement(_)
| AnyJsStatement::JsWithStatement(_)
| AnyJsStatement::JsSwitchStatement(_)
| AnyJsStatement::JsTryStatement(_)
)
)
}

fn is_not_var_declaration(variable: &JsVariableStatement) -> bool {
variable
.declaration()
.ok()
.map_or(false, |decl| !decl.is_var())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{}

{
aLabel: {
}
}

{
function foo() {}
}

{
// a comment
function foo() {}
}


Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalid.cjs
---
# Input
```js
{}

{
aLabel: {
}
}

{
function foo() {}
}

{
// a comment
function foo() {}
}



```

# Diagnostics
```
invalid.cjs:1:1 lint/nursery/noUselessLoneBlockStatements FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This block statement doesn't serve any purpose and can be safely removed.
> 1 │ {}
│ ^^
2 │
3 │ {
i Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
i Safe fix: Remove redundant block.
1 │ {}
│ --
```

```
invalid.cjs:3:1 lint/nursery/noUselessLoneBlockStatements FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This block statement doesn't serve any purpose and can be safely removed.
1 │ {}
2 │
> 3 │ {
│ ^
> 4 │ aLabel: {
> 5 │ }
> 6 │ }
│ ^
7 │
8 │ {
i Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
i Safe fix: Remove redundant block.
1 1 │ {}
2 │ -
3 │ - {
4 2 │ aLabel: {
5 │ - ·}
6 │ - }
3 │ + ·}
7 4 │
8 5 │ {
```

```
invalid.cjs:8:1 lint/nursery/noUselessLoneBlockStatements FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This block statement doesn't serve any purpose and can be safely removed.
6 │ }
7 │
> 8 │ {
│ ^
> 9 │ function foo() {}
> 10 │ }
│ ^
11 │
12 │ {
i Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
i Safe fix: Remove redundant block.
5 5 │ }
6 6 │ }
7 │ -
8 │ - {
9 │ - ·function·foo()·{}
10 │ - }
7 │ + ·function·foo()·{}
11 8 │
12 9 │ {
```

```
invalid.cjs:12:1 lint/nursery/noUselessLoneBlockStatements FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This block statement doesn't serve any purpose and can be safely removed.
10 │ }
11 │
> 12 │ {
│ ^
> 13 │ // a comment
> 14 │ function foo() {}
> 15 │ }
│ ^
16 │
i Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
i Safe fix: Remove redundant block.
9 9 │ function foo() {}
10 10 │ }
11 │ -
12 │ - {
13 11 │ // a comment
14 │ - ·function·foo()·{}
15 │ - }
12 │ + ·function·foo()·{}
16 13 │
17 14 │
```


Loading

0 comments on commit 0a688a9

Please sign in to comment.