Skip to content

Commit

Permalink
Suggest using as_ref on some borrow errors [hack]
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
estebank committed May 30, 2018
1 parent 74d0939 commit a19a03a
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 16 deletions.
88 changes: 72 additions & 16 deletions src/librustc_typeck/check/demand.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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:
///
Expand All @@ -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:
Expand All @@ -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,
Expand All @@ -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));
}
}

Expand All @@ -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,
Expand Down
25 changes: 25 additions & 0 deletions 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]
}
47 changes: 47 additions & 0 deletions 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`.

0 comments on commit a19a03a

Please sign in to comment.