From e443604a24029e0ffd77f917e82980539fdacbbe Mon Sep 17 00:00:00 2001
From: Robin Schroer <git@sulami.xyz>
Date: Fri, 6 Jan 2023 12:52:51 +0900
Subject: [PATCH] unused_self: Don't trigger if the method body contains
 todo!()

If the author is using todo!(), presumably they intend to use self at
some point later, so we don't have a good basis to recommend factoring
out to an associated function.

Fixes #10117.

changelog: Don't trigger [`unused_self`] if the method body contains a `todo!()` call
---
 clippy_lints/src/unused_self.rs | 19 ++++++++++++++++++-
 tests/ui/unused_self.rs         | 10 ++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/clippy_lints/src/unused_self.rs b/clippy_lints/src/unused_self.rs
index 18231b6a7e8..f864c520302 100644
--- a/clippy_lints/src/unused_self.rs
+++ b/clippy_lints/src/unused_self.rs
@@ -1,9 +1,11 @@
 use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::macros::root_macro_call_first_node;
 use clippy_utils::visitors::is_local_used;
 use if_chain::if_chain;
-use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind};
+use rustc_hir::{Body, Impl, ImplItem, ImplItemKind, ItemKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
+use std::ops::ControlFlow;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -57,6 +59,20 @@ impl<'tcx> LateLintPass<'tcx> for UnusedSelf {
         let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id()).def_id;
         let parent_item = cx.tcx.hir().expect_item(parent);
         let assoc_item = cx.tcx.associated_item(impl_item.owner_id);
+        let contains_todo = |cx, body: &'_ Body<'_>| -> bool {
+            clippy_utils::visitors::for_each_expr(body.value, |e| {
+                if let Some(macro_call) = root_macro_call_first_node(cx, e) {
+                    if cx.tcx.item_name(macro_call.def_id).as_str() == "todo" {
+                        ControlFlow::Break(())
+                    } else {
+                        ControlFlow::Continue(())
+                    }
+                } else {
+                    ControlFlow::Continue(())
+                }
+            })
+            .is_some()
+        };
         if_chain! {
             if let ItemKind::Impl(Impl { of_trait: None, .. }) = parent_item.kind;
             if assoc_item.fn_has_self_parameter;
@@ -65,6 +81,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedSelf {
             let body = cx.tcx.hir().body(*body_id);
             if let [self_param, ..] = body.params;
             if !is_local_used(cx, body, self_param.pat.hir_id);
+            if !contains_todo(cx, body);
             then {
                 span_lint_and_help(
                     cx,
diff --git a/tests/ui/unused_self.rs b/tests/ui/unused_self.rs
index 92e8e1dba69..55bd5607185 100644
--- a/tests/ui/unused_self.rs
+++ b/tests/ui/unused_self.rs
@@ -60,6 +60,16 @@ mod unused_self_allow {
         // shouldn't trigger for public methods
         pub fn unused_self_move(self) {}
     }
+
+    pub struct E;
+
+    impl E {
+        // shouldn't trigger if body contains todo!()
+        pub fn unused_self_todo(self) {
+            let x = 42;
+            todo!()
+        }
+    }
 }
 
 pub use unused_self_allow::D;