From 1f2f50c34eb304ce56213dad70ccdeb2f2901512 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= <sosthene@guedon.gdn>
Date: Sun, 20 Nov 2022 17:03:53 +0100
Subject: [PATCH] Fix many false negatives caused by autoderef

---
 .../src/functions/misnamed_getters.rs         | 44 +++++++++----------
 tests/ui/misnamed_getters.rs                  | 36 ++++++++++++---
 tests/ui/misnamed_getters.stderr              | 36 +++++++++++----
 3 files changed, 77 insertions(+), 39 deletions(-)

diff --git a/clippy_lints/src/functions/misnamed_getters.rs b/clippy_lints/src/functions/misnamed_getters.rs
index 5424c0eecfe..27acad45ccf 100644
--- a/clippy_lints/src/functions/misnamed_getters.rs
+++ b/clippy_lints/src/functions/misnamed_getters.rs
@@ -6,6 +6,8 @@
 use rustc_middle::ty;
 use rustc_span::Span;
 
+use std::iter;
+
 use super::MISNAMED_GETTERS;
 
 pub fn check_fn(
@@ -75,39 +77,35 @@ pub fn check_fn(
         }
     };
 
-    let ty = cx.typeck_results().expr_ty_adjusted(self_data);
-
-    let def = {
-        let mut kind = ty.kind();
-        loop {
-            match kind {
-                ty::Adt(def, _) => break def,
-                ty::Ref(_, ty, _) => kind = ty.kind(),
-                // We don't do tuples because the function name cannot be a number
-                _ => return,
-            }
-        }
-    };
-
     let mut used_field = None;
     let mut correct_field = None;
-    for f in def.all_fields() {
-        if f.name.as_str() == name {
-            correct_field = Some(f);
-        }
-        if f.name == used_ident.name {
-            used_field = Some(f);
+    let typeck_results = cx.typeck_results();
+    for adjusted_type in iter::once(typeck_results.expr_ty(self_data))
+        .chain(typeck_results.expr_adjustments(self_data).iter().map(|adj| adj.target))
+    {
+        let ty::Adt(def,_) = adjusted_type.kind() else {
+            continue;
+        };
+
+        for f in def.all_fields() {
+            if f.name.as_str() == name {
+                correct_field = Some(f);
+            }
+            if f.name == used_ident.name {
+                used_field = Some(f);
+            }
         }
     }
 
     let Some(used_field) = used_field else {
-        // FIXME: This can be reached if the field access uses autoderef.
-        // `dec.all_fields()` should be replaced by something that uses autoderef on the unajusted type of `self_data`
+        // Can happen if the field access is a tuple. We don't lint those because the getter name could not start with a number.
         return;
     };
 
     let Some(correct_field) = correct_field else {
-            return;
+        // There is no field corresponding to the getter name.
+        // FIXME: This can be a false positive if the correct field is reachable trought deeper autodereferences than used_field is
+        return;
     };
 
     if cx.tcx.type_of(used_field.did) == cx.tcx.type_of(correct_field.did) {
diff --git a/tests/ui/misnamed_getters.rs b/tests/ui/misnamed_getters.rs
index 2c451bd3a16..03e7dac7df9 100644
--- a/tests/ui/misnamed_getters.rs
+++ b/tests/ui/misnamed_getters.rs
@@ -85,15 +85,37 @@ unsafe fn c_unchecked_mut(&mut self) -> &mut u8 {
     }
 }
 
-struct C {
-    inner: Box<A>,
+struct D {
+    d: u8,
+    inner: A,
 }
-impl C {
-    unsafe fn a(&self) -> &u8 {
-        &self.inner.b
+
+impl core::ops::Deref for D {
+    type Target = A;
+    fn deref(&self) -> &A {
+        &self.inner
     }
-    unsafe fn a_mut(&mut self) -> &mut u8 {
-        &mut self.inner.b
+}
+
+impl core::ops::DerefMut for D {
+    fn deref_mut(&mut self) -> &mut A {
+        &mut self.inner
+    }
+}
+
+impl D {
+    fn a(&self) -> &u8 {
+        &self.b
+    }
+    fn a_mut(&mut self) -> &mut u8 {
+        &mut self.b
+    }
+
+    fn d(&self) -> &u8 {
+        &self.b
+    }
+    fn d_mut(&mut self) -> &mut u8 {
+        &mut self.b
     }
 }
 
diff --git a/tests/ui/misnamed_getters.stderr b/tests/ui/misnamed_getters.stderr
index 5210295893e..1e38a83d019 100644
--- a/tests/ui/misnamed_getters.stderr
+++ b/tests/ui/misnamed_getters.stderr
@@ -127,22 +127,40 @@ LL | |     }
    | |_____^
 
 error: getter function appears to return the wrong field
-  --> $DIR/misnamed_getters.rs:92:5
+  --> $DIR/misnamed_getters.rs:107:5
    |
-LL | /     unsafe fn a(&self) -> &u8 {
-LL | |         &self.inner.b
-   | |         ------------- help: consider using: `&self.inner.a`
+LL | /     fn a(&self) -> &u8 {
+LL | |         &self.b
+   | |         ------- help: consider using: `&self.a`
 LL | |     }
    | |_____^
 
 error: getter function appears to return the wrong field
-  --> $DIR/misnamed_getters.rs:95:5
+  --> $DIR/misnamed_getters.rs:110:5
    |
-LL | /     unsafe fn a_mut(&mut self) -> &mut u8 {
-LL | |         &mut self.inner.b
-   | |         ----------------- help: consider using: `&mut self.inner.a`
+LL | /     fn a_mut(&mut self) -> &mut u8 {
+LL | |         &mut self.b
+   | |         ----------- help: consider using: `&mut self.a`
 LL | |     }
    | |_____^
 
-error: aborting due to 16 previous errors
+error: getter function appears to return the wrong field
+  --> $DIR/misnamed_getters.rs:114:5
+   |
+LL | /     fn d(&self) -> &u8 {
+LL | |         &self.b
+   | |         ------- help: consider using: `&self.d`
+LL | |     }
+   | |_____^
+
+error: getter function appears to return the wrong field
+  --> $DIR/misnamed_getters.rs:117:5
+   |
+LL | /     fn d_mut(&mut self) -> &mut u8 {
+LL | |         &mut self.b
+   | |         ----------- help: consider using: `&mut self.d`
+LL | |     }
+   | |_____^
+
+error: aborting due to 18 previous errors