From d3534a65215dc8b0d071baf7208471f2296fc8a7 Mon Sep 17 00:00:00 2001 From: disco07 Date: Tue, 30 May 2023 07:43:10 +0200 Subject: [PATCH 1/2] fix issues 10836 --- clippy_lints/src/booleans.rs | 29 +++++++++++++++++++++++++++++ tests/ui/nonminimal_bool.rs | 16 +++++++++++++++- tests/ui/nonminimal_bool.stderr | 26 ++++++++++---------------- 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 455f0df7cd0a..4bd48185f37e 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -8,10 +8,12 @@ use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, UnOp}; use rustc_lint::{LateContext, LateLintPass, Level}; +use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::def_id::LocalDefId; use rustc_span::source_map::Span; use rustc_span::sym; +use rustc_span::symbol::Ident; declare_clippy_lint! { /// ### What it does @@ -89,6 +91,27 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool { } } +fn is_impl_not_trait_with_bool_out<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + cx.tcx + .lang_items() + .not_trait() + .filter(|trait_id| implements_trait(cx, ty, *trait_id, &[])) + .and_then(|trait_id| { + cx.tcx.associated_items(trait_id).find_by_name_and_kind( + cx.tcx, + Ident::from_str("Output"), + ty::AssocKind::Type, + trait_id, + ) + }) + .map_or(false, |assoc_item| { + let proj = cx.tcx.mk_projection(assoc_item.def_id, cx.tcx.mk_substs_trait(ty, [])); + let nty = cx.tcx.normalize_erasing_regions(cx.param_env, proj); + + nty.is_bool() + }) +} + struct NonminimalBoolVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, } @@ -473,6 +496,12 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> { self.bool_expr(e); }, ExprKind::Unary(UnOp::Not, inner) => { + if let ExprKind::Unary(UnOp::Not, ex) = inner.kind { + let ty = self.cx.typeck_results().expr_ty(ex); + if is_impl_not_trait_with_bool_out(self.cx, ty) { + return; + } + } if self.cx.typeck_results().node_types()[inner.hir_id].is_bool() { self.bool_expr(e); } diff --git a/tests/ui/nonminimal_bool.rs b/tests/ui/nonminimal_bool.rs index 80cc7c60f56e..54cbbdef22e0 100644 --- a/tests/ui/nonminimal_bool.rs +++ b/tests/ui/nonminimal_bool.rs @@ -10,6 +10,7 @@ fn main() { let e: bool = unimplemented!(); let _ = !true; let _ = !false; + // vvv Should not lint let _ = !!a; let _ = false || a; // don't lint on cfgs @@ -54,7 +55,6 @@ fn issue4548() { fn check_expect() { let a: bool = unimplemented!(); - #[expect(clippy::nonminimal_bool)] let _ = !!a; } @@ -110,3 +110,17 @@ fn issue_10435() { println!("{}", line!()); } } + +fn issue10836() { + struct Foo(bool); + impl std::ops::Not for Foo { + type Output = bool; + + fn not(self) -> Self::Output { + !self.0 + } + } + + // Should not lint + let _: bool = !!Foo(true); +} diff --git a/tests/ui/nonminimal_bool.stderr b/tests/ui/nonminimal_bool.stderr index 91b5805aa97a..fa14471301ee 100644 --- a/tests/ui/nonminimal_bool.stderr +++ b/tests/ui/nonminimal_bool.stderr @@ -13,37 +13,31 @@ LL | let _ = !false; | ^^^^^^ help: try: `true` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:13:13 - | -LL | let _ = !!a; - | ^^^ help: try: `a` - -error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:14:13 + --> $DIR/nonminimal_bool.rs:15:13 | LL | let _ = false || a; | ^^^^^^^^^^ help: try: `a` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:18:13 + --> $DIR/nonminimal_bool.rs:19:13 | LL | let _ = !(!a && b); | ^^^^^^^^^^ help: try: `a || !b` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:19:13 + --> $DIR/nonminimal_bool.rs:20:13 | LL | let _ = !(!a || b); | ^^^^^^^^^^ help: try: `a && !b` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:20:13 + --> $DIR/nonminimal_bool.rs:21:13 | LL | let _ = !a && !(b && c); | ^^^^^^^^^^^^^^^ help: try: `!(a || b && c)` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:28:13 + --> $DIR/nonminimal_bool.rs:29:13 | LL | let _ = a == b && c == 5 && a == b; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -56,7 +50,7 @@ LL | let _ = a == b && c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:29:13 + --> $DIR/nonminimal_bool.rs:30:13 | LL | let _ = a == b || c == 5 || a == b; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -69,7 +63,7 @@ LL | let _ = a == b || c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:30:13 + --> $DIR/nonminimal_bool.rs:31:13 | LL | let _ = a == b && c == 5 && b == a; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -82,7 +76,7 @@ LL | let _ = a == b && c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:31:13 + --> $DIR/nonminimal_bool.rs:32:13 | LL | let _ = a != b || !(a != b || c == d); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -95,7 +89,7 @@ LL | let _ = a != b || c != d; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:32:13 + --> $DIR/nonminimal_bool.rs:33:13 | LL | let _ = a != b && !(a != b && c == d); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -113,5 +107,5 @@ error: this boolean expression can be simplified LL | if matches!(true, true) && true { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(true, true)` -error: aborting due to 13 previous errors +error: aborting due to 12 previous errors From e4927f98fa53a9d4dd466a9035f71fb1c2a5940b Mon Sep 17 00:00:00 2001 From: disco07 Date: Wed, 31 May 2023 00:17:26 +0200 Subject: [PATCH 2/2] change booleans file and update tests --- clippy_lints/src/booleans.rs | 32 +++----------------------------- tests/ui/nonminimal_bool.rs | 2 +- tests/ui/nonminimal_bool.stderr | 26 ++++++++++++++++---------- 3 files changed, 20 insertions(+), 40 deletions(-) diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 4bd48185f37e..ff0102255a0a 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -8,12 +8,10 @@ use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, UnOp}; use rustc_lint::{LateContext, LateLintPass, Level}; -use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::def_id::LocalDefId; use rustc_span::source_map::Span; use rustc_span::sym; -use rustc_span::symbol::Ident; declare_clippy_lint! { /// ### What it does @@ -90,28 +88,6 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool { NonminimalBoolVisitor { cx }.visit_body(body); } } - -fn is_impl_not_trait_with_bool_out<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { - cx.tcx - .lang_items() - .not_trait() - .filter(|trait_id| implements_trait(cx, ty, *trait_id, &[])) - .and_then(|trait_id| { - cx.tcx.associated_items(trait_id).find_by_name_and_kind( - cx.tcx, - Ident::from_str("Output"), - ty::AssocKind::Type, - trait_id, - ) - }) - .map_or(false, |assoc_item| { - let proj = cx.tcx.mk_projection(assoc_item.def_id, cx.tcx.mk_substs_trait(ty, [])); - let nty = cx.tcx.normalize_erasing_regions(cx.param_env, proj); - - nty.is_bool() - }) -} - struct NonminimalBoolVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, } @@ -496,11 +472,9 @@ impl<'a, 'tcx> Visitor<'tcx> for NonminimalBoolVisitor<'a, 'tcx> { self.bool_expr(e); }, ExprKind::Unary(UnOp::Not, inner) => { - if let ExprKind::Unary(UnOp::Not, ex) = inner.kind { - let ty = self.cx.typeck_results().expr_ty(ex); - if is_impl_not_trait_with_bool_out(self.cx, ty) { - return; - } + if let ExprKind::Unary(UnOp::Not, ex) = inner.kind && + !self.cx.typeck_results().node_types()[ex.hir_id].is_bool() { + return; } if self.cx.typeck_results().node_types()[inner.hir_id].is_bool() { self.bool_expr(e); diff --git a/tests/ui/nonminimal_bool.rs b/tests/ui/nonminimal_bool.rs index 54cbbdef22e0..fec6b7713eef 100644 --- a/tests/ui/nonminimal_bool.rs +++ b/tests/ui/nonminimal_bool.rs @@ -10,7 +10,6 @@ fn main() { let e: bool = unimplemented!(); let _ = !true; let _ = !false; - // vvv Should not lint let _ = !!a; let _ = false || a; // don't lint on cfgs @@ -55,6 +54,7 @@ fn issue4548() { fn check_expect() { let a: bool = unimplemented!(); + #[expect(clippy::nonminimal_bool)] let _ = !!a; } diff --git a/tests/ui/nonminimal_bool.stderr b/tests/ui/nonminimal_bool.stderr index fa14471301ee..91b5805aa97a 100644 --- a/tests/ui/nonminimal_bool.stderr +++ b/tests/ui/nonminimal_bool.stderr @@ -13,31 +13,37 @@ LL | let _ = !false; | ^^^^^^ help: try: `true` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:15:13 + --> $DIR/nonminimal_bool.rs:13:13 + | +LL | let _ = !!a; + | ^^^ help: try: `a` + +error: this boolean expression can be simplified + --> $DIR/nonminimal_bool.rs:14:13 | LL | let _ = false || a; | ^^^^^^^^^^ help: try: `a` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:19:13 + --> $DIR/nonminimal_bool.rs:18:13 | LL | let _ = !(!a && b); | ^^^^^^^^^^ help: try: `a || !b` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:20:13 + --> $DIR/nonminimal_bool.rs:19:13 | LL | let _ = !(!a || b); | ^^^^^^^^^^ help: try: `a && !b` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:21:13 + --> $DIR/nonminimal_bool.rs:20:13 | LL | let _ = !a && !(b && c); | ^^^^^^^^^^^^^^^ help: try: `!(a || b && c)` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:29:13 + --> $DIR/nonminimal_bool.rs:28:13 | LL | let _ = a == b && c == 5 && a == b; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -50,7 +56,7 @@ LL | let _ = a == b && c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:30:13 + --> $DIR/nonminimal_bool.rs:29:13 | LL | let _ = a == b || c == 5 || a == b; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -63,7 +69,7 @@ LL | let _ = a == b || c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:31:13 + --> $DIR/nonminimal_bool.rs:30:13 | LL | let _ = a == b && c == 5 && b == a; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -76,7 +82,7 @@ LL | let _ = a == b && c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:32:13 + --> $DIR/nonminimal_bool.rs:31:13 | LL | let _ = a != b || !(a != b || c == d); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -89,7 +95,7 @@ LL | let _ = a != b || c != d; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:33:13 + --> $DIR/nonminimal_bool.rs:32:13 | LL | let _ = a != b && !(a != b && c == d); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -107,5 +113,5 @@ error: this boolean expression can be simplified LL | if matches!(true, true) && true { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(true, true)` -error: aborting due to 12 previous errors +error: aborting due to 13 previous errors