Skip to content

Commit

Permalink
Parser: Result is just an euphemism to Either
Browse files Browse the repository at this point in the history
Implementation show that we so far not needed the custom type for it. Right and
Left already embodies the whole semantic meaning, since Either is mostly used as
Right for computations being successful, and Left for failure. So why bother
everyone with remembering the custom constructors and what they are for if they
are not needed, Either is already know to everybody.

Current implementation by itself shows that `Result` custom type is being YAGNI,
since replacing it with Either changes nothing.
So it relates to the topic of #879.

If we would need ad-hock abilities to "our custom class" - the
`TypeSynonymInstances` are there for us.

I did the main work abstracting implementation from the type and constructors
here, so if we would want in the future to change the type - we would just need
to replace the `either` function with `result` function and that is all.

But just using `either` is more consise.

Type synonym left the signature information there. I'd even considered to remove
`Result` all togather, since the more I talk about it the more it becomes clear
that its use is just what Haskellers use `Either` for.
  • Loading branch information
Anton-Latukha committed Mar 12, 2021
1 parent dbef5ec commit 8ab92d1
Show file tree
Hide file tree
Showing 16 changed files with 233 additions and 166 deletions.
16 changes: 9 additions & 7 deletions main/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,15 @@ main = do
eres <- parseNixFileLoc path
handleResult opts (pure path) eres

handleResult opts mpath = \case
Failure err ->
bool
errorWithoutStackTrace
(liftIO . hPutStrLn stderr)
(ignoreErrors opts)
$ "Parse failed: " <> show err
handleResult opts mpath =
either
(\ err ->
bool
errorWithoutStackTrace
(liftIO . hPutStrLn stderr)
(ignoreErrors opts)
$ "Parse failed: " <> show err
)

(\ expr ->
do
Expand Down
26 changes: 15 additions & 11 deletions main/Repl.hs
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,11 @@ initState mIni = do
}
where
evalText :: (MonadNix e t f m) => Text -> m (NValue t f m)
evalText expr = case parseNixTextLoc expr of
Failure e -> error $ "Impossible happened: Unable to parse expression - '" <> Text.unpack expr <> "' error was " <> show e
Success e -> do evalExprLoc e
evalText expr =
either
(\ e -> error $ "Impossible happened: Unable to parse expression - '" <> Text.unpack expr <> "' error was " <> show e)
(\ e -> do evalExprLoc e)
(parseNixTextLoc expr)

type Repl e t f m = HaskelineT (StateT (IState t f m) m)

Expand All @@ -205,10 +207,10 @@ exec update source = do

-- Parser ( returns AST as `NExprLoc` )
case parseExprOrBinding source of
(Failure err, _) -> do
(Left err, _) -> do
liftIO $ print err
pure Nothing
(Success expr, isBinding) -> do
(Right expr, isBinding) -> do

-- Type Inference ( returns Typing Environment )
--
Expand Down Expand Up @@ -245,12 +247,14 @@ exec update source = do
-- This allows us to handle assignments like @a = 42@
-- which get turned into @{ a = 42; }@
parseExprOrBinding i =
case parseNixTextLoc i of
Success expr -> (Success expr, False)
Failure e ->
case parseNixTextLoc $ toAttrSet i of
Failure _ -> (Failure e, False) -- return the first parsing failure
Success e' -> (Success e', True)
either
(\ e ->
either
(const (Left e, False)) -- return the first parsing failure
(\ e' -> (pure e', True))
(parseNixTextLoc $ toAttrSet i))
(\ expr -> (pure expr, False))
(parseNixTextLoc i)

toAttrSet i =
"{" <> i <> bool ";" mempty (Text.isSuffixOf ";" i) <> "}"
Expand Down
7 changes: 4 additions & 3 deletions src/Nix.hs
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,10 @@ evaluateExpression mpath evaluator handler expr = do
) =<< demand f
where
parseArg s =
case parseNixText s of
Success x -> x
Failure err -> errorWithoutStackTrace (show err)
either
(errorWithoutStackTrace . show)
id
(parseNixText s)

eval' = normalForm <=< nixEvalExpr mpath

Expand Down
2 changes: 1 addition & 1 deletion src/Nix/Builtins.hs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ derivation
=> m (NValue t f m)
derivation = foldFix Eval.eval $$(do
-- This is compiled in so that we only parse it once at compile-time.
let Success expr = parseNixText [text|
let Right expr = parseNixText [text|
drvAttrs @ { outputs ? [ "out" ], ... }:

let
Expand Down
14 changes: 8 additions & 6 deletions src/Nix/Effects.hs
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,10 @@ instance MonadExec IO where
if T.null t
then pure $ Left $ ErrorCall $ "exec has no output :" <> emsg
else
case parseNixTextLoc t of
Failure err -> pure $ Left $ ErrorCall $ "Error parsing output of exec: " <> show err <> " " <> emsg
Success v -> pure $ Right v
either
(\ err -> pure $ Left $ ErrorCall $ "Error parsing output of exec: " <> show err <> " " <> emsg)
(pure . pure)
(parseNixTextLoc t)
err -> pure $ Left $ ErrorCall $ "exec failed: " <> show err <> " " <> emsg

deriving
Expand Down Expand Up @@ -204,9 +205,10 @@ instance MonadInstantiate IO where

pure $ case exitCode of
ExitSuccess ->
case parseNixTextLoc (T.pack out) of
Failure e -> Left $ ErrorCall $ "Error parsing output of nix-instantiate: " <> show e
Success v -> Right v
either
(\ e -> Left $ ErrorCall $ "Error parsing output of nix-instantiate: " <> show e)
pure
(parseNixTextLoc (T.pack out))
status -> Left $ ErrorCall $ "nix-instantiate failed: " <> show status <> ": " <> err

deriving
Expand Down
8 changes: 5 additions & 3 deletions src/Nix/Effects/Basic.hs
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,14 @@ defaultImportPath path = do
maybe
(do
eres <- parseNixFileLoc path
case eres of
Failure err -> throwError $ ErrorCall . show $ fillSep ["Parse during import failed:", err]
Success expr ->
either
(\ err -> throwError $ ErrorCall . show $ fillSep ["Parse during import failed:", err])
(\ expr ->
do
modify (first (M.insert path expr))
pure expr
)
eres
)
pure -- return expr
(M.lookup path imports)
Expand Down
14 changes: 7 additions & 7 deletions src/Nix/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module Nix.Parser
, parseFromFileEx
, Parser
, parseFromText
, Result(..)
, Result
, reservedNames
, OperatorInfo(..)
, NSpecialOp(..)
Expand Down Expand Up @@ -566,8 +566,8 @@ reservedNames =

type Parser = ParsecT Void Text (State SourcePos)

-- This is just a @Either (Doc Void) a@
data Result a = Success a | Failure (Doc Void) deriving (Show, Functor)
type Result a = Either (Doc Void) a


parseFromFileEx :: MonadFile m => Parser a -> FilePath -> m (Result a)
parseFromFileEx p path =
Expand All @@ -576,16 +576,16 @@ parseFromFileEx p path =

pure $
either
(Failure . pretty . errorBundlePretty)
Success
(Left . pretty . errorBundlePretty)
Right
$ (`evalState` initialPos path) $ runParserT p path txt

parseFromText :: Parser a -> Text -> Result a
parseFromText p txt =
let file = "<string>" in
either
(Failure . pretty . errorBundlePretty)
Success
(Left . pretty . errorBundlePretty)
Right
$ (`evalState` initialPos file) $ (`runParserT` file) p txt

{- Parser.Operators -}
Expand Down
32 changes: 19 additions & 13 deletions src/Nix/Reduce.hs
Original file line number Diff line number Diff line change
Expand Up @@ -94,29 +94,35 @@ staticImport pann path = do
(maybe path (\p -> takeDirectory p </> path) mfile)

imports <- gets fst
case M.lookup path' imports of
Just expr -> pure expr
Nothing -> go path'
maybe
(go path')
pure
(M.lookup path' imports)
where
go path = do
liftIO $ putStrLn $ "Importing file " <> path

eres <- liftIO $ parseNixFileLoc path
case eres of
Failure err -> error $ "Parse failed: " <> show err
Success x -> do
either
(\ err -> error $ "Parse failed: " <> show err)
(\ x -> do
let
pos = SourcePos "Reduce.hs" (mkPos 1) (mkPos 1)
span = SrcSpan pos pos
cur = NamedVar (StaticKey "__cur_file" :| mempty)
(Fix (NLiteralPath_ pann path))
pos
cur =
NamedVar
(StaticKey "__cur_file" :| mempty)
(Fix (NLiteralPath_ pann path))
pos
x' = Fix (NLet_ span [cur] x)
modify (first (M.insert path x'))
local (const (pure path, emptyScopes @m @NExprLoc)) $ do
x'' <- foldFix reduce x'
modify (first (M.insert path x''))
pure x''
local (const (pure path, emptyScopes @m @NExprLoc)) $
do
x'' <- foldFix reduce x'
modify (first (M.insert path x''))
pure x''
)
eres

-- gatherNames :: NExprLoc -> HashSet VarName
-- gatherNames = foldFix $ \case
Expand Down
20 changes: 13 additions & 7 deletions src/Nix/TH.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import Nix.Parser

quoteExprExp :: String -> ExpQ
quoteExprExp s = do
expr <- case parseNixText (Text.pack s) of
Failure err -> fail $ show err
Success e -> pure e
expr <-
either
(fail . show)
pure
(parseNixText (Text.pack s))
dataToExpQ
(const Nothing `extQ` metaExp (freeVars expr) `extQ` (pure . liftText))
expr
Expand All @@ -37,10 +39,14 @@ quoteExprExp s = do

quoteExprPat :: String -> PatQ
quoteExprPat s = do
expr <- case parseNixText (Text.pack s) of
Failure err -> fail $ show err
Success e -> pure e
dataToPatQ (const Nothing `extQ` metaPat (freeVars expr)) expr
expr <-
either
(fail . show)
pure
(parseNixText (Text.pack s))
dataToPatQ
(const Nothing `extQ` metaPat (freeVars expr))
expr

freeVars :: NExpr -> Set VarName
freeVars e = case unFix e of
Expand Down
44 changes: 26 additions & 18 deletions tests/EvalTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import Nix
import Nix.Standard
import Nix.TH
import Nix.Value.Equal
import Nix.Utils
import qualified System.Directory as D
import System.Environment
import System.FilePath
Expand Down Expand Up @@ -466,10 +467,10 @@ constantEqual expected actual = do
assertBool message eq

constantEqualText' :: Text -> Text -> Assertion
constantEqualText' expected actual = do
let Success expected' = parseNixTextLoc expected
Success actual' = parseNixTextLoc actual
constantEqual expected' actual'
constantEqualText' expected actual =
do
let (Right expected', Right actual') = both parseNixTextLoc (expected, actual)
constantEqual expected' actual'

constantEqualText :: Text -> Text -> Assertion
constantEqualText expected actual = do
Expand All @@ -479,23 +480,30 @@ constantEqualText expected actual = do
assertEvalMatchesNix actual

assertNixEvalThrows :: Text -> Assertion
assertNixEvalThrows a = do
let Success a' = parseNixTextLoc a
time <- getCurrentTime
let opts = defaultOptions time
errored <- catch
(False <$ runWithBasicEffectsIO opts
(normalForm =<< nixEvalExprLoc mempty a'))
(\(_ :: NixException) -> pure True)
unless errored $
assertFailure "Did not catch nix exception"
assertNixEvalThrows a =
do
time <- getCurrentTime
let
opts = defaultOptions time
Right a' = parseNixTextLoc a
errored <-
catch
(False <$
runWithBasicEffectsIO
opts
(normalForm =<< nixEvalExprLoc mempty a')
)
(\(_ :: NixException) -> pure True)
unless errored $ assertFailure "Did not catch nix exception"

freeVarsEqual :: Text -> [VarName] -> Assertion
freeVarsEqual a xs = do
let Success a' = parseNixText a
freeVarsEqual a xs =
do
let
Right a' = parseNixText a
xs' = S.fromList xs
free = freeVars a'
assertEqual "" xs' free
free' = freeVars a'
assertEqual "" xs' free'

maskedFiles :: [FilePath]
maskedFiles =
Expand Down
11 changes: 7 additions & 4 deletions tests/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,13 @@ ensureNixpkgsCanParse =
getString k m =
let Fix (NStr (DoubleQuoted [Plain str])) = getExpr k m in str

consider path action k = action >>= \case
Failure err -> errorWithoutStackTrace $
"Parsing " <> path <> " failed: " <> show err
Success expr -> k expr
consider path action k =
do
x <- action
either
(\ err -> errorWithoutStackTrace $ "Parsing " <> path <> " failed: " <> show err)
k
x

main :: IO ()
main = do
Expand Down
34 changes: 20 additions & 14 deletions tests/NixLanguageTests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -104,26 +104,32 @@ genTests = do
_ -> error $ "Unexpected: " <> show kind

assertParse :: Options -> FilePath -> Assertion
assertParse _opts file = parseNixFileLoc file >>= \case
Success _expr -> pure () -- pure $! runST $ void $ lint opts expr
Failure err ->
assertFailure $ "Failed to parse " <> file <> ":\n" <> show err
assertParse _opts file =
do
x <- parseNixFileLoc file
either
(\ err -> assertFailure $ "Failed to parse " <> file <> ":\n" <> show err)
(const $ pure ()) -- pure $! runST $ void $ lint opts expr
x

assertParseFail :: Options -> FilePath -> Assertion
assertParseFail opts file = do
eres <- parseNixFileLoc file
catch
(case eres of
Success expr -> do
_ <- pure $! runST $ void $ lint opts expr
assertFailure
$ "Unexpected success parsing `"
<> file
<> ":\nParsed value: "
<> show expr
Failure _ -> pure ()
(either
(const $ pure ())
(\ expr ->
do
_ <- pure $! runST $ void $ lint opts expr
assertFailure $
"Unexpected success parsing `"
<> file
<> ":\nParsed value: "
<> show expr
)
eres
)
$ \(_ :: SomeException) -> pure ()
$ \(_ :: SomeException) -> pure ()

assertLangOk :: Options -> FilePath -> Assertion
assertLangOk opts file = do
Expand Down
Loading

0 comments on commit 8ab92d1

Please sign in to comment.