Merge pull request #82 from serokell/verify

Implement verify flag to check that the output parses and formats the same as the input

Author: Lucus16

main/Main.hs

@@ -9,7 +9,6 @@
 module Main where
 
 import Control.Concurrent (Chan, forkIO, newChan, readChan, writeChan)
-import Data.Bifunctor (first)
 import Data.Either (lefts)
 import Data.Text (Text)
 import Data.Version (showVersion)
@@ -24,7 +23,7 @@ import System.Posix.Signals (Handler(..), installHandler, keyboardSignal)
 
 import qualified Data.Text.IO as TextIO (getContents, hPutStr, putStr)
 
-import Nixfmt
+import qualified Nixfmt
 import System.IO.Atomic (withOutputFile)
 import System.IO.Utf8 (readFileUtf8, withUtf8StdHandles)
 
@@ -36,6 +35,7 @@ data Nixfmt = Nixfmt
     , width :: Width
     , check :: Bool
     , quiet :: Bool
+    , verify :: Bool
     } deriving (Show, Data, Typeable)
 
 options :: Nixfmt
@@ -44,30 +44,28 @@ options = Nixfmt
     , width = 80 &= help "Maximum width in characters"
     , check = False &= help "Check whether files are formatted"
     , quiet = False &= help "Do not report errors"
+    , verify = False &= help "Check that the output parses and formats the same as the input"
     } &= summary ("nixfmt v" ++ showVersion version)
     &= help "Format Nix source code"
 
-format' :: Width -> FilePath -> Text -> Either String Text
-format' w path = first errorBundlePretty . format w path
-
 data Target = Target
     { tDoRead :: IO Text
     , tPath :: FilePath
     , tDoWrite :: Text -> IO ()
     }
 
-formatTarget :: Width -> Target -> IO Result
-formatTarget w Target{tDoRead, tPath, tDoWrite} = do
+formatTarget :: Formatter -> Target -> IO Result
+formatTarget format Target{tDoRead, tPath, tDoWrite} = do
     contents <- tDoRead
-    let formatted = format' w tPath contents
+    let formatted = format tPath contents
     mapM tDoWrite formatted
 
 -- | Return an error if target could not be parsed or was not formatted
 -- correctly.
-checkTarget :: Width -> Target -> IO Result
-checkTarget w Target{tDoRead, tPath} = do
+checkTarget :: Formatter -> Target -> IO Result
+checkTarget format Target{tDoRead, tPath} = do
     contents <- tDoRead
-    return $ case format' w tPath contents of
+    return $ case format tPath contents of
         Left err -> Left err
         Right formatted
             | formatted == contents -> Right ()
@@ -87,16 +85,24 @@ toTargets :: Nixfmt -> [Target]
 toTargets Nixfmt{ files = [] }    = [stdioTarget]
 toTargets Nixfmt{ files = paths } = map fileTarget paths
 
-toOperation :: Nixfmt -> Target -> IO Result
-toOperation Nixfmt{ width = w, check = True } = checkTarget w
-toOperation Nixfmt{ width = w } = formatTarget w
+type Formatter = FilePath -> Text -> Either String Text
+
+toFormatter :: Nixfmt -> Formatter
+toFormatter Nixfmt{ width, verify = True  } = Nixfmt.formatVerify width
+toFormatter Nixfmt{ width, verify = False } = Nixfmt.format width
+
+type Operation = Formatter -> Target -> IO Result
+
+toOperation :: Nixfmt -> Operation
+toOperation Nixfmt{ check = True } = checkTarget
+toOperation Nixfmt{ } = formatTarget
 
 toWriteError :: Nixfmt -> String -> IO ()
 toWriteError Nixfmt{ quiet = False } = hPutStrLn stderr
 toWriteError Nixfmt{ quiet = True } = const $ return ()
 
 toJobs :: Nixfmt -> [IO Result]
-toJobs opts = map (toOperation opts) $ toTargets opts
+toJobs opts = map (toOperation opts $ toFormatter opts) $ toTargets opts
 
 -- TODO: Efficient parallel implementation. This is just a sequential stub.
 -- This was originally implemented using parallel-io, but it gave a factor two

src/Nixfmt.hs

@@ -8,19 +8,40 @@ module Nixfmt
     ( errorBundlePretty
     , ParseErrorBundle
     , format
+    , formatVerify
     ) where
 
+import Data.Bifunctor (bimap, first)
 import Data.Text (Text)
-import Text.Megaparsec (parse)
+import qualified Text.Megaparsec as Megaparsec (parse)
 import Text.Megaparsec.Error (errorBundlePretty)
 
 import Nixfmt.Parser (file)
 import Nixfmt.Predoc (layout)
 import Nixfmt.Pretty ()
 import Nixfmt.Types (ParseErrorBundle)
 
+type Width = Int
+
 -- | @format w filename source@ returns either a parsing error specifying a
 -- failure in @filename@ or a formatted version of @source@ with a maximum width
 -- of @w@ columns where possible.
-format :: Int -> FilePath -> Text -> Either ParseErrorBundle Text
-format width filename = fmap (layout width) . parse file filename
+format :: Width -> FilePath -> Text -> Either String Text
+format width filename
+    = bimap errorBundlePretty (layout width)
+    . Megaparsec.parse file filename
+
+formatVerify :: Width -> FilePath -> Text -> Either String Text
+formatVerify width path unformatted = do
+    unformattedParsed <- parse unformatted
+    let formattedOnce = layout width unformattedParsed
+    formattedOnceParsed <- parse formattedOnce
+    let formattedTwice = layout width formattedOnceParsed
+    if formattedOnceParsed /= unformattedParsed
+    then pleaseReport "Parses differently after formatting."
+    else if formattedOnce /= formattedTwice
+    then pleaseReport "Nixfmt is not idempotent."
+    else Right formattedOnce
+    where
+        parse = first errorBundlePretty . Megaparsec.parse file path
+        pleaseReport x = Left $ path <> ": " <> x <> " This is a bug in nixfmt. Please report it at https://github.com/serokell/nixfmt"

src/Nixfmt/Types.hs

@@ -24,39 +24,44 @@ data Trivium
     = EmptyLine
     | LineComment     Text
     | BlockComment    [Text]
-    deriving (Show)
+    deriving (Eq, Show)
 
 type Trivia = [Trivium]
 
-newtype TrailingComment = TrailingComment Text deriving (Show)
+newtype TrailingComment = TrailingComment Text deriving (Eq, Show)
 
 data Ann a
     = Ann a (Maybe TrailingComment) Trivia
     deriving (Show)
 
+-- | Equality of annotated syntax is defines as equality of their corresponding
+-- semantics, thus ignoring the annotations.
+instance Eq a => Eq (Ann a) where
+    Ann x _ _ == Ann y _ _ = x == y
+
 type Leaf = Ann Token
 
 data StringPart
     = TextPart Text
     | Interpolation Leaf Expression Token
-    deriving (Show)
+    deriving (Eq, Show)
 
 type String = Ann [[StringPart]]
 
 data SimpleSelector
     = IDSelector Leaf
     | InterpolSelector (Ann StringPart)
     | StringSelector String
-    deriving (Show)
+    deriving (Eq, Show)
 
 data Selector
     = Selector (Maybe Leaf) SimpleSelector (Maybe (Leaf, Term))
-    deriving (Show)
+    deriving (Eq, Show)
 
 data Binder
     = Inherit Leaf (Maybe Term) [Leaf] Leaf
     | Assignment [Selector] Leaf Expression Leaf
-    deriving (Show)
+    deriving (Eq, Show)
 
 data Term
     = Token Leaf
@@ -65,18 +70,18 @@ data Term
     | Set (Maybe Leaf) Leaf [Binder] Leaf
     | Selection Term [Selector]
     | Parenthesized Leaf Expression Leaf
-    deriving (Show)
+    deriving (Eq, Show)
 
 data ParamAttr
     = ParamAttr Leaf (Maybe (Leaf, Expression)) (Maybe Leaf)
     | ParamEllipsis Leaf
-    deriving (Show)
+    deriving (Eq, Show)
 
 data Parameter
     = IDParameter Leaf
     | SetParameter Leaf [ParamAttr] Leaf
     | ContextParameter Parameter Leaf Parameter
-    deriving (Show)
+    deriving (Eq, Show)
 
 data Expression
     = Term Term
@@ -91,11 +96,11 @@ data Expression
     | MemberCheck Expression Leaf [Selector]
     | Negation Leaf Expression
     | Inversion Leaf Expression
-    deriving (Show)
+    deriving (Eq, Show)
 
 data File
     = File Leaf Expression
-    deriving (Show)
+    deriving (Eq, Show)
 
 data Token
     = Integer    Int
@@ -165,12 +170,12 @@ data Fixity
     | InfixN
     | InfixR
     | Postfix
-    deriving (Show)
+    deriving (Eq, Show)
 
 data Operator
     = Op Fixity Token
     | Apply
-    deriving (Show)
+    deriving (Eq, Show)
 
 -- | A list of lists of operators where lists that come first contain operators
 -- that bind more strongly.