From a35734c172ba95a6d8758c7c5bb6601bcd3a5485 Mon Sep 17 00:00:00 2001 From: kraktus Date: Fri, 16 Sep 2022 21:38:56 +0200 Subject: [PATCH] revert `manual_assert` suggestion refactor Because `Sugg` helper does not simplify multiple negations --- clippy_lints/src/manual_assert.rs | 11 +++++++---- tests/ui/manual_assert.edition2018.fixed | 4 ++-- tests/ui/manual_assert.edition2018.stderr | 4 ++-- tests/ui/manual_assert.edition2021.fixed | 4 ++-- tests/ui/manual_assert.edition2021.stderr | 4 ++-- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs index 9934c06e7..cbc9c4ca6 100644 --- a/clippy_lints/src/manual_assert.rs +++ b/clippy_lints/src/manual_assert.rs @@ -4,7 +4,7 @@ use clippy_utils::macros::{root_macro_call, FormatArgsExpn}; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{peel_blocks_with_stmt, span_extract_comment, sugg}; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -55,9 +55,12 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert { if !comments.is_empty() { comments += "\n"; } - // we need to negate the expression because `assert!` panics when is `false`, wherease original pattern panicked when evaluating to `true` - let cond_sugg = !sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability); - let sugg = format!("assert!({cond_sugg}, {format_args_snip});"); + let (cond, not) = match cond.kind { + ExprKind::Unary(UnOp::Not, e) => (e, ""), + _ => (cond, "!"), + }; + let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par(); + let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip});"); // we show to the user the suggestion without the comments, but when applicating the fix, include the comments in the block span_lint_and_then( cx, diff --git a/tests/ui/manual_assert.edition2018.fixed b/tests/ui/manual_assert.edition2018.fixed index b5ccc07e4..81e38e1bf 100644 --- a/tests/ui/manual_assert.edition2018.fixed +++ b/tests/ui/manual_assert.edition2018.fixed @@ -28,8 +28,8 @@ fn main() { { panic!("qaqaq{:?}", a); } - assert!(!(!a.is_empty()), "qaqaq{:?}", a); - assert!(!(!a.is_empty()), "qwqwq"); + assert!(a.is_empty(), "qaqaq{:?}", a); + assert!(a.is_empty(), "qwqwq"); if a.len() == 3 { println!("qwq"); println!("qwq"); diff --git a/tests/ui/manual_assert.edition2018.stderr b/tests/ui/manual_assert.edition2018.stderr index df22e0114..e71bf4283 100644 --- a/tests/ui/manual_assert.edition2018.stderr +++ b/tests/ui/manual_assert.edition2018.stderr @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::manual-assert` implied by `-D warnings` help: try instead | -LL | assert!(!(!a.is_empty()), "qaqaq{:?}", a); +LL | assert!(a.is_empty(), "qaqaq{:?}", a); | error: only a `panic!` in `if`-then statement @@ -22,7 +22,7 @@ LL | | } | help: try instead | -LL | assert!(!(!a.is_empty()), "qwqwq"); +LL | assert!(a.is_empty(), "qwqwq"); | error: only a `panic!` in `if`-then statement diff --git a/tests/ui/manual_assert.edition2021.fixed b/tests/ui/manual_assert.edition2021.fixed index b5ccc07e4..81e38e1bf 100644 --- a/tests/ui/manual_assert.edition2021.fixed +++ b/tests/ui/manual_assert.edition2021.fixed @@ -28,8 +28,8 @@ fn main() { { panic!("qaqaq{:?}", a); } - assert!(!(!a.is_empty()), "qaqaq{:?}", a); - assert!(!(!a.is_empty()), "qwqwq"); + assert!(a.is_empty(), "qaqaq{:?}", a); + assert!(a.is_empty(), "qwqwq"); if a.len() == 3 { println!("qwq"); println!("qwq"); diff --git a/tests/ui/manual_assert.edition2021.stderr b/tests/ui/manual_assert.edition2021.stderr index df22e0114..e71bf4283 100644 --- a/tests/ui/manual_assert.edition2021.stderr +++ b/tests/ui/manual_assert.edition2021.stderr @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::manual-assert` implied by `-D warnings` help: try instead | -LL | assert!(!(!a.is_empty()), "qaqaq{:?}", a); +LL | assert!(a.is_empty(), "qaqaq{:?}", a); | error: only a `panic!` in `if`-then statement @@ -22,7 +22,7 @@ LL | | } | help: try instead | -LL | assert!(!(!a.is_empty()), "qwqwq"); +LL | assert!(a.is_empty(), "qwqwq"); | error: only a `panic!` in `if`-then statement