From 925e8207fa8ff6cc4aea7337b6cb3eeb318123d3 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 21:58:29 +0100 Subject: [PATCH] Respond to review comments Update README and CHANGELOG using the util scripts, refine the help message and fix the float_cmp error. --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 5 +++-- clippy_lints/src/trait_bounds.rs | 18 +++++++++------- clippy_lints/src/utils/hir_utils.rs | 30 +++++++++++---------------- src/lintlist/mod.rs | 9 +++++++- tests/ui/float_cmp.stderr | 11 +++++++++- tests/ui/type_repetition_in_bounds.rs | 17 +++++++++------ 8 files changed, 57 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff5aebbc8..089897811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1138,6 +1138,7 @@ Released 2018-09-13 [`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity +[`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg diff --git a/README.md b/README.md index 1e649d898..38651f72e 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 308 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 309 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index faffb1a67..908bbeb5e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -263,9 +263,9 @@ pub mod strings; pub mod suspicious_trait_impl; pub mod swap; pub mod temporary_assignment; +pub mod trait_bounds; pub mod transmute; pub mod transmuting_null; -pub mod trait_bounds; pub mod trivially_copy_pass_by_ref; pub mod try_err; pub mod types; @@ -860,6 +860,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { swap::ALMOST_SWAPPED, swap::MANUAL_SWAP, temporary_assignment::TEMPORARY_ASSIGNMENT, + trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, transmute::TRANSMUTE_BYTES_TO_STR, transmute::TRANSMUTE_INT_TO_BOOL, @@ -870,7 +871,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { transmute::USELESS_TRANSMUTE, transmute::WRONG_TRANSMUTE, transmuting_null::TRANSMUTING_NULL, - trait_bounds::TYPE_REPETITION_IN_BOUNDS, trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, try_err::TRY_ERR, types::ABSURD_EXTREME_COMPARISONS, @@ -1042,6 +1042,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reference::REF_IN_DEREF, swap::MANUAL_SWAP, temporary_assignment::TEMPORARY_ASSIGNMENT, + trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, transmute::TRANSMUTE_BYTES_TO_STR, transmute::TRANSMUTE_INT_TO_BOOL, diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index f5ca3793d..8a719c0dd 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,8 +1,8 @@ use crate::utils::{in_macro, snippet, span_help_and_lint, SpanlessHash}; +use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_tool_lint, impl_lint_pass}; use rustc_data_structures::fx::FxHashMap; -use rustc::hir::*; #[derive(Copy, Clone)] pub struct TraitBounds; @@ -10,11 +10,12 @@ pub struct TraitBounds; declare_clippy_lint! { /// **What it does:** This lint warns about unnecessary type repetitions in trait bounds /// - /// **Why is this bad?** Complexity + /// **Why is this bad?** Repeating the type for every bound makes the code + /// less readable than combining the bounds /// /// **Example:** /// ```rust - /// pub fn foo(t: T) where T: Copy, T: Clone + /// pub fn foo(t: T) where T: Copy, T: Clone /// ``` /// /// Could be written as: @@ -34,7 +35,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { if in_macro(gen.span) { return; } - let hash = | ty | -> u64 { + let hash = |ty| -> u64 { let mut hasher = SpanlessHash::new(cx, cx.tables); hasher.hash_ty(ty); hasher.finish() @@ -44,17 +45,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { if let WherePredicate::BoundPredicate(ref p) = bound { let h = hash(&p.bounded_ty); if let Some(ref v) = map.insert(h, p.bounds.iter().collect::>()) { - let mut hint_string = format!("consider combining the bounds: `{}: ", snippet(cx, p.bounded_ty.span, "_")); + let mut hint_string = format!( + "consider combining the bounds: `{}:", + snippet(cx, p.bounded_ty.span, "_") + ); for b in v.iter() { if let GenericBound::Trait(ref poly_trait_ref, _) = b { let path = &poly_trait_ref.trait_ref.path; - hint_string.push_str(&format!("{}, ", path)); + hint_string.push_str(&format!(" {} +", path)); } } for b in p.bounds.iter() { if let GenericBound::Trait(ref poly_trait_ref, _) = b { let path = &poly_trait_ref.trait_ref.path; - hint_string.push_str(&format!("{}, ", path)); + hint_string.push_str(&format!(" {} +", path)); } } hint_string.truncate(hint_string.len() - 2); diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 449b8c439..db7b10c6b 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -639,23 +639,20 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { for ty in ty_list { self.hash_ty(ty); } - }, - TyKind::Path(qpath) => { - match qpath { - QPath::Resolved(ref maybe_ty, ref path) => { - if let Some(ref ty) = maybe_ty { - self.hash_ty(ty); - } - for segment in &path.segments { - segment.ident.name.hash(&mut self.s); - } - }, - QPath::TypeRelative(ref ty, ref segment) => { + TyKind::Path(qpath) => match qpath { + QPath::Resolved(ref maybe_ty, ref path) => { + if let Some(ref ty) = maybe_ty { self.hash_ty(ty); + } + for segment in &path.segments { segment.ident.name.hash(&mut self.s); - }, - } + } + }, + QPath::TypeRelative(ref ty, ref segment) => { + self.hash_ty(ty); + segment.ident.name.hash(&mut self.s); + }, }, TyKind::Def(_, arg_list) => { for arg in arg_list { @@ -670,14 +667,11 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { }, TyKind::TraitObject(_, lifetime) => { self.hash_lifetime(lifetime); - }, TyKind::Typeof(anon_const) => { self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); }, - TyKind::CVarArgs(lifetime) => { - self.hash_lifetime(lifetime) - }, + TyKind::CVarArgs(lifetime) => self.hash_lifetime(lifetime), TyKind::Err | TyKind::Infer | TyKind::Never => {}, } } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 2d9e800b6..49bce5a6c 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 308] = [ +pub const ALL_LINTS: [Lint; 309] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -1848,6 +1848,13 @@ pub const ALL_LINTS: [Lint; 308] = [ deprecation: None, module: "types", }, + Lint { + name: "type_repetition_in_bounds", + group: "complexity", + desc: "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`", + deprecation: None, + module: "trait_bounds", + }, Lint { name: "unicode_not_nfc", group: "pedantic", diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index 5dc5fbf0f..d1ffc0d15 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -1,3 +1,12 @@ +error: this type has already been used as a bound predicate + --> $DIR/float_cmp.rs:12:5 + | +LL | T: Copy, + | ^^^^^^^ + | + = note: `-D clippy::type-repetition-in-bounds` implied by `-D warnings` + = help: consider combining the bounds: `T: Add, Copy` + error: strict comparison of f32 or f64 --> $DIR/float_cmp.rs:60:5 | @@ -35,5 +44,5 @@ note: std::f32::EPSILON and std::f64::EPSILON are available. LL | twice(x) != twice(ONE as f64); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/type_repetition_in_bounds.rs b/tests/ui/type_repetition_in_bounds.rs index 3aa0d0da5..8b538be76 100644 --- a/tests/ui/type_repetition_in_bounds.rs +++ b/tests/ui/type_repetition_in_bounds.rs @@ -1,14 +1,19 @@ #[deny(clippy::type_repetition_in_bounds)] -pub fn foo(_t: T) where T: Copy, T: Clone { +pub fn foo(_t: T) +where + T: Copy, + T: Clone, +{ unimplemented!(); } -pub fn bar(_t: T, _u: U) where T: Copy, U: Clone { +pub fn bar(_t: T, _u: U) +where + T: Copy, + U: Clone, +{ unimplemented!(); } - -fn main() { - -} +fn main() {}