Skip to content

Commit

Permalink
feat(lint/noEmptyBlockStatements): add rule
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewangelle committed Oct 13, 2023
1 parent ee2e2ab commit b26af0b
Show file tree
Hide file tree
Showing 27 changed files with 2,163 additions and 51 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 @@ -92,6 +92,7 @@ define_categories! {
"lint/nursery/noApproximativeNumericConstant": "https://biomejs.dev/lint/rules/no-approximative-numeric-constant",
"lint/nursery/noConfusingVoidType": "https://biomejs.dev/linter/rules/no-confusing-void-type",
"lint/nursery/noDuplicateJsonKeys": "https://biomejs.dev/linter/rules/no-duplicate-json-keys",
"lint/nursery/noEmptyBlockStatements": "https://biomejs.dev/lint/rules/no-empty-block-statements",
"lint/nursery/noEmptyCharacterClassInRegex": "https://biomejs.dev/lint/rules/no-empty-character-class-in-regex",
"lint/nursery/noExcessiveComplexity": "https://biomejs.dev/linter/rules/no-excessive-complexity",
"lint/nursery/noFallthroughSwitchClause": "https://biomejs.dev/linter/rules/no-fallthrough-switch-clause",
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,304 @@
use biome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_deserialize::{
json::{has_only_known_keys, VisitJsonNode},
DeserializationDiagnostic, VisitNode,
};
use biome_js_syntax::{
JsBlockStatement, JsCatchClause, JsFunctionBody, JsStaticInitializationBlockClassMember,
JsSwitchStatement, JsSyntaxKind,
};
use biome_json_syntax::JsonLanguage;
use biome_rowan::{declare_node_union, AstNode, AstNodeList, SyntaxNode, TextRange};
use bpaf::Bpaf;
use serde::{Deserialize, Serialize};

declare_rule! {
/// Disallow empty block statements and static blocks.
///
/// Empty static blocks and block statements, while not technically errors, usually occur due to refactoring that wasn’t completed. They can cause confusion when reading code.
///
/// This rule disallows empty block statements and static blocks. This rule ignores block statements which contain a comment (for example, in an empty catch or finally block of a try statement to indicate that execution should continue regardless of errors).
/// This rule also ignores static blocks which contain a comment.
///
/// Source: https://eslint.org/docs/latest/rules/no-empty-static-block/
/// Source: https://eslint.org/docs/latest/rules/no-empty/
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// function foo () {}
///
/// const foo = () => {}
///
/// function fooWithNestedEmptyBlock() {
/// let a = 1;
/// function shouldFail(){}
/// return a
/// }
///
/// const fooWithNestedEmptyBlock = () => {
/// let a = 1;
/// const shouldFail = () => {}
/// return a
/// }
/// let someVar;
/// if (someVar) {
/// }
///
/// while (someVar) {
/// }
///
/// switch(someVar) {
/// }
/// try {
/// doSomething();
/// } catch(ex) {
///
/// } finally {
///
/// }
///
// class Foo {
// static {}
// }
/// ```
///
/// ## Valid
///
/// ```js
/// function foo () {let a;}
///
/// const foo = () => {let a;}
///
/// function fooWithComment() {
/// // should work
/// }
///
/// const barWithComment = () => {
/// // should work
/// }
///
/// function fooWithMultilineComment() {
/// /**
/// * this should also work
/// */
/// }
///
/// const barWithMultilineComment = () => {
/// /**
/// * this should also work
/// */
/// }
///
///
/// if (foo) {
/// // empty
/// }
///
/// while (foo) {
/// /* empty */
/// }
///
/// try {
/// doSomething();
/// } catch (ex) {
/// // continue regardless of error
/// }
///
/// try {
/// doSomething();
/// } finally {
/// /* continue regardless of error */
/// }
///
/// class Foo {
/// static {
/// bar();
/// }
/// }
///
/// class Foo {
/// static {
/// // comment
/// }
/// }
/// ```
/// ## Options
///
/// The rule provides one option that is detailed in the following subsections.
///
/// ```json
/// {
/// "//": "...",
/// "options": {
/// "allowEmptyCatch": true
/// }
/// }
/// ```
///
/// ### allowEmptyCatch
///
/// When set to true allows empty catch clauses (that is, which do not contain a comment)
///
/// Default: false
///
/// Examples of additional correct code for this rule with the { "allowEmptyCatch": true } option:
///
/// ```jsx
/// try {
/// doSomething();
/// } catch (ex) {}
///
/// try {
/// doSomething();
/// }
/// catch (ex) {}
/// finally {
/// /* continue regardless of error */
/// }
/// ```
///
pub(crate) NoEmptyBlockStatements {
version: "next",
name: "noEmptyBlockStatements",
recommended: false,
}
}

declare_node_union! {
pub(crate) Query = JsBlockStatement | JsFunctionBody | JsStaticInitializationBlockClassMember | JsCatchClause | JsSwitchStatement
}

impl Rule for NoEmptyBlockStatements {
type Query = Ast<Query>;
type State = TextRange;
type Signals = Option<Self::State>;
type Options = NoEmptyBlockStatementsOptions;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
use Query::*;
let query = ctx.query();
let options = ctx.options();

match query {
JsCatchClause(_) => check_catch(query, options),
_ => check_block(query),
}
}

fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
Some(
RuleDiagnostic::new(
rule_category!(),
state,
markup! {
"No empty blocks allowed."
},
)
.note(markup! {
"Empty static blocks and block statements, while not technically errors, usually occur due to refactoring that wasn’t completed. They can cause confusion when reading code."
}),
)
}
}

fn is_empty(query: &Query) -> bool {
use Query::*;
match query {
JsFunctionBody(body) => body.directives().len() == 0 && body.statements().len() == 0,
JsBlockStatement(block) => block.statements().len() == 0,
JsStaticInitializationBlockClassMember(block) => block.statements().len() == 0,
JsSwitchStatement(statement) => statement.cases().len() == 0,
JsCatchClause(clause) => match clause.body() {
Ok(catch_body) => catch_body.statements().len() == 0,
Err(_) => false,
},
}
}

fn check_catch(query: &Query, options: &NoEmptyBlockStatementsOptions) -> Option<TextRange> {
let is_empty = is_empty(query);
let has_comments = query.syntax().has_comments_descendants();
let text_range = query.syntax().text_range();

if is_empty && !has_comments && !options.allow_empty_catch {
Some(text_range)
} else {
None
}
}

fn check_block(query: &Query) -> Option<TextRange> {
let is_empty = is_empty(query);
let has_comments = query.syntax().has_comments_descendants();
let text_range = query.syntax().text_range();
let parent: SyntaxNode<biome_js_syntax::JsLanguage> = query.syntax().parent()?;
let is_catch = matches!(parent.kind(), JsSyntaxKind::JS_CATCH_CLAUSE);

if is_empty && !has_comments && !is_catch {
Some(text_range)
} else {
None
}
}

/// Rule's options.
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Bpaf)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct NoEmptyBlockStatementsOptions {
/// If `true`, then empty catch blocks are allowed
#[bpaf(hide)]
#[serde(
default = "default_allow_empty_catch",
skip_serializing_if = "is_default_allow_empty_catch"
)]
pub allow_empty_catch: bool,
}

const fn default_allow_empty_catch() -> bool {
false
}

const fn is_default_allow_empty_catch(allow_empty_catch: &bool) -> bool {
*allow_empty_catch == default_allow_empty_catch()
}

impl NoEmptyBlockStatementsOptions {
pub(crate) const KNOWN_KEYS: &'static [&'static str] = &["allowEmptyCatch"];
}

impl Default for NoEmptyBlockStatementsOptions {
fn default() -> Self {
Self {
allow_empty_catch: default_allow_empty_catch(),
}
}
}

impl VisitNode<JsonLanguage> for NoEmptyBlockStatementsOptions {
fn visit_member_name(
&mut self,
node: &SyntaxNode<JsonLanguage>,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<()> {
has_only_known_keys(node, Self::KNOWN_KEYS, diagnostics)
}

fn visit_map(
&mut self,
key: &SyntaxNode<JsonLanguage>,
value: &SyntaxNode<JsonLanguage>,
diagnostics: &mut Vec<DeserializationDiagnostic>,
) -> Option<()> {
let (name, value) = self.get_key_and_value(key, value, diagnostics)?;
let name_text = name.text();
if name_text == "allowEmptyCatch" {
self.allow_empty_catch = self.map_to_boolean(&value, name_text, diagnostics)?
}
Some(())
}
}
23 changes: 22 additions & 1 deletion crates/biome_js_analyze/src/options.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! This module contains the rules that have options

use crate::analyzers::nursery::no_empty_block_statements::{
no_empty_block_statements_options, NoEmptyBlockStatementsOptions,
};
use crate::analyzers::nursery::no_excessive_complexity::{complexity_options, ComplexityOptions};
use crate::semantic_analyzers::nursery::use_exhaustive_dependencies::{
hooks_options, HooksOptions,
Expand Down Expand Up @@ -34,6 +37,10 @@ pub enum PossibleOptions {
NamingConvention(#[bpaf(external(naming_convention_options), hide)] NamingConventionOptions),
/// Options for `noRestrictedGlobals` rule
RestrictedGlobals(#[bpaf(external(restricted_globals_options), hide)] RestrictedGlobalsOptions),
/// Options for `noEmptyBlockStatements` rule
NoEmptyBlockStatements(
#[bpaf(external(no_empty_block_statements_options), hide)] NoEmptyBlockStatementsOptions,
),
/// No options available
#[default]
NoOptions,
Expand Down Expand Up @@ -78,6 +85,13 @@ impl PossibleOptions {
};
RuleOptions::new(options)
}
"noEmptyBlockStatements" => {
let options: NoEmptyBlockStatementsOptions = match self {
PossibleOptions::NoEmptyBlockStatements(options) => options.clone(),
_ => NoEmptyBlockStatementsOptions::default(),
};
RuleOptions::new(options)
}
// TODO: review error
_ => panic!("This rule {:?} doesn't have options", rule_key),
}
Expand Down Expand Up @@ -134,7 +148,14 @@ impl PossibleOptions {
options.visit_map(key.syntax(), value.syntax(), diagnostics)?;
*self = PossibleOptions::RestrictedGlobals(options);
}

"allowEmptyCatch" => {
let mut options = match self {
PossibleOptions::NoEmptyBlockStatements(options) => options.clone(),
_ => NoEmptyBlockStatementsOptions::default(),
};
options.visit_map(key.syntax(), value.syntax(), diagnostics)?;
*self = PossibleOptions::NoEmptyBlockStatements(options);
}
_ => (),
}
}
Expand Down
Loading

0 comments on commit b26af0b

Please sign in to comment.