From f71d59e6a63d8624a121d29230a83ae39c856c6c Mon Sep 17 00:00:00 2001 From: xd009642 Date: Fri, 15 Feb 2019 20:21:13 +0000 Subject: [PATCH 01/17] Lint for type repetition in trait bounds. This lint adds warning if types are redundantly repeated in trait bounds i.e. `T: Copy, T: Clone` instead of `T: Copy + Clone`. This is a late pass trait lint and has necessitated the addition of code to allow hashing of TyKinds without taking into account Span information. --- clippy_lints/src/lib.rs | 3 + clippy_lints/src/trait_bounds.rs | 63 ++++++++++++ clippy_lints/src/utils/hir_utils.rs | 119 +++++++++++++++++++++- tests/ui/type_repetition_in_bounds.rs | 14 +++ tests/ui/type_repetition_in_bounds.stderr | 15 +++ 5 files changed, 210 insertions(+), 4 deletions(-) create mode 100644 clippy_lints/src/trait_bounds.rs create mode 100644 tests/ui/type_repetition_in_bounds.rs create mode 100644 tests/ui/type_repetition_in_bounds.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6d90d315f..faffb1a67 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -265,6 +265,7 @@ pub mod swap; pub mod temporary_assignment; 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; @@ -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, @@ -868,6 +870,7 @@ 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, diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs new file mode 100644 index 000000000..1f4c68507 --- /dev/null +++ b/clippy_lints/src/trait_bounds.rs @@ -0,0 +1,63 @@ +use crate::utils::{in_macro, span_help_and_lint, SpanlessHash}; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::{declare_tool_lint, lint_array}; +use rustc_data_structures::fx::FxHashMap; +use rustc::hir::*; + +declare_clippy_lint! { + pub TYPE_REPETITION_IN_BOUNDS, + complexity, + "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`" +} + +#[derive(Copy, Clone)] +pub struct TraitBounds; + +impl LintPass for TraitBounds { + fn get_lints(&self) -> LintArray { + lint_array!(TYPE_REPETITION_IN_BOUNDS) + } + + fn name(&self) -> &'static str { + "TypeRepetitionInBounds" + } +} + + + +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) { + let mut hint_string = format!("consider combining the bounds: `{:?}: ", p.bounded_ty); + for &b in v.iter() { + hint_string.push_str(&format!("{:?}, ", b)); + } + for &b in p.bounds.iter() { + hint_string.push_str(&format!("{:?}, ", b)); + } + 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, + ); + } + } + } + } +} diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index e9b5cee43..929c15a6e 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -3,7 +3,7 @@ use crate::utils::differing_macro_contexts; use rustc::hir::ptr::P; use rustc::hir::*; use rustc::lint::LateContext; -use rustc::ty::TypeckTables; +use rustc::ty::{Ty, TypeckTables}; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; use syntax::ast::Name; @@ -438,9 +438,11 @@ 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) => { + let c: fn(_, _) -> _ = ExprKind::Cast; + c.hash(&mut self.s); self.hash_expr(e); - // TODO: _ty + self.hash_ty(ty); }, ExprKind::Closure(cap, _, eid, _, _) => { match cap { @@ -512,9 +514,22 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(e); } }, - ExprKind::Tup(ref v) | ExprKind::Array(ref v) => { + ExprKind::Tup(ref tup) => { + let c: fn(_) -> _ = ExprKind::Tup; + c.hash(&mut self.s); + self.hash_exprs(tup); + }, + ExprKind::Array(ref v) => { + let c: fn(_) -> _ = ExprKind::Array; + c.hash(&mut self.s); self.hash_exprs(v); }, + ExprKind::Type(ref e, ref ty) => { + let c: fn(_, _) -> _ = ExprKind::Type; + c.hash(&mut self.s); + self.hash_expr(e); + self.hash_ty(ty); + }, ExprKind::Unary(lop, ref le) => { lop.hash(&mut self.s); self.hash_expr(le); @@ -574,4 +589,100 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { }, } } + + pub fn hash_lifetime(&mut self, lifetime: &Lifetime) { + if let LifetimeName::Param(ref name) = lifetime.name { + match name { + ParamName::Plain(ref ident) => { + ident.name.hash(&mut self.s); + }, + ParamName::Fresh(ref size) => { + size.hash(&mut self.s); + }, + _ => {}, + } + } + } + + pub fn hash_ty(&mut self, ty: &Ty) { + std::mem::discriminant(&ty.node).hash(&mut self.s); + match ty.node { + Ty::Slice(ty) => { + self.hash_ty(ty); + }, + Ty::Array(ty, anon_const) => { + self.hash_ty(ty); + self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); + }, + Ty::Ptr(mut_ty) => { + self.hash_ty(&mut_ty.ty); + mut_ty.mutbl.hash(&mut self.s); + }, + Ty::Rptr(lifetime, mut_ty) => { + self.hash_lifetime(lifetime); + self.hash_ty(&mut_ty.ty); + mut_ty.mutbl.hash(&mut self.s); + }, + Ty::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); + }, + Ty::Tup(ty_list) => { + for ty in ty_list { + self.hash_ty(ty); + } + + }, + Ty::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); + }, + } + }, + Ty::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); + }, + } + } + }, + Ty::TraitObject(_, lifetime) => { + self.hash_lifetime(lifetime); + + }, + Ty::Typeof(anon_const) => { + self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); + }, + Ty::CVarArgs(lifetime) => { + self.hash_lifetime(lifetime); + }, + Ty::Err | Ty::Infer | Ty::Never => {}, + } + } } diff --git a/tests/ui/type_repetition_in_bounds.rs b/tests/ui/type_repetition_in_bounds.rs new file mode 100644 index 000000000..3aa0d0da5 --- /dev/null +++ b/tests/ui/type_repetition_in_bounds.rs @@ -0,0 +1,14 @@ +#[deny(clippy::type_repetition_in_bounds)] + +pub fn foo(_t: T) where T: Copy, T: Clone { + unimplemented!(); +} + +pub fn bar(_t: T, _u: U) where T: Copy, U: Clone { + unimplemented!(); +} + + +fn main() { + +} diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr new file mode 100644 index 000000000..1b49c6953 --- /dev/null +++ b/tests/ui/type_repetition_in_bounds.stderr @@ -0,0 +1,15 @@ +error: this type has already been used as a bound predicate + --> $DIR/type_repetition_in_bounds.rs:3:37 + | +LL | pub fn foo(_t: T) where T: Copy, 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 + From f3e4467c109364c557bfd998dabfeab6a045c605 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 22 Jun 2019 22:22:11 +0100 Subject: [PATCH 02/17] Changed Ty to ty, added lifetime 'tcx --- clippy_lints/src/utils/hir_utils.rs | 34 ++++++++++++++--------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 929c15a6e..276d984f8 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -3,7 +3,7 @@ use crate::utils::differing_macro_contexts; use rustc::hir::ptr::P; use rustc::hir::*; use rustc::lint::LateContext; -use rustc::ty::{Ty, TypeckTables}; +use rustc::ty::{self, Ty, TypeckTables}; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; use syntax::ast::Name; @@ -45,7 +45,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { match (&left.node, &right.node) { (&StmtKind::Local(ref l), &StmtKind::Local(ref r)) => { self.eq_pat(&l.pat, &r.pat) - && both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) + && both(&l.ty, &r.ty, |l, r| self.eq_ty(*l, *r)) && both(&l.init, &r.init, |l, r| self.eq_expr(l, r)) }, (&StmtKind::Expr(ref l), &StmtKind::Expr(ref r)) | (&StmtKind::Semi(ref l), &StmtKind::Semi(ref r)) => { @@ -257,7 +257,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } } - pub fn eq_ty(&mut self, left: &Ty, right: &Ty) -> bool { + pub fn eq_ty(&mut self, left: &Ty<'tcx>, right: &Ty<'tcx>) -> bool { self.eq_ty_kind(&left.node, &right.node) } @@ -604,26 +604,26 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } - pub fn hash_ty(&mut self, ty: &Ty) { + pub fn hash_ty(&mut self, ty: &Ty<'tcx>) { std::mem::discriminant(&ty.node).hash(&mut self.s); - match ty.node { - Ty::Slice(ty) => { + match ty.sty { + ty::Slice(ty) => { self.hash_ty(ty); }, - Ty::Array(ty, anon_const) => { + ty::Array(ty, anon_const) => { self.hash_ty(ty); self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); }, - Ty::Ptr(mut_ty) => { + ty::Ptr(mut_ty) => { self.hash_ty(&mut_ty.ty); mut_ty.mutbl.hash(&mut self.s); }, - Ty::Rptr(lifetime, mut_ty) => { + ty::Rptr(lifetime, mut_ty) => { self.hash_lifetime(lifetime); self.hash_ty(&mut_ty.ty); mut_ty.mutbl.hash(&mut self.s); }, - Ty::BareFn(bfn) => { + ty::BareFn(bfn) => { bfn.unsafety.hash(&mut self.s); bfn.abi.hash(&mut self.s); for arg in &bfn.decl.inputs { @@ -639,13 +639,13 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } bfn.decl.c_variadic.hash(&mut self.s); }, - Ty::Tup(ty_list) => { + ty::Tup(ty_list) => { for ty in ty_list { self.hash_ty(ty); } }, - Ty::Path(qpath) => { + ty::Path(qpath) => { match qpath { QPath::Resolved(ref maybe_ty, ref path) => { if let Some(ref ty) = maybe_ty { @@ -661,7 +661,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { }, } }, - Ty::Def(_, arg_list) => { + ty::Def(_, arg_list) => { for arg in arg_list { match arg { GenericArg::Lifetime(ref l) => self.hash_lifetime(l), @@ -672,17 +672,17 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } }, - Ty::TraitObject(_, lifetime) => { + ty::TraitObject(_, lifetime) => { self.hash_lifetime(lifetime); }, - Ty::Typeof(anon_const) => { + ty::Typeof(anon_const) => { self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); }, - Ty::CVarArgs(lifetime) => { + ty::CVarArgs(lifetime) => { self.hash_lifetime(lifetime); }, - Ty::Err | Ty::Infer | Ty::Never => {}, + ty::Err | ty::Infer | ty::Never => {}, } } } From 792153104c5af9948df6d8def8ab4b2611731eb6 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Wed, 24 Jul 2019 22:27:12 +0100 Subject: [PATCH 03/17] Fix some of the compile errors --- clippy_lints/src/trait_bounds.rs | 22 ++++++-------------- clippy_lints/src/utils/hir_utils.rs | 32 ++++++++++++++--------------- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 1f4c68507..e41437bf1 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,29 +1,19 @@ use crate::utils::{in_macro, span_help_and_lint, SpanlessHash}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use rustc::{declare_tool_lint, lint_array}; +use rustc::{declare_tool_lint, lint_array, impl_lint_pass}; use rustc_data_structures::fx::FxHashMap; use rustc::hir::*; +#[derive(Copy, Clone)] +pub struct TraitBounds; + declare_clippy_lint! { pub TYPE_REPETITION_IN_BOUNDS, complexity, "Types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`" } -#[derive(Copy, Clone)] -pub struct TraitBounds; - -impl LintPass for TraitBounds { - fn get_lints(&self) -> LintArray { - lint_array!(TYPE_REPETITION_IN_BOUNDS) - } - - fn name(&self) -> &'static str { - "TypeRepetitionInBounds" - } -} - - +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) { @@ -38,7 +28,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { 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); + let h = hash(&p.bounded_ty.node); if let Some(ref v) = map.insert(h, p.bounds) { let mut hint_string = format!("consider combining the bounds: `{:?}: ", p.bounded_ty); for &b in v.iter() { diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 276d984f8..5304a9226 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -258,7 +258,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } pub fn eq_ty(&mut self, left: &Ty<'tcx>, right: &Ty<'tcx>) -> bool { - self.eq_ty_kind(&left.node, &right.node) + self.eq_ty_kind(&left.sty, &right.sty) } #[allow(clippy::similar_names)] @@ -604,26 +604,26 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } - pub fn hash_ty(&mut self, ty: &Ty<'tcx>) { + pub fn hash_ty(&mut self, ty: &TyKind) { std::mem::discriminant(&ty.node).hash(&mut self.s); - match ty.sty { - ty::Slice(ty) => { + match ty { + TyKind::Slice(ty) => { self.hash_ty(ty); }, - ty::Array(ty, anon_const) => { + TyKind::Array(ty, anon_const) => { self.hash_ty(ty); self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); }, - ty::Ptr(mut_ty) => { + TyKind::Ptr(mut_ty) => { self.hash_ty(&mut_ty.ty); mut_ty.mutbl.hash(&mut self.s); }, - ty::Rptr(lifetime, mut_ty) => { + TyKind::Rptr(lifetime, mut_ty) => { self.hash_lifetime(lifetime); self.hash_ty(&mut_ty.ty); mut_ty.mutbl.hash(&mut self.s); }, - ty::BareFn(bfn) => { + TyKind::BareFn(bfn) => { bfn.unsafety.hash(&mut self.s); bfn.abi.hash(&mut self.s); for arg in &bfn.decl.inputs { @@ -639,13 +639,13 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } bfn.decl.c_variadic.hash(&mut self.s); }, - ty::Tup(ty_list) => { + TyKind::Tup(ty_list) => { for ty in ty_list { self.hash_ty(ty); } }, - ty::Path(qpath) => { + TyKind::Path(qpath) => { match qpath { QPath::Resolved(ref maybe_ty, ref path) => { if let Some(ref ty) = maybe_ty { @@ -661,7 +661,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { }, } }, - ty::Def(_, arg_list) => { + TyKind::Def(_, arg_list) => { for arg in arg_list { match arg { GenericArg::Lifetime(ref l) => self.hash_lifetime(l), @@ -672,17 +672,17 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } }, - ty::TraitObject(_, lifetime) => { + TyKind::TraitObject(_, lifetime) => { self.hash_lifetime(lifetime); }, - ty::Typeof(anon_const) => { + TyKind::Typeof(anon_const) => { self.hash_expr(&self.cx.tcx.hir().body(anon_const.body).value); }, - ty::CVarArgs(lifetime) => { - self.hash_lifetime(lifetime); + TyKind::CVarArgs(lifetime) => { + self.hash_lifetime(lifetime) }, - ty::Err | ty::Infer | ty::Never => {}, + TyKind::Err | TyKind::Infer | TyKind::Never => {}, } } } From c0259179c3c867e9b25894bf8139145d34359acc Mon Sep 17 00:00:00 2001 From: xd009642 Date: Wed, 24 Jul 2019 22:59:32 +0100 Subject: [PATCH 04/17] Fixed more compile errors Moved to rustc::hir::Ty --- clippy_lints/src/trait_bounds.rs | 6 +++--- clippy_lints/src/utils/hir_utils.rs | 18 +++++++++++------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index e41437bf1..02514377d 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -28,13 +28,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { 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.node); + let h = hash(&p.bounded_ty); if let Some(ref v) = map.insert(h, p.bounds) { let mut hint_string = format!("consider combining the bounds: `{:?}: ", p.bounded_ty); - for &b in v.iter() { + for b in v.iter() { hint_string.push_str(&format!("{:?}, ", b)); } - for &b in p.bounds.iter() { + for b in p.bounds.iter() { hint_string.push_str(&format!("{:?}, ", b)); } 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 5304a9226..4330b5587 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -3,7 +3,7 @@ use crate::utils::differing_macro_contexts; use rustc::hir::ptr::P; use rustc::hir::*; use rustc::lint::LateContext; -use rustc::ty::{self, Ty, TypeckTables}; +use rustc::ty::{self, TypeckTables}; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; use syntax::ast::Name; @@ -45,7 +45,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { match (&left.node, &right.node) { (&StmtKind::Local(ref l), &StmtKind::Local(ref r)) => { self.eq_pat(&l.pat, &r.pat) - && both(&l.ty, &r.ty, |l, r| self.eq_ty(*l, *r)) + && both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) && both(&l.init, &r.init, |l, r| self.eq_expr(l, r)) }, (&StmtKind::Expr(ref l), &StmtKind::Expr(ref r)) | (&StmtKind::Semi(ref l), &StmtKind::Semi(ref r)) => { @@ -257,8 +257,8 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } } - pub fn eq_ty(&mut self, left: &Ty<'tcx>, right: &Ty<'tcx>) -> bool { - self.eq_ty_kind(&left.sty, &right.sty) + pub fn eq_ty(&mut self, left: &Ty, right: &Ty) -> bool { + self.eq_ty_kind(&left.node, &right.node) } #[allow(clippy::similar_names)] @@ -604,8 +604,12 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } - pub fn hash_ty(&mut self, ty: &TyKind) { - std::mem::discriminant(&ty.node).hash(&mut self.s); + 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); @@ -665,7 +669,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { for arg in arg_list { match arg { GenericArg::Lifetime(ref l) => self.hash_lifetime(l), - GenericArg::Type(ref ty) => self.hash_ty(ty), + GenericArg::Type(ref ty) => self.hash_ty(&ty), GenericArg::Const(ref ca) => { self.hash_expr(&self.cx.tcx.hir().body(ca.value.body).value); }, From 7853dac66245d88bace4cdc62b38837aa3e54568 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Fri, 26 Jul 2019 16:46:47 +0100 Subject: [PATCH 05/17] Responded to comments and fixed compile bug Removed the hash of `let c: fn(_,_) -> _ = ExprKind::Cast` and fixed compile issue by collecting HirVec into an actual Vec --- clippy_lints/src/trait_bounds.rs | 4 ++-- clippy_lints/src/utils/hir_utils.rs | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 02514377d..cb836eac3 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,6 +1,6 @@ use crate::utils::{in_macro, span_help_and_lint, SpanlessHash}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use rustc::{declare_tool_lint, lint_array, impl_lint_pass}; +use rustc::{declare_tool_lint, impl_lint_pass}; use rustc_data_structures::fx::FxHashMap; use rustc::hir::*; @@ -29,7 +29,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { 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) { + if let Some(ref v) = map.insert(h, p.bounds.iter().collect::>()) { let mut hint_string = format!("consider combining the bounds: `{:?}: ", p.bounded_ty); for b in v.iter() { hint_string.push_str(&format!("{:?}, ", b)); diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 4330b5587..703c9fac1 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -3,7 +3,7 @@ use crate::utils::differing_macro_contexts; use rustc::hir::ptr::P; use rustc::hir::*; use rustc::lint::LateContext; -use rustc::ty::{self, TypeckTables}; +use rustc::ty::TypeckTables; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; use syntax::ast::Name; @@ -439,8 +439,6 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_exprs(args); }, ExprKind::Cast(ref e, ref ty) => { - let c: fn(_, _) -> _ = ExprKind::Cast; - c.hash(&mut self.s); self.hash_expr(e); self.hash_ty(ty); }, From cac69ec063e9fcb26e0f3102c82b9cb2d56350fe Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 11:51:27 +0100 Subject: [PATCH 06/17] Respond to comments and improve printout Now get the trait names for the diagnostic message and removed more `let c: fn(_) -> _ = T; hashes from hir_utils --- clippy_lints/src/trait_bounds.rs | 10 ++++++++-- clippy_lints/src/utils/hir_utils.rs | 6 ------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index cb836eac3..3ef353a97 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -32,10 +32,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TraitBounds { if let Some(ref v) = map.insert(h, p.bounds.iter().collect::>()) { let mut hint_string = format!("consider combining the bounds: `{:?}: ", p.bounded_ty); for b in v.iter() { - hint_string.push_str(&format!("{:?}, ", b)); + 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() { - hint_string.push_str(&format!("{:?}, ", b)); + 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('`'); diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 703c9fac1..449b8c439 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -513,18 +513,12 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } }, ExprKind::Tup(ref tup) => { - let c: fn(_) -> _ = ExprKind::Tup; - c.hash(&mut self.s); self.hash_exprs(tup); }, ExprKind::Array(ref v) => { - let c: fn(_) -> _ = ExprKind::Array; - c.hash(&mut self.s); self.hash_exprs(v); }, ExprKind::Type(ref e, ref ty) => { - let c: fn(_, _) -> _ = ExprKind::Type; - c.hash(&mut self.s); self.hash_expr(e); self.hash_ty(ty); }, From 5e9906b2c64c07e95252e91a4ce1534cfa4cf10b Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 12:06:25 +0100 Subject: [PATCH 07/17] Added doc comment fixed type printout Added a doc comment for the lint and fixed the printout of the type so it prints T not type(T) --- clippy_lints/src/trait_bounds.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index 3ef353a97..f5ca3793d 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -1,4 +1,4 @@ -use crate::utils::{in_macro, span_help_and_lint, SpanlessHash}; +use crate::utils::{in_macro, snippet, span_help_and_lint, SpanlessHash}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_tool_lint, impl_lint_pass}; use rustc_data_structures::fx::FxHashMap; @@ -8,6 +8,20 @@ use rustc::hir::*; pub struct TraitBounds; declare_clippy_lint! { + /// **What it does:** This lint warns about unnecessary type repetitions in trait bounds + /// + /// **Why is this bad?** Complexity + /// + /// **Example:** + /// ```rust + /// pub fn foo(t: T) where T: Copy, T: Clone + /// ``` + /// + /// Could be written as: + /// + /// ```rust + /// pub fn foo(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: _`" @@ -30,7 +44,7 @@ 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: `{:?}: ", p.bounded_ty); + 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; From c962ddbd29ed5b374c55a0fba43cf3d0af9ad680 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 12:07:02 +0100 Subject: [PATCH 08/17] Updated test stderr --- tests/ui/type_repetition_in_bounds.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr index 1b49c6953..6f5c67afb 100644 --- a/tests/ui/type_repetition_in_bounds.stderr +++ b/tests/ui/type_repetition_in_bounds.stderr @@ -9,7 +9,7 @@ note: lint level defined here | LL | #[deny(clippy::type_repetition_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: consider combining the bounds: `T: Copy + Clone` + = help: consider combining the bounds: `T: Copy, Clone` error: aborting due to previous error From 925e8207fa8ff6cc4aea7337b6cb3eeb318123d3 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 21:58:29 +0100 Subject: [PATCH 09/17] 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() {} From bba2c7f02c3b49ce3f4d5f83f66b52dca5bd310b Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 22:53:26 +0100 Subject: [PATCH 10/17] Updated tests. Removed unnecessary type repetition in float test and regenerated stderr Regenerated type_repetition stderr --- tests/ui/float_cmp.rs | 3 +-- tests/ui/float_cmp.stderr | 23 +++++++---------------- tests/ui/type_repetition_in_bounds.stderr | 8 ++++---- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/tests/ui/float_cmp.rs b/tests/ui/float_cmp.rs index 1ec8af3e3..207c1bcbb 100644 --- a/tests/ui/float_cmp.rs +++ b/tests/ui/float_cmp.rs @@ -8,8 +8,7 @@ const ONE: f32 = ZERO + 1.0; fn twice(x: T) -> T where - T: Add, - T: Copy, + T: Add + Copy, { x + x } diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index d1ffc0d15..116e3e90e 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -1,48 +1,39 @@ -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 + --> $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); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors diff --git a/tests/ui/type_repetition_in_bounds.stderr b/tests/ui/type_repetition_in_bounds.stderr index 6f5c67afb..a72f512b0 100644 --- a/tests/ui/type_repetition_in_bounds.stderr +++ b/tests/ui/type_repetition_in_bounds.stderr @@ -1,15 +1,15 @@ error: this type has already been used as a bound predicate - --> $DIR/type_repetition_in_bounds.rs:3:37 + --> $DIR/type_repetition_in_bounds.rs:6:5 | -LL | pub fn foo(_t: T) where T: Copy, T: Clone { - | ^^^^^^^^ +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` + = help: consider combining the bounds: `T: Copy + Clone` error: aborting due to previous error From ad637193f0e5b90db1c934f3c637679514f67777 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 22:59:46 +0100 Subject: [PATCH 11/17] Hash discriminant of Lifetime::Name --- clippy_lints/src/utils/hir_utils.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index db7b10c6b..64f5f3525 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -584,6 +584,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { pub fn hash_lifetime(&mut self, lifetime: &Lifetime) { 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); @@ -591,7 +592,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { ParamName::Fresh(ref size) => { size.hash(&mut self.s); }, - _ => {}, + ParamName::Error => {}, } } } From 03c543515ad3c963f801c65da8f8bb6a4e0a35da Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sat, 27 Jul 2019 23:04:36 +0100 Subject: [PATCH 12/17] Hash discriminant of lifetime.name --- clippy_lints/src/utils/hir_utils.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 64f5f3525..e3da8c494 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -583,6 +583,7 @@ 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 { From 78ebcaa5263e0fd4c4ec89da648d8386c034ce84 Mon Sep 17 00:00:00 2001 From: xd009642 Date: Sun, 28 Jul 2019 09:31:05 +0100 Subject: [PATCH 13/17] Fix dogfood test --- clippy_lints/src/utils/hir_utils.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index e3da8c494..6fc5939a2 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -438,7 +438,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(fun); self.hash_exprs(args); }, - ExprKind::Cast(ref e, ref ty) => { + ExprKind::Cast(ref e, ref ty) | ExprKind::Type(ref e, ref ty) => { self.hash_expr(e); self.hash_ty(ty); }, @@ -518,10 +518,6 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { ExprKind::Array(ref v) => { self.hash_exprs(v); }, - ExprKind::Type(ref e, ref ty) => { - self.hash_expr(e); - self.hash_ty(ty); - }, ExprKind::Unary(lop, ref le) => { lop.hash(&mut self.s); self.hash_expr(le); @@ -585,7 +581,7 @@ 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); + std::mem::discriminant(name).hash(&mut self.s); match name { ParamName::Plain(ref ident) => { ident.name.hash(&mut self.s); @@ -603,7 +599,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } pub fn hash_tykind(&mut self, ty: &TyKind) { - std::mem::discriminant(&ty).hash(&mut self.s); + std::mem::discriminant(ty).hash(&mut self.s); match ty { TyKind::Slice(ty) => { self.hash_ty(ty); From 2f5c1d031d881c29d18ee94ef575efed93e7e528 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Tue, 30 Jul 2019 06:37:49 +0200 Subject: [PATCH 14/17] Fix breakage due to rust-lang/rust#61856 --- clippy_lints/src/escape.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index cc5f05ede..f276014f4 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -1,5 +1,5 @@ use rustc::hir::intravisit as visit; -use rustc::hir::*; +use rustc::hir::{self, *}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::middle::expr_use_visitor::*; use rustc::middle::mem_categorization::{cmt_, Categorization}; @@ -101,6 +101,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxedLocal { } } +// TODO: Replace with Map::is_argument(..) when it's fixed +fn is_argument(map: &hir::map::Map<'_>, id: HirId) -> bool { + match map.find(id) { + Some(Node::Binding(_)) => (), + _ => return false, + } + + match map.find(map.get_parent_node(id)) { + Some(Node::Arg(_)) => true, + _ => false, + } +} + impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { fn consume(&mut self, _: HirId, _: Span, cmt: &cmt_<'tcx>, mode: ConsumeMode) { if let Categorization::Local(lid) = cmt.cat { @@ -113,11 +126,13 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { fn matched_pat(&mut self, _: &Pat, _: &cmt_<'tcx>, _: MatchMode) {} fn consume_pat(&mut self, consume_pat: &Pat, cmt: &cmt_<'tcx>, _: ConsumeMode) { let map = &self.cx.tcx.hir(); - if map.is_argument(consume_pat.hir_id) { + if is_argument(map, consume_pat.hir_id) { // Skip closure arguments - if let Some(Node::Expr(..)) = map.find(map.get_parent_node(consume_pat.hir_id)) { + let parent_id = map.get_parent_node(consume_pat.hir_id); + if let Some(Node::Expr(..)) = map.find(map.get_parent_node(parent_id)) { return; } + if is_non_trait_box(cmt.ty) && !self.is_large_box(cmt.ty) { self.set.insert(consume_pat.hir_id); } From 41110b0039aca086216f675e148effc68b15bf1d Mon Sep 17 00:00:00 2001 From: Jakub Wieczorek Date: Wed, 31 Jul 2019 00:24:28 +0000 Subject: [PATCH 15/17] Extend the `use_self` lint to suggest uses of `Self::Variant`. --- clippy_lints/src/use_self.rs | 62 +++++++++++++++++++++++------------- tests/ui/use_self.fixed | 8 +++++ tests/ui/use_self.rs | 8 +++++ tests/ui/use_self.stderr | 26 ++++++++++++--- 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index b60b9ccab..3f93e019c 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -51,9 +51,11 @@ declare_lint_pass!(UseSelf => [USE_SELF]); const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; -fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path) { +fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path, last_segment: Option<&PathSegment>) { + let last_segment = last_segment.unwrap_or_else(|| path.segments.last().expect(SEGMENTS_MSG)); + // Path segments only include actual path, no methods or fields. - let last_path_span = path.segments.last().expect(SEGMENTS_MSG).ident.span; + let last_path_span = last_segment.ident.span; // Only take path up to the end of last_path_span. let span = path.span.with_hi(last_path_span.hi()); @@ -80,22 +82,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> { let trait_ty = self.trait_type_walker.next(); let impl_ty = self.impl_type_walker.next(); - if let TyKind::Path(QPath::Resolved(_, path)) = &t.node { + if_chain! { + if let TyKind::Path(QPath::Resolved(_, path)) = &t.node; + // The implementation and trait types don't match which means that // the concrete type was specified by the implementation - if impl_ty != trait_ty { - if let Some(impl_ty) = impl_ty { - if self.item_type == impl_ty { - let is_self_ty = if let def::Res::SelfTy(..) = path.res { - true - } else { - false - }; - - if !is_self_ty { - span_use_self_lint(self.cx, path); - } - } + if impl_ty != trait_ty; + if let Some(impl_ty) = impl_ty; + if self.item_type == impl_ty; + then { + match path.res { + def::Res::SelfTy(..) => {}, + _ => span_use_self_lint(self.cx, path, None) } } } @@ -220,15 +218,35 @@ struct UseSelfVisitor<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { fn visit_path(&mut self, path: &'tcx Path, _id: HirId) { - if path.segments.last().expect(SEGMENTS_MSG).ident.name != kw::SelfUpper { - if self.item_path.res == path.res { - span_use_self_lint(self.cx, path); - } else if let Res::Def(DefKind::Ctor(def::CtorOf::Struct, CtorKind::Fn), ctor_did) = path.res { - if self.item_path.res.opt_def_id() == self.cx.tcx.parent(ctor_did) { - span_use_self_lint(self.cx, path); + if path.segments.len() >= 2 { + let last_but_one = &path.segments[path.segments.len() - 2]; + if last_but_one.ident.name != kw::SelfUpper { + let enum_def_id = match path.res { + Res::Def(DefKind::Variant, variant_def_id) => + self.cx.tcx.parent(variant_def_id), + Res::Def(DefKind::Ctor(def::CtorOf::Variant, _), ctor_def_id) => { + let variant_def_id = self.cx.tcx.parent(ctor_def_id); + variant_def_id.and_then(|def_id| self.cx.tcx.parent(def_id)) + } + _ => None + }; + + if self.item_path.res.opt_def_id() == enum_def_id { + span_use_self_lint(self.cx, path, Some(last_but_one)); } } } + + if path.segments.last().expect(SEGMENTS_MSG).ident.name != kw::SelfUpper { + if self.item_path.res == path.res { + span_use_self_lint(self.cx, path, None); + } else if let Res::Def(DefKind::Ctor(def::CtorOf::Struct, CtorKind::Fn), ctor_def_id) = path.res { + if self.item_path.res.opt_def_id() == self.cx.tcx.parent(ctor_def_id) { + span_use_self_lint(self.cx, path, None); + } + } + } + walk_path(self, path); } diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed index 68af85030..ac2a1708b 100644 --- a/tests/ui/use_self.fixed +++ b/tests/ui/use_self.fixed @@ -265,6 +265,8 @@ mod nesting { enum Enum { A, + B(u64), + C { field: bool } } impl Enum { fn method() { @@ -272,6 +274,12 @@ mod nesting { use self::Enum::*; // Issue 3425 static STATIC: Enum = Enum::A; // Can't use Self as type } + + fn method2() { + let _ = Self::B(42); + let _ = Self::C { field: true }; + let _ = Self::A; + } } } diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 7a6d41552..21b5833e5 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -265,6 +265,8 @@ mod nesting { enum Enum { A, + B(u64), + C { field: bool } } impl Enum { fn method() { @@ -272,6 +274,12 @@ mod nesting { use self::Enum::*; // Issue 3425 static STATIC: Enum = Enum::A; // Can't use Self as type } + + fn method2() { + let _ = Enum::B(42); + let _ = Enum::C { field: true }; + let _ = Enum::A; + } } } diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr index bf1f41fd6..12dd672e3 100644 --- a/tests/ui/use_self.stderr +++ b/tests/ui/use_self.stderr @@ -175,22 +175,40 @@ LL | Bar { foo: Foo {} } | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:304:13 + --> $DIR/use_self.rs:279:21 + | +LL | let _ = Enum::B(42); + | ^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:280:21 + | +LL | let _ = Enum::C { field: true }; + | ^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:281:21 + | +LL | let _ = Enum::A; + | ^^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:312:13 | LL | nested::A::fun_1(); | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:305:13 + --> $DIR/use_self.rs:313:13 | LL | nested::A::A; | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:307:13 + --> $DIR/use_self.rs:315:13 | LL | nested::A {}; | ^^^^^^^^^ help: use the applicable keyword: `Self` -error: aborting due to 31 previous errors +error: aborting due to 34 previous errors From 2a13e83f2baafed62db958aa202666a4182c4fb5 Mon Sep 17 00:00:00 2001 From: Jakub Wieczorek Date: Wed, 31 Jul 2019 00:25:35 +0000 Subject: [PATCH 16/17] Update all the code to pass the updated `use_self` lint. One struct required a temporary `#[allow(dead_code)]` annotation due to a bug in the Rust compiler: https://github.com/rust-lang/rust/issues/63151. --- clippy_dev/src/fmt.rs | 4 +- clippy_lints/src/checked_conversions.rs | 6 +-- clippy_lints/src/consts.rs | 56 +++++++++++----------- clippy_lints/src/excessive_precision.rs | 12 ++--- clippy_lints/src/literal_representation.rs | 14 +++--- clippy_lints/src/methods/mod.rs | 40 ++++++++-------- clippy_lints/src/non_copy_const.rs | 5 +- clippy_lints/src/ptr_offset_with_cast.rs | 8 ++-- clippy_lints/src/types.rs | 8 ++-- clippy_lints/src/utils/conf.rs | 6 +-- 10 files changed, 80 insertions(+), 79 deletions(-) diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index 5ccdbec14..e23673f27 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -15,13 +15,13 @@ pub enum CliError { impl From for CliError { fn from(error: io::Error) -> Self { - CliError::IoError(error) + Self::IoError(error) } } impl From for CliError { fn from(error: walkdir::Error) -> Self { - CliError::WalkDirError(error) + Self::WalkDirError(error) } } diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 3c335acb4..c23f2fbc0 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -160,12 +160,12 @@ impl ConversionType { /// Creates a conversion type if the type is allowed & conversion is valid fn try_new(from: &str, to: &str) -> Option { if UINTS.contains(&from) { - Some(ConversionType::FromUnsigned) + Some(Self::FromUnsigned) } else if SINTS.contains(&from) { if UINTS.contains(&to) { - Some(ConversionType::SignedToUnsigned) + Some(Self::SignedToUnsigned) } else if SINTS.contains(&to) { - Some(ConversionType::SignedToSigned) + Some(Self::SignedToSigned) } else { None } diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 9f6cfe04c..d18474abd 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -48,27 +48,27 @@ pub enum Constant { impl PartialEq for Constant { fn eq(&self, other: &Self) -> bool { match (self, other) { - (&Constant::Str(ref ls), &Constant::Str(ref rs)) => ls == rs, - (&Constant::Binary(ref l), &Constant::Binary(ref r)) => l == r, - (&Constant::Char(l), &Constant::Char(r)) => l == r, - (&Constant::Int(l), &Constant::Int(r)) => l == r, - (&Constant::F64(l), &Constant::F64(r)) => { + (&Self::Str(ref ls), &Self::Str(ref rs)) => ls == rs, + (&Self::Binary(ref l), &Self::Binary(ref r)) => l == r, + (&Self::Char(l), &Self::Char(r)) => l == r, + (&Self::Int(l), &Self::Int(r)) => l == r, + (&Self::F64(l), &Self::F64(r)) => { // We want `Fw32 == FwAny` and `FwAny == Fw64`, and by transitivity we must have // `Fw32 == Fw64`, so don’t compare them. // `to_bits` is required to catch non-matching 0.0, -0.0, and NaNs. l.to_bits() == r.to_bits() }, - (&Constant::F32(l), &Constant::F32(r)) => { + (&Self::F32(l), &Self::F32(r)) => { // We want `Fw32 == FwAny` and `FwAny == Fw64`, and by transitivity we must have // `Fw32 == Fw64`, so don’t compare them. // `to_bits` is required to catch non-matching 0.0, -0.0, and NaNs. f64::from(l).to_bits() == f64::from(r).to_bits() }, - (&Constant::Bool(l), &Constant::Bool(r)) => l == r, - (&Constant::Vec(ref l), &Constant::Vec(ref r)) | (&Constant::Tuple(ref l), &Constant::Tuple(ref r)) => { + (&Self::Bool(l), &Self::Bool(r)) => l == r, + (&Self::Vec(ref l), &Self::Vec(ref r)) | (&Self::Tuple(ref l), &Self::Tuple(ref r)) => { l == r }, - (&Constant::Repeat(ref lv, ref ls), &Constant::Repeat(ref rv, ref rs)) => ls == rs && lv == rv, + (&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => ls == rs && lv == rv, // TODO: are there inter-type equalities? _ => false, } @@ -82,38 +82,38 @@ impl Hash for Constant { { std::mem::discriminant(self).hash(state); match *self { - Constant::Str(ref s) => { + Self::Str(ref s) => { s.hash(state); }, - Constant::Binary(ref b) => { + Self::Binary(ref b) => { b.hash(state); }, - Constant::Char(c) => { + Self::Char(c) => { c.hash(state); }, - Constant::Int(i) => { + Self::Int(i) => { i.hash(state); }, - Constant::F32(f) => { + Self::F32(f) => { f64::from(f).to_bits().hash(state); }, - Constant::F64(f) => { + Self::F64(f) => { f.to_bits().hash(state); }, - Constant::Bool(b) => { + Self::Bool(b) => { b.hash(state); }, - Constant::Vec(ref v) | Constant::Tuple(ref v) => { + Self::Vec(ref v) | Self::Tuple(ref v) => { v.hash(state); }, - Constant::Repeat(ref c, l) => { + Self::Repeat(ref c, l) => { c.hash(state); l.hash(state); }, - Constant::RawPtr(u) => { + Self::RawPtr(u) => { u.hash(state); }, - Constant::Err(ref s) => { + Self::Err(ref s) => { s.hash(state); }, } @@ -123,25 +123,25 @@ impl Hash for Constant { impl Constant { pub fn partial_cmp(tcx: TyCtxt<'_>, cmp_type: Ty<'_>, left: &Self, right: &Self) -> Option { match (left, right) { - (&Constant::Str(ref ls), &Constant::Str(ref rs)) => Some(ls.cmp(rs)), - (&Constant::Char(ref l), &Constant::Char(ref r)) => Some(l.cmp(r)), - (&Constant::Int(l), &Constant::Int(r)) => { + (&Self::Str(ref ls), &Self::Str(ref rs)) => Some(ls.cmp(rs)), + (&Self::Char(ref l), &Self::Char(ref r)) => Some(l.cmp(r)), + (&Self::Int(l), &Self::Int(r)) => { if let ty::Int(int_ty) = cmp_type.sty { Some(sext(tcx, l, int_ty).cmp(&sext(tcx, r, int_ty))) } else { Some(l.cmp(&r)) } }, - (&Constant::F64(l), &Constant::F64(r)) => l.partial_cmp(&r), - (&Constant::F32(l), &Constant::F32(r)) => l.partial_cmp(&r), - (&Constant::Bool(ref l), &Constant::Bool(ref r)) => Some(l.cmp(r)), - (&Constant::Tuple(ref l), &Constant::Tuple(ref r)) | (&Constant::Vec(ref l), &Constant::Vec(ref r)) => l + (&Self::F64(l), &Self::F64(r)) => l.partial_cmp(&r), + (&Self::F32(l), &Self::F32(r)) => l.partial_cmp(&r), + (&Self::Bool(ref l), &Self::Bool(ref r)) => Some(l.cmp(r)), + (&Self::Tuple(ref l), &Self::Tuple(ref r)) | (&Self::Vec(ref l), &Self::Vec(ref r)) => l .iter() .zip(r.iter()) .map(|(li, ri)| Self::partial_cmp(tcx, cmp_type, li, ri)) .find(|r| r.map_or(true, |o| o != Ordering::Equal)) .unwrap_or_else(|| Some(l.len().cmp(&r.len()))), - (&Constant::Repeat(ref lv, ref ls), &Constant::Repeat(ref rv, ref rs)) => { + (&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => { match Self::partial_cmp(tcx, cmp_type, lv, rv) { Some(Equal) => Some(ls.cmp(rs)), x => x, diff --git a/clippy_lints/src/excessive_precision.rs b/clippy_lints/src/excessive_precision.rs index 5f6312378..e996bac39 100644 --- a/clippy_lints/src/excessive_precision.rs +++ b/clippy_lints/src/excessive_precision.rs @@ -143,20 +143,20 @@ impl FloatFormat { fn new(s: &str) -> Self { s.chars() .find_map(|x| match x { - 'e' => Some(FloatFormat::LowerExp), - 'E' => Some(FloatFormat::UpperExp), + 'e' => Some(Self::LowerExp), + 'E' => Some(Self::UpperExp), _ => None, }) - .unwrap_or(FloatFormat::Normal) + .unwrap_or(Self::Normal) } fn format(&self, f: T) -> String where T: fmt::UpperExp + fmt::LowerExp + fmt::Display, { match self { - FloatFormat::LowerExp => format!("{:e}", f), - FloatFormat::UpperExp => format!("{:E}", f), - FloatFormat::Normal => format!("{}", f), + Self::LowerExp => format!("{:e}", f), + Self::UpperExp => format!("{:E}", f), + Self::Normal => format!("{}", f), } } } diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index c2892a278..9cc957f14 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -115,8 +115,8 @@ impl Radix { /// Returns a reasonable digit group size for this radix. crate fn suggest_grouping(&self) -> usize { match *self { - Radix::Binary | Radix::Hexadecimal => 4, - Radix::Octal | Radix::Decimal => 3, + Self::Binary | Self::Hexadecimal => 4, + Self::Octal | Self::Decimal => 3, } } } @@ -285,7 +285,7 @@ enum WarningType { impl WarningType { crate fn display(&self, grouping_hint: &str, cx: &EarlyContext<'_>, span: syntax_pos::Span) { match self { - WarningType::MistypedLiteralSuffix => span_lint_and_sugg( + Self::MistypedLiteralSuffix => span_lint_and_sugg( cx, MISTYPED_LITERAL_SUFFIXES, span, @@ -294,7 +294,7 @@ impl WarningType { grouping_hint.to_string(), Applicability::MaybeIncorrect, ), - WarningType::UnreadableLiteral => span_lint_and_sugg( + Self::UnreadableLiteral => span_lint_and_sugg( cx, UNREADABLE_LITERAL, span, @@ -303,7 +303,7 @@ impl WarningType { grouping_hint.to_owned(), Applicability::MachineApplicable, ), - WarningType::LargeDigitGroups => span_lint_and_sugg( + Self::LargeDigitGroups => span_lint_and_sugg( cx, LARGE_DIGIT_GROUPS, span, @@ -312,7 +312,7 @@ impl WarningType { grouping_hint.to_owned(), Applicability::MachineApplicable, ), - WarningType::InconsistentDigitGrouping => span_lint_and_sugg( + Self::InconsistentDigitGrouping => span_lint_and_sugg( cx, INCONSISTENT_DIGIT_GROUPING, span, @@ -321,7 +321,7 @@ impl WarningType { grouping_hint.to_owned(), Applicability::MachineApplicable, ), - WarningType::DecimalRepresentation => span_lint_and_sugg( + Self::DecimalRepresentation => span_lint_and_sugg( cx, DECIMAL_LITERAL_REPRESENTATION, span, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f689a2d4e..02dfb3e32 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2505,14 +2505,14 @@ impl SelfKind { let is_actually_self = |ty| is_self_ty(ty) || SpanlessEq::new(cx).eq_ty(ty, self_ty); if is_self(arg) { match self { - SelfKind::Value => is_actually_self(ty), - SelfKind::Ref | SelfKind::RefMut => { + Self::Value => is_actually_self(ty), + Self::Ref | Self::RefMut => { if allow_value_for_ref && is_actually_self(ty) { return true; } match ty.node { hir::TyKind::Rptr(_, ref mt_ty) => { - let mutability_match = if self == SelfKind::Ref { + let mutability_match = if self == Self::Ref { mt_ty.mutbl == hir::MutImmutable } else { mt_ty.mutbl == hir::MutMutable @@ -2526,20 +2526,20 @@ impl SelfKind { } } else { match self { - SelfKind::Value => false, - SelfKind::Ref => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASREF_TRAIT), - SelfKind::RefMut => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASMUT_TRAIT), - SelfKind::No => true, + Self::Value => false, + Self::Ref => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASREF_TRAIT), + Self::RefMut => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASMUT_TRAIT), + Self::No => true, } } } fn description(self) -> &'static str { match self { - SelfKind::Value => "self by value", - SelfKind::Ref => "self by reference", - SelfKind::RefMut => "self by mutable reference", - SelfKind::No => "no self", + Self::Value => "self by value", + Self::Ref => "self by reference", + Self::RefMut => "self by mutable reference", + Self::No => "no self", } } } @@ -2609,8 +2609,8 @@ fn single_segment_ty(ty: &hir::Ty) -> Option<&hir::PathSegment> { impl Convention { fn check(&self, other: &str) -> bool { match *self { - Convention::Eq(this) => this == other, - Convention::StartsWith(this) => other.starts_with(this) && this != other, + Self::Eq(this) => this == other, + Self::StartsWith(this) => other.starts_with(this) && this != other, } } } @@ -2618,8 +2618,8 @@ impl Convention { impl fmt::Display for Convention { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { match *self { - Convention::Eq(this) => this.fmt(f), - Convention::StartsWith(this) => this.fmt(f).and_then(|_| '*'.fmt(f)), + Self::Eq(this) => this.fmt(f), + Self::StartsWith(this) => this.fmt(f).and_then(|_| '*'.fmt(f)), } } } @@ -2636,11 +2636,11 @@ impl OutType { fn matches(self, cx: &LateContext<'_, '_>, ty: &hir::FunctionRetTy) -> bool { let is_unit = |ty: &hir::Ty| SpanlessEq::new(cx).eq_ty_kind(&ty.node, &hir::TyKind::Tup(vec![].into())); match (self, ty) { - (OutType::Unit, &hir::DefaultReturn(_)) => true, - (OutType::Unit, &hir::Return(ref ty)) if is_unit(ty) => true, - (OutType::Bool, &hir::Return(ref ty)) if is_bool(ty) => true, - (OutType::Any, &hir::Return(ref ty)) if !is_unit(ty) => true, - (OutType::Ref, &hir::Return(ref ty)) => matches!(ty.node, hir::TyKind::Rptr(_, _)), + (Self::Unit, &hir::DefaultReturn(_)) => true, + (Self::Unit, &hir::Return(ref ty)) if is_unit(ty) => true, + (Self::Bool, &hir::Return(ref ty)) if is_bool(ty) => true, + (Self::Any, &hir::Return(ref ty)) if !is_unit(ty) => true, + (Self::Ref, &hir::Return(ref ty)) => matches!(ty.node, hir::TyKind::Rptr(_, _)), _ => false, } } diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 62e4f692e..644660100 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -84,6 +84,7 @@ declare_clippy_lint! { "referencing const with interior mutability" } +#[allow(dead_code)] #[derive(Copy, Clone)] enum Source { Item { item: Span }, @@ -94,12 +95,12 @@ enum Source { impl Source { fn lint(&self) -> (&'static Lint, &'static str, Span) { match self { - Source::Item { item } | Source::Assoc { item, .. } => ( + Self::Item { item } | Self::Assoc { item, .. } => ( DECLARE_INTERIOR_MUTABLE_CONST, "a const item should never be interior mutable", *item, ), - Source::Expr { expr } => ( + Self::Expr { expr } => ( BORROW_INTERIOR_MUTABLE_CONST, "a const item with interior mutability should not be borrowed", *expr, diff --git a/clippy_lints/src/ptr_offset_with_cast.rs b/clippy_lints/src/ptr_offset_with_cast.rs index 152140c85..ffd6d4ca0 100644 --- a/clippy_lints/src/ptr_offset_with_cast.rs +++ b/clippy_lints/src/ptr_offset_with_cast.rs @@ -133,8 +133,8 @@ enum Method { impl Method { fn suggestion(self) -> &'static str { match self { - Method::Offset => "add", - Method::WrappingOffset => "wrapping_add", + Self::Offset => "add", + Self::WrappingOffset => "wrapping_add", } } } @@ -142,8 +142,8 @@ impl Method { impl fmt::Display for Method { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Method::Offset => write!(f, "offset"), - Method::WrappingOffset => write!(f, "wrapping_offset"), + Self::Offset => write!(f, "offset"), + Self::WrappingOffset => write!(f, "wrapping_offset"), } } } diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 3610c3d12..31d4c3658 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1725,10 +1725,10 @@ impl PartialEq for FullInt { impl PartialOrd for FullInt { fn partial_cmp(&self, other: &Self) -> Option { Some(match (self, other) { - (&FullInt::S(s), &FullInt::S(o)) => s.cmp(&o), - (&FullInt::U(s), &FullInt::U(o)) => s.cmp(&o), - (&FullInt::S(s), &FullInt::U(o)) => Self::cmp_s_u(s, o), - (&FullInt::U(s), &FullInt::S(o)) => Self::cmp_s_u(o, s).reverse(), + (&Self::S(s), &Self::S(o)) => s.cmp(&o), + (&Self::U(s), &Self::U(o)) => s.cmp(&o), + (&Self::S(s), &Self::U(o)) => Self::cmp_s_u(s, o), + (&Self::U(s), &Self::S(o)) => Self::cmp_s_u(o, s).reverse(), }) } } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index efa2697d0..0c5db79df 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -44,15 +44,15 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { match *self { - Error::Io(ref err) => err.fmt(f), - Error::Toml(ref err) => err.fmt(f), + Self::Io(ref err) => err.fmt(f), + Self::Toml(ref err) => err.fmt(f), } } } impl From for Error { fn from(e: io::Error) -> Self { - Error::Io(e) + Self::Io(e) } } From 38e7bd20f29bfb6693f32fe7347ace0b95c7e7eb Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 31 Jul 2019 08:38:08 -0700 Subject: [PATCH 17/17] Don't nudge people towards toilet closures when producing owl results --- tests/ui/drop_forget_ref.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/ui/drop_forget_ref.rs b/tests/ui/drop_forget_ref.rs index b3c75bc57..b60f373e7 100644 --- a/tests/ui/drop_forget_ref.rs +++ b/tests/ui/drop_forget_ref.rs @@ -55,3 +55,38 @@ fn test_similarly_named_function() { forget(&SomeStruct); //OK; call to unrelated function which happens to have the same name std::mem::forget(&SomeStruct); } + +#[derive(Copy, Clone)] +pub struct Error; +fn produce_half_owl_error() -> Result<(), Error> { + Ok(()) +} + +fn produce_half_owl_ok() -> Result { + Ok(true) +} + +#[allow(dead_code)] +fn test_owl_result() -> Result<(), ()> { + produce_half_owl_error().map_err(|_| ())?; + produce_half_owl_ok().map(|_| ())?; + // the following should not be linted, + // we should not force users to use toilet closures + // to produce owl results when drop is more convenient + produce_half_owl_error().map_err(drop)?; + produce_half_owl_ok().map_err(drop)?; + Ok(()) +} + + +#[allow(dead_code)] +fn test_owl_result_2() -> Result { + produce_half_owl_error().map_err(|_| ())?; + produce_half_owl_ok().map(|_| ())?; + // the following should not be linted, + // we should not force users to use toilet closures + // to produce owl results when drop is more convenient + produce_half_owl_error().map_err(drop)?; + produce_half_owl_ok().map(drop)?; + Ok(1) +}