Refactor entity constraint parsing in Quasi module

[danbroooks]

Before submitting your PR, check that you've:

• Documented new APIs with Haddock markup
• Added @since declarations to the Haddock
• Ran stylish-haskell on any changed files.
• Adhered to the code style (see the .editorconfig file for details)

After submitting your PR:

• Update the Changelog.md file with a link to your PR
• Bumped the version number if there isn't an (unreleased) on the Changelog
• Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

---

A further contribution towards #1156, addresses the fold referenced in that issue by creating a type that holds the various possible constraints, and can be built up and merged into one. I have used Monoids / Semigroups however the instances are not lawful. I will put some further detail about this as a comment in the relevant area in the PR.

I have also renamed splitExtras to parseEntityFields as this is more descriptive I think. This function is called when the fields of an entity are extracted, the first step being to split the 'extra' tokens from the rest of the entity fields which are added to a value of type M.Map Text [ExtraLine], where the key is the entity name and its [ExtraLine] values are the entity fields.

[danbroooks]

I went with using Semigroups/Monoids for this change, but this implementation of just1 which is used in the semigroup instance makes it unlawful. I could re-write this to not use these typeclasses, but I thought it was a nice refactoring (means I could use foldMap, mempty).

Alternatively this could use something like First or Last, though it is probably a nicer user experience to receive this error than for it to silently proceed using one of the values over the other in the case of duplicate values. In fact that is definitely not what we would want.

It may be that now I have done an initial refactor, I could tweak things further again to not have unlawful instances, and use something instead foldMap / mempty

[parsonsmatt]

Hmm! I like using the Monoid and Semigroup instances here. That's neat. But I don't like having a Semigroup operation that will error on a relatively common case - that seems like a footgun.

I feel like a type that better represents this is something like:

data SetOnce a 
    = SetOnce a 
     | NotSet 
     | SetManyTimes (NonEmpty a)

instance Semigroup (SetOnce a) where
    NotSet <> a = a

    SetOnce a <> NotSet = SetOnce a
    SetOnce a <> SetOnce b = SetManyTimes (a :| [b])
    SetOnce a <> SetManyTimes as = SetManyTimes (NEL.cons a as)

    a@(SetManyTimes xs) <> NotSet = a
    SetManyTimes xs <> SetOnce a = SetManyTimes (xs `snoc` a)
    SetManyTimes xs <> SetManyTimes ys = SetManyTimes (xs <> ys)

instance Monoid (SetOnce a) where
    mempty = NotSet

Rendering/erroring can then happen outside of the code.

[parsonsmatt]

Hmm! I like using the Monoid and Semigroup instances here. That's neat. But I don't like having a Semigroup operation that will error on a relatively common case - that seems like a footgun.

I feel like a type that better represents this is something like:

data SetOnce a 
    = SetOnce a 
     | NotSet 
     | SetManyTimes (NonEmpty a)

instance Semigroup (SetOnce a) where
    NotSet <> a = a

    SetOnce a <> NotSet = SetOnce a
    SetOnce a <> SetOnce b = SetManyTimes (a :| [b])
    SetOnce a <> SetManyTimes as = SetManyTimes (NEL.cons a as)

    a@(SetManyTimes xs) <> NotSet = a
    SetManyTimes xs <> SetOnce a = SetManyTimes (xs `snoc` a)
    SetManyTimes xs <> SetManyTimes ys = SetManyTimes (xs <> ys)

instance Monoid (SetOnce a) where
    mempty = NotSet

Rendering/erroring can then happen outside of the code.

[parsonsmatt]

This is much nicer 👍🏻