Skip to content

Commit

Permalink
Disable "if_not_else" lints firing on else-ifs
Browse files Browse the repository at this point in the history
1. Convert IfNotElse to LateLintPass and use clippy_utils::is_else_clause for checking.
2. Handle the case where the span comes from desugaring.
3. Update tests.
  • Loading branch information
ahmedkrmn committed Oct 29, 2021
1 parent 00821ca commit 2f327aa
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 11 deletions.
23 changes: 15 additions & 8 deletions clippy_lints/src/if_not_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
//! on the condition

use clippy_utils::diagnostics::span_lint_and_help;
use rustc_ast::ast::{BinOpKind, Expr, ExprKind, UnOp};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_middle::lint::in_external_macro;
use clippy_utils::is_else_clause;
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
Expand Down Expand Up @@ -46,14 +46,21 @@ declare_clippy_lint! {

declare_lint_pass!(IfNotElse => [IF_NOT_ELSE]);

impl EarlyLintPass for IfNotElse {
fn check_expr(&mut self, cx: &EarlyContext<'_>, item: &Expr) {
if in_external_macro(cx.sess, item.span) {
impl LateLintPass<'_> for IfNotElse {
fn check_expr(&mut self, cx: &LateContext<'_>, item: &Expr<'_>) {
// While loops will be desugared to ExprKind::If. This will cause the lint to fire.
// To fix this, return early if this span comes from a macro or desugaring.
if item.span.from_expansion() {
return;
}
if let ExprKind::If(ref cond, _, Some(ref els)) = item.kind {
if let ExprKind::If(cond, _, Some(els)) = item.kind {
if let ExprKind::Block(..) = els.kind {
match cond.kind {
// Disable firing the lint in "else if" expressions.
if is_else_clause(cx.tcx, item) {
return;
}

match cond.peel_drop_temps().kind {
ExprKind::Unary(UnOp::Not, _) => {
span_lint_and_help(
cx,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| Box::new(double_parens::DoubleParens));
store.register_late_pass(|| Box::new(to_string_in_display::ToStringInDisplay::new()));
store.register_early_pass(|| Box::new(unsafe_removed_from_name::UnsafeNameRemoval));
store.register_early_pass(|| Box::new(if_not_else::IfNotElse));
store.register_early_pass(|| Box::new(else_if_without_else::ElseIfWithoutElse));
store.register_early_pass(|| Box::new(int_plus_one::IntPlusOne));
store.register_early_pass(|| Box::new(formatting::Formatting));
Expand Down Expand Up @@ -722,6 +721,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(option_if_let_else::OptionIfLetElse));
store.register_late_pass(|| Box::new(future_not_send::FutureNotSend));
store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));
store.register_late_pass(|| Box::new(if_not_else::IfNotElse));
store.register_late_pass(|| Box::new(equatable_if_let::PatternEquality));
store.register_late_pass(|| Box::new(mut_mutex_lock::MutMutexLock));
store.register_late_pass(|| Box::new(match_on_vec_items::MatchOnVecItems));
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/if_not_else.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#![warn(clippy::all)]
#![warn(clippy::if_not_else)]

fn foo() -> bool {
unimplemented!()
}
fn bla() -> bool {
unimplemented!()
}
Expand All @@ -16,4 +19,11 @@ fn main() {
} else {
println!("Bunny");
}
if !foo() {
println!("Foo");
} else if !bla() {
println!("Bugs");
} else {
println!("Bunny");
}
}
4 changes: 2 additions & 2 deletions tests/ui/if_not_else.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: unnecessary boolean `not` operation
--> $DIR/if_not_else.rs:9:5
--> $DIR/if_not_else.rs:12:5
|
LL | / if !bla() {
LL | | println!("Bugs");
Expand All @@ -12,7 +12,7 @@ LL | | }
= help: remove the `!` and swap the blocks of the `if`/`else`

error: unnecessary `!=` operation
--> $DIR/if_not_else.rs:14:5
--> $DIR/if_not_else.rs:17:5
|
LL | / if 4 != 5 {
LL | | println!("Bugs");
Expand Down

0 comments on commit 2f327aa

Please sign in to comment.