Skip to content

Commit 207b0da

Browse files
committed
Lint on opt.as_ref().map(|x| &**x).
1 parent c211cea commit 207b0da

File tree

4 files changed

+56
-16
lines changed

4 files changed

+56
-16
lines changed

clippy_lints/src/methods/mod.rs

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3159,6 +3159,8 @@ fn lint_option_as_ref_deref<'a, 'tcx>(
31593159
map_args: &[hir::Expr<'_>],
31603160
is_mut: bool,
31613161
) {
3162+
let same_mutability = |m| (is_mut && m == &hir::Mutability::Mut) || (!is_mut && m == &hir::Mutability::Not);
3163+
31623164
let option_ty = cx.tables.expr_ty(&as_ref_args[0]);
31633165
if !match_type(cx, option_ty, &paths::OPTION) {
31643166
return;
@@ -3181,23 +3183,43 @@ fn lint_option_as_ref_deref<'a, 'tcx>(
31813183
hir::ExprKind::Closure(_, _, body_id, _, _) => {
31823184
let closure_body = cx.tcx.hir().body(body_id);
31833185
let closure_expr = remove_blocks(&closure_body.value);
3184-
if_chain! {
3185-
if let hir::ExprKind::MethodCall(_, _, args) = &closure_expr.kind;
3186-
if args.len() == 1;
3187-
if let hir::ExprKind::Path(qpath) = &args[0].kind;
3188-
if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id);
3189-
if closure_body.params[0].pat.hir_id == local_id;
3190-
let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::<Box<[_]>>();
3191-
if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj;
3192-
then {
3193-
let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap();
3194-
deref_aliases.iter().any(|path| match_def_path(cx, method_did, path))
3195-
} else {
3196-
false
3197-
}
3186+
3187+
match &closure_expr.kind {
3188+
hir::ExprKind::MethodCall(_, _, args) => {
3189+
if_chain! {
3190+
if args.len() == 1;
3191+
if let hir::ExprKind::Path(qpath) = &args[0].kind;
3192+
if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id);
3193+
if closure_body.params[0].pat.hir_id == local_id;
3194+
let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::<Box<[_]>>();
3195+
if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj;
3196+
then {
3197+
let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap();
3198+
deref_aliases.iter().any(|path| match_def_path(cx, method_did, path))
3199+
}
3200+
else {
3201+
false
3202+
}
3203+
}
3204+
},
3205+
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, m, ref inner) if same_mutability(m) => {
3206+
if_chain! {
3207+
if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner1) = inner.kind;
3208+
if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner2) = inner1.kind;
3209+
if let hir::ExprKind::Path(ref qpath) = inner2.kind;
3210+
if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, inner2.hir_id);
3211+
if closure_body.params[0].pat.hir_id == local_id;
3212+
then {
3213+
true
3214+
}
3215+
else {
3216+
false
3217+
}
3218+
}
3219+
},
3220+
_ => false,
31983221
}
31993222
},
3200-
32013223
_ => false,
32023224
};
32033225

tests/ui/option_as_ref_deref.fixed

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,7 @@ fn main() {
3535
let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
3636

3737
let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
38+
39+
let _ = opt.as_deref();
40+
let _ = opt.as_deref_mut();
3841
}

tests/ui/option_as_ref_deref.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,7 @@ fn main() {
3838
let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
3939

4040
let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
41+
42+
let _ = opt.as_ref().map(|x| &**x);
43+
let _ = opt.as_mut().map(|x| &mut **x);
4144
}

tests/ui/option_as_ref_deref.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,17 @@ error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases
8888
LL | let _ = opt.clone().as_mut().map(|x| x.deref_mut()).map(|x| x.len());
8989
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.clone().as_deref_mut()`
9090

91-
error: aborting due to 14 previous errors
91+
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead
92+
--> $DIR/option_as_ref_deref.rs:42:13
93+
|
94+
LL | let _ = opt.as_ref().map(|x| &**x);
95+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`
96+
97+
error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
98+
--> $DIR/option_as_ref_deref.rs:43:13
99+
|
100+
LL | let _ = opt.as_mut().map(|x| &mut **x);
101+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`
102+
103+
error: aborting due to 16 previous errors
92104

0 commit comments

Comments
 (0)