Auto merge of #11818 - y21:more_redundant_guards, r=llogiq

bors · bors · commit 8b0bf6423dfa · 2023-11-29T13:20:59.000Z
[`redundant_guards`]: catch `is_empty`, `starts_with` and `ends_with` on slices and `str`s Fixes #11807 Few things worth mentioning: - Taking `snippet`s is now done at callsite, instead of passing a span and doing it in `emit_redundant_guards`. This is because we now need custom suggestion strings in certain places, like `""` for `str::is_empty`. - This now uses `snippet` instead of `snippet_with_applicability`. I don't think this really makes any difference for `MaybeIncorrect`, though? - This could also lint byte strings, as they're of type `&[u8; N]`, but that can be ugly so I decided to leave it out for now changelog: [`redundant_guards`]: catch `str::is_empty`, `slice::is_empty`, `slice::starts_with` and `slice::ends_with`
diff --git 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
@@ -1,14 +1,15 @@
 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;
 use rustc_hir::def::{DefKind, Res};
 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;
 
 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>"),
+                &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>"),
+                &binding,
+                None,
+            );
         }
         // `Some(x) if x == Some(2)`
         // `Some(x) if Some(2) == x`
@@ -78,11 +93,76 @@ 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>"),
+                &binding,
+                None,
+            );
+        } else if let Guard::If(if_expr) = guard
+            && let ExprKind::MethodCall(path, recv, args, ..) = if_expr.kind
+            && let Some(binding) = get_pat_binding(cx, recv, outer_arm)
+        {
+            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, "<needle>").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<Ident>,
@@ -134,19 +214,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<Guard<'_>>,
 ) {
-    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, "<binding_repl>", &mut app);
             let suggestion_span = match *pat_binding {
                 PatBindingInfo {
                     span,
@@ -170,14 +247,11 @@ fn emit_redundant_guards<'tcx>(
                                 Guard::IfLet(l) => ("if let", l.span),
                             };
 
-                            format!(
-                                " {prefix} {}",
-                                snippet_with_applicability(cx, span, "<guard>", &mut app),
-                            )
+                            format!(" {prefix} {}", snippet(cx, span, "<guard>"))
                         }),
                     ),
                 ],
-                app,
+                Applicability::MaybeIncorrect,
             );
         },
     );
diff --git 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::<i32>::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
@@ -193,3 +193,60 @@ mod 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::<i32>::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
@@ -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
 
