OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #3766 - xd009642:issue-3764, r=flip1995

Browse Source 
trait bounds lint - repeated types

This PR is to tackle https://github.com/rust-lang/rust-clippy/issues/3764 it's still a WIP and doesn't work but this is an initial stab. It builds though I haven't added any tests as I'm not sure where lint tests should go?

Unfortunately, it seems id isn't tied to the type itself but I guess where it is in the AST? Looking at https://manishearth.github.io/rust-internals-docs/syntax/ast/struct.Ty.html I can't see any members that would let me tell if a type was repeated in multiple trait bounds.

There may be other issues with how I've implemented this so any assistance is appreciated!

changelog: Add new lint: `type_repetition_in_bounds`
This commit is contained in:
bors 2019-07-30 12:11:17 +00:00
commit c3e913650e
10 changed files with 234 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
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
[`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

2
README.md
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.
[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:

4
clippy_lints/src/lib.rs
View File

@ -263,6 +263,7 @@ 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 trivially_copy_pass_by_ref;
@ -588,6 +589,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box checked_conversions::CheckedConversions);
reg.register_late_lint_pass(box integer_division::IntegerDivision);
reg.register_late_lint_pass(box inherent_to_string::InherentToString);
reg.register_late_lint_pass(box trait_bounds::TraitBounds);
reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
arithmetic::FLOAT_ARITHMETIC,
@ -858,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,
@ -1039,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,

77
clippy_lints/src/trait_bounds.rs Normal file
View File

@ -0,0 +1,77 @@
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;
#[derive(Copy, Clone)]
pub struct TraitBounds;
declare_clippy_lint! {
/// **What it does:** This lint warns about unnecessary type repetitions in trait bounds
///
/// **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: T) where T: Copy, T: Clone
/// ```
///
/// Could be written as:
///
/// ```rust
/// pub fn foo<T>(t: T) where T: Copy + Clone
/// ```
pub TYPE_REPETITION_IN_BOUNDS,
complexity,
"Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`"
}
impl_lint_pass!(TraitBounds => [TYPE_REPETITION_IN_BOUNDS]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds {
fn check_generics(&mut self, cx: &LateContext<'a, 'tcx>, gen: &'tcx Generics) {
if in_macro(gen.span) {
return;
}
let hash = |ty| -> u64 {
let mut hasher = SpanlessHash::new(cx, cx.tables);
hasher.hash_ty(ty);
hasher.finish()
};
let mut map = FxHashMap::default();
for bound in &gen.where_clause.predicates {
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::<Vec<_>>()) {
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));
}
}
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.truncate(hint_string.len() - 2);
hint_string.push('`');
span_help_and_lint(
cx,
TYPE_REPETITION_IN_BOUNDS,
p.span,
"this type has already been used as a bound predicate",
&hint_string,
);
}
}
}
}
}

105
clippy_lints/src/utils/hir_utils.rs
View File

@ -438,9 +438,9 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
self.hash_expr(fun);
self.hash_exprs(args);
},
ExprKind::Cast(ref e, ref _ty) | ExprKind::Type(ref e, ref _ty) => {
ExprKind::Cast(ref e, ref ty) | ExprKind::Type(ref e, ref ty) => {
self.hash_expr(e);
// TODO: _ty
self.hash_ty(ty);
},
ExprKind::Closure(cap, _, eid, _, _) => {
match cap {
@ -512,7 +512,10 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
self.hash_expr(e);
}
},
ExprKind::Tup(ref v) | ExprKind::Array(ref v) => {
ExprKind::Tup(ref tup) => {
self.hash_exprs(tup);
},
ExprKind::Array(ref v) => {
self.hash_exprs(v);
},
ExprKind::Unary(lop, ref le) => {
@ -574,4 +577,100 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
},
}
}
pub fn hash_lifetime(&mut self, lifetime: &Lifetime) {
std::mem::discriminant(&lifetime.name).hash(&mut self.s);
if let LifetimeName::Param(ref name) = lifetime.name {
std::mem::discriminant(name).hash(&mut self.s);
match name {
ParamName::Plain(ref ident) => {
ident.name.hash(&mut self.s);
},
ParamName::Fresh(ref size) => {
size.hash(&mut self.s);
},
ParamName::Error => {},
}
}
}
pub fn hash_ty(&mut self, ty: &Ty) {
self.hash_tykind(&ty.node);
}
pub fn hash_tykind(&mut self, ty: &TyKind) {
std::mem::discriminant(ty).hash(&mut self.s);
match ty {
TyKind::Slice(ty) => {
self.hash_ty(ty);
},
TyKind::Array(ty, anon_const) => {
self.hash_ty(ty);
self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value);
},
TyKind::Ptr(mut_ty) => {
self.hash_ty(&mut_ty.ty);
mut_ty.mutbl.hash(&mut self.s);
},
TyKind::Rptr(lifetime, mut_ty) => {
self.hash_lifetime(lifetime);
self.hash_ty(&mut_ty.ty);
mut_ty.mutbl.hash(&mut self.s);
},
TyKind::BareFn(bfn) => {
bfn.unsafety.hash(&mut self.s);
bfn.abi.hash(&mut self.s);
for arg in &bfn.decl.inputs {
self.hash_ty(&arg);
}
match bfn.decl.output {
FunctionRetTy::DefaultReturn(_) => {
().hash(&mut self.s);
},
FunctionRetTy::Return(ref ty) => {
self.hash_ty(ty);
},
}
bfn.decl.c_variadic.hash(&mut self.s);
},
TyKind::Tup(ty_list) => {
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) => {
self.hash_ty(ty);
segment.ident.name.hash(&mut self.s);
},
},
TyKind::Def(_, arg_list) => {
for arg in arg_list {
match arg {
GenericArg::Lifetime(ref l) => self.hash_lifetime(l),
GenericArg::Type(ref ty) => self.hash_ty(&ty),
GenericArg::Const(ref ca) => {
self.hash_expr(&self.cx.tcx.hir().body(ca.value.body).value);
},
}
}
},
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::Err | TyKind::Infer | TyKind::Never => {},
}
}
}

9
src/lintlist/mod.rs
View File

@ -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",

3
tests/ui/float_cmp.rs
View File

@ -8,8 +8,7 @@ const ONE: f32 = ZERO + 1.0;
fn twice<T>(x: T) -> T
where
T: Add<T, Output = T>,
T: Copy,
T: Add<T, Output = T> + Copy,
{
x + x
}

12
tests/ui/float_cmp.stderr
View File

@ -1,36 +1,36 @@
error: strict comparison of f32 or f64
--> $DIR/float_cmp.rs:60:5
--> $DIR/float_cmp.rs:59:5
|
LL | ONE as f64 != 2.0;
| ^^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(ONE as f64 - 2.0).abs() > error`
|
= note: `-D clippy::float-cmp` implied by `-D warnings`
note: std::f32::EPSILON and std::f64::EPSILON are available.
--> $DIR/float_cmp.rs:60:5
--> $DIR/float_cmp.rs:59:5
|
LL | ONE as f64 != 2.0;
| ^^^^^^^^^^^^^^^^^
error: strict comparison of f32 or f64
--> $DIR/float_cmp.rs:65:5
--> $DIR/float_cmp.rs:64:5
|
LL | x == 1.0;
| ^^^^^^^^ help: consider comparing them within some error: `(x - 1.0).abs() < error`
|
note: std::f32::EPSILON and std::f64::EPSILON are available.
--> $DIR/float_cmp.rs:65:5
--> $DIR/float_cmp.rs:64:5
|
LL | x == 1.0;
| ^^^^^^^^
error: strict comparison of f32 or f64
--> $DIR/float_cmp.rs:68:5
--> $DIR/float_cmp.rs:67:5
|
LL | twice(x) != twice(ONE as f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(twice(x) - twice(ONE as f64)).abs() > error`
|
note: std::f32::EPSILON and std::f64::EPSILON are available.
--> $DIR/float_cmp.rs:68:5
--> $DIR/float_cmp.rs:67:5
|
LL | twice(x) != twice(ONE as f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

19
tests/ui/type_repetition_in_bounds.rs Normal file
View File

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

15
tests/ui/type_repetition_in_bounds.stderr Normal file
View File

@ -0,0 +1,15 @@
error: this type has already been used as a bound predicate
--> $DIR/type_repetition_in_bounds.rs:6:5
|
LL | T: Clone,
| ^^^^^^^^
|
note: lint level defined here
--> $DIR/type_repetition_in_bounds.rs:1:8
|
LL | #[deny(clippy::type_repetition_in_bounds)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: consider combining the bounds: `T: Copy + Clone`
error: aborting due to previous error