Generic codec: Don't silently succeed when decoding encounters invalid types

[TomMD]

Small ask: Can we get semioptional added to the Bi monad module? I want a Codec for Maybe a to fail if the types do not match. Happy to make the PR if this is ok to you.

-- | Optionally lookup a value or Nothing if it does not exist.  Propagates any error other than not found - such as type errors.
semioptional :: TomlCodec a -> TomlCodec (Maybe a)
semioptional Codec{..} =
    Codec
     { codecRead =
         catchError (Just <$> codecRead)
                    (\case
                        KeyNotFound {}   -> pure Nothing
                        TableNotFound {} -> pure Nothing
                        err -> throwError err
                    )
     , codecWrite = traverse codecWrite
     }

Bigger ask: Can we use semioptional instead of dioptional for the genericCodec? Also happy to make this PR.

[chshersh]

Hi @TomMD! Your proposed implementation of semioptional is almost how dioptional was implemented initially and this implementation was removed in favour of current dioptional in this PR:

• [#82] Remove maybeT, add diOptional #101

It turned out that the previous implementation didn't work as expected in our project. This implementation led to an unexpected bug which was hard to find and spot due to the ad-hoc nature of this function. Current dioptional seems abstract and general enough, so it feels like the right abstraction even if it doesn't solve each use-case.

The problem with our previous version was that it compared expected and given keys to return Nothing only when the required key was not found. But we used encoding for sum types and composed keys, so this check has no chances to succeed and let to parse error when it should return Nothing.

The problem with your solution is that you return Nothing on each KeyNotFound error (even nested ones). So, for example, if given TomlCodec is a codec for a table, and actual table lacks only 1 of 7 keys, you will silently ignore that error and return Nothing while what you (most likely) intended is to return Nothing only if the key for the table itself is not found.

So I can see how a codec that returns Nothing on missing keys can be useful, but the implementation is not apparent and had to be carefully thought through.

[rfn01]

Hi all,

I ended up here after dioptional ate a type error, leading to some confusion.

If I understand, Codec cannot accurately detect where a key-lookup fails. Instead, what about handling omitted keys at the lookup, which happens in match and table (and several others)? I think I've adapted those two functions to use Maybe a to signal a missing key. I also sometimes want an omitted list to parse as an empty list, so I adapted match to Monoid a for that purpose. I think these are less surprising than dioptional, but might be wrong.

Any thoughts about better solutions, or ways to improve these?

Thanks!

import Toml.Codec.BiMap (BiMap (..), TomlBiMap)
import Toml.Codec.Types (Codec(..), TomlCodec, TomlEnv, TomlState(..), eitherToTomlState)
import Toml.Type.AnyValue (AnyValue (..))
import Toml.Type.Key (Key)
import Toml.Type.TOML (TOML(..), insertKeyAnyVal, insertTable)
import Toml.Codec.Combinator.Common (whenLeftBiMapError)
import Toml.Codec.Combinator.Table (handleTableErrors)

import qualified Toml.Type.PrefixTree as Prefix

import qualified Data.HashMap.Strict as HashMap

-- based on match from Toml.Codec.Combinator.Common
matchMaybe :: forall a. TomlBiMap a AnyValue -> Key -> TomlCodec (Maybe a)
matchMaybe BiMap{..} key = Codec input output
  where
    input :: TomlEnv (Maybe a)
    input = \toml -> case HashMap.lookup key (tomlPairs toml) of
      Nothing     -> pure Nothing
      Just anyVal -> whenLeftBiMapError key (backward anyVal) (pure . pure)

    output :: Maybe a -> TomlState (Maybe a)
    output Nothing = pure Nothing
    output (Just a) = do
      anyVal <- eitherToTomlState $ forward a
      (Just a) <$ modify (insertKeyAnyVal key anyVal)


-- based on match from Toml.Codec.Combinator.Common
matchEmpty :: forall a. Monoid a => TomlBiMap a AnyValue -> Key -> TomlCodec a
matchEmpty BiMap{..} key = Codec input output
  where
    input :: TomlEnv a
    input = \toml -> case HashMap.lookup key (tomlPairs toml) of
      Nothing     -> pure mempty
      Just anyVal -> whenLeftBiMapError key (backward anyVal) pure

    output :: a -> TomlState a
    output a = do
      anyVal <- eitherToTomlState $ forward a
      a <$ modify (insertKeyAnyVal key anyVal)


-- based on table from Toml.Codec.Combinator.Table
tableMaybe :: forall a. TomlCodec a -> Key -> TomlCodec (Maybe a)
tableMaybe codec key = Codec input output
  where
    input :: TomlEnv (Maybe a)
    input = \t -> case Prefix.lookup key $ tomlTables t of
      Nothing   -> pure Nothing
      Just toml -> pure <$> handleTableErrors codec key toml

    output :: Maybe a -> TomlState (Maybe a)
    output Nothing = pure Nothing
    output (Just a) = do
      mTable <- gets $ Prefix.lookup key . tomlTables
      let toml = fromMaybe mempty mTable
      let (_, newToml) = unTomlState (codecWrite codec a) toml
      (Just a) <$ modify (insertTable key newToml)