Fix bug in (.:?)

[sol]

When used as

(.:?) :: Object -> Text -> Parser (Maybe Value)

it could not retrieve Null values. I think this is a copy & paste error
from (.:) that the type checker sadly can not catch.

[sol]

I don't know where tests for (.:) and (.:?) should go. The current test suite does not contain any tests for those (except (.:) is used to define an instance).

Here is a spec that reproduces the bug:

{-# LANGUAGE OverloadedStrings #-}
module Main (main) where

import Test.Hspec
import Control.Monad
import Data.Aeson
import Data.Aeson.Types

main :: IO ()
main = hspec spec

spec :: Spec
spec = do
  describe ".:?" $ do
    context "when parsing JSON values" $ do
      it "correctly handles Null" $ do
        let v = object ["foo" .= Null]
            p = parseJSON >=> (.:? "foo")
        parseMaybe p v `shouldBe` Just (Just Null)

[sol]

Or better yet, a QuickCheck property:

{-# LANGUAGE OverloadedStrings #-}
module Main (main) where

import           Test.Hspec
import           Test.QuickCheck
import           Control.Applicative
import           Control.Monad
import           Data.Text (pack)
import qualified Data.HashMap.Strict as H
import qualified Data.Vector as V
import           Data.Aeson
import           Data.Aeson.Types

arbitraryObject :: Gen Object
arbitraryObject = H.fromList . map (\(k, v) -> (pack k, v)) <$> arbitrary

instance Arbitrary Value where
  arbitrary = resize 5 $ oneof [
      Object <$> arbitraryObject
    , Array . V.fromList <$> arbitrary
    , String . pack <$> arbitrary
    , Number . fromInteger <$> arbitrary
    , Bool <$> arbitrary
    , pure Null
    ]

main :: IO ()
main = hspec spec

spec :: Spec
spec = do
  describe ".:?" $ do
    it "can parse JSON values" $ do
      property $ \x -> do
        let v = object ["foo" .= (x :: Value)]
            p = parseJSON >=> (.:? "foo")
        parseMaybe p v `shouldBe` Just (Just x)

[sol]

The bug surfaces when using genericParseJSON to parse a record with a field of type Maybe Value.

Here is a way to reproduce this (the second spec item will fail):

{-# LANGUAGE DeriveGeneric, OverloadedStrings #-}
module Main (main) where
import Test.Hspec
import GHC.Generics
import Data.Aeson

data Foo = Foo {
  value :: Maybe Value
} deriving (Eq, Show, Generic)

instance FromJSON Foo

main :: IO ()
main = hspec spec

spec :: Spec
spec = do
  describe "FromJSON instance for Foo" $ do
    it "accepts numeric values" $ do
      decode "{\"value\": 23}" `shouldBe` Just (Foo $ Just $ Number 23)

    it "accepts null values" $ do
      decode "{\"value\": null}" `shouldBe` Just (Foo $ Just Null)

    it "accepts nothing" $ do
      decode "{}" `shouldBe` Just (Foo Nothing)

[sol]

Turns out that round-tripping relies on that behavior.

[sol]

So what we actually need is something along the lines of

(.:?) :: (FromJSON a) => Object -> Text -> Parser (Maybe a)
obj .:? key = case H.lookup key obj of
               Nothing -> pure Nothing
               Just v  -> Just <$> parseJSON v <|> parseJSON v

[sol]

Note that this is functionally equivalent with

(.:?) :: (FromJSON a) => Object -> Text -> Parser (Maybe a)
obj .:? key = case H.lookup key obj of
               Nothing -> pure Nothing
               Just v  -> Just <$> parseJSON v <|> (if v == Null then pure Nothing else empty)

[sol]

updated PR

[bos]

I had a history hiccup, and need you to rebase on top of the new history. Sorry!

Also, if you could please include an example in your commit description of what used to fail and now works, that will be important in understanding what this change actually does.

[bos]

Can you please discuss with @lykahb whether this or #232 is the "more correct" fix?

[zlondrej]

I personaly agree with @lykahb's #232 solution.
Alternative is totaly useless here (it either succeedes with first parser or fails with both of them).
By the way solutions in Comment #6 and Comment #7 are not functionally equivalent.

Without Just <$> parseJSON works with type Maybe a and thus returns Nothing on null.

instance (FromJSON a) => FromJSON (Maybe a) where
    parseJSON Null   = pure Nothing
    parseJSON a      = Just <$> parseJSON a

However with Just <$> parseJSON uses correct instance for a.

One example which I think should work and it doesn't is type Maybe (Maybe a) with following code:

Aeson.withObject "Object" $ \o -> mkObject
        <$> o .:? "key" .!= Nothing

Expected behaviour is following:

• Missing key returns Nothing
• null value returns Just Nothing
• Some value (for example 42) returns Just (Just 42)

And actuall behaviour is this:

• Both missing key or null value returns Nothing
• Some value returns Just (Just 42)

Workaround for this issue:

Aeson.withObject "Object" $ \o -> mkObject
        <$> ((Just <$> o .: "redirect-number") <|> return Nothing)

Basically whenever you want to different value with missing key and null value, it just doesn't work.

[bos]

Applied #232 instead. Thanks.