fix: parsing crashes on code with errors

[doorgan]

Trying to integrate Spitfire into Expert I found a few issues:

• Spitfire.parse_with_comments("@x ]") crashes instead of returning an error:

** (CaseClauseError) no case clause matching: {:error, :no_fuel_remaining}
    (spitfire 0.2.1) lib/spitfire.ex:174: Spitfire.parse_with_comments/2
    iex:27: (file)

• Spitfire.parse("[«]") crashes(this one was triggered by one of Expert's test helpers):

** (FunctionClauseError) no function clause matching in Spitfire.peek_token/1

    The following arguments were given to Spitfire.peek_token/1:

        # 1
        %{
          tokens: [nil | :eot],
          literal_encoder: nil,
          errors: [
            {[line: 1, column: 1], "missing closing bracket for list"},
            {[], "unknown token: eot"}
          ],
          current_token: {:"]", nil},
          peek_token: nil,
          nesting: 0,
          fuel: 150
        }

    Attempted function clauses (showing 7 out of 7):

        defp peek_token(%{peek_token: {:stab_op, _, token}})
        defp peek_token(%{peek_token: {type, _, _, _}}) when type === :list_heredoc or type === :bin_heredoc
        defp peek_token(%{peek_token: {token, _, _}})
        defp peek_token(%{peek_token: {token, _}})
        defp peek_token(%{peek_token: {token, _, _, _, _, _, _}})
        defp peek_token(%{peek_token: :eof})
        defp peek_token(%{tokens: :eot})

    (spitfire 0.2.1) lib/spitfire.ex:2390: Spitfire.peek_token/1
    (spitfire 0.2.1) lib/spitfire.ex:298: anonymous fn/7 in Spitfire.parse_expression/6
    (spitfire 0.2.1) lib/spitfire/while.ex:49: Spitfire.While.do_while/2
    (spitfire 0.2.1) lib/spitfire.ex:196: anonymous fn/1 in Spitfire.parse_program/1
    (spitfire 0.2.1) lib/spitfire/while.ex:5: Spitfire.While2.recurse/3
    (spitfire 0.2.1) lib/spitfire.ex:195: Spitfire.parse_program/1
    (spitfire 0.2.1) lib/spitfire.ex:140: Spitfire.parse/2
    iex:31: (file)

This PR fixes those

[mhanberg]

I'm not sure this exactly the right fix, as the error cases showing 'unknown token: eot' is exposing an implementation detail that isn't a real syntax error.

I think you can maybe split the struct error into its own PR.

I'll dig into it more, how does the tokenizer actually tokenize that special << character?

[doorgan]

I'll dig into it more, how does the tokenizer actually tokenize that special << character?

As [171] which corresponds to "«", which seems correct

I think you can maybe split the struct error into its own PR.

Will do. Also for that one I'm not 100% either that the behavior I chose is the one we really want either, we can discuss further there

[mhanberg]

I merged #67 which fixes the weird character issue.

I need to go to bed, but have another fix in flight for the other error.

The issue is that it should detect if there is a closing bracket/brace/paren when it wasn't expected one, report the error, and continue parsing.

If you want to look into it tomorrow during the day, it'll be around the validate_peek function and look at those "is_list", etc variables in the parse_expression function. Otherwise i can tackle it tomorrow evening.

[doorgan]

@mhanberg it seems the issue was caused by parse_program not advancing the parser if a sub-parser returned before a closing delimiter token. A closing delimiter is never a legal token to start parsing, so:

• In code like @x ] it entered an infinite loop, because it would never advance the parser and it'd try to parse the same token over and over
• In code like Foo.bar(baz)} the sub-parser would consume Foo.bar(baz) but still keep the parser at the closing ), causing it to report "unexpected token" for both ) and }

[doorgan]

One thing that caught my attention is that we have a test with this snippet(doesn't drop the cursor node):

%{state |
          foo: s
        __cursor__()
        ,
          bar: Foo.Bar.load(state.foo, state.baz)}

This would incorrectly report an unexpected ) for the Foo.Bar.load call before this fix, but then I noticed that after parsing, it gets recovered to this:

%{state | foo: s}
s(__cursor__())
nil
Foo.Bar.load(state.foo, state.baz)
nil
nil

Essentially we:

1. Find the unexpected __cursor__() and close the map
2. Extract the following lines from that map
3. The bar: part doesn't make sense outside of a map/kw list, so we drop it

I think dropping bar: there makes sense, but shouldn't the second line be just __cursor__()? The s is already preserved inside the map under the foo key

[mhanberg]

One thing that caught my attention is that we have a test with this snippet(doesn't drop the cursor node):

%{state |

          foo: s

        __cursor__()

        ,

          bar: Foo.Bar.load(state.foo, state.baz)}

This would incorrectly report an unexpected ) for the Foo.Bar.load call before this fix, but then I noticed that after parsing, it gets recovered to this:

%{state | foo: s}

s(__cursor__())

nil

Foo.Bar.load(state.foo, state.baz)

nil

nil

Essentially we:

1. Find the unexpected __cursor__() and close the map

2. Extract the following lines from that map

3. The bar: part doesn't make sense outside of a map/kw list, so we drop it

I think dropping bar: there makes sense, but shouldn't the second line be just __cursor__()? The s is already preserved inside the map under the foo key

Probably best to discuss in a separate issue about improving error recovery.