Could not deduce `RecordFromJSON'` and `RecordToPairs` in `aeson-2.2.0.0`

[julmb]

I have the following code

{-# LANGUAGE DeriveGeneric #-}

import Data.Aeson
import GHC.Generics

data Item f a = Item { root :: a, children :: f a } deriving Generic1

instance FromJSON1 f => FromJSON1 (Item f) where liftParseJSON = genericLiftParseJSON defaultOptions
instance ToJSON1 f => ToJSON1 (Item f) where liftToJSON = genericLiftToJSON defaultOptions
instance (FromJSON1 f, FromJSON a) => FromJSON (Item f a) where parseJSON = parseJSON1
instance (ToJSON1 f, ToJSON a) => ToJSON (Item f a) where toJSON = toJSON1

data Content f a = Content { items :: f (Item f a) } deriving Generic1

instance (Functor f, FromJSON1 f) => FromJSON1 (Content f) where liftParseJSON = genericLiftParseJSON defaultOptions
instance (Functor f, ToJSON1 f) => ToJSON1 (Content f) where liftToJSON = genericLiftToJSON defaultOptions
instance (Functor f, FromJSON1 f, FromJSON a) => FromJSON (Content f a) where parseJSON = parseJSON1
instance (Functor f, ToJSON1 f, ToJSON a) => ToJSON (Content f a) where toJSON = toJSON1

main :: IO ()
main = return ()

With aeson-2.1.2.1, this code works perfectly fine. With aeson-2.2.0.0, I get the following errors:

Main.hs:15:82: error:
    • Could not deduce (aeson-2.2.0.0:Data.Aeson.Types.FromJSON.RecordFromJSON'
                          One (f :.: Rec1 (Item f)))
        arising from a use of ‘genericLiftParseJSON’
      from the context: (Functor f, FromJSON1 f)
        bound by the instance declaration at Main.hs:15:10-58
    • In the expression: genericLiftParseJSON defaultOptions
      In an equation for ‘liftParseJSON’:
          liftParseJSON = genericLiftParseJSON defaultOptions
      In the instance declaration for ‘FromJSON1 (Content f)’
   |
15 | instance (Functor f, FromJSON1 f) => FromJSON1 (Content f) where liftParseJSON = genericLiftParseJSON defaultOptions
   |                                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Main.hs:16:10: error:
    • Could not deduce (aeson-2.2.0.0:Data.Aeson.Types.ToJSON.RecordToPairs
                          (Data.Aeson.Encoding.Internal.Encoding' Value)
                          Series
                          One
                          (S1
                             ('MetaSel
                                ('Just "items")
                                'NoSourceUnpackedness
                                'NoSourceStrictness
                                'DecidedLazy)
                             (f :.: Rec1 (Item f))))
        arising from a use of ‘aeson-2.2.0.0:Data.Aeson.Types.ToJSON.$dmliftToEncoding’
      from the context: (Functor f, ToJSON1 f)
        bound by the instance declaration at Main.hs:16:10-54
    • In the expression:
        aeson-2.2.0.0:Data.Aeson.Types.ToJSON.$dmliftToEncoding
          @(Content f)
      In an equation for ‘liftToEncoding’:
          liftToEncoding
            = aeson-2.2.0.0:Data.Aeson.Types.ToJSON.$dmliftToEncoding
                @(Content f)
      In the instance declaration for ‘ToJSON1 (Content f)’
   |
16 | instance (Functor f, ToJSON1 f) => ToJSON1 (Content f) where liftToJSON = genericLiftToJSON defaultOptions
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Main.hs:16:75: error:
    • Could not deduce (aeson-2.2.0.0:Data.Aeson.Types.ToJSON.RecordToPairs
                          Value
                          (dlist-1.0:Data.DList.Internal.DList
                             aeson-2.2.0.0:Data.Aeson.Types.Internal.Pair)
                          One
                          (S1
                             ('MetaSel
                                ('Just "items")
                                'NoSourceUnpackedness
                                'NoSourceStrictness
                                'DecidedLazy)
                             (f :.: Rec1 (Item f))))
        arising from a use of ‘genericLiftToJSON’
      from the context: (Functor f, ToJSON1 f)
        bound by the instance declaration at Main.hs:16:10-54
    • In the expression: genericLiftToJSON defaultOptions
      In an equation for ‘liftToJSON’:
          liftToJSON = genericLiftToJSON defaultOptions
      In the instance declaration for ‘ToJSON1 (Content f)’
   |
16 | instance (Functor f, ToJSON1 f) => ToJSON1 (Content f) where liftToJSON = genericLiftToJSON defaultOptions
   |                                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I looked at the changelog, and while there have apparently been some changes to the handling of Generics related to omitted fields, I did not find anything that would indicate that my situation should no longer work. Is this a regression or am I doing something wrong?

[phadej]

The behavior has changed.

If encoded as record, encoding Content Maybe a should omit nothing fields, but such composition (i.e. instances for :.:) are not there anymore. The changed instance is for ToJSON case

 instance ( Selector s
-         , GToJSON' enc arity a
+         , GToJSON' enc arity (K1 i t)
          , KeyValuePair enc pairs
-         ) => RecordToPairs enc pairs arity (S1 s a)
+         , ToJSON t
+         ) => RecordToPairs enc pairs arity (S1 s (K1 i t))
   where
-    recordToPairs = fieldToPair
+    recordToPairs opts targs m1
+      | omitNothingFields opts
+      , omitField (unK1 $ unM1 m1 :: t)
+      = mempty
+
+      | otherwise =
+        let key   = Key.fromString $ fieldLabelModifier opts (selName m1)
+            value = gToJSON opts targs (unM1 m1)
+         in key `pair` value

and two other RecordToPairs instanecs for Rec1 and Par1 but not :.: one.

Note, that we need to do omitNothingFields checks on the value inside S1 s now

Probably we'd need to add one more auxiliary class (say FieldToPairs) and figure out how to write RecordToPairs enc pairs arity (S1 s x) instance using FieldToPairs ... x. And similarly for FromJSON.

I have no idea how to do that, but as long as above test case is added as an regression test, and nothing else in test-suite needs to be changed I'll be happy to review the patch.

[julmb]

Alright, I understand. Unfortunately, I currently have neither the necessary insight and experience, nor the time to work on a patch for this. I will make do with the basic FromJSON and ToJSON instances for now.

[julmb]

I have been using aeson 2.1.2.1 for a while now to avoid this issue. However, it seems to be incompatible with newer versions of GHC due to the version bounds on ghc-prim. So I spent some time looking into this again.

First of all, I am surprised that nobody else seems to be running into this issue (no activity on this ticket in almost a year). My original example was quite contrived, but I have since encountered this problem many times. This includes very simple situations like:

newtype Test a = Test { items :: Maybe [a] } deriving (Generic1, FromJSON1, ToJSON1)

I am also surprised that this seems to not have cropped up during the original development of the optional fields feature in #1023 and #1039.

I managed to get the parsing side of things working by adding the following instance:

instance {-# OVERLAPPING #-}
         (Selector s, GFromJSON One (Rec1 g), FromJSON1 f, FromJSON1 g) =>
         RecordFromJSON' One (S1 s (f :.: Rec1 g)) where
    recordParseJSON' args@(_ :* _ :* opts :* From1Args o _ _) obj = recordParseJSONImpl d gParseJSON args obj where
      d = guard (allowOmittedFields opts) >> fmap Comp1 (liftOmittedField (fmap Rec1 (liftOmittedField o)))
    {-# INLINE recordParseJSON' #-}

However, I am not quite understanding what is going on here and I have no idea if this covers all cases.

As an aside, this implementation makes use of the GFromJSON One (f :.: g) instance which seems to be otherwise unused. It almost looks like there was a plan to enable deriving generic instances for composed types, but maybe it was not finished? Or maybe the GFromJSON One (f :.: g) instance is left over from before the optional fields update? Although this instance includes the following note that I also do not quite understand:

-- Note: the ommitedField is not passed here.
-- This might be related for :.: associated the wrong way in Generics Rep.

It also seems like the RecordFromJSON' One (S1 s (f :.: Rec1 g)) instance should reduce to the RecordFromJSON' One (S1 s (Rec1 f)) instance instead of duplicating most of its implementation. I imagine this is what @phadej meant by adding an auxiliary class for fields. However, that would require a refactoring much larger than I am comfortable with on code I barely understand.

[julmb]

Since #1103 did not cover all cases, I did some more digging. It seems like the reason this no longer works is this change from #1039 where instead of a single instance:

RecordFromJSON' arity (S1 s a)

There are now several instances:

RecordFromJSON' arity (S1 s (K1 i a))
RecordFromJSON' arity (S1 s (Rec0 a))
RecordFromJSON' One (S1 s (Rec1 f))
RecordFromJSON' One (S1 s Par1)

An instance involving (:.:) is missing due to the lack of a type class that provides field omission functionality on generic representations.

Unfortunately, I do not understand what @phadej had in mind with the FieldToPairs class. In the mean time, I came up with two approaches that seem to work based on preliminary testing.

1. Add a method to the GFromJSON class (https://github.com/haskell/aeson/compare/master...julmb:aeson:compose-method):

   class GFromJSON arity f where
    gParseJSON :: Options -> FromArgs arity a -> Value -> Parser (f a)
   +    gOmittedField :: FromArgs arity a -> Maybe (f a)

2. Add a new class GOmitFromJSON (https://github.com/haskell/aeson/compare/master...julmb:aeson:compose-class):

   +class GOmitFromJSON arity f where
   +    gOmittedField :: FromArgs arity a -> Maybe (f a)

In both cases, the gOmittedField method can be used to implement a general instance RecordFromJSON' arity (S1 s a) without the need to split things into several cases. Since there already is an instance GFromJSON One (f :.: g), this makes functor composition work as expected.

Approach 1 has the advantage of not introducing any new type classes. However, it requires implementing the gOmittedField method on some representation types like V1 and D1 which can never actually occur inside the S1 type on which gOmittedField is actually invoked. Approach 2 does not have this issue, implementing instances of GOmitFromJSON only on the types which actually appear inside an S1 type constructor. The downside is the addition of yet another type class to an already complex design.

Any thoughts on this? Do people have a preference for one approach over the other?