From 282d313eb957149fdeeade03f218cc4478c91f62 Mon Sep 17 00:00:00 2001 From: Ahmed Karaman <20889958+ahmedkrmn@users.noreply.github.com> Date: Fri, 29 Oct 2021 04:11:43 +0200 Subject: [PATCH] Disable "if_not_else" lints firing on else-ifs 1. Convert IfNotElse to LateLintPass and use clippy_utils::is_else_clause for checking. 2. Update tests. --- clippy_lints/src/if_not_else.rs | 21 +++++++++++++++------ clippy_lints/src/lib.rs | 2 +- tests/ui/if_not_else.rs | 10 ++++++++++ tests/ui/if_not_else.stderr | 4 ++-- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/if_not_else.rs b/clippy_lints/src/if_not_else.rs index 3ce91d421bac..d8c654284fc6 100644 --- a/clippy_lints/src/if_not_else.rs +++ b/clippy_lints/src/if_not_else.rs @@ -2,8 +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 clippy_utils::is_else_clause; +use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -46,13 +47,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<'_>) { + if in_external_macro(cx.sess(), item.span) { return; } - if let ExprKind::If(ref cond, _, Some(ref els)) = item.kind { + if let ExprKind::If(mut cond, _, Some(els)) = item.kind { if let ExprKind::Block(..) = els.kind { + // Disable firing the lint in "else if" expressions. + if is_else_clause(cx.tcx, item) { + return; + } + + if let ExprKind::DropTemps(inner) = &cond.kind { + cond = inner; + } match cond.kind { ExprKind::Unary(UnOp::Not, _) => { span_lint_and_help( diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fd5f164e7a1b..7174d0a082e0 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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)); @@ -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)); diff --git a/tests/ui/if_not_else.rs b/tests/ui/if_not_else.rs index dc3fb1ceac9a..b7012b43d297 100644 --- a/tests/ui/if_not_else.rs +++ b/tests/ui/if_not_else.rs @@ -1,6 +1,9 @@ #![warn(clippy::all)] #![warn(clippy::if_not_else)] +fn foo() -> bool { + unimplemented!() +} fn bla() -> bool { unimplemented!() } @@ -16,4 +19,11 @@ fn main() { } else { println!("Bunny"); } + if !foo() { + println!("Foo"); + } else if !bla() { + println!("Bugs"); + } else { + println!("Bunny"); + } } diff --git a/tests/ui/if_not_else.stderr b/tests/ui/if_not_else.stderr index 53d1b86d02a9..8c8cc44bb035 100644 --- a/tests/ui/if_not_else.stderr +++ b/tests/ui/if_not_else.stderr @@ -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"); @@ -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");