about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-02-12 06:09:07 +0000
committerbors <bors@rust-lang.org>2021-02-12 06:09:07 +0000
commit605e9ba3d7fa9e42afcdee5fd94c95f771693d07 (patch)
treec6723dbec3614b9b7784fa0e2399b3d496a4b7a1
parentfa2c1d63a296442d503f16901bd15e1d2222086e (diff)
parent52f98d832dc5ca982b9edad7807b2a9a9ddd13e0 (diff)
Auto merge of #6179 - flip1995:rewrite_use_self, r=phansch
Rework use_self impl based on ty::Ty comparison #3410 | Take 2

This builds on top of #5531

I already reviewed and approved the commits by `@montrivo.` So only the review of my commits should be necessary.

I would also appreciate your review `@montrivo,` since you are familiar with the challenges here.

Fixes #3410 and Fixes #4143 (same problem)
Fixes #2843
Fixes #3859
Fixes #4734 and fixes #6221
Fixes #4305
Fixes #5078 (even at expression level now 🎉)
Fixes #3881 and Fixes #4887 (same problem)
Fixes #3909

Not yet: #4140 (test added)

All the credit for the fixes goes to `@montrivo.` I only refactored and copy and pasted his code.

changelog: rewrite [`use_self`] lint and fix multiple (8) FPs. One to go.
-rw-r--r--clippy_lints/src/use_self.rs563
-rw-r--r--tests/ui/auxiliary/proc_macro_derive.rs12
-rw-r--r--tests/ui/use_self.fixed224
-rw-r--r--tests/ui/use_self.rs222
-rw-r--r--tests/ui/use_self.stderr118
-rw-r--r--tests/ui/use_self_trait.fixed3
-rw-r--r--tests/ui/use_self_trait.rs1
-rw-r--r--tests/ui/use_self_trait.stderr8
8 files changed, 880 insertions, 271 deletions
diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index 72d1ca73929..a0dd53af661 100644
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -1,24 +1,24 @@
+use crate::utils::{in_macro, meets_msrv, 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, Res};
-use rustc_hir::intravisit::{walk_item, walk_path, walk_ty, NestedVisitorMap, Visitor};
+use rustc_hir::def::DefKind;
 use rustc_hir::{
-    def, FnDecl, FnRetTy, FnSig, GenericArg, HirId, ImplItem, ImplItemKind, Item, ItemKind, Path, PathSegment, QPath,
-    TyKind,
+    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::{DefIdTree, Ty};
+use rustc_middle::ty::{AssocKind, Ty, TyS};
 use rustc_semver::RustcVersion;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::symbol::kw;
+use rustc_span::{BytePos, Span};
 use rustc_typeck::hir_ty_to_ty;
 
-use crate::utils::{differing_macro_contexts, meets_msrv, span_lint_and_sugg};
-
 declare_clippy_lint! {
     /// **What it does:** Checks for unnecessary repetition of structure name when a
     /// replacement with `Self` is applicable.
@@ -28,10 +28,11 @@ declare_clippy_lint! {
     /// feels inconsistent.
     ///
     /// **Known problems:**
-    /// - False positive when using associated types ([#2843](https://github.com/rust-lang/rust-clippy/issues/2843))
-    /// - False positives in some situations when using generics ([#3410](https://github.com/rust-lang/rust-clippy/issues/3410))
+    /// - Unaddressed false negative in fn bodies of trait implementations
+    /// - False positive with assotiated types in traits (#4140)
     ///
     /// **Example:**
+    ///
     /// ```rust
     /// struct Foo {}
     /// impl Foo {
@@ -54,235 +55,409 @@ declare_clippy_lint! {
     "unnecessary structure name repetition whereas `Self` is applicable"
 }
 
-impl_lint_pass!(UseSelf => [USE_SELF]);
-
-const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
-
-fn span_use_self_lint(cx: &LateContext<'_>, path: &Path<'_>, last_segment: Option<&PathSegment<'_>>) {
-    let last_segment = last_segment.unwrap_or_else(|| path.segments.last().expect(SEGMENTS_MSG));
+#[derive(Default)]
+pub struct UseSelf {
+    msrv: Option<RustcVersion>,
+    stack: Vec<StackItem>,
+}
 
-    // Path segments only include actual path, no methods or fields.
-    let last_path_span = last_segment.ident.span;
+const USE_SELF_MSRV: RustcVersion = RustcVersion::new(1, 37, 0);
 
-    if differing_macro_contexts(path.span, last_path_span) {
-        return;
+impl UseSelf {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self {
+            msrv,
+            ..Self::default()
+        }
     }
-
-    // Only take path up to the end of last_path_span.
-    let span = path.span.with_hi(last_path_span.hi());
-
-    span_lint_and_sugg(
-        cx,
-        USE_SELF,
-        span,
-        "unnecessary structure name repetition",
-        "use the applicable keyword",
-        "Self".to_owned(),
-        Applicability::MachineApplicable,
-    );
 }
 
-// FIXME: always use this (more correct) visitor, not just in method signatures.
-struct SemanticUseSelfVisitor<'a, 'tcx> {
-    cx: &'a LateContext<'tcx>,
-    self_ty: Ty<'tcx>,
+#[derive(Debug)]
+enum StackItem {
+    Check {
+        hir_id: HirId,
+        impl_trait_ref_def_id: Option<LocalDefId>,
+        types_to_skip: Vec<HirId>,
+        types_to_lint: Vec<HirId>,
+    },
+    NoCheck,
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for SemanticUseSelfVisitor<'a, 'tcx> {
-    type Map = Map<'tcx>;
+impl_lint_pass!(UseSelf => [USE_SELF]);
 
-    fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'_>) {
-        if let TyKind::Path(QPath::Resolved(_, path)) = &hir_ty.kind {
-            match path.res {
-                def::Res::SelfTy(..) => {},
-                _ => {
-                    if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
-                        span_use_self_lint(self.cx, path, None);
-                    }
-                },
-            }
-        }
+const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
 
-        walk_ty(self, hir_ty)
+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,
+                        types_to_lint: Vec::new(),
+                        types_to_skip: Vec::new(),
+                    });
+                } 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 nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
+    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();
+            },
+            _ => (),
+        }
     }
-}
 
-fn check_trait_method_impl_decl<'tcx>(
-    cx: &LateContext<'tcx>,
-    impl_item: &ImplItem<'_>,
-    impl_decl: &'tcx FnDecl<'_>,
-    impl_trait_ref: ty::TraitRef<'tcx>,
-) {
-    let trait_method = cx
-        .tcx
-        .associated_items(impl_trait_ref.def_id)
-        .find_by_name_and_kind(cx.tcx, impl_item.ident, ty::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);
-
-    let output_hir_ty = if let FnRetTy::Return(ty) = &impl_decl.output {
-        Some(&**ty)
-    } else {
-        None
-    };
-
-    // `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()) {
-            let mut visitor = SemanticUseSelfVisitor { cx, self_ty };
-
-            visitor.visit_ty(&impl_hir_ty);
+    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(&mut StackItem::Check {
+                impl_trait_ref_def_id: Some(def_id),
+                ref mut types_to_skip,
+                ..
+            }) = self.stack.last_mut();
+            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);
+                        types_to_skip.extend(visitor.types_to_skip);
+                    }
+                }
+            }
         }
     }
-}
 
-const USE_SELF_MSRV: RustcVersion = RustcVersion::new(1, 37, 0);
+    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,
+            types_to_lint,
+            types_to_skip,
+            ..
+        }) = self.stack.last_mut()
+        {
+            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);
+            types_to_lint.extend(visitor.types_to_lint);
+            types_to_skip.extend(visitor.types_to_skip);
+        }
+    }
 
-pub struct UseSelf {
-    msrv: Option<RustcVersion>,
-}
+    fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) {
+        if in_macro(hir_ty.span) | in_impl(cx, hir_ty) | !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV) {
+            return;
+        }
 
-impl UseSelf {
-    #[must_use]
-    pub fn new(msrv: Option<RustcVersion>) -> Self {
-        Self { msrv }
+        let lint_dependend_on_expr_kind = if let Some(StackItem::Check {
+            hir_id,
+            types_to_lint,
+            types_to_skip,
+            ..
+        }) = self.stack.last()
+        {
+            if types_to_skip.contains(&hir_ty.hir_id) {
+                false
+            } else if types_to_lint.contains(&hir_ty.hir_id) {
+                true
+            } else {
+                let self_ty = ty_from_hir_id(cx, *hir_id);
+                should_lint_ty(hir_ty, hir_ty_to_ty(cx.tcx, hir_ty), self_ty)
+            }
+        } else {
+            false
+        };
+
+        if 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),
+            }
+        }
     }
-}
 
-impl<'tcx> LateLintPass<'tcx> for UseSelf {
-    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
-        if !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV) {
-            return;
+    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)
+            } else {
+                false
+            }
         }
 
-        if in_external_macro(cx.sess(), item.span) {
+        if in_macro(expr.span) | !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV) {
             return;
         }
-        if_chain! {
-            if let ItemKind::Impl(impl_) = &item.kind;
-            if let TyKind::Path(QPath::Resolved(_, ref item_path)) = impl_.self_ty.kind;
-            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(_)))
-                );
 
-                if should_check {
-                    let visitor = &mut UseSelfVisitor {
-                        item_path,
-                        cx,
-                    };
-                    let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id);
-                    let impl_trait_ref = cx.tcx.impl_trait_ref(impl_def_id);
-
-                    if let Some(impl_trait_ref) = impl_trait_ref {
-                        for impl_item_ref in impl_.items {
-                            let impl_item = cx.tcx.hir().impl_item(impl_item_ref.id);
-                            if let ImplItemKind::Fn(FnSig{ decl: impl_decl, .. }, impl_body_id)
-                                    = &impl_item.kind {
-                                check_trait_method_impl_decl(cx, impl_item, impl_decl, impl_trait_ref);
-
-                                let body = cx.tcx.hir().body(*impl_body_id);
-                                visitor.visit_body(body);
-                            } else {
-                                visitor.visit_impl_item(impl_item);
-                            }
+        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);
+                            },
                         }
-                    } else {
-                        for impl_item_ref in impl_.items {
-                            let impl_item = cx.tcx.hir().impl_item(impl_item_ref.id);
-                            visitor.visit_impl_item(impl_item);
+                    }
+                },
+                // 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 = cx.qpath_res(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(qpath) => {
+                    if expr_ty_matches(cx, expr, self_ty) {
+                        span_lint_on_qpath_resolved(cx, &qpath, true);
+                    }
+                },
+                _ => (),
             }
         }
     }
+
     extract_msrv_attr!(LateContext);
 }
 
-struct UseSelfVisitor<'a, 'tcx> {
-    item_path: &'a Path<'a>,
+#[derive(Default)]
+struct SkipTyCollector {
+    types_to_skip: Vec<HirId>,
+}
+
+impl<'tcx> Visitor<'tcx> for SkipTyCollector {
+    type Map = Map<'tcx>;
+
+    fn visit_ty(&mut self, hir_ty: &hir::Ty<'_>) {
+        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
+    }
+}
+
+struct LintTyCollector<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
+    self_ty: Ty<'tcx>,
+    types_to_lint: Vec<HirId>,
+    types_to_skip: Vec<HirId>,
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
+impl<'a, 'tcx> Visitor<'tcx> for LintTyCollector<'a, 'tcx> {
     type Map = Map<'tcx>;
 
-    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 visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'_>) {
+        if_chain! {
+            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 {
+                self.types_to_lint.push(hir_ty.hir_id);
+            } else {
+                self.types_to_skip.push(hir_ty.hir_id);
             }
+        }
 
-            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);
-                    }
-                }
-            }
+        walk_ty(self, hir_ty)
+    }
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+}
+
+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,
+    );
+}
+
+#[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 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());
+    }
+}
+
+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 TyS::same_type(ty, self_ty);
+        if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind;
+        then {
+            !matches!(path.res, def::Res::SelfTy(..))
+        } else {
+            false
+        }
     }
 }
diff --git a/tests/ui/auxiliary/proc_macro_derive.rs b/tests/ui/auxiliary/proc_macro_derive.rs
index 24891682d36..aebeaf34679 100644
--- a/tests/ui/auxiliary/proc_macro_derive.rs
+++ b/tests/ui/auxiliary/proc_macro_derive.rs
@@ -41,3 +41,15 @@ pub fn derive_foo(_input: TokenStream) -> TokenStream {
         }
     }
 }
+
+#[proc_macro_derive(StructAUseSelf)]
+pub fn derive_use_self(_input: TokenStream) -> proc_macro::TokenStream {
+    quote! {
+        struct A;
+        impl A {
+            fn new() -> A {
+                A
+            }
+        }
+    }
+}
diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed
index bb2012441d9..95e7bc75431 100644
--- a/tests/ui/use_self.fixed
+++ b/tests/ui/use_self.fixed
@@ -1,9 +1,13 @@
 // run-rustfix
 // edition:2018
+// aux-build:proc_macro_derive.rs
 
 #![warn(clippy::use_self)]
 #![allow(dead_code)]
-#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms)]
+#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms, clippy::from_over_into)]
+
+#[macro_use]
+extern crate proc_macro_derive;
 
 fn main() {}
 
@@ -71,13 +75,12 @@ mod lifetimes {
 
 mod issue2894 {
     trait IntoBytes {
-        #[allow(clippy::wrong_self_convention)]
-        fn into_bytes(&self) -> Vec<u8>;
+        fn to_bytes(&self) -> Vec<u8>;
     }
 
     // This should not be linted
     impl IntoBytes for u8 {
-        fn into_bytes(&self) -> Vec<u8> {
+        fn to_bytes(&self) -> Vec<u8> {
             vec![*self]
         }
     }
@@ -110,8 +113,8 @@ mod tuple_structs {
 mod macros {
     macro_rules! use_self_expand {
         () => {
-            fn new() -> Self {
-                Self {}
+            fn new() -> Foo {
+                Foo {}
             }
         };
     }
@@ -119,8 +122,11 @@ mod macros {
     struct Foo {}
 
     impl Foo {
-        use_self_expand!(); // Should lint in local macros
+        use_self_expand!(); // Should not lint in local macros
     }
+
+    #[derive(StructAUseSelf)] // Should not lint in derives
+    struct A;
 }
 
 mod nesting {
@@ -177,11 +183,22 @@ mod issue3410 {
     struct B;
 
     trait Trait<T> {
-        fn a(v: T);
+        fn a(v: T) -> Self;
     }
 
     impl Trait<Vec<A>> for Vec<B> {
-        fn a(_: Vec<A>) {}
+        fn a(_: Vec<A>) -> Self {
+            unimplemented!()
+        }
+    }
+
+    impl<T> Trait<Vec<A>> for Vec<T>
+    where
+        T: Trait<B>,
+    {
+        fn a(v: Vec<A>) -> Self {
+            <Vec<B>>::a(v).into_iter().map(Trait::a).collect()
+        }
     }
 }
 
@@ -252,3 +269,192 @@ mod paths_created_by_lowering {
         }
     }
 }
+
+// reused from #1997
+mod generics {
+    struct Foo<T> {
+        value: T,
+    }
+
+    impl<T> Foo<T> {
+        // `Self` is applicable here
+        fn foo(value: T) -> Self {
+            Self { value }
+        }
+
+        // `Cannot` use `Self` as a return type as the generic types are different
+        fn bar(value: i32) -> Foo<i32> {
+            Foo { value }
+        }
+    }
+}
+
+mod issue4140 {
+    pub struct Error<From, To> {
+        _from: From,
+        _too: To,
+    }
+
+    pub trait From<T> {
+        type From;
+        type To;
+
+        fn from(value: T) -> Self;
+    }
+
+    pub trait TryFrom<T>
+    where
+        Self: Sized,
+    {
+        type From;
+        type To;
+
+        fn try_from(value: T) -> Result<Self, Error<Self::From, Self::To>>;
+    }
+
+    impl<F, T> TryFrom<F> for T
+    where
+        T: From<F>,
+    {
+        type From = Self;
+        type To = Self;
+
+        fn try_from(value: F) -> Result<Self, Error<Self::From, Self::To>> {
+            Ok(From::from(value))
+        }
+    }
+
+    impl From<bool> for i64 {
+        type From = bool;
+        type To = Self;
+
+        fn from(value: bool) -> Self {
+            if value {
+                100
+            } else {
+                0
+            }
+        }
+    }
+}
+
+mod issue2843 {
+    trait Foo {
+        type Bar;
+    }
+
+    impl Foo for usize {
+        type Bar = u8;
+    }
+
+    impl<T: Foo> Foo for Option<T> {
+        type Bar = Option<T::Bar>;
+    }
+}
+
+mod issue3859 {
+    pub struct Foo;
+    pub struct Bar([usize; 3]);
+
+    impl Foo {
+        pub const BAR: usize = 3;
+
+        pub fn foo() {
+            const _X: usize = Foo::BAR;
+            // const _Y: usize = Self::BAR;
+        }
+    }
+}
+
+mod issue4305 {
+    trait Foo: 'static {}
+
+    struct Bar;
+
+    impl Foo for Bar {}
+
+    impl<T: Foo> From<T> for Box<dyn Foo> {
+        fn from(t: T) -> Self {
+            Box::new(t)
+        }
+    }
+}
+
+mod lint_at_item_level {
+    struct Foo {}
+
+    #[allow(clippy::use_self)]
+    impl Foo {
+        fn new() -> Foo {
+            Foo {}
+        }
+    }
+
+    #[allow(clippy::use_self)]
+    impl Default for Foo {
+        fn default() -> Foo {
+            Foo::new()
+        }
+    }
+}
+
+mod lint_at_impl_item_level {
+    struct Foo {}
+
+    impl Foo {
+        #[allow(clippy::use_self)]
+        fn new() -> Foo {
+            Foo {}
+        }
+    }
+
+    impl Default for Foo {
+        #[allow(clippy::use_self)]
+        fn default() -> Foo {
+            Foo::new()
+        }
+    }
+}
+
+mod issue4734 {
+    #[repr(C, packed)]
+    pub struct X {
+        pub x: u32,
+    }
+
+    impl From<X> for u32 {
+        fn from(c: X) -> Self {
+            unsafe { core::mem::transmute(c) }
+        }
+    }
+}
+
+mod nested_paths {
+    use std::convert::Into;
+    mod submod {
+        pub struct B {}
+        pub struct C {}
+
+        impl Into<C> for B {
+            fn into(self) -> C {
+                C {}
+            }
+        }
+    }
+
+    struct A<T> {
+        t: T,
+    }
+
+    impl<T> A<T> {
+        fn new<V: Into<T>>(v: V) -> Self {
+            Self { t: Into::into(v) }
+        }
+    }
+
+    impl A<submod::C> {
+        fn test() -> Self {
+            Self::new::<submod::B>(submod::B {})
+        }
+    }
+}
diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs
index ddfd2beba31..75424f34159 100644
--- a/tests/ui/use_self.rs
+++ b/tests/ui/use_self.rs
@@ -1,9 +1,13 @@
 // run-rustfix
 // edition:2018
+// aux-build:proc_macro_derive.rs
 
 #![warn(clippy::use_self)]
 #![allow(dead_code)]
-#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms)]
+#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms, clippy::from_over_into)]
+
+#[macro_use]
+extern crate proc_macro_derive;
 
 fn main() {}
 
@@ -71,13 +75,12 @@ mod lifetimes {
 
 mod issue2894 {
     trait IntoBytes {
-        #[allow(clippy::wrong_self_convention)]
-        fn into_bytes(&self) -> Vec<u8>;
+        fn to_bytes(&self) -> Vec<u8>;
     }
 
     // This should not be linted
     impl IntoBytes for u8 {
-        fn into_bytes(&self) -> Vec<u8> {
+        fn to_bytes(&self) -> Vec<u8> {
             vec![*self]
         }
     }
@@ -87,7 +90,7 @@ mod existential {
     struct Foo;
 
     impl Foo {
-        fn bad(foos: &[Self]) -> impl Iterator<Item = &Foo> {
+        fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
             foos.iter()
         }
 
@@ -119,8 +122,11 @@ mod macros {
     struct Foo {}
 
     impl Foo {
-        use_self_expand!(); // Should lint in local macros
+        use_self_expand!(); // Should not lint in local macros
     }
+
+    #[derive(StructAUseSelf)] // Should not lint in derives
+    struct A;
 }
 
 mod nesting {
@@ -177,11 +183,22 @@ mod issue3410 {
     struct B;
 
     trait Trait<T> {
-        fn a(v: T);
+        fn a(v: T) -> Self;
     }
 
     impl Trait<Vec<A>> for Vec<B> {
-        fn a(_: Vec<A>) {}
+        fn a(_: Vec<A>) -> Self {
+            unimplemented!()
+        }
+    }
+
+    impl<T> Trait<Vec<A>> for Vec<T>
+    where
+        T: Trait<B>,
+    {
+        fn a(v: Vec<A>) -> Self {
+            <Vec<B>>::a(v).into_iter().map(Trait::a).collect()
+        }
     }
 }
 
@@ -252,3 +269,192 @@ mod paths_created_by_lowering {
         }
     }
 }
+
+// reused from #1997
+mod generics {
+    struct Foo<T> {
+        value: T,
+    }
+
+    impl<T> Foo<T> {
+        // `Self` is applicable here
+        fn foo(value: T) -> Foo<T> {
+            Foo { value }
+        }
+
+        // `Cannot` use `Self` as a return type as the generic types are different
+        fn bar(value: i32) -> Foo<i32> {
+            Foo { value }
+        }
+    }
+}
+
+mod issue4140 {
+    pub struct Error<From, To> {
+        _from: From,
+        _too: To,
+    }
+
+    pub trait From<T> {
+        type From;
+        type To;
+
+        fn from(value: T) -> Self;
+    }
+
+    pub trait TryFrom<T>
+    where
+        Self: Sized,
+    {
+        type From;
+        type To;
+
+        fn try_from(value: T) -> Result<Self, Error<Self::From, Self::To>>;
+    }
+
+    impl<F, T> TryFrom<F> for T
+    where
+        T: From<F>,
+    {
+        type From = T::From;
+        type To = T::To;
+
+        fn try_from(value: F) -> Result<Self, Error<Self::From, Self::To>> {
+            Ok(From::from(value))
+        }
+    }
+
+    impl From<bool> for i64 {
+        type From = bool;
+        type To = Self;
+
+        fn from(value: bool) -> Self {
+            if value {
+                100
+            } else {
+                0
+            }
+        }
+    }
+}
+
+mod issue2843 {
+    trait Foo {
+        type Bar;
+    }
+
+    impl Foo for usize {
+        type Bar = u8;
+    }
+
+    impl<T: Foo> Foo for Option<T> {
+        type Bar = Option<T::Bar>;
+    }
+}
+
+mod issue3859 {
+    pub struct Foo;
+    pub struct Bar([usize; 3]);
+
+    impl Foo {
+        pub const BAR: usize = 3;
+
+        pub fn foo() {
+            const _X: usize = Foo::BAR;
+            // const _Y: usize = Self::BAR;
+        }
+    }
+}
+
+mod issue4305 {
+    trait Foo: 'static {}
+
+    struct Bar;
+
+    impl Foo for Bar {}
+
+    impl<T: Foo> From<T> for Box<dyn Foo> {
+        fn from(t: T) -> Self {
+            Box::new(t)
+        }
+    }
+}
+
+mod lint_at_item_level {
+    struct Foo {}
+
+    #[allow(clippy::use_self)]
+    impl Foo {
+        fn new() -> Foo {
+            Foo {}
+        }
+    }
+
+    #[allow(clippy::use_self)]
+    impl Default for Foo {
+        fn default() -> Foo {
+            Foo::new()
+        }
+    }
+}
+
+mod lint_at_impl_item_level {
+    struct Foo {}
+
+    impl Foo {
+        #[allow(clippy::use_self)]
+        fn new() -> Foo {
+            Foo {}
+        }
+    }
+
+    impl Default for Foo {
+        #[allow(clippy::use_self)]
+        fn default() -> Foo {
+            Foo::new()
+        }
+    }
+}
+
+mod issue4734 {
+    #[repr(C, packed)]
+    pub struct X {
+        pub x: u32,
+    }
+
+    impl From<X> for u32 {
+        fn from(c: X) -> Self {
+            unsafe { core::mem::transmute(c) }
+        }
+    }
+}
+
+mod nested_paths {
+    use std::convert::Into;
+    mod submod {
+        pub struct B {}
+        pub struct C {}
+
+        impl Into<C> for B {
+            fn into(self) -> C {
+                C {}
+            }
+        }
+    }
+
+    struct A<T> {
+        t: T,
+    }
+
+    impl<T> A<T> {
+        fn new<V: Into<T>>(v: V) -> Self {
+            Self { t: Into::into(v) }
+        }
+    }
+
+    impl A<submod::C> {
+        fn test() -> Self {
+            A::new::<submod::B>(submod::B {})
+        }
+    }
+}
diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr
index 80e1bfc75e8..37dfef7cfe0 100644
--- a/tests/ui/use_self.stderr
+++ b/tests/ui/use_self.stderr
@@ -1,5 +1,5 @@
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:14:21
+  --> $DIR/use_self.rs:18:21
    |
 LL |         fn new() -> Foo {
    |                     ^^^ help: use the applicable keyword: `Self`
@@ -7,158 +7,172 @@ LL |         fn new() -> Foo {
    = note: `-D clippy::use-self` implied by `-D warnings`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:15:13
+  --> $DIR/use_self.rs:19:13
    |
 LL |             Foo {}
    |             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:17:22
+  --> $DIR/use_self.rs:21:22
    |
 LL |         fn test() -> Foo {
    |                      ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:18:13
+  --> $DIR/use_self.rs:22:13
    |
 LL |             Foo::new()
    |             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:23:25
+  --> $DIR/use_self.rs:27:25
    |
 LL |         fn default() -> Foo {
    |                         ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:24:13
+  --> $DIR/use_self.rs:28:13
    |
 LL |             Foo::new()
    |             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:90:56
+  --> $DIR/use_self.rs:93:24
    |
-LL |         fn bad(foos: &[Self]) -> impl Iterator<Item = &Foo> {
-   |                                                        ^^^ help: use the applicable keyword: `Self`
+LL |         fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
+   |                        ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:105:13
+  --> $DIR/use_self.rs:93:55
+   |
+LL |         fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
+   |                                                       ^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:108:13
    |
 LL |             TS(0)
    |             ^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:113:25
-   |
-LL |             fn new() -> Foo {
-   |                         ^^^ help: use the applicable keyword: `Self`
-...
-LL |         use_self_expand!(); // Should lint in local macros
-   |         ------------------- in this macro invocation
+  --> $DIR/use_self.rs:143:29
    |
-   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+LL |                 fn bar() -> Bar {
+   |                             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:114:17
-   |
-LL |                 Foo {}
-   |                 ^^^ help: use the applicable keyword: `Self`
-...
-LL |         use_self_expand!(); // Should lint in local macros
-   |         ------------------- in this macro invocation
+  --> $DIR/use_self.rs:144:21
    |
-   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+LL |                     Bar { foo: Foo {} }
+   |                     ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:149:21
+  --> $DIR/use_self.rs:155:21
    |
 LL |         fn baz() -> Foo {
    |                     ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:150:13
+  --> $DIR/use_self.rs:156:13
    |
 LL |             Foo {}
    |             ^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:137:29
-   |
-LL |                 fn bar() -> Bar {
-   |                             ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:138:21
-   |
-LL |                     Bar { foo: Foo {} }
-   |                     ^^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:167:21
+  --> $DIR/use_self.rs:173:21
    |
 LL |             let _ = Enum::B(42);
    |                     ^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:168:21
+  --> $DIR/use_self.rs:174:21
    |
 LL |             let _ = Enum::C { field: true };
    |                     ^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:169:21
+  --> $DIR/use_self.rs:175:21
    |
 LL |             let _ = Enum::A;
    |                     ^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:200:13
+  --> $DIR/use_self.rs:217:13
    |
 LL |             nested::A::fun_1();
    |             ^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:201:13
+  --> $DIR/use_self.rs:218:13
    |
 LL |             nested::A::A;
    |             ^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:203:13
+  --> $DIR/use_self.rs:220:13
    |
 LL |             nested::A {};
    |             ^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:222:13
+  --> $DIR/use_self.rs:239:13
    |
 LL |             TestStruct::from_something()
    |             ^^^^^^^^^^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:236:25
+  --> $DIR/use_self.rs:253:25
    |
 LL |         async fn g() -> S {
    |                         ^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:237:13
+  --> $DIR/use_self.rs:254:13
    |
 LL |             S {}
    |             ^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:241:16
+  --> $DIR/use_self.rs:258:16
    |
 LL |             &p[S::A..S::B]
    |                ^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> $DIR/use_self.rs:241:22
+  --> $DIR/use_self.rs:258:22
    |
 LL |             &p[S::A..S::B]
    |                      ^ help: use the applicable keyword: `Self`
 
-error: aborting due to 25 previous errors
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:281:29
+   |
+LL |         fn foo(value: T) -> Foo<T> {
+   |                             ^^^^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:282:13
+   |
+LL |             Foo { value }
+   |             ^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:319:21
+   |
+LL |         type From = T::From;
+   |                     ^^^^^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:320:19
+   |
+LL |         type To = T::To;
+   |                   ^^^^^ help: use the applicable keyword: `Self`
+
+error: unnecessary structure name repetition
+  --> $DIR/use_self.rs:457:13
+   |
+LL |             A::new::<submod::B>(submod::B {})
+   |             ^ help: use the applicable keyword: `Self`
+
+error: aborting due to 29 previous errors
 
diff --git a/tests/ui/use_self_trait.fixed b/tests/ui/use_self_trait.fixed
index 1582ae114bf..9bcd692fb35 100644
--- a/tests/ui/use_self_trait.fixed
+++ b/tests/ui/use_self_trait.fixed
@@ -47,7 +47,8 @@ impl Mul for Bad {
 
 impl Clone for Bad {
     fn clone(&self) -> Self {
-        Self
+        // FIXME: applicable here
+        Bad
     }
 }
 
diff --git a/tests/ui/use_self_trait.rs b/tests/ui/use_self_trait.rs
index 70667b9797e..de305d40f33 100644
--- a/tests/ui/use_self_trait.rs
+++ b/tests/ui/use_self_trait.rs
@@ -47,6 +47,7 @@ impl Mul for Bad {
 
 impl Clone for Bad {
     fn clone(&self) -> Self {
+        // FIXME: applicable here
         Bad
     }
 }
diff --git a/tests/ui/use_self_trait.stderr b/tests/ui/use_self_trait.stderr
index 4f2506cc119..55af3ff2a93 100644
--- a/tests/ui/use_self_trait.stderr
+++ b/tests/ui/use_self_trait.stderr
@@ -84,11 +84,5 @@ error: unnecessary structure name repetition
 LL |     fn mul(self, rhs: Bad) -> Bad {
    |                               ^^^ help: use the applicable keyword: `Self`
 
-error: unnecessary structure name repetition
-  --> $DIR/use_self_trait.rs:50:9
-   |
-LL |         Bad
-   |         ^^^ help: use the applicable keyword: `Self`
-
-error: aborting due to 15 previous errors
+error: aborting due to 14 previous errors