Skip to content

Commit 1ace535

Browse files
committed
suggest modified code for if_not_else lint
1 parent d5059d8 commit 1ace535

File tree

3 files changed

+100
-5
lines changed

3 files changed

+100
-5
lines changed

clippy_lints/src/if_not_else.rs

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
use clippy_utils::consts::{ConstEvalCtxt, Constant};
2-
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
33
use clippy_utils::is_else_clause;
4+
use clippy_utils::source::{HasSession, indent_of, reindent_multiline, snippet};
5+
use rustc_errors::Applicability;
46
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
57
use rustc_lint::{LateContext, LateLintPass};
68
use rustc_session::declare_lint_pass;
9+
use rustc_span::Span;
10+
use std::borrow::Cow;
711

812
declare_clippy_lint! {
913
/// ### What it does
@@ -54,7 +58,7 @@ fn is_zero_const(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool {
5458

5559
impl LateLintPass<'_> for IfNotElse {
5660
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
57-
if let ExprKind::If(cond, _, Some(els)) = e.kind
61+
if let ExprKind::If(cond, cond_inner, Some(els)) = e.kind
5862
&& let ExprKind::DropTemps(cond) = cond.kind
5963
&& let ExprKind::Block(..) = els.kind
6064
{
@@ -79,8 +83,52 @@ impl LateLintPass<'_> for IfNotElse {
7983
// }
8084
// ```
8185
if !e.span.from_expansion() && !is_else_clause(cx.tcx, e) {
82-
span_lint_and_help(cx, IF_NOT_ELSE, e.span, msg, None, help);
86+
match cond.kind {
87+
ExprKind::Unary(UnOp::Not, _) | ExprKind::Binary(_, _, _) => span_lint_and_sugg(
88+
cx,
89+
IF_NOT_ELSE,
90+
e.span,
91+
msg,
92+
"try",
93+
make_sugg(cx, &cond.kind, cond_inner.span, els.span, "..", Some(e.span)).to_string(),
94+
Applicability::MachineApplicable,
95+
),
96+
_ => span_lint_and_help(cx, IF_NOT_ELSE, e.span, msg, None, help),
97+
}
8398
}
8499
}
85100
}
86101
}
102+
103+
fn make_sugg<'a>(
104+
sess: &impl HasSession,
105+
cond_kind: &'a ExprKind<'a>,
106+
cond_inner: Span,
107+
els_span: Span,
108+
default: &'a str,
109+
indent_relative_to: Option<Span>,
110+
) -> Cow<'a, str> {
111+
let cond_inner_snip = snippet(sess, cond_inner, default);
112+
let els_snip = snippet(sess, els_span, default);
113+
let indent = indent_relative_to.and_then(|s| indent_of(sess, s));
114+
115+
let suggestion = match cond_kind {
116+
ExprKind::Unary(UnOp::Not, cond_rest) => {
117+
format!(
118+
"if {} {} else {}",
119+
snippet(sess, cond_rest.span, default),
120+
els_snip,
121+
cond_inner_snip
122+
)
123+
},
124+
ExprKind::Binary(_, lhs, rhs) => {
125+
let lhs_snip = snippet(sess, lhs.span, default);
126+
let rhs_snip = snippet(sess, rhs.span, default);
127+
128+
format!("if {lhs_snip} == {rhs_snip} {els_snip} else {cond_inner_snip}")
129+
},
130+
_ => String::new(),
131+
};
132+
133+
reindent_multiline(suggestion.into(), true, indent)
134+
}

tests/ui/if_not_else.fixed

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#![warn(clippy::all)]
2+
#![warn(clippy::if_not_else)]
3+
4+
fn foo() -> bool {
5+
unimplemented!()
6+
}
7+
fn bla() -> bool {
8+
unimplemented!()
9+
}
10+
11+
fn main() {
12+
if bla() {
13+
println!("Bunny");
14+
} else {
15+
//~^ ERROR: unnecessary boolean `not` operation
16+
println!("Bugs");
17+
}
18+
if 4 == 5 {
19+
println!("Bunny");
20+
} else {
21+
//~^ ERROR: unnecessary `!=` operation
22+
println!("Bugs");
23+
}
24+
if !foo() {
25+
println!("Foo");
26+
} else if !bla() {
27+
println!("Bugs");
28+
} else {
29+
println!("Bunny");
30+
}
31+
}

tests/ui/if_not_else.stderr

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,17 @@ LL | | println!("Bunny");
99
LL | | }
1010
| |_____^
1111
|
12-
= help: remove the `!` and swap the blocks of the `if`/`else`
1312
= note: `-D clippy::if-not-else` implied by `-D warnings`
1413
= help: to override `-D warnings` add `#[allow(clippy::if_not_else)]`
14+
help: try
15+
|
16+
LL ~ if bla() {
17+
LL + println!("Bunny");
18+
LL + } else {
19+
LL +
20+
LL + println!("Bugs");
21+
LL + }
22+
|
1523

1624
error: unnecessary `!=` operation
1725
--> tests/ui/if_not_else.rs:18:5
@@ -24,7 +32,15 @@ LL | | println!("Bunny");
2432
LL | | }
2533
| |_____^
2634
|
27-
= help: change to `==` and swap the blocks of the `if`/`else`
35+
help: try
36+
|
37+
LL ~ if 4 == 5 {
38+
LL + println!("Bunny");
39+
LL + } else {
40+
LL +
41+
LL + println!("Bugs");
42+
LL + }
43+
|
2844

2945
error: aborting due to 2 previous errors
3046

0 commit comments

Comments
 (0)