From 3b8375d90b0e7d04c992b368bb7632ddab6841ff Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 3 Feb 2016 15:38:23 +0100 Subject: [PATCH] warn on `use`ing all variants of an enum --- README.md | 3 +- src/enum_glob_use.rs | 61 +++++++++++++++++++++++++++++ src/lib.rs | 3 ++ tests/compile-fail/enum_glob_use.rs | 20 ++++++++++ 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 src/enum_glob_use.rs create mode 100644 tests/compile-fail/enum_glob_use.rs diff --git a/README.md b/README.md index 006ddcd7a7f..424c89b28b8 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 109 lints included in this crate: +There are 110 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -33,6 +33,7 @@ name [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected +[enum_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use) | allow | finds use items that import all variants of an enum [enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#enum_variant_names) | warn | finds enums where all variants share a prefix/postfix [eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`) [expl_impl_clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy) | warn | implementing `Clone` explicitly on `Copy` types diff --git a/src/enum_glob_use.rs b/src/enum_glob_use.rs new file mode 100644 index 00000000000..d6df307fec2 --- /dev/null +++ b/src/enum_glob_use.rs @@ -0,0 +1,61 @@ +//! lint on `use`ing all variants of an enum + +use rustc::lint::{LateLintPass, LintPass, LateContext, LintArray, LintContext}; +use rustc_front::hir::*; +use rustc::front::map::Node::NodeItem; +use rustc::front::map::PathElem::PathName; +use rustc::middle::ty::TyEnum; +use utils::span_lint; +use syntax::codemap::Span; +use syntax::ast::NodeId; + +/// **What it does:** Warns when `use`ing all variants of an enum +/// +/// **Why is this bad?** It is usually better style to use the prefixed name of an enum variant, rather than importing variants +/// +/// **Known problems:** Old-style enums that prefix the variants are still around +/// +/// **Example:** `use std::cmp::Ordering::*;` +declare_lint! { pub ENUM_GLOB_USE, Allow, + "finds use items that import all variants of an enum" } + +pub struct EnumGlobUse; + +impl LintPass for EnumGlobUse { + fn get_lints(&self) -> LintArray { + lint_array!(ENUM_GLOB_USE) + } +} + +impl LateLintPass for EnumGlobUse { + fn check_mod(&mut self, cx: &LateContext, m: &Mod, _: Span, _: NodeId) { + // only check top level `use` statements + for item in &m.item_ids { + self.lint_item(cx, cx.krate.item(item.id)); + } + } +} + +impl EnumGlobUse { + fn lint_item(&self, cx: &LateContext, item: &Item) { + if item.vis == Visibility::Public { + return; // re-exports are fine + } + if let ItemUse(ref item_use) = item.node { + if let ViewPath_::ViewPathGlob(_) = item_use.node { + let def = cx.tcx.def_map.borrow()[&item.id]; + if let Some(NodeItem(it)) = cx.tcx.map.get_if_local(def.def_id()) { + if let ItemEnum(..) = it.node { + span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); + } + } else { + if let Some(&PathName(_)) = cx.sess().cstore.item_path(def.def_id()).last() { + if let TyEnum(..) = cx.sess().cstore.item_type(&cx.tcx, def.def_id()).ty.sty { + span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); + } + } + } + } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 44dd9fa8c86..4f3de70db4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -37,6 +37,7 @@ pub mod utils; pub mod consts; pub mod types; pub mod misc; +pub mod enum_glob_use; pub mod eq_op; pub mod bit_mask; pub mod ptr_arg; @@ -95,6 +96,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box misc::CmpNan); reg.register_late_lint_pass(box eq_op::EqOp); reg.register_early_lint_pass(box enum_variants::EnumVariantNames); + reg.register_late_lint_pass(box enum_glob_use::EnumGlobUse); reg.register_late_lint_pass(box bit_mask::BitMask); reg.register_late_lint_pass(box ptr_arg::PtrArg); reg.register_late_lint_pass(box needless_bool::NeedlessBool); @@ -153,6 +155,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_group("clippy_pedantic", vec![ + enum_glob_use::ENUM_GLOB_USE, matches::SINGLE_MATCH_ELSE, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, diff --git a/tests/compile-fail/enum_glob_use.rs b/tests/compile-fail/enum_glob_use.rs new file mode 100644 index 00000000000..fc5f531ba90 --- /dev/null +++ b/tests/compile-fail/enum_glob_use.rs @@ -0,0 +1,20 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![deny(clippy, clippy_pedantic)] +#![allow(unused_imports, dead_code)] + +use std::cmp::Ordering::*; //~ ERROR: don't use glob imports for enum variants + +enum Enum {} + +use self::Enum::*; //~ ERROR: don't use glob imports for enum variants + +fn blarg() { + use self::Enum::*; // ok, just for a function +} + +mod blurg { + pub use std::cmp::Ordering::*; // ok, re-export +} + +fn main() {}