From f959ccc09b28b9aaa49b8282691b45564c9f1326 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 8 Aug 2023 17:19:53 +0200 Subject: [PATCH 1/2] [`redundant_guards`]: don't lint on floats --- clippy_lints/src/matches/redundant_guards.rs | 5 +++-- tests/ui/redundant_guards.fixed | 7 +++++++ tests/ui/redundant_guards.rs | 7 +++++++ tests/ui/redundant_guards.stderr | 16 ++++++++-------- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 6383326aa38d..0e7059caadb6 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -2,6 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::path_to_local; use clippy_utils::source::snippet_with_applicability; use clippy_utils::visitors::{for_each_expr, is_local_used}; +use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, MatchSource, Node, Pat, PatKind}; @@ -177,8 +178,8 @@ fn expr_can_be_pat(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { ExprKind::AddrOf(..) | ExprKind::Array(..) | ExprKind::Tup(..) - | ExprKind::Struct(..) - | ExprKind::Lit(..) => true, + | ExprKind::Struct(..) => true, + ExprKind::Lit(lit) if !matches!(lit.node, LitKind::Float(..)) => true, _ => false, } { return ControlFlow::Continue(()); diff --git a/tests/ui/redundant_guards.fixed b/tests/ui/redundant_guards.fixed index 77ac76668649..2adedf3d0298 100644 --- a/tests/ui/redundant_guards.fixed +++ b/tests/ui/redundant_guards.fixed @@ -15,6 +15,13 @@ struct B { struct C(u32, u32); +fn issue11304() { + match 0.1 { + x if x == 0.0 => todo!(), + _ => todo!(), + } +} + fn main() { let c = C(1, 2); match c { diff --git a/tests/ui/redundant_guards.rs b/tests/ui/redundant_guards.rs index b072e4ea14d1..cf2859952144 100644 --- a/tests/ui/redundant_guards.rs +++ b/tests/ui/redundant_guards.rs @@ -15,6 +15,13 @@ struct B { struct C(u32, u32); +fn issue11304() { + match 0.1 { + x if x == 0.0 => todo!(), + _ => todo!(), + } +} + fn main() { let c = C(1, 2); match c { diff --git a/tests/ui/redundant_guards.stderr b/tests/ui/redundant_guards.stderr index c2a92071d1df..f1b3fd083814 100644 --- a/tests/ui/redundant_guards.stderr +++ b/tests/ui/redundant_guards.stderr @@ -1,5 +1,5 @@ error: redundant guard - --> $DIR/redundant_guards.rs:21:20 + --> $DIR/redundant_guards.rs:28:20 | LL | C(x, y) if let 1 = y => .., | ^^^^^^^^^ @@ -12,7 +12,7 @@ LL + C(x, 1) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:27:20 + --> $DIR/redundant_guards.rs:34:20 | LL | Some(x) if matches!(x, Some(1) if true) => .., | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,7 +23,7 @@ LL | Some(Some(1)) if true => .., | ~~~~~~~ ~~~~~~~ error: redundant guard - --> $DIR/redundant_guards.rs:28:20 + --> $DIR/redundant_guards.rs:35:20 | LL | Some(x) if matches!(x, Some(1)) => { | ^^^^^^^^^^^^^^^^^^^^ @@ -35,7 +35,7 @@ LL + Some(Some(1)) => { | error: redundant guard - --> $DIR/redundant_guards.rs:32:20 + --> $DIR/redundant_guards.rs:39:20 | LL | Some(x) if let Some(1) = x => .., | ^^^^^^^^^^^^^^^ @@ -47,7 +47,7 @@ LL + Some(Some(1)) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:33:20 + --> $DIR/redundant_guards.rs:40:20 | LL | Some(x) if x == Some(2) => .., | ^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL + Some(Some(2)) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:56:20 + --> $DIR/redundant_guards.rs:63:20 | LL | B { e } if matches!(e, Some(A(2))) => .., | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -71,7 +71,7 @@ LL + B { e: Some(A(2)) } => .., | error: redundant guard - --> $DIR/redundant_guards.rs:93:20 + --> $DIR/redundant_guards.rs:100:20 | LL | E::A(y) if y == "not from an or pattern" => {}, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -83,7 +83,7 @@ LL + E::A("not from an or pattern") => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:100:14 + --> $DIR/redundant_guards.rs:107:14 | LL | x if matches!(x, Some(0)) => .., | ^^^^^^^^^^^^^^^^^^^^ From b6156502afc765bac1ef48242009fe03c53232cc Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 8 Aug 2023 17:52:17 +0200 Subject: [PATCH 2/2] document the new behavior and add test for float in struct --- clippy_lints/src/matches/redundant_guards.rs | 5 +++++ tests/ui/redundant_guards.fixed | 6 ++++++ tests/ui/redundant_guards.rs | 6 ++++++ tests/ui/redundant_guards.stderr | 16 ++++++++-------- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 0e7059caadb6..29af4812351e 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -161,6 +161,11 @@ fn emit_redundant_guards<'tcx>( } /// Checks if the given `Expr` can also be represented as a `Pat`. +/// +/// All literals generally also work as patterns, however float literals are special. +/// They are currently (as of 2023/08/08) still allowed in patterns, but that will become +/// an error in the future, and rustc already actively warns against this (see rust#41620), +/// so we don't consider those as usable within patterns for linting purposes. fn expr_can_be_pat(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { for_each_expr(expr, |expr| { if match expr.kind { diff --git a/tests/ui/redundant_guards.fixed b/tests/ui/redundant_guards.fixed index 2adedf3d0298..49d7336ee371 100644 --- a/tests/ui/redundant_guards.fixed +++ b/tests/ui/redundant_guards.fixed @@ -15,11 +15,17 @@ struct B { struct C(u32, u32); +#[derive(PartialEq)] +struct FloatWrapper(f32); fn issue11304() { match 0.1 { x if x == 0.0 => todo!(), _ => todo!(), } + match FloatWrapper(0.1) { + x if x == FloatWrapper(0.0) => todo!(), + _ => todo!(), + } } fn main() { diff --git a/tests/ui/redundant_guards.rs b/tests/ui/redundant_guards.rs index cf2859952144..87761010de2c 100644 --- a/tests/ui/redundant_guards.rs +++ b/tests/ui/redundant_guards.rs @@ -15,11 +15,17 @@ struct B { struct C(u32, u32); +#[derive(PartialEq)] +struct FloatWrapper(f32); fn issue11304() { match 0.1 { x if x == 0.0 => todo!(), _ => todo!(), } + match FloatWrapper(0.1) { + x if x == FloatWrapper(0.0) => todo!(), + _ => todo!(), + } } fn main() { diff --git a/tests/ui/redundant_guards.stderr b/tests/ui/redundant_guards.stderr index f1b3fd083814..5bdf43d23c53 100644 --- a/tests/ui/redundant_guards.stderr +++ b/tests/ui/redundant_guards.stderr @@ -1,5 +1,5 @@ error: redundant guard - --> $DIR/redundant_guards.rs:28:20 + --> $DIR/redundant_guards.rs:34:20 | LL | C(x, y) if let 1 = y => .., | ^^^^^^^^^ @@ -12,7 +12,7 @@ LL + C(x, 1) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:34:20 + --> $DIR/redundant_guards.rs:40:20 | LL | Some(x) if matches!(x, Some(1) if true) => .., | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,7 +23,7 @@ LL | Some(Some(1)) if true => .., | ~~~~~~~ ~~~~~~~ error: redundant guard - --> $DIR/redundant_guards.rs:35:20 + --> $DIR/redundant_guards.rs:41:20 | LL | Some(x) if matches!(x, Some(1)) => { | ^^^^^^^^^^^^^^^^^^^^ @@ -35,7 +35,7 @@ LL + Some(Some(1)) => { | error: redundant guard - --> $DIR/redundant_guards.rs:39:20 + --> $DIR/redundant_guards.rs:45:20 | LL | Some(x) if let Some(1) = x => .., | ^^^^^^^^^^^^^^^ @@ -47,7 +47,7 @@ LL + Some(Some(1)) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:40:20 + --> $DIR/redundant_guards.rs:46:20 | LL | Some(x) if x == Some(2) => .., | ^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL + Some(Some(2)) => .., | error: redundant guard - --> $DIR/redundant_guards.rs:63:20 + --> $DIR/redundant_guards.rs:69:20 | LL | B { e } if matches!(e, Some(A(2))) => .., | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -71,7 +71,7 @@ LL + B { e: Some(A(2)) } => .., | error: redundant guard - --> $DIR/redundant_guards.rs:100:20 + --> $DIR/redundant_guards.rs:106:20 | LL | E::A(y) if y == "not from an or pattern" => {}, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -83,7 +83,7 @@ LL + E::A("not from an or pattern") => {}, | error: redundant guard - --> $DIR/redundant_guards.rs:107:14 + --> $DIR/redundant_guards.rs:113:14 | LL | x if matches!(x, Some(0)) => .., | ^^^^^^^^^^^^^^^^^^^^