Auto merge of #51100 - estebank:as-ref, r=oli-obk

Suggest using `as_ref` on some borrow errors [hack]

When encountering the following code:

```rust
struct Foo;
fn takes_ref(_: &Foo) {}
let ref opt = Some(Foo);

opt.map(|arg| takes_ref(arg));
```

Suggest using `opt.as_ref().map(|arg| takes_ref(arg));` instead.

This is a stop gap solution until we expand typeck to deal with these
cases in a more graceful way.

#43861

Author: bors

src/librustc_typeck/check/demand.rs

@@ -19,7 +19,7 @@ use syntax::util::parser::PREC_POSTFIX;
 use syntax_pos::Span;
 use rustc::hir;
 use rustc::hir::def::Def;
-use rustc::hir::map::NodeItem;
+use rustc::hir::map::{NodeItem, NodeExpr};
 use rustc::hir::{Item, ItemConst, print};
 use rustc::ty::{self, Ty, AssociatedItem};
 use rustc::ty::adjustment::AllowTwoPhase;
@@ -140,8 +140,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             }
         }
 
-        if let Some((msg, suggestion)) = self.check_ref(expr, checked_ty, expected) {
-            err.span_suggestion(expr.span, msg, suggestion);
+        if let Some((sp, msg, suggestion)) = self.check_ref(expr, checked_ty, expected) {
+            err.span_suggestion(sp, msg, suggestion);
         } else if !self.check_for_cast(&mut err, expr, expr_ty, expected) {
             let methods = self.get_conversion_methods(expr.span, expected, checked_ty);
             if let Ok(expr_text) = self.tcx.sess.codemap().span_to_snippet(expr.span) {
@@ -194,6 +194,57 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    /// Identify some cases where `as_ref()` would be appropriate and suggest it.
+    ///
+    /// Given the following code:
+    /// ```
+    /// struct Foo;
+    /// fn takes_ref(_: &Foo) {}
+    /// let ref opt = Some(Foo);
+    ///
+    /// opt.map(|arg| takes_ref(arg));
+    /// ```
+    /// Suggest using `opt.as_ref().map(|arg| takes_ref(arg));` instead.
+    ///
+    /// It only checks for `Option` and `Result` and won't work with
+    /// ```
+    /// opt.map(|arg| { takes_ref(arg) });
+    /// ```
+    fn can_use_as_ref(&self, expr: &hir::Expr) -> Option<(Span, &'static str, String)> {
+        if let hir::ExprPath(hir::QPath::Resolved(_, ref path)) = expr.node {
+            if let hir::def::Def::Local(id) = path.def {
+                let parent = self.tcx.hir.get_parent_node(id);
+                if let Some(NodeExpr(hir::Expr {
+                    id,
+                    node: hir::ExprClosure(_, decl, ..),
+                    ..
+                })) = self.tcx.hir.find(parent) {
+                    let parent = self.tcx.hir.get_parent_node(*id);
+                    if let (Some(NodeExpr(hir::Expr {
+                        node: hir::ExprMethodCall(path, span, expr),
+                        ..
+                    })), 1) = (self.tcx.hir.find(parent), decl.inputs.len()) {
+                        let self_ty = self.tables.borrow().node_id_to_type(expr[0].hir_id);
+                        let self_ty = format!("{:?}", self_ty);
+                        let name = path.name.as_str();
+                        let is_as_ref_able = (
+                            self_ty.starts_with("&std::option::Option") ||
+                            self_ty.starts_with("&std::result::Result") ||
+                            self_ty.starts_with("std::option::Option") ||
+                            self_ty.starts_with("std::result::Result")
+                        ) && (name == "map" || name == "and_then");
+                        if is_as_ref_able {
+                            return Some((span.shrink_to_lo(),
+                                         "consider using `as_ref` instead",
+                                         "as_ref().".into()));
+                        }
+                    }
+                }
+            }
+        }
+        None
+    }
+
     /// This function is used to determine potential "simple" improvements or users' errors and
     /// provide them useful help. For example:
     ///
@@ -214,32 +265,33 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                  expr: &hir::Expr,
                  checked_ty: Ty<'tcx>,
                  expected: Ty<'tcx>)
-                 -> Option<(&'static str, String)> {
+                 -> Option<(Span, &'static str, String)> {
+        let sp = expr.span;
         match (&expected.sty, &checked_ty.sty) {
             (&ty::TyRef(_, exp, _), &ty::TyRef(_, check, _)) => match (&exp.sty, &check.sty) {
                 (&ty::TyStr, &ty::TyArray(arr, _)) |
                 (&ty::TyStr, &ty::TySlice(arr)) if arr == self.tcx.types.u8 => {
                     if let hir::ExprLit(_) = expr.node {
                         let sp = self.sess().codemap().call_span_if_macro(expr.span);
                         if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(sp) {
-                            return Some(("consider removing the leading `b`",
+                            return Some((sp,
+                                         "consider removing the leading `b`",
                                          src[1..].to_string()));
                         }
                     }
-                    None
                 },
                 (&ty::TyArray(arr, _), &ty::TyStr) |
                 (&ty::TySlice(arr), &ty::TyStr) if arr == self.tcx.types.u8 => {
                     if let hir::ExprLit(_) = expr.node {
                         let sp = self.sess().codemap().call_span_if_macro(expr.span);
                         if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(sp) {
-                            return Some(("consider adding a leading `b`",
+                            return Some((sp,
+                                         "consider adding a leading `b`",
                                          format!("b{}", src)));
                         }
                     }
-                    None
                 }
-                _ => None,
+                _ => {}
             },
             (&ty::TyRef(_, _, mutability), _) => {
                 // Check if it can work when put into a ref. For example:
@@ -266,17 +318,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                             hir::ExprCast(_, _) | hir::ExprBinary(_, _, _) => format!("({})", src),
                             _ => src,
                         };
+                        if let Some(sugg) = self.can_use_as_ref(expr) {
+                            return Some(sugg);
+                        }
                         return Some(match mutability {
                             hir::Mutability::MutMutable => {
-                                ("consider mutably borrowing here", format!("&mut {}", sugg_expr))
+                                (sp, "consider mutably borrowing here", format!("&mut {}",
+                                                                                sugg_expr))
                             }
                             hir::Mutability::MutImmutable => {
-                                ("consider borrowing here", format!("&{}", sugg_expr))
+                                (sp, "consider borrowing here", format!("&{}", sugg_expr))
                             }
                         });
                     }
                 }
-                None
             }
             (_, &ty::TyRef(_, checked, _)) => {
                 // We have `&T`, check if what was expected was `T`. If so,
@@ -292,7 +347,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                         // Maybe remove `&`?
                         hir::ExprAddrOf(_, ref expr) => {
                             if let Ok(code) = self.tcx.sess.codemap().span_to_snippet(expr.span) {
-                                return Some(("consider removing the borrow", code));
+                                return Some((sp, "consider removing the borrow", code));
                             }
                         }
 
@@ -303,17 +358,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                                                                 expr.span) {
                                 let sp = self.sess().codemap().call_span_if_macro(expr.span);
                                 if let Ok(code) = self.tcx.sess.codemap().span_to_snippet(sp) {
-                                    return Some(("consider dereferencing the borrow",
+                                    return Some((sp,
+                                                 "consider dereferencing the borrow",
                                                  format!("*{}", code)));
                                 }
                             }
                         }
                     }
                 }
-                None
             }
-            _ => None,
+            _ => {}
         }
+        None
     }
 
     fn check_for_cast(&self,

src/test/ui/suggestions/as-ref.rs

@@ -0,0 +1,25 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+struct Foo;
+fn takes_ref(_: &Foo) {}
+
+fn main() {
+  let ref opt = Some(Foo);
+  opt.map(|arg| takes_ref(arg));
+  //~^ ERROR mismatched types [E0308]
+  opt.and_then(|arg| Some(takes_ref(arg)));
+  //~^ ERROR mismatched types [E0308]
+  let ref opt: Result<_, ()> = Ok(Foo);
+  opt.map(|arg| takes_ref(arg));
+  //~^ ERROR mismatched types [E0308]
+  opt.and_then(|arg| Ok(takes_ref(arg)));
+  //~^ ERROR mismatched types [E0308]
+}

src/test/ui/suggestions/as-ref.stderr

@@ -0,0 +1,47 @@
+error[E0308]: mismatched types
+  --> $DIR/as-ref.rs:16:27
+   |
+LL |   opt.map(|arg| takes_ref(arg));
+   |       -                   ^^^ expected &Foo, found struct `Foo`
+   |       |
+   |       help: consider using `as_ref` instead: `as_ref().`
+   |
+   = note: expected type `&Foo`
+              found type `Foo`
+
+error[E0308]: mismatched types
+  --> $DIR/as-ref.rs:18:37
+   |
+LL |   opt.and_then(|arg| Some(takes_ref(arg)));
+   |       -                             ^^^ expected &Foo, found struct `Foo`
+   |       |
+   |       help: consider using `as_ref` instead: `as_ref().`
+   |
+   = note: expected type `&Foo`
+              found type `Foo`
+
+error[E0308]: mismatched types
+  --> $DIR/as-ref.rs:21:27
+   |
+LL |   opt.map(|arg| takes_ref(arg));
+   |       -                   ^^^ expected &Foo, found struct `Foo`
+   |       |
+   |       help: consider using `as_ref` instead: `as_ref().`
+   |
+   = note: expected type `&Foo`
+              found type `Foo`
+
+error[E0308]: mismatched types
+  --> $DIR/as-ref.rs:23:35
+   |
+LL |   opt.and_then(|arg| Ok(takes_ref(arg)));
+   |       -                           ^^^ expected &Foo, found struct `Foo`
+   |       |
+   |       help: consider using `as_ref` instead: `as_ref().`
+   |
+   = note: expected type `&Foo`
+              found type `Foo`
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0308`.