aeson icon indicating copy to clipboard operation
aeson copied to clipboard

Unexpected behaviour with Maybe typed fields and UntaggedValue

Open edwardb96 opened this issue 6 years ago • 1 comments

I'm not sure if this is a bug or a feature so I thought I'd check. Here is a minimal working example.

{-# LANGUAGE TemplateHaskell, OverloadedStrings #-}
import Data.Aeson
import Data.Aeson.TH

data Fruit = Apple { a :: Integer, b :: Maybe Bool }
           | Banana { a :: Integer }
           deriving Show
$(deriveJSON defaultOptions { sumEncoding = UntaggedValue } 'Apple)

main :: IO ()
main = 
  do
    putStrLn $ show (decode "{ \"a\": 10 }" :: Maybe Fruit) 
    -- Expected Just (Banana {a = 10}), got Just (Apple {a = 10, b = Nothing})
    putStrLn $ show (decode "{ \"a\": 10, \"b\": true }" :: Maybe Fruit) 
    -- Should be and got Just (Apple {a = 10, b = Just True}) 

Since the omitNothingFields is False by default, and when it is false Nothings are encoded as null (rather than omitted). I'd expect { "a": 10 } to decode as a Banana.

Likewise when omitNothingFields is false I'd expect to get Nothing for the following example.

{-# LANGUAGE TemplateHaskell, OverloadedStrings #-}
import Data.Aeson
import Data.Aeson.TH

data Fruit = Apple { a :: Integer, b :: Maybe Bool }
           deriving Show
$(deriveJSON defaultOptions 'Apple)

main :: IO ()
main = 
  do
    putStrLn $ show (decode "{ \"a\": 10 }" :: Maybe Fruit) -- Expected Nothing, got (Apple { a = 10, b = Nothing})

I expect this is a design choice rather than a bug, If this is the case perhaps it could be documented more clearly or highlighted in the documentation for UntaggedValue e.g.

Note: Since omitted Maybe typed fields decode to Nothing, the set of non-maybe fields from each constructor should be disjoint.

If this is indeed a bug, I have hacked together a patch for my own purposes but would be happy to tidy it up and turn it into a PR.

edwardb96 avatar Jul 28 '19 17:07 edwardb96

That sounds like #571. So it's more an accident than a design choice, and as you point out, it's not clear whether it's worth fixing (which would be a breaking change) as opposed to simply documenting the behavior (and one could argue that multi-constructor records are fishy anyway).

Lysxia avatar Jul 28 '19 19:07 Lysxia