Check if deref target implements is_empty for len_zero lint

clippy_lints/src/eta_reduction.rs

@@ -182,7 +182,7 @@ fn check_clousure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tc
                 // will succeed iff `T: 'static`. But the region of `T` is always erased by `typeck.expr_ty()` when
                 // T is a generic type. For example, return type of `Option<String>::as_deref()` is a generic.
                 // So we have a hack like this.
-                && generic_args.len() > 0
+                && !generic_args.is_empty()
             {
                 return;
             }

clippy_lints/src/len_zero.rs

@@ -1,6 +1,7 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{SpanRangeExt, snippet_with_context};
 use clippy_utils::sugg::{Sugg, has_enclosing_paren};
+use clippy_utils::ty::implements_trait;
 use clippy_utils::{get_item_name, get_parent_as_impl, is_lint_allowed, is_trait_method, peel_ref_operators};
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -620,18 +621,30 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
         })
     }
 
-    let ty = &cx.typeck_results().expr_ty(expr).peel_refs();
-    match ty.kind() {
-        ty::Dynamic(tt, ..) => tt.principal().is_some_and(|principal| {
-            let is_empty = sym!(is_empty);
-            cx.tcx
-                .associated_items(principal.def_id())
-                .filter_by_name_unhygienic(is_empty)
-                .any(|item| is_is_empty(cx, item))
-        }),
-        ty::Alias(ty::Projection, proj) => has_is_empty_impl(cx, proj.def_id),
-        ty::Adt(id, _) => has_is_empty_impl(cx, id.did()),
-        ty::Array(..) | ty::Slice(..) | ty::Str => true,
-        _ => false,
+    fn ty_has_is_empty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, depth: usize) -> bool {
+        match ty.kind() {
+            ty::Dynamic(tt, ..) => tt.principal().is_some_and(|principal| {
+                let is_empty = sym!(is_empty);
+                cx.tcx
+                    .associated_items(principal.def_id())
+                    .filter_by_name_unhygienic(is_empty)
+                    .any(|item| is_is_empty(cx, item))
+            }),
+            ty::Alias(ty::Projection, proj) => has_is_empty_impl(cx, proj.def_id),
+            ty::Adt(id, _) => {
+                has_is_empty_impl(cx, id.did())
+                    || (cx.tcx.recursion_limit().value_within_limit(depth)
+                        && cx.tcx.get_diagnostic_item(sym::Deref).is_some_and(|deref_id| {
+                            implements_trait(cx, ty, deref_id, &[])
+                                && cx
+                                    .get_associated_type(ty, deref_id, "Target")
+                                    .is_some_and(|deref_ty| ty_has_is_empty(cx, deref_ty, depth + 1))
+                        }))
+            },
+            ty::Array(..) | ty::Slice(..) | ty::Str => true,
+            _ => false,
+        }
     }
+
+    ty_has_is_empty(cx, cx.typeck_results().expr_ty(expr).peel_refs(), 0)
 }

tests/ui/len_zero.fixed

@@ -108,6 +108,8 @@ fn main() {
     let d2s = DerefToDerefToString {};
     println!("{}", (**d2s).is_empty());
 
+    println!("{}", std::borrow::Cow::Borrowed("").is_empty());
+
     let y = One;
     if y.len() == 0 {
         // No error; `One` does not have `.is_empty()`.
@@ -226,3 +228,23 @@ fn binop_with_macros() {
 
     (!has_is_empty.is_empty()).then(|| println!("This can happen."));
 }
+
+fn no_infinite_recursion() -> bool {
+    struct S;
+
+    impl Deref for S {
+        type Target = Self;
+        fn deref(&self) -> &Self::Target {
+            self
+        }
+    }
+
+    impl PartialEq<&'static str> for S {
+        fn eq(&self, _other: &&'static str) -> bool {
+            false
+        }
+    }
+
+    // Do not crash while checking if S implements `.is_empty()`
+    S == ""
+}

tests/ui/len_zero.rs

@@ -108,6 +108,8 @@ fn main() {
     let d2s = DerefToDerefToString {};
     println!("{}", &**d2s == "");
 
+    println!("{}", std::borrow::Cow::Borrowed("") == "");
+
     let y = One;
     if y.len() == 0 {
         // No error; `One` does not have `.is_empty()`.
@@ -226,3 +228,23 @@ fn binop_with_macros() {
 
     (compare_to!(0) < has_is_empty.len()).then(|| println!("This can happen."));
 }
+
+fn no_infinite_recursion() -> bool {
+    struct S;
+
+    impl Deref for S {
+        type Target = Self;
+        fn deref(&self) -> &Self::Target {
+            self
+        }
+    }
+
+    impl PartialEq<&'static str> for S {
+        fn eq(&self, _other: &&'static str) -> bool {
+            false
+        }
+    }
+
+    // Do not crash while checking if S implements `.is_empty()`
+    S == ""
+}

tests/ui/len_zero.stderr

@@ -58,107 +58,113 @@ error: comparison to empty slice
 LL |     println!("{}", &**d2s == "");
    |                    ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(**d2s).is_empty()`
 
+error: comparison to empty slice
+  --> tests/ui/len_zero.rs:111:20
+   |
+LL |     println!("{}", std::borrow::Cow::Borrowed("") == "");
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `std::borrow::Cow::Borrowed("").is_empty()`
+
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:124:8
+  --> tests/ui/len_zero.rs:126:8
    |
 LL |     if has_is_empty.len() == 0 {
    |        ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:127:8
+  --> tests/ui/len_zero.rs:129:8
    |
 LL |     if has_is_empty.len() != 0 {
    |        ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:130:8
+  --> tests/ui/len_zero.rs:132:8
    |
 LL |     if has_is_empty.len() > 0 {
    |        ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
 error: length comparison to one
-  --> tests/ui/len_zero.rs:133:8
+  --> tests/ui/len_zero.rs:135:8
    |
 LL |     if has_is_empty.len() < 1 {
    |        ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
 
 error: length comparison to one
-  --> tests/ui/len_zero.rs:136:8
+  --> tests/ui/len_zero.rs:138:8
    |
 LL |     if has_is_empty.len() >= 1 {
    |        ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:147:8
+  --> tests/ui/len_zero.rs:149:8
    |
 LL |     if 0 == has_is_empty.len() {
    |        ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:150:8
+  --> tests/ui/len_zero.rs:152:8
    |
 LL |     if 0 != has_is_empty.len() {
    |        ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:153:8
+  --> tests/ui/len_zero.rs:155:8
    |
 LL |     if 0 < has_is_empty.len() {
    |        ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
 error: length comparison to one
-  --> tests/ui/len_zero.rs:156:8
+  --> tests/ui/len_zero.rs:158:8
    |
 LL |     if 1 <= has_is_empty.len() {
    |        ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
 error: length comparison to one
-  --> tests/ui/len_zero.rs:159:8
+  --> tests/ui/len_zero.rs:161:8
    |
 LL |     if 1 > has_is_empty.len() {
    |        ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:173:8
+  --> tests/ui/len_zero.rs:175:8
    |
 LL |     if with_is_empty.len() == 0 {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `with_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:185:6
+  --> tests/ui/len_zero.rs:187:6
    |
 LL |     (has_is_empty.len() > 0).then(|| println!("This can happen."));
    |      ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:186:6
+  --> tests/ui/len_zero.rs:188:6
    |
 LL |     (has_is_empty.len() == 0).then(|| println!("Or this!"));
    |      ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:190:8
+  --> tests/ui/len_zero.rs:192:8
    |
 LL |     if b.len() != 0 {}
    |        ^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!b.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:224:8
+  --> tests/ui/len_zero.rs:226:8
    |
 LL |     if has_is_empty.len() == compare_to!(0) {}
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:225:8
+  --> tests/ui/len_zero.rs:227:8
    |
 LL |     if has_is_empty.len() == zero!() {}
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
 
 error: length comparison to zero
-  --> tests/ui/len_zero.rs:227:6
+  --> tests/ui/len_zero.rs:229:6
    |
 LL |     (compare_to!(0) < has_is_empty.len()).then(|| println!("This can happen."));
    |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
 
-error: aborting due to 26 previous errors
+error: aborting due to 27 previous errors