From df82bc9cf95ac9a085fe4573209310894a780252 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Wed, 15 Nov 2023 11:47:35 +0100 Subject: [PATCH 1/5] Put hierarchy checks for test trait behind an overridable def --- .../scalajslib/ScalaTestsErrorTests.scala | 9 +++- scalalib/src/mill/scalalib/ScalaModule.scala | 47 ++++++++++--------- .../scalanativelib/ScalaTestsErrorTests.scala | 7 +++ 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala b/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala index 8a9cb541b1b..205ae010a18 100644 --- a/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala +++ b/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala @@ -1,6 +1,5 @@ package mill.scalajslib -import mill._ import mill.define.Discover import mill.scalalib.TestModule import mill.util.TestUtil @@ -12,8 +11,10 @@ object ScalaTestsErrorTests extends TestSuite { def scalaVersion = sys.props.getOrElse("TEST_SCALA_3_3_VERSION", ???) def scalaJSVersion = sys.props.getOrElse("TEST_SCALAJS_VERSION", ???) object test extends ScalaTests with TestModule.Utest + object testDisabledError extends ScalaTests with TestModule.Utest { + override def hierarchyChecks(): Unit = {} + } } - override lazy val millDiscover = Discover[this.type] } @@ -27,5 +28,9 @@ object ScalaTestsErrorTests extends TestSuite { message == s"scalaTestsError is a `ScalaJSModule`. scalaTestsError.test needs to extend `ScalaJSTests`." ) } + test("extends-ScalaTests-disabled-hierarchy-check") { + // expect no throws exception + ScalaTestsError.scalaTestsError.testDisabledError + } } } diff --git a/scalalib/src/mill/scalalib/ScalaModule.scala b/scalalib/src/mill/scalalib/ScalaModule.scala index 5a391444cb8..a59f17e0ef1 100644 --- a/scalalib/src/mill/scalalib/ScalaModule.scala +++ b/scalalib/src/mill/scalalib/ScalaModule.scala @@ -28,28 +28,33 @@ trait ScalaModule extends JavaModule with TestModule.ScalaModuleBase { outer => type ScalaModuleTests = ScalaTests trait ScalaTests extends JavaModuleTests with ScalaModule { - try { - if ( - Class.forName("mill.scalajslib.ScalaJSModule").isInstance(outer) && !Class.forName( - "mill.scalajslib.ScalaJSModule$ScalaJSTests" - ).isInstance(this) - ) throw new MillException( - s"$outer is a `ScalaJSModule`. $this needs to extend `ScalaJSTests`." - ) - } catch { - case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaJSModule - } - try { - if ( - Class.forName("mill.scalanativelib.ScalaNativeModule").isInstance(outer) && !Class.forName( - "mill.scalanativelib.ScalaNativeModule$ScalaNativeTests" - ).isInstance(this) - ) throw new MillException( - s"$outer is a `ScalaNativeModule`. $this needs to extend `ScalaNativeTests`." - ) - } catch { - case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaNativeModule + protected def hierarchyChecks(): Unit = { + try { + if ( + Class.forName("mill.scalajslib.ScalaJSModule").isInstance(outer) && !Class.forName( + "mill.scalajslib.ScalaJSModule$ScalaJSTests" + ).isInstance(this) + ) throw new MillException( + s"$outer is a `ScalaJSModule`. $this needs to extend `ScalaJSTests`." + ) + } catch { + case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaJSModule + } + try { + if ( + Class.forName("mill.scalanativelib.ScalaNativeModule").isInstance( + outer + ) && !Class.forName( + "mill.scalanativelib.ScalaNativeModule$ScalaNativeTests" + ).isInstance(this) + ) throw new MillException( + s"$outer is a `ScalaNativeModule`. $this needs to extend `ScalaNativeTests`." + ) + } catch { + case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaNativeModule + } } + hierarchyChecks() override def scalaOrganization: Target[String] = outer.scalaOrganization() override def scalaVersion: Target[String] = outer.scalaVersion() diff --git a/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala b/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala index af843d2102c..5727dfd94b7 100644 --- a/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala +++ b/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala @@ -12,6 +12,9 @@ object ScalaTestsErrorTests extends TestSuite { def scalaVersion = sys.props.getOrElse("TEST_SCALA_3_3_VERSION", ???) def scalaNativeVersion = sys.props.getOrElse("TEST_SCALANATIVE_VERSION", ???) object test extends ScalaTests with TestModule.Utest + object testDisabledError extends ScalaTests with TestModule.Utest { + override def hierarchyChecks(): Unit = {} + } } override lazy val millDiscover = Discover[this.type] @@ -27,5 +30,9 @@ object ScalaTestsErrorTests extends TestSuite { message == s"scalaTestsError is a `ScalaNativeModule`. scalaTestsError.test needs to extend `ScalaNativeTests`." ) } + test("extends-ScalaTests-disabled-hierarchy-check") { + // expect no throws exception + ScalaTestsError.scalaTestsError.testDisabledError + } } } From e97d4392e280333ec493ae5810ebc4623cbeedd0 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Wed, 15 Nov 2023 16:19:42 +0100 Subject: [PATCH 2/5] Reworked hierarchyChecks implementation --- .../scalajslib/ScalaTestsErrorTests.scala | 2 +- scalalib/src/mill/scalalib/ScalaModule.scala | 53 +++++++++++-------- .../scalanativelib/ScalaTestsErrorTests.scala | 2 +- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala b/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala index 205ae010a18..f5582116b61 100644 --- a/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala +++ b/scalajslib/test/src/mill/scalajslib/ScalaTestsErrorTests.scala @@ -25,7 +25,7 @@ object ScalaTestsErrorTests extends TestSuite { } val message = error.getCause.getMessage assert( - message == s"scalaTestsError is a `ScalaJSModule`. scalaTestsError.test needs to extend `ScalaJSTests`." + message == s"scalaTestsError is a `mill.scalajslib.ScalaJSModule`. scalaTestsError.test needs to extend `ScalaJSTests`." ) } test("extends-ScalaTests-disabled-hierarchy-check") { diff --git a/scalalib/src/mill/scalalib/ScalaModule.scala b/scalalib/src/mill/scalalib/ScalaModule.scala index a59f17e0ef1..1f6638a3675 100644 --- a/scalalib/src/mill/scalalib/ScalaModule.scala +++ b/scalalib/src/mill/scalalib/ScalaModule.scala @@ -29,30 +29,39 @@ trait ScalaModule extends JavaModule with TestModule.ScalaModuleBase { outer => trait ScalaTests extends JavaModuleTests with ScalaModule { protected def hierarchyChecks(): Unit = { - try { - if ( - Class.forName("mill.scalajslib.ScalaJSModule").isInstance(outer) && !Class.forName( - "mill.scalajslib.ScalaJSModule$ScalaJSTests" - ).isInstance(this) - ) throw new MillException( - s"$outer is a `ScalaJSModule`. $this needs to extend `ScalaJSTests`." - ) - } catch { - case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaJSModule - } - try { - if ( - Class.forName("mill.scalanativelib.ScalaNativeModule").isInstance( - outer - ) && !Class.forName( - "mill.scalanativelib.ScalaNativeModule$ScalaNativeTests" - ).isInstance(this) - ) throw new MillException( - s"$outer is a `ScalaNativeModule`. $this needs to extend `ScalaNativeTests`." + val outerInnerSets = Seq( + ( + "mill.scalajslib.ScalaJSModule", + "mill.scalajslib.ScalaJSModule$ScalaJSTests", + "ScalaJSTests" + ), + ( + "mill.scalanativelib.ScalaNativeModule", + "mill.scalanativelib.ScalaNativeModule$ScalaNativeTests", + "ScalaNativeTests" + ), + ( + "mill.scalalib.SbtModule", + "mill.scalalib.SbtModule$SbtModuleTests", + "SbtModuleTests" + ), + ( + "mill.scalalib.MavenModule", + "mill.scalalib.MavenModule$MavenModuleTests", + "MavenModuleTests" ) - } catch { - case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaNativeModule + ) + for { + (mod, testMod, testModShort) <- outerInnerSets } + try { + if (Class.forName(mod).isInstance(outer) && !Class.forName(testMod).isInstance(this)) + throw new MillException( + s"$outer is a `${mod}`. $this needs to extend `${testModShort}`." + ) + } catch { + case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaJSModule + } } hierarchyChecks() diff --git a/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala b/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala index 5727dfd94b7..fcc13540fcd 100644 --- a/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala +++ b/scalanativelib/test/src/mill/scalanativelib/ScalaTestsErrorTests.scala @@ -27,7 +27,7 @@ object ScalaTestsErrorTests extends TestSuite { } val message = error.getCause.getMessage assert( - message == s"scalaTestsError is a `ScalaNativeModule`. scalaTestsError.test needs to extend `ScalaNativeTests`." + message == s"scalaTestsError is a `mill.scalanativelib.ScalaNativeModule`. scalaTestsError.test needs to extend `ScalaNativeTests`." ) } test("extends-ScalaTests-disabled-hierarchy-check") { From 3eb439fd5df5f168dfd8834dca3b4e140f90677b Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Thu, 16 Nov 2023 07:53:24 +0100 Subject: [PATCH 3/5] Compressed the associator map --- scalalib/src/mill/scalalib/ScalaModule.scala | 27 +++++--------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/scalalib/src/mill/scalalib/ScalaModule.scala b/scalalib/src/mill/scalalib/ScalaModule.scala index 1f6638a3675..50b609738d5 100644 --- a/scalalib/src/mill/scalalib/ScalaModule.scala +++ b/scalalib/src/mill/scalalib/ScalaModule.scala @@ -30,29 +30,14 @@ trait ScalaModule extends JavaModule with TestModule.ScalaModuleBase { outer => trait ScalaTests extends JavaModuleTests with ScalaModule { protected def hierarchyChecks(): Unit = { val outerInnerSets = Seq( - ( - "mill.scalajslib.ScalaJSModule", - "mill.scalajslib.ScalaJSModule$ScalaJSTests", - "ScalaJSTests" - ), - ( - "mill.scalanativelib.ScalaNativeModule", - "mill.scalanativelib.ScalaNativeModule$ScalaNativeTests", - "ScalaNativeTests" - ), - ( - "mill.scalalib.SbtModule", - "mill.scalalib.SbtModule$SbtModuleTests", - "SbtModuleTests" - ), - ( - "mill.scalalib.MavenModule", - "mill.scalalib.MavenModule$MavenModuleTests", - "MavenModuleTests" - ) + ("mill.scalajslib.ScalaJSModule", "ScalaJSTests"), + ("mill.scalanativelib.ScalaNativeModule", "ScalaNativeTests"), + ("mill.scalalib.SbtModule", "SbtModuleTests"), + ("mill.scalalib.MavenModule", "MavenModuleTests") ) for { - (mod, testMod, testModShort) <- outerInnerSets + (mod, testModShort) <- outerInnerSets + testMod = s"${mod}$$${testModShort}" } try { if (Class.forName(mod).isInstance(outer) && !Class.forName(testMod).isInstance(this)) From f5a2f401b274d5f646539418b35e708b428747a9 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Thu, 16 Nov 2023 07:58:20 +0100 Subject: [PATCH 4/5] Moved check logic into JavaModule to also catch issues in MavenModule --- scalalib/src/mill/scalalib/JavaModule.scala | 32 +++++++++++++++++++- scalalib/src/mill/scalalib/ScalaModule.scala | 32 +------------------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/scalalib/src/mill/scalalib/JavaModule.scala b/scalalib/src/mill/scalalib/JavaModule.scala index 96b073359dc..4601003f73d 100644 --- a/scalalib/src/mill/scalalib/JavaModule.scala +++ b/scalalib/src/mill/scalalib/JavaModule.scala @@ -9,7 +9,7 @@ import coursier.parse.ModuleParser import coursier.util.ModuleMatcher import mainargs.Flag import mill.Agg -import mill.api.{Ctx, JarManifest, PathRef, Result, internal} +import mill.api.{Ctx, JarManifest, MillException, PathRef, Result, internal} import mill.define.{Command, ModuleRef, Segment, Task, TaskModule} import mill.scalalib.internal.ModuleUtils import mill.scalalib.api.CompilationResult @@ -34,6 +34,9 @@ trait JavaModule def zincWorker: ModuleRef[ZincWorkerModule] = ModuleRef(mill.scalalib.ZincWorkerModule) trait JavaModuleTests extends JavaModule with TestModule { + // Run some consistence checks + hierarchyChecks() + override def moduleDeps: Seq[JavaModule] = Seq(outer) override def repositoriesTask: Task[Seq[Repository]] = outer.repositoriesTask override def resolutionCustomizer: Task[Option[coursier.Resolution => coursier.Resolution]] = @@ -47,6 +50,33 @@ trait JavaModule PathRef(this.millSourcePath / src.path.relativeTo(outer.millSourcePath)) } } + + /** + * JavaModule and its derivates define inner test modules. + * To avoid unexpected misbehavior due to the use of the wrong inner test trait + * we apply some hierarchy consistency checks. + * If for some reasons, those are too restrictive to you, you can override this method. + */ + protected def hierarchyChecks(): Unit = { + val outerInnerSets = Seq( + ("mill.scalajslib.ScalaJSModule", "ScalaJSTests"), + ("mill.scalanativelib.ScalaNativeModule", "ScalaNativeTests"), + ("mill.scalalib.SbtModule", "SbtModuleTests"), + ("mill.scalalib.MavenModule", "MavenModuleTests") + ) + for { + (mod, testModShort) <- outerInnerSets + testMod = s"${mod}$$${testModShort}" + } + try { + if (Class.forName(mod).isInstance(outer) && !Class.forName(testMod).isInstance(this)) + throw new MillException( + s"$outer is a `${mod}`. $this needs to extend `${testModShort}`." + ) + } catch { + case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaJSModule + } + } } def defaultCommandName(): String = "run" diff --git a/scalalib/src/mill/scalalib/ScalaModule.scala b/scalalib/src/mill/scalalib/ScalaModule.scala index 50b609738d5..2071117b7ce 100644 --- a/scalalib/src/mill/scalalib/ScalaModule.scala +++ b/scalalib/src/mill/scalalib/ScalaModule.scala @@ -1,15 +1,7 @@ package mill package scalalib -import mill.api.{ - DummyInputStream, - JarManifest, - MillException, - PathRef, - Result, - SystemStreams, - internal -} +import mill.api.{DummyInputStream, JarManifest, PathRef, Result, SystemStreams, internal} import mill.main.BuildInfo import mill.util.{Jvm, Util} import mill.util.Jvm.createJar @@ -28,28 +20,6 @@ trait ScalaModule extends JavaModule with TestModule.ScalaModuleBase { outer => type ScalaModuleTests = ScalaTests trait ScalaTests extends JavaModuleTests with ScalaModule { - protected def hierarchyChecks(): Unit = { - val outerInnerSets = Seq( - ("mill.scalajslib.ScalaJSModule", "ScalaJSTests"), - ("mill.scalanativelib.ScalaNativeModule", "ScalaNativeTests"), - ("mill.scalalib.SbtModule", "SbtModuleTests"), - ("mill.scalalib.MavenModule", "MavenModuleTests") - ) - for { - (mod, testModShort) <- outerInnerSets - testMod = s"${mod}$$${testModShort}" - } - try { - if (Class.forName(mod).isInstance(outer) && !Class.forName(testMod).isInstance(this)) - throw new MillException( - s"$outer is a `${mod}`. $this needs to extend `${testModShort}`." - ) - } catch { - case _: ClassNotFoundException => // if we can't find the classes, we certainly are not in a ScalaJSModule - } - } - hierarchyChecks() - override def scalaOrganization: Target[String] = outer.scalaOrganization() override def scalaVersion: Target[String] = outer.scalaVersion() override def scalacPluginIvyDeps: Target[Agg[Dep]] = outer.scalacPluginIvyDeps() From 9ecbd78128251a504d9ebc7316aa61cee07a9cc0 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Thu, 16 Nov 2023 08:02:47 +0100 Subject: [PATCH 5/5] . --- scalalib/src/mill/scalalib/JavaModule.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/scalalib/src/mill/scalalib/JavaModule.scala b/scalalib/src/mill/scalalib/JavaModule.scala index 4601003f73d..d6184a6035f 100644 --- a/scalalib/src/mill/scalalib/JavaModule.scala +++ b/scalalib/src/mill/scalalib/JavaModule.scala @@ -56,6 +56,7 @@ trait JavaModule * To avoid unexpected misbehavior due to the use of the wrong inner test trait * we apply some hierarchy consistency checks. * If for some reasons, those are too restrictive to you, you can override this method. + * @throws MillException */ protected def hierarchyChecks(): Unit = { val outerInnerSets = Seq(