Weird diagnostic when typing a . instead of a ,

[oli-obk]

Compiling

use std::{foo. bar};
fn main() {}

yields

error: expected one of `,` or `as`, found `.`
 --> src/main.rs:2:14
  |
2 | use std::{foo. bar};
  |              ^ expected one of `,` or `as` here

error: expected one of `;` or `as`, found `.`
 --> src/main.rs:2:14
  |
2 | use std::{foo. bar};
  |              ^ expected one of `;` or `as` here

error: aborting due to 2 previous errors

The first error makes sense, but the second one seems pretty bogus to me.

[Freyskeyd]

Yes I agree, I will take a look at the code.

[estebank]

I could have sworn that there was a dupe for this ticket.

parse_view_path parses only the path of an use statement. It uses parse_path_list_items to parse the contents within braces ({...}). This ends up calling parse_unspanned_seq, which uses parse_seq_to_before_end that just raises errors when it finds them, but you can use parse_seq_to_before_tokens which lets you provide a closure/fn to handle errors. With this you could keep a hashmap of spans, only emit one per span and cancel the rest.

We could go the extra mile and 1) try to be eager in what is accepted by attempting to parse the idents regardless and when finding errors skipping until the next thing that looks like an ident or }, and 2) suggest using commas in these places.

[nikomatsakis]

Another example, from #33188 (closed as a duplicate of this issue):

Compiling the following:

struct S(pub () ());

produces:

<anon>:1:17: 1:18 error: expected one of `+` or `,`, found `(`
<anon>:1 struct S(pub () ());
                         ^
<anon>:1:17: 1:18 error: expected one of `+`, `;`, or `where`, found `(`
<anon>:1 struct S(pub () ());
                         ^

[thombles]

Hi, I'm working on solving this now

[estebank]

@thombles, great! You probably want to modify parse_seq_to_before_end to collect all the errors (by passing a closure that captures a hashmap in that method's body to parse_seq_to_before_tokens, and compare the spans to only emit once per span. This should catch all these cases then.

[thombles]

I've replaced parse_seq_to_before_end with my own function. It turns out it is only responsible for the first emit(), so my HashSet isn't doing any good. Not sure where the second one is coming from yet but I'll track it down.

[estebank]

I see. Let me know if you need any help tracking it down.

[thombles]

I have a new solution coming together. I think it will need to be fairly conservative. The basic problem is that parse_seq_to_before_tokens aborts as soon as it hits a separator error. The calling code assumes that we've parsed up to the end of the sequence when really we're still in the middle of it and this creates new and confusing errors.

The general improvement is to make parse_seq_to_before_tokens consume up to the end of the sequence even when it suffers an error, where possible. This works well. A bit too well.

use std::{foo. bar}; // consumes up to }. Great!
callback(path.as_ref(); // proceeds to next line until it finds ) - Not good. Even worse errors.

So I think we can handle two cases:

1. Missing separators like struct S(pub () ()); - if the "separator" actually parses as a sequence value, emit the error, then attempt to continue.
2. Specific hard-coded wrong characters like use std::{foo. bar};. This was suggested by @petrochenkov in reviewing my first PR. i.e., if we're looking for a , and find a . we will emit an error and continue, but if we find a ; then we need to terminate immediately. It's more likely that the ket is missing and we don't want to eat the following tokens.

I'm working towards a new pull request but I thought I'd write this up here in case anyone has any thoughts!