From 8dfb3ec8a45316365f6d950fe7d46e120cd42301 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 3 Dec 2021 16:35:04 +0100 Subject: [PATCH 1/3] Add new lint to warn when #[must_use] attribute should be used on a method --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_suspicious.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/return_self_not_must_use.rs | 105 +++++++++++++++++++ clippy_utils/src/lib.rs | 7 ++ tests/ui/return_self_not_must_use.rs | 42 ++++++++ tests/ui/return_self_not_must_use.stderr | 26 +++++ 9 files changed, 186 insertions(+) create mode 100644 clippy_lints/src/return_self_not_must_use.rs create mode 100644 tests/ui/return_self_not_must_use.rs create mode 100644 tests/ui/return_self_not_must_use.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 157ea0c96..2f64cd0e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3118,6 +3118,7 @@ Released 2018-09-13 [`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn [`result_unit_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unit_err +[`return_self_not_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 04912120e..f836d6443 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -245,6 +245,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(reference::REF_IN_DEREF), LintId::of(regex::INVALID_REGEX), LintId::of(repeat_once::REPEAT_ONCE), + LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE), LintId::of(returns::LET_AND_RETURN), LintId::of(returns::NEEDLESS_RETURN), LintId::of(self_assignment::SELF_ASSIGNMENT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index bb159e503..68889f4f5 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -422,6 +422,7 @@ store.register_lints(&[ regex::INVALID_REGEX, regex::TRIVIAL_REGEX, repeat_once::REPEAT_ONCE, + return_self_not_must_use::RETURN_SELF_NOT_MUST_USE, returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, same_name_method::SAME_NAME_METHOD, diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 414bfc42f..70dc92548 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -17,6 +17,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(mut_key::MUTABLE_KEY_TYPE), LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY), LintId::of(octal_escapes::OCTAL_ESCAPES), + LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE), LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bd9710ec4..c6b14ecac 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -341,6 +341,7 @@ mod ref_option_ref; mod reference; mod regex; mod repeat_once; +mod return_self_not_must_use; mod returns; mod same_name_method; mod self_assignment; @@ -853,6 +854,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray)); store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); store.register_late_pass(|| Box::new(needless_late_init::NeedlessLateInit)); + store.register_late_pass(|| Box::new(return_self_not_must_use::ReturnSelfNotMustUse)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/return_self_not_must_use.rs b/clippy_lints/src/return_self_not_must_use.rs new file mode 100644 index 000000000..1118da6c8 --- /dev/null +++ b/clippy_lints/src/return_self_not_must_use.rs @@ -0,0 +1,105 @@ +use clippy_utils::{diagnostics::span_lint, must_use_attr, nth_arg, return_ty}; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, FnDecl, HirId, TraitItem, TraitItemKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// This lint warns when a method returning `Self` doesn't have the `#[must_use]` attribute. + /// + /// ### Why is this bad? + /// It prevents to "forget" to use the newly created value. + /// + /// ### Limitations + /// This lint is only applied on methods taking a `self` argument. It would be mostly noise + /// if it was added on constructors for example. + /// + /// ### Example + /// ```rust + /// pub struct Bar; + /// + /// impl Bar { + /// // Bad + /// pub fn bar(&self) -> Self { + /// Self + /// } + /// + /// // Good + /// #[must_use] + /// pub fn foo(&self) -> Self { + /// Self + /// } + /// } + /// ``` + #[clippy::version = "1.59.0"] + pub RETURN_SELF_NOT_MUST_USE, + suspicious, + "missing `#[must_use]` annotation on a method returning `Self`" +} + +declare_lint_pass!(ReturnSelfNotMustUse => [RETURN_SELF_NOT_MUST_USE]); + +fn check_method(cx: &LateContext<'tcx>, decl: &'tcx FnDecl<'tcx>, fn_def: LocalDefId, span: Span, hir_id: HirId) { + if_chain! { + // If it comes from an external macro, better ignore it. + if !in_external_macro(cx.sess(), span); + if decl.implicit_self.has_implicit_self(); + // We only show this warning for public exported methods. + if cx.access_levels.is_exported(fn_def); + if cx.tcx.visibility(fn_def.to_def_id()).is_public(); + // No need to warn if the attribute is already present. + if must_use_attr(cx.tcx.hir().attrs(hir_id)).is_none(); + let ret_ty = return_ty(cx, hir_id); + let self_arg = nth_arg(cx, hir_id, 0); + // If `Self` has the same type as the returned type, then we want to warn. + // + // For this check, we don't want to remove the reference on the returned type because if + // there is one, we shouldn't emit a warning! + if self_arg.peel_refs() == ret_ty; + + then { + span_lint( + cx, + RETURN_SELF_NOT_MUST_USE, + span, + "missing `#[must_use]` attribute on a method returning `Self`", + ); + } + } +} + +impl<'tcx> LateLintPass<'tcx> for ReturnSelfNotMustUse { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl<'tcx>, + _: &'tcx Body<'tcx>, + span: Span, + hir_id: HirId, + ) { + if_chain! { + // We are only interested in methods, not in functions or associated functions. + if matches!(kind, FnKind::Method(_, _, _)); + if let Some(fn_def) = cx.tcx.hir().opt_local_def_id(hir_id); + if let Some(impl_def) = cx.tcx.impl_of_method(fn_def.to_def_id()); + // We don't want this method to be te implementation of a trait because the + // `#[must_use]` should be put on the trait definition directly. + if cx.tcx.trait_id_of_impl(impl_def).is_none(); + + then { + check_method(cx, decl, fn_def, span, hir_id); + } + } + } + + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'tcx>) { + if let TraitItemKind::Fn(ref sig, _) = item.kind { + check_method(cx, sig.decl, item.def_id, item.span, item.hir_id()); + } + } +} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index f011380c1..779c812ca 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1329,6 +1329,13 @@ pub fn return_ty<'tcx>(cx: &LateContext<'tcx>, fn_item: hir::HirId) -> Ty<'tcx> cx.tcx.erase_late_bound_regions(ret_ty) } +/// Convenience function to get the nth argument type of a function. +pub fn nth_arg<'tcx>(cx: &LateContext<'tcx>, fn_item: hir::HirId, nth: usize) -> Ty<'tcx> { + let fn_def_id = cx.tcx.hir().local_def_id(fn_item); + let arg = cx.tcx.fn_sig(fn_def_id).input(nth); + cx.tcx.erase_late_bound_regions(arg) +} + /// Checks if an expression is constructing a tuple-like enum variant or struct pub fn is_ctor_or_promotable_const_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { if let ExprKind::Call(fun, _) = expr.kind { diff --git a/tests/ui/return_self_not_must_use.rs b/tests/ui/return_self_not_must_use.rs new file mode 100644 index 000000000..bdf3f3d79 --- /dev/null +++ b/tests/ui/return_self_not_must_use.rs @@ -0,0 +1,42 @@ +#![crate_type = "lib"] + +#[derive(Clone)] +pub struct Bar; + +pub trait Whatever { + fn what(&self) -> Self; + // There should be no warning here! + fn what2(&self) -> &Self; +} + +impl Bar { + // There should be no warning here! + pub fn not_new() -> Self { + Self + } + pub fn foo(&self) -> Self { + Self + } + pub fn bar(self) -> Self { + self + } + // There should be no warning here! + fn foo2(&self) -> Self { + Self + } + // There should be no warning here! + pub fn foo3(&self) -> &Self { + self + } +} + +impl Whatever for Bar { + // There should be no warning here! + fn what(&self) -> Self { + self.foo2() + } + // There should be no warning here! + fn what2(&self) -> &Self { + self + } +} diff --git a/tests/ui/return_self_not_must_use.stderr b/tests/ui/return_self_not_must_use.stderr new file mode 100644 index 000000000..3793a5559 --- /dev/null +++ b/tests/ui/return_self_not_must_use.stderr @@ -0,0 +1,26 @@ +error: missing `#[must_use]` attribute on a method returning `Self` + --> $DIR/return_self_not_must_use.rs:7:5 + | +LL | fn what(&self) -> Self; + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::return-self-not-must-use` implied by `-D warnings` + +error: missing `#[must_use]` attribute on a method returning `Self` + --> $DIR/return_self_not_must_use.rs:17:5 + | +LL | / pub fn foo(&self) -> Self { +LL | | Self +LL | | } + | |_____^ + +error: missing `#[must_use]` attribute on a method returning `Self` + --> $DIR/return_self_not_must_use.rs:20:5 + | +LL | / pub fn bar(self) -> Self { +LL | | self +LL | | } + | |_____^ + +error: aborting due to 3 previous errors + From 2b35edbb84c6de7bdd253d56ea4a7b2c50a1513a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 3 Dec 2021 17:51:55 +0100 Subject: [PATCH 2/3] Update other UI tests as well --- tests/ui/auxiliary/option_helpers.rs | 2 +- tests/ui/deref_addrof.fixed | 1 + tests/ui/deref_addrof.rs | 1 + tests/ui/deref_addrof.stderr | 20 ++++++------- tests/ui/should_impl_trait/corner_cases.rs | 3 +- tests/ui/should_impl_trait/method_list_1.rs | 3 +- .../ui/should_impl_trait/method_list_1.stderr | 28 ++++++++--------- tests/ui/should_impl_trait/method_list_2.rs | 3 +- .../ui/should_impl_trait/method_list_2.stderr | 30 +++++++++---------- 9 files changed, 48 insertions(+), 43 deletions(-) diff --git a/tests/ui/auxiliary/option_helpers.rs b/tests/ui/auxiliary/option_helpers.rs index 86a637ce3..f9bc9436b 100644 --- a/tests/ui/auxiliary/option_helpers.rs +++ b/tests/ui/auxiliary/option_helpers.rs @@ -1,4 +1,4 @@ -#![allow(dead_code, unused_variables)] +#![allow(dead_code, unused_variables, clippy::return_self_not_must_use)] /// Utility macro to test linting behavior in `option_methods()` /// The lints included in `option_methods()` should not lint if the call to map is partially diff --git a/tests/ui/deref_addrof.fixed b/tests/ui/deref_addrof.fixed index 9a150c67a..0029fc673 100644 --- a/tests/ui/deref_addrof.fixed +++ b/tests/ui/deref_addrof.fixed @@ -1,4 +1,5 @@ // run-rustfix +#![allow(clippy::return_self_not_must_use)] #![warn(clippy::deref_addrof)] fn get_number() -> usize { diff --git a/tests/ui/deref_addrof.rs b/tests/ui/deref_addrof.rs index 80ba7e9bd..f2f02dd5e 100644 --- a/tests/ui/deref_addrof.rs +++ b/tests/ui/deref_addrof.rs @@ -1,4 +1,5 @@ // run-rustfix +#![allow(clippy::return_self_not_must_use)] #![warn(clippy::deref_addrof)] fn get_number() -> usize { diff --git a/tests/ui/deref_addrof.stderr b/tests/ui/deref_addrof.stderr index 1a14f31af..5bc1cbfa2 100644 --- a/tests/ui/deref_addrof.stderr +++ b/tests/ui/deref_addrof.stderr @@ -1,5 +1,5 @@ error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:18:13 + --> $DIR/deref_addrof.rs:19:13 | LL | let b = *&a; | ^^^ help: try this: `a` @@ -7,49 +7,49 @@ LL | let b = *&a; = note: `-D clippy::deref-addrof` implied by `-D warnings` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:20:13 + --> $DIR/deref_addrof.rs:21:13 | LL | let b = *&get_number(); | ^^^^^^^^^^^^^^ help: try this: `get_number()` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:25:13 + --> $DIR/deref_addrof.rs:26:13 | LL | let b = *&bytes[1..2][0]; | ^^^^^^^^^^^^^^^^ help: try this: `bytes[1..2][0]` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:29:13 + --> $DIR/deref_addrof.rs:30:13 | LL | let b = *&(a); | ^^^^^ help: try this: `(a)` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:31:13 + --> $DIR/deref_addrof.rs:32:13 | LL | let b = *(&a); | ^^^^^ help: try this: `a` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:34:13 + --> $DIR/deref_addrof.rs:35:13 | LL | let b = *((&a)); | ^^^^^^^ help: try this: `a` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:36:13 + --> $DIR/deref_addrof.rs:37:13 | LL | let b = *&&a; | ^^^^ help: try this: `&a` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:38:14 + --> $DIR/deref_addrof.rs:39:14 | LL | let b = **&aref; | ^^^^^^ help: try this: `aref` error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:44:9 + --> $DIR/deref_addrof.rs:45:9 | LL | *& $visitor | ^^^^^^^^^^^ help: try this: `$visitor` @@ -60,7 +60,7 @@ LL | m!(self) = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info) error: immediately dereferencing a reference - --> $DIR/deref_addrof.rs:51:9 + --> $DIR/deref_addrof.rs:52:9 | LL | *& mut $visitor | ^^^^^^^^^^^^^^^ help: try this: `$visitor` diff --git a/tests/ui/should_impl_trait/corner_cases.rs b/tests/ui/should_impl_trait/corner_cases.rs index d7e8d02bd..1ccb0a1d1 100644 --- a/tests/ui/should_impl_trait/corner_cases.rs +++ b/tests/ui/should_impl_trait/corner_cases.rs @@ -7,7 +7,8 @@ clippy::needless_lifetimes, clippy::missing_safety_doc, clippy::wrong_self_convention, - clippy::missing_panics_doc + clippy::missing_panics_doc, + clippy::return_self_not_must_use )] use std::ops::Mul; diff --git a/tests/ui/should_impl_trait/method_list_1.rs b/tests/ui/should_impl_trait/method_list_1.rs index ea962f943..20d49f5a9 100644 --- a/tests/ui/should_impl_trait/method_list_1.rs +++ b/tests/ui/should_impl_trait/method_list_1.rs @@ -7,7 +7,8 @@ clippy::needless_lifetimes, clippy::missing_safety_doc, clippy::wrong_self_convention, - clippy::missing_panics_doc + clippy::missing_panics_doc, + clippy::return_self_not_must_use )] use std::ops::Mul; diff --git a/tests/ui/should_impl_trait/method_list_1.stderr b/tests/ui/should_impl_trait/method_list_1.stderr index bf8b47d56..2b7d4628c 100644 --- a/tests/ui/should_impl_trait/method_list_1.stderr +++ b/tests/ui/should_impl_trait/method_list_1.stderr @@ -1,5 +1,5 @@ error: method `add` can be confused for the standard trait method `std::ops::Add::add` - --> $DIR/method_list_1.rs:24:5 + --> $DIR/method_list_1.rs:25:5 | LL | / pub fn add(self, other: T) -> T { LL | | unimplemented!() @@ -10,7 +10,7 @@ LL | | } = help: consider implementing the trait `std::ops::Add` or choosing a less ambiguous method name error: method `as_mut` can be confused for the standard trait method `std::convert::AsMut::as_mut` - --> $DIR/method_list_1.rs:28:5 + --> $DIR/method_list_1.rs:29:5 | LL | / pub fn as_mut(&mut self) -> &mut T { LL | | unimplemented!() @@ -20,7 +20,7 @@ LL | | } = help: consider implementing the trait `std::convert::AsMut` or choosing a less ambiguous method name error: method `as_ref` can be confused for the standard trait method `std::convert::AsRef::as_ref` - --> $DIR/method_list_1.rs:32:5 + --> $DIR/method_list_1.rs:33:5 | LL | / pub fn as_ref(&self) -> &T { LL | | unimplemented!() @@ -30,7 +30,7 @@ LL | | } = help: consider implementing the trait `std::convert::AsRef` or choosing a less ambiguous method name error: method `bitand` can be confused for the standard trait method `std::ops::BitAnd::bitand` - --> $DIR/method_list_1.rs:36:5 + --> $DIR/method_list_1.rs:37:5 | LL | / pub fn bitand(self, rhs: T) -> T { LL | | unimplemented!() @@ -40,7 +40,7 @@ LL | | } = help: consider implementing the trait `std::ops::BitAnd` or choosing a less ambiguous method name error: method `bitor` can be confused for the standard trait method `std::ops::BitOr::bitor` - --> $DIR/method_list_1.rs:40:5 + --> $DIR/method_list_1.rs:41:5 | LL | / pub fn bitor(self, rhs: Self) -> Self { LL | | unimplemented!() @@ -50,7 +50,7 @@ LL | | } = help: consider implementing the trait `std::ops::BitOr` or choosing a less ambiguous method name error: method `bitxor` can be confused for the standard trait method `std::ops::BitXor::bitxor` - --> $DIR/method_list_1.rs:44:5 + --> $DIR/method_list_1.rs:45:5 | LL | / pub fn bitxor(self, rhs: Self) -> Self { LL | | unimplemented!() @@ -60,7 +60,7 @@ LL | | } = help: consider implementing the trait `std::ops::BitXor` or choosing a less ambiguous method name error: method `borrow` can be confused for the standard trait method `std::borrow::Borrow::borrow` - --> $DIR/method_list_1.rs:48:5 + --> $DIR/method_list_1.rs:49:5 | LL | / pub fn borrow(&self) -> &str { LL | | unimplemented!() @@ -70,7 +70,7 @@ LL | | } = help: consider implementing the trait `std::borrow::Borrow` or choosing a less ambiguous method name error: method `borrow_mut` can be confused for the standard trait method `std::borrow::BorrowMut::borrow_mut` - --> $DIR/method_list_1.rs:52:5 + --> $DIR/method_list_1.rs:53:5 | LL | / pub fn borrow_mut(&mut self) -> &mut str { LL | | unimplemented!() @@ -80,7 +80,7 @@ LL | | } = help: consider implementing the trait `std::borrow::BorrowMut` or choosing a less ambiguous method name error: method `clone` can be confused for the standard trait method `std::clone::Clone::clone` - --> $DIR/method_list_1.rs:56:5 + --> $DIR/method_list_1.rs:57:5 | LL | / pub fn clone(&self) -> Self { LL | | unimplemented!() @@ -90,7 +90,7 @@ LL | | } = help: consider implementing the trait `std::clone::Clone` or choosing a less ambiguous method name error: method `cmp` can be confused for the standard trait method `std::cmp::Ord::cmp` - --> $DIR/method_list_1.rs:60:5 + --> $DIR/method_list_1.rs:61:5 | LL | / pub fn cmp(&self, other: &Self) -> Self { LL | | unimplemented!() @@ -100,7 +100,7 @@ LL | | } = help: consider implementing the trait `std::cmp::Ord` or choosing a less ambiguous method name error: method `deref` can be confused for the standard trait method `std::ops::Deref::deref` - --> $DIR/method_list_1.rs:68:5 + --> $DIR/method_list_1.rs:69:5 | LL | / pub fn deref(&self) -> &Self { LL | | unimplemented!() @@ -110,7 +110,7 @@ LL | | } = help: consider implementing the trait `std::ops::Deref` or choosing a less ambiguous method name error: method `deref_mut` can be confused for the standard trait method `std::ops::DerefMut::deref_mut` - --> $DIR/method_list_1.rs:72:5 + --> $DIR/method_list_1.rs:73:5 | LL | / pub fn deref_mut(&mut self) -> &mut Self { LL | | unimplemented!() @@ -120,7 +120,7 @@ LL | | } = help: consider implementing the trait `std::ops::DerefMut` or choosing a less ambiguous method name error: method `div` can be confused for the standard trait method `std::ops::Div::div` - --> $DIR/method_list_1.rs:76:5 + --> $DIR/method_list_1.rs:77:5 | LL | / pub fn div(self, rhs: Self) -> Self { LL | | unimplemented!() @@ -130,7 +130,7 @@ LL | | } = help: consider implementing the trait `std::ops::Div` or choosing a less ambiguous method name error: method `drop` can be confused for the standard trait method `std::ops::Drop::drop` - --> $DIR/method_list_1.rs:80:5 + --> $DIR/method_list_1.rs:81:5 | LL | / pub fn drop(&mut self) { LL | | unimplemented!() diff --git a/tests/ui/should_impl_trait/method_list_2.rs b/tests/ui/should_impl_trait/method_list_2.rs index b66356880..3efec1c52 100644 --- a/tests/ui/should_impl_trait/method_list_2.rs +++ b/tests/ui/should_impl_trait/method_list_2.rs @@ -7,7 +7,8 @@ clippy::needless_lifetimes, clippy::missing_safety_doc, clippy::wrong_self_convention, - clippy::missing_panics_doc + clippy::missing_panics_doc, + clippy::return_self_not_must_use )] use std::ops::Mul; diff --git a/tests/ui/should_impl_trait/method_list_2.stderr b/tests/ui/should_impl_trait/method_list_2.stderr index 426fe3b1a..b6fd43569 100644 --- a/tests/ui/should_impl_trait/method_list_2.stderr +++ b/tests/ui/should_impl_trait/method_list_2.stderr @@ -1,5 +1,5 @@ error: method `eq` can be confused for the standard trait method `std::cmp::PartialEq::eq` - --> $DIR/method_list_2.rs:25:5 + --> $DIR/method_list_2.rs:26:5 | LL | / pub fn eq(&self, other: &Self) -> bool { LL | | unimplemented!() @@ -10,7 +10,7 @@ LL | | } = help: consider implementing the trait `std::cmp::PartialEq` or choosing a less ambiguous method name error: method `from_iter` can be confused for the standard trait method `std::iter::FromIterator::from_iter` - --> $DIR/method_list_2.rs:29:5 + --> $DIR/method_list_2.rs:30:5 | LL | / pub fn from_iter(iter: T) -> Self { LL | | unimplemented!() @@ -20,7 +20,7 @@ LL | | } = help: consider implementing the trait `std::iter::FromIterator` or choosing a less ambiguous method name error: method `from_str` can be confused for the standard trait method `std::str::FromStr::from_str` - --> $DIR/method_list_2.rs:33:5 + --> $DIR/method_list_2.rs:34:5 | LL | / pub fn from_str(s: &str) -> Result { LL | | unimplemented!() @@ -30,7 +30,7 @@ LL | | } = help: consider implementing the trait `std::str::FromStr` or choosing a less ambiguous method name error: method `hash` can be confused for the standard trait method `std::hash::Hash::hash` - --> $DIR/method_list_2.rs:37:5 + --> $DIR/method_list_2.rs:38:5 | LL | / pub fn hash(&self, state: &mut T) { LL | | unimplemented!() @@ -40,7 +40,7 @@ LL | | } = help: consider implementing the trait `std::hash::Hash` or choosing a less ambiguous method name error: method `index` can be confused for the standard trait method `std::ops::Index::index` - --> $DIR/method_list_2.rs:41:5 + --> $DIR/method_list_2.rs:42:5 | LL | / pub fn index(&self, index: usize) -> &Self { LL | | unimplemented!() @@ -50,7 +50,7 @@ LL | | } = help: consider implementing the trait `std::ops::Index` or choosing a less ambiguous method name error: method `index_mut` can be confused for the standard trait method `std::ops::IndexMut::index_mut` - --> $DIR/method_list_2.rs:45:5 + --> $DIR/method_list_2.rs:46:5 | LL | / pub fn index_mut(&mut self, index: usize) -> &mut Self { LL | | unimplemented!() @@ -60,7 +60,7 @@ LL | | } = help: consider implementing the trait `std::ops::IndexMut` or choosing a less ambiguous method name error: method `into_iter` can be confused for the standard trait method `std::iter::IntoIterator::into_iter` - --> $DIR/method_list_2.rs:49:5 + --> $DIR/method_list_2.rs:50:5 | LL | / pub fn into_iter(self) -> Self { LL | | unimplemented!() @@ -70,7 +70,7 @@ LL | | } = help: consider implementing the trait `std::iter::IntoIterator` or choosing a less ambiguous method name error: method `mul` can be confused for the standard trait method `std::ops::Mul::mul` - --> $DIR/method_list_2.rs:53:5 + --> $DIR/method_list_2.rs:54:5 | LL | / pub fn mul(self, rhs: Self) -> Self { LL | | unimplemented!() @@ -80,7 +80,7 @@ LL | | } = help: consider implementing the trait `std::ops::Mul` or choosing a less ambiguous method name error: method `neg` can be confused for the standard trait method `std::ops::Neg::neg` - --> $DIR/method_list_2.rs:57:5 + --> $DIR/method_list_2.rs:58:5 | LL | / pub fn neg(self) -> Self { LL | | unimplemented!() @@ -90,7 +90,7 @@ LL | | } = help: consider implementing the trait `std::ops::Neg` or choosing a less ambiguous method name error: method `next` can be confused for the standard trait method `std::iter::Iterator::next` - --> $DIR/method_list_2.rs:61:5 + --> $DIR/method_list_2.rs:62:5 | LL | / pub fn next(&mut self) -> Option { LL | | unimplemented!() @@ -100,7 +100,7 @@ LL | | } = help: consider implementing the trait `std::iter::Iterator` or choosing a less ambiguous method name error: method `not` can be confused for the standard trait method `std::ops::Not::not` - --> $DIR/method_list_2.rs:65:5 + --> $DIR/method_list_2.rs:66:5 | LL | / pub fn not(self) -> Self { LL | | unimplemented!() @@ -110,7 +110,7 @@ LL | | } = help: consider implementing the trait `std::ops::Not` or choosing a less ambiguous method name error: method `rem` can be confused for the standard trait method `std::ops::Rem::rem` - --> $DIR/method_list_2.rs:69:5 + --> $DIR/method_list_2.rs:70:5 | LL | / pub fn rem(self, rhs: Self) -> Self { LL | | unimplemented!() @@ -120,7 +120,7 @@ LL | | } = help: consider implementing the trait `std::ops::Rem` or choosing a less ambiguous method name error: method `shl` can be confused for the standard trait method `std::ops::Shl::shl` - --> $DIR/method_list_2.rs:73:5 + --> $DIR/method_list_2.rs:74:5 | LL | / pub fn shl(self, rhs: Self) -> Self { LL | | unimplemented!() @@ -130,7 +130,7 @@ LL | | } = help: consider implementing the trait `std::ops::Shl` or choosing a less ambiguous method name error: method `shr` can be confused for the standard trait method `std::ops::Shr::shr` - --> $DIR/method_list_2.rs:77:5 + --> $DIR/method_list_2.rs:78:5 | LL | / pub fn shr(self, rhs: Self) -> Self { LL | | unimplemented!() @@ -140,7 +140,7 @@ LL | | } = help: consider implementing the trait `std::ops::Shr` or choosing a less ambiguous method name error: method `sub` can be confused for the standard trait method `std::ops::Sub::sub` - --> $DIR/method_list_2.rs:81:5 + --> $DIR/method_list_2.rs:82:5 | LL | / pub fn sub(self, rhs: Self) -> Self { LL | | unimplemented!() From e93767b395ca18ac895e0d52520c2586b7aae0e6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 3 Dec 2021 18:00:41 +0100 Subject: [PATCH 3/3] Apply new lint on clippy source code --- clippy_utils/src/hir_utils.rs | 2 ++ clippy_utils/src/sugg.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 7438b6eab..c1eaa5c51 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -41,6 +41,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } /// Consider expressions containing potential side effects as not equal. + #[must_use] pub fn deny_side_effects(self) -> Self { Self { allow_side_effects: false, @@ -48,6 +49,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { } } + #[must_use] pub fn expr_fallback(self, expr_fallback: impl FnMut(&Expr<'_>, &Expr<'_>) -> bool + 'a) -> Self { Self { expr_fallback: Some(Box::new(expr_fallback)), diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index 872942685..586934df4 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -294,6 +294,7 @@ impl<'a> Sugg<'a> { /// Adds parentheses to any expression that might need them. Suitable to the /// `self` argument of a method call /// (e.g., to build `bar.foo()` or `(1 + 2).foo()`). + #[must_use] pub fn maybe_par(self) -> Self { match self { Sugg::NonParen(..) => self,