Skip use_self inside macro expansions of a impl Self block (rust-lang#13128)

Alexendoo · web-flow · commit 20b2461938ca · 2025-02-06T14:43:39.000Z
changelog: [`use_self`] Skip if inside macro expansions of a `impl Self` block Fixes rust-lang#13092. r? Alexendoo
diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
@@ -2,7 +2,7 @@ use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::is_from_proc_macro;
 use clippy_utils::msrvs::{self, Msrv};
-use clippy_utils::ty::same_type_and_consts;
+use clippy_utils::ty::{same_type_and_consts, ty_from_hir_ty};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
 use rustc_hir::def::{CtorOf, DefKind, Res};
@@ -12,7 +12,6 @@ use rustc_hir::{
     self as hir, AmbigArg, Expr, ExprKind, FnRetTy, FnSig, GenericArgsParentheses, GenericParam, GenericParamKind,
     HirId, Impl, ImplItemKind, Item, ItemKind, Pat, PatExpr, PatExprKind, PatKind, Path, QPath, Ty, TyKind,
 };
-use rustc_hir_analysis::lower_ty;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::Ty as MiddleTy;
 use rustc_session::impl_lint_pass;
@@ -73,7 +72,6 @@ impl UseSelf {
 enum StackItem {
     Check {
         impl_id: LocalDefId,
-        in_body: u32,
         types_to_skip: FxHashSet<HirId>,
     },
     NoCheck,
@@ -96,8 +94,8 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
                 .as_ref()
                 .is_none_or(|params| params.parenthesized == GenericArgsParentheses::No)
             && !item.span.from_expansion()
+            // expensive, should be last check
             && !is_from_proc_macro(cx, item)
-        // expensive, should be last check
         {
             // Self cannot be used inside const generic parameters
             let types_to_skip = generics
@@ -117,7 +115,6 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
                 .collect();
             StackItem::Check {
                 impl_id: item.owner_id.def_id,
-                in_body: 0,
                 types_to_skip,
             }
         } else {
@@ -131,6 +128,11 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
     }
 
     fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
+        // Checking items of `impl Self` blocks in which macro expands into.
+        if impl_item.span.from_expansion() {
+            self.stack.push(StackItem::NoCheck);
+            return;
+        }
         // We want to skip types in trait `impl`s that aren't declared as `Self` in the trait
         // declaration. The collection of those types is all this method implementation does.
         if let ImplItemKind::Fn(FnSig { decl, .. }, ..) = impl_item.kind
@@ -186,18 +188,11 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
         }
     }
 
-    fn check_body(&mut self, _: &LateContext<'_>, _: &hir::Body<'_>) {
-        // `lower_ty` cannot be called in `Body`s or it will panic (sometimes). But in bodies
-        // we can use `cx.typeck_results.node_type(..)` to get the `ty::Ty` from a `hir::Ty`.
-        // However the `node_type()` method can *only* be called in bodies.
-        if let Some(&mut StackItem::Check { ref mut in_body, .. }) = self.stack.last_mut() {
-            *in_body = in_body.saturating_add(1);
-        }
-    }
-
-    fn check_body_post(&mut self, _: &LateContext<'_>, _: &hir::Body<'_>) {
-        if let Some(&mut StackItem::Check { ref mut in_body, .. }) = self.stack.last_mut() {
-            *in_body = in_body.saturating_sub(1);
+    fn check_impl_item_post(&mut self, _: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
+        if impl_item.span.from_expansion()
+            && let Some(StackItem::NoCheck) = self.stack.last()
+        {
+            self.stack.pop();
         }
     }
 
@@ -206,7 +201,6 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
             && self.msrv.meets(msrvs::TYPE_ALIAS_ENUM_VARIANTS)
             && let Some(&StackItem::Check {
                 impl_id,
-                in_body,
                 ref types_to_skip,
             }) = self.stack.last()
             && let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind
@@ -215,12 +209,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
                 Res::SelfTyParam { .. } | Res::SelfTyAlias { .. } | Res::Def(DefKind::TyParam, _)
             )
             && !types_to_skip.contains(&hir_ty.hir_id)
-            && let ty = if in_body > 0 {
-                cx.typeck_results().node_type(hir_ty.hir_id)
-            } else {
-                // We don't care about ignoring infer vars here
-                lower_ty(cx.tcx, hir_ty.as_unambig_ty())
-            }
+            && let ty = ty_from_hir_ty(cx, hir_ty.as_unambig_ty())
             && let impl_ty = cx.tcx.type_of(impl_id).instantiate_identity()
             && same_type_and_consts(ty, impl_ty)
             // Ensure the type we encounter and the one from the impl have the same lifetime parameters. It may be that
diff --git a/clippy_lints/src/zero_sized_map_values.rs b/clippy_lints/src/zero_sized_map_values.rs
@@ -1,10 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_help;
-use clippy_utils::ty::{is_normalizable, is_type_diagnostic_item};
+use clippy_utils::ty::{is_normalizable, is_type_diagnostic_item, ty_from_hir_ty};
 use rustc_hir::{self as hir, AmbigArg, HirId, ItemKind, Node};
-use rustc_hir_analysis::lower_ty;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::layout::LayoutOf as _;
-use rustc_middle::ty::{self, Ty, TypeVisitableExt};
+use rustc_middle::ty::{self, TypeVisitableExt};
 use rustc_session::declare_lint_pass;
 use rustc_span::sym;
 
@@ -82,15 +81,3 @@ fn in_trait_impl(cx: &LateContext<'_>, hir_id: HirId) -> bool {
     }
     false
 }
-
-fn ty_from_hir_ty<'tcx>(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> {
-    cx.maybe_typeck_results()
-        .and_then(|results| {
-            if results.hir_owner == hir_ty.hir_id.owner {
-                results.node_type_opt(hir_ty.hir_id)
-            } else {
-                None
-            }
-        })
-        .unwrap_or_else(|| lower_ty(cx.tcx, hir_ty))
-}
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
@@ -39,6 +39,7 @@ extern crate rustc_data_structures;
 extern crate rustc_driver;
 extern crate rustc_errors;
 extern crate rustc_hir;
+extern crate rustc_hir_analysis;
 extern crate rustc_hir_typeck;
 extern crate rustc_index;
 extern crate rustc_infer;
diff --git a/clippy_utils/src/ty/mod.rs b/clippy_utils/src/ty/mod.rs
@@ -10,6 +10,7 @@ use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::{Expr, FnDecl, LangItem, TyKind};
+use rustc_hir_analysis::lower_ty;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::LateContext;
 use rustc_middle::mir::ConstValue;
@@ -36,6 +37,19 @@ use crate::{def_path_def_ids, match_def_path, path_res};
 mod type_certainty;
 pub use type_certainty::expr_type_is_certain;
 
+/// Lower a [`hir::Ty`] to a [`rustc_middle::Ty`].
+pub fn ty_from_hir_ty<'tcx>(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> {
+    cx.maybe_typeck_results()
+        .and_then(|results| {
+            if results.hir_owner == hir_ty.hir_id.owner {
+                results.node_type_opt(hir_ty.hir_id)
+            } else {
+                None
+            }
+        })
+        .unwrap_or_else(|| lower_ty(cx.tcx, hir_ty))
+}
+
 /// Checks if the given type implements copy.
 pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
     cx.type_is_copy_modulo_regions(ty)
diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed
@@ -667,3 +667,48 @@ mod issue_10371 {
         }
     }
 }
+
+mod issue_13092 {
+    use std::cell::RefCell;
+    macro_rules! macro_inner_item {
+        ($ty:ty) => {
+            fn foo(_: $ty) {
+                fn inner(_: $ty) {}
+            }
+        };
+    }
+
+    #[derive(Default)]
+    struct MyStruct;
+
+    impl MyStruct {
+        macro_inner_item!(MyStruct);
+    }
+
+    impl MyStruct {
+        thread_local! {
+            static SPECIAL: RefCell<MyStruct> = RefCell::default();
+        }
+    }
+}
+
+mod crash_check_13128 {
+    struct A;
+
+    impl A {
+        fn a() {
+            struct B;
+
+            // pushes a NoCheck
+            impl Iterator for &B {
+                // Pops the NoCheck
+                type Item = A;
+
+                // Lints A -> Self
+                fn next(&mut self) -> Option<A> {
+                    Some(A)
+                }
+            }
+        }
+    }
+}
diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs
@@ -667,3 +667,48 @@ mod issue_10371 {
         }
     }
 }
+
+mod issue_13092 {
+    use std::cell::RefCell;
+    macro_rules! macro_inner_item {
+        ($ty:ty) => {
+            fn foo(_: $ty) {
+                fn inner(_: $ty) {}
+            }
+        };
+    }
+
+    #[derive(Default)]
+    struct MyStruct;
+
+    impl MyStruct {
+        macro_inner_item!(MyStruct);
+    }
+
+    impl MyStruct {
+        thread_local! {
+            static SPECIAL: RefCell<MyStruct> = RefCell::default();
+        }
+    }
+}
+
+mod crash_check_13128 {
+    struct A;
+
+    impl A {
+        fn a() {
+            struct B;
+
+            // pushes a NoCheck
+            impl Iterator for &B {
+                // Pops the NoCheck
+                type Item = A;
+
+                // Lints A -> Self
+                fn next(&mut self) -> Option<A> {
+                    Some(A)
+                }
+            }
+        }
+    }
+}
