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

Add helpful error message when detecting Rust style enums #4942

Closed
IGI-111 opened this issue Aug 12, 2023 · 3 comments · Fixed by #5397
Closed

Add helpful error message when detecting Rust style enums #4942

IGI-111 opened this issue Aug 12, 2023 · 3 comments · Fixed by #5397
Assignees
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: parser Everything to do with the parser compiler: ui Mostly compiler messages good first issue Good for newcomers

Comments

@IGI-111
Copy link
Contributor

IGI-111 commented Aug 12, 2023

Users moving from Rust might be tempted to use the Rust enum syntax:

enum A {
    Foo,
    Bar(u32),
}

When the correct Sway syntax is:

enum A {
    Foo: (),
    Bar: u32,
}

This is surprising, so we should have an error message that detects this pattern and asks the developer if they meant the latter.

@IGI-111 IGI-111 added good first issue Good for newcomers compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: parser Everything to do with the parser compiler: ui Mostly compiler messages labels Aug 12, 2023
@ironcev ironcev self-assigned this Aug 12, 2023
@ironcev ironcev removed their assignment Dec 7, 2023
@ironcev
Copy link
Member

ironcev commented Dec 7, 2023

How the error message should look like? Let's use expressive diagnostics and have a detailed error similar to the one Rust is giving in this case:

enum E {
    A: (),
}

Error:

error: expected one of `(`, `,`, `=`, `{`, or `}`, found `:`
 --> src/main.rs:2:6
  |
1 | enum E {
  |      - while parsing this enum
2 |     A: (),
  |      ^ expected one of `(`, `,`, `=`, `{`, or `}`
  |
  = help: enum variants can be `Variant`, `Variant = <integer>`, `Variant(Type, ..., TypeN)` or `Variant { fields: Types }`

Plus the additional Sway specific in-source help message saying, e.g. "Did you mean Foo: ()?" or "Did you mean Bar: u32?"

@jjcnn jjcnn self-assigned this Dec 14, 2023
@jjcnn
Copy link
Contributor

jjcnn commented Dec 14, 2023

Plus the additional Sway specific in-source help message saying, e.g. "Did you mean Foo: ()?" or "Did you mean Bar: u32?"

This help message requires a way for ParseErrorKind to format a sway_ast::ty::Ty, and I can't to find a formatter for that type.

Is it worth the trouble to implement one, or should we instead go with something like "Enum kinds have the form 'Foo : ()' or 'Bar : u32'", without referring to the specific thing the user wrote?

@ironcev
Copy link
Member

ironcev commented Dec 14, 2023

Indeed you are right, the type formatter is available further down in the type checking phase. I don't think that it's worth the trouble. Let's skip the specific in-source help and display just the general hint how enum variants can look like.

jjcnn added a commit that referenced this issue Dec 18, 2023
## Description

Fixes #4942. 

Please do review this carefully. I've added a test triggers the new
error message, but I might very well have forgotten to add something
somewhere.

## 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.

---------

Co-authored-by: Igor Rončević <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler: parser Everything to do with the parser compiler: ui Mostly compiler messages good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants