From 65bc6cb8bf1f1005ae02a0044f0f3027c702a807 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 28 Jan 2022 14:54:28 -0500 Subject: [PATCH] Lint `explicit_auto_deref` without a leading borrow --- clippy_lints/src/dereference.rs | 29 +++++++++++++++++++--- tests/ui/explicit_auto_deref.fixed | 14 +++++++++++ tests/ui/explicit_auto_deref.rs | 14 +++++++++++ tests/ui/explicit_auto_deref.stderr | 14 ++++++++++- tests/ui/needless_borrow_pat.rs | 2 +- tests/ui/ref_binding_to_reference.rs | 2 +- tests/ui/search_is_some_fixable_none.fixed | 2 +- tests/ui/search_is_some_fixable_none.rs | 2 +- tests/ui/search_is_some_fixable_some.fixed | 2 +- tests/ui/search_is_some_fixable_some.rs | 2 +- 10 files changed, 72 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 12c796bd100..32d1e26b74c 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -246,6 +246,8 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { (None, kind) => { let expr_ty = typeck.expr_ty(expr); let (position, parent_ctxt) = get_expr_position(cx, expr); + let (stability, adjustments) = walk_parents(cx, expr); + match kind { RefOp::Deref => { if let Position::FieldAccess(name) = position @@ -255,6 +257,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { State::ExplicitDerefField { name }, StateData { span: expr.span, hir_id: expr.hir_id }, )); + } else if stability.is_deref_stable() { + self.state = Some(( + State::ExplicitDeref { deref_span: expr.span, deref_hir_id: expr.hir_id }, + StateData { span: expr.span, hir_id: expr.hir_id }, + )); } } RefOp::Method(target_mut) @@ -278,7 +285,6 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { )); }, RefOp::AddrOf => { - let (stability, adjustments) = walk_parents(cx, expr); // Find the number of times the borrow is auto-derefed. let mut iter = adjustments.iter(); let mut deref_count = 0usize; @@ -632,6 +638,7 @@ impl AutoDerefStability { /// Walks up the parent expressions attempting to determine both how stable the auto-deref result /// is, and which adjustments will be applied to it. Note this will not consider auto-borrow /// locations as those follow different rules. +#[allow(clippy::too_many_lines)] fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (AutoDerefStability, &'tcx [Adjustment<'tcx>]) { let mut adjustments = [].as_slice(); let stability = walk_to_expr_usage(cx, e, &mut |node, child_id| { @@ -643,16 +650,26 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (AutoDerefSt Node::Local(Local { ty: Some(ty), .. }) => Some(binding_ty_auto_deref_stability(ty)), Node::Item(&Item { kind: ItemKind::Static(..) | ItemKind::Const(..), + def_id, .. }) | Node::TraitItem(&TraitItem { kind: TraitItemKind::Const(..), + def_id, .. }) | Node::ImplItem(&ImplItem { kind: ImplItemKind::Const(..), + def_id, .. - }) => Some(AutoDerefStability::Deref), + }) => { + let ty = cx.tcx.type_of(def_id); + Some(if ty.is_ref() { + AutoDerefStability::None + } else { + AutoDerefStability::Deref + }) + }, Node::Item(&Item { kind: ItemKind::Fn(..), @@ -670,7 +687,9 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (AutoDerefSt .. }) => { let output = cx.tcx.fn_sig(def_id.to_def_id()).skip_binder().output(); - Some(if output.has_placeholders() || output.has_opaque_types() { + Some(if !output.is_ref() { + AutoDerefStability::None + } else if output.has_placeholders() || output.has_opaque_types() { AutoDerefStability::Reborrow } else { AutoDerefStability::Deref @@ -684,7 +703,9 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (AutoDerefSt .fn_sig(cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap())) .skip_binder() .output(); - Some(if output.has_placeholders() || output.has_opaque_types() { + Some(if !output.is_ref() { + AutoDerefStability::None + } else if output.has_placeholders() || output.has_opaque_types() { AutoDerefStability::Reborrow } else { AutoDerefStability::Deref diff --git a/tests/ui/explicit_auto_deref.fixed b/tests/ui/explicit_auto_deref.fixed index 91d0379b70e..d20a58e53bd 100644 --- a/tests/ui/explicit_auto_deref.fixed +++ b/tests/ui/explicit_auto_deref.fixed @@ -6,6 +6,7 @@ unused_braces, clippy::borrowed_box, clippy::needless_borrow, + clippy::needless_return, clippy::ptr_arg, clippy::redundant_field_names, clippy::too_many_arguments, @@ -184,4 +185,17 @@ fn main() { } let s6 = S6 { foo: S5 { foo: 5 } }; let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` + + let ref_str = &"foo"; + let _ = f_str(ref_str); + let ref_ref_str = &ref_str; + let _ = f_str(ref_ref_str); + + fn _f5(x: &u32) -> u32 { + if true { + *x + } else { + return *x; + } + } } diff --git a/tests/ui/explicit_auto_deref.rs b/tests/ui/explicit_auto_deref.rs index e57553b2a99..f49bc8bb9f0 100644 --- a/tests/ui/explicit_auto_deref.rs +++ b/tests/ui/explicit_auto_deref.rs @@ -6,6 +6,7 @@ unused_braces, clippy::borrowed_box, clippy::needless_borrow, + clippy::needless_return, clippy::ptr_arg, clippy::redundant_field_names, clippy::too_many_arguments, @@ -184,4 +185,17 @@ fn main() { } let s6 = S6 { foo: S5 { foo: 5 } }; let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` + + let ref_str = &"foo"; + let _ = f_str(*ref_str); + let ref_ref_str = &ref_str; + let _ = f_str(**ref_ref_str); + + fn _f5(x: &u32) -> u32 { + if true { + *x + } else { + return *x; + } + } } diff --git a/tests/ui/explicit_auto_deref.stderr b/tests/ui/explicit_auto_deref.stderr index 54f1a2cd886..de8c40ce5be 100644 --- a/tests/ui/explicit_auto_deref.stderr +++ b/tests/ui/explicit_auto_deref.stderr @@ -168,5 +168,17 @@ error: deref which would be done by auto-deref LL | let _ = (**b).foo; | ^^^^^ help: try this: `b` -error: aborting due to 28 previous errors +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:189:19 + | +LL | let _ = f_str(*ref_str); + | ^^^^^^^^ help: try this: `ref_str` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:191:19 + | +LL | let _ = f_str(**ref_ref_str); + | ^^^^^^^^^^^^^ help: try this: `ref_ref_str` + +error: aborting due to 30 previous errors diff --git a/tests/ui/needless_borrow_pat.rs b/tests/ui/needless_borrow_pat.rs index 04b6283da3c..222e8e61799 100644 --- a/tests/ui/needless_borrow_pat.rs +++ b/tests/ui/needless_borrow_pat.rs @@ -1,7 +1,7 @@ // FIXME: run-rustfix waiting on multi-span suggestions #![warn(clippy::needless_borrow)] -#![allow(clippy::needless_borrowed_reference)] +#![allow(clippy::needless_borrowed_reference, clippy::explicit_auto_deref)] fn f1(_: &str) {} macro_rules! m1 { diff --git a/tests/ui/ref_binding_to_reference.rs b/tests/ui/ref_binding_to_reference.rs index 570ef406e4a..c8d0e56b197 100644 --- a/tests/ui/ref_binding_to_reference.rs +++ b/tests/ui/ref_binding_to_reference.rs @@ -2,7 +2,7 @@ #![feature(lint_reasons)] #![warn(clippy::ref_binding_to_reference)] -#![allow(clippy::needless_borrowed_reference)] +#![allow(clippy::needless_borrowed_reference, clippy::explicit_auto_deref)] fn f1(_: &str) {} macro_rules! m2 { diff --git a/tests/ui/search_is_some_fixable_none.fixed b/tests/ui/search_is_some_fixable_none.fixed index 6831fb2cf59..5190c5304c7 100644 --- a/tests/ui/search_is_some_fixable_none.fixed +++ b/tests/ui/search_is_some_fixable_none.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_none.rs b/tests/ui/search_is_some_fixable_none.rs index 767518ab0c0..310d87333a9 100644 --- a/tests/ui/search_is_some_fixable_none.rs +++ b/tests/ui/search_is_some_fixable_none.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_some.fixed b/tests/ui/search_is_some_fixable_some.fixed index 7c940a2b069..5a2aee465d1 100644 --- a/tests/ui/search_is_some_fixable_some.fixed +++ b/tests/ui/search_is_some_fixable_some.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_some.rs b/tests/ui/search_is_some_fixable_some.rs index 77fd52e4ce7..0e98ae18a21 100644 --- a/tests/ui/search_is_some_fixable_some.rs +++ b/tests/ui/search_is_some_fixable_some.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code)] +#![allow(dead_code, clippy::explicit_auto_deref)] #![warn(clippy::search_is_some)] fn main() {