about summary refs log tree commit diff
diff options
context:
space:
mode:
authorflip1995 <hello@philkrones.com>2020-10-16 13:25:38 +0200
committerflip1995 <philipp.krones@embecosm.com>2021-02-10 11:57:28 +0100
commitae2dd671f5003e6722ca4d18ef447141db237213 (patch)
treeaf27af439fc4d34ade940d8c946655414248951c
parentfc334fb8f4cc7e6513578d88f52b2899f624a1de (diff)
downloadrust-ae2dd671f5003e6722ca4d18ef447141db237213.tar.gz
rust-ae2dd671f5003e6722ca4d18ef447141db237213.zip
Rewrite use_self lint one more time
This rewrite gets rid of complicated visitors, by using the lint infrastructure as much as possible
-rw-r--r--clippy_lints/src/use_self.rs604
1 files changed, 344 insertions, 260 deletions
diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index 023fce947b3..fbd996ad0e9 100644
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -1,21 +1,22 @@
-use crate::utils;
-use crate::utils::snippet_opt;
-use crate::utils::span_lint_and_sugg;
+use crate::utils::{meets_msrv, qpath_res, snippet_opt, span_lint_and_sugg};
 use if_chain::if_chain;
+
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_hir::def::DefKind;
-use rustc_hir::intravisit::{walk_expr, walk_impl_item, walk_ty, NestedVisitorMap, Visitor};
 use rustc_hir::{
-    def, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, ImplItem, ImplItemKind, ItemKind, Node, Path, PathSegment,
+    def,
+    def_id::LocalDefId,
+    intravisit::{walk_ty, NestedVisitorMap, Visitor},
+    Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, PathSegment,
     QPath, TyKind,
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::hir::map::Map;
 use rustc_middle::lint::in_external_macro;
-use rustc_middle::ty;
-use rustc_middle::ty::Ty;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_middle::ty::{AssocKind, Ty};
+use rustc_semver::RustcVersion;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{BytePos, Span};
 use rustc_typeck::hir_ty_to_ty;
 
@@ -28,9 +29,11 @@ declare_clippy_lint! {
     /// feels inconsistent.
     ///
     /// **Known problems:**
-    /// Unaddressed false negatives related to unresolved internal compiler errors.
+    /// - Unaddressed false negative in fn bodies of trait implementations
+    /// - False positive with assotiated types in traits (#4140)
     ///
     /// **Example:**
+    ///
     /// ```rust
     /// struct Foo {}
     /// impl Foo {
@@ -53,113 +56,216 @@ declare_clippy_lint! {
     "unnecessary structure name repetition whereas `Self` is applicable"
 }
 
-declare_lint_pass!(UseSelf => [USE_SELF]);
+#[derive(Default)]
+pub struct UseSelf {
+    msrv: Option<RustcVersion>,
+    stack: Vec<StackItem>,
+    types_to_skip: Vec<HirId>,
+    types_to_lint: Vec<HirId>,
+}
 
-const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
+const USE_SELF_MSRV: RustcVersion = RustcVersion::new(1, 37, 0);
 
-fn span_lint(cx: &LateContext<'_>, span: Span) {
-    span_lint_and_sugg(
-        cx,
-        USE_SELF,
-        span,
-        "unnecessary structure name repetition",
-        "use the applicable keyword",
-        "Self".to_owned(),
-        Applicability::MachineApplicable,
-    );
+impl UseSelf {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self {
+            msrv,
+            ..Self::default()
+        }
+    }
 }
 
-#[allow(clippy::cast_possible_truncation)]
-fn span_lint_until_last_segment<'tcx>(cx: &LateContext<'tcx>, span: Span, segment: &'tcx PathSegment<'tcx>) {
-    let sp = span.with_hi(segment.ident.span.lo());
-    // remove the trailing ::
-    let span_without_last_segment = match snippet_opt(cx, sp) {
-        Some(snippet) => match snippet.rfind("::") {
-            Some(bidx) => sp.with_hi(sp.lo() + BytePos(bidx as u32)),
-            None => sp,
-        },
-        None => sp,
-    };
-    span_lint(cx, span_without_last_segment);
+#[derive(Debug, PartialEq, Eq, Copy, Clone)]
+enum StackItem {
+    Check {
+        hir_id: HirId,
+        impl_trait_ref_def_id: Option<LocalDefId>,
+    },
+    NoCheck,
 }
 
-fn span_lint_on_path_until_last_segment<'tcx>(cx: &LateContext<'tcx>, path: &'tcx Path<'tcx>) {
-    if path.segments.len() > 1 {
-        span_lint_until_last_segment(cx, path.span, path.segments.last().unwrap());
+impl_lint_pass!(UseSelf => [USE_SELF]);
+
+const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
+
+impl<'tcx> LateLintPass<'tcx> for UseSelf {
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        // We push the self types of `impl`s on a stack here. Only the top type on the stack is
+        // relevant for linting, since this is the self type of the `impl` we're currently in. To
+        // avoid linting on nested items, we push `StackItem::NoCheck` on the stack to signal, that
+        // we're in an `impl` or nested item, that we don't want to lint
+        //
+        // NB: If you push something on the stack in this method, remember to also pop it in the
+        // `check_item_post` method.
+        match &item.kind {
+            ItemKind::Impl(Impl {
+                self_ty: hir_self_ty,
+                of_trait,
+                ..
+            }) => {
+                let should_check = if let TyKind::Path(QPath::Resolved(_, ref item_path)) = hir_self_ty.kind {
+                    let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
+                    parameters.as_ref().map_or(true, |params| {
+                        !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)))
+                    })
+                } else {
+                    false
+                };
+                let impl_trait_ref_def_id = of_trait.as_ref().map(|_| cx.tcx.hir().local_def_id(item.hir_id));
+                if should_check {
+                    self.stack.push(StackItem::Check {
+                        hir_id: hir_self_ty.hir_id,
+                        impl_trait_ref_def_id,
+                    });
+                } else {
+                    self.stack.push(StackItem::NoCheck);
+                }
+            },
+            ItemKind::Static(..)
+            | ItemKind::Const(..)
+            | ItemKind::Fn(..)
+            | ItemKind::Enum(..)
+            | ItemKind::Struct(..)
+            | ItemKind::Union(..)
+            | ItemKind::Trait(..) => {
+                self.stack.push(StackItem::NoCheck);
+            },
+            _ => (),
+        }
     }
-}
 
-fn span_lint_on_qpath_resolved<'tcx>(cx: &LateContext<'tcx>, qpath: &'tcx QPath<'tcx>, until_last_segment: bool) {
-    if let QPath::Resolved(_, path) = qpath {
-        if until_last_segment {
-            span_lint_on_path_until_last_segment(cx, path);
-        } else {
-            span_lint(cx, path.span);
+    fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
+        use ItemKind::{Const, Enum, Fn, Impl, Static, Struct, Trait, Union};
+        match item.kind {
+            Impl { .. } | Static(..) | Const(..) | Fn(..) | Enum(..) | Struct(..) | Union(..) | Trait(..) => {
+                self.stack.pop();
+            },
+            _ => (),
         }
     }
-}
 
-struct BodyVisitor<'a, 'tcx> {
-    cx: &'a LateContext<'tcx>,
-    self_ty: Ty<'tcx>,
-}
+    fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
+        // 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_chain! {
+            if let ImplItemKind::Fn(FnSig { decl, .. }, ..) = impl_item.kind;
+            if let Some(StackItem::Check { impl_trait_ref_def_id: Some(def_id), .. }) = self.stack.last().copied();
+            if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(def_id);
+            then {
+                // `self_ty` is the semantic self type of `impl <trait> for <type>`. This cannot be
+                // `Self`.
+                let self_ty = impl_trait_ref.self_ty();
+
+                // `trait_method_sig` is the signature of the function, how it is declared in the
+                // trait, not in the impl of the trait.
+                let trait_method = cx
+                    .tcx
+                    .associated_items(impl_trait_ref.def_id)
+                    .find_by_name_and_kind(cx.tcx, impl_item.ident, AssocKind::Fn, impl_trait_ref.def_id)
+                    .expect("impl method matches a trait method");
+                let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
+                let trait_method_sig = cx.tcx.erase_late_bound_regions(trait_method_sig);
+
+                // `impl_inputs_outputs` is an iterator over the types (`hir::Ty`) declared in the
+                // implementation of the trait.
+                let output_hir_ty = if let FnRetTy::Return(ty) = &decl.output {
+                    Some(&**ty)
+                } else {
+                    None
+                };
+                let impl_inputs_outputs = decl.inputs.iter().chain(output_hir_ty);
+
+                // `impl_hir_ty` (of type `hir::Ty`) represents the type written in the signature.
+                //
+                // `trait_sem_ty` (of type `ty::Ty`) is the semantic type for the signature in the
+                // trait declaration. This is used to check if `Self` was used in the trait
+                // declaration.
+                //
+                // If `any`where in the `trait_sem_ty` the `self_ty` was used verbatim (as opposed
+                // to `Self`), we want to skip linting that type and all subtypes of it. This
+                // avoids suggestions to e.g. replace `Vec<u8>` with `Vec<Self>`, in an `impl Trait
+                // for u8`, when the trait always uses `Vec<u8>`.
+                //
+                // See also https://github.com/rust-lang/rust-clippy/issues/2894.
+                for (impl_hir_ty, trait_sem_ty) in impl_inputs_outputs.zip(trait_method_sig.inputs_and_output) {
+                    if trait_sem_ty.walk().any(|inner| inner == self_ty.into()) {
+                        let mut visitor = SkipTyCollector::default();
+                        visitor.visit_ty(&impl_hir_ty);
+                        self.types_to_skip.extend(visitor.types_to_skip);
+                    }
+                }
+            }
+        }
+    }
 
-impl<'a, 'tcx> BodyVisitor<'a, 'tcx> {
-    fn check_trait_method_impl_decl(
-        &mut self,
-        impl_item: &ImplItem<'tcx>,
-        impl_decl: &'tcx FnDecl<'tcx>,
-        impl_trait_ref: ty::TraitRef<'tcx>,
-    ) {
-        let tcx = self.cx.tcx;
-        let trait_method = tcx
-            .associated_items(impl_trait_ref.def_id)
-            .find_by_name_and_kind(tcx, impl_item.ident, ty::AssocKind::Fn, impl_trait_ref.def_id)
-            .expect("impl method matches a trait method");
-
-        let trait_method_sig = tcx.fn_sig(trait_method.def_id);
-        let trait_method_sig = tcx.erase_late_bound_regions(&trait_method_sig);
-
-        let output_hir_ty = if let FnRetTy::Return(ty) = &impl_decl.output {
-            Some(&**ty)
-        } else {
-            None
-        };
+    fn check_impl_item_post(&mut self, _: &LateContext<'_>, _: &hir::ImplItem<'_>) {
+        self.types_to_skip.clear();
+    }
 
-        // `impl_hir_ty` (of type `hir::Ty`) represents the type written in the signature.
-        // `trait_ty` (of type `ty::Ty`) is the semantic type for the signature in the trait.
-        // We use `impl_hir_ty` to see if the type was written as `Self`,
-        // `hir_ty_to_ty(...)` to check semantic types of paths, and
-        // `trait_ty` to determine which parts of the signature in the trait, mention
-        // the type being implemented verbatim (as opposed to `Self`).
-        for (impl_hir_ty, trait_ty) in impl_decl
-            .inputs
-            .iter()
-            .chain(output_hir_ty)
-            .zip(trait_method_sig.inputs_and_output)
-        {
-            // Check if the input/output type in the trait method specifies the implemented
-            // type verbatim, and only suggest `Self` if that isn't the case.
-            // This avoids suggestions to e.g. replace `Vec<u8>` with `Vec<Self>`,
-            // in an `impl Trait for u8`, when the trait always uses `Vec<u8>`.
-            // See also https://github.com/rust-lang/rust-clippy/issues/2894.
-            let self_ty = impl_trait_ref.self_ty();
-            if !trait_ty.walk().any(|inner| inner == self_ty.into()) {
-                self.visit_ty(&impl_hir_ty);
-            }
+    fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) {
+        // `hir_ty_to_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.
+        //
+        // This method implementation determines which types should get linted in a `Body` and
+        // which shouldn't, with a visitor. We could directly lint in the visitor, but then we
+        // could only allow this lint on item scope. And we would have to check if those types are
+        // already dealt with in `check_ty` anyway.
+        if let Some(StackItem::Check { hir_id, .. }) = self.stack.last() {
+            let self_ty = ty_from_hir_id(cx, *hir_id);
+
+            let mut visitor = LintTyCollector {
+                cx,
+                self_ty,
+                types_to_lint: vec![],
+                types_to_skip: vec![],
+            };
+            visitor.visit_expr(&body.value);
+            self.types_to_lint.extend(visitor.types_to_lint);
+            self.types_to_skip.extend(visitor.types_to_skip);
         }
     }
-}
 
-impl<'a, 'tcx> Visitor<'tcx> for BodyVisitor<'a, 'tcx> {
-    type Map = Map<'tcx>;
+    fn check_body_post(&mut self, _: &LateContext<'_>, _: &hir::Body<'_>) {
+        self.types_to_lint.clear();
+    }
 
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
+    fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) {
+        if in_external_macro(cx.sess(), hir_ty.span)
+            | in_impl(cx, hir_ty)
+            | self.types_to_skip.contains(&hir_ty.hir_id)
+            | !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV)
+        {
+            return;
+        }
+
+        let lint_dependend_on_expr_kind = || {
+            // FIXME: this span manipulation should not be necessary
+            // @flip1995 found an ast lowering issue in
+            // https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#l142-l162
+            match cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_ty.hir_id)) {
+                Some(Node::Expr(Expr {
+                    kind: ExprKind::Path(QPath::TypeRelative(_, segment)),
+                    ..
+                })) => span_lint_until_last_segment(cx, hir_ty.span, segment),
+                _ => span_lint(cx, hir_ty.span),
+            }
+        };
+
+        if self.types_to_lint.contains(&hir_ty.hir_id) {
+            lint_dependend_on_expr_kind();
+        } else if let Some(StackItem::Check { hir_id, .. }) = self.stack.last() {
+            let self_ty = ty_from_hir_id(cx, *hir_id);
+
+            if should_lint_ty(hir_ty, hir_ty_to_ty(cx.tcx, hir_ty), self_ty) {
+                lint_dependend_on_expr_kind();
+            }
+        }
     }
 
-    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
-        fn expr_ty_matches<'tcx>(expr: &'tcx Expr<'tcx>, self_ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        fn expr_ty_matches(cx: &LateContext<'_>, expr: &Expr<'_>, self_ty: Ty<'_>) -> bool {
             let def_id = expr.hir_id.owner;
             if cx.tcx.has_typeck_results(def_id) {
                 cx.tcx.typeck(def_id).expr_ty_opt(expr) == Some(self_ty)
@@ -167,205 +273,183 @@ impl<'a, 'tcx> Visitor<'tcx> for BodyVisitor<'a, 'tcx> {
                 false
             }
         }
-        match expr.kind {
-            ExprKind::Struct(QPath::Resolved(_, path), ..) => {
-                if expr_ty_matches(expr, self.self_ty, self.cx) {
-                    match path.res {
-                        def::Res::SelfTy(..) => (),
-                        def::Res::Def(DefKind::Variant, _) => span_lint_on_path_until_last_segment(self.cx, path),
-                        _ => {
-                            span_lint(self.cx, path.span);
-                        },
+
+        if in_external_macro(cx.sess(), expr.span) | !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV) {
+            return;
+        }
+
+        if let Some(StackItem::Check { hir_id, .. }) = self.stack.last() {
+            let self_ty = ty_from_hir_id(cx, *hir_id);
+
+            match &expr.kind {
+                ExprKind::Struct(QPath::Resolved(_, path), ..) => {
+                    if expr_ty_matches(cx, expr, self_ty) {
+                        match path.res {
+                            def::Res::SelfTy(..) => (),
+                            def::Res::Def(DefKind::Variant, _) => span_lint_on_path_until_last_segment(cx, path),
+                            _ => {
+                                span_lint(cx, path.span);
+                            },
+                        }
                     }
-                }
-            },
-            // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`)
-            ExprKind::Call(fun, _) => {
-                if let Expr {
-                    kind: ExprKind::Path(ref qpath),
-                    ..
-                } = fun
-                {
-                    if expr_ty_matches(expr, self.self_ty, self.cx) {
-                        let res = utils::qpath_res(self.cx, qpath, fun.hir_id);
-
-                        if let def::Res::Def(DefKind::Ctor(ctor_of, _), ..) = res {
-                            match ctor_of {
-                                def::CtorOf::Variant => {
-                                    span_lint_on_qpath_resolved(self.cx, qpath, true);
-                                },
-                                def::CtorOf::Struct => {
-                                    span_lint_on_qpath_resolved(self.cx, qpath, false);
-                                },
+                },
+                // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`)
+                ExprKind::Call(fun, _) => {
+                    if let Expr {
+                        kind: ExprKind::Path(ref qpath),
+                        ..
+                    } = fun
+                    {
+                        if expr_ty_matches(cx, expr, self_ty) {
+                            let res = qpath_res(cx, qpath, fun.hir_id);
+
+                            if let def::Res::Def(DefKind::Ctor(ctor_of, _), ..) = res {
+                                match ctor_of {
+                                    def::CtorOf::Variant => {
+                                        span_lint_on_qpath_resolved(cx, qpath, true);
+                                    },
+                                    def::CtorOf::Struct => {
+                                        span_lint_on_qpath_resolved(cx, qpath, false);
+                                    },
+                                }
                             }
                         }
                     }
-                }
-            },
-            // unit enum variants (`Enum::A`)
-            ExprKind::Path(ref qpath) => {
-                if expr_ty_matches(expr, self.self_ty, self.cx) {
-                    span_lint_on_qpath_resolved(self.cx, qpath, true);
-                }
-            },
-            _ => (),
+                },
+                // unit enum variants (`Enum::A`)
+                ExprKind::Path(qpath) => {
+                    if expr_ty_matches(cx, expr, self_ty) {
+                        span_lint_on_qpath_resolved(cx, &qpath, true);
+                    }
+                },
+                _ => (),
+            }
         }
-        walk_expr(self, expr);
     }
+
+    extract_msrv_attr!(LateContext);
 }
 
-struct FnSigVisitor<'a, 'tcx> {
-    cx: &'a LateContext<'tcx>,
-    self_ty: Ty<'tcx>,
+#[derive(Default)]
+struct SkipTyCollector {
+    types_to_skip: Vec<HirId>,
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for FnSigVisitor<'a, 'tcx> {
+impl<'tcx> Visitor<'tcx> for SkipTyCollector {
     type Map = Map<'tcx>;
 
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
-    }
+    fn visit_ty(&mut self, hir_ty: &hir::Ty<'_>) {
+        self.types_to_skip.push(hir_ty.hir_id);
 
-    fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) {
-        if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind {
-            match path.res {
-                def::Res::SelfTy(..) => {},
-                _ => {
-                    match self.cx.tcx.hir().find(self.cx.tcx.hir().get_parent_node(hir_ty.hir_id)) {
-                        Some(Node::Expr(Expr {
-                            kind: ExprKind::Path(QPath::TypeRelative(_, segment)),
-                            ..
-                        })) => {
-                            // The following block correctly identifies applicable lint locations
-                            // but `hir_ty_to_ty` calls cause odd ICEs.
-                            //
-                            if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
-                                // fixme: this span manipulation should not be necessary
-                                // @flip1995 found an ast lowering issue in
-                                // https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#l142-l162
-                                span_lint_until_last_segment(self.cx, hir_ty.span, segment);
-                            }
-                        },
-                        _ => {
-                            if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
-                                span_lint(self.cx, hir_ty.span)
-                            }
-                        },
-                    }
-                },
-            }
-        }
+        walk_ty(self, hir_ty)
+    }
 
-        walk_ty(self, hir_ty);
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
     }
 }
 
-impl<'tcx> LateLintPass<'tcx> for UseSelf {
-    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
-        if in_external_macro(cx.sess(), impl_item.span) {
-            return;
-        }
+struct LintTyCollector<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    self_ty: Ty<'tcx>,
+    types_to_lint: Vec<HirId>,
+    types_to_skip: Vec<HirId>,
+}
 
-        let parent_id = cx.tcx.hir().get_parent_item(impl_item.hir_id);
-        let imp = cx.tcx.hir().expect_item(parent_id);
+impl<'a, 'tcx> Visitor<'tcx> for LintTyCollector<'a, 'tcx> {
+    type Map = Map<'tcx>;
 
+    fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'_>) {
         if_chain! {
-            if let ItemKind::Impl { self_ty: hir_self_ty, .. } = imp.kind;
-            if let TyKind::Path(QPath::Resolved(_, ref item_path)) = hir_self_ty.kind;
+            if let Some(ty) = self.cx.typeck_results().node_type_opt(hir_ty.hir_id);
+            if should_lint_ty(hir_ty, ty, self.self_ty);
             then {
-                let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
-                let should_check = parameters.as_ref().map_or(
-                    true,
-                    |params| !params.parenthesized
-                        &&!params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)))
-                );
-
-                // TODO: don't short-circuit upon lifetime parameters
-                if should_check {
-                    let self_ty = hir_ty_to_ty(cx.tcx, hir_self_ty);
-                    let body_visitor = &mut BodyVisitor { cx, self_ty };
-                    let fn_sig_visitor = &mut FnSigVisitor { cx, self_ty };
-
-                    let tcx = cx.tcx;
-                    let impl_def_id = tcx.hir().local_def_id(imp.hir_id);
-                    let impl_trait_ref = tcx.impl_trait_ref(impl_def_id);
-                    if_chain! {
-                        if let Some(impl_trait_ref) = impl_trait_ref;
-                        if let ImplItemKind::Fn(FnSig { decl: impl_decl, .. }, impl_body_id) = &impl_item.kind;
-                        then {
-                            body_visitor.check_trait_method_impl_decl(impl_item, impl_decl, impl_trait_ref);
-                            let body = tcx.hir().body(*impl_body_id);
-                            body_visitor.visit_body(body);
-                        } else {
-                            walk_impl_item(body_visitor, impl_item);
-                            walk_impl_item(fn_sig_visitor, impl_item);
-                        }
-                    }
-                }
+                self.types_to_lint.push(hir_ty.hir_id);
+            } else {
+                self.types_to_skip.push(hir_ty.hir_id);
             }
         }
+
+        walk_ty(self, hir_ty)
+    }
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
     }
-    extract_msrv_attr!(LateContext);
 }
 
-struct UseSelfVisitor<'a, 'tcx> {
-    item_path: &'a Path<'a>,
-    cx: &'a LateContext<'tcx>,
+fn span_lint(cx: &LateContext<'_>, span: Span) {
+    span_lint_and_sugg(
+        cx,
+        USE_SELF,
+        span,
+        "unnecessary structure name repetition",
+        "use the applicable keyword",
+        "Self".to_owned(),
+        Applicability::MachineApplicable,
+    );
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
-    type Map = Map<'tcx>;
+#[allow(clippy::cast_possible_truncation)]
+fn span_lint_until_last_segment(cx: &LateContext<'_>, span: Span, segment: &PathSegment<'_>) {
+    let sp = span.with_hi(segment.ident.span.lo());
+    // remove the trailing ::
+    let span_without_last_segment = match snippet_opt(cx, sp) {
+        Some(snippet) => match snippet.rfind("::") {
+            Some(bidx) => sp.with_hi(sp.lo() + BytePos(bidx as u32)),
+            None => sp,
+        },
+        None => sp,
+    };
+    span_lint(cx, span_without_last_segment);
+}
 
-    fn visit_path(&mut self, path: &'tcx Path<'_>, _id: HirId) {
-        if !path.segments.iter().any(|p| p.ident.span.is_dummy()) {
-            if path.segments.len() >= 2 {
-                let last_but_one = &path.segments[path.segments.len() - 2];
-                if last_but_one.ident.name != kw::SelfUpper {
-                    let enum_def_id = match path.res {
-                        Res::Def(DefKind::Variant, variant_def_id) => self.cx.tcx.parent(variant_def_id),
-                        Res::Def(DefKind::Ctor(def::CtorOf::Variant, _), ctor_def_id) => {
-                            let variant_def_id = self.cx.tcx.parent(ctor_def_id);
-                            variant_def_id.and_then(|def_id| self.cx.tcx.parent(def_id))
-                        },
-                        _ => None,
-                    };
-
-                    if self.item_path.res.opt_def_id() == enum_def_id {
-                        span_use_self_lint(self.cx, path, Some(last_but_one));
-                    }
-                }
-            }
+fn span_lint_on_path_until_last_segment(cx: &LateContext<'_>, path: &Path<'_>) {
+    if path.segments.len() > 1 {
+        span_lint_until_last_segment(cx, path.span, path.segments.last().unwrap());
+    }
+}
 
-            if path.segments.last().expect(SEGMENTS_MSG).ident.name != kw::SelfUpper {
-                if self.item_path.res == path.res {
-                    span_use_self_lint(self.cx, path, None);
-                } else if let Res::Def(DefKind::Ctor(def::CtorOf::Struct, _), ctor_def_id) = path.res {
-                    if self.item_path.res.opt_def_id() == self.cx.tcx.parent(ctor_def_id) {
-                        span_use_self_lint(self.cx, path, None);
-                    }
-                }
-            }
+fn span_lint_on_qpath_resolved(cx: &LateContext<'_>, qpath: &QPath<'_>, until_last_segment: bool) {
+    if let QPath::Resolved(_, path) = qpath {
+        if until_last_segment {
+            span_lint_on_path_until_last_segment(cx, path);
+        } else {
+            span_lint(cx, path.span);
         }
+    }
+}
 
-        walk_path(self, path);
+fn ty_from_hir_id<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Ty<'tcx> {
+    if let Some(Node::Ty(hir_ty)) = cx.tcx.hir().find(hir_id) {
+        hir_ty_to_ty(cx.tcx, hir_ty)
+    } else {
+        unreachable!("This function should only be called with `HirId`s that are for sure `Node::Ty`")
     }
+}
 
-    fn visit_item(&mut self, item: &'tcx Item<'_>) {
-        match item.kind {
-            ItemKind::Use(..)
-            | ItemKind::Static(..)
-            | ItemKind::Enum(..)
-            | ItemKind::Struct(..)
-            | ItemKind::Union(..)
-            | ItemKind::Impl { .. }
-            | ItemKind::Fn(..) => {
-                // Don't check statements that shadow `Self` or where `Self` can't be used
-            },
-            _ => walk_item(self, item),
+fn in_impl(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'_>) -> bool {
+    let map = cx.tcx.hir();
+    let parent = map.get_parent_node(hir_ty.hir_id);
+    if_chain! {
+        if let Some(Node::Item(item)) = map.find(parent);
+        if let ItemKind::Impl { .. } = item.kind;
+        then {
+            true
+        } else {
+            false
         }
     }
+}
 
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::All(self.cx.tcx.hir())
+fn should_lint_ty(hir_ty: &hir::Ty<'_>, ty: Ty<'_>, self_ty: Ty<'_>) -> bool {
+    if_chain! {
+        if ty == self_ty;
+        if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind;
+        then {
+            !matches!(path.res, def::Res::SelfTy(..))
+        } else {
+            false
+        }
     }
 }