Decoder contract

[sjakobi]

#1711 raised my attention to this contract that I somehow hadn't been aware of before.

dhall-haskell/dhall/src/Dhall.hs

Lines 265 to 300 in 0c4b59a

	{-| Every `Decoder` must obey the contract that if an expression's type matches the
	the `expected` type then the `extract` function must not fail with a type error.
	If not, then this value is returned.
	This value indicates that an invalid `Decoder` was provided to the `input`
	function
	-}
	data InvalidDecoder s a = InvalidDecoder
	{ invalidDecoderExpected :: Expr s a
	, invalidDecoderExpression :: Expr s a
	}
	deriving (Typeable)
	instance (Pretty s, Typeable s, Pretty a, Typeable a) => Exception (InvalidDecoder s a)
	_ERROR :: String
	_ERROR = "\ESC[1;31mError\ESC[0m"
	instance (Pretty s, Pretty a, Typeable s, Typeable a) => Show (InvalidDecoder s a) where
	show InvalidDecoder { .. } =
	_ERROR <> ": Invalid Dhall.Decoder \n\
	\ \n\
	\Every Decoder must provide an extract function that succeeds if an expression \n\
	\matches the expected type. You provided a Decoder that disobeys this contract \n\
	\ \n\
	\The Decoder provided has the expected dhall type: \n\
	\ \n\
	\" <> show txt0 <> "\n\
	\ \n\
	\and it couldn't extract a value from the well-typed expression: \n\
	\ \n\
	\" <> show txt1 <> "\n\
	\ \n"
	where
	txt0 = Dhall.Util.insert invalidDecoderExpected
	txt1 = Dhall.Util.insert invalidDecoderExpression

If I understand this contract correctly, we have at least two Decoders that break it:

• dhall-haskell/dhall/src/Dhall.hs

  Lines 939 to 940 in 0c4b59a

  	setFromDistinctList :: (Ord a, Show a) => Decoder a -> Decoder (Data.Set.Set a)
  	setFromDistinctList = setHelper Data.Set.size Data.Set.fromList

• dhall-haskell/dhall/src/Dhall.hs

  Lines 968 to 971 in 0c4b59a

  	hashSetFromDistinctList :: (Hashable a, Ord a, Show a)
  	=> Decoder a
  	-> Decoder (Data.HashSet.HashSet a)
  	hashSetFromDistinctList = setHelper Data.HashSet.size Data.HashSet.fromList

We also use these Decoders in the instances for Set and HashSet.

I've also suggested defining contract-breaking Decoders to @SiriusStarr before…

What's the reasoning behind this contract?

If we want to keep this contract, we should document it in the haddocks for Decoder.

[Gabriella439]

@sjakobi: That contract was created before we started adding support for custom decoders with post-marshaling validation checks. I think it's fine to remove the contract or update it to reflect the new use cases for custom decoders.

[sjakobi]

if an expression's type matches the expected type then the extract function must not fail with a type error.

I think I had simply overlooked the with a type error bit. The contract does seem to allow extract to fail with a non-type error. Maybe we could add a sentence like

extract may fail for other reasons though.