about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2024-12-24 00:01:39 +0100
committerSamuel Tardieu <sam@rfc1149.net>2025-01-01 14:14:39 +0100
commit4c9c2cca2b3fdb0e926ca41555c27e53dff6b9a6 (patch)
treea32381dd8d23c56fe3b4c2cd4a2e38b5aa254583
parentb3fadd5a8ef45bcd24e9dc31d32926cb7ad26444 (diff)
downloadrust-4c9c2cca2b3fdb0e926ca41555c27e53dff6b9a6.tar.gz
rust-4c9c2cca2b3fdb0e926ca41555c27e53dff6b9a6.zip
Check if deref target implements `is_empty` for `len_zero` lint
In this case, the lint can be triggered as well as `is_empty()` will be
found on the target type.
-rw-r--r--clippy_lints/src/eta_reduction.rs2
-rw-r--r--clippy_lints/src/len_zero.rs39
-rw-r--r--tests/ui/len_zero.fixed22
-rw-r--r--tests/ui/len_zero.rs22
-rw-r--r--tests/ui/len_zero.stderr42
5 files changed, 95 insertions, 32 deletions
diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
index 8c22e43349f..f658e5d7922 100644
--- a/clippy_lints/src/eta_reduction.rs
+++ b/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;
             }
diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs
index 3ea758e176f..fb93a195b83 100644
--- a/clippy_lints/src/len_zero.rs
+++ b/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)
 }
diff --git a/tests/ui/len_zero.fixed b/tests/ui/len_zero.fixed
index 27319d9c20e..c9c476ba421 100644
--- a/tests/ui/len_zero.fixed
+++ b/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 == ""
+}
diff --git a/tests/ui/len_zero.rs b/tests/ui/len_zero.rs
index 03c05bc6ed7..610a5448d10 100644
--- a/tests/ui/len_zero.rs
+++ b/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 == ""
+}
diff --git a/tests/ui/len_zero.stderr b/tests/ui/len_zero.stderr
index 5c849a2aca6..8d6b57e4b6d 100644
--- a/tests/ui/len_zero.stderr
+++ b/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