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

Special case suggestion for missing tail expression when dealing with Ok(()) and Some(()) #90553

Closed
estebank opened this issue Nov 4, 2021 · 4 comments · Fixed by #90575 or #93072
Closed
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. D-papercut Diagnostics: An error or lint that needs small tweaks. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Nov 4, 2021

The suggestion in the following code isn't great:

error[E0308]: mismatched types
  --> src/main.rs:72:5
   |
72 | /     while let Some(i) = x.try_next().await? {
73 | |         println!("{:?}", i);
74 | |     }
   | |_____^ expected enum `Option`, found `()`
   |
   = note:   expected enum `Option<()>`
           found unit type `()`
help: try using a variant of the expected enum
   |
72 ~     Some(while let Some(i) = x.try_next().await? {
73 +         println!("{:?}", i);
74 +     })
   |

It makes sense why it happens: the while let expression resolves to type (), it's the tail expression of an async fn that returns Option<()>, so wrapping the expression in Some(()) would make the type checker happy, but suggesting that seems almost always wrong? I think I would prefer it if we special cased this and didn't gave a suggestion for Result<(), _> or Option<()>.

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-papercut Diagnostics: An error or lint that needs small tweaks. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Nov 4, 2021
@m-ou-se m-ou-se self-assigned this Nov 4, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Nov 4, 2021

How about something like this?

image

@m-ou-se
Copy link
Member

m-ou-se commented Nov 4, 2021

(Just need to figure out a way to detect whether the expression was the final one in a block.)

@estebank
Copy link
Contributor Author

estebank commented Nov 4, 2021

That looks good!

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 18, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 18, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 19, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 19, 2021
@bors bors closed this as completed in 7354bb3 Nov 20, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jan 19, 2022

Re-opening this, because it doesn't work yet for for loops:

help: try wrapping the expression in `Ok`
  |
4 ~     Ok(for _ in x {
5 |         ...
6 ~     })
  |

@m-ou-se m-ou-se reopened this Jan 19, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 17, 2022
…ion-with-desugaring, r=estebank

Compatible variants suggestion with desugaring

This fixes rust-lang#90553 for `for` loops and other desugarings.

r? `@estebank`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 2, 2022
…ion-with-desugaring, r=estebank

Compatible variants suggestion with desugaring

This fixes rust-lang#90553 for `for` loops and other desugarings.

r? `@estebank`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 2, 2022
…ion-with-desugaring, r=estebank

Compatible variants suggestion with desugaring

This fixes rust-lang#90553 for `for` loops and other desugarings.

r? ``@estebank``
@bors bors closed this as completed in bdad4a7 Mar 3, 2022
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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. D-papercut Diagnostics: An error or lint that needs small tweaks. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
2 participants