From 07a00efe61fcb1e52103c2acb6f921ced6d11749 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume.gomez@huawei.com>
Date: Sun, 19 Dec 2021 15:10:36 +0100
Subject: [PATCH] Don't emit RETURN_SELF_NOT_MUST_USE lint if `Self` already is
 marked as `#[must_use]`

---
 clippy_lints/src/return_self_not_must_use.rs | 11 ++++++-----
 tests/ui/return_self_not_must_use.rs         | 10 ++++++++++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/clippy_lints/src/return_self_not_must_use.rs b/clippy_lints/src/return_self_not_must_use.rs
index 2ff7f6bbb38..b57ec96bc7e 100644
--- a/clippy_lints/src/return_self_not_must_use.rs
+++ b/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,9 +51,9 @@ 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.
@@ -60,6 +61,8 @@ fn check_method(cx: &LateContext<'tcx>, decl: &'tcx FnDecl<'tcx>, fn_def: LocalD
         // 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 @@ fn check_fn(
             // 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.
diff --git a/tests/ui/return_self_not_must_use.rs b/tests/ui/return_self_not_must_use.rs
index 2fa389dec0a..7dd5742dae9 100644
--- a/tests/ui/return_self_not_must_use.rs
+++ b/tests/ui/return_self_not_must_use.rs
@@ -45,3 +45,13 @@ fn what2(&self) -> &Self {
         self
     }
 }
+
+#[must_use]
+pub struct Foo;
+
+impl Foo {
+    // There should be no warning here! (`Foo` already implements `#[must_use]`)
+    fn foo(&self) -> Self {
+        Self
+    }
+}