add unnecessary_self_imports lint

ebobrow · ebobrow · commit 76db21e05d08 · 2021-04-12T18:49:52.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2519,6 +2519,7 @@ Released 2018-09-13
 [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
 [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
+[`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
 [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
 [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
 [`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -356,6 +356,7 @@ mod unicode;
 mod unit_return_expecting_ord;
 mod unit_types;
 mod unnamed_address;
+mod unnecessary_self_imports;
 mod unnecessary_sort_by;
 mod unnecessary_wraps;
 mod unnested_or_patterns;
@@ -963,6 +964,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         unit_types::UNIT_CMP,
         unnamed_address::FN_ADDRESS_COMPARISONS,
         unnamed_address::VTABLE_ADDRESS_COMPARISONS,
+        unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS,
         unnecessary_sort_by::UNNECESSARY_SORT_BY,
         unnecessary_wraps::UNNECESSARY_WRAPS,
         unnested_or_patterns::UNNESTED_OR_PATTERNS,
@@ -1048,6 +1050,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box default_numeric_fallback::DefaultNumericFallback);
     store.register_late_pass(|| box inconsistent_struct_constructor::InconsistentStructConstructor);
     store.register_late_pass(|| box non_octal_unix_permissions::NonOctalUnixPermissions);
+    store.register_late_pass(|| box unnecessary_self_imports::UnnecessarySelfImports);
 
     let msrv = conf.msrv.as_ref().and_then(|s| {
         parse_msrv(s, None, None).or_else(|| {
@@ -1708,6 +1711,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(unit_types::UNIT_CMP),
         LintId::of(unnamed_address::FN_ADDRESS_COMPARISONS),
         LintId::of(unnamed_address::VTABLE_ADDRESS_COMPARISONS),
+        LintId::of(unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS),
         LintId::of(unnecessary_sort_by::UNNECESSARY_SORT_BY),
         LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
         LintId::of(unused_io_amount::UNUSED_IO_AMOUNT),
@@ -1832,6 +1836,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
         LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME),
         LintId::of(try_err::TRY_ERR),
+        LintId::of(unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS),
         LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
         LintId::of(unused_unit::UNUSED_UNIT),
         LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS),
diff --git a/clippy_lints/src/modulo_arithmetic.rs b/clippy_lints/src/modulo_arithmetic.rs
@@ -4,7 +4,7 @@ use clippy_utils::sext;
 use if_chain::if_chain;
 use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{self};
+use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use std::fmt::Display;
 
diff --git a/clippy_lints/src/unnecessary_self_imports.rs b/clippy_lints/src/unnecessary_self_imports.rs
@@ -0,0 +1,177 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::in_macro;
+use clippy_utils::source::snippet_with_applicability;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Item, ItemKind, Mod, Node, Path, UseKind, VisibilityKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::def_id::DefId;
+use rustc_span::symbol::Ident;
+use rustc_span::{BytePos, Span};
+
+declare_clippy_lint! {
+    /// **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`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// use std::io::{self};
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::io;
+    /// ```
+    pub UNNECESSARY_SELF_IMPORTS,
+    style,
+    "imports ending in `self`, which can be omitted"
+}
+
+declare_lint_pass!(UnnecessarySelfImports => [UNNECESSARY_SELF_IMPORTS]);
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessarySelfImports {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
+        if_chain! {
+            if !in_macro(item.span);
+            if let ItemKind::Use(path, UseKind::ListStem) = &item.kind;
+            if let Some(ident) = is_self_import(cx, item, path);
+            if let Some(Res::Def(DefKind::Mod, def_id)) = path.segments[path.segments.len() - 2].res;
+            if let Some(last) = path.segments.last();
+            if !mod_contains_item(cx, def_id, last.ident) || !item_is_in_scope(cx, ident, path.span);
+            then {
+                let mut applicability = Applicability::MachineApplicable;
+                let snippet = if ident == path.segments[path.segments.len() - 1].ident {
+                    let adjusted_span = path.span.with_hi(path.span.hi() - BytePos(8));
+                    format!(
+                        "{}",
+                        snippet_with_applicability(cx, adjusted_span, "..", &mut applicability)
+                    )
+                } else {
+                    let adjusted_span = path.span.until(ident.span);
+                    let adjusted_span = adjusted_span.with_hi(adjusted_span.hi() - BytePos(11));
+                    format!(
+                        "{} as {}",
+                        snippet_with_applicability(cx, adjusted_span, "..", &mut applicability),
+                        snippet_with_applicability(cx, ident.span, "..", &mut applicability)
+                    )
+                };
+                span_lint_and_sugg(
+                    cx,
+                    UNNECESSARY_SELF_IMPORTS,
+                    path.span,
+                    "import ending with `self`",
+                    "consider omitting `::{self}`",
+                    snippet,
+                    applicability,
+                );
+            }
+        }
+    }
+}
+
+fn is_self_import(cx: &LateContext<'_>, current_import: &Item<'_>, current_import_path: &Path<'_>) -> Option<Ident> {
+    let definitions = cx.tcx.hir().definitions();
+    let mut amt = 0;
+    let mut ident: Option<Ident> = None;
+
+    for def in definitions.iter_local_def_id() {
+        if_chain! {
+            if let Some(Node::Item(item)) = cx.tcx.hir().get_if_local(def.to_def_id());
+            if let ItemKind::Use(path, kind) = item.kind;
+            if matches!(kind, UseKind::Single | UseKind::Glob);
+            if current_import.span.contains(item.span);
+            then {
+                amt += 1;
+                if amt > 1 { return None; }
+
+                if_chain! {
+                    if current_import_path.segments.len() == path.segments.len();
+                    let current_import_last = &current_import_path.segments[current_import_path.segments.len() - 1];
+                    let item_last = &path.segments[path.segments.len() - 1];
+                    if current_import_last.ident == item_last.ident;
+                    then {
+                        ident = Some(item.ident);
+                    }
+                }
+            }
+        }
+    }
+    ident
+}
+
+fn mod_contains_item(cx: &LateContext<'_>, def_id: DefId, ident: Ident) -> bool {
+    if let Some(Node::Item(node)) = cx.tcx.hir().get_if_local(def_id) {
+        if let ItemKind::Mod(Mod { item_ids, .. }) = &node.kind {
+            for item in item_ids.iter() {
+                if_chain! {
+                    if let Some(Node::Item(node)) = cx.tcx.hir().get_if_local(item.def_id.to_def_id());
+                    if let VisibilityKind::Public = node.vis.node;
+                    if node.ident == ident;
+                    if matches!(node.kind, ItemKind::Fn(..) | ItemKind::Const(..) | ItemKind::Static(..));
+                    then { return true; }
+
+                }
+            }
+        }
+    } else {
+        for item in cx.tcx.item_children(def_id).iter() {
+            if_chain! {
+                if item.ident == ident;
+                if matches!(item.res, Res::Def(DefKind::Fn | DefKind::Const | DefKind::Static, _));
+                then { return true; }
+            }
+        }
+    }
+    false
+}
+
+fn item_is_in_scope(cx: &LateContext<'_>, ident: Ident, current_import: Span) -> bool {
+    let definitions = cx.tcx.hir().definitions();
+    let mut outer_mod: Option<Span> = None;
+
+    for def in definitions.iter_local_def_id() {
+        if let Some(Node::Item(item)) = cx.tcx.hir().get_if_local(def.to_def_id()) {
+            let should_check = match outer_mod {
+                Some(curr_mod) => {
+                    if curr_mod.contains(item.span) {
+                        false
+                    } else {
+                        outer_mod = None;
+                        true
+                    }
+                },
+                None => true,
+            };
+
+            if should_check {
+                match item.kind {
+                    ItemKind::Fn(..) | ItemKind::Const(..) | ItemKind::Static(..) => {
+                        if item.ident == ident {
+                            return true;
+                        }
+                    },
+                    ItemKind::Use(path, kind) => {
+                        if_chain! {
+                            if let UseKind::Single = kind;
+                            if !current_import.contains(path.span);
+                            if item.ident == ident;
+
+                            then {
+                                return true;
+                            }
+                        }
+                    },
+                    _ => {
+                        outer_mod = Some(item.span);
+                    },
+                }
+            }
+        }
+    }
+    false
+}
diff --git a/tests/ui/unnecessary_self_imports.fixed b/tests/ui/unnecessary_self_imports.fixed
@@ -0,0 +1,38 @@
+// run-rustfix
+#![warn(clippy::unnecessary_self_imports)]
+#![allow(non_camel_case_types, non_upper_case_globals, unused_imports, dead_code)]
+
+mod a {
+    pub mod mod_and_fn_in_scope {}
+    pub fn mod_and_fn_in_scope() {}
+
+    pub mod only_module {}
+
+    pub mod mod_and_fn {}
+    pub fn mod_and_fn() {}
+
+    pub enum enum_and_const {}
+    pub const enum_and_const: u32 = 1;
+
+    pub struct struct_and_static {}
+    pub static struct_and_static: u32 = 1;
+}
+
+mod b {
+    pub static struct_and_static: u32 = 2;
+}
+
+fn mod_and_fn_in_scope() {}
+const enum_and_const: u32 = 2;
+
+use std::io;
+
+use a::enum_and_const as alias;
+use a::mod_and_fn::{self, *};
+use a::mod_and_fn_in_scope::{self};
+use a::only_module;
+use a::struct_and_static::{self};
+
+use b::struct_and_static;
+
+fn main() {}
diff --git a/tests/ui/unnecessary_self_imports.rs b/tests/ui/unnecessary_self_imports.rs
@@ -0,0 +1,38 @@
+// run-rustfix
+#![warn(clippy::unnecessary_self_imports)]
+#![allow(non_camel_case_types, non_upper_case_globals, unused_imports, dead_code)]
+
+mod a {
+    pub mod mod_and_fn_in_scope {}
+    pub fn mod_and_fn_in_scope() {}
+
+    pub mod only_module {}
+
+    pub mod mod_and_fn {}
+    pub fn mod_and_fn() {}
+
+    pub enum enum_and_const {}
+    pub const enum_and_const: u32 = 1;
+
+    pub struct struct_and_static {}
+    pub static struct_and_static: u32 = 1;
+}
+
+mod b {
+    pub static struct_and_static: u32 = 2;
+}
+
+fn mod_and_fn_in_scope() {}
+const enum_and_const: u32 = 2;
+
+use std::io::{self};
+
+use a::enum_and_const::{self as alias};
+use a::mod_and_fn::{self, *};
+use a::mod_and_fn_in_scope::{self};
+use a::only_module::{self};
+use a::struct_and_static::{self};
+
+use b::struct_and_static;
+
+fn main() {}
diff --git a/tests/ui/unnecessary_self_imports.stderr b/tests/ui/unnecessary_self_imports.stderr
@@ -0,0 +1,22 @@
+error: import ending with `self`
+  --> $DIR/unnecessary_self_imports.rs:28:5
+   |
+LL | use std::io::{self};
+   |     ^^^^^^^^^^^^^^^ help: consider omitting `::{self}`: `std::io`
+   |
+   = note: `-D clippy::unnecessary-self-imports` implied by `-D warnings`
+
+error: import ending with `self`
+  --> $DIR/unnecessary_self_imports.rs:30:5
+   |
+LL | use a::enum_and_const::{self as alias};
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider omitting `::{self}`: `a::enum_and_const as alias`
+
+error: import ending with `self`
+  --> $DIR/unnecessary_self_imports.rs:33:5
+   |
+LL | use a::only_module::{self};
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: consider omitting `::{self}`: `a::only_module`
+
+error: aborting due to 3 previous errors
+
