aeson icon indicating copy to clipboard operation
aeson copied to clipboard

Add sum encoding TaggedFlatObject

Open poscat0x04 opened this issue 4 years ago • 11 comments

Closes #827

This PR adds a SumEncoding variant TaggedFlatObject :: String -> SumEncoding for encoding and decoding JSON objects whose tags are on the same level as the contents (hence the name "flat"). The decoding/encoding rule of TaggedFlatObject is as follows:

  1. Encode single constructor data types without field names as an array (This is the same as TaggedObject)
  2. Encode single constructor data types with field names as a plain record (This is the same as TaggedObject)
  3. Encode multi constructor data types without fields names as a record whose keys are the positions of the fields(starting from 1) and values are the fields encoded as JSON with an additional KV pair "$tagFieldName": "$constructorName". For example A 1 2 will be encoded as {"tag": "A", "1": 1, "2": "2" }. Note that the tag will overwrite any fields that have the same name.
  4. Encode multi constructor data types with field names as a record whose keys are the field names and values are the fields encoded as JSON with an additional KV pair "$tagFieldName" : "$constructorName" For example A { field1 = 1, field2 = "2" } will be encoded as {"tag": "A", "field1": 1, "field2": "2" }. Note that the tag will overwrite any fields that have the same name.

poscat0x04 avatar Jan 18 '21 04:01 poscat0x04

It should now work with GHC 7.* (not sure why my force push didn't trigger CI build)

poscat0x04 avatar Jan 22 '21 10:01 poscat0x04

For some reason, the INCOHERENT pragma did not work for ghc-7.8.4.

poscat0x04 avatar Feb 01 '21 03:02 poscat0x04

@phadej Is this ok now?

poscat0x04 avatar Feb 01 '21 03:02 poscat0x04

No $tagName here, or what does "plain record" mean?

Plain record just means a record without any additional field.

I think starting at 0 would make more sense as it matches the indices when we use arrays.

Yeah, that makes sense. TBH the current choice of encoding constructors without field names is a bit arbitrary. I initially did not plan to support them but then figured out it would affect other SumEncoding variants because we currently cannot distinguish different SumEncodings at the type level (relevant code).

I would also make the indices ints so that this is closer to an array.

I'm not sure if I understand. JSON keys must be strings, right?

Can we give an error if this occurs instead, at least in TH? Or will it be more confusing if TH/Generics differ here?

Yeah, I think they should have the same behavior.

poscat0x04 avatar Feb 18 '21 08:02 poscat0x04

I'm not sure if I understand. JSON keys must be strings, right?

Guess who has been doing too much JavaScript lately runs away, please disregard

bergmark avatar Feb 18 '21 09:02 bergmark

I managed to hit some github hot key that posted my review for me... @phadej do you want to look over this again?

bergmark avatar Feb 26 '21 13:02 bergmark

Eh it looks like the tests don't compile, but only on GHC 7.8 :-/

bergmark avatar Feb 26 '21 13:02 bergmark

The issue test-failure issue might be something which will get fixed with #839. I think I can wipe out all use over overlapping instances for good. Just need to get to it. (And get courage to commit a breaking change).

phadej avatar Feb 26 '21 14:02 phadej

You can do it!

bergmark avatar Feb 26 '21 15:02 bergmark

What's the status of this PR? It would be a useful feature for our codebase, FWIW.

tadfisher avatar Nov 18 '21 18:11 tadfisher

My current opinion is to throw Options into /dev/null and rather provide a bunch of newtypes to be used with deriving via, for different encoding schemes.

Current generics and template haskell code is very much unmaintainable (as it tries to do evertything), so this PR is probably not going anywhere anytime soon.

phadej avatar Nov 18 '21 18:11 phadej