Merge #1006: Upd {{Value, Eval Provenance, JSON} builders, Option getters, Utils}

https://github.com/haskell-nix/hnix/wiki/Naming-style-guidelines

Closes a lot of #949

Option getter names are not ideal (I can not name them properly currently because every one of them requires in-depth lookup of what was the semantic meanings implied but not expressed, as it was not documented, to name them a superset of them must be considered. And strangely, at least a couple of them, based on the code, - are redundant, a synonym of one another. But current naming is than before & push thoughts into the right direction.


`NixContext` & `NixString` now have contents before the carrier, as frequently the carrier is modified, & `swap` happens once - before writing into state monad.

...

Most of this is the stuff I wanted to do for a long time.

Author: Anton-Latukha

.hlint.yaml

@@ -19,21 +19,24 @@
 - ignore:
     name: Use Foldable.forM_
 - hint:
-    lhs: "pure ()"
+    lhs: "pure mempty"
     note: "Use 'stub'"
     rhs: stub
+
 - hint:
-    lhs: "return ()"
-    note: "Use 'stub'"
-    rhs: stub
+    lhs: "[]"
+    note: "Use `mempty`"
+    rhs: mempty
+
 - hint:
-    lhs: "(: [])"
+    lhs: "(: mempty)"
     note: "Use `one`"
     rhs: one
 - hint:
-    lhs: "(:| [])"
+    lhs: "(:| mempty)"
     note: "Use `one`"
     rhs: one
+
 - hint:
     lhs: Data.Sequence.singleton
     note: "Use `one`"
@@ -91,7 +94,7 @@
     note: "Use `one`"
     rhs: one
 - warn:
-    lhs: "Control.Exception.evaluate (x `deepseq` ())"
+    lhs: "Control.Exception.evaluate (x `deepseq` mempty)"
     rhs: evaluateNF_ x
 - warn:
     lhs: "void (evaluateWHNF x)"
@@ -106,19 +109,19 @@
     lhs: "foldl' (*) 1"
     rhs: product
 - hint:
-    lhs: "fmap and (sequence s)"
+    lhs: "fmap and (sequenceA s)"
     note: Applying this hint would mean that some actions that were being executed previously would no longer be executed.
     rhs: andM s
 - hint:
-    lhs: "and <$> sequence s"
+    lhs: "and <$> sequenceA s"
     note: Applying this hint would mean that some actions that were being executed previously would no longer be executed.
     rhs: andM s
 - hint:
-    lhs: "fmap or (sequence s)"
+    lhs: "fmap or (sequenceA s)"
     note: Applying this hint would mean that some actions that were being executed previously would no longer be executed.
     rhs: orM s
 - hint:
-    lhs: "or <$> sequence s"
+    lhs: "or <$> sequenceA s"
     note: Applying this hint would mean that some actions that were being executed previously would no longer be executed.
     rhs: orM s
 - hint:
@@ -148,32 +151,16 @@
     note: "Use 'asumMap'"
     rhs: asumMap f
 - hint:
-    lhs: "asum (map f xs)"
+    lhs: "asum (fmap f xs)"
     note: "Use 'asumMap'"
     rhs: asumMap f xs
 - warn:
-    lhs: "map fst &&& map snd"
+    lhs: "fmap fst &&& fmap snd"
     rhs: unzip
 - hint:
     lhs: "fmap (fmap f) x"
     note: "Use '(<<$>>)'"
     rhs: "f <<$>> x"
-- hint:
-    lhs: "(\\f -> f x) <$> ff"
-    note: Use flap operator
-    rhs: "ff ?? x"
-- hint:
-    lhs: "fmap (\\f -> f x) ff"
-    note: Use flap operator
-    rhs: "ff ?? x"
-- hint:
-    lhs: "fmap ($ x) ff"
-    note: Use flap operator
-    rhs: "ff ?? x"
-- hint:
-    lhs: "($ x) <$> ff"
-    note: Use flap operator
-    rhs: "ff ?? x"
 - warn:
     lhs: "fmap f (nonEmpty x)"
     rhs: viaNonEmpty f x
@@ -184,10 +171,10 @@
     lhs: "f <$> nonEmpty x"
     rhs: viaNonEmpty f x
 - warn:
-    lhs: partitionEithers . map f
+    lhs: partitionEithers . fmap f
     rhs: partitionWith f
 - warn:
-    lhs: partitionEithers $ map f x
+    lhs: partitionEithers $ fmap f x
     rhs: partitionWith f x
 - warn:
     lhs: forever
@@ -224,40 +211,16 @@
     lhs: "maybe (Right r) Left"
     rhs: maybeToLeft r
 - warn:
-    lhs: "case m of [] -> return (); (x:xs) -> f (x :| xs)"
-    rhs: whenNotNull m f
-- warn:
-    lhs: "case m of [] -> pure ()  ; (x:xs) -> f (x :| xs)"
+    lhs: "case m of [] -> stub ; (x:xs) -> f (x :| xs)"
     rhs: whenNotNull m f
 - warn:
-    lhs: "case m of [] -> pass     ; (x:xs) -> f (x :| xs)"
+    lhs: "case m of (x:xs) -> f (x :| xs); [] -> stub"
     rhs: whenNotNull m f
 - warn:
-    lhs: "case m of (x:xs) -> f (x :| xs); [] -> return ()"
-    rhs: whenNotNull m f
-- warn:
-    lhs: "case m of (x:xs) -> f (x :| xs); [] -> pure ()  "
-    rhs: whenNotNull m f
-- warn:
-    lhs: "case m of (x:xs) -> f (x :| xs); [] -> pass     "
-    rhs: whenNotNull m f
-- warn:
-    lhs: "m >>= \\case [] -> pass     ; (x:xs) -> f (x :| xs)"
+    lhs: "m >>= \\case [] -> stub ; (x:xs) -> f (x :| xs)"
     rhs: whenNotNullM m f
 - warn:
-    lhs: "m >>= \\case [] -> pure ()  ; (x:xs) -> f (x :| xs)"
-    rhs: whenNotNullM m f
-- warn:
-    lhs: "m >>= \\case [] -> return (); (x:xs) -> f (x :| xs)"
-    rhs: whenNotNullM m f
-- warn:
-    lhs: "m >>= \\case (x:xs) -> f (x :| xs); [] -> pass     "
-    rhs: whenNotNullM m f
-- warn:
-    lhs: "m >>= \\case (x:xs) -> f (x :| xs); [] -> pure ()  "
-    rhs: whenNotNullM m f
-- warn:
-    lhs: "m >>= \\case (x:xs) -> f (x :| xs); [] -> return ()"
+    lhs: "m >>= \\case (x:xs) -> f (x :| xs); [] -> stub"
     rhs: whenNotNullM m f
 - warn:
     lhs: mapMaybe leftToMaybe
@@ -292,27 +255,15 @@
 - warn:
     lhs: "MaybeT (pure m)"
     rhs: hoistMaybe m
-- warn:
-    lhs: "MaybeT (return m)"
-    rhs: hoistMaybe m
 - warn:
     lhs: MaybeT . pure
     rhs: hoistMaybe
-- warn:
-    lhs: MaybeT . return
-    rhs: hoistMaybe
 - warn:
     lhs: "ExceptT (pure m)"
     rhs: hoistEither m
-- warn:
-    lhs: "ExceptT (return m)"
-    rhs: hoistEither m
 - warn:
     lhs: ExceptT . pure
     rhs: hoistEither
-- warn:
-    lhs: ExceptT . return
-    rhs: hoistEither
 - warn:
     lhs: fromMaybe mempty
     rhs: maybeToMonoid
@@ -1525,16 +1476,16 @@
     note: "'writeIORef' is already exported from Relude"
     rhs: writeIORef
 - warn:
-    lhs: "atomicModifyIORef ref (\\a -> (f a, ()))"
+    lhs: "atomicModifyIORef ref (\\a -> (f a, mempty))"
     rhs: atomicModifyIORef_ ref f
 - warn:
-    lhs: "atomicModifyIORef ref $ \\a -> (f a, ())"
+    lhs: "atomicModifyIORef ref $ \\a -> (f a, mempty)"
     rhs: atomicModifyIORef_ ref f
 - warn:
-    lhs: "atomicModifyIORef' ref $ \\a -> (f a, ())"
+    lhs: "atomicModifyIORef' ref $ \\a -> (f a, mempty)"
     rhs: "atomicModifyIORef'_ ref f"
 - warn:
-    lhs: "atomicModifyIORef' ref (\\a -> (f a, ()))"
+    lhs: "atomicModifyIORef' ref (\\a -> (f a, mempty))"
     rhs: "atomicModifyIORef'_ ref f"
 - warn:
     lhs: Data.Text.IO.getLine
@@ -2767,8 +2718,32 @@
     note: "Monad law"
     rhs: id
 
+- hint:
+    lhs: "join (f . pure)"
+    note: "Monad law"
+    rhs: (f =<< pure)
+
 - hint:
     lhs: "join . (f <<$>>)"
     note: "Monad law"
     rhs: fmap f . join
 
+- hint:
+    lhs: "Data.Traversable.sequence"
+    note: "Modern name is `sequenceA`"
+    rhs: sequenceA
+
+- hint:
+    lhs: "()"
+    note: "Use `mempty`"
+    rhs: mempty
+
+- hint:
+    lhs: "return"
+    note: "Please, use `pure` instead. even GHC already deprecates `return` starting from `9.2`."
+    rhs: pure
+
+- hint:
+    lhs: "map"
+    note: "Use `fmap`"
+    rhs: fmap

main/Main.hs

@@ -36,8 +36,8 @@ import           Nix.Eval
 main :: IO ()
 main =
   do
-    time <- getCurrentTime
-    opts <- execParser $ nixOptionsInfo time
+    currentTime <- getCurrentTime
+    opts <- execParser $ nixOptionsInfo currentTime
 
     main' opts
 
@@ -57,7 +57,7 @@ main' opts@Options{..} = runWithBasicEffectsIO opts execContentsFilesOrRepl
     -- | The base case: read expressions from the last CLI directive (@[FILE]@) listed on the command line.
     loadFromCliFilePathList :: StdIO
     loadFromCliFilePathList =
-      case filePaths of
+      case getFilePaths of
         []     -> runRepl
         ["-"]  -> readExpressionFromStdin
         _paths -> processSeveralFiles (coerce _paths)
@@ -80,11 +80,11 @@ main' opts@Options{..} = runWithBasicEffectsIO opts execContentsFilesOrRepl
         do
           let file = replaceExtension binaryCacheFile "nixc"
           processCLIOptions (pure file) =<< liftIO (readCache binaryCacheFile)
-      ) <$> readFrom
+      ) <$> getReadFrom
 
     -- | The `--expr` option: read expression from the argument string
     loadLiteralExpression :: Maybe StdIO
-    loadLiteralExpression = processExpr <$> expression
+    loadLiteralExpression = processExpr <$> getExpression
 
     -- | The `--file` argument: read expressions from the files listed in the argument file
     loadExpressionFromFile :: Maybe StdIO
@@ -96,7 +96,7 @@ main' opts@Options{..} = runWithBasicEffectsIO opts execContentsFilesOrRepl
         (\case
           "-" -> Text.getContents
           _fp -> readFile _fp
-        ) <$> fromFile
+        ) <$> getFromFile
 
   processExpr :: Text -> StdIO
   processExpr = handleResult mempty . parseNixTextLoc
@@ -110,13 +110,13 @@ main' opts@Options{..} = runWithBasicEffectsIO opts execContentsFilesOrRepl
         bool
           errorWithoutStackTrace
           (liftIO . hPutStrLn stderr)
-          ignoreErrors
+          isIgnoreErrors
           $ "Parse failed: " <> show err
       )
 
       (\ expr ->
         do
-          when check $
+          when isCheck $
             do
               expr' <- liftIO $ reduceExpr mpath expr
               either
@@ -138,28 +138,28 @@ main' opts@Options{..} = runWithBasicEffectsIO opts execContentsFilesOrRepl
                     @StdThun
                     frames
 
-          when repl $
+          when isRepl $
             withEmptyNixContext $
               bool
                 Repl.main
                 ((Repl.main' . pure) =<< nixEvalExprLoc (coerce mpath) expr)
-                evaluate
+                isEvaluate
       )
 
   --  2021-07-15: NOTE: Logic of CLI Option processing is scattered over several functions, needs to be consolicated.
   processCLIOptions :: Maybe Path -> NExprLoc -> StdIO
   processCLIOptions mpath expr
-    | evaluate =
+    | isEvaluate =
       if
-        | tracing                       -> evaluateExprWithEvaluator nixTracingEvalExprLoc expr
-        | Just path <- reduce           -> evaluateExprWithEvaluator (reduction path . coerce) expr
-        | null arg || null argstr       -> evaluateExprWithEvaluator nixEvalExprLoc expr
+        | isTrace                       -> evaluateExprWith nixTracingEvalExprLoc expr
+        | Just path <- getReduce           -> evaluateExprWith (reduction path . coerce) expr
+        | null getArg || null getArgstr       -> evaluateExprWith nixEvalExprLoc expr
         | otherwise                     -> processResult printer <=< nixEvalExprLoc (coerce mpath) $ expr
-    | xml                        = fail "Rendering expression trees to XML is not yet implemented"
-    | json                       = fail "Rendering expression trees to JSON is not implemented"
-    | verbose >= DebugInfo       =  liftIO . putStr . ppShow . stripAnnotation $ expr
-    | cache , Just path <- mpath =  liftIO . writeCache (replaceExtension path "nixc") $ expr
-    | parseOnly                  =  void . liftIO . Exception.evaluate . force $ expr
+    | isXml                        = fail "Rendering expression trees to XML is not yet implemented"
+    | isJson                       = fail "Rendering expression trees to JSON is not implemented"
+    | getVerbosity >= DebugInfo       =  liftIO . putStr . ppShow . stripAnnotation $ expr
+    | isCache , Just path <- mpath =  liftIO . writeCache (replaceExtension path "nixc") $ expr
+    | isParseOnly                  =  void . liftIO . Exception.evaluate . force $ expr
     | otherwise                  =
       liftIO .
         renderIO
@@ -169,26 +169,26 @@ main' opts@Options{..} = runWithBasicEffectsIO opts execContentsFilesOrRepl
           . stripAnnotation
           $ expr
    where
-    evaluateExprWithEvaluator evaluator = evaluateExpression (coerce mpath) evaluator printer
+    evaluateExprWith evaluator = evaluateExpression (coerce mpath) evaluator printer
 
     printer
       :: StdVal
       -> StdIO
     printer
-      | finder    = findAttrs <=< fromValue @(AttrSet StdVal)
+      | isFinder    = findAttrs <=< fromValue @(AttrSet StdVal)
       | otherwise = printer'
      where
       printer'
-        | xml       = fun (stringIgnoreContext . toXML)                     normalForm
+        | isXml       = fun (ignoreContext . toXML)                     normalForm
         -- 2021-05-27: NOTE: With naive fix of the #941
         -- This is overall a naive printer implementation, as options should interact/respect one another.
         -- A nice question: "Should respect one another to what degree?": Go full combinator way, for which
         -- old Nix CLI is nototrious for (and that would mean to reimplement the old Nix CLI),
         -- OR: https://github.com/haskell-nix/hnix/issues/172 and have some sane standart/default behaviour for (most) keys.
-        | json      = fun (stringIgnoreContext . mempty . nvalueToJSONNixString) normalForm
-        | strict    = fun (show . prettyNValue)                             normalForm
-        | values    = fun (show . prettyNValueProv)                         removeEffects
-        | otherwise = fun (show . prettyNValue)                             removeEffects
+        | isJson      = fun (ignoreContext . mempty . toJSONNixString) normalForm
+        | isStrict    = fun (show . prettyNValue)                       normalForm
+        | isValues    = fun (show . prettyNValueProv)                   removeEffects
+        | otherwise = fun (show . prettyNValue)                       removeEffects
        where
         fun
           :: (b -> Text)