[move-compiler] Improved declaration handling after a parse error #20498
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
awelc
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
awelc
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
| @@ -1546,6 +1563,9 @@ fn parse_sequence(context: &mut Context) -> Result<Sequence, Box<Diagnostic>> { | |||
| seq.push(item); | |||
| last_semicolon_loc = Some(current_token_loc(context.tokens)); | |||
| if let Err(diag) = consume_token(context.tokens, Tok::Semicolon) { | |||
| if continue_sequence_after_error(context, diag.as_ref().clone()) { | |||
There was a problem hiding this comment.
I'll leave for @cgswords to review, but personally, I would love to see some exploration on using newline information. I think it feels potentially a more "general" solution, but who knows
e.g.
x.foo
// ^
// CURSOR HERE
x.bar();
just looking for let doesn't help here but seeing that the next token is on another line helps a lot.
awelc
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
awelc
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
awelc
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
awelc
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
| @@ -1503,6 +1503,45 @@ fn parse_sequence_item(context: &mut Context) -> Result<SequenceItem, Box<Diagno | |||
| )) | |||
| } | |||
|
|
|||
| // Checks if parsing of a sequence should continue after encountering an error. | |||
| fn continue_sequence_after_error( | |||
There was a problem hiding this comment.
Nit: should_continue_sequence_parsing or similar?
There was a problem hiding this comment.
Changed to should_continue_sequence_after_error
| fn continue_sequence_after_error( | ||
| context: &mut Context, | ||
| diag: Diagnostic, | ||
| last_token_preceded_by_eol: bool, |
There was a problem hiding this comment.
Small nit: since we already take context, we do not strictly need to pass this as well. And it would be weird to pass anything other than context.tokens.last_token_preceded_by_eol().
There was a problem hiding this comment.
Yeah, this wasn't very smart - I simply slapped another parameter when updating the previous version without thinking too much. Thanks! And fixed!
| error[E01002]: unexpected token | ||
| ┌─ tests/move_2024/ide_mode/seq_item_after_incomplete.move:25:9 | ||
| │ | ||
| 25 │ let _tmp1 = 42; |
There was a problem hiding this comment.
Can we also suppress this error in IDE mode? Maybe as a follow-up PR?
There was a problem hiding this comment.
Not sure if we should, though... It's a legitimate parsing error and I can imagine someone forgetting a semicolon at the end of the line and being surprised that the IDE does not show a squiggly but the actual build in CLI fails...
awelc
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
Description
This PR finesses parsing resilience for a specific situation which creates questionable user experience in the IDE. This specific case occurs when a valid sequence item in on the following line from parsing error. Consider the following example:
The
param.bexpression is invalid (as the programmer is still typing) but this should not be be the reason to not compile the followingparam.bar()incorrectly (and generate another error about wrong return value.The following example shows a scenario that's even worse:
Here, when a programmer starts typing an incomplete (and unparsable)
let v =, if we don't compile declaration of_tmp1correctly, it would lead to unnecessary cascading errors to appear due to_tmp1being undefined.Test plan
Additional tests have been added. All tests (including the modified ones) must pass
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.