Array combinators don't propagate fail "with consumption"

[Hi-Angel]

Describe the bug

Parsing is designed to distinguish between fail with and without data consumed. In particular, the combinators from Parsing.Combinators as well as ones from Data.Array propagate fail if it happens after data was consumed.

Unfortunately, this isn't the case for at least many* combinators from Parsing.Combinators.Array

To Reproduce

Execute the following code:

module Main where

import Prelude

import Effect (Effect)
import Effect.Class.Console (logShow)
import Parsing (Parser, fail, runParser)
import Parsing.Combinators ((<|>))
import Parsing.Combinators as CombList
import Parsing.Combinators.Array as CombArray
import Parsing.String (anyChar, char)

failAfterConsuming :: ∀ a. Parser String a
failAfterConsuming = anyChar *> fail "failure"

main :: Effect Unit
main = do
  logShow $ runParser "abc" $ CombArray.many (char 'a' <|> failAfterConsuming)
  logShow $ runParser "abc" $ CombArray.many1 (char 'a' <|> failAfterConsuming)
  logShow $ runParser "abc" $ CombList.many (char 'a' <|> failAfterConsuming)
  logShow $ runParser "abc" $ CombList.many1 (char 'a' <|> failAfterConsuming)

Expected behavior

All 4 parses would return Left

Actual behavior

The Array-based parses return successfully:

(Right ['a'])
(Right (NonEmptyArray ['a']))
(Left (ParseError "failure" (Position { column: 3, index: 2, line: 1 })))
(Left (ParseError "failure" (Position { column: 3, index: 2, line: 1 })))

Workarounds

many from Data.Array works as expected. However, there isn't analogous function for many1.

[jamesdbrock]

Thank you for the report and for the reproduction. That does look like a bug.

[jamesdbrock]

I will try to fix this but may not get to it in the next few days.

If someone else wants to attempt a PR for this, I would recommend to make a PR with 2 commits:

1. First commit: Add the test cases from this issue to https://github.com/purescript-contrib/purescript-parsing/blob/main/test/Test/Main.purs with a comment linking to this issue.
2. Second commit: Fix the bugs so the tests pass.

[jamesdbrock]

Parsing.Combinators.Array.many:
https://github.com/purescript-contrib/purescript-parsing/blob/v10.2.0/src/Parsing/Combinators/Array.purs#L38-L38

Parsing.Combinators.many:
https://github.com/purescript/purescript-lists/blob/v7.0.0/src/Data/List.purs#L171-L171

[natefaubion]

To be clear, the issue here is that we shouldn't be using try, right? In general, we really shouldn't use try in any combinators. It should be up to the author to make that decision.

[jamesdbrock]

To be clear, the issue here is that we shouldn't be using try, right? In general, we really shouldn't use try in any combinators. It should be up to the author to make that decision.

Yes.