Skip to content

Conversion errors from &str to String should suggest .to_owned() instead #45777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
mqudsi opened this issue Nov 5, 2017 · 4 comments
Closed

Conversion errors from &str to String should suggest .to_owned() instead #45777

mqudsi opened this issue Nov 5, 2017 · 4 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-diagnostics Working group: Diagnostics

Comments

@mqudsi
Copy link
Contributor

mqudsi commented Nov 5, 2017

Currently, attempting to compile code that expects a String while providing an &'static str causes the following to be shown:

error[E0308]: mismatched types
  --> src/incoming.rs:70:90
   |
70 |             timestamp: headers.get_raw("x-timestamp").unwrap_or(ErrorKind::MissingHeader("x-timestamp"))?,
   |                                                                                          ^^^^^^^^^^^^^ expected struct `std::string::String`, found reference
   |
   = note: expected type `std::string::String`
              found type `&'static str`
   = help: here are some functions which might fulfill your needs:
           - .escape_debug()
           - .escape_default()
           - .escape_unicode()
           - .to_lowercase()
           - .to_uppercase()

I'm guessing that these suggestions are automatically generated/derived by the compiler, but none of them are good "generic" suggestions.

The compiler should probably suggest either .to_owned() or .into() in this case instead.
@mqudsi mqudsi changed the title Conversion errors from &str to String should suggestion .to_owned() Conversion errors from &str to String should suggest .to_owned() instead Nov 5, 2017
@zackmdavis
Copy link
Member

(See also #42929 for related discussion.)

@kennytm kennytm added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Nov 5, 2017
@steveklabnik
Copy link
Member

to_string is the idiomatic choice here.

@mqudsi
Copy link
Contributor Author

mqudsi commented Nov 5, 2017

@steveklabnik thanks... man, do I wish rust wasn't 1.0 yet so all these naming issues could be cleaned up.

@zackmdavis zackmdavis mentioned this issue Nov 13, 2017
Closed
@zackmdavis zackmdavis mentioned this issue Dec 3, 2017
Merged
@estebank estebank added the WG-diagnostics Working group: Diagnostics label Dec 8, 2017
zackmdavis added a commit to zackmdavis/rust that referenced this issue Jan 7, 2018
@zackmdavis
type error method suggestions use whitelisted identity-like conversions
aba56dd 
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.
bors added a commit that referenced this issue Jan 13, 2018
@bors
Auto merge of #46461 - zackmdavis:elemental_method_suggestion_jambore…
6eff103 
…e, r=estebank

 type error method suggestions use whitelisted identity-like conversions

![method_jamboree_summit](https://user-images.githubusercontent.com/1076988/33523646-e5c43184-d7c0-11e7-98e5-1bff426ade86.png)

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 pretty and good for RLS and friends.

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 #46459 and #46460 have been filed and are ref'd in a FIXME.

This is hoped to resolve #42929, #44672, and #45777.
@estebank
Copy link
Contributor

Current output:

error[E0308]: mismatched types
 --> src/main.rs:2:21
  |
2 |     let x: String = "";
  |                     ^^
  |                     |
  |                     expected struct `std::string::String`, found reference
  |                     help: try using a conversion method: `"".to_string()`
  |
  = note: expected type `std::string::String`
             found type `&'static str`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-diagnostics Working group: Diagnostics
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@steveklabnik @kennytm @mqudsi @zackmdavis @estebank