From 9891af348c01f8cf14ab1e10754faccad04fcaa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 1 Nov 2022 19:07:51 +0100 Subject: [PATCH] missnamed_getters: Match owned methods --- .../src/functions/missnamed_getters.rs | 27 +++++++--- tests/ui/missnamed_getters.rs | 39 ++++++++++++++ tests/ui/missnamed_getters.stderr | 54 +++++++++++++++++-- 3 files changed, 110 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/functions/missnamed_getters.rs b/clippy_lints/src/functions/missnamed_getters.rs index c522bb780..1f4eefc62 100644 --- a/clippy_lints/src/functions/missnamed_getters.rs +++ b/clippy_lints/src/functions/missnamed_getters.rs @@ -35,21 +35,34 @@ pub fn check_fn( }; name }, + ImplicitSelfKind::Imm | ImplicitSelfKind::Mut => name, _ => return, }; // Body must be &(mut) .name - // self_data is not neccessarilly self - let (self_data, used_ident, span) = if_chain! { + // self_data is not neccessarilly self, to also lint sub-getters, etc… + + let block_expr = 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; + then { + block_expr + } else { + return; + } + }; + let expr_span = block_expr.span; + + let mut expr = block_expr; + if let ExprKind::AddrOf(_, _, tmp) = expr.kind { + expr = tmp; + } + let (self_data, used_ident) = if_chain! { if let ExprKind::Field(self_data, ident) = expr.kind; if ident.name.as_str() != name; then { - (self_data,ident,block_expr.span) + (self_data,ident) } else { return; } @@ -108,12 +121,12 @@ pub fn check_fn( }; if cx.tcx.type_of(used_field.did) == cx.tcx.type_of(correct_field.did) { - let snippet = snippet(cx, span, ".."); + let snippet = snippet(cx, expr_span, ".."); let sugg = format!("{}{name}", snippet.strip_suffix(used_field.name.as_str()).unwrap()); span_lint_and_sugg( cx, MISSNAMED_GETTERS, - span, + expr_span, "getter function appears to return the wrong field", "consider using", sugg, diff --git a/tests/ui/missnamed_getters.rs b/tests/ui/missnamed_getters.rs index b47f6edc5..f9c2351f8 100644 --- a/tests/ui/missnamed_getters.rs +++ b/tests/ui/missnamed_getters.rs @@ -4,12 +4,32 @@ struct A { a: u8, b: u8, + c: u8, } impl A { fn a(&self) -> &u8 { &self.b } + fn a_mut(&mut self) -> &mut u8 { + &mut self.b + } + + fn b(self) -> u8 { + self.a + } + + fn b_mut(&mut self) -> &mut u8 { + &mut self.a + } + + fn c(&self) -> &u8 { + &self.b + } + + fn c_mut(&mut self) -> &mut u8 { + &mut self.a + } } union B { @@ -21,6 +41,25 @@ impl B { unsafe fn a(&self) -> &u8 { &self.b } + unsafe fn a_mut(&mut self) -> &mut u8 { + &mut self.b + } + + unsafe fn b(self) -> u8 { + self.a + } + + unsafe fn b_mut(&mut self) -> &mut u8 { + &mut self.a + } + + unsafe fn c(&self) -> &u8 { + &self.b + } + + unsafe fn c_mut(&mut self) -> &mut u8 { + &mut self.a + } } fn main() { diff --git a/tests/ui/missnamed_getters.stderr b/tests/ui/missnamed_getters.stderr index 8e31a42b9..276096ade 100644 --- a/tests/ui/missnamed_getters.stderr +++ b/tests/ui/missnamed_getters.stderr @@ -1,5 +1,5 @@ error: getter function appears to return the wrong field - --> $DIR/missnamed_getters.rs:11:9 + --> $DIR/missnamed_getters.rs:12:9 | LL | &self.b | ^^^^^^^ help: consider using: `&self.a` @@ -7,10 +7,58 @@ LL | &self.b = note: `-D clippy::missnamed-getters` implied by `-D warnings` error: getter function appears to return the wrong field - --> $DIR/missnamed_getters.rs:22:9 + --> $DIR/missnamed_getters.rs:15:9 + | +LL | &mut self.b + | ^^^^^^^^^^^ help: consider using: `&mut self.a` + +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:19:9 + | +LL | self.a + | ^^^^^^ help: consider using: `self.b` + +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:23:9 + | +LL | &mut self.a + | ^^^^^^^^^^^ help: consider using: `&mut self.b` + +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:27:9 + | +LL | &self.b + | ^^^^^^^ help: consider using: `&self.c` + +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:31:9 + | +LL | &mut self.a + | ^^^^^^^^^^^ help: consider using: `&mut self.c` + +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:42:9 | LL | &self.b | ^^^^^^^ help: consider using: `&self.a` -error: aborting due to 2 previous errors +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:45:9 + | +LL | &mut self.b + | ^^^^^^^^^^^ help: consider using: `&mut self.a` + +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:49:9 + | +LL | self.a + | ^^^^^^ help: consider using: `self.b` + +error: getter function appears to return the wrong field + --> $DIR/missnamed_getters.rs:53:9 + | +LL | &mut self.a + | ^^^^^^^^^^^ help: consider using: `&mut self.b` + +error: aborting due to 10 previous errors