Lint explicit_auto_deref without a leading borrow

This commit is contained in:
Jason Newcomb 2022-01-28 14:54:28 -05:00
parent 442a68c64b
commit 65bc6cb8bf
10 changed files with 72 additions and 11 deletions

View File

@ -246,6 +246,8 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
(None, kind) => { (None, kind) => {
let expr_ty = typeck.expr_ty(expr); let expr_ty = typeck.expr_ty(expr);
let (position, parent_ctxt) = get_expr_position(cx, expr); let (position, parent_ctxt) = get_expr_position(cx, expr);
let (stability, adjustments) = walk_parents(cx, expr);
match kind { match kind {
RefOp::Deref => { RefOp::Deref => {
if let Position::FieldAccess(name) = position if let Position::FieldAccess(name) = position
@ -255,6 +257,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
State::ExplicitDerefField { name }, State::ExplicitDerefField { name },
StateData { span: expr.span, hir_id: expr.hir_id }, 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) RefOp::Method(target_mut)
@ -278,7 +285,6 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
)); ));
}, },
RefOp::AddrOf => { RefOp::AddrOf => {
let (stability, adjustments) = walk_parents(cx, expr);
// Find the number of times the borrow is auto-derefed. // Find the number of times the borrow is auto-derefed.
let mut iter = adjustments.iter(); let mut iter = adjustments.iter();
let mut deref_count = 0usize; 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 /// 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 /// is, and which adjustments will be applied to it. Note this will not consider auto-borrow
/// locations as those follow different rules. /// 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>]) { fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (AutoDerefStability, &'tcx [Adjustment<'tcx>]) {
let mut adjustments = [].as_slice(); let mut adjustments = [].as_slice();
let stability = walk_to_expr_usage(cx, e, &mut |node, child_id| { 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::Local(Local { ty: Some(ty), .. }) => Some(binding_ty_auto_deref_stability(ty)),
Node::Item(&Item { Node::Item(&Item {
kind: ItemKind::Static(..) | ItemKind::Const(..), kind: ItemKind::Static(..) | ItemKind::Const(..),
def_id,
.. ..
}) })
| Node::TraitItem(&TraitItem { | Node::TraitItem(&TraitItem {
kind: TraitItemKind::Const(..), kind: TraitItemKind::Const(..),
def_id,
.. ..
}) })
| Node::ImplItem(&ImplItem { | Node::ImplItem(&ImplItem {
kind: ImplItemKind::Const(..), 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 { Node::Item(&Item {
kind: ItemKind::Fn(..), 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(); 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 AutoDerefStability::Reborrow
} else { } else {
AutoDerefStability::Deref 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())) .fn_sig(cx.tcx.hir().body_owner_def_id(cx.enclosing_body.unwrap()))
.skip_binder() .skip_binder()
.output(); .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 AutoDerefStability::Reborrow
} else { } else {
AutoDerefStability::Deref AutoDerefStability::Deref

View File

@ -6,6 +6,7 @@
unused_braces, unused_braces,
clippy::borrowed_box, clippy::borrowed_box,
clippy::needless_borrow, clippy::needless_borrow,
clippy::needless_return,
clippy::ptr_arg, clippy::ptr_arg,
clippy::redundant_field_names, clippy::redundant_field_names,
clippy::too_many_arguments, clippy::too_many_arguments,
@ -184,4 +185,17 @@ fn main() {
} }
let s6 = S6 { foo: S5 { foo: 5 } }; let s6 = S6 { foo: S5 { foo: 5 } };
let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` 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;
}
}
} }

View File

@ -6,6 +6,7 @@
unused_braces, unused_braces,
clippy::borrowed_box, clippy::borrowed_box,
clippy::needless_borrow, clippy::needless_borrow,
clippy::needless_return,
clippy::ptr_arg, clippy::ptr_arg,
clippy::redundant_field_names, clippy::redundant_field_names,
clippy::too_many_arguments, clippy::too_many_arguments,
@ -184,4 +185,17 @@ fn main() {
} }
let s6 = S6 { foo: S5 { foo: 5 } }; let s6 = S6 { foo: S5 { foo: 5 } };
let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` 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;
}
}
} }

View File

@ -168,5 +168,17 @@ error: deref which would be done by auto-deref
LL | let _ = (**b).foo; LL | let _ = (**b).foo;
| ^^^^^ help: try this: `b` | ^^^^^ 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

View File

@ -1,7 +1,7 @@
// FIXME: run-rustfix waiting on multi-span suggestions // FIXME: run-rustfix waiting on multi-span suggestions
#![warn(clippy::needless_borrow)] #![warn(clippy::needless_borrow)]
#![allow(clippy::needless_borrowed_reference)] #![allow(clippy::needless_borrowed_reference, clippy::explicit_auto_deref)]
fn f1(_: &str) {} fn f1(_: &str) {}
macro_rules! m1 { macro_rules! m1 {

View File

@ -2,7 +2,7 @@
#![feature(lint_reasons)] #![feature(lint_reasons)]
#![warn(clippy::ref_binding_to_reference)] #![warn(clippy::ref_binding_to_reference)]
#![allow(clippy::needless_borrowed_reference)] #![allow(clippy::needless_borrowed_reference, clippy::explicit_auto_deref)]
fn f1(_: &str) {} fn f1(_: &str) {}
macro_rules! m2 { macro_rules! m2 {

View File

@ -1,5 +1,5 @@
// run-rustfix // run-rustfix
#![allow(dead_code)] #![allow(dead_code, clippy::explicit_auto_deref)]
#![warn(clippy::search_is_some)] #![warn(clippy::search_is_some)]
fn main() { fn main() {

View File

@ -1,5 +1,5 @@
// run-rustfix // run-rustfix
#![allow(dead_code)] #![allow(dead_code, clippy::explicit_auto_deref)]
#![warn(clippy::search_is_some)] #![warn(clippy::search_is_some)]
fn main() { fn main() {

View File

@ -1,5 +1,5 @@
// run-rustfix // run-rustfix
#![allow(dead_code)] #![allow(dead_code, clippy::explicit_auto_deref)]
#![warn(clippy::search_is_some)] #![warn(clippy::search_is_some)]
fn main() { fn main() {

View File

@ -1,5 +1,5 @@
// run-rustfix // run-rustfix
#![allow(dead_code)] #![allow(dead_code, clippy::explicit_auto_deref)]
#![warn(clippy::search_is_some)] #![warn(clippy::search_is_some)]
fn main() { fn main() {