fix: ambiguous map update syntax

[doorgan]

Code like %{a do :ok end | b c, d => e} is ambiguous because it can be interpreted as:

%{(... | b(c, d)) => e} - what Spitfires parsed
%{... | b(c, d) => e} - what Elixir chooses

Elixir is aware of this ambiguity and warns about it:

warning: missing parentheses on expression following operator "|", you must add parentheses to avoid ambiguities

Since this is ambiguous syntax, Spitfire it not entirely wrong here, but we should follow what Elixir produces anyways.

The only way I got this to trigger(from examples in the property tests in Lukasz prs) is with the lhs of | being a do/end block, I'm not 100% sure why, but withut it the Elixir parser straight out returns a syntax error.

We were allowing => to be consumed inside the no-parens call arguments and the map entry parser never got a chance to see it. The fix marks map entry contexts (map/struct literals and map updates) with map_context and blocks => from being consumed inside nested expressions. That keeps => at the entry level and allows the existing ambiguity handling in parse_pipe_op/2 to match Elixir’s choice. last_assoc_meta context is used to reconstruct the ast after resolving the ambiguity, and put the assoc metadata in the right node.

I added a new with_state/4 macro to make it easier to manage parser context. Setting and remember to unset parser context is very error prone, so this should help.

[mhanberg]

Could you add a comment that has an example of code that matches each clause? I'm getting confused cuz this says pairs but it's a list of one element.

[doorgan]

Done. It was called pairs because it's the key/value pairs for a => b, c => d. It may also be a single element(the second clause) so I renamed this to entries instead.
I added a comment with the shape each clause handles.

[mhanberg]

Oh I think I was getting confused at to what the ast for this is. The args are the pairs and the "call" is the pipe?

[doorgan]

So this pattern [{{call, call_meta, [_, _ | _] = args}, value}] matches this interpretation: %{... | (b(c, d)) => e}
Where:

• call is b(c, d)
• The ambiguity happens with a 2+ args call(as the comma separating the args causes the ambiguity when the source code has no parens), so [_, _ | _] = args is checking for c, d, ... in that call
• value is e
• There is no :"=>" ast node, this pair represents that

The pipe parsing happens in parse_expression when the flag is_map is true, then parse_pipe_op -> parse_map_update_pairs -> parse_comma_list. The last function there is what produces a list of entries for the map, and from that list is where this "pair" comes from if it finds an assoc_op(=>) token.

Then, in parse_pipe_op(which is only called for map updates, not list cons like [head | tail], we call this function that sees the ambiguity and resolves it.

[doorgan]

Going over this again I realize if there's more entries like %{a do :ok end | b c, d => e, f => g} then there's still a mismatch, so I'll have to fix that too

[mhanberg]

Could you write a few unit tests for this?

[doorgan]

I'm moving this to draft, extending the current fix to handle %{... | a b, c => d, f => g} is too hacky and doing it properly is requiring a larger refactor to explicitly parse matched/unmatched/no_parens expressions.

[doorgan]

Closing in favor of #93, the approach here is not enough and doesn't fix the root cause.