rustc_parse: improve the error diagnostic for "missing let in let chain"

[Unique-Usman]

No description provided.

[rustbot]

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Unique-Usman]

My approach seems not to be perfect and I have been stucked on this for a while. This is the issue this is supposed to fix - >#117977

[Unique-Usman]

@estebank

[Unique-Usman]

r? @estebank

[Unique-Usman]

This is an improvement for

fn main() {
    let x = Some(42);
    if let Some(_) = x
        && Some(x) = x
    {}
}

initially it gives

error: expected expression, found `let` statement
 --> foo.rs:3:8
  |
3 |     if let Some(_) = x
  |        ^^^^^^^^^^^^^^^
  |
  = note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to continue the let-chain
  |
4 |         && let Some(x) = x
  |            +++
help: you might have meant to compare for equality
  |
4 |         && Some(x) == x
  |                     +

error[E0308]: mismatched types
 --> foo.rs:4:12
  |
4 |         && Some(x) = x
  |            ^^^^^^^ expected `bool`, found `Option<Option<{integer}>>`
  |
  = note: expected type `bool`
             found enum `Option<Option<{integer}>>`
help: use `Option::is_some` to test if the `Option` has a value
  |
4 |         && Some(x).is_some() = x
  |                   ++++++++++

error[E0308]: mismatched types
 --> foo.rs:3:8
  |
3 |       if let Some(_) = x
  |  ________^
4 | |         && Some(x) = x
  | |______________________^ expected `bool`, found `()`

error: aborting due to 3 previous errors

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

but now it gives

error: expected expression, found `let` statement
 --> foo.rs:3:8
  |
3 |     if let Some(_) = x
  |        ^^^^^^^^^^^^^^^
  |
  = note: -Ztrack-diagnostics: created at compiler/rustc_parse/src/parser/expr.rs:4263:51
  = note: only supported directly in conditions of `if` and `while` expressions
help: you might have meant to continue the let-chain
  |
4 |         && let Some(x) = x
  |            +++
help: you might have meant to compare for equality
  |
4 |         && Some(x) == x
  |                     +

error: aborting due to 1 previous error

[Unique-Usman]

Interestingly,

fn main() {
    let x = Some(42);
    if let Some(_) = x
        && Some(x) = x
    {}
}

still give the same result.

error: expected expression, found `let` statement
 --> foo.rs:4:12
  |
4 |         && let Some(x) = x
  |            ^^^
  |
  = note: only supported directly in conditions of `if` and `while` expressions

error[E0308]: mismatched types
 --> foo.rs:3:18
  |
3 |     if Some(_) = x
  |                  ^ expected `bool`, found `Option<{integer}>`
  |
  = note: expected type `bool`
             found enum `Option<{integer}>`

error[E0308]: mismatched types
 --> foo.rs:3:8
  |
3 |       if Some(_) = x
  |  ________^^^^^^^___-
  | |        |
  | |        expected `bool`, found `Option<_>`
4 | |         && let Some(x) = x
  | |__________________________- this expression has type `bool`
  |
  = note: expected type `bool`
             found enum `Option<_>`

error[E0308]: mismatched types
 --> foo.rs:3:8
  |
3 |       if Some(_) = x
  |  ________^
4 | |         && let Some(x) = x
  | |__________________________^ expected `bool`, found `()`
  |
help: consider adding `let`
  |
3 |     if let Some(_) = x
  |        +++

error: aborting due to 4 previous errors

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

[estebank]

I'm not sure I follow your last comment. The diagnostic you're showing doesn't correspond with the code. I think you meant the following?

fn main() {
    let x = Some(42);
    if Some(_) = x
        && let Some(x) = x
    {}
}

If so, this is the output I see:

error: expected expression, found `let` statement
 --> foo.rs:4:12
  |
4 |         && let Some(x) = x
  |            ^^^
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_parse/src/parser/expr.rs:2775:43
  = note: only supported directly in conditions of `if` and `while` expressions

error[E0308]: mismatched types
 --> foo.rs:3:18
  |
3 |     if Some(_) = x
  |                  ^ expected `bool`, found `Option<{integer}>`
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs:1905:35
  = note: expected type `bool`
             found enum `Option<{integer}>`

error[E0308]: mismatched types
 --> foo.rs:3:8
  |
3 |       if Some(_) = x
  |  ________^^^^^^^___-
  | |        |
  | |        expected `bool`, found `Option<_>`
4 | |         && let Some(x) = x
  | |__________________________- this expression has type `bool`
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs:1905:35
  = note: expected type `bool`
             found enum `Option<_>`

error[E0308]: mismatched types
 --> foo.rs:3:8
  |
3 |       if Some(_) = x
  |  ________^
4 | |         && let Some(x) = x
  | |__________________________^ expected `bool`, found `()`
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs:1905:35
help: consider adding `let`
  |
3 |     if let Some(_) = x
  |        +++

And I think it's coming from a different part of the parser, in parse_expr_let, so it would require different specific handling.

View changes since this review

[estebank]

Unrelated: Noticing this suggestion and it is quite bad :-/

[estebank]

This change makes me think that we might want to restrict the silencing to cases where we had any suggestions. The need to remove the run-rustfix generally points at us giving less ideal output. That being said, this code as is is not great to begin with, and == would likely be intended... Can you see if we can make it so that we don't cause changes on this output while still silencing the others?

[Unique-Usman]

Yeah, that is valid. Looks quite weird to me also, I knew we had to do some restrictions, I will do that.

[estebank]

I think all you'd have to do is gate this behind missing_let or comparison being Some.

[Unique-Usman]

Yeah, thanks. I did gate it earlier, but not around Some parts, so it didn’t work. I’ll gate it around it now.

[Unique-Usman]

I'm not sure I follow your last comment. The diagnostic you're showing doesn't correspond with the code. I think you meant the following?

fn main() {
    let x = Some(42);
    if Some(_) = x
        && let Some(x) = x
    {}
}

If so, this is the output I see:

error: expected expression, found `let` statement
 --> foo.rs:4:12
  |
4 |         && let Some(x) = x
  |            ^^^
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_parse/src/parser/expr.rs:2775:43
  = note: only supported directly in conditions of `if` and `while` expressions

error[E0308]: mismatched types
 --> foo.rs:3:18
  |
3 |     if Some(_) = x
  |                  ^ expected `bool`, found `Option<{integer}>`
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs:1905:35
  = note: expected type `bool`
             found enum `Option<{integer}>`

error[E0308]: mismatched types
 --> foo.rs:3:8
  |
3 |       if Some(_) = x
  |  ________^^^^^^^___-
  | |        |
  | |        expected `bool`, found `Option<_>`
4 | |         && let Some(x) = x
  | |__________________________- this expression has type `bool`
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs:1905:35
  = note: expected type `bool`
             found enum `Option<_>`

error[E0308]: mismatched types
 --> foo.rs:3:8
  |
3 |       if Some(_) = x
  |  ________^
4 | |         && let Some(x) = x
  | |__________________________^ expected `bool`, found `()`
  |
  = note: -Ztrack-diagnostics: created at /rustc-dev/4fa80a5e733e2202d7ca4c203c2fdfda41cfe7dc/compiler/rustc_trait_selection/src/error_reYeahporting/infer/mod.rs:1905:35
help: consider adding `let`
  |
3 |     if let Some(_) = x
  |        +++

And I think it's coming from a different part of the parser, in parse_expr_let, so it would require different specific handling.

View changes since this review

Yeah, I meant this. Yeah, it needs to be handle in different part of the code.

[estebank]

@bors r+

@Unique-Usman would you want to take a look at improving the case that is left (where the first condition is an assignment/not a let and then a let is found later)? It is ok if you find that it might require a more significant refactor and then decide it isn't worth it.

[rust-bors]

📌 Commit 9ca8ed3 has been approved by estebank

It is now in the queue for this repository.

[Unique-Usman]

@bors r+

@Unique-Usman would you want to take a look at improving the case that is left (where the first condition is an assignment/not a let and then a let is found later)? It is ok if you find that it might require a more significant refactor and then decide it isn't worth it.

Definitely, I will take a look, thanks for the review.