Add missnamed_getters lint

Author: sgued

CHANGELOG.md

@@ -4198,6 +4198,7 @@ Released 2018-09-13
 [`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
 [`missing_spin_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_spin_loop
 [`missing_trait_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_trait_methods
+[`missnamed_getters`]: https://rust-lang.github.io/rust-clippy/master/index.html#missnamed_getters
 [`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
 [`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
 [`mixed_read_write_in_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_read_write_in_expression

clippy_lints/src/declared_lints.rs

@@ -184,6 +184,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::functions::RESULT_UNIT_ERR_INFO,
     crate::functions::TOO_MANY_ARGUMENTS_INFO,
     crate::functions::TOO_MANY_LINES_INFO,
+    crate::functions::MISSNAMED_GETTERS_INFO,
     crate::future_not_send::FUTURE_NOT_SEND_INFO,
     crate::if_let_mutex::IF_LET_MUTEX_INFO,
     crate::if_not_else::IF_NOT_ELSE_INFO,

clippy_lints/src/functions/missnamed_getters.rs

@@ -0,0 +1,123 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use rustc_errors::Applicability;
+use rustc_hir::{intravisit::FnKind, Body, ExprKind, FnDecl, HirId, ImplicitSelfKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use rustc_span::Span;
+
+use super::MISSNAMED_GETTERS;
+
+pub fn check_fn(
+    cx: &LateContext<'_>,
+    kind: FnKind<'_>,
+    decl: &FnDecl<'_>,
+    body: &Body<'_>,
+    _span: Span,
+    _hir_id: HirId,
+) {
+    let FnKind::Method(ref ident, sig) = kind else {
+            return;
+        };
+
+    // Takes only &(mut) self
+    if decl.inputs.len() != 1 {
+        return;
+    }
+
+    let name = ident.name.as_str();
+
+    let name = match sig.decl.implicit_self {
+        ImplicitSelfKind::ImmRef => name,
+        ImplicitSelfKind::MutRef => {
+            let Some(name) = name.strip_suffix("_mut") else {
+                    return;
+                };
+            name
+        },
+        _ => return,
+    };
+
+    // Body must be &(mut) <self_data>.name
+    // self_data is not neccessarilly self
+    let (self_data, used_ident, span) = if_chain! {
+        if let ExprKind::Block(block,_) = body.value.kind;
+        if block.stmts.is_empty();
+        if let Some(block_expr) = block.expr;
+        // replace with while for as many addrof needed
+        if let ExprKind::AddrOf(_,_, expr) = block_expr.kind;
+        if let ExprKind::Field(self_data, ident) = expr.kind;
+        if ident.name.as_str() != name;
+        then {
+            (self_data,ident,block_expr.span)
+        } else {
+            return;
+        }
+    };
+
+    let ty = cx.typeck_results().expr_ty(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 variants = def.variants();
+
+    // We're accessing a field, so it should be an union or a struct and have one and only one variant
+    if variants.len() != 1 {
+        if cfg!(debug_assertions) {
+            panic!("Struct or union expected to have only one variant");
+        } else {
+            // Don't ICE when possible
+            return;
+        }
+    }
+
+    let first = variants.last().unwrap();
+    let fields = &variants[first];
+
+    let mut used_field = None;
+    let mut correct_field = None;
+    for f in &fields.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 {
+            if cfg!(debug_assertions) {
+                panic!("Struct doesn't contain the correct field");
+            } else {
+                // Don't ICE when possible
+                return;
+            }
+        };
+    let Some(correct_field) = correct_field else {
+            return;
+        };
+
+    if cx.tcx.type_of(used_field.did) == cx.tcx.type_of(correct_field.did) {
+        let snippet = snippet(cx, span, "..");
+        let sugg = format!("{}{name}", snippet.strip_suffix(used_field.name.as_str()).unwrap());
+        span_lint_and_sugg(
+            cx,
+            MISSNAMED_GETTERS,
+            span,
+            "getter function appears to return the wrong field",
+            "consider using",
+            sugg,
+            Applicability::MaybeIncorrect,
+        );
+    }
+}

clippy_lints/src/functions/mod.rs

@@ -1,3 +1,4 @@
+mod missnamed_getters;
 mod must_use;
 mod not_unsafe_ptr_arg_deref;
 mod result;
@@ -260,6 +261,25 @@ declare_clippy_lint! {
     "function returning `Result` with large `Err` type"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// ### Why is this bad?
+    ///
+    /// ### Example
+    /// ```rust
+    /// // example code where clippy issues a warning
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// // example code which does not raise clippy warning
+    /// ```
+    #[clippy::version = "1.66.0"]
+    pub MISSNAMED_GETTERS,
+    suspicious,
+    "default lint description"
+}
+
 #[derive(Copy, Clone)]
 pub struct Functions {
     too_many_arguments_threshold: u64,
@@ -286,6 +306,7 @@ impl_lint_pass!(Functions => [
     MUST_USE_CANDIDATE,
     RESULT_UNIT_ERR,
     RESULT_LARGE_ERR,
+    MISSNAMED_GETTERS,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Functions {
@@ -301,6 +322,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions {
         too_many_arguments::check_fn(cx, kind, decl, span, hir_id, self.too_many_arguments_threshold);
         too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold);
         not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, hir_id);
+        missnamed_getters::check_fn(cx, kind, decl, body, span, hir_id);
     }
 
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {

tests/ui/missnamed_getters.rs

@@ -0,0 +1,28 @@
+#![allow(unused)]
+#![warn(clippy::missnamed_getters)]
+
+struct A {
+    a: u8,
+    b: u8,
+}
+
+impl A {
+    fn a(&self) -> &u8 {
+        &self.b
+    }
+}
+
+union B {
+    a: u8,
+    b: u8,
+}
+
+impl B {
+    unsafe fn a(&self) -> &u8 {
+        &self.b
+    }
+}
+
+fn main() {
+    // test code goes here
+}

tests/ui/missnamed_getters.stderr

@@ -0,0 +1,16 @@
+error: getter function appears to return the wrong field
+  --> $DIR/missnamed_getters.rs:11:9
+   |
+LL |         &self.b
+   |         ^^^^^^^ help: consider using: `&self.a`
+   |
+   = note: `-D clippy::missnamed-getters` implied by `-D warnings`
+
+error: getter function appears to return the wrong field
+  --> $DIR/missnamed_getters.rs:22:9
+   |
+LL |         &self.b
+   |         ^^^^^^^ help: consider using: `&self.a`
+
+error: aborting due to 2 previous errors
+