diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f1f73c1fd2..180e7d8bedc 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 0d3fc43a644..0c9ae6380d8 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/functions/missnamed_getters.rs b/clippy_lints/src/functions/missnamed_getters.rs new file mode 100644 index 00000000000..c522bb780b3 --- /dev/null +++ b/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) .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, + ); + } +} diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index ae0e0833446..726df02444f 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/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<'_>) { diff --git a/tests/ui/missnamed_getters.rs b/tests/ui/missnamed_getters.rs new file mode 100644 index 00000000000..b47f6edc5ba --- /dev/null +++ b/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 +} diff --git a/tests/ui/missnamed_getters.stderr b/tests/ui/missnamed_getters.stderr new file mode 100644 index 00000000000..8e31a42b97c --- /dev/null +++ b/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 +