Include the borrow in the suggestion for explicit_auto_deref

This commit is contained in:
Jason Newcomb 2022-07-05 22:30:36 -04:00
parent c990e2922a
commit 9ce0f82b8a
4 changed files with 98 additions and 101 deletions

View File

@ -183,24 +183,24 @@ enum State {
}, },
DerefedBorrow(DerefedBorrow), DerefedBorrow(DerefedBorrow),
ExplicitDeref { ExplicitDeref {
// Span and id of the top-level deref expression if the parent expression is a borrow. mutability: Option<Mutability>,
deref_span_id: Option<(Span, HirId)>,
}, },
ExplicitDerefField { ExplicitDerefField {
name: Symbol, name: Symbol,
}, },
Reborrow { Reborrow {
deref_span: Span, mutability: Mutability,
deref_hir_id: HirId, },
Borrow {
mutability: Mutability,
}, },
Borrow,
} }
// A reference operation considered by this lint pass // A reference operation considered by this lint pass
enum RefOp { enum RefOp {
Method(Mutability), Method(Mutability),
Deref, Deref,
AddrOf, AddrOf(Mutability),
} }
struct RefPat { struct RefPat {
@ -263,7 +263,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
)); ));
} else if position.is_deref_stable() { } else if position.is_deref_stable() {
self.state = Some(( self.state = Some((
State::ExplicitDeref { deref_span_id: None }, State::ExplicitDeref { mutability: None },
StateData { span: expr.span, hir_id: expr.hir_id, position }, StateData { span: expr.span, hir_id: expr.hir_id, position },
)); ));
} }
@ -289,7 +289,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
}, },
)); ));
}, },
RefOp::AddrOf => { RefOp::AddrOf(mutability) => {
// 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;
@ -359,7 +359,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
)); ));
} else if position.is_deref_stable() { } else if position.is_deref_stable() {
self.state = Some(( self.state = Some((
State::Borrow, State::Borrow { mutability },
StateData { StateData {
span: expr.span, span: expr.span,
hir_id: expr.hir_id, hir_id: expr.hir_id,
@ -395,7 +395,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
data, data,
)); ));
}, },
(Some((State::DerefedBorrow(state), data)), RefOp::AddrOf) if state.count != 0 => { (Some((State::DerefedBorrow(state), data)), RefOp::AddrOf(_)) if state.count != 0 => {
self.state = Some(( self.state = Some((
State::DerefedBorrow(DerefedBorrow { State::DerefedBorrow(DerefedBorrow {
count: state.count - 1, count: state.count - 1,
@ -404,12 +404,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
data, data,
)); ));
}, },
(Some((State::DerefedBorrow(state), data)), RefOp::AddrOf) => { (Some((State::DerefedBorrow(state), data)), RefOp::AddrOf(mutability)) => {
let position = data.position; let position = data.position;
report(cx, expr, State::DerefedBorrow(state), data); report(cx, expr, State::DerefedBorrow(state), data);
if position.is_deref_stable() { if position.is_deref_stable() {
self.state = Some(( self.state = Some((
State::Borrow, State::Borrow { mutability },
StateData { StateData {
span: expr.span, span: expr.span,
hir_id: expr.hir_id, hir_id: expr.hir_id,
@ -430,43 +430,28 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
)); ));
} else if position.is_deref_stable() { } else if position.is_deref_stable() {
self.state = Some(( self.state = Some((
State::ExplicitDeref { deref_span_id: None }, State::ExplicitDeref { mutability: None },
StateData { span: expr.span, hir_id: expr.hir_id, position }, StateData { span: expr.span, hir_id: expr.hir_id, position },
)); ));
} }
}, },
(Some((State::Borrow, data)), RefOp::Deref) => { (Some((State::Borrow { mutability }, data)), RefOp::Deref) => {
if typeck.expr_ty(sub_expr).is_ref() { if typeck.expr_ty(sub_expr).is_ref() {
self.state = Some(( self.state = Some((State::Reborrow { mutability }, data));
State::Reborrow {
deref_span: expr.span,
deref_hir_id: expr.hir_id,
},
data,
));
} else { } else {
self.state = Some(( self.state = Some((
State::ExplicitDeref { State::ExplicitDeref {
deref_span_id: Some((expr.span, expr.hir_id)), mutability: Some(mutability),
}, },
data, data,
)); ));
} }
}, },
( (Some((State::Reborrow { mutability }, data)), RefOp::Deref) => {
Some((
State::Reborrow {
deref_span,
deref_hir_id,
},
data,
)),
RefOp::Deref,
) => {
self.state = Some(( self.state = Some((
State::ExplicitDeref { State::ExplicitDeref {
deref_span_id: Some((deref_span, deref_hir_id)), mutability: Some(mutability),
}, },
data, data,
)); ));
@ -573,7 +558,7 @@ fn try_parse_ref_op<'tcx>(
ExprKind::Unary(UnOp::Deref, sub_expr) if !typeck.expr_ty(sub_expr).is_unsafe_ptr() => { ExprKind::Unary(UnOp::Deref, sub_expr) if !typeck.expr_ty(sub_expr).is_unsafe_ptr() => {
return Some((RefOp::Deref, sub_expr)); return Some((RefOp::Deref, sub_expr));
}, },
ExprKind::AddrOf(BorrowKind::Ref, _, sub_expr) => return Some((RefOp::AddrOf, sub_expr)), ExprKind::AddrOf(BorrowKind::Ref, mutability, sub_expr) => return Some((RefOp::AddrOf(mutability), sub_expr)),
_ => return None, _ => return None,
}; };
if tcx.is_diagnostic_item(sym::deref_method, def_id) { if tcx.is_diagnostic_item(sym::deref_method, def_id) {
@ -1074,7 +1059,7 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
diag.span_suggestion(data.span, "change this to", sugg, app); diag.span_suggestion(data.span, "change this to", sugg, app);
}); });
}, },
State::ExplicitDeref { deref_span_id } => { State::ExplicitDeref { mutability } => {
if matches!( if matches!(
expr.kind, expr.kind,
ExprKind::Block(..) ExprKind::Block(..)
@ -1088,29 +1073,33 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
return; return;
} }
let (span, hir_id, precedence) = if let Some((span, hir_id)) = deref_span_id let (prefix, precedence) = if let Some(mutability) = mutability
&& !cx.typeck_results().expr_ty(expr).is_ref() && !cx.typeck_results().expr_ty(expr).is_ref()
{ {
(span, hir_id, PREC_PREFIX) let prefix = match mutability {
Mutability::Not => "&",
Mutability::Mut => "&mut ",
};
(prefix, 0)
} else { } else {
(data.span, data.hir_id, data.position.precedence()) ("", data.position.precedence())
}; };
span_lint_hir_and_then( span_lint_hir_and_then(
cx, cx,
EXPLICIT_AUTO_DEREF, EXPLICIT_AUTO_DEREF,
hir_id, data.hir_id,
span, data.span,
"deref which would be done by auto-deref", "deref which would be done by auto-deref",
|diag| { |diag| {
let mut app = Applicability::MachineApplicable; let mut app = Applicability::MachineApplicable;
let (snip, snip_is_macro) = snippet_with_context(cx, expr.span, span.ctxt(), "..", &mut app); let (snip, snip_is_macro) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app);
let sugg = let sugg =
if !snip_is_macro && expr.precedence().order() < precedence && !has_enclosing_paren(&snip) { if !snip_is_macro && expr.precedence().order() < precedence && !has_enclosing_paren(&snip) {
format!("({})", snip) format!("{}({})", prefix, snip)
} else { } else {
snip.into() format!("{}{}", prefix, snip)
}; };
diag.span_suggestion(span, "try this", sugg, app); diag.span_suggestion(data.span, "try this", sugg, app);
}, },
); );
}, },
@ -1141,7 +1130,7 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
}, },
); );
}, },
State::Borrow | State::Reborrow { .. } => (), State::Borrow { .. } | State::Reborrow { .. } => (),
} }
} }

View File

@ -68,6 +68,7 @@ fn main() {
let _: &str = &s; let _: &str = &s;
let _: &str = &{ String::new() }; let _: &str = &{ String::new() };
let _: &str = &mut { String::new() };
let _ = &*s; // Don't lint. Inferred type would change. let _ = &*s; // Don't lint. Inferred type would change.
let _: &_ = &*s; // Don't lint. Inferred type would change. let _: &_ = &*s; // Don't lint. Inferred type would change.

View File

@ -68,6 +68,7 @@ fn main() {
let _: &str = &*s; let _: &str = &*s;
let _: &str = &*{ String::new() }; let _: &str = &*{ String::new() };
let _: &str = &mut *{ String::new() };
let _ = &*s; // Don't lint. Inferred type would change. let _ = &*s; // Don't lint. Inferred type would change.
let _: &_ = &*s; // Don't lint. Inferred type would change. let _: &_ = &*s; // Don't lint. Inferred type would change.

View File

@ -1,208 +1,214 @@
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:69:20 --> $DIR/explicit_auto_deref.rs:69:19
| |
LL | let _: &str = &*s; LL | let _: &str = &*s;
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
| |
= note: `-D clippy::explicit-auto-deref` implied by `-D warnings` = note: `-D clippy::explicit-auto-deref` implied by `-D warnings`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:70:20 --> $DIR/explicit_auto_deref.rs:70:19
| |
LL | let _: &str = &*{ String::new() }; LL | let _: &str = &*{ String::new() };
| ^^^^^^^^^^^^^^^^^^ help: try this: `{ String::new() }` | ^^^^^^^^^^^^^^^^^^^ help: try this: `&{ String::new() }`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:74:12 --> $DIR/explicit_auto_deref.rs:71:19
|
LL | let _: &str = &mut *{ String::new() };
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `&mut { String::new() }`
error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:75:11
| |
LL | f_str(&*s); LL | f_str(&*s);
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:78:14 --> $DIR/explicit_auto_deref.rs:79:13
| |
LL | f_str_t(&*s, &*s); // Don't lint second param. LL | f_str_t(&*s, &*s); // Don't lint second param.
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:81:25 --> $DIR/explicit_auto_deref.rs:82:24
| |
LL | let _: &Box<i32> = &**b; LL | let _: &Box<i32> = &**b;
| ^^^ help: try this: `b` | ^^^^ help: try this: `&b`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:87:8 --> $DIR/explicit_auto_deref.rs:88:7
| |
LL | c(&*s); LL | c(&*s);
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
error: deref which would be done by auto-deref 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 LL | &**x
| ^^^^ help: try this: `x` | ^^^^ help: try this: `x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:97:11 --> $DIR/explicit_auto_deref.rs:98:11
| |
LL | { &**x } LL | { &**x }
| ^^^^ help: try this: `x` | ^^^^ help: try this: `x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:101:9 --> $DIR/explicit_auto_deref.rs:102:9
| |
LL | &**{ x } LL | &**{ x }
| ^^^^^^^^ help: try this: `{ x }` | ^^^^^^^^ help: try this: `{ x }`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:105:9 --> $DIR/explicit_auto_deref.rs:106:9
| |
LL | &***x LL | &***x
| ^^^^^ help: try this: `x` | ^^^^^ help: try this: `x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:122:13 --> $DIR/explicit_auto_deref.rs:123:12
| |
LL | f1(&*x); LL | f1(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:123:13 --> $DIR/explicit_auto_deref.rs:124:12
| |
LL | f2(&*x); LL | f2(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:124:13 --> $DIR/explicit_auto_deref.rs:125:12
| |
LL | f3(&*x); LL | f3(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:125:28 --> $DIR/explicit_auto_deref.rs:126:27
| |
LL | f4.callable_str()(&*x); LL | f4.callable_str()(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:126:13 --> $DIR/explicit_auto_deref.rs:127:12
| |
LL | f5(&*x); LL | f5(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:127:13 --> $DIR/explicit_auto_deref.rs:128:12
| |
LL | f6(&*x); LL | f6(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:128:28 --> $DIR/explicit_auto_deref.rs:129:27
| |
LL | f7.callable_str()(&*x); LL | f7.callable_str()(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:129:26 --> $DIR/explicit_auto_deref.rs:130:25
| |
LL | f8.callable_t()(&*x); LL | f8.callable_t()(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:130:13 --> $DIR/explicit_auto_deref.rs:131:12
| |
LL | f9(&*x); LL | f9(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:131:14 --> $DIR/explicit_auto_deref.rs:132:13
| |
LL | f10(&*x); LL | f10(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:132:27 --> $DIR/explicit_auto_deref.rs:133:26
| |
LL | f11.callable_t()(&*x); LL | f11.callable_t()(&*x);
| ^^ help: try this: `x` | ^^^ help: try this: `&x`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:136:17 --> $DIR/explicit_auto_deref.rs:137:16
| |
LL | let _ = S1(&*s); LL | let _ = S1(&*s);
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:141:22 --> $DIR/explicit_auto_deref.rs:142:21
| |
LL | let _ = S2 { s: &*s }; LL | let _ = S2 { s: &*s };
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:157:30 --> $DIR/explicit_auto_deref.rs:158:30
| |
LL | let _ = Self::S1(&**s); LL | let _ = Self::S1(&**s);
| ^^^^ help: try this: `s` | ^^^^ help: try this: `s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:158:35 --> $DIR/explicit_auto_deref.rs:159:35
| |
LL | let _ = Self::S2 { s: &**s }; LL | let _ = Self::S2 { s: &**s };
| ^^^^ help: try this: `s` | ^^^^ help: try this: `s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:161:21 --> $DIR/explicit_auto_deref.rs:162:20
| |
LL | let _ = E1::S1(&*s); LL | let _ = E1::S1(&*s);
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:162:26 --> $DIR/explicit_auto_deref.rs:163:25
| |
LL | let _ = E1::S2 { s: &*s }; LL | let _ = E1::S2 { s: &*s };
| ^^ help: try this: `s` | ^^^ help: try this: `&s`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:180:13 --> $DIR/explicit_auto_deref.rs:181:13
| |
LL | let _ = (*b).foo; LL | let _ = (*b).foo;
| ^^^^ help: try this: `b` | ^^^^ help: try this: `b`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:181:13 --> $DIR/explicit_auto_deref.rs:182:13
| |
LL | let _ = (**b).foo; LL | let _ = (**b).foo;
| ^^^^^ help: try this: `b` | ^^^^^ help: try this: `b`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:196:19 --> $DIR/explicit_auto_deref.rs:197:19
| |
LL | let _ = f_str(*ref_str); LL | let _ = f_str(*ref_str);
| ^^^^^^^^ help: try this: `ref_str` | ^^^^^^^^ help: try this: `ref_str`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:198:19 --> $DIR/explicit_auto_deref.rs:199:19
| |
LL | let _ = f_str(**ref_ref_str); LL | let _ = f_str(**ref_ref_str);
| ^^^^^^^^^^^^^ help: try this: `ref_ref_str` | ^^^^^^^^^^^^^ help: try this: `ref_ref_str`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:208:13 --> $DIR/explicit_auto_deref.rs:209:13
| |
LL | f_str(&&*ref_str); // `needless_borrow` will suggest removing both references LL | f_str(&&*ref_str); // `needless_borrow` will suggest removing both references
| ^^^^^^^^ help: try this: `ref_str` | ^^^^^^^^ help: try this: `ref_str`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:209:12 --> $DIR/explicit_auto_deref.rs:210:12
| |
LL | f_str(&&**ref_str); // `needless_borrow` will suggest removing only one reference LL | f_str(&&**ref_str); // `needless_borrow` will suggest removing only one reference
| ^^^^^^^^^^ help: try this: `ref_str` | ^^^^^^^^^^ help: try this: `ref_str`
error: deref which would be done by auto-deref error: deref which would be done by auto-deref
--> $DIR/explicit_auto_deref.rs:218:41 --> $DIR/explicit_auto_deref.rs:219:41
| |
LL | let _ = || -> &'static str { return *s }; LL | let _ = || -> &'static str { return *s };
| ^^ help: try this: `s` | ^^ help: try this: `s`
error: aborting due to 34 previous errors error: aborting due to 35 previous errors