Always print source of formatted file unless --check is specified (#2205

)

This PR was already merged in #2173,
but main was subsequently forced pushed as part of the 0.4.0 release and
these changes were erased by mistake.

This PR changes the behaviour of the formatter when run on files that
are already formatted. Previously the source of a file that was already
formatted was not output by the formatter.

After this PR, the formatter always outputs the contents of a formatted
file (when used on a single file, and if the --check option is not
specified).

If the `format: false` pragma is set then the source is echoed verbatim,
without highlighting (because it's not possible to get the highlighting
without the formatting).

This probably helps implementing the formatter in the vscode extension,
see anoma/vscode-juvix#98

app/Commands/Format.hs

@@ -59,12 +59,17 @@ runCommand opts = do
 
 renderModeFromOptions :: FormatTarget -> FormatOptions -> FormattedFileInfo -> FormatRenderMode
 renderModeFromOptions target opts formattedInfo
-  | opts ^. formatInPlace = EditInPlace formattedInfo
+  | opts ^. formatInPlace = whenContentsModified (EditInPlace formattedInfo)
   | opts ^. formatCheck = NoEdit Silent
   | otherwise = case target of
       TargetFile -> NoEdit (ReformattedFile (formattedInfo ^. formattedFileInfoContentsAnsi))
-      TargetDir -> NoEdit (InputPath (formattedInfo ^. formattedFileInfoPath))
+      TargetDir -> whenContentsModified (NoEdit (InputPath (formattedInfo ^. formattedFileInfoPath)))
       TargetStdin -> NoEdit (ReformattedFile (formattedInfo ^. formattedFileInfoContentsAnsi))
+  where
+    whenContentsModified :: FormatRenderMode -> FormatRenderMode
+    whenContentsModified res
+      | formattedInfo ^. formattedFileInfoContentsModified = res
+      | otherwise = NoEdit Silent
 
 renderFormattedOutput :: forall r. Members '[Embed IO, App, Resource, Files] r => FormatTarget -> FormatOptions -> FormattedFileInfo -> Sem r ()
 renderFormattedOutput target opts fInfo = do

app/TopCommand/Options.hs

@@ -134,7 +134,7 @@ parseUtility =
                   ( vsep
                       [ "juvix format is used to format Juvix source files.",
                         "",
-                        "Given an unformatted file, it prints the reformatted source to standard output.",
+                        "Given a file, it prints the reformatted source to standard output.",
                         "Given a project directory it prints a list of unformatted files in the project."
                       ]
                   )

src/Juvix/Formatter.hs

@@ -1,5 +1,6 @@
 module Juvix.Formatter where
 
+import Data.List.NonEmpty qualified as NonEmpty
 import Data.Text qualified as T
 import Juvix.Compiler.Concrete.Language
 import Juvix.Compiler.Concrete.Print (ppOutDefault)
@@ -12,7 +13,8 @@ import Juvix.Prelude.Pretty
 
 data FormattedFileInfo = FormattedFileInfo
   { _formattedFileInfoPath :: Path Abs File,
-    _formattedFileInfoContentsAnsi :: NonEmpty AnsiText
+    _formattedFileInfoContentsAnsi :: NonEmpty AnsiText,
+    _formattedFileInfoContentsModified :: Bool
   }
 
 data ScopeEff m a where
@@ -52,31 +54,35 @@ formattedFileInfoContentsText = to infoToPlainText
 
 -- | Format a single Juvix file.
 --
--- If the file requires formatting then the function returns 'FormatResultNotFormatted'
--- and outputs a FormattedFileInfo containing the formatted contents of the file.
+-- If the file requires formatting then the function returns 'FormatResultNotFormatted'.
 --
--- If the file does not require formatting then the function returns
--- 'FormatResultOK' and nothing is output.
+-- If the file does not require formatting then the function returns 'FormatResultOK'.
+--
+-- The function also outputs a FormattedFileInfo containing the formatted
+-- contents of the file.
 format ::
   forall r.
   Members '[ScopeEff, Files, Output FormattedFileInfo] r =>
   Path Abs File ->
   Sem r FormatResult
 format p = do
   originalContents <- readFile' p
-  formattedContents <- formatPath p
-  formatResultFromContents originalContents formattedContents p
+  runReader originalContents $ do
+    formattedContents <- formatPath p
+    formatResultFromContents formattedContents p
 
 -- | Format a Juvix project.
 --
 -- Format all files in the Juvix project containing the passed directory.
 --
--- If any file requires formatting then the function returns 'FormatResultNotFormatted'
--- This function also outputs a FormattedFileInfo (containing the formatted
--- contents of a file) for every file in the project that requires formatting.
+-- If any file requires formatting then the function returns
+-- 'FormatResultNotFormatted'
 --
 -- If all files in the project are already formatted then the function returns
--- 'FormatResultOK' and nothing is output.
+-- 'FormatResultOK'.
+--
+-- This function also outputs a FormattedFileInfo (containing the formatted
+-- contents of a file) for every processed file.
 --
 -- NB: This function does not traverse into Juvix sub-projects, i.e into
 -- subdirectories that contain a juvix.yaml file.
@@ -99,7 +105,7 @@ formatProject p = do
       res <- combineResults <$> mapM format juvixFiles
       return (res, RecurseFilter (\hasJuvixYaml d -> not hasJuvixYaml && not (isHiddenDirectory d)))
 
-formatPath :: Member ScopeEff r => Path Abs File -> Sem r (Maybe (NonEmpty AnsiText))
+formatPath :: Members [Reader Text, ScopeEff] r => Path Abs File -> Sem r (NonEmpty AnsiText)
 formatPath p = do
   res <- scopeFile p
   formatScoperResult res
@@ -111,32 +117,34 @@ formatStdin ::
 formatStdin = do
   res <- scopeStdin
   let originalContents = fromMaybe "" (res ^. Scoper.resultParserResult . resultEntry . entryPointStdin)
-  formattedContents <- formatScoperResult res
-  formatResultFromContents originalContents formattedContents formatStdinPath
+  runReader originalContents $ do
+    formattedContents <- formatScoperResult res
+    formatResultFromContents formattedContents formatStdinPath
 
 formatResultFromContents ::
   forall r.
-  Members '[Output FormattedFileInfo] r =>
-  Text ->
-  Maybe (NonEmpty AnsiText) ->
+  Members '[Reader Text, Output FormattedFileInfo] r =>
+  NonEmpty AnsiText ->
   Path Abs File ->
   Sem r FormatResult
-formatResultFromContents originalContents mfc filepath =
-  case mfc of
-    Just formattedContents
-      | originalContents /= ansiPlainText formattedContents -> do
-          output
-            ( FormattedFileInfo
-                { _formattedFileInfoPath = filepath,
-                  _formattedFileInfoContentsAnsi = formattedContents
-                }
-            )
-          return FormatResultNotFormatted
-      | otherwise -> return FormatResultOK
-    Nothing ->
-      return FormatResultOK
-
-formatScoperResult :: Scoper.ScoperResult -> Sem r (Maybe (NonEmpty AnsiText))
+formatResultFromContents formattedContents filepath = do
+  originalContents <- ask
+  if
+      | originalContents /= ansiPlainText formattedContents -> mkResult FormatResultNotFormatted
+      | otherwise -> mkResult FormatResultOK
+  where
+    mkResult :: FormatResult -> Sem r FormatResult
+    mkResult res = do
+      output
+        ( FormattedFileInfo
+            { _formattedFileInfoPath = filepath,
+              _formattedFileInfoContentsAnsi = formattedContents,
+              _formattedFileInfoContentsModified = res == FormatResultNotFormatted
+            }
+        )
+      return res
+
+formatScoperResult :: Member (Reader Text) r => Scoper.ScoperResult -> Sem r (NonEmpty AnsiText)
 formatScoperResult res = do
   let cs = res ^. Scoper.comments
       formattedModules = run (runReader cs (mapM formatTopModule (res ^. Scoper.resultModules)))
@@ -145,11 +153,11 @@ formatScoperResult res = do
       case pragmas ^. withLocParam . withSourceValue . pragmasFormat of
         Just PragmaFormat {..}
           | not _pragmaFormat ->
-              return Nothing
+              NonEmpty.singleton . mkAnsiText @Text <$> ask
         _ ->
-          return (Just formattedModules)
+          return formattedModules
     Nothing ->
-      return (Just formattedModules)
+      return formattedModules
   where
     formatTopModule :: Member (Reader Comments) r => Module 'Scoped 'ModuleTop -> Sem r AnsiText
     formatTopModule m = do

tests/smoke/Commands/format.smoke.yaml

@@ -16,7 +16,8 @@ tests:
       - format
       - positive/Format.juvix
     exit-status: 0
-    stdout: ''
+    stdout:
+      contains: "module -- Declaring a top module of name:\nFormat;"
 
   - name: format-unformatted-file
     command:
@@ -33,6 +34,36 @@ tests:
       contains: module Foo;
     exit-status: 0
 
+  - name: format-unformatted-file-pragma-false
+    command:
+      shell:
+        - bash
+      script: |
+        temp=$(mktemp -d)
+        trap 'rm -rf -- "$temp"' EXIT
+        cd $temp
+        touch juvix.yaml
+        echo -e '{-# format: false #-}\nmodule Foo ;' >> Foo.juvix
+        juvix format Foo.juvix
+    stdout:
+      contains: module Foo ;
+    exit-status: 0
+
+  - name: format-unformatted-file-pragma-true
+    command:
+      shell:
+        - bash
+      script: |
+        temp=$(mktemp -d)
+        trap 'rm -rf -- "$temp"' EXIT
+        cd $temp
+        touch juvix.yaml
+        echo -e '{-# format: true #-}\nmodule Foo ;' >> Foo.juvix
+        juvix format Foo.juvix
+    stdout:
+      contains: module Foo;
+    exit-status: 0
+
   - name: format-unformatted-file-check-no-stdout
     command:
       shell: