diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d4ac61b3..60cf729dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ Other improvements: cached locally. - `spago publish` now allows to publish a package with some test (but only test!) dependencies not present in the registry. +- errors and warnings are now explicitly labeled as "ERROR" and "WARNING" in + Spago build output. - always using forward slash as path separator in lockfile, regardless of the platform, so that the lockfile doesn't keep changing when team members run Spago on different platforms. diff --git a/src/Spago/Psa/Printer.purs b/src/Spago/Psa/Printer.purs index 968afc0dd..5536c94c8 100644 --- a/src/Spago/Psa/Printer.purs +++ b/src/Spago/Psa/Printer.purs @@ -5,9 +5,7 @@ -- Copyright © Nathan Faubion -- https://opensource.org/license/mit/ module Spago.Psa.Printer - ( renderWarning - , renderError - , renderStats + ( renderStats , renderVerboseStats , printDefaultOutputToErr , printJsonOutputToOut @@ -50,11 +48,11 @@ printJsonOutputToOut output = do printDefaultOutputToErr :: PsaArgs -> Output -> Effect Unit printDefaultOutputToErr options output = do forWithIndex_ output.warnings \i warning -> do - Console.error $ printDoc (renderWarning lenWarnings (i + 1) warning) + Console.error $ printDoc (renderCitation Warn lenWarnings (i + 1) warning) Console.error "" forWithIndex_ output.errors \i error -> do - Console.error $ printDoc (renderError lenErrors (i + 1) error) + Console.error $ printDoc (renderCitation Err lenErrors (i + 1) error) Console.error "" Console.error $ printDoc (renderStats' output.stats) @@ -70,17 +68,21 @@ printDefaultOutputToErr options output = do Core.CompactStats -> renderStats Core.VerboseStats -> renderVerboseStats -renderWarning :: Int -> Int -> PsaAnnotedError -> D.Doc Ansi.GraphicsParam -renderWarning = renderWrapper Ansi.BrightYellow +data Citation = Warn | Err -renderError :: Int -> Int -> PsaAnnotedError -> D.Doc Ansi.GraphicsParam -renderError = renderWrapper Ansi.BrightRed +citationColor :: Citation -> Ansi.Color +citationColor Warn = Ansi.BrightYellow +citationColor Err = Ansi.BrightRed -renderWrapper :: Ansi.Color -> Int -> Int -> PsaAnnotedError -> D.Doc Ansi.GraphicsParam -renderWrapper color total index { error, path, position, source, message } = +citationLabel :: Citation -> String +citationLabel Warn = "WARNING" +citationLabel Err = "ERROR" + +renderCitation :: Citation -> Int -> Int -> PsaAnnotedError -> D.Doc Ansi.GraphicsParam +renderCitation cit total index { error, path, position, source, message } = D.foldWithSeparator (D.break <> D.break) [ D.words $ - [ renderStatus color total index error.errorCode + [ renderStatus cit total index error.errorCode , renderPath path error.moduleName <> foldMap renderPosition position ] , D.indent $ D.lines @@ -92,9 +94,11 @@ renderWrapper color total index { error, path, position, source, message } = toLines :: String -> D.Doc Ansi.GraphicsParam toLines = D.lines <<< map D.text <<< Str.split (Str.Pattern "\n") -renderStatus :: Ansi.Color -> Int -> Int -> String -> D.Doc Ansi.GraphicsParam -renderStatus color total index code = - DA.foreground color $ D.text $ "[" <> show index <> "/" <> show total <> " " <> code <> "]" +renderStatus :: Citation -> Int -> Int -> String -> D.Doc Ansi.GraphicsParam +renderStatus cit total index code = + DA.foreground (citationColor cit) + $ D.text + $ "[" <> citationLabel cit <> " " <> show index <> "/" <> show total <> " " <> code <> "]" renderModuleName :: Maybe String -> D.Doc Ansi.GraphicsParam renderModuleName Nothing = DA.dim $ D.text "(unknown module)" diff --git a/test-fixtures/build/1148-warnings-diff-errors/errors/expected-stderr.txt b/test-fixtures/build/1148-warnings-diff-errors/errors/expected-stderr.txt new file mode 100644 index 000000000..a3ffb4e6f --- /dev/null +++ b/test-fixtures/build/1148-warnings-diff-errors/errors/expected-stderr.txt @@ -0,0 +1,43 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: foo + +Downloading dependencies... +Building... +[1 of 2] Compiling Foo +[2 of 2] Compiling Main +[ERROR 1/2 TypesDoNotUnify] src/Foo.purs:4:5 + + 4 x = "nope" + ^^^^^^ + + Could not match type + String + with type + Int + while checking that type String + is at least as general as type Int + while checking that expression "nope" + has type Int + in value declaration x + +[ERROR 2/2 TypesDoNotUnify] src/Main.purs:10:7 + + 10 log 42 + ^^ + + Could not match type + Int + with type + String + while checking that type Int + is at least as general as type String + while checking that expression 42 + has type String + in value declaration main + + Src Lib All +Warnings 0 0 0 +Errors 2 0 2 + +✘ Failed to build. diff --git a/test-fixtures/build/1148-warnings-diff-errors/errors/spago.yaml b/test-fixtures/build/1148-warnings-diff-errors/errors/spago.yaml new file mode 100644 index 000000000..164648573 --- /dev/null +++ b/test-fixtures/build/1148-warnings-diff-errors/errors/spago.yaml @@ -0,0 +1,10 @@ +package: + name: foo + dependencies: + - console + - effect + - prelude + +workspace: + packageSet: + registry: 41.5.0 diff --git a/test-fixtures/build/1148-warnings-diff-errors/errors/src/Foo.purs b/test-fixtures/build/1148-warnings-diff-errors/errors/src/Foo.purs new file mode 100644 index 000000000..86224c155 --- /dev/null +++ b/test-fixtures/build/1148-warnings-diff-errors/errors/src/Foo.purs @@ -0,0 +1,4 @@ +module Foo where + +x :: Int +x = "nope" diff --git a/test-fixtures/build/1148-warnings-diff-errors/errors/src/Main.purs b/test-fixtures/build/1148-warnings-diff-errors/errors/src/Main.purs new file mode 100644 index 000000000..5fdeca2f0 --- /dev/null +++ b/test-fixtures/build/1148-warnings-diff-errors/errors/src/Main.purs @@ -0,0 +1,11 @@ +module Main where + +import Prelude + +import Effect +import Effect.Console (log) + +main :: Effect Unit +main = do + log 42 + pure unit diff --git a/test-fixtures/build/1148-warnings-diff-errors/warnings/expected-stderr.txt b/test-fixtures/build/1148-warnings-diff-errors/warnings/expected-stderr.txt new file mode 100644 index 000000000..87f4531b2 --- /dev/null +++ b/test-fixtures/build/1148-warnings-diff-errors/warnings/expected-stderr.txt @@ -0,0 +1,43 @@ +Reading Spago workspace configuration... + +✓ Selecting package to build: foo + +Downloading dependencies... +Building... +[1 of 1] Compiling Main +[WARNING 1/4 ImplicitImport] src/Main.purs:3:1 + + 3 import Prelude + ^^^^^^^^^^^^^^ + + Module Prelude has unspecified imports, consider using the explicit form: + import Prelude (Unit, pure, unit) + +[WARNING 2/4 ImplicitImport] src/Main.purs:5:1 + + 5 import Effect + ^^^^^^^^^^^^^ + + Module Effect has unspecified imports, consider using the explicit form: + import Effect (Effect) + +[WARNING 3/4 UnusedImport] src/Main.purs:6:1 + + 6 import Effect.Console (log) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + The import of Effect.Console is redundant + +[WARNING 4/4 UnusedName] src/Main.purs:10:7 + + 10 let unusedVar = 1 + ^^^^^^^^^^^^^ + + Name unusedVar was introduced but not used. + in value declaration main + + Src Lib All +Warnings 4 0 4 +Errors 0 0 0 + +✓ Build succeeded. diff --git a/test-fixtures/build/1148-warnings-diff-errors/warnings/spago.yaml b/test-fixtures/build/1148-warnings-diff-errors/warnings/spago.yaml new file mode 100644 index 000000000..164648573 --- /dev/null +++ b/test-fixtures/build/1148-warnings-diff-errors/warnings/spago.yaml @@ -0,0 +1,10 @@ +package: + name: foo + dependencies: + - console + - effect + - prelude + +workspace: + packageSet: + registry: 41.5.0 diff --git a/test-fixtures/build/1148-warnings-diff-errors/warnings/src/Main.purs b/test-fixtures/build/1148-warnings-diff-errors/warnings/src/Main.purs new file mode 100644 index 000000000..47a53c88d --- /dev/null +++ b/test-fixtures/build/1148-warnings-diff-errors/warnings/src/Main.purs @@ -0,0 +1,11 @@ +module Main where + +import Prelude + +import Effect +import Effect.Console (log) + +main :: Effect Unit +main = do + let unusedVar = 1 + pure unit diff --git a/test/Prelude.purs b/test/Prelude.purs index eb73192d7..1ab419ae0 100644 --- a/test/Prelude.purs +++ b/test/Prelude.purs @@ -396,5 +396,11 @@ escapePathInErrMsg = case Process.platform of assertWarning :: forall m. MonadThrow Error m => Array String -> Boolean -> String -> m Unit assertWarning paths shouldHave stdErr = do when (not $ Array.all (\exp -> shouldHave == (String.contains (Pattern exp) stdErr)) paths) do - Assert.fail $ "STDERR contained one or more texts:\n" <> show paths <> "\n\nStderr was:\n" <> stdErr + Assert.fail + $ "STDERR " + <> (if shouldHave then "did not contain" else "contained") + <> " one or more texts:\n" + <> show paths + <> "\n\nStderr was:\n" + <> stdErr diff --git a/test/Spago/Build.purs b/test/Spago/Build.purs index da5217d85..8a5f2df4e 100644 --- a/test/Spago/Build.purs +++ b/test/Spago/Build.purs @@ -3,6 +3,7 @@ module Test.Spago.Build where import Test.Prelude import Data.Foldable (fold) +import Data.String as String import Node.FS.Aff as FSA import Node.Path as Path import Node.Platform as Platform @@ -211,6 +212,32 @@ spec = Spec.around withTempDir do spago [ "build" ] >>= shouldBeSuccessErr (fixture "build/migrate-config/migrated-output.txt") checkFixture "spago.yaml" (fixture "build/migrate-config/migrated-spago.yaml") + Spec.it "#1148: outputs errors and warnings after build" \{ spago, fixture } -> do + let + shouldBeSuccessErr' = checkOutputsWithPathSeparatorPatchErr isRight + shouldBeFailureErr' = checkOutputsWithPathSeparatorPatchErr isLeft + + checkOutputsWithPathSeparatorPatchErr result expectedFixture = + checkOutputs' + { stdoutFile: Nothing + , stderrFile: Just $ fixture expectedFixture + , result + , sanitize: + String.trim + >>> String.replaceAll (String.Pattern $ "src\\") (String.Replacement "src/") + >>> String.replaceAll (String.Pattern $ "\r\n") (String.Replacement "\n") + } + + FS.copyTree { src: fixture "build/1148-warnings-diff-errors", dst: "." } + + liftEffect $ Process.chdir "errors" + spago [ "install" ] >>= shouldBeSuccess + spago [ "build" ] >>= shouldBeFailureErr' "build/1148-warnings-diff-errors/errors/expected-stderr.txt" + + liftEffect $ Process.chdir "../warnings" + spago [ "install" ] >>= shouldBeSuccess + spago [ "build" ] >>= shouldBeSuccessErr' "build/1148-warnings-diff-errors/warnings/expected-stderr.txt" + Pedantic.spec Monorepo.spec diff --git a/test/Spago/Build/Monorepo.purs b/test/Spago/Build/Monorepo.purs index 7300f0ecf..e1f9c2fab 100644 --- a/test/Spago/Build/Monorepo.purs +++ b/test/Spago/Build/Monorepo.purs @@ -142,8 +142,8 @@ spec = Spec.describe "monorepo" do FS.copyTree { src: fixture "monorepo/strict-true-uncensored-warnings", dst: "." } let errs = - [ "[1/2 UnusedName] " <> escapePathInErrMsg [ "package-b", "src", "Main.purs:6:13" ] - , "[2/2 UnusedName] " <> escapePathInErrMsg [ "package-b", "test", "Main.purs:6:13" ] + [ "[ERROR 1/2 UnusedName] " <> escapePathInErrMsg [ "package-b", "src", "Main.purs:6:13" ] + , "[ERROR 2/2 UnusedName] " <> escapePathInErrMsg [ "package-b", "test", "Main.purs:6:13" ] ] hasUnusedWarningError = assertWarning errs true spago [ "build" ] >>= check { stdout: mempty, stderr: hasUnusedWarningError, result: isLeft } diff --git a/test/Spago/Test.purs b/test/Spago/Test.purs index 2bff0b29b..37f4ce3f3 100644 --- a/test/Spago/Test.purs +++ b/test/Spago/Test.purs @@ -60,7 +60,7 @@ spec = Spec.around withTempDir do FS.mkdirp "subpackage/test" FS.writeTextFile "subpackage/src/Main.purs" (Init.srcMainTemplate "Subpackage.Main") - -- We write a file into the current working directory. + -- We write a file into the current working directory. -- The subpackage test will read the given file without changing its directory -- and log its content as its output. let textFilePath = "foo.txt" @@ -130,8 +130,8 @@ spec = Spec.around withTempDir do let exp = case Process.platform of - Just Platform.Win32 -> "[1/1 UnusedName] test\\Test\\Main.purs:10:5" - _ -> "[1/1 UnusedName] test/Test/Main.purs:10:5" + Just Platform.Win32 -> "[WARNING 1/1 UnusedName] test\\Test\\Main.purs:10:5" + _ -> "[WARNING 1/1 UnusedName] test/Test/Main.purs:10:5" hasUnusedNameWarningError stdErr = do unless (String.contains (String.Pattern exp) stdErr) do