Return NonEmpty types for combinators requiring at least one match

[aij]

It would be nice to not have to check for Nil from combinators that require at least one match.

The fact that they require at least one match could be encoded in the type as either NonEmptyList a or NonEmpty List a. (not sure which would be preferable)

---

Environment

• PureScript 0.12.5
• Pulp 12.4.2
• purescript-parsing 5.0.3

Current behavior

These functions in Text.Parsing.Parser.Combinators currently produce a (Nilable) List:

-- | Parse phrases delimited by a separator, requiring at least one match.
sepBy1 :: forall m s a sep. Monad m => ParserT s m a -> ParserT s m sep -> ParserT s m (List a)

-- | Parse phrases delimited and optionally terminated by a separator, requiring at least one match.
sepEndBy1 :: forall m s a sep. Monad m => ParserT s m a -> ParserT s m sep -> ParserT s m (List a)

-- | Parse phrases delimited and terminated by a separator, requiring at least one match.
endBy1 :: forall m s a sep. Monad m => ParserT s m a -> ParserT s m sep -> ParserT s m (List a)

As a workaround, I defined a modified/alternate version of sepBy1 locally:

-- | Parse phrases delimited by a separator, requiring at least one match.
sepBy1Nel :: forall m s a sep. Monad m => ParserT s m a -> ParserT s m sep -> ParserT s m (NonEmptyList a)
sepBy1Nel p sep = do
  a <- p
  as <- List.many $ sep *> p
  pure $ NonEmptyList (a :| as)

I can then use NonEmpty.unsnoc and not worry about how to handle the impossible Nothing result from List.unsnoc

Here's a live example: https://github.com/ccap/purescript-ccap-codegen/blob/2c09022f7f37a5494bf58be8f28354d7071c774a/src/Ccap/Codegen/Parser.purs#L89

(admittedly, we're not using NonEmpty as pervasively as we could...)

Expected behavior

When a list is guaranteed to be non-empty, indicate it in the type.

I would be happy to open a PR if it would be welcome. If so, do you have a preference between changing the types of the existing combinators or adding new ones (to maintain backwards compatibility)?

[garyb]

I'd be fine with it being a breaking change personally, it definitely seems like the right type for these kind of things. It would only be a minor fix (probably deleting an unnecessary fallback) for people to upgrade too. 🙂

[thomashoneyman]

I think this is a sensible change to merge as part of the upcoming 0.14 breaking changes.

[robertdp]

I think this issue is resolved