diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 09c167dd7a307..34e1231e757ac 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -152,6 +152,7 @@ mod unicorn { pub mod no_empty_file; pub mod no_instanceof_array; pub mod no_invalid_remove_event_listener; + pub mod no_lonely_if; pub mod no_negated_condition; pub mod no_new_array; pub mod no_new_buffer; @@ -301,6 +302,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::no_empty_file, unicorn::no_instanceof_array, unicorn::no_invalid_remove_event_listener, + unicorn::no_lonely_if, unicorn::no_negated_condition, unicorn::no_new_array, unicorn::no_new_buffer, diff --git a/crates/oxc_linter/src/rules/unicorn/no_lonely_if.rs b/crates/oxc_linter/src/rules/unicorn/no_lonely_if.rs new file mode 100644 index 0000000000000..a6851948cd538 --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/no_lonely_if.rs @@ -0,0 +1,212 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`.")] +#[diagnostic(severity(warning), help("Move the inner `if` test to the outer `if` test."))] +struct NoLonelyIfDiagnostic(#[label] pub Span, #[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoLonelyIf; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow `if` statements as the only statement in `if` blocks without `else`. + /// + /// ### Why is this bad? + /// + /// It can be confusing to have an `if` statement without an `else` clause as the only statement in an `if` block. + /// + /// ### Example + /// ```javascript + /// // Bad + /// if (foo) { + /// if (bar) { + /// } + /// } + /// if (foo) if (bar) baz(); + /// + /// // Good + /// if (foo && bar) { + /// } + /// if (foo && bar) baz(); + /// ``` + NoLonelyIf, + pedantic +); + +impl Rule for NoLonelyIf { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::IfStatement(if_stmt) = node.kind() else { return }; + + if if_stmt.alternate.is_some() { + return; + } + + let Some(parent) = ctx.nodes().parent_node(node.id()) else { + return; + }; + + let parent_if_stmt_span = match parent.kind() { + AstKind::BlockStatement(block_stmt) => { + if block_stmt.body.len() != 1 { + return; + } + + let Some(parent) = ctx.nodes().parent_node(parent.id()) else { + return; + }; + + let AstKind::IfStatement(parent_if_stmt) = parent.kind() else { + return; + }; + + if parent_if_stmt.alternate.is_some() { + return; + } + parent_if_stmt.span + } + AstKind::IfStatement(parent_if_stmt) => { + if parent_if_stmt.alternate.is_some() { + return; + } + + parent_if_stmt.span + } + _ => return, + }; + + ctx.diagnostic(NoLonelyIfDiagnostic( + Span { start: if_stmt.span.start, end: if_stmt.span.start + 2 }, + Span { start: parent_if_stmt_span.start, end: parent_if_stmt_span.start + 2 }, + )); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r#"if (a) { if (b) { } } else { }"#, + r#"if (a) { if (b) { } foo(); } else {}"#, + r#"if (a) { } else { if (y) { } }"#, + r#"if (a) { b ? c() : d()}"#, + ]; + + let fail = vec![ + r#" + if (a) { + if (b) { + } + } + "#, + // Inner one is `BlockStatement` + r#" + if (a) if (b) { + foo(); + } + "#, + // Outer one is `BlockStatement` + r#" + if (a) { + if (b) foo(); + } + "#, + // No `BlockStatement` + r#"if (a) if (b) foo();"#, + r#" + if (a) { + if (b) foo() + } + "#, + // `EmptyStatement` + r#"if (a) if (b);"#, + // Nested + r#" + if (a) { + if (b) { + // Should not report + } + } else if (c) { + if (d) { + } + } + "#, + // Need parenthesis + r#" + function * foo() { + if (a || b) + if (a ?? b) + if (a ? b : c) + if (a = b) + if (a += b) + if (a -= b) + if (a &&= b) + if (yield a) + if (a, b); + } + "#, + // Should not add parenthesis + r#" + async function foo() { + if (a) + if (await a) + if (a.b) + if (a && b); + } + "#, + // Don't case parenthesis in outer test + // 'if (((a || b))) if (((c || d)));', + // Comments + r#" + if // 1 + ( + // 2 + a // 3 + .b // 4 + ) // 5 + { + // 6 + if ( + // 7 + c // 8 + .d // 9 + ) { + // 10 + foo(); + // 11 + } + // 12 + } + "#, + // Semicolon + r#" + if (a) { + if (b) foo() + } + [].forEach(bar) + "#, + r#" + if (a) + if (b) foo() + ;[].forEach(bar) + "#, + r#" + if (a) { + if (b) foo() + } + ;[].forEach(bar) + "#, + ]; + + Tester::new_without_config(NoLonelyIf::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_lonely_if.snap b/crates/oxc_linter/src/snapshots/no_lonely_if.snap new file mode 100644 index 0000000000000..b2aa4f3563ddf --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_lonely_if.snap @@ -0,0 +1,241 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_lonely_if +--- + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if (a) { + · ── + 3 │ if (b) { + · ── + 4 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if (a) if (b) { + · ── ── + 3 │ foo(); + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if (a) { + · ── + 3 │ if (b) foo(); + · ── + 4 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ if (a) if (b) foo(); + · ── ── + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if (a) { + · ── + 3 │ if (b) foo() + · ── + 4 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ if (a) if (b); + · ── ── + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:5:1] + 5 │ } + 6 │ } else if (c) { + · ── + 7 │ if (d) { + · ── + 8 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:2:1] + 2 │ function * foo() { + 3 │ if (a || b) + · ── + 4 │ if (a ?? b) + · ── + 5 │ if (a ? b : c) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:3:1] + 3 │ if (a || b) + 4 │ if (a ?? b) + · ── + 5 │ if (a ? b : c) + · ── + 6 │ if (a = b) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:4:1] + 4 │ if (a ?? b) + 5 │ if (a ? b : c) + · ── + 6 │ if (a = b) + · ── + 7 │ if (a += b) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:5:1] + 5 │ if (a ? b : c) + 6 │ if (a = b) + · ── + 7 │ if (a += b) + · ── + 8 │ if (a -= b) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:6:1] + 6 │ if (a = b) + 7 │ if (a += b) + · ── + 8 │ if (a -= b) + · ── + 9 │ if (a &&= b) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:7:1] + 7 │ if (a += b) + 8 │ if (a -= b) + · ── + 9 │ if (a &&= b) + · ── + 10 │ if (yield a) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:8:1] + 8 │ if (a -= b) + 9 │ if (a &&= b) + · ── + 10 │ if (yield a) + · ── + 11 │ if (a, b); + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:9:1] + 9 │ if (a &&= b) + 10 │ if (yield a) + · ── + 11 │ if (a, b); + · ── + 12 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:2:1] + 2 │ async function foo() { + 3 │ if (a) + · ── + 4 │ if (await a) + · ── + 5 │ if (a.b) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:3:1] + 3 │ if (a) + 4 │ if (await a) + · ── + 5 │ if (a.b) + · ── + 6 │ if (a && b); + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:4:1] + 4 │ if (await a) + 5 │ if (a.b) + · ── + 6 │ if (a && b); + · ── + 7 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if // 1 + · ── + 3 │ ( + ╰──── + ╭─[no_lonely_if.tsx:9:1] + 9 │ // 6 + 10 │ if ( + · ── + 11 │ // 7 + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if (a) { + · ── + 3 │ if (b) foo() + · ── + 4 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if (a) + · ── + 3 │ if (b) foo() + · ── + 4 │ ;[].forEach(bar) + ╰──── + help: Move the inner `if` test to the outer `if` test. + + ⚠ eslint-plugin-unicorn(no-lonely-if): Unexpected `if` as the only statement in a `if` block without `else`. + ╭─[no_lonely_if.tsx:1:1] + 1 │ + 2 │ if (a) { + · ── + 3 │ if (b) foo() + · ── + 4 │ } + ╰──── + help: Move the inner `if` test to the outer `if` test. + +