Auto merge of rust-lang#11996 - J-ZhengLi:issue11992, r=xFrednet,ARan…

…domDev99

fix suggestion for [`len_zero`] with macros

fixes: rust-lang#11992

changelog: fix suggestion for [`len_zero`] with macros

clippy_lints/src/len_zero.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::source::snippet_with_context;
-use clippy_utils::sugg::Sugg;
+use clippy_utils::source::{snippet_opt, snippet_with_context};
+use clippy_utils::sugg::{has_enclosing_paren, Sugg};
 use clippy_utils::{get_item_name, get_parent_as_impl, is_lint_allowed, peel_ref_operators};
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
@@ -192,7 +192,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
 
         if let ExprKind::Binary(Spanned { node: cmp, .. }, left, right) = expr.kind {
             // expr.span might contains parenthesis, see issue #10529
-            let actual_span = left.span.with_hi(right.span.hi());
+            let actual_span = span_without_enclosing_paren(cx, expr.span);
             match cmp {
                 BinOpKind::Eq => {
                     check_cmp(cx, actual_span, left, right, "", 0); // len == 0
@@ -218,6 +218,20 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
     }
 }
 
+fn span_without_enclosing_paren(cx: &LateContext<'_>, span: Span) -> Span {
+    let Some(snippet) = snippet_opt(cx, span) else {
+        return span;
+    };
+    if has_enclosing_paren(snippet) {
+        let source_map = cx.tcx.sess.source_map();
+        let left_paren = source_map.start_point(span);
+        let right_parent = source_map.end_point(span);
+        left_paren.between(right_parent)
+    } else {
+        span
+    }
+}
+
 fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items: &[TraitItemRef]) {
     fn is_named_self(cx: &LateContext<'_>, item: &TraitItemRef, name: Symbol) -> bool {
         item.ident.name == name
@@ -495,6 +509,10 @@ fn check_for_is_empty(
 }
 
 fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) {
+    if method.span.from_expansion() {
+        return;
+    }
+
     if let (&ExprKind::MethodCall(method_path, receiver, args, _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) {
         // check if we are in an is_empty() method
         if let Some(name) = get_item_name(cx, method) {

tests/ui/len_zero.fixed

@@ -189,3 +189,40 @@ fn main() {
 fn test_slice(b: &[u8]) {
     if !b.is_empty() {}
 }
+
+// issue #11992
+fn binop_with_macros() {
+    macro_rules! len {
+        ($seq:ident) => {
+            $seq.len()
+        };
+    }
+
+    macro_rules! compare_to {
+        ($val:literal) => {
+            $val
+        };
+        ($val:expr) => {{ $val }};
+    }
+
+    macro_rules! zero {
+        () => {
+            0
+        };
+    }
+
+    let has_is_empty = HasIsEmpty;
+    // Don't lint, suggesting changes might break macro compatibility.
+    (len!(has_is_empty) > 0).then(|| println!("This can happen."));
+    // Don't lint, suggesting changes might break macro compatibility.
+    if len!(has_is_empty) == 0 {}
+    // Don't lint
+    if has_is_empty.len() == compare_to!(if true { 0 } else { 1 }) {}
+    // This is fine
+    if has_is_empty.len() == compare_to!(1) {}
+
+    if has_is_empty.is_empty() {}
+    if has_is_empty.is_empty() {}
+
+    (!has_is_empty.is_empty()).then(|| println!("This can happen."));
+}

tests/ui/len_zero.rs

@@ -189,3 +189,40 @@ fn main() {
 fn test_slice(b: &[u8]) {
     if b.len() != 0 {}
 }
+
+// issue #11992
+fn binop_with_macros() {
+    macro_rules! len {
+        ($seq:ident) => {
+            $seq.len()
+        };
+    }
+
+    macro_rules! compare_to {
+        ($val:literal) => {
+            $val
+        };
+        ($val:expr) => {{ $val }};
+    }
+
+    macro_rules! zero {
+        () => {
+            0
+        };
+    }
+
+    let has_is_empty = HasIsEmpty;
+    // Don't lint, suggesting changes might break macro compatibility.
+    (len!(has_is_empty) > 0).then(|| println!("This can happen."));
+    // Don't lint, suggesting changes might break macro compatibility.
+    if len!(has_is_empty) == 0 {}
+    // Don't lint
+    if has_is_empty.len() == compare_to!(if true { 0 } else { 1 }) {}
+    // This is fine
+    if has_is_empty.len() == compare_to!(1) {}
+
+    if has_is_empty.len() == compare_to!(0) {}
+    if has_is_empty.len() == zero!() {}
+
+    (compare_to!(0) < has_is_empty.len()).then(|| println!("This can happen."));
+}

tests/ui/len_zero.stderr

@@ -142,5 +142,23 @@ error: length comparison to zero
 LL |     if b.len() != 0 {}
    |        ^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!b.is_empty()`
 
-error: aborting due to 23 previous errors
+error: length comparison to zero
+  --> tests/ui/len_zero.rs:224: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
+   |
+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
+   |
+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