diff options
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 84 | ||||
| -rw-r--r-- | tests/ui/methods.rs | 15 | ||||
| -rw-r--r-- | tests/ui/methods.stderr | 26 |
3 files changed, 77 insertions, 48 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 124fc1d9878..3972af1913f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1426,11 +1426,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { then { if cx.access_levels.is_exported(impl_item.hir_id) { // check missing trait implementations - for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS { + for &(method_name, n_args, fn_header, self_kind, out_type, trait_name) in &TRAIT_METHODS { if name == method_name && - sig.decl.inputs.len() == n_args && - out_type.matches(cx, &sig.decl.output) && - self_kind.matches(cx, self_ty, first_arg_ty) { + sig.decl.inputs.len() == n_args && + out_type.matches(cx, &sig.decl.output) && + self_kind.matches(cx, self_ty, first_arg_ty) && + fn_header_equals(*fn_header, sig.header) { span_lint(cx, SHOULD_IMPLEMENT_TRAIT, impl_item.span, &format!( "defining a method called `{}` on this type; consider implementing \ the `{}` trait or choosing a less ambiguous name", name, trait_name)); @@ -3266,38 +3267,45 @@ const CONVENTIONS: [(Convention, &[SelfKind]); 7] = [ (Convention::StartsWith("to_"), &[SelfKind::Ref]), ]; +const FN_HEADER: hir::FnHeader = hir::FnHeader { + unsafety: hir::Unsafety::Normal, + constness: hir::Constness::NotConst, + asyncness: hir::IsAsync::NotAsync, + abi: rustc_target::spec::abi::Abi::Rust, +}; + #[rustfmt::skip] -const TRAIT_METHODS: [(&str, usize, SelfKind, OutType, &str); 30] = [ - ("add", 2, SelfKind::Value, OutType::Any, "std::ops::Add"), - ("as_mut", 1, SelfKind::RefMut, OutType::Ref, "std::convert::AsMut"), - ("as_ref", 1, SelfKind::Ref, OutType::Ref, "std::convert::AsRef"), - ("bitand", 2, SelfKind::Value, OutType::Any, "std::ops::BitAnd"), - ("bitor", 2, SelfKind::Value, OutType::Any, "std::ops::BitOr"), - ("bitxor", 2, SelfKind::Value, OutType::Any, "std::ops::BitXor"), - ("borrow", 1, SelfKind::Ref, OutType::Ref, "std::borrow::Borrow"), - ("borrow_mut", 1, SelfKind::RefMut, OutType::Ref, "std::borrow::BorrowMut"), - ("clone", 1, SelfKind::Ref, OutType::Any, "std::clone::Clone"), - ("cmp", 2, SelfKind::Ref, OutType::Any, "std::cmp::Ord"), - ("default", 0, SelfKind::No, OutType::Any, "std::default::Default"), - ("deref", 1, SelfKind::Ref, OutType::Ref, "std::ops::Deref"), - ("deref_mut", 1, SelfKind::RefMut, OutType::Ref, "std::ops::DerefMut"), - ("div", 2, SelfKind::Value, OutType::Any, "std::ops::Div"), - ("drop", 1, SelfKind::RefMut, OutType::Unit, "std::ops::Drop"), - ("eq", 2, SelfKind::Ref, OutType::Bool, "std::cmp::PartialEq"), - ("from_iter", 1, SelfKind::No, OutType::Any, "std::iter::FromIterator"), - ("from_str", 1, SelfKind::No, OutType::Any, "std::str::FromStr"), - ("hash", 2, SelfKind::Ref, OutType::Unit, "std::hash::Hash"), - ("index", 2, SelfKind::Ref, OutType::Ref, "std::ops::Index"), - ("index_mut", 2, SelfKind::RefMut, OutType::Ref, "std::ops::IndexMut"), - ("into_iter", 1, SelfKind::Value, OutType::Any, "std::iter::IntoIterator"), - ("mul", 2, SelfKind::Value, OutType::Any, "std::ops::Mul"), - ("neg", 1, SelfKind::Value, OutType::Any, "std::ops::Neg"), - ("next", 1, SelfKind::RefMut, OutType::Any, "std::iter::Iterator"), - ("not", 1, SelfKind::Value, OutType::Any, "std::ops::Not"), - ("rem", 2, SelfKind::Value, OutType::Any, "std::ops::Rem"), - ("shl", 2, SelfKind::Value, OutType::Any, "std::ops::Shl"), - ("shr", 2, SelfKind::Value, OutType::Any, "std::ops::Shr"), - ("sub", 2, SelfKind::Value, OutType::Any, "std::ops::Sub"), +const TRAIT_METHODS: [(&str, usize, &hir::FnHeader, SelfKind, OutType, &str); 30] = [ + ("add", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Add"), + ("as_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, "std::convert::AsMut"), + ("as_ref", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, "std::convert::AsRef"), + ("bitand", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::BitAnd"), + ("bitor", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::BitOr"), + ("bitxor", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::BitXor"), + ("borrow", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, "std::borrow::Borrow"), + ("borrow_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, "std::borrow::BorrowMut"), + ("clone", 1, &FN_HEADER, SelfKind::Ref, OutType::Any, "std::clone::Clone"), + ("cmp", 2, &FN_HEADER, SelfKind::Ref, OutType::Any, "std::cmp::Ord"), + ("default", 0, &FN_HEADER, SelfKind::No, OutType::Any, "std::default::Default"), + ("deref", 1, &FN_HEADER, SelfKind::Ref, OutType::Ref, "std::ops::Deref"), + ("deref_mut", 1, &FN_HEADER, SelfKind::RefMut, OutType::Ref, "std::ops::DerefMut"), + ("div", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Div"), + ("drop", 1, &FN_HEADER, SelfKind::RefMut, OutType::Unit, "std::ops::Drop"), + ("eq", 2, &FN_HEADER, SelfKind::Ref, OutType::Bool, "std::cmp::PartialEq"), + ("from_iter", 1, &FN_HEADER, SelfKind::No, OutType::Any, "std::iter::FromIterator"), + ("from_str", 1, &FN_HEADER, SelfKind::No, OutType::Any, "std::str::FromStr"), + ("hash", 2, &FN_HEADER, SelfKind::Ref, OutType::Unit, "std::hash::Hash"), + ("index", 2, &FN_HEADER, SelfKind::Ref, OutType::Ref, "std::ops::Index"), + ("index_mut", 2, &FN_HEADER, SelfKind::RefMut, OutType::Ref, "std::ops::IndexMut"), + ("into_iter", 1, &FN_HEADER, SelfKind::Value, OutType::Any, "std::iter::IntoIterator"), + ("mul", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Mul"), + ("neg", 1, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Neg"), + ("next", 1, &FN_HEADER, SelfKind::RefMut, OutType::Any, "std::iter::Iterator"), + ("not", 1, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Not"), + ("rem", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Rem"), + ("shl", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Shl"), + ("shr", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Shr"), + ("sub", 2, &FN_HEADER, SelfKind::Value, OutType::Any, "std::ops::Sub"), ]; #[rustfmt::skip] @@ -3510,3 +3518,9 @@ fn lint_filetype_is_file(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: & let help_msg = format!("use `{}FileType::is_dir()` instead", help_unary); span_lint_and_help(cx, FILETYPE_IS_FILE, span, &lint_msg, &help_msg); } + +fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool { + expected.constness == actual.constness + && expected.unsafety == actual.unsafety + && expected.asyncness == actual.asyncness +} diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index 2af33b26340..7880cf36415 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -6,6 +6,7 @@ clippy::blacklisted_name, clippy::default_trait_access, clippy::missing_docs_in_private_items, + clippy::missing_safety_doc, clippy::non_ascii_literal, clippy::new_without_default, clippy::needless_pass_by_value, @@ -83,6 +84,20 @@ impl T { } } +pub struct T1; + +impl T1 { + // Shouldn't trigger lint as it is unsafe. + pub unsafe fn add(self, rhs: T1) -> T1 { + self + } + + // Should not trigger lint since this is an async function. + pub async fn next(&mut self) -> Option<T1> { + None + } +} + struct Lt<'a> { foo: &'a u32, } diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index 878e78fdcc5..01cf487ac14 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -1,5 +1,5 @@ error: defining a method called `add` on this type; consider implementing the `std::ops::Add` trait or choosing a less ambiguous name - --> $DIR/methods.rs:38:5 + --> $DIR/methods.rs:39:5 | LL | / pub fn add(self, other: T) -> T { LL | | self @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::should-implement-trait` implied by `-D warnings` error: methods called `new` usually return `Self` - --> $DIR/methods.rs:154:5 + --> $DIR/methods.rs:169:5 | LL | / fn new() -> i32 { LL | | 0 @@ -19,7 +19,7 @@ LL | | } = note: `-D clippy::new-ret-no-self` implied by `-D warnings` error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:173:13 + --> $DIR/methods.rs:188:13 | LL | let _ = v.iter().filter(|&x| *x < 0).next(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -28,7 +28,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next(); = note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)` error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:176:13 + --> $DIR/methods.rs:191:13 | LL | let _ = v.iter().filter(|&x| { | _____________^ @@ -38,7 +38,7 @@ LL | | ).next(); | |___________________________^ error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:193:22 + --> $DIR/methods.rs:208:22 | LL | let _ = v.iter().find(|&x| *x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)` @@ -46,25 +46,25 @@ LL | let _ = v.iter().find(|&x| *x < 0).is_some(); = note: `-D clippy::search-is-some` implied by `-D warnings` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:194:20 + --> $DIR/methods.rs:209:20 | LL | let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:195:20 + --> $DIR/methods.rs:210:20 | LL | let _ = (0..1).find(|x| *x == 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:196:22 + --> $DIR/methods.rs:211:22 | LL | let _ = v.iter().find(|x| **x == 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x == 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:199:13 + --> $DIR/methods.rs:214:13 | LL | let _ = v.iter().find(|&x| { | _____________^ @@ -74,13 +74,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:205:22 + --> $DIR/methods.rs:220:22 | LL | let _ = v.iter().position(|&x| x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:208:13 + --> $DIR/methods.rs:223:13 | LL | let _ = v.iter().position(|&x| { | _____________^ @@ -90,13 +90,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:214:22 + --> $DIR/methods.rs:229:22 | LL | let _ = v.iter().rposition(|&x| x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:217:13 + --> $DIR/methods.rs:232:13 | LL | let _ = v.iter().rposition(|&x| { | _____________^ |