diff --git a/CHANGELOG.md b/CHANGELOG.md index 7885bdfb12c2..d9a198f8243a 100644 --- 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 index 78a95b004034..6f7c0f56c9c3 100644 --- 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 index 6a52de4f7136..64e9dc85466e 100644 --- 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 new file mode 100644 index 000000000000..7665b40123b3 --- /dev/null +++ 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 { + let definitions = cx.tcx.hir().definitions(); + let mut amt = 0; + let mut ident: Option = 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 = ¤t_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 = 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 new file mode 100644 index 000000000000..1a0bfa85009e --- /dev/null +++ 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 new file mode 100644 index 000000000000..1c13d8787175 --- /dev/null +++ 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 new file mode 100644 index 000000000000..672e65578946 --- /dev/null +++ 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 +