diff --git a/src/Spago/Purs/Graph.purs b/src/Spago/Purs/Graph.purs index 07243b3f8..0bff6dfeb 100644 --- a/src/Spago/Purs/Graph.purs +++ b/src/Spago/Purs/Graph.purs @@ -183,68 +183,86 @@ checkImports graph = do packageGraph <- runSpago (Record.union { selected: NEA.singleton selected } env) $ getModuleGraphWithPackage graph let - packageName = selected.package.name - testPackageName = unsafeCoerce (PackageName.print packageName <> ":test") - - declaredDependencies = unwrap selected.package.dependencies - declaredTestDependencies = maybe Map.empty (unwrap <<< _.dependencies) selected.package.test - - -- The direct dependencies specified in the config - dependencyPackages = Map.mapMaybe (const (Just Map.empty)) declaredDependencies - dependencyTestPackages = Map.mapMaybe (const (Just Map.empty)) $ Map.union declaredDependencies declaredTestDependencies - - -- Compile the globs for the project, we get the set of source files in the project - projectGlob :: Set FilePath <- map Set.fromFoldable do - map Array.fold $ traverse compileGlob (Config.sourceGlob NoTestGlobs packageName (WorkspacePackage selected)) - - -- Same but for tests - projectTestsGlob :: Set FilePath <- map Set.fromFoldable do - map Array.fold $ traverse compileGlob (Config.sourceGlob OnlyTestGlobs packageName (WorkspacePackage selected)) - - let - -- Filter this improved graph to only have the project modules - projectGraph = Map.filterWithKey (\_ { path } -> Set.member path projectGlob) packageGraph - projectTestsGraph = Map.filterWithKey (\_ { path } -> Set.member path projectTestsGlob) packageGraph - - -- Go through all the modules in the project graph, figure out which packages each module depends on, - -- accumulate all of that in a single place - accumulateImported importedPkgs' (Tuple moduleName { depends }) = - let - accumulateDep importedPkgs importedModule = case Map.lookup importedModule packageGraph of - Nothing -> importedPkgs - -- Skip dependencies on modules in the same package, we are not interested in that - Just { package } | package == packageName -> importedPkgs - Just { package } | package == testPackageName -> importedPkgs - Just { package: importedPackage } -> Map.alter - ( case _ of - Nothing -> Just $ Map.singleton moduleName (Set.singleton importedModule) - Just p -> Just $ Map.alter - ( case _ of - Nothing -> Just $ Set.singleton importedModule - Just set -> Just $ Set.insert importedModule set - ) - moduleName - p - ) - importedPackage - importedPkgs - in - foldl accumulateDep importedPkgs' depends - - importedPackages :: ImportedPackages - importedPackages = foldl accumulateImported Map.empty (Map.toUnfoldable projectGraph :: Array _) - importedTestPackages = foldl accumulateImported Map.empty (Map.toUnfoldable projectTestsGraph :: Array _) - - unused = Map.keys $ Map.difference dependencyPackages importedPackages - transitive = Map.difference importedPackages dependencyPackages - unusedTest = - if Set.isEmpty projectTestsGlob then Set.empty - else Map.keys $ Map.difference (Map.difference dependencyTestPackages dependencyPackages) importedTestPackages - transitiveTest = - if Set.isEmpty projectTestsGlob then Map.empty - else Map.difference importedTestPackages dependencyTestPackages - - pure { unused, transitive, unusedTest, transitiveTest } + dropValues = Map.mapMaybe (const (Just Map.empty)) + srcDeps = unwrap selected.package.dependencies + srcResult <- getUsedUnusedTransitiveFor + { selected + , packageName: selected.package.name + , dependencyPackages: dropValues srcDeps + , isSrc: true + , packageGraph + } + let srcDepsUsed = Map.filterKeys (flip Map.member srcResult.used) srcDeps + testResult <- getUsedUnusedTransitiveFor + { selected + , packageName: selected.package.name + , dependencyPackages: + dropValues + -- add the used source dependencies, not the declared ones. + $ Map.unionWith const srcDepsUsed + -- get test deps + $ maybe Map.empty (unwrap <<< _.dependencies) selected.package.test + , isSrc: false + , packageGraph + } + + pure + { unused: srcResult.unused + , transitive: srcResult.transitive + , unusedTest: Set.difference testResult.unused $ Map.keys srcResult.used + , transitiveTest: differenceAll testResult.transitive [ srcResult.used, srcResult.transitive ] + } + where + differenceAll sourceMap removalsArray = foldl Map.difference sourceMap removalsArray + getUsedUnusedTransitiveFor { selected, packageName, dependencyPackages, isSrc, packageGraph } = do + let + testPackageName = unsafeCoerce (PackageName.print packageName <> ":test") + + testGlobOption + | isSrc = NoTestGlobs + | otherwise = OnlyTestGlobs + + -- Compile the globs for the project, we get the set of source files in the project + glob :: Set FilePath <- map Set.fromFoldable do + map Array.fold $ traverse compileGlob (Config.sourceGlob testGlobOption packageName (WorkspacePackage selected)) + + let + -- Filter this improved graph to only have the project modules + projectGraph = Map.filterWithKey (\_ { path } -> Set.member path glob) packageGraph + + -- Go through all the modules in the project graph, figure out which packages each module depends on, + -- accumulate all of that in a single place + accumulateImported importedPkgs' (Tuple moduleName { depends }) = + let + accumulateDep importedPkgs importedModule = case Map.lookup importedModule packageGraph of + Nothing -> importedPkgs + -- Skip dependencies on modules in the same package, we are not interested in that + Just { package } | package == packageName -> importedPkgs + Just { package } | package == testPackageName -> importedPkgs + Just { package: importedPackage } -> Map.alter + ( case _ of + Nothing -> Just $ Map.singleton moduleName (Set.singleton importedModule) + Just p -> Just $ Map.alter + ( case _ of + Nothing -> Just $ Set.singleton importedModule + Just set -> Just $ Set.insert importedModule set + ) + moduleName + p + ) + importedPackage + importedPkgs + in + foldl accumulateDep importedPkgs' depends + + importedPackages :: ImportedPackages + importedPackages = foldl accumulateImported Map.empty (Map.toUnfoldable projectGraph :: Array _) + + pure + { used: if isSrc then Map.intersection dependencyPackages importedPackages else Map.empty + , unused: Map.keys $ Map.difference dependencyPackages importedPackages + , transitive: Map.difference importedPackages dependencyPackages + } -------------------------------------------------------------------------------- -- Errors diff --git a/test-fixtures/check-direct-import-transitive-dependency.txt b/test-fixtures/check-direct-import-transitive-dependency.txt deleted file mode 100644 index e77ac94da..000000000 --- a/test-fixtures/check-direct-import-transitive-dependency.txt +++ /dev/null @@ -1,28 +0,0 @@ -Reading Spago workspace configuration... -Read the package set from the registry - -✅ Selecting package to build: 7368613235362d34312f4e59746b7869335477336d33414d72 - -Downloading dependencies... -Building... - Src Lib All -Warnings 0 0 0 -Errors 0 0 0 - -✅ Build succeeded. - -Looking for unused and undeclared transitive dependencies... - -❌ Sources for package '7368613235362d34312f4e59746b7869335477336d33414d72' declares unused dependencies - please remove them from the project config: - - console - - effect - - -❌ Sources for package '7368613235362d34312f4e59746b7869335477336d33414d72' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: - control - from `Main`, which imports: - Control.Alt - - -Run the following command to install them all: - spago install -p 7368613235362d34312f4e59746b7869335477336d33414d72 control diff --git a/test-fixtures/pedantic/check-direct-import-transitive-dependency-both.txt b/test-fixtures/pedantic/check-direct-import-transitive-dependency-both.txt new file mode 100644 index 000000000..23001f4f7 --- /dev/null +++ b/test-fixtures/pedantic/check-direct-import-transitive-dependency-both.txt @@ -0,0 +1,33 @@ +Reading Spago workspace configuration... +Read the package set from the registry + +✅ Selecting package to build: pedantic + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✅ Build succeeded. + +Looking for unused and undeclared transitive dependencies... + +❌ Sources for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: + maybe + from `Main`, which imports: + Data.Maybe + + +Run the following command to install them all: + spago install -p pedantic maybe + + +❌ Tests for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: + control + from `Test.Main`, which imports: + Control.Alt + + +Run the following command to install them all: + spago install --test-deps -p pedantic control diff --git a/test-fixtures/pedantic/check-direct-import-transitive-dependency-test.txt b/test-fixtures/pedantic/check-direct-import-transitive-dependency-test.txt new file mode 100644 index 000000000..1c3c96cfd --- /dev/null +++ b/test-fixtures/pedantic/check-direct-import-transitive-dependency-test.txt @@ -0,0 +1,23 @@ +Reading Spago workspace configuration... +Read the package set from the registry + +✅ Selecting package to build: pedantic + +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 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: + control + from `Test.Main`, which imports: + Control.Alt + + +Run the following command to install them all: + spago install --test-deps -p pedantic control diff --git a/test-fixtures/pedantic/check-direct-import-transitive-dependency.txt b/test-fixtures/pedantic/check-direct-import-transitive-dependency.txt new file mode 100644 index 000000000..8131ae6c4 --- /dev/null +++ b/test-fixtures/pedantic/check-direct-import-transitive-dependency.txt @@ -0,0 +1,23 @@ +Reading Spago workspace configuration... +Read the package set from the registry + +✅ Selecting package to build: pedantic + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✅ Build succeeded. + +Looking for unused and undeclared transitive dependencies... + +❌ Sources for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: + control + from `Main`, which imports: + Control.Alt + + +Run the following command to install them all: + spago install -p pedantic control diff --git a/test-fixtures/pedantic/check-pedantic-packages.txt b/test-fixtures/pedantic/check-pedantic-packages.txt new file mode 100644 index 000000000..7af052601 --- /dev/null +++ b/test-fixtures/pedantic/check-pedantic-packages.txt @@ -0,0 +1,43 @@ +Reading Spago workspace configuration... +Read the package set from the registry + +✅ Selecting package to build: pedantic + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✅ Build succeeded. + +Looking for unused and undeclared transitive dependencies... + +❌ Sources for package 'pedantic' declares unused dependencies - please remove them from the project config: + - console + - effect + - either + + +❌ Sources for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: + newtype + from `Main`, which imports: + Data.Newtype + + +Run the following command to install them all: + spago install -p pedantic newtype + + +❌ Tests for package 'pedantic' declares unused dependencies - please remove them from the project config: + - tuples + + +❌ Tests for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: + either + from `Test.Main`, which imports: + Data.Either + + +Run the following command to install them all: + spago install --test-deps -p pedantic either diff --git a/test-fixtures/pedantic/check-unused-dependency-in-source.txt b/test-fixtures/pedantic/check-unused-dependency-in-source.txt new file mode 100644 index 000000000..99365f26d --- /dev/null +++ b/test-fixtures/pedantic/check-unused-dependency-in-source.txt @@ -0,0 +1,31 @@ +Reading Spago workspace configuration... +Read the package set from the registry + +✅ Selecting package to build: pedantic + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✅ Build succeeded. + +Looking for unused and undeclared transitive dependencies... + +❌ Sources for package 'pedantic' declares unused dependencies - please remove them from the project config: + - console + - effect + + +❌ Tests for package 'pedantic' import the following transitive dependencies - please add them to the project dependencies, or remove the imports: + console + from `Test.Main`, which imports: + Effect.Class.Console + effect + from `Test.Main`, which imports: + Effect + + +Run the following command to install them all: + spago install --test-deps -p pedantic console effect diff --git a/test-fixtures/check-unused-dependency.txt b/test-fixtures/pedantic/check-unused-dependency.txt similarity index 56% rename from test-fixtures/check-unused-dependency.txt rename to test-fixtures/pedantic/check-unused-dependency.txt index 042aa88ce..83f7d1b1c 100644 --- a/test-fixtures/check-unused-dependency.txt +++ b/test-fixtures/pedantic/check-unused-dependency.txt @@ -1,7 +1,7 @@ Reading Spago workspace configuration... Read the package set from the registry -✅ Selecting package to build: 7368613235362d2f444a2b4f56375435646a59726b53586548 +✅ Selecting package to build: pedantic Downloading dependencies... Building... @@ -13,6 +13,6 @@ Errors 0 0 0 Looking for unused and undeclared transitive dependencies... -❌ Sources for package '7368613235362d2f444a2b4f56375435646a59726b53586548' declares unused dependencies - please remove them from the project config: +❌ Sources for package 'pedantic' declares unused dependencies - please remove them from the project config: - console - effect diff --git a/test-fixtures/pedantic/check-unused-source-and-test-dependency.txt b/test-fixtures/pedantic/check-unused-source-and-test-dependency.txt new file mode 100644 index 000000000..c762edab8 --- /dev/null +++ b/test-fixtures/pedantic/check-unused-source-and-test-dependency.txt @@ -0,0 +1,23 @@ +Reading Spago workspace configuration... +Read the package set from the registry + +✅ Selecting package to build: pedantic + +Downloading dependencies... +Building... + Src Lib All +Warnings 0 0 0 +Errors 0 0 0 + +✅ Build succeeded. + +Looking for unused and undeclared transitive dependencies... + +❌ Sources for package 'pedantic' declares unused dependencies - please remove them from the project config: + - console + - effect + + +❌ Tests for package 'pedantic' declares unused dependencies - please remove them from the project config: + - console + - effect diff --git a/test-fixtures/check-unused-test-dependency.txt b/test-fixtures/pedantic/check-unused-test-dependency.txt similarity index 56% rename from test-fixtures/check-unused-test-dependency.txt rename to test-fixtures/pedantic/check-unused-test-dependency.txt index 706b06844..136eefc3b 100644 --- a/test-fixtures/check-unused-test-dependency.txt +++ b/test-fixtures/pedantic/check-unused-test-dependency.txt @@ -1,7 +1,7 @@ Reading Spago workspace configuration... Read the package set from the registry -✅ Selecting package to build: 7368613235362d2f444a2b4f56375435646a59726b53586548 +✅ Selecting package to build: pedantic Downloading dependencies... Building... @@ -13,5 +13,5 @@ Errors 0 0 0 Looking for unused and undeclared transitive dependencies... -❌ Tests for package '7368613235362d2f444a2b4f56375435646a59726b53586548' declares unused dependencies - please remove them from the project config: +❌ Tests for package 'pedantic' declares unused dependencies - please remove them from the project config: - newtype diff --git a/test/Prelude.purs b/test/Prelude.purs index 4744523a9..7fde51cc1 100644 --- a/test/Prelude.purs +++ b/test/Prelude.purs @@ -42,7 +42,9 @@ withTempDir = Aff.bracket createTempDir cleanupTempDir temp <- mkTemp' $ Just "spago-test-" FS.mkdirp temp liftEffect $ Process.chdir temp - log $ "Running test in " <> temp + isDebug <- liftEffect $ map isJust $ Process.lookupEnv "SPAGO_TEST_DEBUG" + when isDebug do + log $ "Running test in " <> temp let fixturesPath = oldCwd <> Path.sep <> "test-fixtures" @@ -185,6 +187,18 @@ mkPackageName = unsafeFromRight <<< PackageName.parse mkVersion :: String -> Version mkVersion = unsafeFromRight <<< Version.parse +writeMain :: Array String -> String +writeMain rest = writePursFile { moduleName: "Main", rest } + +writeTestMain :: Array String -> String +writeTestMain rest = writePursFile { moduleName: "Test.Main", rest } + +writePursFile :: { moduleName :: String, rest :: Array String } -> String +writePursFile { moduleName, rest } = + Array.intercalate "\n" $ Array.cons modNameLine $ rest + where + modNameLine = "module " <> moduleName <> " where" + mkDependencies :: Array String -> Config.Dependencies mkDependencies = Config.Dependencies <<< Map.fromFoldable <<< map (flip Tuple Nothing <<< mkPackageName) diff --git a/test/Spago/Build.purs b/test/Spago/Build.purs index 3b9db3271..2bc2b105c 100644 --- a/test/Spago/Build.purs +++ b/test/Spago/Build.purs @@ -6,9 +6,9 @@ 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.Pedantic as Pedantic import Test.Spago.Build.Polyrepo as BuildPolyrepo import Test.Spec (Spec) import Test.Spec as Spec @@ -78,87 +78,6 @@ spec = Spec.around withTempDir do FS.exists "output" `Assert.shouldReturn` true FS.exists (Path.concat [ "subpackage", "output" ]) `Assert.shouldReturn` false - Spec.describe "pedantic packages" do - - 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: mkDependencies [ "newtype" ] - , exec_args: 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 let srcMain = Path.concat [ "src", "Main.purs" ] @@ -217,6 +136,8 @@ spec = Spec.around withTempDir do spagoYaml <- FS.readTextFile "spago.yaml" spagoYaml `shouldContain` "- prelude: \">=6.0.1 <7.0.0\"" + Pedantic.spec + BuildPolyrepo.spec -- Spec.it "runs a --before command" \{ spago } -> do diff --git a/test/Spago/Build/Pedantic.purs b/test/Spago/Build/Pedantic.purs new file mode 100644 index 000000000..a832f84a1 --- /dev/null +++ b/test/Spago/Build/Pedantic.purs @@ -0,0 +1,253 @@ +module Test.Spago.Build.Pedantic (spec) where + +import Test.Prelude + +import Data.Array as Array +import Data.Map as Map +import Node.Path as Path +import Spago.Core.Config (Dependencies(..), Config) +import Spago.Core.Config as Core +import Spago.FS as FS +import Test.Spec (SpecT) +import Test.Spec as Spec +import Test.Spec.Assertions as Assert + +spec :: SpecT Aff TestDirs Identity Unit +spec = + Spec.describe "pedantic packages" do + + Spec.describe "fails when imports from transitive dependencies" do + + -- Here we are importing the `Control.Alt` module, which is in the `control` + -- package, which comes through `maybe` but we are not importing directly + Spec.it "appear in the source package" \{ spago, fixture } -> do + setup spago + ( defaultSetupConfig + { installSourcePackages = [ "maybe" ] + , main = Just + [ "import Prelude" + , "import Data.Maybe as Maybe" + , "import Effect as Effect" + , "import Effect.Console as Console" + , "import Control.Alt as Alt" + , "main = unit" + ] + } + ) + spago [ "build", "--pedantic-packages" ] + >>= shouldBeFailureErr (fixture "pedantic/check-direct-import-transitive-dependency.txt") + editSpagoYaml addPedanticFlagToSrc + spago [ "build" ] + >>= shouldBeFailureErr (fixture "pedantic/check-direct-import-transitive-dependency.txt") + + -- We are importing `Control.Alt` in the test package, which is in the `control` + -- package, which comes through `maybe` but we are not importing directly, so we + -- should get a transitivity warning about that + Spec.it "appear in the test package" \{ spago, fixture } -> do + setup spago + ( defaultSetupConfig + { installTestPackages = [ "maybe" ] + , main = Just + [ "import Prelude" + , "import Data.Maybe as Maybe" + , "import Effect as Effect" + , "import Effect.Console as Console" + , "main = unit" + ] + , testMain = Just + [ "import Prelude" + , "import Data.Maybe as Maybe" + , "import Effect as Effect" + , "import Effect.Console as Console" + , "import Control.Alt as Alt" + , "main = unit" + ] + } + ) + -- Here we install `maybe` only in test, so there should be a transitive + -- warning to install `maybe` in source, since we use it there and the flag + -- will turn it on for both source and test + spago [ "build", "--pedantic-packages" ] + >>= shouldBeFailureErr (fixture "pedantic/check-direct-import-transitive-dependency-both.txt") + editSpagoYaml addPedanticFlagToTest + -- Otherwise we can just turn it on for the test, and it will complain only about `control` + spago [ "build" ] + >>= shouldBeFailureErr (fixture "pedantic/check-direct-import-transitive-dependency-test.txt") + + Spec.describe "fails with warnings about unused dependencies" do + + -- Here we install `effect` and `console` in the test package, and we don't use them + -- in the source, so we should get an "unused" warning about them + Spec.it "in a source package" \{ spago, fixture } -> do + setup spago + ( defaultSetupConfig + { installTestPackages = [ "effect", "console" ] + , main = Just + [ "import Prelude" + , "main = unit" + ] + } + ) + spago [ "build", "--pedantic-packages" ] >>= shouldBeFailureErr (fixture "pedantic/check-unused-dependency.txt") + editSpagoYaml addPedanticFlagToSrc + spago [ "build" ] >>= shouldBeFailureErr (fixture "pedantic/check-unused-dependency.txt") + + -- Here we do not install `effect` and `console` in the test package, and we don't use them + -- in the source, so we should get an "unused" warning about them for the source, and a prompt + -- to install them in test + Spec.it "in a source package, but they are used in test" \{ spago, fixture } -> do + setup spago + ( defaultSetupConfig + { main = Just + [ "import Prelude" + , "main = unit" + ] + } + ) + spago [ "build", "--pedantic-packages" ] >>= shouldBeFailureErr (fixture "pedantic/check-unused-dependency-in-source.txt") + editSpagoYaml addPedanticFlagToSrc + spago [ "build" ] >>= shouldBeFailureErr (fixture "pedantic/check-unused-dependency.txt") + + -- Complain about the unused `newtype` dependency in the test package + Spec.it "in a test package" \{ spago, fixture } -> do + setup spago + ( defaultSetupConfig + { installTestPackages = [ "newtype" ] + , testMain = Just + [ "import Prelude" + , "main = unit" + ] + } + ) + -- first just add the flag + spago [ "build", "--pedantic-packages" ] + >>= shouldBeFailureErr (fixture "pedantic/check-unused-test-dependency.txt") + -- then prove that it also works when using it from the config + editSpagoYaml addPedanticFlagToTest + spago [ "build" ] + >>= shouldBeFailureErr (fixture "pedantic/check-unused-test-dependency.txt") + + -- `console` and `effect` are going to be unused for both source and test packages + Spec.it "in both the source and test packages" \{ spago, fixture } -> do + setup spago + ( defaultSetupConfig + { installSourcePackages = [ "prelude", "effect", "console" ] + , installTestPackages = [ "prelude", "effect", "console" ] + , main = Just + [ "import Prelude" + , "main = unit" + ] + , testMain = Just + [ "import Prelude" + , "main = unit" + ] + } + ) + spago [ "build", "--pedantic-packages" ] + >>= shouldBeFailureErr (fixture "pedantic/check-unused-source-and-test-dependency.txt") + editSpagoYaml (addPedanticFlagToSrc >>> addPedanticFlagToTest) + spago [ "build" ] + >>= shouldBeFailureErr (fixture "pedantic/check-unused-source-and-test-dependency.txt") + + -- The source package adds `control` and `either`, but: + -- * `either` is unused + -- * `control` is used, and that's fine + -- * `newtype` is used but not declare, so we get a "transitive" warning + -- The test package adds `tuples`, but: + -- * `tuples` is unused + -- * `newtype` is transitively imported, but we already warned about that in the source + -- so consider that fixed + -- * `either` is transitively imported, and it's going to be removed from the source + -- dependencies, so we get a "transitive" warning to install it in test + Spec.it "fails to build and reports deduplicated src and test unused/transitive dependencies" \{ spago, fixture } -> do + setup spago + ( defaultSetupConfig + { installSourcePackages = [ "prelude", "control", "either" ] + , installTestPackages = [ "tuples" ] + , main = Just + [ "import Prelude" + , "import Data.Newtype as Newtype" + , "import Control.Alt as Alt" + , "" + , "foo :: Int" + , "foo = 1" + ] + , testMain = Just + [ "import Prelude" + , "import Data.Newtype (class Newtype)" + , "import Data.Either (Either(..))" + , "" + , "newtype Bar = Bar Int" + , "derive instance Newtype Bar _" + , "" + , "foo :: Either Bar Int" + , "foo = Right 1" + ] + } + ) + spago [ "build", "--pedantic-packages" ] >>= shouldBeFailureErr (fixture "pedantic/check-pedantic-packages.txt") + editSpagoYaml (addPedanticFlagToSrc >>> addPedanticFlagToTest) + spago [ "build" ] >>= shouldBeFailureErr (fixture "pedantic/check-pedantic-packages.txt") + +editSpagoYaml :: (Config -> Config) -> Aff Unit +editSpagoYaml f = do + content <- FS.readYamlDocFile Core.configCodec "spago.yaml" + case content of + Left err -> + Assert.fail $ "Failed to decode spago.yaml file\n" <> err + Right { yaml: config } -> + FS.writeYamlFile Core.configCodec "spago.yaml" $ f config + +addPedanticFlagToSrc :: Config -> Config +addPedanticFlagToSrc config = config + { package = config.package <#> \r -> r + { build = Just + { pedantic_packages: Just true + , strict: Nothing + , censor_project_warnings: Nothing + } + } + } + +addPedanticFlagToTest :: Config -> Config +addPedanticFlagToTest config = config + { package = config.package <#> \r -> r + { test = Just + { main: "Test.Main" + , pedantic_packages: Just true + , strict: Nothing + , censor_test_warnings: Nothing + , dependencies: maybe (Dependencies Map.empty) _.dependencies r.test + , exec_args: r.test >>= _.exec_args + } + } + } + +type SetupConfig = + { installSourcePackages :: Array String + , installTestPackages :: Array String + , main :: Maybe (Array String) + , testMain :: Maybe (Array String) + } + +defaultSetupConfig :: SetupConfig +defaultSetupConfig = + { installSourcePackages: [] + , installTestPackages: [] + , main: Nothing + , testMain: Nothing + } + +setup :: (Array String -> Aff (Either ExecError ExecResult)) -> SetupConfig -> Aff Unit +setup spago config = do + spago [ "init", "--name", "pedantic" ] >>= shouldBeSuccess + unless (Array.null config.installSourcePackages) do + spago ([ "install" ] <> config.installSourcePackages) >>= shouldBeSuccess + unless (Array.null config.installTestPackages) do + spago ([ "install", "--test-deps" ] <> config.installTestPackages) >>= shouldBeSuccess + for_ config.main \main -> + FS.writeTextFile (Path.concat [ "src", "Main.purs" ]) $ writeMain main + for_ config.testMain \testMain -> + FS.writeTextFile (Path.concat [ "test", "Test", "Main.purs" ]) $ writeTestMain testMain + -- get rid of "Compiling ..." messages and other compiler warnings + spago [ "build" ] >>= shouldBeSuccess diff --git a/test/Spago/Build/Polyrepo.purs b/test/Spago/Build/Polyrepo.purs index 9d4bc9531..fd74561f1 100644 --- a/test/Spago/Build/Polyrepo.purs +++ b/test/Spago/Build/Polyrepo.purs @@ -364,7 +364,7 @@ spec = Spec.describe "polyrepo" do { modName: mkTestModuleName packageName , imports: [ "import Control.Alt as Control" - , "import Data.Newtype as Newtype" + , "import Safe.Coerce as Coerce" ] , body: [ "" @@ -383,7 +383,11 @@ spec = Spec.describe "polyrepo" do [ toMsgPrefix isSrc <> " for package '" <> package <> "' declares unused dependencies - please remove them from the project config:" , " - " <> (if isSrc then "tuples" else "either") ] - mkTransitiveDepErr isSrc package = + mkTransitiveDepErr isSrc package = do + let + { pkg, mkModName, pkgModName } = + if isSrc then { pkg: "newtype", mkModName: mkSrcModuleName, pkgModName: "Data.Newtype" } + else { pkg: "safe-coerce", mkModName: mkTestModuleName, pkgModName: "Safe.Coerce" } Array.intercalate "\n" [ Array.fold [ toMsgPrefix isSrc @@ -391,9 +395,9 @@ spec = Spec.describe "polyrepo" do , 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" + , " " <> pkg + , " from `" <> mkModName package <> "`, which imports:" + , " " <> pkgModName ] Spec.it "when package config has 'pedantic_packages: true', build fails with expected errors" \{ spago } -> do writeWorkspaceOnlySpagoYamlFile