diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dfbc5ef5f..bd3d09b18 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -54,6 +54,9 @@ spago build # Can of course run the tests with spago test + +# To see tests' stdout/stderr output while the tests are running, run +SPAGO_TEST_DEBUG=1 spago ``` ## Developing docs diff --git a/README.md b/README.md index 7da36ef63..cc74530a4 100644 --- a/README.md +++ b/README.md @@ -1187,9 +1187,6 @@ workspace: build_opts: # Directory for the compiler products - optional, defaults to `output`. output: "output" - # Fail the build if `spago.yml` has redundant/underspecified packages. - # Optional boolean that defaults to `false`. - pedantic_packages: false # Specify whether to censor warnings coming from the compiler # for files in the `.spago` directory`. # Optional and can be one of two possible values @@ -1242,6 +1239,10 @@ package: # Optional section to further customise the build for this package. build: + # Fail the build if this package's `dependencies` field has redundant/underspecified packages. + # Optional boolean that defaults to `false`. + pedantic_packages: false + # Specify whether to censor warnings coming from the compiler # for files from this package. # Optional and can be one of two possible values @@ -1305,6 +1306,11 @@ package: execArgs: - "--cli-arg" - "foo" + + # Fail the build if this package's test's `dependencies` field has redundant/underspecified packages. + # Optional boolean that defaults to `false`. + pedantic_packages: false + # Specify whether to censor warnings coming from the compiler # for files from this package's test code. # Optional and can be one of two possible values diff --git a/bin/src/Main.purs b/bin/src/Main.purs index 9532a1f7e..51112bd58 100644 --- a/bin/src/Main.purs +++ b/bin/src/Main.purs @@ -656,7 +656,6 @@ mkBundleEnv bundleArgs = do newWorkspace = workspace { buildOptions { output = bundleArgs.output <|> workspace.buildOptions.output - , pedanticPackages = bundleArgs.pedanticPackages || workspace.buildOptions.pedanticPackages } } esbuild <- Esbuild.getEsbuild @@ -767,7 +766,6 @@ mkBuildEnv buildArgs dependencies = do newWorkspace = workspace { buildOptions { output = buildArgs.output <|> workspace.buildOptions.output - , pedanticPackages = buildArgs.pedanticPackages || workspace.buildOptions.pedanticPackages } -- Override the backend args from the config if they are passed in through a flag , backend = map @@ -788,6 +786,7 @@ mkBuildEnv buildArgs dependencies = do { statVerbosity: buildArgs.statVerbosity , strict: buildArgs.strict } + , pedanticPackages: buildArgs.pedanticPackages } mkPublishEnv :: forall a. Fetch.PackageTransitiveDeps -> Spago (Fetch.FetchEnv a) (Publish.PublishEnv a) diff --git a/core/src/Config.purs b/core/src/Config.purs index bfe747681..1e5303336 100644 --- a/core/src/Config.purs +++ b/core/src/Config.purs @@ -126,6 +126,7 @@ type TestConfig = , dependencies :: Dependencies , censor_test_warnings :: Maybe CensorBuildWarnings , strict :: Maybe Boolean + , pedantic_packages :: Maybe Boolean } testConfigCodec :: JsonCodec TestConfig @@ -135,6 +136,7 @@ testConfigCodec = CAR.object "TestConfig" , dependencies: dependenciesCodec , censor_test_warnings: CAR.optional censorBuildWarningsCodec , strict: CAR.optional CA.boolean + , pedantic_packages: CAR.optional CA.boolean } type BackendConfig = @@ -151,12 +153,14 @@ backendConfigCodec = CAR.object "BackendConfig" type PackageBuildOptionsInput = { censor_project_warnings :: Maybe CensorBuildWarnings , strict :: Maybe Boolean + , pedantic_packages :: Maybe Boolean } packageBuildOptionsCodec :: JsonCodec PackageBuildOptionsInput packageBuildOptionsCodec = CAR.object "PackageBuildOptionsInput" { censor_project_warnings: CAR.optional censorBuildWarningsCodec , strict: CAR.optional CA.boolean + , pedantic_packages: CAR.optional CA.boolean } type BundleConfig = @@ -298,7 +302,6 @@ workspaceConfigCodec = CAR.object "WorkspaceConfig" type WorkspaceBuildOptionsInput = { output :: Maybe FilePath - , pedantic_packages :: Maybe Boolean , censor_library_warnings :: Maybe CensorBuildWarnings , stat_verbosity :: Maybe StatVerbosity } @@ -306,7 +309,6 @@ type WorkspaceBuildOptionsInput = buildOptionsCodec :: JsonCodec WorkspaceBuildOptionsInput buildOptionsCodec = CAR.object "WorkspaceBuildOptionsInput" { output: CAR.optional CA.string - , pedantic_packages: CAR.optional CA.boolean , censor_library_warnings: CAR.optional censorBuildWarningsCodec , stat_verbosity: CAR.optional statVerbosityCodec } diff --git a/src/Spago/Command/Build.purs b/src/Spago/Command/Build.purs index 737461f77..6f9d5dcca 100644 --- a/src/Spago/Command/Build.purs +++ b/src/Spago/Command/Build.purs @@ -6,7 +6,9 @@ module Spago.Command.Build import Spago.Prelude +import Control.Monad.ST as ST import Data.Array as Array +import Data.Array.ST as STA import Data.Map as Map import Data.Set as Set import Data.Traversable (sequence) @@ -30,6 +32,7 @@ type BuildEnv a = , logOptions :: LogOptions , workspace :: Workspace , psaCliFlags :: PsaTypes.PsaOutputOptions + , pedanticPackages :: Boolean | a } @@ -46,6 +49,7 @@ run opts = do , workspace , logOptions , psaCliFlags + , pedanticPackages } <- ask BuildInfo.writeBuildInfo @@ -135,23 +139,27 @@ run opts = do Right _r -> logSuccess "Backend build succeeded." - when workspace.buildOptions.pedanticPackages do + let + pedanticPkgs = ST.run do + arr <- STA.new + ST.foreach selectedPackages \p -> do + let reportSrc = pedanticPackages || (fromMaybe false $ p.package.build >>= _.pedantic_packages) + let reportTest = pedanticPackages || (fromMaybe false $ p.package.test >>= _.pedantic_packages) + when (reportSrc || reportTest) do + void $ STA.push (Tuple p { reportSrc, reportTest }) arr + STA.unsafeFreeze arr + unless (Array.null pedanticPkgs) do logInfo $ "Looking for unused and undeclared transitive dependencies..." - maybeGraph <- Graph.runGraph globs opts.pursArgs - for_ maybeGraph \graph -> do - env <- ask - case workspace.selected of - Just selected -> do - errors <- Graph.toImportErrors selected <$> runSpago (Record.union { graph, selected } env) Graph.checkImports - unless (Array.null errors) do - die' errors - Nothing -> do - -- TODO: here we could go through all the workspace packages and run the check for each - -- The complication is that "dependencies" includes all the dependencies for all packages - errors <- map Array.fold $ for (Config.getWorkspacePackages workspace.packageSet) \selected -> do - Graph.toImportErrors selected <$> runSpago (Record.union { graph, selected } env) Graph.checkImports - unless (Array.null errors) do - die' errors + eitherGraph <- Graph.runGraph globs opts.pursArgs + graph <- either die pure eitherGraph + env <- ask + -- TODO: when the entire workspace is built, + -- here we could go through all the workspace packages and run the check for each + -- The complication is that "dependencies" includes all the dependencies for all packages + errors <- map Array.fold $ for pedanticPkgs \(Tuple selected options) -> do + Graph.toImportErrors selected options <$> runSpago (Record.union { graph, selected } env) Graph.checkImports + unless (Array.null errors) do + die' errors -- TODO: if we are building with all the packages (i.e. selected = Nothing), -- then we can use the graph to remove outdated modules from `output`! diff --git a/src/Spago/Command/Init.purs b/src/Spago/Command/Init.purs index f9b6204c0..9d6114b76 100644 --- a/src/Spago/Command/Init.purs +++ b/src/Spago/Command/Init.purs @@ -100,7 +100,7 @@ defaultConfig { name, withWorkspace, testModuleName } = do pkg = { name , dependencies: [ "effect", "console", "prelude" ] - , test: Just { moduleMain: testModuleName, strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: testModuleName, strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } defaultConfig' case withWorkspace of @@ -110,8 +110,20 @@ defaultConfig { name, withWorkspace, testModuleName } = do type DefaultConfigPackageOptions = { name :: PackageName , dependencies :: Array String - , test :: Maybe { moduleMain :: String, strict :: Maybe Boolean, censorTestWarnings :: Maybe Config.CensorBuildWarnings } - , build :: Maybe { strict :: Maybe Boolean, censorProjectWarnings :: Maybe Config.CensorBuildWarnings } + , test :: + Maybe + { moduleMain :: String + , strict :: Maybe Boolean + , censorTestWarnings :: Maybe Config.CensorBuildWarnings + , pedanticPackages :: Maybe Boolean + , dependencies :: Maybe Config.Dependencies + } + , build :: + Maybe + { strict :: Maybe Boolean + , censorProjectWarnings :: Maybe Config.CensorBuildWarnings + , pedanticPackages :: Maybe Boolean + } } type DefaultConfigWorkspaceOptions = @@ -141,17 +153,19 @@ defaultConfig' opts = { name , dependencies: Dependencies $ Map.fromFoldable $ map mkDep dependencies , description: Nothing - , build: build <#> \{ censorProjectWarnings, strict } -> + , build: build <#> \{ censorProjectWarnings, strict, pedanticPackages } -> { censor_project_warnings: censorProjectWarnings , strict + , pedantic_packages: pedanticPackages } , run: Nothing - , test: test <#> \{ moduleMain, censorTestWarnings, strict } -> - { dependencies: Dependencies Map.empty + , test: test <#> \{ moduleMain, censorTestWarnings, strict, pedanticPackages, dependencies: testDeps } -> + { dependencies: fromMaybe (Dependencies Map.empty) testDeps , execArgs: Nothing , main: moduleMain , censor_test_warnings: censorTestWarnings , strict + , pedantic_packages: pedanticPackages } , publish: Nothing , bundle: Nothing diff --git a/src/Spago/Command/Publish.purs b/src/Spago/Command/Publish.purs index 1155769dd..e67823de8 100644 --- a/src/Spago/Command/Publish.purs +++ b/src/Spago/Command/Publish.purs @@ -114,6 +114,7 @@ publish _args = do { statVerbosity: (Nothing :: Maybe Core.StatVerbosity) , strict: (Nothing :: Maybe Boolean) } + , pedanticPackages: false } ( Build.run { depsOnly: false @@ -125,10 +126,13 @@ publish _args = do -- We then need to check that the dependency graph is accurate. If not, queue the errors let allDependencies = Fetch.toAllDependencies dependencies let globs = Build.getBuildGlobs { selected: [ selected ], withTests: false, dependencies: allDependencies, depsOnly: false } - maybeGraph <- Graph.runGraph globs [] - for_ maybeGraph \graph -> do - graphCheckErrors <- Graph.toImportErrors selected <$> runSpago (Record.union { graph, selected } env) Graph.checkImports - for_ graphCheckErrors addError + eitherGraph <- Graph.runGraph globs [] + case eitherGraph of + Right graph -> do + graphCheckErrors <- Graph.toImportErrors selected { reportSrc: true, reportTest: false } <$> runSpago (Record.union { graph, selected } env) Graph.checkImports + for_ graphCheckErrors addError + Left err -> + die err -- Check if all the packages have ranges, error if not let @@ -309,6 +313,7 @@ publish _args = do { statVerbosity: (Nothing :: Maybe Core.StatVerbosity) , strict: (Nothing :: Maybe Boolean) } + , pedanticPackages: false } ( Build.run { depsOnly: false diff --git a/src/Spago/Config.purs b/src/Spago/Config.purs index 109cd0afa..0837f5dea 100644 --- a/src/Spago/Config.purs +++ b/src/Spago/Config.purs @@ -70,7 +70,6 @@ type Workspace = type BuildOptions = { output :: Maybe FilePath - , pedanticPackages :: Boolean , censorLibWarnings :: Maybe Core.CensorBuildWarnings , statVerbosity :: Maybe Core.StatVerbosity } @@ -381,7 +380,6 @@ readWorkspace maybeSelectedPackage = do buildOptions :: BuildOptions buildOptions = { output: _.output =<< workspace.build_opts - , pedanticPackages: fromMaybe false (_.pedantic_packages =<< workspace.build_opts) , censorLibWarnings: _.censor_library_warnings =<< workspace.build_opts , statVerbosity: _.stat_verbosity =<< workspace.build_opts } diff --git a/src/Spago/Purs/Graph.purs b/src/Spago/Purs/Graph.purs index 9134ed09b..0307d8aa5 100644 --- a/src/Spago/Purs/Graph.purs +++ b/src/Spago/Purs/Graph.purs @@ -162,12 +162,19 @@ checkImports = do pure { unused, transitive, unusedTest, transitiveTest } -toImportErrors :: WorkspacePackage -> ImportCheckResult -> Array Docc -toImportErrors selected { unused, unusedTest, transitive, transitiveTest } = - (if Set.isEmpty unused then [] else [ unusedError false selected unused ]) - <> (if Map.isEmpty transitive then [] else [ transitiveError false selected transitive ]) - <> (if Set.isEmpty unusedTest then [] else [ unusedError true selected unusedTest ]) - <> (if Map.isEmpty transitiveTest then [] else [ transitiveError true selected transitiveTest ]) +toImportErrors + :: WorkspacePackage + -> { reportSrc :: Boolean + , reportTest :: Boolean + } + -> ImportCheckResult + -> Array Docc +toImportErrors selected opts { unused, unusedTest, transitive, transitiveTest } = join + [ if opts.reportSrc && (not $ Set.isEmpty unused) then [ unusedError false selected unused ] else [] + , if opts.reportSrc && (not $ Map.isEmpty transitive) then [ transitiveError false selected transitive ] else [] + , if opts.reportTest && (not $ Set.isEmpty unusedTest) then [ unusedError true selected unusedTest ] else [] + , if opts.reportTest && (not $ Map.isEmpty transitiveTest) then [ transitiveError true selected transitiveTest ] else [] + ] compileGlob :: forall a. FilePath -> Spago (LogEnv a) (Array FilePath) compileGlob sourcePath = do @@ -176,15 +183,10 @@ compileGlob sourcePath = do logDebug [ toDoc "Encountered some globs that are not in cwd, proceeding anyways:", indent $ toDoc failed ] pure (succeeded <> failed) -runGraph :: forall a. Set FilePath -> Array String -> Spago (PreGraphEnv a) (Maybe Purs.ModuleGraph) -runGraph globs pursArgs = do - maybeGraph <- Purs.graph globs pursArgs - case maybeGraph of - Left err -> do - logWarn $ "Could not decode the output of `purs graph`, error: " <> CA.printJsonDecodeError err - pure Nothing - Right graph -> - pure $ Just graph +runGraph :: forall a. Set FilePath -> Array String -> Spago (PreGraphEnv a) (Either String Purs.ModuleGraph) +runGraph globs pursArgs = map (lmap toErrorMessage) $ Purs.graph globs pursArgs + where + toErrorMessage = append "Could not decode the output of `purs graph`, error: " <<< CA.printJsonDecodeError unusedError :: Boolean -> WorkspacePackage -> Set PackageName -> Docc unusedError isTest selected unused = toDoc diff --git a/test-fixtures/check-unused-test-dependency.txt b/test-fixtures/check-unused-test-dependency.txt new file mode 100644 index 000000000..706b06844 --- /dev/null +++ b/test-fixtures/check-unused-test-dependency.txt @@ -0,0 +1,17 @@ +Reading Spago workspace configuration... +Read the package set from the registry + +✅ Selecting package to build: 7368613235362d2f444a2b4f56375435646a59726b53586548 + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✅ Build succeeded. + +Looking for unused and undeclared transitive dependencies... + +❌ Tests for package '7368613235362d2f444a2b4f56375435646a59726b53586548' declares unused dependencies - please remove them from the project config: + - newtype diff --git a/test/Prelude.purs b/test/Prelude.purs index 5ea021fe2..8dac87f14 100644 --- a/test/Prelude.purs +++ b/test/Prelude.purs @@ -106,7 +106,9 @@ check checkers execResult = do Left err -> err.stderr Right res -> res.stderr - when false do + printStdoutStderr <- liftEffect $ map isJust $ Process.lookupEnv "SPAGO_TEST_DEBUG" + + when printStdoutStderr do log $ "STDOUT:\n" <> prettyPrint stdout log $ "STDERR:\n" <> prettyPrint stderr execResult `Assert.shouldSatisfy` checkers.result diff --git a/test/Spago/Build.purs b/test/Spago/Build.purs index 23c89070f..d37c7b0de 100644 --- a/test/Spago/Build.purs +++ b/test/Spago/Build.purs @@ -2,10 +2,12 @@ module Test.Spago.Build where import Test.Prelude +import Data.Map as Map import Node.FS.Aff as FSA import Node.Path as Path import Node.Process as Process import Spago.Command.Init as Init +import Spago.Core.Config (configCodec) import Spago.Core.Config as Config import Spago.FS as FS import Test.Spago.Build.Polyrepo as BuildPolyrepo @@ -77,18 +79,86 @@ spec = Spec.around withTempDir do FS.exists "output" `Assert.shouldReturn` true FS.exists (Path.concat [ "subpackage", "output" ]) `Assert.shouldReturn` false - Spec.it "fails when there are imports from transitive dependencies and --pedantic-packages is passed" \{ spago, fixture } -> do - spago [ "init", "--name", "7368613235362d34312f4e59746b7869335477336d33414d72" ] >>= shouldBeSuccess - spago [ "install", "maybe" ] >>= shouldBeSuccess - FS.writeTextFile (Path.concat [ "src", "Main.purs" ]) "module Main where\nimport Prelude\nimport Data.Maybe\nimport Control.Alt\nmain = unit" - spago [ "build" ] >>= shouldBeSuccess - spago [ "build", "--pedantic-packages" ] >>= shouldBeFailureErr (fixture "check-direct-import-transitive-dependency.txt") + Spec.describe "pedantic packages" do - Spec.it "--pedantic-packages also warns about unused dependencies" \{ spago, fixture } -> do - spago [ "init", "--name", "7368613235362d2f444a2b4f56375435646a59726b53586548" ] >>= shouldBeSuccess - FS.writeTextFile (Path.concat [ "src", "Main.purs" ]) "module Main where\nimport Prelude\nmain = unit" - spago [ "build" ] >>= shouldBeSuccess - spago [ "build", "--pedantic-packages" ] >>= shouldBeFailureErr (fixture "check-unused-dependency.txt") + let + modifyPackageConfig f = do + content <- FS.readYamlDocFile configCodec "spago.yaml" + case content of + Left err -> + Assert.fail $ "Failed to decode spago.yaml file\n" <> err + Right { yaml: config } -> + FS.writeYamlFile configCodec "spago.yaml" $ f config + + addPedanticPackagesToSrc = modifyPackageConfig \config -> + config + { package = config.package <#> \r -> r + { build = Just + { pedantic_packages: Just true + , strict: Nothing + , censor_project_warnings: Nothing + } + } + } + + Spec.describe "fails when there are imports from transitive dependencies and" do + let + setupSrcTransitiveTests spago = do + spago [ "init", "--name", "7368613235362d34312f4e59746b7869335477336d33414d72" ] >>= shouldBeSuccess + spago [ "install", "maybe" ] >>= shouldBeSuccess + FS.writeTextFile (Path.concat [ "src", "Main.purs" ]) "module Main where\nimport Prelude\nimport Data.Maybe\nimport Control.Alt\nmain = unit" + -- get rid of "Compiling ..." messages and other compiler warnings + spago [ "build" ] >>= shouldBeSuccess + + Spec.it "passed --pedantic-packages CLI flag" \{ spago, fixture } -> do + setupSrcTransitiveTests spago + spago [ "build", "--pedantic-packages" ] >>= shouldBeFailureErr (fixture "check-direct-import-transitive-dependency.txt") + + Spec.it "package config has 'pedantic_packages: true'" \{ spago, fixture } -> do + setupSrcTransitiveTests spago + addPedanticPackagesToSrc + spago [ "build" ] >>= shouldBeFailureErr (fixture "check-direct-import-transitive-dependency.txt") + + Spec.describe "warns about unused dependencies when" do + let + setupSrcUnusedDeps spago = do + spago [ "init", "--name", "7368613235362d2f444a2b4f56375435646a59726b53586548" ] >>= shouldBeSuccess + FS.writeTextFile (Path.concat [ "src", "Main.purs" ]) "module Main where\nimport Prelude\nmain = unit" + -- get rid of "Compiling ..." messages and other compiler warnings + spago [ "build" ] >>= shouldBeSuccess + + Spec.it "passing --pedantic-packages CLI flag" \{ spago, fixture } -> do + setupSrcUnusedDeps spago + spago [ "build", "--pedantic-packages" ] >>= shouldBeFailureErr (fixture "check-unused-dependency.txt") + + Spec.it "package config has 'pedantic_packages: true'" \{ spago, fixture } -> do + setupSrcUnusedDeps spago + addPedanticPackagesToSrc + spago [ "build" ] >>= shouldBeFailureErr (fixture "check-unused-dependency.txt") + + Spec.it "package's test config has 'pedantic_packages: true' and test code has unused dependencies" \{ spago, fixture } -> do + spago [ "init", "--name", "7368613235362d2f444a2b4f56375435646a59726b53586548" ] >>= shouldBeSuccess + FS.writeTextFile (Path.concat [ "test", "Test", "Main.purs" ]) "module Test.Main where\nimport Prelude\nmain = unit" + let + modifyPkgTestConfig pedantic = modifyPackageConfig \config -> + config + { package = config.package <#> \r -> r + { test = Just + { main: "Test.Main" + , pedantic_packages: Just pedantic + , strict: Nothing + , censor_test_warnings: Nothing + , dependencies: Config.Dependencies $ Map.fromFoldable $ map (flip Tuple Nothing <<< mkPackageName) [ "newtype" ] + , execArgs: Nothing + } + } + } + modifyPkgTestConfig false + -- get rid of "Compiling ..." messages and other compiler warnings + spago [ "build" ] >>= shouldBeSuccess + -- now add pedantic + modifyPkgTestConfig true + spago [ "build" ] >>= shouldBeFailureErr (fixture "check-unused-test-dependency.txt") Spec.it "--strict causes build to fail if there are warnings" \{ spago, fixture } -> do spago [ "init" ] >>= shouldBeSuccess diff --git a/test/Spago/Build/Polyrepo.purs b/test/Spago/Build/Polyrepo.purs index 365e2f7d2..77085e9be 100644 --- a/test/Spago/Build/Polyrepo.purs +++ b/test/Spago/Build/Polyrepo.purs @@ -8,6 +8,7 @@ module Test.Spago.Build.Polyrepo where import Test.Prelude import Data.Array as Array +import Data.Map as Map import Data.String (Pattern(..), Replacement(..)) import Data.String as String import Node.Path as Path @@ -109,7 +110,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-one-package-a" , dependencies: [ "console", "effect", "prelude" ] - , test: Just { moduleMain: "Subpackage.A.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.A.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -130,7 +131,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-one-package-b" , dependencies: [ "console", "effect", "prelude" ] - , test: Just { moduleMain: "Subpackage.B.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.B.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -190,7 +191,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-two-package-a" , dependencies: [ "console", "effect", "prelude", "case-two-package-shared" ] - , test: Just { moduleMain: "Subpackage.A.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.A.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -214,7 +215,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-two-package-b" , dependencies: [ "console", "effect", "prelude", "case-two-package-shared" ] - , test: Just { moduleMain: "Subpackage.B.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.B.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -278,7 +279,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-three-package-b" , dependencies: [ "console", "effect", "prelude", "case-three-package-c" ] - , test: Just { moduleMain: "Subpackage.B.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.B.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -302,7 +303,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-three-package-a" , dependencies: [ "console", "effect", "prelude", "case-three-package-b", "case-three-package-c" ] - , test: Just { moduleMain: "Subpackage.A.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.A.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -366,7 +367,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-four-package-b" , dependencies: [ "console", "effect", "prelude" ] - , test: Just { moduleMain: "Subpackage.SameName.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.SameName.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -387,7 +388,7 @@ spec = Spec.describe "polyrepo" do , spagoYaml: Init.defaultConfig' $ PackageOnly { name: mkPackageName "case-four-package-a" , dependencies: [ "console", "effect", "prelude" ] - , test: Just { moduleMain: "Subpackage.SameName.Test.Main", strict: Nothing, censorTestWarnings: Nothing } + , test: Just { moduleMain: "Subpackage.SameName.Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } , build: Nothing } , srcMain: mkMainModuleContent @@ -437,6 +438,7 @@ spec = Spec.describe "polyrepo" do Just $ Config.CensorSpecificWarnings $ pure $ Config.ByCode "UnusedName" else Nothing + , pedanticPackages: Nothing } , test: Just { moduleMain: "Test.Subpackage." <> packageToModuleName packageName @@ -446,6 +448,8 @@ spec = Spec.describe "polyrepo" do Just $ Config.CensorSpecificWarnings $ pure $ Config.ByCode "UnusedName" else Nothing + , pedanticPackages: Nothing + , dependencies: Nothing } } , srcMain: mkMainFile "" @@ -531,3 +535,123 @@ spec = Spec.describe "polyrepo" do unless (String.contains (Pattern msg) stdErr) do Assert.fail $ "STDERR did not contain text:\n" <> msg <> "\n\nStderr was:\n" <> stdErr spago [ "build", "--ensure-ranges" ] >>= check { stdout: mempty, stderr: hasNoRootPackageError, result: isLeft } + + Spec.describe "pedantic packages" do + let + mkSrcModuleName packageName = "Src." <> packageToModuleName packageName + mkTestModuleName packageName = "Test." <> packageToModuleName packageName + + {- + /-- tuples (unused dep by `src`) + newtype (transitive dep) <-- control (direct dep) <--+ + \-- either (unused dep by `test`) + + - src and test both import `Data.Newtype` (from `newtype` package) unnecessarily, + thereby triggering the 'unused transitive dependency' warning + - src imports `tuples` + - test imports `either` because it inherit's `src`'s dependencies implicitly -} + setupPackage packageName { src, test } = void $ setupDir + { packageName: packageName + , spagoYaml: Init.defaultConfig' $ PackageOnly + { name: mkPackageName packageName + , dependencies: [ "prelude", "control", "tuples" ] + , build: Just + { pedanticPackages: Just src + , censorProjectWarnings: Nothing + , strict: Nothing + } + , test: Just + { pedanticPackages: Just test + , censorTestWarnings: Nothing + , strict: Nothing + , moduleMain: mkTestModuleName packageName + , dependencies: Just $ Config.Dependencies $ Map.fromFoldable $ map (flip Tuple Nothing <<< mkPackageName) [ "prelude", "control", "either" ] + } + } + , srcMain: mkModuleContent + { modName: mkSrcModuleName packageName + , imports: + [ "import Control.Alt as Control" + , "import Data.Newtype as Newtype" + ] + , body: + [ "" + , "packageName :: String" + , "packageName = \"package\" <> \"" <> packageName <> "\"" + ] + } + , testMain: mkModuleContent + { modName: mkTestModuleName packageName + , imports: + [ "import Control.Alt as Control" + , "import Data.Newtype as Newtype" + ] + , body: + [ "" + , "packageName :: String" + , "packageName = \"package\" <> \"" <> packageName <> "\"" + ] + } + } + + toMsgPrefix isSrc + | isSrc = "Sources" + | otherwise = "Tests" + + mkUnusedDepErr isSrc package = + Array.intercalate "\n" + [ toMsgPrefix isSrc <> " for package '" <> package <> "' declares unused dependencies - please remove them from the project config:" + , " - " <> (if isSrc then "tuples" else "either") + ] + mkTransitiveDepErr isSrc package = + Array.intercalate "\n" + [ Array.fold + [ toMsgPrefix isSrc + , " for package '" + , package + , "' import the following transitive dependencies - please add them to the project dependencies, or remove the imports:" + ] + , " newtype" + , " from `" <> (if isSrc then mkSrcModuleName else mkTestModuleName) package <> "`, which imports:" + , " Data.Newtype" + ] + Spec.it "when package config has 'pedantic_packages: true', build fails with expected errors" \{ spago } -> do + writeWorkspaceOnlySpagoYamlFile + setupPackage "package-a" { src: true, test: false } + setupPackage "package-b" { src: false, test: true } + setupPackage "package-c" { src: true, test: true } + + let + errs = + [ mkUnusedDepErr true "package-a" + , mkTransitiveDepErr true "package-a" + , mkUnusedDepErr false "package-b" + , mkTransitiveDepErr false "package-b" + , mkUnusedDepErr true "package-c" + , mkTransitiveDepErr true "package-c" + , mkUnusedDepErr false "package-c" + , mkTransitiveDepErr false "package-c" + ] + hasExpectedErrors stdErr = do + let unfoundTexts = Array.filter (\exp -> not $ String.contains (Pattern exp) stdErr) errs + unless (Array.null unfoundTexts) do + Assert.fail $ "STDERR did not contain expected texts:\n" <> (Array.intercalate "\n\n" unfoundTexts) <> "\n\nStderr was:\n" <> stdErr + spago [ "build" ] >>= check { stdout: mempty, stderr: hasExpectedErrors, result: isLeft } + + Spec.it "passing --pedantic-packages overrides package and test configs" \{ spago } -> do + writeWorkspaceOnlySpagoYamlFile + setupPackage "package-a" { src: true, test: false } + setupPackage "package-b" { src: false, test: true } + setupPackage "package-c" { src: true, test: true } + + let + errs = do + pkg <- [ "package-a", "package-b", "package-c" ] + isSrc <- [ true, false ] + fn <- [ mkUnusedDepErr, mkTransitiveDepErr ] + pure $ fn isSrc pkg + hasExpectedErrors stdErr = do + let unfoundTexts = Array.filter (\exp -> not $ String.contains (Pattern exp) stdErr) errs + unless (Array.null unfoundTexts) do + Assert.fail $ "STDERR did not contain expected texts:\n" <> (Array.intercalate "\n\n" unfoundTexts) <> "\n\nStderr was:\n" <> stdErr + spago [ "build", "--pedantic-packages" ] >>= check { stdout: mempty, stderr: hasExpectedErrors, result: isLeft } diff --git a/test/Spago/Test.purs b/test/Spago/Test.purs index cd268f011..ff60d268c 100644 --- a/test/Spago/Test.purs +++ b/test/Spago/Test.purs @@ -98,8 +98,8 @@ spec = Spec.around withTempDir do FS.writeYamlFile Config.configCodec "spago.yaml" $ Init.defaultConfig' $ PackageAndWorkspace { name: mkPackageName "package-a" , dependencies: [ "prelude", "effect", "console" ] - , test: Just { moduleMain: "Test.Main", strict: Nothing, censorTestWarnings: Nothing } - , build: Just { strict: Just true, censorProjectWarnings: Nothing } + , test: Just { moduleMain: "Test.Main", strict: Nothing, censorTestWarnings: Nothing, pedanticPackages: Nothing, dependencies: Nothing } + , build: Just { strict: Just true, censorProjectWarnings: Nothing, pedanticPackages: Nothing } } { setVersion: Just $ unsafeFromRight $ Version.parse "0.0.1" }