Skip to content
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.

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

Incorrect suggestion on type error #55336

Closed
estebank opened this issue Oct 25, 2018 · 1 comment
Closed

Incorrect suggestion on type error #55336

estebank opened this issue Oct 25, 2018 · 1 comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@estebank
Copy link
Contributor

We're incorrectly suggesting wrapping with Ok() when it won't help:

error[E0308]: try expression alternatives have incompatible types
   -->
    |
503 | /   segments
504 | |     .next()
505 | |     .ok_or(ApplicationIssue::InvalidMessage(foo::Error::InvalidHeader))?
    | |________________________________________________________________________^ expected struct `failure::Error`, found enum `foo::Error`
    |
    = note: expected type `std::result::Result<_, failure::Error>`
               found type `std::result::Result<_, foo::Error>`
help: try wrapping with a success variant
    |
503 |   Ok(segments
504 |     .next()
505 |     .ok_or(ApplicationIssue::InvalidMessage(foo::Error::InvalidHeader))?)

It should be suggesting using using map_err instead.

@estebank estebank added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 25, 2018
@zackmdavis
Copy link
Member

Duplicate of #52537. Let me commit to submitting a fix for this on (or, less likely, before) Sunday the twenty-eighth (hopefully preserving the Ok-wrapping suggestion in the cases where it is correct, but if we have to scrap the whole suggestion, we will; false-positives are pretty bad)

zackmdavis added a commit to zackmdavis/rust that referenced this issue Oct 27, 2018
This suggestion was introduced in rust-lang#51938 / 6cc78bf (while
introducing different language for type errors coming from `?` rather
than a `match`), but it has a lot of false-positives (as repeatedly
reported in Issues rust-lang#52537, rust-lang#52598, rust-lang#54578, rust-lang#55336), and incorrect
suggestions carry more badness than marginal good suggestions do
goodness. Just get rid of it (unless and until someone figures out how
to do it correctly).

Resolves rust-lang#52537, resolves rust-lang#54578.
pietroalbini pushed a commit to alexcrichton/rust that referenced this issue Oct 29, 2018
This suggestion was introduced in rust-lang#51938 / 6cc78bf (while
introducing different language for type errors coming from `?` rather
than a `match`), but it has a lot of false-positives (as repeatedly
reported in Issues rust-lang#52537, rust-lang#52598, rust-lang#54578, rust-lang#55336), and incorrect
suggestions carry more badness than marginal good suggestions do
goodness. Just get rid of it (unless and until someone figures out how
to do it correctly).

Resolves rust-lang#52537, resolves rust-lang#54578.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints
Projects
None yet
Development

No branches or pull requests

2 participants