From e88e637b67a8cdc4134153fcd4424829fc43c9ed Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 21:07:54 -0700 Subject: [PATCH] Add empty_enum lint (just a copy of large_enum_variant for now) --- clippy_lints/src/derive.rs | 4 +- clippy_lints/src/empty_enum.rs | 82 +++++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + clippy_lints/src/map_clone.rs | 4 +- clippy_lints/src/methods.rs | 4 +- clippy_lints/src/needless_bool.rs | 12 +++-- clippy_lints/src/no_effect.rs | 8 ++- clippy_lints/src/precedence.rs | 11 ++--- clippy_lints/src/shadow.rs | 8 ++- clippy_lints/src/utils/sugg.rs | 5 +- clippy_lints/src/vec.rs | 8 ++- 11 files changed, 120 insertions(+), 28 deletions(-) create mode 100644 clippy_lints/src/empty_enum.rs diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index b2a8d26000d..1e4959a9ccc 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -173,6 +173,8 @@ fn check_copy_clone<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, item: &Item, trait_ref EXPL_IMPL_CLONE_ON_COPY, item.span, "you are implementing `Clone` explicitly on a `Copy` type", - |db| { db.span_note(item.span, "consider deriving `Clone` or removing `Copy`"); }); + |db| { + db.span_note(item.span, "consider deriving `Clone` or removing `Copy`"); + }); } } diff --git a/clippy_lints/src/empty_enum.rs b/clippy_lints/src/empty_enum.rs new file mode 100644 index 00000000000..17be52d659a --- /dev/null +++ b/clippy_lints/src/empty_enum.rs @@ -0,0 +1,82 @@ +//! lint when there is an enum with no variants + +use rustc::lint::*; +use rustc::hir::*; +use utils::{span_lint_and_then, snippet_opt}; +use rustc::ty::layout::TargetDataLayout; +use rustc::ty::TypeFoldable; +use rustc::traits::Reveal; + +/// **What it does:** Checks for `enum`s with no variants. +/// +/// **Why is this bad?** Enum's with no variants should be replaced with `!`. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// enum Test {} +/// ``` +declare_lint! { + pub EMPTY_ENUM, + Warn, + "enum with no variants" +} + +#[derive(Copy,Clone)] +pub struct EmptyEnum; + +impl LintPass for EmptyEnum { + fn get_lints(&self) -> LintArray { + lint_array!(EMPTY_ENUM) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EmptyEnum { + fn check_item(&mut self, cx: &LateContext, item: &Item) { + let did = cx.tcx.hir.local_def_id(item.id); + if let ItemEnum(ref def, _) = item.node { + let ty = cx.tcx.item_type(did); + let adt = ty.ty_adt_def().expect("already checked whether this is an enum"); + for (i, variant) in adt.variants.iter().enumerate() { + let data_layout = TargetDataLayout::parse(cx.sess()); + cx.tcx.infer_ctxt((), Reveal::All).enter(|infcx| { + let size: u64 = variant.fields + .iter() + .map(|f| { + let ty = cx.tcx.item_type(f.did); + if ty.needs_subst() { + 0 // we can't reason about generics, so we treat them as zero sized + } else { + ty.layout(&infcx) + .expect("layout should be computable for concrete type") + .size(&data_layout) + .bytes() + } + }) + .sum(); + if size > 0 { + span_lint_and_then(cx, EMPTY_ENUM, def.variants[i].span, "large enum variant found", |db| { + if variant.fields.len() == 1 { + let span = match def.variants[i].node.data { + VariantData::Struct(ref fields, _) | + VariantData::Tuple(ref fields, _) => fields[0].ty.span, + VariantData::Unit(_) => unreachable!(), + }; + if let Some(snip) = snippet_opt(cx, span) { + db.span_suggestion(span, + "consider boxing the large fields to reduce the total size of \ + the enum", + format!("Box<{}>", snip)); + return; + } + } + db.span_help(def.variants[i].span, + "consider boxing the large fields to reduce the total size of the enum"); + }); + } + }); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bbdd62a3013..650ad08f154 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -72,6 +72,7 @@ macro_rules! declare_restriction_lint { pub mod doc; pub mod double_parens; pub mod drop_forget_ref; +pub mod empty_enum; pub mod entry; pub mod enum_clike; pub mod enum_glob_use; @@ -265,6 +266,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { max_single_char_names: conf.max_single_char_names, }); reg.register_late_lint_pass(box drop_forget_ref::Pass); + reg.register_late_lint_pass(box empty_enum::EmptyEnum); reg.register_late_lint_pass(box types::AbsurdExtremeComparisons); reg.register_late_lint_pass(box types::InvalidUpcastComparisons); reg.register_late_lint_pass(box regex::Pass::default()); diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 087ae8c31b9..2cf5cd981cc 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -1,8 +1,8 @@ use rustc::lint::*; use rustc::hir::*; use syntax::ast; -use utils::{is_adjusted, match_path, match_trait_method, match_type, remove_blocks, paths, snippet, span_help_and_lint, - walk_ptrs_ty, walk_ptrs_ty_depth, iter_input_pats}; +use utils::{is_adjusted, match_path, match_trait_method, match_type, remove_blocks, paths, snippet, + span_help_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, iter_input_pats}; /// **What it does:** Checks for mapping `clone()` over an iterator. /// diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 30ffcc9f6e3..f0732c1cd90 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -1213,7 +1213,9 @@ fn lint_single_char_pattern(cx: &LateContext, expr: &hir::Expr, arg: &hir::Expr) SINGLE_CHAR_PATTERN, arg.span, "single-character string constant used as pattern", - |db| { db.span_suggestion(expr.span, "try using a char instead:", hint); }); + |db| { + db.span_suggestion(expr.span, "try using a char instead:", hint); + }); } } } diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index c9ade60c339..5eae3034aff 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -74,7 +74,9 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { NEEDLESS_BOOL, e.span, "this if-then-else expression returns a bool literal", - |db| { db.span_suggestion(e.span, "you can reduce it to", hint); }); + |db| { + db.span_suggestion(e.span, "you can reduce it to", hint); + }); }; match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) { (RetBool(true), RetBool(true)) | @@ -121,7 +123,9 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { BOOL_COMPARISON, e.span, "equality checks against true are unnecessary", - |db| { db.span_suggestion(e.span, "try simplifying it as shown:", hint); }); + |db| { + db.span_suggestion(e.span, "try simplifying it as shown:", hint); + }); }, (Other, Bool(true)) => { let hint = snippet(cx, left_side.span, "..").into_owned(); @@ -129,7 +133,9 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { BOOL_COMPARISON, e.span, "equality checks against true are unnecessary", - |db| { db.span_suggestion(e.span, "try simplifying it as shown:", hint); }); + |db| { + db.span_suggestion(e.span, "try simplifying it as shown:", hint); + }); }, (Bool(false), Other) => { let hint = Sugg::hir(cx, right_side, ".."); diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index f53fcb60706..c0ca07b3466 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -120,11 +120,9 @@ fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) { return; } } - span_lint_and_then(cx, - UNNECESSARY_OPERATION, - stmt.span, - "statement can be reduced", - |db| { db.span_suggestion(stmt.span, "replace it with", snippet); }); + span_lint_and_then(cx, UNNECESSARY_OPERATION, stmt.span, "statement can be reduced", |db| { + db.span_suggestion(stmt.span, "replace it with", snippet); + }); } } } diff --git a/clippy_lints/src/precedence.rs b/clippy_lints/src/precedence.rs index 0dff1495dfb..949a2c7e767 100644 --- a/clippy_lints/src/precedence.rs +++ b/clippy_lints/src/precedence.rs @@ -36,12 +36,11 @@ fn get_lints(&self) -> LintArray { impl EarlyLintPass for Precedence { fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) { if let ExprKind::Binary(Spanned { node: op, .. }, ref left, ref right) = expr.node { - let span_sugg = - |expr: &Expr, sugg| { - span_lint_and_then(cx, PRECEDENCE, expr.span, "operator precedence can trip the unwary", |db| { - db.span_suggestion(expr.span, "consider parenthesizing your expression", sugg); - }); - }; + let span_sugg = |expr: &Expr, sugg| { + span_lint_and_then(cx, PRECEDENCE, expr.span, "operator precedence can trip the unwary", |db| { + db.span_suggestion(expr.span, "consider parenthesizing your expression", sugg); + }); + }; if !is_bit_op(op) { return; diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 62ef155c321..689e45ef87a 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -250,7 +250,9 @@ fn lint_shadow<'a, 'tcx: 'a>( &format!("`{}` is shadowed by itself in `{}`", snippet(cx, pattern_span, "_"), snippet(cx, expr.span, "..")), - |db| { db.span_note(prev_span, "previous binding is here"); }); + |db| { + db.span_note(prev_span, "previous binding is here"); + }); } else if contains_self(cx, name, expr) { span_lint_and_then(cx, SHADOW_REUSE, @@ -280,7 +282,9 @@ fn lint_shadow<'a, 'tcx: 'a>( SHADOW_UNRELATED, span, &format!("`{}` shadows a previous declaration", snippet(cx, pattern_span, "_")), - |db| { db.span_note(prev_span, "previous binding is here"); }); + |db| { + db.span_note(prev_span, "previous binding is here"); + }); } } diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 2c10db158cb..378af61f58d 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -258,9 +258,8 @@ fn is_arith(op: &AssocOp) -> bool { fn needs_paren(op: &AssocOp, other: &AssocOp, dir: Associativity) -> bool { other.precedence() < op.precedence() || (other.precedence() == op.precedence() && - ((op != other && associativity(op) != dir) || - (op == other && associativity(op) != Associativity::Both))) || is_shift(op) && is_arith(other) || - is_shift(other) && is_arith(op) + ((op != other && associativity(op) != dir) || (op == other && associativity(op) != Associativity::Both))) || + is_shift(op) && is_arith(other) || is_shift(other) && is_arith(op) } let lhs_paren = if let Sugg::BinOp(ref lop, _) = *lhs { diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index a8d2701ccfa..fafe39170bb 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -81,11 +81,9 @@ fn check_vec_macro(cx: &LateContext, vec_args: &higher::VecArgs, span: Span) { }, }; - span_lint_and_then(cx, - USELESS_VEC, - span, - "useless use of `vec!`", - |db| { db.span_suggestion(span, "you can use a slice directly", snippet); }); + span_lint_and_then(cx, USELESS_VEC, span, "useless use of `vec!`", |db| { + db.span_suggestion(span, "you can use a slice directly", snippet); + }); } /// Return the item type of the vector (ie. the `T` in `Vec`). -- 2.44.0