Correctly parse ado without any statements in it

[MonoidMusician]

Fixes natefaubion/purescript-tidy#108

The issue was that the parser wasn't backtracking enough to abort valueParser and fall back to pure [], so instead I parse the layout tokens up front so we can commit to the right path early. Perhaps it could be more elegant.

Tested on the examples in the issue and on our work codebase. Do you want me to add a test to test/Main.purs?

[natefaubion]

This seems like it should be equivalent to optional? If we had an empty layout, then valueParser should immediately fail if given a TokLayoutEnd, shouldn't it? It's not obvious to me what "backtracking far enough" means then, and that there isn't some other internal bug. Do we know where backtracking is halting?

[MonoidMusician]

Okay, I was a little hasty in ascribing it to backtracking. Putting try in there does not help.

What's really messing it up is the indentation-based parser recovery when we call it in parseAdo (this is the only usage of layout). An an example change, this manages to parse the empty ado in:

--- a/src/PureScript/CST/Parser.purs
+++ b/src/PureScript/CST/Parser.purs
@@ -673,7 +673,7 @@ parseDo = do
 parseAdo :: Parser (Recovered Expr)
 parseAdo = do
   keyword <- tokQualifiedKeyword "ado"
-  statements <- layout (recoverDoStatement parseDoStatement)
+  statements <- layout parseDoStatement
   in_ <- tokKeyword "in"
   result <- parseExpr
   pure $ ExprAdo { keyword, statements, in: in_, result }

[natefaubion]

It makes sense to me that there is something odd with recovery, but I'm not sure what. In the case of ado in, I would expect recoverIndent to immediately hit a TokLayoutEnd where col == indent, resulting in empty tokens, yielding Nothing, which would propagate the error.

purescript-language-cst-parser/src/PureScript/CST/Parser.purs

Lines 1155 to 1164 in 5afff30

	let
	Tuple tokens newStream = recoverTokensWhile
	( \tok indent -> case tok.value of
	TokLayoutEnd col -> col > indent
	TokLayoutSep col -> col > indent
	_ -> true
	)
	stream
	if Array.null tokens then
	Nothing

purescript-language-cst-parser/src/PureScript/CST/Parser/Monad.purs

Lines 149 to 151 in 5afff30

	case k error state1.stream of
	Nothing ->
	runFn2 resume (state2 { consumed = state1.consumed }) error

Maybe something is wrong with the state threading in recover. The logic of the original code seems OK to me, which makes me think this is an internal bug and not related to the specific formulation of this parser.

[MonoidMusician]

Okay, I'll push a test for this case and dig deeper.

[natefaubion]

I think that this should have a regression test, yes. We test against publicly available code, but this was triggered by an edge case that doesn't exist in any public code.

[natefaubion]

Thanks!