Skip to content

Commit aba56dd

Browse files
committed
type error method suggestions use whitelisted identity-like conversions
Previously, on a type mismatch (and if this wasn't preëmpted by a higher-priority suggestion), we would look for argumentless methods returning the expected type, and list them in a `help` note. This had two major shortcomings. Firstly, a lot of the suggestions didn't really make sense (if you used a &str where a String was expected, `.to_ascii_uppercase()` is probably not the solution you were hoping for). Secondly, we weren't generating suggestions from the most useful traits! We address the first problem with an internal `#[rustc_conversion_suggestion]` attribute meant to mark methods that keep the "same value" in the relevant sense, just converting the type. We address the second problem by making `FnCtxt.probe_for_return_type` pass the `ProbeScope::AllTraits` to `probe_op`: this would seem to be safe because grep reveals no other callers of `probe_for_return_type`. Also, structured suggestions are preferred (because they're pretty, but also for RLS and friends). Also also, we make the E0055 autoderef recursion limit error use the one-time-diagnostics set, because we can potentially hit the limit a lot during probing. (Without this, test/ui/did_you_mean/recursion_limit_deref.rs would report "aborting due to 51 errors"). Unfortunately, the trait probing is still not all one would hope for: at a minimum, we don't know how to rule out `into()` in cases where it wouldn't actually work, and we don't know how to rule in `.to_owned()` where it would. Issues rust-lang#46459 and rust-lang#46460 have been filed and are ref'd in a FIXME. This is hoped to resolve rust-lang#42929, rust-lang#44672, and rust-lang#45777.
1 parent 72176cf commit aba56dd

File tree

14 files changed

+147
-65
lines changed

14 files changed

+147
-65
lines changed

Diff for: src/liballoc/slice.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,7 @@ impl<T> [T] {
15951595
/// let x = s.to_vec();
15961596
/// // Here, `s` and `x` can be modified independently.
15971597
/// ```
1598+
#[rustc_conversion_suggestion]
15981599
#[stable(feature = "rust1", since = "1.0.0")]
15991600
#[inline]
16001601
pub fn to_vec(&self) -> Vec<T>

Diff for: src/liballoc/string.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2034,6 +2034,7 @@ pub trait ToString {
20342034
///
20352035
/// assert_eq!(five, i.to_string());
20362036
/// ```
2037+
#[rustc_conversion_suggestion]
20372038
#[stable(feature = "rust1", since = "1.0.0")]
20382039
fn to_string(&self) -> String;
20392040
}

Diff for: src/librustc_typeck/check/autoderef.rs

+17-10
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use super::{FnCtxt, LvalueOp};
1414
use super::method::MethodCallee;
1515

1616
use rustc::infer::InferOk;
17+
use rustc::session::DiagnosticMessageId;
1718
use rustc::traits;
1819
use rustc::ty::{self, Ty, TraitRef};
1920
use rustc::ty::{ToPredicate, TypeFoldable};
@@ -56,19 +57,25 @@ impl<'a, 'gcx, 'tcx> Iterator for Autoderef<'a, 'gcx, 'tcx> {
5657
return Some((self.cur_ty, 0));
5758
}
5859

59-
if self.steps.len() == tcx.sess.recursion_limit.get() {
60+
if self.steps.len() >= tcx.sess.recursion_limit.get() {
6061
// We've reached the recursion limit, error gracefully.
6162
let suggested_limit = tcx.sess.recursion_limit.get() * 2;
62-
struct_span_err!(tcx.sess,
63-
self.span,
64-
E0055,
65-
"reached the recursion limit while auto-dereferencing {:?}",
66-
self.cur_ty)
67-
.span_label(self.span, "deref recursion limit reached")
68-
.help(&format!(
69-
"consider adding a `#[recursion_limit=\"{}\"]` attribute to your crate",
63+
let msg = format!("reached the recursion limit while auto-dereferencing {:?}",
64+
self.cur_ty);
65+
let error_id = (DiagnosticMessageId::ErrorId(55), Some(self.span), msg.clone());
66+
let fresh = tcx.sess.one_time_diagnostics.borrow_mut().insert(error_id);
67+
if fresh {
68+
struct_span_err!(tcx.sess,
69+
self.span,
70+
E0055,
71+
"reached the recursion limit while auto-dereferencing {:?}",
72+
self.cur_ty)
73+
.span_label(self.span, "deref recursion limit reached")
74+
.help(&format!(
75+
"consider adding a `#![recursion_limit=\"{}\"]` attribute to your crate",
7076
suggested_limit))
71-
.emit();
77+
.emit();
78+
}
7279
return None;
7380
}
7481

Diff for: src/librustc_typeck/check/demand.rs

+34-37
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
use std::iter;
1112

1213
use check::FnCtxt;
1314
use rustc::infer::InferOk;
@@ -137,49 +138,45 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
137138
if let Some((msg, suggestion)) = self.check_ref(expr, checked_ty, expected) {
138139
err.span_suggestion(expr.span, msg, suggestion);
139140
} else {
140-
let mode = probe::Mode::MethodCall;
141-
let suggestions = self.probe_for_return_type(syntax_pos::DUMMY_SP,
142-
mode,
143-
expected,
144-
checked_ty,
145-
ast::DUMMY_NODE_ID);
146-
if suggestions.len() > 0 {
147-
err.help(&format!("here are some functions which \
148-
might fulfill your needs:\n{}",
149-
self.get_best_match(&suggestions).join("\n")));
141+
let methods = self.get_conversion_methods(expected, checked_ty);
142+
if let Ok(expr_text) = self.tcx.sess.codemap().span_to_snippet(expr.span) {
143+
let suggestions = iter::repeat(expr_text).zip(methods.iter())
144+
.map(|(receiver, method)| format!("{}.{}()", receiver, method.name))
145+
.collect::<Vec<_>>();
146+
if !suggestions.is_empty() {
147+
err.span_suggestions(expr.span,
148+
"try using a conversion method",
149+
suggestions);
150+
}
150151
}
151152
}
152153
(expected, Some(err))
153154
}
154155

155-
fn format_method_suggestion(&self, method: &AssociatedItem) -> String {
156-
format!("- .{}({})",
157-
method.name,
158-
if self.has_no_input_arg(method) {
159-
""
160-
} else {
161-
"..."
162-
})
163-
}
164-
165-
fn display_suggested_methods(&self, methods: &[AssociatedItem]) -> Vec<String> {
166-
methods.iter()
167-
.take(5)
168-
.map(|method| self.format_method_suggestion(&*method))
169-
.collect::<Vec<String>>()
170-
}
156+
fn get_conversion_methods(&self, expected: Ty<'tcx>, checked_ty: Ty<'tcx>)
157+
-> Vec<AssociatedItem> {
158+
let mut methods = self.probe_for_return_type(syntax_pos::DUMMY_SP,
159+
probe::Mode::MethodCall,
160+
expected,
161+
checked_ty,
162+
ast::DUMMY_NODE_ID);
163+
methods.retain(|m| {
164+
self.has_no_input_arg(m) &&
165+
self.tcx.get_attrs(m.def_id).iter()
166+
// This special internal attribute is used to whitelist
167+
// "identity-like" conversion methods to be suggested here.
168+
//
169+
// FIXME (#46459 and #46460): ideally
170+
// `std::convert::Into::into` and `std::borrow:ToOwned` would
171+
// also be `#[rustc_conversion_suggestion]`, if not for
172+
// method-probing false-positives and -negatives (respectively).
173+
//
174+
// FIXME? Other potential candidate methods: `as_ref` and
175+
// `as_mut`?
176+
.find(|a| a.check_name("rustc_conversion_suggestion")).is_some()
177+
});
171178

172-
fn get_best_match(&self, methods: &[AssociatedItem]) -> Vec<String> {
173-
let no_argument_methods: Vec<_> =
174-
methods.iter()
175-
.filter(|ref x| self.has_no_input_arg(&*x))
176-
.map(|x| x.clone())
177-
.collect();
178-
if no_argument_methods.len() > 0 {
179-
self.display_suggested_methods(&no_argument_methods)
180-
} else {
181-
self.display_suggested_methods(&methods)
182-
}
179+
methods
183180
}
184181

185182
// This function checks if the method isn't static and takes other arguments than `self`.

Diff for: src/librustc_typeck/check/method/probe.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
190190
scope_expr_id);
191191
let method_names =
192192
self.probe_op(span, mode, None, Some(return_type), IsSuggestion(true),
193-
self_ty, scope_expr_id, ProbeScope::TraitsInScope,
193+
self_ty, scope_expr_id, ProbeScope::AllTraits,
194194
|probe_cx| Ok(probe_cx.candidate_method_names()))
195195
.unwrap_or(vec![]);
196196
method_names
@@ -199,7 +199,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
199199
self.probe_op(
200200
span, mode, Some(method_name), Some(return_type),
201201
IsSuggestion(true), self_ty, scope_expr_id,
202-
ProbeScope::TraitsInScope, |probe_cx| probe_cx.pick()
202+
ProbeScope::AllTraits, |probe_cx| probe_cx.pick()
203203
).ok().map(|pick| pick.item)
204204
})
205205
.collect()

Diff for: src/libstd/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@
229229

230230
// Turn warnings into errors, but only after stage0, where it can be useful for
231231
// code to emit warnings during language transitions
232-
#![deny(warnings)]
232+
#![cfg_attr(not(stage0), deny(warnings))]
233233

234234
// std may use features in a platform-specific way
235235
#![allow(unused_features)]

Diff for: src/libstd/path.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1702,6 +1702,7 @@ impl Path {
17021702
/// let path_buf = Path::new("foo.txt").to_path_buf();
17031703
/// assert_eq!(path_buf, std::path::PathBuf::from("foo.txt"));
17041704
/// ```
1705+
#[rustc_conversion_suggestion]
17051706
#[stable(feature = "rust1", since = "1.0.0")]
17061707
pub fn to_path_buf(&self) -> PathBuf {
17071708
PathBuf::from(self.inner.to_os_string())

Diff for: src/libsyntax/feature_gate.rs

+7
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,13 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG
966966
never be stable",
967967
cfg_fn!(rustc_attrs))),
968968

969+
// whitelists "identity-like" conversion methods to suggest on type mismatch
970+
("rustc_conversion_suggestion", Whitelisted, Gated(Stability::Unstable,
971+
"rustc_attrs",
972+
"this is an internal attribute that will \
973+
never be stable",
974+
cfg_fn!(rustc_attrs))),
975+
969976
("wasm_import_memory", Whitelisted, Gated(Stability::Unstable,
970977
"wasm_import_memory",
971978
"wasm_import_memory attribute is currently unstable",

Diff for: src/test/ui/deref-suggestion.stderr

+4-7
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,13 @@ error[E0308]: mismatched types
22
--> $DIR/deref-suggestion.rs:18:9
33
|
44
18 | foo(s); //~ ERROR mismatched types
5-
| ^ expected struct `std::string::String`, found reference
5+
| ^
6+
| |
7+
| expected struct `std::string::String`, found reference
8+
| help: try using a conversion method: `s.to_string()`
69
|
710
= note: expected type `std::string::String`
811
found type `&std::string::String`
9-
= help: here are some functions which might fulfill your needs:
10-
- .escape_debug()
11-
- .escape_default()
12-
- .escape_unicode()
13-
- .to_ascii_lowercase()
14-
- .to_ascii_uppercase()
1512

1613
error[E0308]: mismatched types
1714
--> $DIR/deref-suggestion.rs:23:10

Diff for: src/test/ui/did_you_mean/recursion_limit_deref.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ error[E0055]: reached the recursion limit while auto-dereferencing I
44
62 | let x: &Bottom = &t; //~ ERROR mismatched types
55
| ^^ deref recursion limit reached
66
|
7-
= help: consider adding a `#[recursion_limit="20"]` attribute to your crate
7+
= help: consider adding a `#![recursion_limit="20"]` attribute to your crate
88

99
error[E0055]: reached the recursion limit while auto-dereferencing I
1010
|
11-
= help: consider adding a `#[recursion_limit="20"]` attribute to your crate
11+
= help: consider adding a `#![recursion_limit="20"]` attribute to your crate
1212

1313
error[E0308]: mismatched types
1414
--> $DIR/recursion_limit_deref.rs:62:22

Diff for: src/test/ui/span/coerce-suggestions.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@ error[E0308]: mismatched types
66
|
77
= note: expected type `usize`
88
found type `std::string::String`
9-
= help: here are some functions which might fulfill your needs:
10-
- .capacity()
11-
- .len()
129

1310
error[E0308]: mismatched types
1411
--> $DIR/coerce-suggestions.rs:19:19
@@ -44,7 +41,10 @@ error[E0308]: mismatched types
4441
--> $DIR/coerce-suggestions.rs:27:9
4542
|
4643
27 | f = box f;
47-
| ^^^^^ cyclic type of infinite size
44+
| ^^^^^
45+
| |
46+
| cyclic type of infinite size
47+
| help: try using a conversion method: `box f.to_string()`
4848

4949
error[E0308]: mismatched types
5050
--> $DIR/coerce-suggestions.rs:31:9

Diff for: src/test/ui/span/issue-34264.stderr

-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ error[E0308]: mismatched types
3333
|
3434
= note: expected type `usize`
3535
found type `&'static str`
36-
= help: here are some functions which might fulfill your needs:
37-
- .len()
3836

3937
error[E0061]: this function takes 2 parameters but 3 parameters were supplied
4038
--> $DIR/issue-34264.rs:20:5

Diff for: src/test/ui/suggestions/conversion-methods.rs

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2017 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+
use std::path::{Path, PathBuf};
12+
13+
14+
fn main() {
15+
let _tis_an_instants_play: String = "'Tis a fond Ambush—"; //~ ERROR mismatched types
16+
let _just_to_make_bliss: PathBuf = Path::new("/ern/her/own/surprise");
17+
//~^ ERROR mismatched types
18+
19+
let _but_should_the_play: String = 2; // Perhaps surprisingly, we suggest .to_string() here
20+
//~^ ERROR mismatched types
21+
22+
let _prove_piercing_earnest: Vec<usize> = &[1, 2, 3]; //~ ERROR mismatched types
23+
}

Diff for: src/test/ui/suggestions/conversion-methods.stderr

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/conversion-methods.rs:15:41
3+
|
4+
15 | let _tis_an_instants_play: String = "'Tis a fond Ambush—"; //~ ERROR mismatched types
5+
| ^^^^^^^^^^^^^^^^^^^^^
6+
| |
7+
| expected struct `std::string::String`, found reference
8+
| help: try using a conversion method: `"'Tis a fond Ambush—".to_string()`
9+
|
10+
= note: expected type `std::string::String`
11+
found type `&'static str`
12+
13+
error[E0308]: mismatched types
14+
--> $DIR/conversion-methods.rs:16:40
15+
|
16+
16 | let _just_to_make_bliss: PathBuf = Path::new("/ern/her/own/surprise");
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
| |
19+
| expected struct `std::path::PathBuf`, found reference
20+
| help: try using a conversion method: `Path::new("/ern/her/own/surprise").to_path_buf()`
21+
|
22+
= note: expected type `std::path::PathBuf`
23+
found type `&std::path::Path`
24+
25+
error[E0308]: mismatched types
26+
--> $DIR/conversion-methods.rs:19:40
27+
|
28+
19 | let _but_should_the_play: String = 2; // Perhaps surprisingly, we suggest .to_string() here
29+
| ^
30+
| |
31+
| expected struct `std::string::String`, found integral variable
32+
| help: try using a conversion method: `2.to_string()`
33+
|
34+
= note: expected type `std::string::String`
35+
found type `{integer}`
36+
37+
error[E0308]: mismatched types
38+
--> $DIR/conversion-methods.rs:22:47
39+
|
40+
22 | let _prove_piercing_earnest: Vec<usize> = &[1, 2, 3]; //~ ERROR mismatched types
41+
| ^^^^^^^^^^
42+
| |
43+
| expected struct `std::vec::Vec`, found reference
44+
| help: try using a conversion method: `&[1, 2, 3].to_vec()`
45+
|
46+
= note: expected type `std::vec::Vec<usize>`
47+
found type `&[{integer}; 3]`
48+
49+
error: aborting due to 4 previous errors
50+

0 commit comments

Comments
 (0)