Doesn't work as expected with optional

[pbrisbin]

I'm willing to bet that I've got a misunderstanding here, but I'm surprised by the behavior of Env.Parser with optional:

The following is something I do quite a bit with optparse-applicative:

optParser :: Options.Parser Things
optParser = Things <$> optional
    (Options.option (Options.eitherReader readThing) (Options.long "thing"))

readThing :: String -> Either String Thing
readThing = \case
    "thing" -> Right Thing
    x -> Left $ "Must be thing, was " <> x

The option is itself built with eitherReader, so invalid input (via Left) is an error. But then it's wrapped with optional to make it, well, optional. This is a great separation of concerns as I'll often define the option (Parser Thing) in one place, and then wrap it up with optional (or not) in another place (Parser Things).

Importantly, this does not replace errors with Nothing. If you run this program with missing input, you get Nothing. But if you run it with invalid input you get an error,

% stack runhaskell --package envparse --package optparse-applicative repro.hs
Via ENV: Things {mThing = Nothing}
Via CLI: Things {mThing = Nothing}

% stack runhaskell --package envparse --package optparse-applicative repro.hs -- --thing x
Via ENV: Things {mThing = Nothing}
option --thing: Must be thing, was x

Usage: repro.hs [--thing ARG]

This is what I want.

I'd expect the equivalent envparse program, to behave the same in this regard:

envParser :: Env.Parser Env.Error Things
envParser = Things <$> optional
    (Env.var (first Env.UnreadError . readThing) "THING" mempty)

But it does not. You just get Nothing for both missing and invalid input.

% THING=x stack runhaskell --package envparse --package optparse-applicative repro.hs
Via ENV: Things {mThing = Nothing}
Via CLI: Things {mThing = Nothing}

Is this expected?

Complete example program

{-# LANGUAGE DerivingStrategies #-}
{-# LANGUAGE LambdaCase #-}

module Main where

import Data.Bifunctor (first)
import qualified Env
import Options.Applicative (optional)
import qualified Options.Applicative as Options

main :: IO ()
main = do
    e <- Env.parse (Env.header "example") envParser
    putStrLn $ "Via ENV: " <> show e

    o <- Options.execParser $ Options.info optParser Options.fullDesc
    putStrLn $ "Via CLI: " <> show o


newtype Things = Things
    { mThing :: Maybe Thing
    }
    deriving stock Show

data Thing = Thing
    deriving stock Show

readThing :: String -> Either String Thing
readThing = \case
    "thing" -> Right Thing
    x -> Left $ "Must be thing, was " <> x

envParser :: Env.Parser Env.Error Things
envParser = Things <$> optional
    (Env.var (first Env.UnreadError . readThing) "THING" mempty)

optParser :: Options.Parser Things
optParser = Things <$> optional
    (Options.option (Options.eitherReader readThing) (Options.long "thing"))

[pbrisbin]

Just wanted to let anyone interested know that there is a workaround...

Instead of something like,

optional (Env.var f "NAME" g)

You can do

Env.var (Just <$> f) "NAME" (g <> Env.def Nothing)

In my testing so far, it behaves how I want:

1. Not passing NAME gets Nothing via Env.def
2. Passing a valid NAME (according to f) gets Just
3. Passing an invalid name (according to f) still fails

[supki]

While I'm sure there's a reason optparse-applicative does things this way, I certainly wouldn't want to emulate its (<|>). When I write a <|> b, I expect for b to actually happen on failures in a:

{-# LANGUAGE LambdaCase #-}
module Main where

import Control.Applicative ((<|>))
import Data.Bifunctor (first)
import qualified Env
import qualified Options.Applicative as Options

main :: IO ()
main = do
    e <- Env.parse (Env.header "example") envParser
    putStrLn $ "Via ENV: " <> show e

    o <- Options.execParser $ Options.info optParser Options.fullDesc
    putStrLn $ "Via CLI: " <> show o

data Thing = Thing
    deriving Show

readThing :: String -> Either String Thing
readThing = \case
    "thing" -> Right Thing
    x -> Left $ "Must be thing, was " <> x

conjureThing :: Applicative m => m Thing
conjureThing =
  pure Thing

envParser :: Env.Parser Env.Error Thing
envParser =
  Env.var (first Env.UnreadError . readThing) "THING" mempty
 <|>
  conjureThing

optParser :: Options.Parser Thing
optParser =
  Options.option (Options.eitherReader readThing) (Options.long "thing")
 <|>
  conjureThing

% env THING=x stack runhaskell --package envparse --package optparse-applicative repro.hs -- --thing x
Via ENV: Thing
option --thing: Must be thing, was x

Usage: repro.hs [--thing ARG]

[supki]

I like your workaround in that it clearly separates the case of a failing parser and an entirely missing variable. Maybe this should be provided by the library.

[pbrisbin]

Yeah, I can definitely see the point that, for any monad with failure semantics a <|> b should recover failures in a to b. However, can you agree that the behavior of optparse-applicative is desired?

If some CLI tool wants [--foo=a|b|c | --bar=d|e|f], I do not want --foo=x --bar=d to be success, I want the invalid option --foo=x to generate an error. If optparse-applicative's <|> didn't work this way, I'm not sure how you'd achieve such behavior. Further, I think optional (fooOption <|> barOption) is the "obvious" way to do it.

If you squint, it could be justified if you think of Options.Parser as a monad that combines failure and presence semantics -- it's not purely a failure-monad. Under this view, the way its optional and (<|>) work could be seen as reasonable -- maybe?

[pbrisbin]

I keep bumping into this, and having to remember, explain to my team, and code in the workaround I supplied is getting pretty tedious (and error-prone).

I'm curious if we can discuss a bit more about changing envparse' behavior, starting from when you said,

While I'm sure there's a reason optparse-applicative does things this way, I certainly wouldn't want to emulate its (<|>).

This implies to me that whatever reason optparse-applicative has for doing this, it "certainly" must not apply to envparse. I think that's my main disagreement.

I think the reason optparse-applicative does it that way is because that behavior makes perfect sense for this design space of parsing options where such nuanced semantics exist, and to do otherwise would surprise users trying to solve problems in that space. I'd also argue optparse-applicative is one of the best, if not the best library in this design space. Why would envparse, being that it's the exact same design space, not want to follow its lead?