From 544da95c4b91f440738aba30223da6827652e68e Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Mon, 18 Sep 2023 17:37:31 +0200 Subject: [PATCH] feat(lint/noUselessElse): add rule --- CHANGELOG.md | 4 + .../src/categories.rs | 3 +- .../biome_js_analyze/src/analyzers/nursery.rs | 2 + .../src/analyzers/nursery/no_useless_else.rs | 195 +++++++ .../nursery/use_is_array.rs | 2 +- .../specs/nursery/noUselessElse/invalid.js | 94 ++++ .../nursery/noUselessElse/invalid.js.snap | 501 ++++++++++++++++++ .../specs/nursery/noUselessElse/missed.js | 53 ++ .../nursery/noUselessElse/missed.js.snap | 63 +++ .../specs/nursery/noUselessElse/valid.js | 77 +++ .../specs/nursery/noUselessElse/valid.js.snap | 87 +++ crates/biome_rowan/src/ast/mod.rs | 6 +- .../src/configuration/linter/rules.rs | 69 ++- .../src/configuration/parse/json/rules.rs | 24 + editors/vscode/configuration_schema.json | 7 + .../@biomejs/backend-jsonrpc/src/workspace.ts | 7 +- .../@biomejs/biome/configuration_schema.json | 7 + .../components/generated/NumberOfRules.astro | 2 +- .../src/content/docs/internals/changelog.mdx | 4 + .../src/content/docs/linter/rules/index.mdx | 2 + .../docs/linter/rules/no-useless-else.md | 184 +++++++ 21 files changed, 1361 insertions(+), 32 deletions(-) create mode 100644 crates/biome_js_analyze/src/analyzers/nursery/no_useless_else.rs create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js.snap create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js.snap create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js create mode 100644 crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js.snap create mode 100644 website/src/content/docs/linter/rules/no-useless-else.md diff --git a/CHANGELOG.md b/CHANGELOG.md index c59971b14b3b..b1882085b313 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Add [noMisleadingInstantiator](https://biomejs.dev/linter/rules/no-mileading-instantiator) rule. The rule reports the misleading use of the `new` and `constructor` methods. Contributed by @unvalley +- Add [noUselessElse](https://biomejs.dev/linter/rules/no-useless-else) rule. + The rule reports `else` clauses that can be omitted because their `if` branches break. + Contributed by @Conaclos + #### Bug fixes - Fix [#294](https://github.com/biomejs/biome/issues/294). [noConfusingVoidType](https://biomejs.dev/linter/rules/no-confusing-void-type/) no longer reports false positives for return types. Contributed by @b4s36t4 diff --git a/crates/biome_diagnostics_categories/src/categories.rs b/crates/biome_diagnostics_categories/src/categories.rs index a2ef48c296e2..62a255622556 100644 --- a/crates/biome_diagnostics_categories/src/categories.rs +++ b/crates/biome_diagnostics_categories/src/categories.rs @@ -27,7 +27,6 @@ define_categories! { "lint/a11y/noSvgWithoutTitle": "https://biomejs.dev/linter/rules/no-svg-without-title", "lint/a11y/useAltText": "https://biomejs.dev/linter/rules/use-alt-text", "lint/a11y/useAnchorContent": "https://biomejs.dev/linter/rules/use-anchor-content", - "lint/a11y/useValidAriaValues": "https://biomejs.dev/linter/rules/use-valid-aria-values", "lint/a11y/useAriaPropsForRole": "https://biomejs.dev/linter/rules/use-aria-props-for-role", "lint/a11y/useButtonType": "https://biomejs.dev/linter/rules/use-button-type", "lint/a11y/useHeadingContent": "https://biomejs.dev/linter/rules/use-heading-content", @@ -38,6 +37,7 @@ define_categories! { "lint/a11y/useMediaCaption": "https://biomejs.dev/linter/rules/use-media-caption", "lint/a11y/useValidAnchor": "https://biomejs.dev/linter/rules/use-valid-anchor", "lint/a11y/useValidAriaProps": "https://biomejs.dev/linter/rules/use-valid-aria-props", + "lint/a11y/useValidAriaValues": "https://biomejs.dev/linter/rules/use-valid-aria-values", "lint/a11y/useValidLang": "https://biomejs.dev/linter/rules/use-valid-lang", "lint/complexity/noBannedTypes": "https://biomejs.dev/linter/rules/no-banned-types", "lint/complexity/noExtraBooleanCast": "https://biomejs.dev/linter/rules/no-extra-boolean-cast", @@ -96,6 +96,7 @@ define_categories! { "lint/nursery/noGlobalIsFinite": "https://biomejs.dev/linter/rules/no-global-is-finite", "lint/nursery/noGlobalIsNan": "https://biomejs.dev/linter/rules/no-global-is-nan", "lint/nursery/noMisleadingInstantiator": "https://biomejs.dev/linter/rules/no-misleading-instantiator", + "lint/nursery/noUselessElse": "https://biomejs.dev/lint/rules/no-useless-else", "lint/nursery/noVoid": "https://biomejs.dev/linter/rules/no-void", "lint/nursery/useArrowFunction": "https://biomejs.dev/linter/rules/use-arrow-function", "lint/nursery/useBiomeSuppressionComment": "https://biomejs.dev/lint/rules/use-biome-suppression-comment", diff --git a/crates/biome_js_analyze/src/analyzers/nursery.rs b/crates/biome_js_analyze/src/analyzers/nursery.rs index 63dbe9e17163..75c63bc88608 100644 --- a/crates/biome_js_analyze/src/analyzers/nursery.rs +++ b/crates/biome_js_analyze/src/analyzers/nursery.rs @@ -6,6 +6,7 @@ pub(crate) mod no_confusing_void_type; pub(crate) mod no_excessive_complexity; pub(crate) mod no_fallthrough_switch_clause; pub(crate) mod no_misleading_instantiator; +pub(crate) mod no_useless_else; pub(crate) mod no_void; pub(crate) mod use_arrow_function; pub(crate) mod use_collapsed_else_if; @@ -20,6 +21,7 @@ declare_group! { self :: no_excessive_complexity :: NoExcessiveComplexity , self :: no_fallthrough_switch_clause :: NoFallthroughSwitchClause , self :: no_misleading_instantiator :: NoMisleadingInstantiator , + self :: no_useless_else :: NoUselessElse , self :: no_void :: NoVoid , self :: use_arrow_function :: UseArrowFunction , self :: use_collapsed_else_if :: UseCollapsedElseIf , diff --git a/crates/biome_js_analyze/src/analyzers/nursery/no_useless_else.rs b/crates/biome_js_analyze/src/analyzers/nursery/no_useless_else.rs new file mode 100644 index 000000000000..b5717c8c8010 --- /dev/null +++ b/crates/biome_js_analyze/src/analyzers/nursery/no_useless_else.rs @@ -0,0 +1,195 @@ +use biome_analyze::{ + context::RuleContext, declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic, +}; +use biome_console::markup; +use biome_diagnostics::Applicability; +use biome_js_factory::make; +use biome_js_syntax::{AnyJsStatement, JsElseClause, JsIfStatement, JsStatementList}; +use biome_rowan::{AstNode, AstNodeList, BatchMutationExt}; + +use crate::JsRuleAction; + +declare_rule! { + /// Disallow `else` block when the `if` block breaks early. + /// + /// If an `if` block breaks early using a breaking statement (`return`, `break`, `continue`, or `throw`), + /// then the `else` block becomes useless. + /// Its contents can be placed outside of the block. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// while (x > 0) { + /// if (f(x)) { + /// break; + /// } else { + /// x++ + /// } + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// function f() { + /// if (x < 0) { + /// return 0; + /// } else { + /// return x; + /// } + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// function f() { + /// if (x < 0) { + /// throw new RangeError(); + /// } else { + /// return x; + /// } + /// } + /// ``` + /// + /// ## Valid + /// + /// ```js + /// function f() { + /// if (x < 0) { + /// return 0; + /// } + /// return x; + /// } + /// ``` + pub(crate) NoUselessElse { + version: "next", + name: "noUselessElse", + recommended: true, + } +} + +impl Rule for NoUselessElse { + type Query = Ast; + type State = (); + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let else_clause = ctx.query(); + let if_stmt = else_clause.parent::()?; + let mut stmt_stack = vec![(if_stmt.consequent().ok()?, ScopeMetadata::default())]; + while let Some((stmt, metadata)) = stmt_stack.pop() { + match stmt { + AnyJsStatement::JsBlockStatement(block_stmt) => { + let Some(last) = block_stmt.statements().iter().last() else { + // empty statement block + return None; + }; + stmt_stack.push((last, metadata)); + } + AnyJsStatement::JsBreakStatement(_) => { + if metadata.is_breakable { + // We are inside a breakable structure (switch statement) + // that we saw in a previous iteration. + return None; + } + } + AnyJsStatement::JsContinueStatement(_) + | AnyJsStatement::JsReturnStatement(_) + | AnyJsStatement::JsThrowStatement(_) => {} + AnyJsStatement::JsIfStatement(if_stmt) => { + let Some(else_clause) = if_stmt.else_clause() else { + // No else clause + return None; + }; + stmt_stack.push((if_stmt.consequent().ok()?, metadata)); + stmt_stack.push((else_clause.alternate().ok()?, metadata)); + } + AnyJsStatement::JsSwitchStatement(switch_stmt) => { + // To simplify, We do not take fallthoughs into account. + // Thus, this can miss some useless else. + let cases = switch_stmt.cases(); + let Some(last_case) = cases.last() else { + // Empty switch + return None; + }; + if last_case.consequent().is_empty() { + return None; + } + for switch_clause in cases.iter() { + if let Some(last) = switch_clause.consequent().last() { + stmt_stack.push((last, ScopeMetadata { is_breakable: true })); + } + } + } + _ => { + // labeled statements, loops, try-catch, with statement, and others + return None; + } + } + } + Some(()) + } + + fn diagnostic(ctx: &RuleContext, _: &Self::State) -> Option { + let else_clause = ctx.query(); + Some( + RuleDiagnostic::new( + rule_category!(), + else_clause.range(), + markup! { + "This ""else"" clause can be omitted." + }, + ) + .detail( + else_clause.syntax().parent()?.text_trimmed_range(), + markup! { + "This ""if"" statement uses an early breaking statement." + }, + ), + ) + } + + fn action(ctx: &RuleContext, _: &Self::State) -> Option { + let else_clause = ctx.query(); + let if_stmt = else_clause.parent::()?; + if let Some(stmts_list) = JsStatementList::cast(if_stmt.syntax().parent()?) { + let else_alternative = else_clause.alternate().ok()?; + let if_pos = stmts_list + .iter() + .position(|x| x.syntax() == if_stmt.syntax())?; + let new_if_stmt = AnyJsStatement::JsIfStatement(if_stmt.clone().with_else_clause(None)) + .with_trailing_trivia_pieces([])?; + let prev_stmts = stmts_list.iter().take(if_pos).chain([new_if_stmt]); + let next_stmts = stmts_list.iter().skip(if_pos + 1); + // We collect the statements because `chain` is not able to produce an `ExactSizeIterator`. + let new_stmts: Vec<_> = + if let AnyJsStatement::JsBlockStatement(else_block_stmts) = else_alternative { + prev_stmts + .chain(else_block_stmts.statements().iter()) + .chain(next_stmts) + .collect() + } else { + prev_stmts + .chain([else_alternative]) + .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::MaybeIncorrect, + message: markup! { "Omit the ""else"" clause." }.to_owned(), + mutation, + }); + } + None + } +} + +#[derive(Debug, Copy, Clone, Default)] +struct ScopeMetadata { + // We are inside a breakable structure + is_breakable: bool, +} diff --git a/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_is_array.rs b/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_is_array.rs index 798722d0ad00..4fb92bdc1d96 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_is_array.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/nursery/use_is_array.rs @@ -85,7 +85,7 @@ impl Rule for UseIsArray { make::token(T![.]), make::js_name(make::ident("isArray")).into(), ); - let arg = AnyJsCallArgument::AnyJsExpression(node.left().ok()?.trim()?); + let arg = AnyJsCallArgument::AnyJsExpression(node.left().ok()?.trim_trivia()?); let instanceof_trailing_trivia = node.instanceof_token().ok()?.trailing_trivia().pieces(); let args = make::js_call_arguments( make::token(T!['(']).with_trailing_trivia_pieces(trim_leading_trivia_pieces( diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js new file mode 100644 index 000000000000..764661f0ddf3 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js @@ -0,0 +1,94 @@ +function f (x) { + if (x < 0) { + throw new RangeError(); + } else { + return x; + } +} + +function f (x) { + f(); + if (x < 0) { + throw new RangeError(); + } else { + g(); + } + h(); +} + +function f (x) { + if (x < 0) { + throw new RangeError(); + } else return x; +} + +function f (x) { + if (x < 0) + throw new RangeError(); + else + return x; +} + +function f (x) { + if (x < 0) { + throw new RangeError(); + } else if (x === 0) { + return 1; + } else { + return x; + } +} + +function f (x) { + while (true) { + if (x < 0) { + break; + } else { + x -= g(x) + } + } + return x; +} + +function f (x) { + while (true) { + if (x < 0) { + break; + } else { + x -= g(x) + } + } + return x; +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + case 1: + return 0; + default: + return x; + } + } else { + return x; + } +} + +function f (x) { // 0 + // 1 + if (x > 0 && x < 5) { + switch (x) { + case 0: + case 1: + return 0; + default: + return x; + } + } /*a*/ else /*b*/ { // c + // d + return x; // e + // f + } // g + // 2 +} // 3 diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js.snap new file mode 100644 index 000000000000..b200db6a85e9 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/invalid.js.snap @@ -0,0 +1,501 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```js +function f (x) { + if (x < 0) { + throw new RangeError(); + } else { + return x; + } +} + +function f (x) { + f(); + if (x < 0) { + throw new RangeError(); + } else { + g(); + } + h(); +} + +function f (x) { + if (x < 0) { + throw new RangeError(); + } else return x; +} + +function f (x) { + if (x < 0) + throw new RangeError(); + else + return x; +} + +function f (x) { + if (x < 0) { + throw new RangeError(); + } else if (x === 0) { + return 1; + } else { + return x; + } +} + +function f (x) { + while (true) { + if (x < 0) { + break; + } else { + x -= g(x) + } + } + return x; +} + +function f (x) { + while (true) { + if (x < 0) { + break; + } else { + x -= g(x) + } + } + return x; +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + case 1: + return 0; + default: + return x; + } + } else { + return x; + } +} + +function f (x) { // 0 + // 1 + if (x > 0 && x < 5) { + switch (x) { + case 0: + case 1: + return 0; + default: + return x; + } + } /*a*/ else /*b*/ { // c + // d + return x; // e + // f + } // g + // 2 +} // 3 + +``` + +# Diagnostics +``` +invalid.js:4:7 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 2 │ if (x < 0) { + 3 │ throw new RangeError(); + > 4 │ } else { + │ ^^^^^^ + > 5 │ return x; + > 6 │ } + │ ^ + 7 │ } + 8 │ + + i This if statement uses an early breaking statement. + + 1 │ function f (x) { + > 2 │ if (x < 0) { + │ ^^^^^^^^^^^^ + > 3 │ throw new RangeError(); + > 4 │ } else { + > 5 │ return x; + > 6 │ } + │ ^ + 7 │ } + 8 │ + + i Suggested fix: Omit the else clause. + + 2 2 │ if (x < 0) { + 3 3 │ throw new RangeError(); + 4 │ - ····}·else·{ + 4 │ + ····} + 5 5 │ return x; + 6 │ - ····} + 7 6 │ } + 8 7 │ + + +``` + +``` +invalid.js:13:7 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 11 │ if (x < 0) { + 12 │ throw new RangeError(); + > 13 │ } else { + │ ^^^^^^ + > 14 │ g(); + > 15 │ } + │ ^ + 16 │ h(); + 17 │ } + + i This if statement uses an early breaking statement. + + 9 │ function f (x) { + 10 │ f(); + > 11 │ if (x < 0) { + │ ^^^^^^^^^^^^ + > 12 │ throw new RangeError(); + > 13 │ } else { + > 14 │ g(); + > 15 │ } + │ ^ + 16 │ h(); + 17 │ } + + i Suggested fix: Omit the else clause. + + 11 11 │ if (x < 0) { + 12 12 │ throw new RangeError(); + 13 │ - ····}·else·{ + 13 │ + ····} + 14 14 │ g(); + 15 │ - ····} + 16 15 │ h(); + 17 16 │ } + + +``` + +``` +invalid.js:22:7 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 20 │ if (x < 0) { + 21 │ throw new RangeError(); + > 22 │ } else return x; + │ ^^^^^^^^^^^^^^ + 23 │ } + 24 │ + + i This if statement uses an early breaking statement. + + 19 │ function f (x) { + > 20 │ if (x < 0) { + │ ^^^^^^^^^^^^ + > 21 │ throw new RangeError(); + > 22 │ } else return x; + │ ^^^^^^^^^^^^^^^^ + 23 │ } + 24 │ + + i Suggested fix: Omit the else clause. + + 22 │ ····}·else·return·x; + │ ------ + +``` + +``` +invalid.js:28:5 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 26 │ if (x < 0) + 27 │ throw new RangeError(); + > 28 │ else + │ ^^^^ + > 29 │ return x; + │ ^^^^^^^^^ + 30 │ } + 31 │ + + i This if statement uses an early breaking statement. + + 25 │ function f (x) { + > 26 │ if (x < 0) + │ ^^^^^^^^^^ + > 27 │ throw new RangeError(); + > 28 │ else + > 29 │ return x; + │ ^^^^^^^^^ + 30 │ } + 31 │ + + i Suggested fix: Omit the else clause. + + 26 26 │ if (x < 0) + 27 27 │ throw new RangeError(); + 28 │ - ····else + 29 28 │ return x; + 30 29 │ } + + +``` + +``` +invalid.js:35:7 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 33 │ if (x < 0) { + 34 │ throw new RangeError(); + > 35 │ } else if (x === 0) { + │ ^^^^^^^^^^^^^^^^^^^ + > 36 │ return 1; + > 37 │ } else { + > 38 │ return x; + > 39 │ } + │ ^ + 40 │ } + 41 │ + + i This if statement uses an early breaking statement. + + 32 │ function f (x) { + > 33 │ if (x < 0) { + │ ^^^^^^^^^^^^ + > 34 │ throw new RangeError(); + ... + > 38 │ return x; + > 39 │ } + │ ^ + 40 │ } + 41 │ + + i Suggested fix: Omit the else clause. + + 35 │ ····}·else·if·(x·===·0)·{ + │ ------ + +``` + +``` +invalid.js:37:7 lint/nursery/noUselessElse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 35 │ } else if (x === 0) { + 36 │ return 1; + > 37 │ } else { + │ ^^^^^^ + > 38 │ return x; + > 39 │ } + │ ^ + 40 │ } + 41 │ + + i This if statement uses an early breaking statement. + + 33 │ if (x < 0) { + 34 │ throw new RangeError(); + > 35 │ } else if (x === 0) { + │ ^^^^^^^^^^^^^^ + > 36 │ return 1; + > 37 │ } else { + > 38 │ return x; + > 39 │ } + │ ^ + 40 │ } + 41 │ + + +``` + +``` +invalid.js:46:11 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 44 │ if (x < 0) { + 45 │ break; + > 46 │ } else { + │ ^^^^^^ + > 47 │ x -= g(x) + > 48 │ } + │ ^ + 49 │ } + 50 │ return x; + + i This if statement uses an early breaking statement. + + 42 │ function f (x) { + 43 │ while (true) { + > 44 │ if (x < 0) { + │ ^^^^^^^^^^^^ + > 45 │ break; + > 46 │ } else { + > 47 │ x -= g(x) + > 48 │ } + │ ^ + 49 │ } + 50 │ return x; + + i Suggested fix: Omit the else clause. + + 44 44 │ if (x < 0) { + 45 45 │ break; + 46 │ - ········}·else·{ + 46 │ + ········} + 47 47 │ x -= g(x) + 48 │ - ········} + 49 48 │ } + 50 49 │ return x; + + +``` + +``` +invalid.js:57:11 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 55 │ if (x < 0) { + 56 │ break; + > 57 │ } else { + │ ^^^^^^ + > 58 │ x -= g(x) + > 59 │ } + │ ^ + 60 │ } + 61 │ return x; + + i This if statement uses an early breaking statement. + + 53 │ function f (x) { + 54 │ while (true) { + > 55 │ if (x < 0) { + │ ^^^^^^^^^^^^ + > 56 │ break; + > 57 │ } else { + > 58 │ x -= g(x) + > 59 │ } + │ ^ + 60 │ } + 61 │ return x; + + i Suggested fix: Omit the else clause. + + 55 55 │ if (x < 0) { + 56 56 │ break; + 57 │ - ········}·else·{ + 57 │ + ········} + 58 58 │ x -= g(x) + 59 │ - ········} + 60 59 │ } + 61 60 │ return x; + + +``` + +``` +invalid.js:73:7 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 71 │ return x; + 72 │ } + > 73 │ } else { + │ ^^^^^^ + > 74 │ return x; + > 75 │ } + │ ^ + 76 │ } + 77 │ + + i This if statement uses an early breaking statement. + + 64 │ function f (x) { + > 65 │ if (x > 0 && x < 5) { + │ ^^^^^^^^^^^^^^^^^^^^^ + > 66 │ switch (x) { + ... + > 74 │ return x; + > 75 │ } + │ ^ + 76 │ } + 77 │ + + i Suggested fix: Omit the else clause. + + 71 71 │ return x; + 72 72 │ } + 73 │ - ····}·else·{ + 73 │ + ····} + 74 74 │ return x; + 75 │ - ····} + 76 75 │ } + 77 76 │ + + +``` + +``` +invalid.js:88:13 lint/nursery/noUselessElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This else clause can be omitted. + + 86 │ return x; + 87 │ } + > 88 │ } /*a*/ else /*b*/ { // c + │ ^^^^^^^^^^^^^^^^^ + > 89 │ // d + > 90 │ return x; // e + > 91 │ // f + > 92 │ } // g + │ ^ + 93 │ // 2 + 94 │ } // 3 + + i This if statement uses an early breaking statement. + + 78 │ function f (x) { // 0 + 79 │ // 1 + > 80 │ if (x > 0 && x < 5) { + │ ^^^^^^^^^^^^^^^^^^^^^ + > 81 │ switch (x) { + ... + > 91 │ // f + > 92 │ } // g + │ ^ + 93 │ // 2 + 94 │ } // 3 + + i Suggested fix: Omit the else clause. + + 86 86 │ return x; + 87 87 │ } + 88 │ - ····}·/*a*/·else·/*b*/·{·//·c + 88 │ + ····} + 89 89 │ // d + 90 90 │ return x; // e + 91 │ - ········//·f + 92 │ - ····}·//·g + 93 91 │ // 2 + 94 92 │ } // 3 + + +``` + + diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js new file mode 100644 index 000000000000..d1094a14f586 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js @@ -0,0 +1,53 @@ +// Examples which are not reported by the rule (false negative cases) + +function f (x) { + if (x < 0) { + while(true) { + return 0; + } + } else { + return x; + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + f(); // fallthough + default: + return 0; + } + } else { + return x; + } +} + + +function f (x) { + if (x > 0 && x < 5) { + try { + g(x); + } catch { + g(0); + } finally { + return 0; + } + } else { + return x; + } +} + + +function f (x) { + if (x > 0 && x < 5) { + try { + g(x); + return x; + } catch { + return 0; + } + } else { + return x; + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js.snap new file mode 100644 index 000000000000..815b49732e11 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/missed.js.snap @@ -0,0 +1,63 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: missed.js +--- +# Input +```js +// Examples which are not reported by the rule (false negative cases) + +function f (x) { + if (x < 0) { + while(true) { + return 0; + } + } else { + return x; + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + f(); // fallthough + default: + return 0; + } + } else { + return x; + } +} + + +function f (x) { + if (x > 0 && x < 5) { + try { + g(x); + } catch { + g(0); + } finally { + return 0; + } + } else { + return x; + } +} + + +function f (x) { + if (x > 0 && x < 5) { + try { + g(x); + return x; + } catch { + return 0; + } + } else { + return x; + } +} + +``` + + diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js new file mode 100644 index 000000000000..18527e4e9e76 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js @@ -0,0 +1,77 @@ +function f(x) { + if (x < 0) { + + } else { + return 0 + } +} + +function f(x) { + if (x < 0) { + g(); + } else { + return 0 + } +} + +function f(x) { + if (x < 0) { + g(); + if(x === -1) { + return 1; + } + } else { + return 0 + } +} + +function f(x) { + if (x < 0) { + throw new RangeError(); + } + return x; +} + +function f(x) { + if (x < 0) { + while(x < 0) { + x += f(x); + } + } else { + return 0 + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) {} + } else { + return x; + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + case 1: + return 0; + default: + } + } else { + return x; + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + break; + default: + break; + } + } else { + return x; + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js.snap new file mode 100644 index 000000000000..7ac1f81620a4 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noUselessElse/valid.js.snap @@ -0,0 +1,87 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```js +function f(x) { + if (x < 0) { + + } else { + return 0 + } +} + +function f(x) { + if (x < 0) { + g(); + } else { + return 0 + } +} + +function f(x) { + if (x < 0) { + g(); + if(x === -1) { + return 1; + } + } else { + return 0 + } +} + +function f(x) { + if (x < 0) { + throw new RangeError(); + } + return x; +} + +function f(x) { + if (x < 0) { + while(x < 0) { + x += f(x); + } + } else { + return 0 + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) {} + } else { + return x; + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + case 1: + return 0; + default: + } + } else { + return x; + } +} + +function f (x) { + if (x > 0 && x < 5) { + switch (x) { + case 0: + break; + default: + break; + } + } else { + return x; + } +} + +``` + + diff --git a/crates/biome_rowan/src/ast/mod.rs b/crates/biome_rowan/src/ast/mod.rs index 04375935095f..6def1b4306bc 100644 --- a/crates/biome_rowan/src/ast/mod.rs +++ b/crates/biome_rowan/src/ast/mod.rs @@ -292,7 +292,7 @@ pub trait AstNode: Clone { } /// Return a new version of this node without leading and trailing newlines and whitespaces. - fn trim(self) -> Option { + fn trim_trivia(self) -> Option { Self::cast( self.into_syntax() .trim_leading_trivia()? @@ -301,12 +301,12 @@ pub trait AstNode: Clone { } /// Return a new version of this node without leading newlines and whitespaces. - fn trim_start(self) -> Option { + fn trim_leading_trivia(self) -> Option { Self::cast(self.into_syntax().trim_leading_trivia()?) } /// Return a new version of this node without trailing newlines and whitespaces. - fn trim_end(self) -> Option { + fn trim_trailing_trivia(self) -> Option { Self::cast(self.into_syntax().trim_trailing_trivia()?) } } diff --git a/crates/biome_service/src/configuration/linter/rules.rs b/crates/biome_service/src/configuration/linter/rules.rs index b2137b9a56c7..8478a53a25d7 100644 --- a/crates/biome_service/src/configuration/linter/rules.rs +++ b/crates/biome_service/src/configuration/linter/rules.rs @@ -2210,6 +2210,10 @@ pub struct Nursery { )] #[serde(skip_serializing_if = "Option::is_none")] pub no_misleading_instantiator: Option, + #[doc = "Disallow else block when the if block breaks early."] + #[bpaf(long("no-useless-else"), argument("on|off|warn"), optional, hide)] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_useless_else: Option, #[doc = "Disallow the use of void operators, which is not a familiar operator."] #[bpaf(long("no-void"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] @@ -2260,7 +2264,7 @@ pub struct Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 16] = [ + pub(crate) const GROUP_RULES: [&'static str; 17] = [ "noAccumulatingSpread", "noConfusingVoidType", "noDuplicateJsonKeys", @@ -2269,6 +2273,7 @@ impl Nursery { "noGlobalIsFinite", "noGlobalIsNan", "noMisleadingInstantiator", + "noUselessElse", "noVoid", "useArrowFunction", "useCollapsedElseIf", @@ -2278,27 +2283,29 @@ impl Nursery { "useImportRestrictions", "useIsArray", ]; - const RECOMMENDED_RULES: [&'static str; 8] = [ + const RECOMMENDED_RULES: [&'static str; 9] = [ "noDuplicateJsonKeys", "noGlobalIsFinite", "noGlobalIsNan", "noMisleadingInstantiator", + "noUselessElse", "useArrowFunction", "useExhaustiveDependencies", "useGroupedTypeImport", "useIsArray", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 8] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 9] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 16] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 17] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), @@ -2315,6 +2322,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { @@ -2371,46 +2379,51 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.no_void.as_ref() { + if let Some(rule) = self.no_useless_else.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.use_arrow_function.as_ref() { + if let Some(rule) = self.no_void.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.use_collapsed_else_if.as_ref() { + if let Some(rule) = self.use_arrow_function.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { + if let Some(rule) = self.use_collapsed_else_if.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.use_hook_at_top_level.as_ref() { + if let Some(rule) = self.use_grouped_type_import.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.use_hook_at_top_level.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_is_array.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } + if let Some(rule) = self.use_is_array.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2455,46 +2468,51 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.no_void.as_ref() { + if let Some(rule) = self.no_useless_else.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.use_arrow_function.as_ref() { + if let Some(rule) = self.no_void.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.use_collapsed_else_if.as_ref() { + if let Some(rule) = self.use_arrow_function.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { + if let Some(rule) = self.use_collapsed_else_if.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.use_hook_at_top_level.as_ref() { + if let Some(rule) = self.use_grouped_type_import.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.use_hook_at_top_level.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_is_array.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } + if let Some(rule) = self.use_is_array.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2505,10 +2523,10 @@ impl Nursery { pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) } - pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 8] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 9] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 16] { + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 17] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] @@ -2539,6 +2557,7 @@ impl Nursery { "noGlobalIsFinite" => self.no_global_is_finite.as_ref(), "noGlobalIsNan" => self.no_global_is_nan.as_ref(), "noMisleadingInstantiator" => self.no_misleading_instantiator.as_ref(), + "noUselessElse" => self.no_useless_else.as_ref(), "noVoid" => self.no_void.as_ref(), "useArrowFunction" => self.use_arrow_function.as_ref(), "useCollapsedElseIf" => self.use_collapsed_else_if.as_ref(), diff --git a/crates/biome_service/src/configuration/parse/json/rules.rs b/crates/biome_service/src/configuration/parse/json/rules.rs index 202831289098..1aaa62af3c0a 100644 --- a/crates/biome_service/src/configuration/parse/json/rules.rs +++ b/crates/biome_service/src/configuration/parse/json/rules.rs @@ -2031,6 +2031,7 @@ impl VisitNode for Nursery { "noGlobalIsFinite", "noGlobalIsNan", "noMisleadingInstantiator", + "noUselessElse", "noVoid", "useArrowFunction", "useCollapsedElseIf", @@ -2242,6 +2243,29 @@ impl VisitNode for Nursery { )); } }, + "noUselessElse" => match value { + AnyJsonValue::JsonStringValue(_) => { + let mut configuration = RuleConfiguration::default(); + self.map_to_known_string(&value, name_text, &mut configuration, diagnostics)?; + self.no_useless_else = Some(configuration); + } + AnyJsonValue::JsonObjectValue(_) => { + let mut rule_configuration = RuleConfiguration::default(); + rule_configuration.map_rule_configuration( + &value, + name_text, + "noUselessElse", + diagnostics, + )?; + self.no_useless_else = Some(rule_configuration); + } + _ => { + diagnostics.push(DeserializationDiagnostic::new_incorrect_type( + "object or string", + value.range(), + )); + } + }, "noVoid" => match value { AnyJsonValue::JsonStringValue(_) => { let mut configuration = RuleConfiguration::default(); diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 20edce3c8c2e..7aaa71cac330 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -1019,6 +1019,13 @@ { "type": "null" } ] }, + "noUselessElse": { + "description": "Disallow else block when the if block breaks early.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noVoid": { "description": "Disallow the use of void operators, which is not a familiar operator.", "anyOf": [ diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index b20c7f2b2dda..dfcf6152b6db 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -681,6 +681,10 @@ export interface Nursery { * Enforce proper usage of new and constructor. */ noMisleadingInstantiator?: RuleConfiguration; + /** + * Disallow else block when the if block breaks early. + */ + noUselessElse?: RuleConfiguration; /** * Disallow the use of void operators, which is not a familiar operator. */ @@ -1206,7 +1210,6 @@ export type Category = | "lint/a11y/noSvgWithoutTitle" | "lint/a11y/useAltText" | "lint/a11y/useAnchorContent" - | "lint/a11y/useValidAriaValues" | "lint/a11y/useAriaPropsForRole" | "lint/a11y/useButtonType" | "lint/a11y/useHeadingContent" @@ -1217,6 +1220,7 @@ export type Category = | "lint/a11y/useMediaCaption" | "lint/a11y/useValidAnchor" | "lint/a11y/useValidAriaProps" + | "lint/a11y/useValidAriaValues" | "lint/a11y/useValidLang" | "lint/complexity/noBannedTypes" | "lint/complexity/noExtraBooleanCast" @@ -1275,6 +1279,7 @@ export type Category = | "lint/nursery/noGlobalIsFinite" | "lint/nursery/noGlobalIsNan" | "lint/nursery/noMisleadingInstantiator" + | "lint/nursery/noUselessElse" | "lint/nursery/noVoid" | "lint/nursery/useArrowFunction" | "lint/nursery/useBiomeSuppressionComment" diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 20edce3c8c2e..7aaa71cac330 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -1019,6 +1019,13 @@ { "type": "null" } ] }, + "noUselessElse": { + "description": "Disallow else block when the if block breaks early.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noVoid": { "description": "Disallow the use of void operators, which is not a familiar operator.", "anyOf": [ diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index 79ddcc67dde3..162acf325203 100644 --- a/website/src/components/generated/NumberOfRules.astro +++ b/website/src/components/generated/NumberOfRules.astro @@ -1,2 +1,2 @@ -

Biome's linter has a total of 158 rules

\ No newline at end of file +

Biome's linter has a total of 159 rules

\ No newline at end of file diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 363f0e7f683b..d4a9c7bc58c6 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -42,6 +42,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Add [noMisleadingInstantiator](https://biomejs.dev/linter/rules/no-mileading-instantiator) rule. The rule reports the misleading use of the `new` and `constructor` methods. Contributed by @unvalley +- Add [noUselessElse](https://biomejs.dev/linter/rules/no-useless-else) rule. + The rule reports `else` clauses that can be omitted because their `if` branches break. + Contributed by @Conaclos + #### Bug fixes - Fix [#294](https://github.com/biomejs/biome/issues/294). [noConfusingVoidType](https://biomejs.dev/linter/rules/no-confusing-void-type/) no longer reports false positives for return types. Contributed by @b4s36t4 diff --git a/website/src/content/docs/linter/rules/index.mdx b/website/src/content/docs/linter/rules/index.mdx index cc1be0146d13..0e836335c99e 100644 --- a/website/src/content/docs/linter/rules/index.mdx +++ b/website/src/content/docs/linter/rules/index.mdx @@ -354,6 +354,8 @@ Use Number.isFinite instead of global isFinite. Use Number.isNaN instead of global isNaN. ### [noMisleadingInstantiator](/linter/rules/no-misleading-instantiator) Enforce proper usage of new and constructor. +### [noUselessElse](/linter/rules/no-useless-else) +Disallow else block when the if block breaks early. ### [noVoid](/linter/rules/no-void) Disallow the use of void operators, which is not a familiar operator. ### [useArrowFunction](/linter/rules/use-arrow-function) diff --git a/website/src/content/docs/linter/rules/no-useless-else.md b/website/src/content/docs/linter/rules/no-useless-else.md new file mode 100644 index 000000000000..b23b19e9053c --- /dev/null +++ b/website/src/content/docs/linter/rules/no-useless-else.md @@ -0,0 +1,184 @@ +--- +title: noUselessElse (since vnext) +--- + + +:::caution +This rule is part of the [nursery](/linter/rules/#nursery) group. +::: + +Disallow `else` block when the `if` block breaks early. + +If an `if` block breaks early using a breaking statement (`return`, `break`, `continue`, or `throw`), +then the `else` block becomes useless. +Its contents can be placed outside of the block. + +## Examples + +### Invalid + +```jsx +while (x > 0) { + if (f(x)) { + break; + } else { + x++ + } +} +``` + +

nursery/noUselessElse.js:4:7 lint/nursery/noUselessElse  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ This else clause can be omitted.
+  
+    2 │     if (f(x)) {
+    3 │         break;
+  > 4 │     } else {
+      │       ^^^^^^
+  > 5 │         x++
+  > 6 │     }
+      │     ^
+    7 │ }
+    8 │ 
+  
+  ℹ This if statement uses an early breaking statement.
+  
+    1 │ while (x > 0) {
+  > 2 │     if (f(x)) {
+      │     ^^^^^^^^^^^
+  > 3 │         break;
+  > 4 │     } else {
+  > 5 │         x++
+  > 6 │     }
+      │     ^
+    7 │ }
+    8 │ 
+  
+  ℹ Suggested fix: Omit the else clause.
+  
+    2 2 │       if (f(x)) {
+    3 3 │           break;
+    4   │ - ····}·else·{
+      4 │ + ····}
+    5 5 │           x++
+    6   │ - ····}
+    7 6 │   }
+    8 7 │   
+  
+
+ +```jsx +function f() { + if (x < 0) { + return 0; + } else { + return x; + } +} +``` + +
nursery/noUselessElse.js:4:7 lint/nursery/noUselessElse  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ This else clause can be omitted.
+  
+    2 │     if (x < 0) {
+    3 │         return 0;
+  > 4 │     } else {
+      │       ^^^^^^
+  > 5 │         return x;
+  > 6 │     }
+      │     ^
+    7 │ }
+    8 │ 
+  
+  ℹ This if statement uses an early breaking statement.
+  
+    1 │ function f() {
+  > 2 │     if (x < 0) {
+      │     ^^^^^^^^^^^^
+  > 3 │         return 0;
+  > 4 │     } else {
+  > 5 │         return x;
+  > 6 │     }
+      │     ^
+    7 │ }
+    8 │ 
+  
+  ℹ Suggested fix: Omit the else clause.
+  
+    2 2 │       if (x < 0) {
+    3 3 │           return 0;
+    4   │ - ····}·else·{
+      4 │ + ····}
+    5 5 │           return x;
+    6   │ - ····}
+    7 6 │   }
+    8 7 │   
+  
+
+ +```jsx +function f() { + if (x < 0) { + throw new RangeError(); + } else { + return x; + } +} +``` + +
nursery/noUselessElse.js:4:7 lint/nursery/noUselessElse  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ This else clause can be omitted.
+  
+    2 │     if (x < 0) {
+    3 │         throw new RangeError();
+  > 4 │     } else {
+      │       ^^^^^^
+  > 5 │         return x;
+  > 6 │     }
+      │     ^
+    7 │ }
+    8 │ 
+  
+  ℹ This if statement uses an early breaking statement.
+  
+    1 │ function f() {
+  > 2 │     if (x < 0) {
+      │     ^^^^^^^^^^^^
+  > 3 │         throw new RangeError();
+  > 4 │     } else {
+  > 5 │         return x;
+  > 6 │     }
+      │     ^
+    7 │ }
+    8 │ 
+  
+  ℹ Suggested fix: Omit the else clause.
+  
+    2 2 │       if (x < 0) {
+    3 3 │           throw new RangeError();
+    4   │ - ····}·else·{
+      4 │ + ····}
+    5 5 │           return x;
+    6   │ - ····}
+    7 6 │   }
+    8 7 │   
+  
+
+ +## Valid + +```jsx +function f() { + if (x < 0) { + return 0; + } + return x; +} +``` + +## Related links + +- [Disable a rule](/linter/#disable-a-lint-rule) +- [Rule options](/linter/#rule-options)