Skip to content
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

parser recovery: let statement without an = sign fails to return an AST #4911

Closed
JoshuaBatty opened this issue Aug 4, 2023 · 0 comments · Fixed by #4925
Closed

parser recovery: let statement without an = sign fails to return an AST #4911

JoshuaBatty opened this issue Aug 4, 2023 · 0 comments · Fixed by #4925
Assignees

Comments

@JoshuaBatty
Copy link
Member

JoshuaBatty commented Aug 4, 2023

Looking at the below example, we can see squiggly lines are drawn under all of the typed tokens that could be parsed in the language server.

Screenshot 2023-07-26 at 12 45 01 pm

Now, if we add let at the top of the function then you notice we lose access to all the tokens.

Screenshot 2023-07-26 at 12 49 22 pm

The parser fails to recover on the below examples.

let
let x
let x:
let x: u8

and only recovers when an = sign is typed leading to an expression

let x = 
@xunilrj xunilrj self-assigned this Aug 11, 2023
xunilrj added a commit that referenced this issue Aug 15, 2023
## Description

This PR closes #4911.

It primarily does two things:

1 - Extend a lot of functions that return `CompileError` to return
`Vec<CompileError>` allowing multiple errors to return;
2 - Implements the concept of parser recovery.

This happens when using the function `guarded_parse_with_recovery`,
exemplified below. When the guard fails, this function return
`Ok(None)`; when it succeeds it returns `Ok(item)`. The exciting part is
when the parsing fails.

It returns an instance of `Recoverer` which contains a reference to the
original parser before any tentative parser was done and the forked as
left by the parsing function.

The idea is that it is the caller's responsibility to put the forked
parser in a "good position". For that, it offers some helpers function
and access to the forked parser.

To avoid boilerplate code one recovery strategy is already implemented
`recover_at_next_line_with_fallback_error` which consumes everything at
the forked parser's current line and emits an error if none were
generated by the tentative parser.

```rust
 match parser.guarded_parse_with_recovery::<LetToken, StatementLet>() {
        Ok(None) => {}
        Ok(Some(item)) => return stmt(Statement::Let(item)),
        Err(r) => {
            let (spans, error) =
                r.recover_at_next_line_with_fallback_error(ParseErrorKind::InvalidStatement);
            return stmt(Statement::Error(spans, error));
        }
    }
```

With this, we have the LSP not "dying" when a strange error happens. The
errors themselves are not brilliant, but they will be improved in other
PRs.


![image](https://github.com/FuelLabs/sway/assets/83425/234b8b34-d19e-45ec-9fd3-d538ff5c06d2)

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants