Detect missing else in let statement

[bb1yd]

View all comments

Fixes #135857

I add the help message in rustc_resolve. It now will display something like this:

error: expected identifier, found keyword `return`
 --> ./test.rs:8:24
  |
8 |      let Some(a) = bar{return;};
  |                    --- ^^^^^^ expected identifier, found keyword
  |                    |
  |                    while parsing this struct
  |
help: escape `return` to use it as an identifier
  |
8 |      let Some(a) = bar{r#return;};
  |                        ++

error[E0574]: expected struct, variant or union type, found local variable `bar`
 --> ./test.rs:8:20
  |
8 |      let Some(a) = bar{return;};
  |                    ^^^ not a struct, variant or union type
  |
help: try adding `else` here:
  |
8 |      let Some(a) = bar else {return;};
  |                        ++++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0574`.

r? estebank

[bb1yd]

@bors squash msg="fix issue-156949"

[rust-bors]

🔨 4 commits were squashed into 6bbafec.

[estebank]

Thank you for taking this on! I have some nitpicks for things that we should touch-up, but I think you're in the right direction.

View changes since this review

[estebank]

We can address this later, but we are emitting two errors per mistake here, where we should strive to emit a single one. I originally envisioned dealing with this during parse instead of during resolution. Doing this during resolution does give us the benefit of being able to identify that foo is a local binding instead of a struct, which increases the accuracy of the diagnostic, so that's reasonable.

[bb1yd]

I think I should just handle this in the parser. I think it's not worth to use stash and try_steal_modify_and_emit_err on this simple issue.

[bb1yd]

@estebank Fixed

[bb1yd]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

[bb1yd]

I change the code to rustc_parse and it now only emit one error:

error: expected identifier, found keyword `return`
  --> $DIR/detect-missing-else-in-let-1.rs:4:23
   |
LL |     let Some(a) = foo{return;};
   |                   --- ^^^^^^ expected identifier, found keyword
   |                   |
   |                   while parsing this struct
   |
help: escape `return` to use it as an identifier
   |
LL |     let Some(a) = foo{r#return;};
   |                       ++
help: you might have meant to write a diverging block on a refutable `let` statement by using `let-else`
      for more information, visit <https://doc.rust-lang.org/beta/rust-by-example/flow_control/let_else.html>
   |
LL |     let Some(a) = foo else {return;};
   |                       ++++

error: aborting due to 1 previous error

@rustbot ready

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.