search

vektah / goparsify

golang parser-combinator library
MIT License
73 stars 16 forks source link

Fix nested Any() not correctly tracking longestError. Fixes #3 #7

Closed superfell closed 5 years ago

superfell commented 5 years ago

We ran into this issue with nested Any() as well, digging into the debug traces, you can see that the inner any failed the individual parsers but didn't return any error back up the chain.

=== RUN   TestAny/Nested/Any(Seq,Any)
combinator_test.go:97 | bob             | ps {
combinator_test.go:94 | bob             |   seq {
combinator_test.go:94 |                 |     [a-z] found "bob"
combinator_test.go:94 |                 |     : did not find :
combinator_test.go:94 | bob             |   } did not find :
combinator_test.go:93 | bob             |   anyTrueFalse {
combinator_test.go:93 | bob             |     true did not find true
combinator_test.go:93 | bob             |     false did not find false
combinator_test.go:93 | bob             |   } found "[bob,]"
combinator_test.go:97 | bob             | } found "[bob,]"

What happens is in the outer Any() it runs a parser which sets an error, it then calls recover and calls the next parser. If that parser is an Any() then it saves error in longestError, but because recover was called, pos is set, but expected is empty. Subsequently the inner Any() will check the error pos against longest, but if all the inner Any()'s errors are before the outer one, it ends up returning out of the inner any() without having updated longestError. When it then returns back up the chain, error.expected is still not set.

This fix checks expected as well as pos when checking if it should update longestError.

superfell commented 5 years ago

Ahh, I didn't see PR #6 before, that's a better fix than this one.