Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): Add eslint/no-lonely-if #9660

Merged
merged 8 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
290 changes: 290 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_lonely_if.rs
Original file line number Diff line number Diff line change
@@ -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();
}
84 changes: 84 additions & 0 deletions crates/oxc_linter/src/snapshots/eslint_no_lonely_if.snap
Original file line number Diff line number Diff line change
@@ -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.