diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index f09dd7e8f3635..c989a758f51b6 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -102,6 +102,7 @@ mod eslint { pub mod no_label_var; pub mod no_labels; pub mod no_lone_blocks; + pub mod no_lonely_if; pub mod no_loss_of_precision; pub mod no_magic_numbers; pub mod no_multi_assign; @@ -571,6 +572,7 @@ oxc_macros::declare_all_lint_rules! { eslint::max_lines, eslint::max_params, eslint::new_cap, + eslint::no_lonely_if, eslint::no_useless_call, eslint::no_unneeded_ternary, eslint::no_extra_label, diff --git a/crates/oxc_linter/src/rules/eslint/no_lonely_if.rs b/crates/oxc_linter/src/rules/eslint/no_lonely_if.rs new file mode 100644 index 0000000000000..a85837c299697 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_lonely_if.rs @@ -0,0 +1,290 @@ +use crate::{AstNode, context::LintContext, rule::Rule}; +use oxc_ast::AstKind; +use oxc_ast::ast::{Statement, Statement::BlockStatement}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +fn no_lonely_if_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Unexpected `if` as the only statement in an `else` block") + .with_help("Consider using `else if` instead.") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoLonelyIf; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow `if` statements as the only statement in `else` blocks + /// + /// ### Why is this bad? + /// + /// When an `if` statement is the only statement in an `else` block, it is often clearer to use + /// an `else if` instead. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// if (condition) { + /// // ... + /// } else { + /// if (anotherCondition) { + /// // ... + /// } + /// } + /// ``` + /// + /// ```js + /// if (condition) { + /// // ... + /// } else { + /// if (anotherCondition) { + /// // ... + /// } else { + /// // ... + /// } + /// } + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// if (condition) { + /// // ... + /// } else if (anotherCondition) { + /// // ... + /// } + /// ``` + /// + /// ```js + /// if (condition) { + /// // ... + /// } else if (anotherCondition) { + /// // ... + /// } else { + /// // ... + /// } + /// ``` + /// + /// ```js + /// if (condition) { + /// // ... + /// } else { + /// if (anotherCondition) { + /// // ... + /// } + /// doSomething(); + /// } + /// ``` + NoLonelyIf, + eslint, + pedantic, + pending +); + +impl Rule for NoLonelyIf { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::IfStatement(if_stmt) = node.kind() else { + return; + }; + + let Some(ref alternate) = if_stmt.alternate else { + return; + }; + + let BlockStatement(b) = alternate else { + return; + }; + + if b.body.len() != 1 { + return; + }; + + let Some(only_stmt) = b.body.first() else { + return; + }; + + if let Some(AstKind::IfStatement(_)) = ctx.nodes().parent_kind(node.id()) { + return; + }; + + if let Statement::BlockStatement(inner_block) = only_stmt { + if inner_block.body.len() == 1 { + if let Some(Statement::IfStatement(lonely_if)) = inner_block.body.first() { + ctx.diagnostic(no_lonely_if_diagnostic(Span::sized(lonely_if.span.start, 2))); + }; + } + } else if let Statement::IfStatement(lonely_if) = only_stmt { + ctx.diagnostic(no_lonely_if_diagnostic(Span::sized(lonely_if.span.start, 2))); + }; + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "if (a) {;} else if (b) {;}", + "if (a) {;} else { if (b) {;} ; }", + "if (a) if (a) {} else { if (b) {} } else {}", + "if (a) { + if (b) {} else { } + } else { + if (c) { } + if (d) { } + }", + ]; + + let fail = vec![ + "if (a) {;} else { if (b) {;} }", + "if (foo) {} else { if (bar) baz(); }", + "if (foo) {} else { if (bar) baz() } qux();", + "if (foo) {} else { if (bar) baz(); } qux();", + "if (a) { + foo(); + } else { + /* otherwise, do the other thing */ if (b) { + bar(); + } + }", + "if (a) { + foo(); + } else { + if (b) { + bar(); + } /* this comment will prevent this test case from being autofixed. */ + }", + // No fix; removing the braces would cause a SyntaxError. + "if (foo) { + } else { + if (bar) baz(); + } + qux();", + // Not fixed; removing the braces would change the semantics due to ASI. + "if (foo) { + } else { + if (bar) baz(); + } + [1, 2, 3].forEach(foo);", + // Not fixed; removing the braces would change the semantics due to ASI. + "if (foo) { } else { + if (bar) baz++; + } + foo;", + // Not fixed; bar() would be interpreted as a template literal tag + "if (a) { + foo(); + } else { + if (b) bar(); + } + `template literal`;", + ]; + + /* Pending + let fix = vec![ + ( + "if (a) { + foo(); + } else { + if (b) { + bar(); + } + }", + "if (a) { + foo(); + } else if (b) { + bar(); + }", + None, + ), + ( + "if (a) { + foo(); + } /* comment */ + else { + if (b) { + bar(); + } + }", + "if (a) { + foo(); + } /* comment */ + else { + if (b) { + bar(); + } + }", + None, + ), + ( + "if (a) { + foo(); + } else { + if ( /* this comment is ok */ + b) { + bar(); + } + }", + "if (a) { + foo(); + } else if ( /* this comment is ok */ + b) { + bar(); + }", + None, + ), + ( + "if (foo) {} else { if (bar) baz(); }", + "if (foo) {} else if (bar) baz();", + None, + ), + ( + "if (foo) { } else { if (bar) baz(); } qux();", + "if (foo) { } else if (bar) baz(); qux();", + None, + ), + ( + "if (foo) { } else { if (bar) baz++; } foo;", + "if (foo) { } else if (bar) baz++; foo;", + None, + ), + ( + "if (a) { + foo(); + } else { + if (b) { + bar(); + } else if (c) { + baz(); + } else { + qux(); + } + }", + "if (a) { + foo(); + } else if (b) { + bar(); + } else if (c) { + baz(); + } else { + qux(); + }", + None, + ), + ("if (a) {;} else { if (b) {;} }", "if (a) {;} else if (b) {;}", None), + ("if (foo) {} else { if (bar) baz(); }", "if (foo) {} else if (bar) baz();", None), + ( + "if (foo) {} else { if (bar) baz(); } qux();", + "if (foo) {} else if (bar) baz(); qux();", + None, + ), + ]; + */ + + Tester::new(NoLonelyIf::NAME, NoLonelyIf::PLUGIN, pass, fail) + // .expect_fix(fix) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_no_lonely_if.snap b/crates/oxc_linter/src/snapshots/eslint_no_lonely_if.snap new file mode 100644 index 0000000000000..cc0559da12e79 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_lonely_if.snap @@ -0,0 +1,84 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:1:19] + 1 │ if (a) {;} else { if (b) {;} } + · ── + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:1:20] + 1 │ if (foo) {} else { if (bar) baz(); } + · ── + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:1:20] + 1 │ if (foo) {} else { if (bar) baz() } qux(); + · ── + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:1:20] + 1 │ if (foo) {} else { if (bar) baz(); } qux(); + · ── + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:4:48] + 3 │ } else { + 4 │ /* otherwise, do the other thing */ if (b) { + · ── + 5 │ bar(); + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:4:12] + 3 │ } else { + 4 │ if (b) { + · ── + 5 │ bar(); + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:3:12] + 2 │ } else { + 3 │ if (bar) baz(); + · ── + 4 │ } + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:3:12] + 2 │ } else { + 3 │ if (bar) baz(); + · ── + 4 │ } + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:2:12] + 1 │ if (foo) { } else { + 2 │ if (bar) baz++; + · ── + 3 │ } + ╰──── + help: Consider using `else if` instead. + + ⚠ eslint(no-lonely-if): Unexpected `if` as the only statement in an `else` block + ╭─[no_lonely_if.tsx:4:12] + 3 │ } else { + 4 │ if (b) bar(); + · ── + 5 │ } + ╰──── + help: Consider using `else if` instead.