Add lifted typeclasses and some instances

[phadej]

WIP, have to add rest of the instances. That should be straightforward. Comments are welcome.

Resolves #352

[RyanGlScott]

I'm working on the GHC generics and TH support for these classes, and I've hit a bit of a stumbling block. I need to have generalized versions of (.:) and (.:?) which work for instances of FromJSON1 and FromJSON2, but since that would require passing more arguments, I can't use infix names for these functions. What would be a good prefix name equivalents for (.:) and (.:?)?

[phadej]

@RyanGlScott parseField or parseOptionalField?

[RyanGlScott]

That sounds good.

Another thorny issue: I need to lift the KeyValue classes to make things work when deriving ToJSON1 or ToJSON2:

class KeyValue kv where
    (.=) :: ToJSON v => Text -> v -> kv

class KeyValue1 kv where
    liftedKeyValue :: ToJSON1 f => (v -> Value) -> Text -> f v -> kv

class KeyValue2 kv where
    liftedKeyValue2 :: ToJSON2 f => (v1 -> Value) -> (v2 -> Value)
                                 -> Text -> f v1 v2 -> kv

With these, we can make instances for Pair (which Data.Aeson.TH uses internally):

instance KeyValue Pair where
    name .= value = keyValuePair toJSON name value

instance KeyValue1 Pair where
    liftedKeyValue tj name value = keyValuePair (liftToJSON tj) name value

instance KeyValue2 Pair where
    liftedKeyValue2 tj1 tj2 name value =
        keyValuePair (liftToJSON2 tj1 tj2) name value

keyValuePair :: (v -> Value) -> Text -> v -> Pair
keyValuePair tj name value = (name, tj value)

But it's not quite as straightforward to make KeyValue1 and KeyValue2 instances for Series:

instance KeyValue Series where
    name .= value = Value . Encoding $
                    E.text name <> B.char7 ':' <> builder value

Instead of using toJSON under the hood, this instance uses toEncoding, so lifting this instance is a bit awkward. I can see three options:

1. Don't create KeyValue1/KeyValue2 instances for Series at all. The easiest (but most incomplete) option.
2. Use Encoding . E.encodeToBuilder . tj to turn tj :: v -> Value into a function of type v -> Encoding. This works, but is inefficient.
3. Change ToJSON1 and ToJSON2 to take more arguments (similarly to how Show1/Show2 does it):

class ToJSON1 f where
    liftToJSON :: (a -> Value) -> (a -> Encoding) -> f a -> Value
    liftToEncoding :: (a -> Value) -> (a -> Encoding) -> f a -> Encoding

class ToJSON2 f where
    liftToJSON2 :: (a -> Value) -> (a -> Encoding) -> (b -> Value) -> (b -> Encoding) -> f a b -> Value
    liftToEncoding2 :: (a -> Value) -> (a -> Encoding) -> (b -> Value) -> (b -> Encoding) -> f a b -> Encoding

And update KeyValue1/KeyValue2. This would be a pretty significant change, but it would solve the above problem and allow the most customization.

[phadej]

@RyanGlScott do you really need KeyValue1 and KeyValue2 at all? can't you use named functions? AFAICS the class exists for .= -operator, which is handy for human users, yet generics code could use specialised variants directly?

[RyanGlScott]

You're right, it would probably be far easier to just hardcode the key-value pair functionality I need in Data.Aeson.TH.

But we're not in the clear yet, I'm afraid. I just spotted this in Data.Aeson.TH:

-- Infix constructors.
argsToEncoding opts multiCons (InfixC _ conName _) = do
    al <- newName "argL"
    ar <- newName "argR"
    match (infixP (varP al) conName (varP ar))
          ( normalB
          $ sumToEncoding opts multiCons conName
          $ [|toEncoding|] `appE` listE [ [|toJSON|] `appE` varE a
                                        | a <- [al,ar]
                                        ]
          )
          []

• First of all, is this even correct? This generates a toEncoding expression for an infix constructor by calling toJSON on its two fields. Is this a typo (and should it actually be [|toEncoding|] appE varE a)?
• If it isn't a typo, how to we get around this?

[phadej]

AFAICS I made all instances, except for the map types. Would add them as part of #341. Review is welcome.

[andrewthad]

@phadej Thanks for writing all those instances. I'd also like to see instances for Proxy, Compose, Const, Data.Functor.Product, and Data.Functor.Sum. I'd be happy to contribute them if you'd like. Also, I was recently discussing candidates for Eq1,Ord1, etc. instances on the ghc-devs mailing list, and here are some of the higher-kinded types from base that came up:

• Down (from Data.Ord)
• Complex (from Data.Complex)
• Ratio (from Data.Ratio)
• First, Last (from Data.Monoid)
• ZipList (from Control.Applicative)
• Ptr, FunPtr (from Foreign.Ptr, only Eq1 and Ord1)
• StablePtr (from Foreign.StablePtr, only Eq1)
• K1, U1, Par1, Rec1, etc. (from GHC.Generics)
• StableName (from System.Mem.StableName)
• Tuple types
• All (* -> *)-kinded datatypes in Data.Monoid
• Data.Type.Equality.(:~:) and Data.Type.Coercion.Coercion
• Fixed
• ST, STRef
• Chan
• MVar, IORef, TVar

Clearly, most of these are bad candidates for FromJSON1/ToJSON1 instances, but I thought I'd post the list here in case it stirred up any thoughts.

[RyanGlScott]

Hm, I think I answered my own question above. We can't replace [|toJSON|] with [|toEncoding|] because then we'd be splicing in an expression of the form:

toEncoding [toEncoding a, toEncoding b]

And Encoding isn't a ToJSON instance. Urk.

@phadej, I'm afraid the only way to do this right is to change ToJSON1 and ToJSON2 to add more function arguments as in here. Would you mind updating your PR to allow this?

[phadej]

Sorry, on mobile atm. I'd say that changing typeclasses only to benefit generic derivatona is a bad excuse. And it also doesn't make sense for liftToJSON to even know about Encoding

[phadej]

@andrewthad there aren't instances for those types, but the ones which makes sense to have instances: yes, but I'd rather have them in separate PR

[RyanGlScott]

@phadej, I agree that it seems a bit funny, which is why I asked if perhaps we should be doing something besides splicing in [|toJSON|] in a TH-generated definition for (lift)ToEncoding(2). But as it stands, unless we change something, there's literally no way to make this work. Do you have any suggestions?

[andrewthad]

@phadej That sounds good. I'll wait to PR the other instances until this one goes through.

[andrewthad]

@RyanGlScott I can barely read or write TH, but I think that the existing TH toEncoding for infix data constructors is wrong. If I understand correctly, the last line (beginning with $ [|toEncoding|]) currently generates something equivalent to this:

toEncoding [toJSON al, toJSON ar]

Which converts al and ar to Value before converting the Values into a builder. This defeats the whole purpose of toEncoding. It should generate something like this:

E.char7 '[' <> toEncoding al <> E.char7 ',' <> toEncoding ar <> E.char7 ']'

I think it should be changed to do generate this instead. If I understand correctly, that would also solve the problems you are having with writing a lifted version of it.

[RyanGlScott]

Thanks @andrewthad, that sounds like a much more reasonable approach. I've made a PR to @phadej's PR here which implements TH and generics support: phadej#1

[andrewthad]

If there is any more discussion to be had here, it's probably better to discuss it on #391, which includes all commits in this PR.

[phadej]

Close in favour of #391