Skip to content

Commit

Permalink
Lint explicit_auto_deref without a leading borrow
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed Jan 28, 2022
1 parent 818f534 commit a31eaad
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 40 deletions.
29 changes: 25 additions & 4 deletions clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,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
Expand All @@ -246,6 +248,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
State::ExplicitDerefField { name },
StateData { span: expr.span },
));
} else if stability.is_deref_stable() {
self.state = Some((
State::ExplicitDeref { deref_span: expr.span },
StateData { span: expr.span },
));
}
}
RefOp::Method(target_mut)
Expand All @@ -266,7 +273,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;
Expand Down Expand Up @@ -581,6 +587,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| {
Expand All @@ -592,16 +599,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(..),
Expand All @@ -619,7 +636,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
Expand All @@ -633,7 +652,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
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) => {
// for the Array case we don't need to care for the len == 0 case
// because we don't want to lint functions returning empty arrays
is_must_use_ty(cx, *ty)
is_must_use_ty(cx, ty)
},
ty::Tuple(substs) => substs.types().any(|ty| is_must_use_ty(cx, ty)),
ty::Opaque(ref def_id, _) => {
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/explicit_auto_deref.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -183,4 +184,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;
}
}
}
14 changes: 14 additions & 0 deletions tests/ui/explicit_auto_deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -183,4 +184,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;
}
}
}
70 changes: 41 additions & 29 deletions tests/ui/explicit_auto_deref.stderr
Original file line number Diff line number Diff line change
@@ -1,172 +1,184 @@
error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:62:20
--> $DIR/explicit_auto_deref.rs:63:20
|
LL | let _: &str = &*s;
| ^^ help: try this: `s`
|
= note: `-D clippy::explicit-auto-deref` implied by `-D warnings`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:66:12
--> $DIR/explicit_auto_deref.rs:67:12
|
LL | f_str(&*s);
| ^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:70:14
--> $DIR/explicit_auto_deref.rs:71:14
|
LL | f_str_t(&*s, &*s); // Don't lint second param.
| ^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:73:25
--> $DIR/explicit_auto_deref.rs:74:25
|
LL | let _: &Box<i32> = &**b;
| ^^^ help: try this: `b`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:79:8
--> $DIR/explicit_auto_deref.rs:80:8
|
LL | c(&*s);
| ^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:85:9
--> $DIR/explicit_auto_deref.rs:86:9
|
LL | &**x
| ^^^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:89:11
--> $DIR/explicit_auto_deref.rs:90:11
|
LL | { &**x }
| ^^^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:93:9
--> $DIR/explicit_auto_deref.rs:94:9
|
LL | &**{ x }
| ^^^^^^^^ help: try this: `{ x }`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:97:9
--> $DIR/explicit_auto_deref.rs:98:9
|
LL | &***x
| ^^^^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:114:13
--> $DIR/explicit_auto_deref.rs:115:13
|
LL | f1(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:115:13
--> $DIR/explicit_auto_deref.rs:116:13
|
LL | f2(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:116:13
--> $DIR/explicit_auto_deref.rs:117:13
|
LL | f3(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:117:28
--> $DIR/explicit_auto_deref.rs:118:28
|
LL | f4.callable_str()(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:118:13
--> $DIR/explicit_auto_deref.rs:119:13
|
LL | f5(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:119:13
--> $DIR/explicit_auto_deref.rs:120:13
|
LL | f6(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:120:28
--> $DIR/explicit_auto_deref.rs:121:28
|
LL | f7.callable_str()(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:121:26
--> $DIR/explicit_auto_deref.rs:122:26
|
LL | f8.callable_t()(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:122:13
--> $DIR/explicit_auto_deref.rs:123:13
|
LL | f9(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:123:14
--> $DIR/explicit_auto_deref.rs:124:14
|
LL | f10(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:124:27
--> $DIR/explicit_auto_deref.rs:125:27
|
LL | f11.callable_t()(&*x);
| ^^ help: try this: `x`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:128:17
--> $DIR/explicit_auto_deref.rs:129:17
|
LL | let _ = S1(&*s);
| ^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:133:22
--> $DIR/explicit_auto_deref.rs:134:22
|
LL | let _ = S2 { s: &*s };
| ^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:149:30
--> $DIR/explicit_auto_deref.rs:150:30
|
LL | let _ = Self::S1(&**s);
| ^^^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:150:35
--> $DIR/explicit_auto_deref.rs:151:35
|
LL | let _ = Self::S2 { s: &**s };
| ^^^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:153:21
--> $DIR/explicit_auto_deref.rs:154:21
|
LL | let _ = E1::S1(&*s);
| ^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:154:26
--> $DIR/explicit_auto_deref.rs:155:26
|
LL | let _ = E1::S2 { s: &*s };
| ^^ help: try this: `s`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:172:13
--> $DIR/explicit_auto_deref.rs:173:13
|
LL | let _ = (*b).foo;
| ^^^^ help: try this: `b`

error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:173:13
--> $DIR/explicit_auto_deref.rs:174:13
|
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

2 changes: 1 addition & 1 deletion tests/ui/needless_borrow_pat.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/ref_binding_to_reference.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// FIXME: run-rustfix waiting on multi-span suggestions

#![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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/search_is_some_fixable_none.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// run-rustfix
#![allow(dead_code)]
#![allow(dead_code, clippy::explicit_auto_deref)]
#![warn(clippy::search_is_some)]

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/search_is_some_fixable_none.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// run-rustfix
#![allow(dead_code)]
#![allow(dead_code, clippy::explicit_auto_deref)]
#![warn(clippy::search_is_some)]

fn main() {
Expand Down
Loading

0 comments on commit a31eaad

Please sign in to comment.