haskell / aeson

A fast Haskell JSON library
Other
1.26k stars 321 forks source link

Why do the generic classes allow `V1` void datatypes, which can't be en/decoded? #1002

Closed raehik closed 1 year ago

raehik commented 1 year ago

In Data.Aeson.Types.ToJSON and Data.Aeson.Types.FromJSON, instances are provided for V1 types, which is the generic representation for void datatypes such as data Void.

Why are these runtime errors and not compile-time errors? ToJSON Void and FromJSON Void instances are useful because Void is a concrete type, so the explicitness makes me comfortable that a user would expect an error (and perhaps wants one). In the general case i.e. for the abstract void datatype, I feel a compile-time error is more appropriate. Better no instance than an absurd one!

This was originally introduced by #661 . Void and V1 were added together. User bergmark added that they had originally avoided V1 in another library. I wasn't able to find much discussion. I think it would be good to figure out


In such cases where the V1 instance is hit, it's possible to emit a pretty type error for the user (code taken from here):

-- | Refuse to derive instance for void datatype.
instance TypeError GErrRefuseVoid => GPut V1 where
    gput = undefined

type GErrRefuseVoid =
    'Text "Refusing to derive binary representation for void datatype"

I would gladly make this change if it's found appropriate.

Lysxia commented 1 year ago

I agree that the instance seems of dubious usefulness. But it doesn't seem harmful either, so I would still lean towards not changing the status quo.

Perhaps there is a use case in a prototyping scenario, where you sketch out the types of your application and leave some empty to fill them out later (but one could also make them a unit type with a TODO constructor, with essentially the same effect).

Another potential use case is if you're automatically generating Haskell code, having V1 lets you use deriving uniformly for all generated types.

raehik commented 1 year ago

I note that deriving stock instances like Show, Eq, Ord all work on empty data types, so my suggestion might not be consistent with the rest of the Haskell ecosystem. Which isn't to say it's the wrong idea -- I feel recent language changes have been moving away from the fairly implicit nature of instance deriving (e.g. DerivingVia) -- but I'm now a bit less sure about pushing this through aeson.

I agree that the instance seems of dubious usefulness. But it doesn't seem harmful either, so I would still lean towards not changing the status quo.

I do think they are slightly harmful, as disallowing V1 may allow aeson to catch some programmer errors.

phadej commented 1 year ago

They were added as someone asked for them in https://github.com/haskell/aeson/issues/661

I don't see any harm in having those. The instances for Void/empty types make sense: parseJSON always fail and toJSON calls absurd. That's the same as with Show and Read instances.