gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
17.25k stars 715 forks source link

'Expected one of : "}" a record constructor' -- Error message improvement for custom types #3324

Closed sajorobinson closed 1 month ago

sajorobinson commented 2 months ago

I was having trouble creating a custom type and was having trouble understanding the error message.

Expected one of:
"}"
a record constructor

I later learned I was trying to make the type incorrectly. I needed to make a constructor for it. Knowing that now, the error message makes a lot more sense, but I didn't understand it then.

The error message could possibly be improved to be clearer. Possibly:

A screenshot is below.

Screenshot_2024-06-22_at_18 54 18

Thanks!

lpil commented 2 months ago

Thank you!

rdghosal commented 2 months ago

I'm taking a look! 👀

lpil commented 2 months ago

Thank you! What do you think the error ought to be?

rdghosal commented 2 months ago

@lpil I think that "a record constructor" and the message contents are pretty descriptive, but I think a change in formatting could improve readability.

I'm currently working on changing this error message to look something like the following... What do you think? (Note, the expected tokens will indeed be inlined as shown.)

Expected a "}" or a record constructor
sajorobinson commented 2 months ago

Something I noticed is the way the error is presented, it seems like it's supposed to be an unordered list of possible things it's expecting.

I think I would have interpreted its meaning more quickly if it looked like:

Expected one of:

- "}"
- A record constructor

or, if markdown unordered lists aren't desirable, something using indents like:

Expected one of:

    "}"
    A record constructor
rdghosal commented 2 months ago

Agreed; if we go with a list, there needs to be something that denotes list items (the capital "A" in "A record" is a nice touch too).

Also, I noticed that my proposed solution results in horizontal growth of the error message, though, in practice, this shouldn't be an issue. I suppose it's a matter of preference...

lpil commented 2 months ago

Seeing as this is a common mistake perhaps we could show what the correct code should be. We could use the name of the type as the name of the constructor we display

lpil commented 2 months ago
pub type User {
  name: String
}
error: Syntax error
   ┌─ /Users/louis/src/lpil/puck/src/puck.gleam:23:3
   │
23 │   name: String
   │   ^^^^ I was not expecting this

Each custom type variant must have a constructor:

    pub type User {
      User(
        name: String,
      )
    }
rdghosal commented 2 months ago

Ooo, that's a great idea 👌

I'll see if I can fold in a similar approach for other instances of unexpected tokens.

giacomocavalieri commented 1 month ago

This can be closed I think!