From e88e637b67a8cdc4134153fcd4424829fc43c9ed Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 21:07:54 -0700 Subject: [PATCH 1/7] 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 @@ pub mod derive; 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 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool { 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 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison { 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 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison { 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 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { 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 @@ impl LintPass for Precedence { 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 @@ pub fn make_assoc(op: AssocOp, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> { 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`). From 49238ad1d20538c778b4b3a820f4e630ab8d2a1f Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 21:52:44 -0700 Subject: [PATCH 2/7] Implement empty_enum lint and add a test --- clippy_lints/src/empty_enum.rs | 50 +++++--------------------------- tests/compile-fail/empty_enum.rs | 10 +++++++ 2 files changed, 17 insertions(+), 43 deletions(-) create mode 100644 tests/compile-fail/empty_enum.rs diff --git a/clippy_lints/src/empty_enum.rs b/clippy_lints/src/empty_enum.rs index 17be52d659a..f930a8599e2 100644 --- a/clippy_lints/src/empty_enum.rs +++ b/clippy_lints/src/empty_enum.rs @@ -2,10 +2,7 @@ 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; +use utils::span_lint_and_then; /// **What it does:** Checks for `enum`s with no variants. /// @@ -19,7 +16,7 @@ use rustc::traits::Reveal; /// ``` declare_lint! { pub EMPTY_ENUM, - Warn, + Allow, "enum with no variants" } @@ -35,46 +32,13 @@ impl LintPass for EmptyEnum { 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 { + if let ItemEnum(..) = 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"); - }); - } + if adt.variants.is_empty() { + span_lint_and_then(cx, EMPTY_ENUM, item.span, "enum with no variants", |db| { + db.span_help(item.span, + "consider using the uninhabited type `!` or a wrapper around it"); }); } } diff --git a/tests/compile-fail/empty_enum.rs b/tests/compile-fail/empty_enum.rs new file mode 100644 index 00000000000..f6a9de3c7f0 --- /dev/null +++ b/tests/compile-fail/empty_enum.rs @@ -0,0 +1,10 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(empty_enum)] + +enum Empty {} //~ ERROR enum with no variants + //~^ HELP consider using the uninhabited type `!` or a wrapper around it + +fn main() { +} From 1193f4fb689dc54b66b083ce74bd24cbcb91d637 Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 21:54:21 -0700 Subject: [PATCH 3/7] Run update_lints.py --- CHANGELOG.md | 1 + README.md | 3 ++- clippy_lints/src/lib.rs | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39751ef031c..b446de51c02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -306,6 +306,7 @@ All notable changes to this project will be documented in this file. [`double_parens`]: https://github.com/Manishearth/rust-clippy/wiki#double_parens [`drop_ref`]: https://github.com/Manishearth/rust-clippy/wiki#drop_ref [`duplicate_underscore_argument`]: https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument +[`empty_enum`]: https://github.com/Manishearth/rust-clippy/wiki#empty_enum [`empty_loop`]: https://github.com/Manishearth/rust-clippy/wiki#empty_loop [`enum_clike_unportable_variant`]: https://github.com/Manishearth/rust-clippy/wiki#enum_clike_unportable_variant [`enum_glob_use`]: https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use diff --git a/README.md b/README.md index 18166118361..eb1af46c234 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,7 @@ transparently: ## Lints -There are 184 lints included in this crate: +There are 185 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -220,6 +220,7 @@ name [double_parens](https://github.com/Manishearth/rust-clippy/wiki#double_parens) | warn | Warn on unnecessary double parentheses [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | calls to `std::mem::drop` with a reference instead of an owned 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_enum](https://github.com/Manishearth/rust-clippy/wiki#empty_enum) | allow | enum with no variants [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}`, which should block or sleep [enum_clike_unportable_variant](https://github.com/Manishearth/rust-clippy/wiki#enum_clike_unportable_variant) | warn | C-like enums that are `repr(isize/usize)` and have values that don't fit into an `i32` [enum_glob_use](https://github.com/Manishearth/rust-clippy/wiki#enum_glob_use) | allow | use items that import all variants of an enum diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 650ad08f154..c4f5458b002 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -306,6 +306,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_lint_group("clippy_pedantic", vec![ booleans::NONMINIMAL_BOOL, + empty_enum::EMPTY_ENUM, enum_glob_use::ENUM_GLOB_USE, enum_variants::PUB_ENUM_VARIANT_NAMES, enum_variants::STUTTER, From c922eb9db5bf122cd92d973240c29832ce8883f9 Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 22:09:54 -0700 Subject: [PATCH 4/7] Suggest to use a wrapper in the wiki for the empty_enum lint --- clippy_lints/src/empty_enum.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/empty_enum.rs b/clippy_lints/src/empty_enum.rs index f930a8599e2..896087f4706 100644 --- a/clippy_lints/src/empty_enum.rs +++ b/clippy_lints/src/empty_enum.rs @@ -6,7 +6,8 @@ use utils::span_lint_and_then; /// **What it does:** Checks for `enum`s with no variants. /// -/// **Why is this bad?** Enum's with no variants should be replaced with `!`. +/// **Why is this bad?** Enum's with no variants should be replaced with `!`, the uninhabited type, +/// or a wrapper around it. /// /// **Known problems:** None. /// From 31919aff3b4bf0369e6cc26978a64123c4a10998 Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 22:22:33 -0700 Subject: [PATCH 5/7] Revert changes from accidentally running rustfmt --- clippy_lints/src/derive.rs | 4 +--- 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 +++++--- 9 files changed, 28 insertions(+), 36 deletions(-) diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 1e4959a9ccc..b2a8d26000d 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -173,8 +173,6 @@ 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/map_clone.rs b/clippy_lints/src/map_clone.rs index 2cf5cd981cc..087ae8c31b9 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 f0732c1cd90..30ffcc9f6e3 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -1213,9 +1213,7 @@ 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 5eae3034aff..c9ade60c339 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -74,9 +74,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool { 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)) | @@ -123,9 +121,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison { 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(); @@ -133,9 +129,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoolComparison { 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 c0ca07b3466..f53fcb60706 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -120,9 +120,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { 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 949a2c7e767..0dff1495dfb 100644 --- a/clippy_lints/src/precedence.rs +++ b/clippy_lints/src/precedence.rs @@ -36,11 +36,12 @@ impl LintPass for Precedence { 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 689e45ef87a..62ef155c321 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -250,9 +250,7 @@ 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, @@ -282,9 +280,7 @@ 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 378af61f58d..2c10db158cb 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -258,8 +258,9 @@ pub fn make_assoc(op: AssocOp, lhs: &Sugg, rhs: &Sugg) -> Sugg<'static> { 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 fafe39170bb..a8d2701ccfa 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -81,9 +81,11 @@ 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`). From 7570af0557b5fde37e256c3a186ab1d0eec1f390 Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sat, 4 Feb 2017 22:59:39 -0700 Subject: [PATCH 6/7] Make tests pass --- tests/compile-fail/empty_enum.rs | 1 + tests/compile-fail/enum_glob_use.rs | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/compile-fail/empty_enum.rs b/tests/compile-fail/empty_enum.rs index f6a9de3c7f0..ac9b314c00a 100644 --- a/tests/compile-fail/empty_enum.rs +++ b/tests/compile-fail/empty_enum.rs @@ -1,6 +1,7 @@ #![feature(plugin)] #![plugin(clippy)] +#![allow(dead_code)] #![deny(empty_enum)] enum Empty {} //~ ERROR enum with no variants diff --git a/tests/compile-fail/enum_glob_use.rs b/tests/compile-fail/enum_glob_use.rs index 12fd104312a..86539c2b13e 100644 --- a/tests/compile-fail/enum_glob_use.rs +++ b/tests/compile-fail/enum_glob_use.rs @@ -5,7 +5,9 @@ use std::cmp::Ordering::*; //~ ERROR: don't use glob imports for enum variants -enum Enum {} +enum Enum { + _Foo, +} use self::Enum::*; //~ ERROR: don't use glob imports for enum variants From 59e0ae75d05332ce0c8413000ba1bae0b1b16848 Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Sun, 5 Feb 2017 09:51:31 -0700 Subject: [PATCH 7/7] Make rustfmt happy --- clippy_lints/src/empty_enum.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clippy_lints/src/empty_enum.rs b/clippy_lints/src/empty_enum.rs index 896087f4706..631ac8b0fe2 100644 --- a/clippy_lints/src/empty_enum.rs +++ b/clippy_lints/src/empty_enum.rs @@ -38,8 +38,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EmptyEnum { let adt = ty.ty_adt_def().expect("already checked whether this is an enum"); if adt.variants.is_empty() { span_lint_and_then(cx, EMPTY_ENUM, item.span, "enum with no variants", |db| { - db.span_help(item.span, - "consider using the uninhabited type `!` or a wrapper around it"); + db.span_help(item.span, "consider using the uninhabited type `!` or a wrapper around it"); }); } }