From e597832a781a4fc0f5626d37d0c6c78ebfd70fce Mon Sep 17 00:00:00 2001 From: Elliot Bobrow Date: Tue, 20 Apr 2021 19:58:25 -0700 Subject: [PATCH] apply changes from review --- clippy_lints/src/unnecessary_self_imports.rs | 56 +++++++++----------- tests/ui/unnecessary_self_imports.fixed | 3 +- tests/ui/unnecessary_self_imports.rs | 3 +- tests/ui/unnecessary_self_imports.stderr | 16 +++--- 4 files changed, 40 insertions(+), 38 deletions(-) diff --git a/clippy_lints/src/unnecessary_self_imports.rs b/clippy_lints/src/unnecessary_self_imports.rs index bcb5e05c41d8..7765fee84bf3 100644 --- a/clippy_lints/src/unnecessary_self_imports.rs +++ b/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,7 +26,7 @@ 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]); @@ -33,37 +34,32 @@ 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"); - } + }, ); } } diff --git a/tests/ui/unnecessary_self_imports.fixed b/tests/ui/unnecessary_self_imports.fixed index a23d830d9a4d..1185eaa1d552 100644 --- a/tests/ui/unnecessary_self_imports.fixed +++ b/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() {} diff --git a/tests/ui/unnecessary_self_imports.rs b/tests/ui/unnecessary_self_imports.rs index 4a599029ee71..56bfbc09402a 100644 --- a/tests/ui/unnecessary_self_imports.rs +++ b/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() {} diff --git a/tests/ui/unnecessary_self_imports.stderr b/tests/ui/unnecessary_self_imports.stderr index 17c1050f4b68..83a5618c983d 100644 --- a/tests/ui/unnecessary_self_imports.stderr +++ b/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