Respond to review comments

Update README and CHANGELOG using the util scripts, refine the help message and fix the float_cmp error.
This commit is contained in:
xd009642 2019-07-27 21:58:29 +01:00
parent c962ddbd29
commit 925e8207fa
8 changed files with 57 additions and 36 deletions

View file

@ -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 [`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 [`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_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 [`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 [`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 [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg

View file

@ -6,7 +6,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. 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: We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

View file

@ -263,9 +263,9 @@ pub mod strings;
pub mod suspicious_trait_impl; pub mod suspicious_trait_impl;
pub mod swap; pub mod swap;
pub mod temporary_assignment; pub mod temporary_assignment;
pub mod trait_bounds;
pub mod transmute; pub mod transmute;
pub mod transmuting_null; pub mod transmuting_null;
pub mod trait_bounds;
pub mod trivially_copy_pass_by_ref; pub mod trivially_copy_pass_by_ref;
pub mod try_err; pub mod try_err;
pub mod types; pub mod types;
@ -860,6 +860,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
swap::ALMOST_SWAPPED, swap::ALMOST_SWAPPED,
swap::MANUAL_SWAP, swap::MANUAL_SWAP,
temporary_assignment::TEMPORARY_ASSIGNMENT, temporary_assignment::TEMPORARY_ASSIGNMENT,
trait_bounds::TYPE_REPETITION_IN_BOUNDS,
transmute::CROSSPOINTER_TRANSMUTE, transmute::CROSSPOINTER_TRANSMUTE,
transmute::TRANSMUTE_BYTES_TO_STR, transmute::TRANSMUTE_BYTES_TO_STR,
transmute::TRANSMUTE_INT_TO_BOOL, transmute::TRANSMUTE_INT_TO_BOOL,
@ -870,7 +871,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
transmute::USELESS_TRANSMUTE, transmute::USELESS_TRANSMUTE,
transmute::WRONG_TRANSMUTE, transmute::WRONG_TRANSMUTE,
transmuting_null::TRANSMUTING_NULL, transmuting_null::TRANSMUTING_NULL,
trait_bounds::TYPE_REPETITION_IN_BOUNDS,
trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF,
try_err::TRY_ERR, try_err::TRY_ERR,
types::ABSURD_EXTREME_COMPARISONS, types::ABSURD_EXTREME_COMPARISONS,
@ -1042,6 +1042,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reference::REF_IN_DEREF, reference::REF_IN_DEREF,
swap::MANUAL_SWAP, swap::MANUAL_SWAP,
temporary_assignment::TEMPORARY_ASSIGNMENT, temporary_assignment::TEMPORARY_ASSIGNMENT,
trait_bounds::TYPE_REPETITION_IN_BOUNDS,
transmute::CROSSPOINTER_TRANSMUTE, transmute::CROSSPOINTER_TRANSMUTE,
transmute::TRANSMUTE_BYTES_TO_STR, transmute::TRANSMUTE_BYTES_TO_STR,
transmute::TRANSMUTE_INT_TO_BOOL, transmute::TRANSMUTE_INT_TO_BOOL,

View file

@ -1,8 +1,8 @@
use crate::utils::{in_macro, snippet, span_help_and_lint, SpanlessHash}; use crate::utils::{in_macro, snippet, span_help_and_lint, SpanlessHash};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_tool_lint, impl_lint_pass}; use rustc::{declare_tool_lint, impl_lint_pass};
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
use rustc::hir::*;
#[derive(Copy, Clone)] #[derive(Copy, Clone)]
pub struct TraitBounds; pub struct TraitBounds;
@ -10,11 +10,12 @@ pub struct TraitBounds;
declare_clippy_lint! { declare_clippy_lint! {
/// **What it does:** This lint warns about unnecessary type repetitions in trait bounds /// **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:** /// **Example:**
/// ```rust /// ```rust
/// pub fn foo<T>(t: T) where T: Copy, T: Clone /// pub fn foo<T>(t: T) where T: Copy, T: Clone
/// ``` /// ```
/// ///
/// Could be written as: /// Could be written as:
@ -34,7 +35,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds {
if in_macro(gen.span) { if in_macro(gen.span) {
return; return;
} }
let hash = | ty | -> u64 { let hash = |ty| -> u64 {
let mut hasher = SpanlessHash::new(cx, cx.tables); let mut hasher = SpanlessHash::new(cx, cx.tables);
hasher.hash_ty(ty); hasher.hash_ty(ty);
hasher.finish() hasher.finish()
@ -44,17 +45,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds {
if let WherePredicate::BoundPredicate(ref p) = bound { if let WherePredicate::BoundPredicate(ref p) = bound {
let h = hash(&p.bounded_ty); let h = hash(&p.bounded_ty);
if let Some(ref v) = map.insert(h, p.bounds.iter().collect::<Vec<_>>()) { if let Some(ref v) = map.insert(h, p.bounds.iter().collect::<Vec<_>>()) {
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() { for b in v.iter() {
if let GenericBound::Trait(ref poly_trait_ref, _) = b { if let GenericBound::Trait(ref poly_trait_ref, _) = b {
let path = &poly_trait_ref.trait_ref.path; 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() { for b in p.bounds.iter() {
if let GenericBound::Trait(ref poly_trait_ref, _) = b { if let GenericBound::Trait(ref poly_trait_ref, _) = b {
let path = &poly_trait_ref.trait_ref.path; 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); hint_string.truncate(hint_string.len() - 2);

View file

@ -639,23 +639,20 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
for ty in ty_list { for ty in ty_list {
self.hash_ty(ty); self.hash_ty(ty);
} }
}, },
TyKind::Path(qpath) => { TyKind::Path(qpath) => match qpath {
match qpath { QPath::Resolved(ref maybe_ty, ref path) => {
QPath::Resolved(ref maybe_ty, ref path) => { if let Some(ref ty) = maybe_ty {
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); self.hash_ty(ty);
}
for segment in &path.segments {
segment.ident.name.hash(&mut self.s); 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) => { TyKind::Def(_, arg_list) => {
for arg in arg_list { for arg in arg_list {
@ -670,14 +667,11 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
}, },
TyKind::TraitObject(_, lifetime) => { TyKind::TraitObject(_, lifetime) => {
self.hash_lifetime(lifetime); self.hash_lifetime(lifetime);
}, },
TyKind::Typeof(anon_const) => { TyKind::Typeof(anon_const) => {
self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value);
}, },
TyKind::CVarArgs(lifetime) => { TyKind::CVarArgs(lifetime) => self.hash_lifetime(lifetime),
self.hash_lifetime(lifetime)
},
TyKind::Err | TyKind::Infer | TyKind::Never => {}, TyKind::Err | TyKind::Infer | TyKind::Never => {},
} }
} }

View file

@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS; pub use lint::LINT_LEVELS;
// begin lint list, do not remove this comment, its used in `update_lints` // begin lint list, do not remove this comment, its used in `update_lints`
pub const ALL_LINTS: [Lint; 308] = [ pub const ALL_LINTS: [Lint; 309] = [
Lint { Lint {
name: "absurd_extreme_comparisons", name: "absurd_extreme_comparisons",
group: "correctness", group: "correctness",
@ -1848,6 +1848,13 @@ pub const ALL_LINTS: [Lint; 308] = [
deprecation: None, deprecation: None,
module: "types", 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 { Lint {
name: "unicode_not_nfc", name: "unicode_not_nfc",
group: "pedantic", group: "pedantic",

View file

@ -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<T, Output = T>, Copy`
error: strict comparison of f32 or f64 error: strict comparison of f32 or f64
--> $DIR/float_cmp.rs:60:5 --> $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); LL | twice(x) != twice(ONE as f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 3 previous errors error: aborting due to 4 previous errors

View file

@ -1,14 +1,19 @@
#[deny(clippy::type_repetition_in_bounds)] #[deny(clippy::type_repetition_in_bounds)]
pub fn foo<T>(_t: T) where T: Copy, T: Clone { pub fn foo<T>(_t: T)
where
T: Copy,
T: Clone,
{
unimplemented!(); unimplemented!();
} }
pub fn bar<T, U>(_t: T, _u: U) where T: Copy, U: Clone { pub fn bar<T, U>(_t: T, _u: U)
where
T: Copy,
U: Clone,
{
unimplemented!(); unimplemented!();
} }
fn main() {}
fn main() {
}