about summary refs log tree commit diff
diff options
context:
space:
mode:
authorxiongmao86 <xiongmao86dev@sina.com>2020-04-06 21:48:38 +0800
committerxiongmao86 <xiongmao86dev@sina.com>2020-04-06 22:53:59 +0800
commit4f14826e091e08f1ee4ff06d3ca6d5015045cfb5 (patch)
tree262d133706802023f349ec7b8a478f9b3b54f4d5
parentc211cea3e99e04c2980a853b6637de22881b72eb (diff)
downloadrust-4f14826e091e08f1ee4ff06d3ca6d5015045cfb5.tar.gz
rust-4f14826e091e08f1ee4ff06d3ca6d5015045cfb5.zip
Lint on opt.as_ref().map(|x| &**x).
-rw-r--r--clippy_lints/src/loops.rs4
-rw-r--r--clippy_lints/src/methods/mod.rs49
-rw-r--r--tests/ui/option_as_ref_deref.fixed3
-rw-r--r--tests/ui/option_as_ref_deref.rs3
-rw-r--r--tests/ui/option_as_ref_deref.stderr14
5 files changed, 55 insertions, 18 deletions
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 43217b6cc64..7cc21e4bdd2 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -654,7 +654,7 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult
 
 fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
     let stmts = block.stmts.iter().map(stmt_to_expr);
-    let expr = once(block.expr.as_ref().map(|p| &**p));
+    let expr = once(block.expr.as_deref());
     let mut iter = stmts.chain(expr).filter_map(|e| e);
     never_loop_expr_seq(&mut iter, main_loop_id)
 }
@@ -662,7 +662,7 @@ fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
 fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> {
     match stmt.kind {
         StmtKind::Semi(ref e, ..) | StmtKind::Expr(ref e, ..) => Some(e),
-        StmtKind::Local(ref local) => local.init.as_ref().map(|p| &**p),
+        StmtKind::Local(ref local) => local.init.as_deref(),
         _ => None,
     }
 }
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 527508af8a3..6f85d1d6959 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -3159,6 +3159,8 @@ fn lint_option_as_ref_deref<'a, 'tcx>(
     map_args: &[hir::Expr<'_>],
     is_mut: bool,
 ) {
+    let same_mutability = |m| (is_mut && m == &hir::Mutability::Mut) || (!is_mut && m == &hir::Mutability::Not);
+
     let option_ty = cx.tables.expr_ty(&as_ref_args[0]);
     if !match_type(cx, option_ty, &paths::OPTION) {
         return;
@@ -3181,23 +3183,40 @@ fn lint_option_as_ref_deref<'a, 'tcx>(
         hir::ExprKind::Closure(_, _, body_id, _, _) => {
             let closure_body = cx.tcx.hir().body(body_id);
             let closure_expr = remove_blocks(&closure_body.value);
-            if_chain! {
-                if let hir::ExprKind::MethodCall(_, _, args) = &closure_expr.kind;
-                if args.len() == 1;
-                if let hir::ExprKind::Path(qpath) = &args[0].kind;
-                if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id);
-                if closure_body.params[0].pat.hir_id == local_id;
-                let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::<Box<[_]>>();
-                if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj;
-                then {
-                    let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap();
-                    deref_aliases.iter().any(|path| match_def_path(cx, method_did, path))
-                } else {
-                    false
-                }
+
+            match &closure_expr.kind {
+                hir::ExprKind::MethodCall(_, _, args) => {
+                    if_chain! {
+                        if args.len() == 1;
+                        if let hir::ExprKind::Path(qpath) = &args[0].kind;
+                        if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id);
+                        if closure_body.params[0].pat.hir_id == local_id;
+                        let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::<Box<[_]>>();
+                        if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj;
+                        then {
+                            let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap();
+                            deref_aliases.iter().any(|path| match_def_path(cx, method_did, path))
+                        } else {
+                            false
+                        }
+                    }
+                },
+                hir::ExprKind::AddrOf(hir::BorrowKind::Ref, m, ref inner) if same_mutability(m) => {
+                    if_chain! {
+                        if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner1) = inner.kind;
+                        if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner2) = inner1.kind;
+                        if let hir::ExprKind::Path(ref qpath) = inner2.kind;
+                        if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, inner2.hir_id);
+                        then {
+                            closure_body.params[0].pat.hir_id == local_id
+                        } else {
+                            false
+                        }
+                    }
+                },
+                _ => false,
             }
         },
-
         _ => false,
     };
 
diff --git a/tests/ui/option_as_ref_deref.fixed b/tests/ui/option_as_ref_deref.fixed
index 973e5b308a2..076692e6445 100644
--- a/tests/ui/option_as_ref_deref.fixed
+++ b/tests/ui/option_as_ref_deref.fixed
@@ -35,4 +35,7 @@ fn main() {
     let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
 
     let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
+
+    let _ = opt.as_deref();
+    let _ = opt.as_deref_mut();
 }
diff --git a/tests/ui/option_as_ref_deref.rs b/tests/ui/option_as_ref_deref.rs
index baad85ab908..3bf5f715f83 100644
--- a/tests/ui/option_as_ref_deref.rs
+++ b/tests/ui/option_as_ref_deref.rs
@@ -38,4 +38,7 @@ fn main() {
     let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
 
     let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
+
+    let _ = opt.as_ref().map(|x| &**x);
+    let _ = opt.as_mut().map(|x| &mut **x);
 }
diff --git a/tests/ui/option_as_ref_deref.stderr b/tests/ui/option_as_ref_deref.stderr
index 09a0fa058e6..6c2bf851706 100644
--- a/tests/ui/option_as_ref_deref.stderr
+++ b/tests/ui/option_as_ref_deref.stderr
@@ -88,5 +88,17 @@ error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases
 LL |     let _ = opt.clone().as_mut().map(|x| x.deref_mut()).map(|x| x.len());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.clone().as_deref_mut()`
 
-error: aborting due to 14 previous errors
+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
+  --> $DIR/option_as_ref_deref.rs:42:13
+   |
+LL |     let _ = opt.as_ref().map(|x| &**x);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`
+
+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
+  --> $DIR/option_as_ref_deref.rs:43:13
+   |
+LL |     let _ = opt.as_mut().map(|x| &mut **x);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`
+
+error: aborting due to 16 previous errors