-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This commit: - Adds a new late pass lint - Adds relevant UI tests
- Loading branch information
1 parent
95c62ff
commit 2f896be
Showing
6 changed files
with
282 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
use clippy_utils::diagnostics::span_lint_and_help; | ||
use rustc_hir::{Expr, ExprKind}; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_session::declare_lint_pass; | ||
|
||
declare_clippy_lint! { | ||
/// ### What it does | ||
/// Detects `if` statements where the condition is another `if` statement. | ||
/// ### Why is this bad? | ||
/// This makes code hard to read. | ||
/// ### Example | ||
/// ```no_run | ||
/// let a = 3; | ||
/// let b = 4; | ||
/// let c = 5; | ||
/// if if a == b { | ||
/// 4 | ||
/// } else { | ||
/// 5 | ||
/// } == c { | ||
/// // Do something. | ||
/// } | ||
/// ``` | ||
/// Use instead: | ||
/// ```no_run | ||
/// let a = 3; | ||
/// let b = 4; | ||
/// let c = 5; | ||
/// let value = if a == b { | ||
/// 4 | ||
/// } else { | ||
/// 5 | ||
/// }; | ||
/// if value == c { | ||
/// // Do something. | ||
/// } | ||
/// ``` | ||
#[clippy::version = "1.79.0"] | ||
pub STACKED_IFS, | ||
style, | ||
"finds if statements with another if statement as the condition" | ||
} | ||
|
||
declare_lint_pass!(StackedIfs => [STACKED_IFS]); | ||
|
||
impl LateLintPass<'_> for StackedIfs { | ||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { | ||
// Ensure that the expression isn't a part of a constant or macro expansion. | ||
if !expr.span.from_expansion() { | ||
stacked_ifs(cx, expr); | ||
} | ||
} | ||
} | ||
|
||
fn stacked_ifs(cx: &LateContext<'_>, expr: &Expr<'_>) { | ||
// Check for if statements where the condition is another if statement. | ||
let ExprKind::If(condition, _, _) = expr.kind else { | ||
return; | ||
}; | ||
|
||
let condition = condition.peel_drop_temps(); | ||
|
||
if condition.span.from_expansion() { | ||
return; | ||
} | ||
|
||
if let ExprKind::If(..) = condition.kind { | ||
emit_lint(cx, condition); | ||
} | ||
|
||
if let ExprKind::Binary(_, lhs, rhs) = condition.kind { | ||
if let ExprKind::If(..) = lhs.kind | ||
&& !lhs.span.from_expansion() | ||
{ | ||
emit_lint(cx, lhs); | ||
} | ||
if let ExprKind::If(..) = rhs.kind | ||
&& !rhs.span.from_expansion() | ||
{ | ||
emit_lint(cx, rhs); | ||
} | ||
} | ||
} | ||
|
||
fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>) { | ||
span_lint_and_help( | ||
cx, | ||
STACKED_IFS, | ||
expr.span, | ||
"stacked `if` found", | ||
None, | ||
"avoid using an `if` statement as a condition for another `if` statement", | ||
); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
#![warn(clippy::stacked_ifs)] | ||
#![allow(clippy::manual_unwrap_or_default)] | ||
|
||
macro_rules! m { | ||
() => { | ||
let a = true; | ||
let b = false; | ||
if if a { b } else { true } { | ||
println!("True"); | ||
} | ||
}; | ||
} | ||
|
||
fn main() { | ||
let a = 3; | ||
let b = 4; | ||
if a == b { // No error | ||
// Do something. | ||
} else { | ||
// Do something else. | ||
} | ||
|
||
if if a % 2 == 0 { | ||
//~^ ERROR: stacked `if` found | ||
a - 1 | ||
} else { | ||
a + 1 | ||
} == b | ||
{ | ||
// Do something. | ||
} | ||
|
||
if if a % 2 == 0 { | ||
//~^ ERROR: stacked `if` found | ||
a - 1 | ||
} else { | ||
a + 1 | ||
} == b | ||
{ | ||
// Do something. | ||
} else { | ||
// Do something else. | ||
} | ||
|
||
if b == if a % 2 == 0 { | ||
//~^ ERROR: stacked `if` found | ||
a - 1 | ||
} else { | ||
a + 1 | ||
} { | ||
// Do something. | ||
} else { | ||
// Do something else. | ||
} | ||
|
||
let c = 5; | ||
let d = 6; | ||
|
||
// More complex statements (example from issue 12483): | ||
if if if a == b { | ||
//~^ ERROR: stacked `if` found | ||
//~^^ ERROR: stacked `if` found | ||
b == c | ||
} else { | ||
a == c | ||
} { | ||
a == d | ||
} else { | ||
c == d | ||
} { | ||
println!("True!"); | ||
} else { | ||
println!("False!"); | ||
} | ||
|
||
let a = true; | ||
let b = false; | ||
|
||
if if a { b } else { true } { | ||
//~^ ERROR: stacked `if` found | ||
// Do something. | ||
} | ||
|
||
let x = Some(10); | ||
|
||
if if let Some(num) = x { num } else { 0 } == 0 { | ||
//~^ ERROR: stacked `if` found | ||
// Do something. | ||
} | ||
|
||
m!(); // No error | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
error: stacked `if` found | ||
--> tests/ui/stacked_ifs.rs:23:8 | ||
| | ||
LL | if if a % 2 == 0 { | ||
| ________^ | ||
LL | | | ||
LL | | a - 1 | ||
LL | | } else { | ||
LL | | a + 1 | ||
LL | | } == b | ||
| |_____^ | ||
| | ||
= help: avoid using an `if` statement as a condition for another `if` statement | ||
= note: `-D clippy::stacked-ifs` implied by `-D warnings` | ||
= help: to override `-D warnings` add `#[allow(clippy::stacked_ifs)]` | ||
|
||
error: stacked `if` found | ||
--> tests/ui/stacked_ifs.rs:33:8 | ||
| | ||
LL | if if a % 2 == 0 { | ||
| ________^ | ||
LL | | | ||
LL | | a - 1 | ||
LL | | } else { | ||
LL | | a + 1 | ||
LL | | } == b | ||
| |_____^ | ||
| | ||
= help: avoid using an `if` statement as a condition for another `if` statement | ||
|
||
error: stacked `if` found | ||
--> tests/ui/stacked_ifs.rs:45:13 | ||
| | ||
LL | if b == if a % 2 == 0 { | ||
| _____________^ | ||
LL | | | ||
LL | | a - 1 | ||
LL | | } else { | ||
LL | | a + 1 | ||
LL | | } { | ||
| |_____^ | ||
| | ||
= help: avoid using an `if` statement as a condition for another `if` statement | ||
|
||
error: stacked `if` found | ||
--> tests/ui/stacked_ifs.rs:60:8 | ||
| | ||
LL | if if if a == b { | ||
| ________^ | ||
LL | | | ||
LL | | | ||
LL | | b == c | ||
... | | ||
LL | | c == d | ||
LL | | } { | ||
| |_____^ | ||
| | ||
= help: avoid using an `if` statement as a condition for another `if` statement | ||
|
||
error: stacked `if` found | ||
--> tests/ui/stacked_ifs.rs:60:11 | ||
| | ||
LL | if if if a == b { | ||
| ___________^ | ||
LL | | | ||
LL | | | ||
LL | | b == c | ||
LL | | } else { | ||
LL | | a == c | ||
LL | | } { | ||
| |_____^ | ||
| | ||
= help: avoid using an `if` statement as a condition for another `if` statement | ||
|
||
error: stacked `if` found | ||
--> tests/ui/stacked_ifs.rs:79:8 | ||
| | ||
LL | if if a { b } else { true } { | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: avoid using an `if` statement as a condition for another `if` statement | ||
|
||
error: stacked `if` found | ||
--> tests/ui/stacked_ifs.rs:86:8 | ||
| | ||
LL | if if let Some(num) = x { num } else { 0 } == 0 { | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= help: avoid using an `if` statement as a condition for another `if` statement | ||
|
||
error: aborting due to 7 previous errors | ||
|