Skip to content

Suggest ; on parse error when applicable #87197

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

Closed
estebank opened this issue Jul 16, 2021 · 9 comments · Fixed by #87436
Closed

Suggest ; on parse error when applicable #87197

estebank opened this issue Jul 16, 2021 · 9 comments · Fixed by #87436
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The lexing & parsing of Rust source code to an AST E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Jul 16, 2021

Given the following code:

fn main() {
    let x = 100;
    println!("{}", x)
    let y = 200;
    println!("{}", y)
}

The current output is:

error: expected one of `.`, `;`, `?`, `}`, or an operator, found keyword `let`
 --> src/main.rs:4:5
  |
3 |     println!("{}", x)
  |                      - expected one of `.`, `;`, `?`, `}`, or an operator
4 |     let y = 200;
  |     ^^^ unexpected token

We should detect cases where a semicolon would be appropriate (probe that the current token and maybe limited lookahead confirm that what comes next could be a valid statement on its own) and suggest a semicolon. If there is a missing }, we already suggest that it might belong there. We might want to silence one or the other depending on whether the current token might be starting a statement or an item (let vs struct, etc.).

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The lexing & parsing of Rust source code to an AST T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 16, 2021
@khang00

This comment has been minimized.

@khang00

This comment has been minimized.

@pierwill
Copy link
Member

Just a guess: could this be addressed with another else if self.look_ahead(1, |t|) { block, as in

} else if self.look_ahead(1, |t| {
?

@pierwill
Copy link
Member

pierwill commented Jul 19, 2021

I think we would need to answer this question: why is the logic here not handling the cases we want to handle?

} else if self.look_ahead(0, |t| {
t == &token::CloseDelim(token::Brace)
|| (
t.can_begin_expr() && t != &token::Semi && t != &token::Pound
// Avoid triggering with too many trailing `#` in raw string.
)
}) {
// Missing semicolon typo. This is triggered if the next token could either start a
// new statement or is a block close. For example:
//
// let x = 32
// let y = 42;
let sp = self.prev_token.span.shrink_to_hi();
self.struct_span_err(sp, &msg)
.span_label(self.token.span, "unexpected token")
.span_suggestion_short(sp, "add `;` here", ";".to_string(), appl)
.emit();
return Ok(());
}

@pierwill
Copy link
Member

Just a guess: could this be addressed with another else if self.look_ahead(1, |t|) { block, as in

} else if self.look_ahead(1, |t| {

?

This seems to be on track given the clue "probe that the current token and maybe limited lookahead confirm that what comes next could be a valid statement on its own"...

@pierwill

This comment has been minimized.

@pierwill

This comment has been minimized.

@estebank
Copy link
Contributor Author

estebank commented Jul 19, 2021

I think we would need to answer this question: why is the logic here not handling the cases we want to handle?

That is a great question: as the code is written it looks like it should work. I don't know why it isn't triggering :(

It might be that expect_semi() isn't being called in these case. It might be a good idea to uplift most of this method into

pub(super) fn expected_one_of_not_found(
&mut self,
edible: &[TokenKind],
inedible: &[TokenKind],
) -> PResult<'a, bool /* recovered */> {

@ebobrow
Copy link
Contributor

ebobrow commented Jul 19, 2021

I'd like to give this a try.

@rustbot claim

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-parser Area: The lexing & parsing of Rust source code to an AST E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants