apply changes from review

clippy_lints/src/unnecessary_self_imports.rs

@@ -1,18 +1,19 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::source::snippet;
 use if_chain::if_chain;
-use rustc_ast::{Item, ItemKind, Path, PathSegment, UseTree, UseTreeKind};
+use rustc_ast::{Item, ItemKind, PathSegment, UseTree, UseTreeKind};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::symbol::kw::SelfLower;
+use rustc_span::symbol::kw;
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for imports ending in `::{self};`.
+    /// **What it does:** Checks for imports ending in `::{self}`.
     ///
-    /// **Why is this bad?** In most cases, this can be written much more cleanly by omitting `self`.
+    /// **Why is this bad?** In most cases, this can be written much more cleanly by omitting `::{self}`.
     ///
-    /// **Known problems:** Sometimes removing `::{self}` will break code (https://github.com/rust-lang/rustfmt/issues/3568).
+    /// **Known problems:** Removing `::{self}` will cause any non-module items at the same path to also be imported.
+    /// This might cause a naming conflict (https://github.com/rust-lang/rustfmt/issues/3568). This lint makes no attempt
+    /// to detect this scenario and that is why it is a restriction lint.
     ///
     /// **Example:**
     ///
@@ -25,45 +26,40 @@ declare_clippy_lint! {
     /// ```
     pub UNNECESSARY_SELF_IMPORTS,
     restriction,
-    "imports ending in `self`, which can be omitted"
+    "imports ending in `::{self}`, which can be omitted"
 }
 
 declare_lint_pass!(UnnecessarySelfImports => [UNNECESSARY_SELF_IMPORTS]);
 
 impl EarlyLintPass for UnnecessarySelfImports {
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
         if_chain! {
-            if let ItemKind::Use(
-                UseTree { kind: UseTreeKind::Nested(nodes), prefix: Path { span, .. }, .. }
-            ) = &item.kind;
-            if nodes.len() == 1;
-            if let (UseTree { prefix, kind, .. }, _) = &nodes[0];
-            if let PathSegment { ident, .. } = prefix.segments[0];
-            if ident.name == SelfLower;
+            if let ItemKind::Use(use_tree) = &item.kind;
+            if let UseTree { kind: UseTreeKind::Nested(nodes), .. } = use_tree;
+            if let [(UseTree { prefix, kind, .. }, _)] = &**nodes;
+            if let [PathSegment { ident, .. }, ..] = &*prefix.segments;
+            if ident.name == kw::SelfLower;
+            if let Some(last_segment) = use_tree.prefix.segments.last();
 
             then {
-                let adjusted_span = item.span.with_hi(span.hi());
-                let snippet = if let UseTreeKind::Simple(Some(alias), ..) = kind {
-                    format!(
-                        "{} as {};",
-                        snippet(cx, adjusted_span, ".."),
-                        snippet(cx, alias.span, "..")
-                    )
-                } else {
-                    format!(
-                        "{};",
-                        snippet(cx, adjusted_span, "..")
-                    )
-                };
                 span_lint_and_then(
                     cx,
                     UNNECESSARY_SELF_IMPORTS,
                     item.span,
-                    "import ending with `self`",
+                    "import ending with `::{self}`",
                     |diag| {
-                        diag.span_suggestion(item.span, "consider omitting `self`", snippet, Applicability::MaybeIncorrect);
+                        diag.span_suggestion(
+                            last_segment.span().with_hi(item.span.hi()),
+                            "consider omitting `::{self}`",
+                            format!(
+                                "{}{};",
+                                last_segment.ident,
+                                if let UseTreeKind::Simple(Some(alias), ..) = kind { format!(" as {}", alias) } else { String::new() },
+                            ),
+                            Applicability::MaybeIncorrect,
+                        );
                         diag.note("this will slightly change semantics; any non-module items at the same path will also be imported");
-                    }
+                    },
                 );
             }
         }

tests/ui/unnecessary_self_imports.fixed

@@ -4,6 +4,7 @@
 
 use std::collections::hash_map::{self, *};
 use std::fs as alias;
-use std::io;
+use std::io::{self, Read};
+use std::rc;
 
 fn main() {}

tests/ui/unnecessary_self_imports.rs

@@ -4,6 +4,7 @@
 
 use std::collections::hash_map::{self, *};
 use std::fs::{self as alias};
-use std::io::{self};
+use std::io::{self, Read};
+use std::rc::{self};
 
 fn main() {}

tests/ui/unnecessary_self_imports.stderr

@@ -1,17 +1,21 @@
-error: import ending with `self`
+error: import ending with `::{self}`
   --> $DIR/unnecessary_self_imports.rs:6:1
    |
 LL | use std::fs::{self as alias};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider omitting `self`: `use std::fs as alias;`
+   | ^^^^^^^^^--------------------
+   |          |
+   |          help: consider omitting `::{self}`: `fs as alias;`
    |
    = note: `-D clippy::unnecessary-self-imports` implied by `-D warnings`
    = note: this will slightly change semantics; any non-module items at the same path will also be imported
 
-error: import ending with `self`
-  --> $DIR/unnecessary_self_imports.rs:7:1
+error: import ending with `::{self}`
+  --> $DIR/unnecessary_self_imports.rs:8:1
    |
-LL | use std::io::{self};
-   | ^^^^^^^^^^^^^^^^^^^^ help: consider omitting `self`: `use std::io;`
+LL | use std::rc::{self};
+   | ^^^^^^^^^-----------
+   |          |
+   |          help: consider omitting `::{self}`: `rc;`
    |
    = note: this will slightly change semantics; any non-module items at the same path will also be imported