Skip to content

Commit a19a03a

Browse files
committed
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.
1 parent 74d0939 commit a19a03a

File tree

3 files changed

+144
-16
lines changed

3 files changed

+144
-16
lines changed

src/librustc_typeck/check/demand.rs

+72-16
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use syntax::util::parser::PREC_POSTFIX;
1919
use syntax_pos::Span;
2020
use rustc::hir;
2121
use rustc::hir::def::Def;
22-
use rustc::hir::map::NodeItem;
22+
use rustc::hir::map::{NodeItem, NodeExpr};
2323
use rustc::hir::{Item, ItemConst, print};
2424
use rustc::ty::{self, Ty, AssociatedItem};
2525
use rustc::ty::adjustment::AllowTwoPhase;
@@ -140,8 +140,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
140140
}
141141
}
142142

143-
if let Some((msg, suggestion)) = self.check_ref(expr, checked_ty, expected) {
144-
err.span_suggestion(expr.span, msg, suggestion);
143+
if let Some((sp, msg, suggestion)) = self.check_ref(expr, checked_ty, expected) {
144+
err.span_suggestion(sp, msg, suggestion);
145145
} else if !self.check_for_cast(&mut err, expr, expr_ty, expected) {
146146
let methods = self.get_conversion_methods(expr.span, expected, checked_ty);
147147
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> {
194194
}
195195
}
196196

197+
/// Identify some cases where `as_ref()` would be appropriate and suggest it.
198+
///
199+
/// Given the following code:
200+
/// ```
201+
/// struct Foo;
202+
/// fn takes_ref(_: &Foo) {}
203+
/// let ref opt = Some(Foo);
204+
///
205+
/// opt.map(|arg| takes_ref(arg));
206+
/// ```
207+
/// Suggest using `opt.as_ref().map(|arg| takes_ref(arg));` instead.
208+
///
209+
/// It only checks for `Option` and `Result` and won't work with
210+
/// ```
211+
/// opt.map(|arg| { takes_ref(arg) });
212+
/// ```
213+
fn can_use_as_ref(&self, expr: &hir::Expr) -> Option<(Span, &'static str, String)> {
214+
if let hir::ExprPath(hir::QPath::Resolved(_, ref path)) = expr.node {
215+
if let hir::def::Def::Local(id) = path.def {
216+
let parent = self.tcx.hir.get_parent_node(id);
217+
if let Some(NodeExpr(hir::Expr {
218+
id,
219+
node: hir::ExprClosure(_, decl, ..),
220+
..
221+
})) = self.tcx.hir.find(parent) {
222+
let parent = self.tcx.hir.get_parent_node(*id);
223+
if let (Some(NodeExpr(hir::Expr {
224+
node: hir::ExprMethodCall(path, span, expr),
225+
..
226+
})), 1) = (self.tcx.hir.find(parent), decl.inputs.len()) {
227+
let self_ty = self.tables.borrow().node_id_to_type(expr[0].hir_id);
228+
let self_ty = format!("{:?}", self_ty);
229+
let name = path.name.as_str();
230+
let is_as_ref_able = (
231+
self_ty.starts_with("&std::option::Option") ||
232+
self_ty.starts_with("&std::result::Result") ||
233+
self_ty.starts_with("std::option::Option") ||
234+
self_ty.starts_with("std::result::Result")
235+
) && (name == "map" || name == "and_then");
236+
if is_as_ref_able {
237+
return Some((span.shrink_to_lo(),
238+
"consider using `as_ref` instead",
239+
"as_ref().".into()));
240+
}
241+
}
242+
}
243+
}
244+
}
245+
None
246+
}
247+
197248
/// This function is used to determine potential "simple" improvements or users' errors and
198249
/// provide them useful help. For example:
199250
///
@@ -214,32 +265,33 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
214265
expr: &hir::Expr,
215266
checked_ty: Ty<'tcx>,
216267
expected: Ty<'tcx>)
217-
-> Option<(&'static str, String)> {
268+
-> Option<(Span, &'static str, String)> {
269+
let sp = expr.span;
218270
match (&expected.sty, &checked_ty.sty) {
219271
(&ty::TyRef(_, exp, _), &ty::TyRef(_, check, _)) => match (&exp.sty, &check.sty) {
220272
(&ty::TyStr, &ty::TyArray(arr, _)) |
221273
(&ty::TyStr, &ty::TySlice(arr)) if arr == self.tcx.types.u8 => {
222274
if let hir::ExprLit(_) = expr.node {
223275
let sp = self.sess().codemap().call_span_if_macro(expr.span);
224276
if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(sp) {
225-
return Some(("consider removing the leading `b`",
277+
return Some((sp,
278+
"consider removing the leading `b`",
226279
src[1..].to_string()));
227280
}
228281
}
229-
None
230282
},
231283
(&ty::TyArray(arr, _), &ty::TyStr) |
232284
(&ty::TySlice(arr), &ty::TyStr) if arr == self.tcx.types.u8 => {
233285
if let hir::ExprLit(_) = expr.node {
234286
let sp = self.sess().codemap().call_span_if_macro(expr.span);
235287
if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(sp) {
236-
return Some(("consider adding a leading `b`",
288+
return Some((sp,
289+
"consider adding a leading `b`",
237290
format!("b{}", src)));
238291
}
239292
}
240-
None
241293
}
242-
_ => None,
294+
_ => {}
243295
},
244296
(&ty::TyRef(_, _, mutability), _) => {
245297
// Check if it can work when put into a ref. For example:
@@ -266,17 +318,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
266318
hir::ExprCast(_, _) | hir::ExprBinary(_, _, _) => format!("({})", src),
267319
_ => src,
268320
};
321+
if let Some(sugg) = self.can_use_as_ref(expr) {
322+
return Some(sugg);
323+
}
269324
return Some(match mutability {
270325
hir::Mutability::MutMutable => {
271-
("consider mutably borrowing here", format!("&mut {}", sugg_expr))
326+
(sp, "consider mutably borrowing here", format!("&mut {}",
327+
sugg_expr))
272328
}
273329
hir::Mutability::MutImmutable => {
274-
("consider borrowing here", format!("&{}", sugg_expr))
330+
(sp, "consider borrowing here", format!("&{}", sugg_expr))
275331
}
276332
});
277333
}
278334
}
279-
None
280335
}
281336
(_, &ty::TyRef(_, checked, _)) => {
282337
// We have `&T`, check if what was expected was `T`. If so,
@@ -292,7 +347,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
292347
// Maybe remove `&`?
293348
hir::ExprAddrOf(_, ref expr) => {
294349
if let Ok(code) = self.tcx.sess.codemap().span_to_snippet(expr.span) {
295-
return Some(("consider removing the borrow", code));
350+
return Some((sp, "consider removing the borrow", code));
296351
}
297352
}
298353

@@ -303,17 +358,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
303358
expr.span) {
304359
let sp = self.sess().codemap().call_span_if_macro(expr.span);
305360
if let Ok(code) = self.tcx.sess.codemap().span_to_snippet(sp) {
306-
return Some(("consider dereferencing the borrow",
361+
return Some((sp,
362+
"consider dereferencing the borrow",
307363
format!("*{}", code)));
308364
}
309365
}
310366
}
311367
}
312368
}
313-
None
314369
}
315-
_ => None,
370+
_ => {}
316371
}
372+
None
317373
}
318374

319375
fn check_for_cast(&self,

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

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
struct Foo;
12+
fn takes_ref(_: &Foo) {}
13+
14+
fn main() {
15+
let ref opt = Some(Foo);
16+
opt.map(|arg| takes_ref(arg));
17+
//~^ ERROR mismatched types [E0308]
18+
opt.and_then(|arg| Some(takes_ref(arg)));
19+
//~^ ERROR mismatched types [E0308]
20+
let ref opt: Result<_, ()> = Ok(Foo);
21+
opt.map(|arg| takes_ref(arg));
22+
//~^ ERROR mismatched types [E0308]
23+
opt.and_then(|arg| Ok(takes_ref(arg)));
24+
//~^ ERROR mismatched types [E0308]
25+
}

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

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/as-ref.rs:16:27
3+
|
4+
LL | opt.map(|arg| takes_ref(arg));
5+
| - ^^^ expected &Foo, found struct `Foo`
6+
| |
7+
| help: consider using `as_ref` instead: `as_ref().`
8+
|
9+
= note: expected type `&Foo`
10+
found type `Foo`
11+
12+
error[E0308]: mismatched types
13+
--> $DIR/as-ref.rs:18:37
14+
|
15+
LL | opt.and_then(|arg| Some(takes_ref(arg)));
16+
| - ^^^ expected &Foo, found struct `Foo`
17+
| |
18+
| help: consider using `as_ref` instead: `as_ref().`
19+
|
20+
= note: expected type `&Foo`
21+
found type `Foo`
22+
23+
error[E0308]: mismatched types
24+
--> $DIR/as-ref.rs:21:27
25+
|
26+
LL | opt.map(|arg| takes_ref(arg));
27+
| - ^^^ expected &Foo, found struct `Foo`
28+
| |
29+
| help: consider using `as_ref` instead: `as_ref().`
30+
|
31+
= note: expected type `&Foo`
32+
found type `Foo`
33+
34+
error[E0308]: mismatched types
35+
--> $DIR/as-ref.rs:23:35
36+
|
37+
LL | opt.and_then(|arg| Ok(takes_ref(arg)));
38+
| - ^^^ expected &Foo, found struct `Foo`
39+
| |
40+
| help: consider using `as_ref` instead: `as_ref().`
41+
|
42+
= note: expected type `&Foo`
43+
found type `Foo`
44+
45+
error: aborting due to 4 previous errors
46+
47+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)