diff options
author | Esteban Küber <esteban@kuber.com.ar> | 2018-05-26 17:02:56 -0700 |
---|---|---|
committer | Esteban Küber <esteban@kuber.com.ar> | 2018-05-30 09:47:09 -0700 |
commit | a19a03a31fc1145f66513de74e9b5f89558719ec (patch) | |
tree | ee0b22341763cd0cb2585dacd9b65fab7c6d8d47 | |
parent | Auto merge of #50955 - steveklabnik:update-libbacktrace, r=alexcrichton (diff) | |
download | grust-a19a03a31fc1145f66513de74e9b5f89558719ec.tar.gz grust-a19a03a31fc1145f66513de74e9b5f89558719ec.tar.bz2 grust-a19a03a31fc1145f66513de74e9b5f89558719ec.tar.xz |
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.
-rw-r--r-- | src/librustc_typeck/check/demand.rs | 88 | ||||
-rw-r--r-- | src/test/ui/suggestions/as-ref.rs | 25 | ||||
-rw-r--r-- | src/test/ui/suggestions/as-ref.stderr | 47 |
3 files changed, 144 insertions, 16 deletions
diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 06d854c15f..5b922af821 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs | |||
@@ -19,7 +19,7 @@ use syntax::util::parser::PREC_POSTFIX; | |||
19 | use syntax_pos::Span; | 19 | use syntax_pos::Span; |
20 | use rustc::hir; | 20 | use rustc::hir; |
21 | use rustc::hir::def::Def; | 21 | use rustc::hir::def::Def; |
22 | use rustc::hir::map::NodeItem; | 22 | use rustc::hir::map::{NodeItem, NodeExpr}; |
23 | use rustc::hir::{Item, ItemConst, print}; | 23 | use rustc::hir::{Item, ItemConst, print}; |
24 | use rustc::ty::{self, Ty, AssociatedItem}; | 24 | use rustc::ty::{self, Ty, AssociatedItem}; |
25 | use rustc::ty::adjustment::AllowTwoPhase; | 25 | use rustc::ty::adjustment::AllowTwoPhase; |
@@ -140,8 +140,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
140 | } | 140 | } |
141 | } | 141 | } |
142 | 142 | ||
143 | if let Some((msg, suggestion)) = self.check_ref(expr, checked_ty, expected) { | 143 | if let Some((sp, msg, suggestion)) = self.check_ref(expr, checked_ty, expected) { |
144 | err.span_suggestion(expr.span, msg, suggestion); | 144 | err.span_suggestion(sp, msg, suggestion); |
145 | } else if !self.check_for_cast(&mut err, expr, expr_ty, expected) { | 145 | } else if !self.check_for_cast(&mut err, expr, expr_ty, expected) { |
146 | let methods = self.get_conversion_methods(expr.span, expected, checked_ty); | 146 | let methods = self.get_conversion_methods(expr.span, expected, checked_ty); |
147 | if let Ok(expr_text) = self.tcx.sess.codemap().span_to_snippet(expr.span) { | 147 | 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> { | |||
194 | } | 194 | } |
195 | } | 195 | } |
196 | 196 | ||
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 | |||
197 | /// This function is used to determine potential "simple" improvements or users' errors and | 248 | /// This function is used to determine potential "simple" improvements or users' errors and |
198 | /// provide them useful help. For example: | 249 | /// provide them useful help. For example: |
199 | /// | 250 | /// |
@@ -214,7 +265,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
214 | expr: &hir::Expr, | 265 | expr: &hir::Expr, |
215 | checked_ty: Ty<'tcx>, | 266 | checked_ty: Ty<'tcx>, |
216 | expected: Ty<'tcx>) | 267 | expected: Ty<'tcx>) |
217 | -> Option<(&'static str, String)> { | 268 | -> Option<(Span, &'static str, String)> { |
269 | let sp = expr.span; | ||
218 | match (&expected.sty, &checked_ty.sty) { | 270 | match (&expected.sty, &checked_ty.sty) { |
219 | (&ty::TyRef(_, exp, _), &ty::TyRef(_, check, _)) => match (&exp.sty, &check.sty) { | 271 | (&ty::TyRef(_, exp, _), &ty::TyRef(_, check, _)) => match (&exp.sty, &check.sty) { |
220 | (&ty::TyStr, &ty::TyArray(arr, _)) | | 272 | (&ty::TyStr, &ty::TyArray(arr, _)) | |
@@ -222,24 +274,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
222 | if let hir::ExprLit(_) = expr.node { | 274 | if let hir::ExprLit(_) = expr.node { |
223 | let sp = self.sess().codemap().call_span_if_macro(expr.span); | 275 | let sp = self.sess().codemap().call_span_if_macro(expr.span); |
224 | if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(sp) { | 276 | 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`", | ||
226 | src[1..].to_string())); | 279 | src[1..].to_string())); |
227 | } | 280 | } |
228 | } | 281 | } |
229 | None | ||
230 | }, | 282 | }, |
231 | (&ty::TyArray(arr, _), &ty::TyStr) | | 283 | (&ty::TyArray(arr, _), &ty::TyStr) | |
232 | (&ty::TySlice(arr), &ty::TyStr) if arr == self.tcx.types.u8 => { | 284 | (&ty::TySlice(arr), &ty::TyStr) if arr == self.tcx.types.u8 => { |
233 | if let hir::ExprLit(_) = expr.node { | 285 | if let hir::ExprLit(_) = expr.node { |
234 | let sp = self.sess().codemap().call_span_if_macro(expr.span); | 286 | let sp = self.sess().codemap().call_span_if_macro(expr.span); |
235 | if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(sp) { | 287 | 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`", | ||
237 | format!("b{}", src))); | 290 | format!("b{}", src))); |
238 | } | 291 | } |
239 | } | 292 | } |
240 | None | ||
241 | } | 293 | } |
242 | _ => None, | 294 | _ => {} |
243 | }, | 295 | }, |
244 | (&ty::TyRef(_, _, mutability), _) => { | 296 | (&ty::TyRef(_, _, mutability), _) => { |
245 | // Check if it can work when put into a ref. For example: | 297 | // Check if it can work when put into a ref. For example: |
@@ -266,17 +318,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
266 | hir::ExprCast(_, _) | hir::ExprBinary(_, _, _) => format!("({})", src), | 318 | hir::ExprCast(_, _) | hir::ExprBinary(_, _, _) => format!("({})", src), |
267 | _ => src, | 319 | _ => src, |
268 | }; | 320 | }; |
321 | if let Some(sugg) = self.can_use_as_ref(expr) { | ||
322 | return Some(sugg); | ||
323 | } | ||
269 | return Some(match mutability { | 324 | return Some(match mutability { |
270 | hir::Mutability::MutMutable => { | 325 | hir::Mutability::MutMutable => { |
271 | ("consider mutably borrowing here", format!("&mut {}", sugg_expr)) | 326 | (sp, "consider mutably borrowing here", format!("&mut {}", |
327 | sugg_expr)) | ||
272 | } | 328 | } |
273 | hir::Mutability::MutImmutable => { | 329 | hir::Mutability::MutImmutable => { |
274 | ("consider borrowing here", format!("&{}", sugg_expr)) | 330 | (sp, "consider borrowing here", format!("&{}", sugg_expr)) |
275 | } | 331 | } |
276 | }); | 332 | }); |
277 | } | 333 | } |
278 | } | 334 | } |
279 | None | ||
280 | } | 335 | } |
281 | (_, &ty::TyRef(_, checked, _)) => { | 336 | (_, &ty::TyRef(_, checked, _)) => { |
282 | // We have `&T`, check if what was expected was `T`. If so, | 337 | // We have `&T`, check if what was expected was `T`. If so, |
@@ -292,7 +347,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
292 | // Maybe remove `&`? | 347 | // Maybe remove `&`? |
293 | hir::ExprAddrOf(_, ref expr) => { | 348 | hir::ExprAddrOf(_, ref expr) => { |
294 | if let Ok(code) = self.tcx.sess.codemap().span_to_snippet(expr.span) { | 349 | 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)); |
296 | } | 351 | } |
297 | } | 352 | } |
298 | 353 | ||
@@ -303,17 +358,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
303 | expr.span) { | 358 | expr.span) { |
304 | let sp = self.sess().codemap().call_span_if_macro(expr.span); | 359 | let sp = self.sess().codemap().call_span_if_macro(expr.span); |
305 | if let Ok(code) = self.tcx.sess.codemap().span_to_snippet(sp) { | 360 | 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", | ||
307 | format!("*{}", code))); | 363 | format!("*{}", code))); |
308 | } | 364 | } |
309 | } | 365 | } |
310 | } | 366 | } |
311 | } | 367 | } |
312 | } | 368 | } |
313 | None | ||
314 | } | 369 | } |
315 | _ => None, | 370 | _ => {} |
316 | } | 371 | } |
372 | None | ||
317 | } | 373 | } |
318 | 374 | ||
319 | fn check_for_cast(&self, | 375 | fn check_for_cast(&self, |
diff --git a/src/test/ui/suggestions/as-ref.rs b/src/test/ui/suggestions/as-ref.rs new file mode 100644 index 0000000000..ae1c98c856 --- /dev/null +++ b/src/test/ui/suggestions/as-ref.rs | |||
@@ -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 | } | ||
diff --git a/src/test/ui/suggestions/as-ref.stderr b/src/test/ui/suggestions/as-ref.stderr new file mode 100644 index 0000000000..27016445ec --- /dev/null +++ b/src/test/ui/suggestions/as-ref.stderr | |||
@@ -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`. | ||