flix / flix

The Flix Programming Language
https://flix.dev/
Other
2.08k stars 150 forks source link

Parser: suboptimal parser error #7653

Open JonathanStarup opened 1 month ago

JonathanStarup commented 1 month ago

I'm not sure if this is too minor in the current stage of development, but I would hope this error to be prettier at some point

enum E { case Tag(Int32) }
def example(e: E): Int32 = match e {
    case E.tag(x) => x
}
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected CurlyR, but found Dot
12 |     case E.tag(x) => x
               ^
               Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected declaration but found NameLowerCase
12 |     case E.tag(x) => x
                ^^^
                Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected declaration but found ParenL
12 |     case E.tag(x) => x
                   ^
                   Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected declaration but found NameLowerCase
12 |     case E.tag(x) => x
                    ^
                    Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected declaration but found ParenR
12 |     case E.tag(x) => x
                     ^
                     Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected declaration but found ArrowThickR
12 |     case E.tag(x) => x
                       ^^
                       Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected declaration but found NameLowerCase
12 |     case E.tag(x) => x
                          ^
                          Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Expected declaration but found CurlyR
13 | }
     ^
     Here
Syntactic Context: Unknown.
❌ -- Parse Error -------------------------------------------------- .../Example.flix
>> Parse Error: Malformed match rule.
12 |     case E.tag(x) => x
         ^^^^^^
         Here
Syntactic Context: OtherExpr.
herluf-ba commented 1 month ago

Hard agree 👍 Thanks for making me aware of it

herluf-ba commented 1 month ago

@JonathanStarup I think this is already better on master actually. I merged some improvements yesterday. Trying your example I get:

-- Parse Error -------------------------------------------------- main/foo.flix

>> Parse Error: Expected '}' before '.'

3 |     case E.tag(x) => x
             ^
             Here
Syntactic Context: OtherExpr.

-- Parse Error -------------------------------------------------- main/foo.flix

>> Parse Error: Expected <declaration> before '('

3 |     case E.tag(x) => x
                  ^
                  Here
Syntactic Context: OtherDecl.
JonathanStarup commented 1 month ago

Cool :) thats better yeah. will it be something like hey you have a lowercase constructor or how ambitious is the parser errors?

herluf-ba commented 1 month ago

Something like that is feasible, and I hope to do that in the next couple of months :) Right now the first error message stems from the tag-pattern parse rule, specifically in the call to name:

    private def tagPat()(implicit s: State): Mark.Closed = {
      val mark = open()
      name(NAME_TAG, allowQualified = true, context = SyntacticContext.Pat.OtherPat)
      if (at(TokenKind.ParenL)) {
        tuplePat()
      }
      close(mark, TreeKind.Pattern.Tag)
    }

What I would like to to is add another optional argument to name, expect and expectAny to maybe add a contextual tip or something.

name(NAME_TAG, allowQualified = true, context = SyntacticContext.Pat.OtherPat, Some("Constructors use uppercase names"))

Which could land us at:

-- Parse Error -------------------------------------------------- main/foo.flix

>> Expected <Name> before <name>

3 |     case E.tag(x) => x
               ^^^
               Here
Hint: Constructors use uppercase names.

Although there is a nice challenge left, with how qualified names can work when we also terminate fixpoint constraints with ., before that can happen 👍