Auto merge of rust-lang#8146 - GuillaumeGomez:must-use-self, r=xFrednet

Don't emit RETURN_SELF_NOT_MUST_USE lint if `Self` already is marked as `#[must_use]`

New bug discovered with this lint. Hopefully, this is the last one.

---

changelog: none

Author: bors

clippy_lints/src/return_self_not_must_use.rs

@@ -1,4 +1,5 @@
-use clippy_utils::{diagnostics::span_lint, must_use_attr, nth_arg, return_ty};
+use clippy_utils::ty::is_must_use_ty;
+use clippy_utils::{diagnostics::span_lint, nth_arg, return_ty};
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{Body, FnDecl, HirId, TraitItem, TraitItemKind};
@@ -50,16 +51,18 @@ fn check_method(cx: &LateContext<'tcx>, decl: &'tcx FnDecl<'tcx>, fn_def: LocalD
         if decl.implicit_self.has_implicit_self();
         // We only show this warning for public exported methods.
         if cx.access_levels.is_exported(fn_def);
+        // We don't want to emit this lint if the `#[must_use]` attribute is already there.
+        if !cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::must_use));
         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;
+        // If `Self` is already marked as `#[must_use]`, no need for the attribute here.
+        if !is_must_use_ty(cx, ret_ty);
 
         then {
             span_lint(
@@ -86,8 +89,6 @@ impl<'tcx> LateLintPass<'tcx> for ReturnSelfNotMustUse {
             // 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);
-            // We don't want to emit this lint if the `#[must_use]` attribute is already there.
-            if !cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::must_use));
             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.

tests/ui/return_self_not_must_use.rs

@@ -45,3 +45,13 @@ impl Whatever for Bar {
         self
     }
 }
+
+#[must_use]
+pub struct Foo;
+
+impl Foo {
+    // There should be no warning here! (`Foo` already implements `#[must_use]`)
+    fn foo(&self) -> Self {
+        Self
+    }
+}