Fix unexpected error with preset ENV and .env.example (#156)

* Fix building on Stackage LTS

- Bump LTS, remove extra-deps
- Relax lower bound on mtl

* Reproduce bug

* Fix `union`/`unionBy` bug

When the ambient environment is combined with the read values, this code
used `union` which compares key _and value_. This ends up duplicating
the variable in the list that's intersected (correctly on key only
there) and the difference in length results in the missing-variable
error unnecessarily. We already have a `unionEnvs` here that does the
right thing, we just have to use it.

* Don't union in neededVars

I could be missing something, but this didn't make sense to me:

If we're in this branch, then the `ambient+read` variables have the same
keys as needed, this means `read+needed` would only add those keys in
needed that are already in the `ambient`, which also means they need to
be set. Using only `read` here is functionally equivalent.

I'd also argue that actually _using_ anything from the `.env.example`
file (beyond validating the `.env`), if that were somehow happening
despite my analysis above, would be surprising to end users anyway.

* Refactor .env.example check, improve error message

I found all of the `*By` and `cmpEnv` stuff really hard to follow and
unnecessarily complex. I think it led to the recently-fixed bug.

I find this version more straight forward. Hopefully it's objectively
less complicated too; it at least reduced two conditionals to one, which
is one quantitative improvement.

This version also made it easier to identify specifically the missing
keys for the error message. The code before would report *all* keys from
the example file(s) and leave it up to the user to sort out what was
present or not. Now we can report specific keys directly. I made some
minor quality-of-life improvements to the message too.

Before:

    Missing env vars! Please, check (this/these) var(s) (is/are) set:
    THIS_ENV, THAT_ENV, MISSING_ENV, THE_OTHER_ENV, ....

After (one `.env`, one `.env.example`):

    The following variables are present in .env.example, but not set in
    the current environment or .env: MISSING_ENV

After (many `.env`s, many `.env.example`s):

    The following variables are present in one of .env.example.1,
    .env.example.2, but not set in the current environment, or any of
    .env.1, .env.2: MISSING_ENV

This should be much less confusing to end-users.

* Model unexpected empty list case in the types

Author: pbrisbin

dotenv.cabal

@@ -96,7 +96,7 @@ library
                        , shellwords >= 0.1.3.0
                        , text
                        , exceptions >= 0.8 && < 0.11
-                       , mtl >= 2.3 && < 2.4
+                       , mtl >= 2.2.2 && < 2.4
 
   hs-source-dirs:      src
   ghc-options:         -Wall

spec/Configuration/DotenvSpec.hs

@@ -72,7 +72,7 @@ spec = do
           unsetEnv "ANOTHER_ENV"
           loadFile config `shouldThrow` anyErrorCall
 
-      context "when the needed env vars are not missing" $
+      context "when the needed env vars are not missing" $ do
         it "should succeed when loading all of the needed env vars" $ do
           -- Load extra information
           me <- init <$> readCreateProcess (shell "whoami") ""
@@ -88,6 +88,13 @@ spec = do
           lookupEnv "UNICODE_TEST" `shouldReturn` Just "Manabí"
           lookupEnv "ANOTHER_ENV" `shouldReturn` Just "hello"
 
+        it "succeeds when variable is preset" $ do
+          setEnv "DOTENV" "preset"
+
+          loadFile config
+
+          lookupEnv "DOTENV" `shouldReturn` Just "preset"
+
   describe "parseFile" $ after_ clearEnvs $ do
     it "returns variables from a file without changing the environment" $ do
       lookupEnv "DOTENV" `shouldReturn` Nothing

src/Configuration/Dotenv.hs

@@ -31,8 +31,10 @@ import           Control.Exception                   (throw)
 import           Control.Monad                       (unless, when)
 import           Control.Monad.Catch
 import           Control.Monad.IO.Class              (MonadIO (..))
-import           Data.List                           (intersectBy, union,
-                                                      unionBy)
+import           Data.Function                       (on)
+import           Data.List.NonEmpty                  (NonEmpty(..))
+import qualified Data.List.NonEmpty as NE
+import           Data.List                           ((\\), intercalate, union)
 import           System.IO.Error                     (isDoesNotExistError)
 import           Text.Megaparsec                     (errorBundlePretty, parse)
 
@@ -59,22 +61,36 @@ loadFile ::
   -> m ()
 loadFile config@Config {..} = do
   environment <- liftIO getEnvironment
-  readVars <- fmap concat (mapM parseFile configPath)
-  neededVars <- fmap concat (mapM parseFile configExamplePath)
-  let coincidences = (environment `union` readVars) `intersectEnvs` neededVars
-      cmpEnvs env1 env2 = fst env1 == fst env2
-      intersectEnvs = intersectBy cmpEnvs
-      unionEnvs = unionBy cmpEnvs
-      vars =
-        if (not . null) neededVars
-          then if length neededVars == length coincidences
-                 then readVars `unionEnvs` neededVars
-                 else error $
-                      "Missing env vars! Please, check (this/these) var(s) (is/are) set:" ++
-                      concatMap ((++) " " . fst) neededVars
-          else readVars
+
+  vars <- case (NE.nonEmpty configPath, NE.nonEmpty configExamplePath) of
+    (Nothing, _) -> pure []
+    (Just envs, Nothing) -> concat <$> mapM parseFile envs
+    (Just envs, Just envExamples) -> do
+      readVars <- concat <$> mapM parseFile envs
+      neededKeys <- map fst . concat <$> mapM parseFile envExamples
+
+      let
+        presentKeys = (union `on` map fst) environment readVars
+        missingKeys = neededKeys \\ presentKeys
+
+      pure $
+        if null missingKeys
+          then readVars
+          else error $ concat
+            [ "The following variables are present in "
+            , showPaths "one of " envExamples
+            , ", but not set in the current environment, or"
+            , showPaths "any of " envs
+            , ": "
+            , intercalate ", " missingKeys
+            ]
+
   unless allowDuplicates $ (lookUpDuplicates . map fst) vars
   runReaderT (mapM_ applySetting vars) config
+ where
+  showPaths :: String -> NonEmpty FilePath -> String
+  showPaths _ (p:|[]) = p
+  showPaths prefix ps = prefix <> intercalate ", " (NE.toList ps)
 
 -- | Parses the given dotenv file and returns values /without/ adding them to
 -- the environment.

stack.yaml

@@ -1,6 +1 @@
-resolver: lts-19.0
-packages:
-- '.'
-extra-deps:
-- optparse-applicative-0.17.0.0
-- shellwords-0.1.3.0
+resolver: lts-20.11

stack.yaml.lock

@@ -3,24 +3,10 @@
 # For more information, please see the documentation at:
 #   https://docs.haskellstack.org/en/stable/lock_files
 
-packages:
-- completed:
-    hackage: optparse-applicative-0.17.0.0@sha256:0713e54cbb341e5cae979e2ac441eb3a5ff42e303001f432bd58c19e5638bdda,4967
-    pantry-tree:
-      size: 3124
-      sha256: 3b4398845ee6718fe2303870a5ab75bd190eb712fed2f22caaae30f790c490fd
-  original:
-    hackage: optparse-applicative-0.17.0.0
-- completed:
-    hackage: shellwords-0.1.3.0@sha256:baf03b1e80dbc5dffbe6ba5451ee7c389ac39e3e2f9c24030c26a639522a1032,2226
-    pantry-tree:
-      size: 496
-      sha256: ce076c4ec323107f8e84d25147b4f7c865e8cd27d804f384b9d1e2d5706ba0c3
-  original:
-    hackage: shellwords-0.1.3.0
+packages: []
 snapshots:
 - completed:
-    size: 616897
-    url: https://raw.githubusercontent.com/commercialhaskell/stackage-snapshots/master/lts/19/0.yaml
-    sha256: bbf2be02f17940bac1f87cb462d4fb0c3355de6dcfc53d84f4f9ad3ee2164f65
-  original: lts-19.0
+    sha256: adbc602422dde10cc330175da7de8609e70afc41449a7e2d6e8b1827aa0e5008
+    size: 649342
+    url: https://raw.githubusercontent.com/commercialhaskell/stackage-snapshots/master/lts/20/11.yaml
+  original: lts-20.11