Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip use_self inside macro expansions of a impl Self block #13128

Merged
merged 6 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 13 additions & 24 deletions clippy_lints/src/use_self.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -12,7 +12,6 @@ use rustc_hir::{
self as hir, AmbigArg, Expr, ExprKind, FnRetTy, FnSig, GenericArgsParentheses, GenericParam, GenericParamKind,
HirId, Impl, ImplItemKind, Item, ItemKind, Pat, 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;
Expand Down Expand Up @@ -73,7 +72,6 @@ impl UseSelf {
enum StackItem {
Check {
impl_id: LocalDefId,
in_body: u32,
types_to_skip: FxHashSet<HirId>,
},
NoCheck,
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}

Expand All @@ -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
Expand All @@ -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
Expand Down
17 changes: 2 additions & 15 deletions clippy_lints/src/zero_sized_map_values.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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))
}
1 change: 1 addition & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 14 additions & 0 deletions clippy_utils/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down
45 changes: 45 additions & 0 deletions tests/ui/use_self.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
}
45 changes: 45 additions & 0 deletions tests/ui/use_self.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}
}