From eed466281c8d0e3a920a2cde01a5b4b5c0df5f0e Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Thu, 1 Jun 2023 14:29:56 -0500 Subject: [PATCH] ignore `Foo::deref` altogether --- clippy_lints/src/dereference.rs | 47 ++++++++++---------------- tests/ui/explicit_deref_methods.fixed | 15 ++++---- tests/ui/explicit_deref_methods.rs | 3 +- tests/ui/explicit_deref_methods.stderr | 40 ++-------------------- 4 files changed, 30 insertions(+), 75 deletions(-) diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 96d1bcea6e5..93b6e2083b5 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -56,9 +56,11 @@ /// let b = &*a; /// ``` /// - /// This lint excludes: + /// This lint excludes all of: /// ```rust,ignore /// let _ = d.unwrap().deref(); + /// let _ = Foo::deref(&foo); + /// let _ = ::deref(&foo); /// ``` #[clippy::version = "1.44.0"] pub EXPLICIT_DEREF_METHODS, @@ -155,7 +157,7 @@ #[derive(Default)] pub struct Dereferencing<'tcx> { - state: Option<(State<'tcx>, StateData)>, + state: Option<(State, 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,13 +212,12 @@ struct DerefedBorrow { } #[derive(Debug)] -enum State<'tcx> { +enum State { // 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, }, @@ -313,16 +314,10 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { && 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, - call_args, + is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), target_mut, }, StateData { @@ -445,7 +440,6 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { ty_changed_count + 1 }, is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), - call_args: None, target_mut, }, data, @@ -1480,16 +1474,15 @@ 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; - let (expr_str, expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app); + let (expr_str, _expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app); let ty = cx.typeck_results().expr_ty(expr); let (_, ref_count) = peel_mid_ty_refs(ty); let deref_str = if ty_changed_count >= ref_count && ref_count != 0 { @@ -1512,23 +1505,19 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State<'_>, "&" }; - let mut expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX { - format!("({expr_str})") + // expr_str (the suggestion) is never shown if is_final_ufcs is true, since it's + // `expr.kind == ExprKind::Call`. Therefore, this is, afaik, always unnecessary. + /* + expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX { + Cow::Owned(format!("({expr_str})")) } else { - expr_str.into_owned() + expr_str }; + */ - // 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(); + // Fix #10850, do not lint if it's `Foo::deref` instead of `foo.deref()`. + if is_final_ufcs { + return; } span_lint_and_sugg( diff --git a/tests/ui/explicit_deref_methods.fixed b/tests/ui/explicit_deref_methods.fixed index 6cf7a28cafa..1710b170fb8 100644 --- a/tests/ui/explicit_deref_methods.fixed +++ b/tests/ui/explicit_deref_methods.fixed @@ -75,15 +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 + // make sure `Aaa::deref` instead of `aaa.deref()` is not linted, as well as fully qualified + // syntax - &*Aaa; - &mut *Aaa; - &*Aaa; - &mut *Aaa; + Aaa::deref(&Aaa); + Aaa::deref_mut(&mut Aaa); + ::deref(&Aaa); + ::deref_mut(&mut Aaa); let mut aaa = Aaa; - &*aaa; - &mut *aaa; + Aaa::deref(&aaa); + Aaa::deref_mut(&mut aaa); // following should not require linting diff --git a/tests/ui/explicit_deref_methods.rs b/tests/ui/explicit_deref_methods.rs index 77501fa6b6e..85147e1cb69 100644 --- a/tests/ui/explicit_deref_methods.rs +++ b/tests/ui/explicit_deref_methods.rs @@ -75,7 +75,8 @@ 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 + // make sure `Aaa::deref` instead of `aaa.deref()` is not linted, as well as fully qualified + // syntax Aaa::deref(&Aaa); Aaa::deref_mut(&mut Aaa); diff --git a/tests/ui/explicit_deref_methods.stderr b/tests/ui/explicit_deref_methods.stderr index 30ebcd47b2b..592563ffa8c 100644 --- a/tests/ui/explicit_deref_methods.stderr +++ b/tests/ui/explicit_deref_methods.stderr @@ -67,46 +67,10 @@ LL | let b = opt_a.unwrap().deref(); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&*opt_a.unwrap()` error: explicit `deref` method call - --> $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 + --> $DIR/explicit_deref_methods.rs:113:31 | LL | let b: &str = expr_deref!(a.deref()); | ^^^^^^^^^ help: try this: `&*a` -error: aborting due to 18 previous errors +error: aborting due to 12 previous errors