Don't lint if the let ... pattern is through a macro boundary

Author: est31

clippy_lints/src/manual_let_else.rs

@@ -9,6 +9,7 @@ use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
 use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -61,6 +62,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse {
             if !in_external_macro(cx.sess(), stmt.span);
             if let StmtKind::Local(local) = stmt.kind;
             if let Some(init) = local.init;
+            if !from_different_macros(init.span, stmt.span);
             if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init);
             then {
                 if_let_or_match
@@ -178,6 +180,23 @@ fn expr_diverges(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
     does_diverge
 }
 
+/// Returns true if the two spans come from different macro sites,
+/// or one comes from an invocation and the other is not from a macro at all.
+fn from_different_macros(span_a: Span, span_b: Span) -> bool {
+    // This pre-check is a speed up so that we don't build outer_expn_data unless needed.
+    match (span_a.from_expansion(), span_b.from_expansion()) {
+        (false, false) => return false,
+        (true, false) | (false, true) => return true,
+        // We need to determine if both are from the same macro
+        (true, true) => (),
+    }
+    let data_for_comparison = |sp: Span| {
+        let expn_data = sp.ctxt().outer_expn_data();
+        (expn_data.kind, expn_data.call_site)
+    };
+    data_for_comparison(span_a) != data_for_comparison(span_b)
+}
+
 fn pat_has_no_bindings(pat: &'_ Pat<'_>) -> bool {
     let mut has_no_bindings = true;
     pat.each_binding_or_first(&mut |_, _, _, _| has_no_bindings = false);

tests/ui/manual_let_else.rs

@@ -1,5 +1,5 @@
 #![allow(unused_braces, unused_variables, dead_code)]
-#![allow(clippy::collapsible_else_if, clippy::unused_unit)]
+#![allow(clippy::collapsible_else_if, clippy::unused_unit, clippy::never_loop)]
 #![warn(clippy::manual_let_else)]
 
 fn g() -> Option<()> {
@@ -82,6 +82,19 @@ fn fire() {
     } else {
         return;
     };
+
+    // entirely inside macro lints
+    macro_rules! create_binding_if_some {
+        ($n:ident, $e:expr) => {
+            let $n = if let Some(v) = $e {
+                v
+            } else {
+                return
+            };
+        };
+    }
+    create_binding_if_some!(w, g());
+
 }
 
 fn not_fire() {
@@ -147,4 +160,24 @@ fn not_fire() {
         };
         Some(v)
     }
-}
+
+    // Macro boundary inside let
+    macro_rules! some_or_return {
+        ($e:expr) => {
+            if let Some(v) = $e {
+                v
+            } else {
+                return
+            }
+        };
+    }
+    let v = some_or_return!(g());
+
+    // Also macro boundary inside let, but inside a macro
+    macro_rules! create_binding_if_some {
+        ($n:ident, $e:expr) => {
+            let $n = some_or_return!($e);
+        };
+    }
+    create_binding_if_some!(v, g());
+}

tests/ui/manual_let_else.stderr

@@ -100,15 +100,20 @@ LL | |         return;
 LL | |     };
    | |______^
 
-error: this loop never actually loops
-  --> $DIR/manual_let_else.rs:136:9
+error: this could be rewritten as `let else`
+  --> $DIR/manual_let_else.rs:89:13
    |
-LL | /         loop {
-LL | |             break;
-LL | |         }
-   | |_________^
+LL | /             let $n = if let Some(v) = $e {
+LL | |                 v
+LL | |             } else {
+LL | |                 return
+LL | |             };
+   | |______________^
+...
+LL |       create_binding_if_some!(w, g());
+   |       ------------------------------- in this macro invocation
    |
-   = note: `#[deny(clippy::never_loop)]` on by default
+   = note: this error originates in the macro `create_binding_if_some` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: aborting due to 12 previous errors