aeson icon indicating copy to clipboard operation
aeson copied to clipboard

Rework TH instances context

Open stevladimir opened this issue 10 months ago • 1 comments

Instead of deriving instances for all datatype variables iterate over constructors generating instances based on how variables are used. Does the right thing in cases like

  • Phantom type variables Does not add corresponding instance to the context for unused variables
  • Type families Instead of generating (ToJSON a) for data Foo a = Foo (SomeTypeFamily a) generates ToJSON (SomeTypeFamily a)

Note that it slightly changes the behavior in cases like

data Foo a = Foo a deriving (ToJSON)

data Bar a = Bar (Foo a)
deriveToJSON defaultOptions ''Bar

will result in ToJSON (Foo a) constraint rather than ToJSON a (current behavior). I believe the former is more precise as that is exactly what generated encoder wants. The small downside is requirement for FlexibleContext extension.

TODO and open questions:

  • [ ] How to test TH? Are there any helpers to get easy access to instance context. I think it would be nice to have something like
$(getContext $ deriveToJSON ...) `shouldBe` "ToJSON a, ToJSON b"

Can add if required. But maybe there are already established approach to test that.

  • [ ] Which test cases to add? I sketched up some cases, but that list is not exhaustive. And they should be reworked according to previous point
  • [ ] Should we test separately From and To classes?
  • [ ] Add Changelog

stevladimir avatar Mar 16 '25 10:03 stevladimir

Should I explicitly ping some of the maintainers? No rush - asking just to make sure someone will eventually put an eye on this. It is mostly ready except for some minor things mentioned in TODO list, but it makes sense to get some feedback (maybe it is no go at all) before proceeding with them. @phadej maybe you can give a look or recommend someone to ping?

stevladimir avatar Jul 14 '25 07:07 stevladimir