Skip to content

Commit

Permalink
add unnecessary_self_imports lint
Browse files Browse the repository at this point in the history
  • Loading branch information
ebobrow committed Apr 13, 2021
1 parent e9728b8 commit 76db21e
Show file tree
Hide file tree
Showing 7 changed files with 282 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/modulo_arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
177 changes: 177 additions & 0 deletions clippy_lints/src/unnecessary_self_imports.rs
Original file line number Diff line number Diff line change
@@ -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
}
38 changes: 38 additions & 0 deletions tests/ui/unnecessary_self_imports.fixed
Original file line number Diff line number Diff line change
@@ -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() {}
38 changes: 38 additions & 0 deletions tests/ui/unnecessary_self_imports.rs
Original file line number Diff line number Diff line change
@@ -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() {}
22 changes: 22 additions & 0 deletions tests/ui/unnecessary_self_imports.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 76db21e

Please sign in to comment.