From ef587d22a403ee5d637625d1ccf157efd6402ac3 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 15 Nov 2023 19:33:30 +0100 Subject: [PATCH 1/5] [`redundant_guards`]: lint empty string checks --- clippy_lints/src/matches/redundant_guards.rs | 53 +++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 4a44d596a46..9cc546de22e 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::path_to_local; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::snippet; use clippy_utils::visitors::{for_each_expr, is_local_used}; use rustc_ast::{BorrowKind, LitKind}; use rustc_errors::Applicability; @@ -9,6 +9,7 @@ use rustc_lint::LateContext; use rustc_span::symbol::Ident; use rustc_span::Span; +use std::borrow::Cow; use std::ops::ControlFlow; use super::REDUNDANT_GUARDS; @@ -41,7 +42,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { (PatKind::Ref(..), None) | (_, Some(_)) => continue, _ => arm.pat.span, }; - emit_redundant_guards(cx, outer_arm, if_expr.span, pat_span, &binding, arm.guard); + emit_redundant_guards( + cx, + outer_arm, + if_expr.span, + snippet(cx, pat_span, ""), + &binding, + arm.guard, + ); } // `Some(x) if let Some(2) = x` else if let Guard::IfLet(let_expr) = guard @@ -52,7 +60,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { (PatKind::Ref(..), None) | (_, Some(_)) => continue, _ => let_expr.pat.span, }; - emit_redundant_guards(cx, outer_arm, let_expr.span, pat_span, &binding, None); + emit_redundant_guards( + cx, + outer_arm, + let_expr.span, + snippet(cx, pat_span, ""), + &binding, + None, + ); } // `Some(x) if x == Some(2)` // `Some(x) if Some(2) == x` @@ -78,7 +93,25 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { (ExprKind::AddrOf(..), None) | (_, Some(_)) => continue, _ => pat.span, }; - emit_redundant_guards(cx, outer_arm, if_expr.span, pat_span, &binding, None); + emit_redundant_guards( + cx, + outer_arm, + if_expr.span, + snippet(cx, pat_span, ""), + &binding, + None, + ); + } else if let Guard::If(if_expr) = guard + && let ExprKind::MethodCall(path, recv, ..) = if_expr.kind + && let Some(binding) = get_pat_binding(cx, recv, outer_arm) + { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + + if path.ident.name == sym!(is_empty) && ty.is_str() { + // `s if s.is_empty()` becomes "" + + emit_redundant_guards(cx, outer_arm, if_expr.span, r#""""#.into(), &binding, None) + } } } } @@ -134,19 +167,16 @@ fn emit_redundant_guards<'tcx>( cx: &LateContext<'tcx>, outer_arm: &Arm<'tcx>, guard_span: Span, - pat_span: Span, + binding_replacement: Cow<'static, str>, pat_binding: &PatBindingInfo, inner_guard: Option>, ) { - let mut app = Applicability::MaybeIncorrect; - span_lint_and_then( cx, REDUNDANT_GUARDS, guard_span.source_callsite(), "redundant guard", |diag| { - let binding_replacement = snippet_with_applicability(cx, pat_span, "", &mut app); let suggestion_span = match *pat_binding { PatBindingInfo { span, @@ -170,14 +200,11 @@ fn emit_redundant_guards<'tcx>( Guard::IfLet(l) => ("if let", l.span), }; - format!( - " {prefix} {}", - snippet_with_applicability(cx, span, "", &mut app), - ) + format!(" {prefix} {}", snippet(cx, span, "")) }), ), ], - app, + Applicability::MaybeIncorrect, ); }, ); From 998a311a131068ea6d4c42ab21d076b0f386929d Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 15 Nov 2023 19:37:36 +0100 Subject: [PATCH 2/5] [`redundant_guards`]: lint empty slice checks --- clippy_lints/src/matches/redundant_guards.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 9cc546de22e..9133e785af0 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -106,11 +106,17 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { && let Some(binding) = get_pat_binding(cx, recv, outer_arm) { let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + let slice_like = ty.is_slice() || ty.is_array(); - if path.ident.name == sym!(is_empty) && ty.is_str() { + if path.ident.name == sym!(is_empty) { // `s if s.is_empty()` becomes "" + // `arr if arr.is_empty()` becomes [] - emit_redundant_guards(cx, outer_arm, if_expr.span, r#""""#.into(), &binding, None) + if ty.is_str() { + emit_redundant_guards(cx, outer_arm, if_expr.span, r#""""#.into(), &binding, None) + } else if slice_like { + emit_redundant_guards(cx, outer_arm, if_expr.span, "[]".into(), &binding, None) + } } } } From 676f1f6ef81ce5ee71f8eeecef4db9de6405900b Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 15 Nov 2023 20:53:43 +0100 Subject: [PATCH 3/5] [`redundant_guards`]: lint `slice::{starts_with,ends_with}` --- clippy_lints/src/matches/redundant_guards.rs | 36 +++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 9133e785af0..8d5907d6faf 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -102,22 +102,48 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { None, ); } else if let Guard::If(if_expr) = guard - && let ExprKind::MethodCall(path, recv, ..) = if_expr.kind + && let ExprKind::MethodCall(path, recv, args, ..) = if_expr.kind && let Some(binding) = get_pat_binding(cx, recv, outer_arm) { let ty = cx.typeck_results().expr_ty(recv).peel_refs(); let slice_like = ty.is_slice() || ty.is_array(); - if path.ident.name == sym!(is_empty) { + let sugg = if path.ident.name == sym!(is_empty) { // `s if s.is_empty()` becomes "" // `arr if arr.is_empty()` becomes [] if ty.is_str() { - emit_redundant_guards(cx, outer_arm, if_expr.span, r#""""#.into(), &binding, None) + r#""""#.into() } else if slice_like { - emit_redundant_guards(cx, outer_arm, if_expr.span, "[]".into(), &binding, None) + "[]".into() + } else { + continue; } - } + } else if slice_like + && let Some(needle) = args.first() + && let ExprKind::AddrOf(.., needle) = needle.kind + && let ExprKind::Array(needles) = needle.kind + && needles.iter().all(|needle| expr_can_be_pat(cx, needle)) + { + // `arr if arr.starts_with(&[123])` becomes [123, ..] + // `arr if arr.ends_with(&[123])` becomes [.., 123] + + let mut sugg = snippet(cx, needle.span, "").into_owned(); + + if path.ident.name == sym!(starts_with) { + sugg.insert_str(sugg.len() - 1, ", .."); + } else if path.ident.name == sym!(ends_with) { + sugg.insert_str(1, ".., "); + } else { + continue; + } + + sugg.into() + } else { + continue; + }; + + emit_redundant_guards(cx, outer_arm, if_expr.span, sugg, &binding, None); } } } From 1b4e2ef3d797b902a8d9870e5cc33795951ee619 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 15 Nov 2023 21:10:03 +0100 Subject: [PATCH 4/5] fix empty needle corner case and add tests --- clippy_lints/src/matches/redundant_guards.rs | 5 +- tests/ui/redundant_guards.fixed | 57 +++++++++++++ tests/ui/redundant_guards.rs | 57 +++++++++++++ tests/ui/redundant_guards.stderr | 86 +++++++++++++++++++- 4 files changed, 203 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 8d5907d6faf..365d178f548 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -127,10 +127,13 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { { // `arr if arr.starts_with(&[123])` becomes [123, ..] // `arr if arr.ends_with(&[123])` becomes [.., 123] + // `arr if arr.starts_with(&[])` becomes [..] (why would anyone write this?) let mut sugg = snippet(cx, needle.span, "").into_owned(); - if path.ident.name == sym!(starts_with) { + if needles.is_empty() { + sugg.insert_str(1, ".."); + } else if path.ident.name == sym!(starts_with) { sugg.insert_str(sugg.len() - 1, ", .."); } else if path.ident.name == sym!(ends_with) { sugg.insert_str(1, ".., "); diff --git a/tests/ui/redundant_guards.fixed b/tests/ui/redundant_guards.fixed index f8af9092725..aef26ef225c 100644 --- a/tests/ui/redundant_guards.fixed +++ b/tests/ui/redundant_guards.fixed @@ -193,3 +193,60 @@ mod issue11465 { } } } + +fn issue11807() { + #![allow(clippy::single_match)] + + match Some(Some("")) { + Some(Some("")) => {}, + _ => {}, + } + + match Some(Some(String::new())) { + // Do not lint: String deref-coerces to &str + Some(Some(x)) if x.is_empty() => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some([])) => {}, + _ => {}, + } + + match Some(Some([] as [i32; 0])) { + Some(Some([])) => {}, + _ => {}, + } + + match Some(Some(Vec::<()>::new())) { + // Do not lint: Vec deref-coerces to &[T] + Some(Some(x)) if x.is_empty() => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some([..])) => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some([1, ..])) => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some([1, 2, ..])) => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some([.., 1, 2])) => {}, + _ => {}, + } + + match Some(Some(Vec::::new())) { + // Do not lint: deref coercion + Some(Some(x)) if x.starts_with(&[1, 2]) => {}, + _ => {}, + } +} diff --git a/tests/ui/redundant_guards.rs b/tests/ui/redundant_guards.rs index b46f8a6207e..5d476f5b040 100644 --- a/tests/ui/redundant_guards.rs +++ b/tests/ui/redundant_guards.rs @@ -193,3 +193,60 @@ fn issue11465() { } } } + +fn issue11807() { + #![allow(clippy::single_match)] + + match Some(Some("")) { + Some(Some(x)) if x.is_empty() => {}, + _ => {}, + } + + match Some(Some(String::new())) { + // Do not lint: String deref-coerces to &str + Some(Some(x)) if x.is_empty() => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some(x)) if x.is_empty() => {}, + _ => {}, + } + + match Some(Some([] as [i32; 0])) { + Some(Some(x)) if x.is_empty() => {}, + _ => {}, + } + + match Some(Some(Vec::<()>::new())) { + // Do not lint: Vec deref-coerces to &[T] + Some(Some(x)) if x.is_empty() => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some(x)) if x.starts_with(&[]) => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some(x)) if x.starts_with(&[1]) => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some(x)) if x.starts_with(&[1, 2]) => {}, + _ => {}, + } + + match Some(Some(&[] as &[i32])) { + Some(Some(x)) if x.ends_with(&[1, 2]) => {}, + _ => {}, + } + + match Some(Some(Vec::::new())) { + // Do not lint: deref coercion + Some(Some(x)) if x.starts_with(&[1, 2]) => {}, + _ => {}, + } +} diff --git a/tests/ui/redundant_guards.stderr b/tests/ui/redundant_guards.stderr index b8d7834e358..f78d2a814f9 100644 --- a/tests/ui/redundant_guards.stderr +++ b/tests/ui/redundant_guards.stderr @@ -203,5 +203,89 @@ LL - B { ref c, .. } if matches!(c, &1) => {}, LL + B { c: 1, .. } => {}, | -error: aborting due to 17 previous errors +error: redundant guard + --> $DIR/redundant_guards.rs:201:26 + | +LL | Some(Some(x)) if x.is_empty() => {}, + | ^^^^^^^^^^^^ + | +help: try + | +LL - Some(Some(x)) if x.is_empty() => {}, +LL + Some(Some("")) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:212:26 + | +LL | Some(Some(x)) if x.is_empty() => {}, + | ^^^^^^^^^^^^ + | +help: try + | +LL - Some(Some(x)) if x.is_empty() => {}, +LL + Some(Some([])) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:217:26 + | +LL | Some(Some(x)) if x.is_empty() => {}, + | ^^^^^^^^^^^^ + | +help: try + | +LL - Some(Some(x)) if x.is_empty() => {}, +LL + Some(Some([])) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:228:26 + | +LL | Some(Some(x)) if x.starts_with(&[]) => {}, + | ^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL - Some(Some(x)) if x.starts_with(&[]) => {}, +LL + Some(Some([..])) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:233:26 + | +LL | Some(Some(x)) if x.starts_with(&[1]) => {}, + | ^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL - Some(Some(x)) if x.starts_with(&[1]) => {}, +LL + Some(Some([1, ..])) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:238:26 + | +LL | Some(Some(x)) if x.starts_with(&[1, 2]) => {}, + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL - Some(Some(x)) if x.starts_with(&[1, 2]) => {}, +LL + Some(Some([1, 2, ..])) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:243:26 + | +LL | Some(Some(x)) if x.ends_with(&[1, 2]) => {}, + | ^^^^^^^^^^^^^^^^^^^^ + | +help: try + | +LL - Some(Some(x)) if x.ends_with(&[1, 2]) => {}, +LL + Some(Some([.., 1, 2])) => {}, + | + +error: aborting due to 24 previous errors From 8f9c738ce9b8912571761ac08e50585a77d270c3 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 15 Nov 2023 21:34:48 +0100 Subject: [PATCH 5/5] dogfood clippy --- clippy_lints/src/manual_async_fn.rs | 15 ++- clippy_lints/src/matches/redundant_guards.rs | 98 +++++++++++--------- 2 files changed, 61 insertions(+), 52 deletions(-) diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs index a5d91c949bc..f03cc89e13a 100644 --- a/clippy_lints/src/manual_async_fn.rs +++ b/clippy_lints/src/manual_async_fn.rs @@ -187,14 +187,11 @@ fn desugared_async_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) } fn suggested_ret(cx: &LateContext<'_>, output: &Ty<'_>) -> Option<(&'static str, String)> { - match output.kind { - TyKind::Tup(tys) if tys.is_empty() => { - let sugg = "remove the return type"; - Some((sugg, String::new())) - }, - _ => { - let sugg = "return the output of the future directly"; - snippet_opt(cx, output.span).map(|snip| (sugg, format!(" -> {snip}"))) - }, + if let TyKind::Tup([]) = output.kind { + let sugg = "remove the return type"; + Some((sugg, String::new())) + } else { + let sugg = "return the output of the future directly"; + snippet_opt(cx, output.span).map(|snip| (sugg, format!(" -> {snip}"))) } } diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 365d178f548..f57b22374c8 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -8,7 +8,7 @@ use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, MatchSource, Node, Pat, PatKind}; use rustc_lint::LateContext; use rustc_span::symbol::Ident; -use rustc_span::Span; +use rustc_span::{Span, Symbol}; use std::borrow::Cow; use std::ops::ControlFlow; @@ -105,52 +105,64 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { && let ExprKind::MethodCall(path, recv, args, ..) = if_expr.kind && let Some(binding) = get_pat_binding(cx, recv, outer_arm) { - let ty = cx.typeck_results().expr_ty(recv).peel_refs(); - let slice_like = ty.is_slice() || ty.is_array(); - - let sugg = if path.ident.name == sym!(is_empty) { - // `s if s.is_empty()` becomes "" - // `arr if arr.is_empty()` becomes [] - - if ty.is_str() { - r#""""#.into() - } else if slice_like { - "[]".into() - } else { - continue; - } - } else if slice_like - && let Some(needle) = args.first() - && let ExprKind::AddrOf(.., needle) = needle.kind - && let ExprKind::Array(needles) = needle.kind - && needles.iter().all(|needle| expr_can_be_pat(cx, needle)) - { - // `arr if arr.starts_with(&[123])` becomes [123, ..] - // `arr if arr.ends_with(&[123])` becomes [.., 123] - // `arr if arr.starts_with(&[])` becomes [..] (why would anyone write this?) - - let mut sugg = snippet(cx, needle.span, "").into_owned(); - - if needles.is_empty() { - sugg.insert_str(1, ".."); - } else if path.ident.name == sym!(starts_with) { - sugg.insert_str(sugg.len() - 1, ", .."); - } else if path.ident.name == sym!(ends_with) { - sugg.insert_str(1, ".., "); - } else { - continue; - } - - sugg.into() - } else { - continue; - }; - - emit_redundant_guards(cx, outer_arm, if_expr.span, sugg, &binding, None); + check_method_calls(cx, outer_arm, path.ident.name, recv, args, if_expr, &binding); } } } +fn check_method_calls<'tcx>( + cx: &LateContext<'tcx>, + arm: &Arm<'tcx>, + method: Symbol, + recv: &Expr<'_>, + args: &[Expr<'_>], + if_expr: &Expr<'_>, + binding: &PatBindingInfo, +) { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + let slice_like = ty.is_slice() || ty.is_array(); + + let sugg = if method == sym!(is_empty) { + // `s if s.is_empty()` becomes "" + // `arr if arr.is_empty()` becomes [] + + if ty.is_str() { + r#""""#.into() + } else if slice_like { + "[]".into() + } else { + return; + } + } else if slice_like + && let Some(needle) = args.first() + && let ExprKind::AddrOf(.., needle) = needle.kind + && let ExprKind::Array(needles) = needle.kind + && needles.iter().all(|needle| expr_can_be_pat(cx, needle)) + { + // `arr if arr.starts_with(&[123])` becomes [123, ..] + // `arr if arr.ends_with(&[123])` becomes [.., 123] + // `arr if arr.starts_with(&[])` becomes [..] (why would anyone write this?) + + let mut sugg = snippet(cx, needle.span, "").into_owned(); + + if needles.is_empty() { + sugg.insert_str(1, ".."); + } else if method == sym!(starts_with) { + sugg.insert_str(sugg.len() - 1, ", .."); + } else if method == sym!(ends_with) { + sugg.insert_str(1, ".., "); + } else { + return; + } + + sugg.into() + } else { + return; + }; + + emit_redundant_guards(cx, arm, if_expr.span, sugg, binding, None); +} + struct PatBindingInfo { span: Span, byref_ident: Option,