diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index b27ffe73ffd..96d1bcea6e5 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -155,7 +155,7 @@ impl_lint_pass!(Dereferencing<'_> => [ #[derive(Default)] pub struct Dereferencing<'tcx> { - state: Option<(State, StateData)>, + state: Option<(State<'tcx>, StateData)>, // While parsing a `deref` method call in ufcs form, the path to the function is itself an // expression. This is to store the id of that expression so it can be skipped when @@ -210,12 +210,13 @@ struct DerefedBorrow { } #[derive(Debug)] -enum State { +enum State<'tcx> { // Any number of deref method calls. DerefMethod { // The number of calls in a sequence which changed the referenced type ty_changed_count: usize, is_final_ufcs: bool, + call_args: Option<&'tcx [Expr<'tcx>]>, /// The required mutability target_mut: Mutability, }, @@ -312,10 +313,16 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { && position.lint_explicit_deref() => { let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr))); + let (is_final_ufcs, call_args) = if let ExprKind::Call(_, args) = expr.kind { + (true, Some(args)) + } else { + (false, None) + }; self.state = Some(( State::DerefMethod { ty_changed_count, - is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), + is_final_ufcs, + call_args, target_mut, }, StateData { @@ -438,6 +445,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { ty_changed_count + 1 }, is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), + call_args: None, target_mut, }, data, @@ -1472,11 +1480,12 @@ fn ty_contains_field(ty: Ty<'_>, name: Symbol) -> bool { } #[expect(clippy::needless_pass_by_value, clippy::too_many_lines)] -fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) { +fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State<'_>, data: StateData) { match state { State::DerefMethod { ty_changed_count, is_final_ufcs, + call_args, target_mut, } => { let mut app = Applicability::MachineApplicable; @@ -1503,12 +1512,25 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data "&" }; - let expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX { + let mut expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX { format!("({expr_str})") } else { expr_str.into_owned() }; + // Fix #10850, changes suggestion if it's `Foo::deref` instead of `foo.deref`. Since `Foo::deref` is + // a `Call` instead of a `MethodCall` this should catch all instances of this, even if it's fully + // qualified or whatnot. + if is_final_ufcs && let Some(args) = call_args { + // Remove ref if it's there + let arg = if let ExprKind::AddrOf(.., arg) = args[0].kind { + arg + } else { + &args[0] + }; + expr_str = snippet_with_applicability(cx, arg.span, "{ .. }", &mut app).to_string(); + } + span_lint_and_sugg( cx, EXPLICIT_DEREF_METHODS, diff --git a/tests/ui/explicit_deref_methods.fixed b/tests/ui/explicit_deref_methods.fixed index 60482c66da7..6cf7a28cafa 100644 --- a/tests/ui/explicit_deref_methods.fixed +++ b/tests/ui/explicit_deref_methods.fixed @@ -1,11 +1,12 @@ //@run-rustfix #![warn(clippy::explicit_deref_methods)] -#![allow(unused_variables)] +#![allow(unused_variables, unused_must_use)] #![allow( clippy::borrow_deref_ref, suspicious_double_ref_op, clippy::explicit_auto_deref, clippy::needless_borrow, + clippy::no_effect, clippy::uninlined_format_args )] @@ -28,6 +29,22 @@ impl Deref for CustomVec { } } +struct Aaa; + +impl Deref for Aaa { + type Target = (); + + fn deref(&self) -> &Self::Target { + todo!(); + } +} + +impl DerefMut for Aaa { + fn deref_mut(&mut self) -> &mut Self::Target { + todo!(); + } +} + fn main() { let a: &mut String = &mut String::from("foo"); @@ -58,6 +75,16 @@ fn main() { let opt_a = Some(a.clone()); let b = &*opt_a.unwrap(); + // make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax + + &*Aaa; + &mut *Aaa; + &*Aaa; + &mut *Aaa; + let mut aaa = Aaa; + &*aaa; + &mut *aaa; + // following should not require linting let cv = CustomVec(vec![0, 42]); diff --git a/tests/ui/explicit_deref_methods.rs b/tests/ui/explicit_deref_methods.rs index e3613e216bb..77501fa6b6e 100644 --- a/tests/ui/explicit_deref_methods.rs +++ b/tests/ui/explicit_deref_methods.rs @@ -1,11 +1,12 @@ //@run-rustfix #![warn(clippy::explicit_deref_methods)] -#![allow(unused_variables)] +#![allow(unused_variables, unused_must_use)] #![allow( clippy::borrow_deref_ref, suspicious_double_ref_op, clippy::explicit_auto_deref, clippy::needless_borrow, + clippy::no_effect, clippy::uninlined_format_args )] @@ -28,6 +29,22 @@ impl Deref for CustomVec { } } +struct Aaa; + +impl Deref for Aaa { + type Target = (); + + fn deref(&self) -> &Self::Target { + todo!(); + } +} + +impl DerefMut for Aaa { + fn deref_mut(&mut self) -> &mut Self::Target { + todo!(); + } +} + fn main() { let a: &mut String = &mut String::from("foo"); @@ -58,6 +75,16 @@ fn main() { let opt_a = Some(a.clone()); let b = opt_a.unwrap().deref(); + // make sure `Aaa::deref` instead of `aaa.deref()` works as well as fully qualified syntax + + Aaa::deref(&Aaa); + Aaa::deref_mut(&mut Aaa); + ::deref(&Aaa); + ::deref_mut(&mut Aaa); + let mut aaa = Aaa; + Aaa::deref(&aaa); + Aaa::deref_mut(&mut aaa); + // following should not require linting let cv = CustomVec(vec![0, 42]); diff --git a/tests/ui/explicit_deref_methods.stderr b/tests/ui/explicit_deref_methods.stderr index 4b10ed1377b..30ebcd47b2b 100644 --- a/tests/ui/explicit_deref_methods.stderr +++ b/tests/ui/explicit_deref_methods.stderr @@ -1,5 +1,5 @@ error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:36:19 + --> $DIR/explicit_deref_methods.rs:53:19 | LL | let b: &str = a.deref(); | ^^^^^^^^^ help: try this: `&*a` @@ -7,70 +7,106 @@ LL | let b: &str = a.deref(); = note: `-D clippy::explicit-deref-methods` implied by `-D warnings` error: explicit `deref_mut` method call - --> $DIR/explicit_deref_methods.rs:38:23 + --> $DIR/explicit_deref_methods.rs:55:23 | LL | let b: &mut str = a.deref_mut(); | ^^^^^^^^^^^^^ help: try this: `&mut **a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:41:39 + --> $DIR/explicit_deref_methods.rs:58:39 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:41:50 + --> $DIR/explicit_deref_methods.rs:58:50 | LL | let b: String = format!("{}, {}", a.deref(), a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:43:20 + --> $DIR/explicit_deref_methods.rs:60:20 | LL | println!("{}", a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:46:11 + --> $DIR/explicit_deref_methods.rs:63:11 | LL | match a.deref() { | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:50:28 + --> $DIR/explicit_deref_methods.rs:67:28 | LL | let b: String = concat(a.deref()); | ^^^^^^^^^ help: try this: `&*a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:52:13 + --> $DIR/explicit_deref_methods.rs:69:13 | LL | let b = just_return(a).deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:54:28 + --> $DIR/explicit_deref_methods.rs:71:28 | LL | let b: String = concat(just_return(a).deref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `just_return(a)` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:56:19 + --> $DIR/explicit_deref_methods.rs:73:19 | LL | let b: &str = a.deref().deref(); | ^^^^^^^^^^^^^^^^^ help: try this: `&**a` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:59:13 + --> $DIR/explicit_deref_methods.rs:76:13 | LL | let b = opt_a.unwrap().deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` error: explicit `deref` method call - --> $DIR/explicit_deref_methods.rs:85:31 + --> $DIR/explicit_deref_methods.rs:80:5 + | +LL | Aaa::deref(&Aaa); + | ^^^^^^^^^^^^^^^^ help: try this: `&*Aaa` + +error: explicit `deref_mut` method call + --> $DIR/explicit_deref_methods.rs:81:5 + | +LL | Aaa::deref_mut(&mut Aaa); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *Aaa` + +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:82:5 + | +LL | ::deref(&Aaa); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*Aaa` + +error: explicit `deref_mut` method call + --> $DIR/explicit_deref_methods.rs:83:5 + | +LL | ::deref_mut(&mut Aaa); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *Aaa` + +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:85:5 + | +LL | Aaa::deref(&aaa); + | ^^^^^^^^^^^^^^^^ help: try this: `&*aaa` + +error: explicit `deref_mut` method call + --> $DIR/explicit_deref_methods.rs:86:5 + | +LL | Aaa::deref_mut(&mut aaa); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut *aaa` + +error: explicit `deref` method call + --> $DIR/explicit_deref_methods.rs:112:31 | LL | let b: &str = expr_deref!(a.deref()); | ^^^^^^^^^ help: try this: `&*a` -error: aborting due to 12 previous errors +error: aborting due to 18 previous errors