Opportunity to improve error message for mistyped inclusive range (`..==`)

[timClicks]

Given the following code: [playground link]

fn main() {
    println!("{:?}", 0 ..== 1)
}

The current output is:

  Compiling playground v0.0.1 (/playground)
error[E0586]: inclusive range with no end
 --> src/main.rs:2:24
  |
2 |     println!("{:?}", 0 ..== 1)
  |                        ^^^ help: use `..` instead
  |
  = note: inclusive ranges must be bounded at the end (`..=b` or `a..=b`)

error: expected `,`, found `=`
 --> src/main.rs:2:27
  |
2 |     println!("{:?}", 0 ..== 1)
  |                           ^ expected `,`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0586`.
error: could not compile `playground`

To learn more, run the command again with --verbose.

Ideally the output should look like:

  Compiling playground v0.0.1 (/playground)
error[E0XXXX]: inclusive range typo
 --> src/main.rs:2:24
  |
2 |     println!("{:?}", 0 ..== 1)
  |                        ^^^^ help: use `..=` instead
  |
  = note: inclusive ranges end with a single equals sign (`..=`)

[timClicks]

Other test cases, provided by @estebank

fn main() {
    println!("{:?}", 0..1 == 1);
    println!("{:?}", 0.. == 1);
    println!("{:?}", 0..== 1);
}

[inquisitivecrystal]

Thank you for filing this! I'm going to have a look and see if I can figure it out.

@rustbot claim
@rustbot label +C-enhancement +D-newcomer-roadblock

[inquisitivecrystal]

This is surprisingly complicated in a bunch of ways.

The relevant function, at least for the base case, is parse_pat_range_begin_with. At first glance, all you've gotta do is handle the case where there's a Eq token next in the queue.

Now for the problems. First off, implementing this cleanly requires adding at least one new feature to Parser. Currently, the way you check if a token is next up is to call the eat function. However, this function in turn calls check, which logs whatever token it's checking for as an expected token. This token isn't expected. It being logged as an expected token messes up the error message for other cases. The solution is to add versions of eat and check that do not log the token as expected. Adding these alternate versions is probably a good idea anyway, since this isn't the first time the problem has come up.

Then there's the whole context sensitivity thing. Dealing with this the way Esteban suggested is way above my pay grade. But you've got to do something, or you can't even handle simple cases like if let X..= = 0. What I could realistically do is look for ..== without any spaces separating the two ==. That isn't the right way to handle it, but it would work for most cases. However, even that requires another change to Parser. It would need to log the Spacing of the token before the present token. That's somewhat reasonable, as it already logs the previous token itself. Still, it's a bit much as a change to make to implement something in a quick but dirty way.

I'm open to advice on whether a minimalist approach might be acceptable, but I'm currently inclined to leave this for someone who actually knows what they're doing.

[estebank]

@inquisitivecrystal you don't have to add a new method, you use look_ahead. You can see how to use it for example in is_keyword_ahead. That function doesn't affect the "expected token" list and won't cause any effects to existing error messages.

As for the context sensitive checks, you can also use look_ahead to see if the next thing is something that looks like a range, a literal or an identifier and use that, as well as checking ..='s span.hi() against the ='s span.lo() to see if they touch or not (differentiating between ..= = and ..==. The .. == case will need to be dealt with in a different part of the parser I think, and should recover in such a way as if you had written(x..) ==.

Now, we don't need to address all the cases, we can work incrementally, starting on the low hanging fruit and eventually getting to the "perfect output from the compiler reading my mind". :)

[inquisitivecrystal]

Thanks, @estebank. That's extraordinarily helpful.

I've implemented this, limited to the ..== case (so not ..= = or .. ==). I'm going to wait till #83918 is merged, since that will simplify my implementation substantially and they'd merge conflict anyway. I'll r? you when I open the PR.

Now to try this again:
@rustbot claim
@rustbot label +C-enhancement +D-newcomer-roadblock

[timClicks]

Wow, well done @inquisitivecrystal. Over time I hope that I can improve things myself. Nice work!