From 0718394ab6e8059f2854282844a7632b29e68dc3 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Mon, 6 Nov 2023 11:32:27 +0100 Subject: [PATCH] Refactoring (#2842) Added some explicit return values and fixed some compiler warnings. Use `SubPath` instead of `RelPath` for tests executable paths. Reordered in/out stream initialization for consistency. Pull request: https://github.com/com-lihaoyi/mill/pull/2842 --- build.sc | 11 ++++++----- .../mill/integration/ExampleTestSuite.scala | 10 +++++----- .../test/src/InvalidMetaModuleTests.scala | 2 +- .../src/MultipleTopLevelModulesTests.scala | 2 +- .../src/ThingsOutsideTopLevelModuleTests.scala | 2 +- main/api/src/mill/api/SystemStreams.scala | 4 ++-- main/codesig/src/LocalSummary.scala | 2 +- main/codesig/src/ResolvedCalls.scala | 2 +- main/define/src/mill/define/Task.scala | 12 ++++++------ main/src/mill/main/MainModule.scala | 4 ++-- .../worker/ScalaJSModuleSplitStyle.scala | 18 ++++++++++++++++++ .../scalajslib/worker/ScalaJSWorkerImpl.scala | 17 ----------------- .../src/mill/scalalib/api/ZincWorkerUtil.scala | 16 ++++++++-------- scalalib/src/mill/scalalib/JavaModule.scala | 4 ---- scalalib/src/mill/scalalib/ScalaModule.scala | 2 -- .../src/mill/scalalib/ZincWorkerModule.scala | 4 ---- .../mill/scalalib/bsp/BuildScAwareness.scala | 5 ++--- .../dependency/versions/VersionParser.scala | 3 ++- .../mill/scalalib/internal/ModuleUtils.scala | 2 +- 19 files changed, 57 insertions(+), 65 deletions(-) create mode 100644 scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSModuleSplitStyle.scala diff --git a/build.sc b/build.sc index 9add67396fb..34dd901eecd 100644 --- a/build.sc +++ b/build.sc @@ -222,11 +222,12 @@ val bridgeScalaVersions = Seq( // of them. Compiler bridges not in this set will get downloaded and compiled // on the fly anyway. For publishing, we publish everything or a specific version // if given. -val compilerBridgeScalaVersions = interp.watchValue(sys.env.get("MILL_COMPILER_BRIDGE_VERSIONS")) match { - case None => Seq.empty[String] - case Some("all") => bridgeScalaVersions - case Some(versions) => versions.split(',').map(_.trim).toSeq -} +val compilerBridgeScalaVersions = + interp.watchValue(sys.env.get("MILL_COMPILER_BRIDGE_VERSIONS")) match { + case None => Seq.empty[String] + case Some("all") => bridgeScalaVersions + case Some(versions) => versions.split(',').map(_.trim).toSeq + } val bridgeVersion = "0.0.1" trait MillJavaModule extends JavaModule { diff --git a/example/src/mill/integration/ExampleTestSuite.scala b/example/src/mill/integration/ExampleTestSuite.scala index 15d996f1a84..142e83dc325 100644 --- a/example/src/mill/integration/ExampleTestSuite.scala +++ b/example/src/mill/integration/ExampleTestSuite.scala @@ -91,7 +91,7 @@ object ExampleTestSuite extends IntegrationTestSuite { case "mill" => evalStdout(rest) case cmd => val tokens = cmd +: rest - val executable = workspaceRoot / os.RelPath(tokens.head) + val executable = workspaceRoot / os.SubPath(tokens.head) if (!os.exists(executable)) { throw new Exception( s"Executable $executable not found.\n" + @@ -167,7 +167,7 @@ object ExampleTestSuite extends IntegrationTestSuite { } finally { zipFile.close() } case Seq("printf", literal, ">>", path) => - mangleFile(os.Path(path, workspacePath), _ + ujson.read('"' + literal + '"').str) + mangleFile(os.Path(path, workspacePath), _ + ujson.read(s""""${literal}"""").str) } } @@ -217,17 +217,17 @@ object ExampleTestSuite extends IntegrationTestSuite { } } - def globMatches(expected: String, filtered: String) = { + def globMatches(expected: String, filtered: String): Boolean = { filtered .linesIterator .exists( StringContext - .glob(expected.split("\\.\\.\\.", -1), _) + .glob(expected.split("\\.\\.\\.", -1).toIndexedSeq, _) .nonEmpty ) } - def globMatchesAny(expected: String, filtered: String) = { + def globMatchesAny(expected: String, filtered: String): Boolean = { expected.linesIterator.exists(globMatches(_, filtered)) } } diff --git a/integration/failure/invalid-meta-module/test/src/InvalidMetaModuleTests.scala b/integration/failure/invalid-meta-module/test/src/InvalidMetaModuleTests.scala index f33a30eeaf3..2fa2a724a73 100644 --- a/integration/failure/invalid-meta-module/test/src/InvalidMetaModuleTests.scala +++ b/integration/failure/invalid-meta-module/test/src/InvalidMetaModuleTests.scala @@ -4,7 +4,7 @@ import utest._ object InvalidMetaModuleTests extends IntegrationTestSuite { val tests = Tests { - val workspaceRoot = initWorkspace() + initWorkspace() test("success") { val res = evalStdout("resolve", "_") diff --git a/integration/failure/multiple-top-level-modules/test/src/MultipleTopLevelModulesTests.scala b/integration/failure/multiple-top-level-modules/test/src/MultipleTopLevelModulesTests.scala index fb447ee8f45..d524eee7046 100644 --- a/integration/failure/multiple-top-level-modules/test/src/MultipleTopLevelModulesTests.scala +++ b/integration/failure/multiple-top-level-modules/test/src/MultipleTopLevelModulesTests.scala @@ -4,7 +4,7 @@ import utest._ object MultipleTopLevelModulesTests extends IntegrationTestSuite { val tests = Tests { - val workspaceRoot = initWorkspace() + initWorkspace() test("success") { val res = evalStdout("resolve", "_") diff --git a/integration/failure/things-outside-top-level-module/test/src/ThingsOutsideTopLevelModuleTests.scala b/integration/failure/things-outside-top-level-module/test/src/ThingsOutsideTopLevelModuleTests.scala index 6c64f93c579..825f25db561 100644 --- a/integration/failure/things-outside-top-level-module/test/src/ThingsOutsideTopLevelModuleTests.scala +++ b/integration/failure/things-outside-top-level-module/test/src/ThingsOutsideTopLevelModuleTests.scala @@ -4,7 +4,7 @@ import utest._ object ThingsOutsideTopLevelModuleTests extends IntegrationTestSuite { val tests = Tests { - val workspaceRoot = initWorkspace() + initWorkspace() test("success") { val res = evalStdout("resolve", "_") diff --git a/main/api/src/mill/api/SystemStreams.scala b/main/api/src/mill/api/SystemStreams.scala index 930b0431216..aecd81a7566 100644 --- a/main/api/src/mill/api/SystemStreams.scala +++ b/main/api/src/mill/api/SystemStreams.scala @@ -49,13 +49,13 @@ object SystemStreams { def originalErr: PrintStream = original.err def withStreams[T](systemStreams: SystemStreams)(t: => T): T = { - val out = System.out val in = System.in + val out = System.out val err = System.err try { System.setIn(systemStreams.in) - System.setErr(systemStreams.err) System.setOut(systemStreams.out) + System.setErr(systemStreams.err) Console.withIn(systemStreams.in) { Console.withOut(systemStreams.out) { Console.withErr(systemStreams.err) { diff --git a/main/codesig/src/LocalSummary.scala b/main/codesig/src/LocalSummary.scala index 868fc22e23c..346b1870833 100644 --- a/main/codesig/src/LocalSummary.scala +++ b/main/codesig/src/LocalSummary.scala @@ -140,7 +140,7 @@ object LocalSummary { def hash(x: Int): Unit = insnHash = scala.util.hashing.MurmurHash3.mix(insnHash, x) - def completeHash() = { + def completeHash(): Unit = { insnSigs.append(scala.util.hashing.MurmurHash3.finalizeHash(0, insnHash)) insnHash = 0 } diff --git a/main/codesig/src/ResolvedCalls.scala b/main/codesig/src/ResolvedCalls.scala index 2c4debb4a7e..c39e14c471b 100644 --- a/main/codesig/src/ResolvedCalls.scala +++ b/main/codesig/src/ResolvedCalls.scala @@ -125,7 +125,7 @@ object ResolvedCalls { val allSamImplementors = allSamImplementors0 .groupMap(_._1)(_._2) - .mapValues(_.toSet) + .view.mapValues(_.toSet) .toMap allSamImplementors diff --git a/main/define/src/mill/define/Task.scala b/main/define/src/mill/define/Task.scala index 10e9970dd18..2c529383a96 100644 --- a/main/define/src/mill/define/Task.scala +++ b/main/define/src/mill/define/Task.scala @@ -53,8 +53,8 @@ object Task { } private[define] class Sequence[+T](inputs0: Seq[Task[T]]) extends Task[Seq[T]] { - val inputs = inputs0 - def evaluate(ctx: mill.api.Ctx) = { + val inputs: Seq[Task[_]] = inputs0 + def evaluate(ctx: mill.api.Ctx): Result[Seq[T]] = { for (i <- 0 until ctx.args.length) yield ctx.args(i).asInstanceOf[T] } @@ -63,7 +63,7 @@ object Task { inputs0: Seq[Task[T]], f: (IndexedSeq[T], mill.api.Ctx) => Result[V] ) extends Task[V] { - val inputs = inputs0 + val inputs: Seq[Task[_]] = inputs0 def evaluate(ctx: mill.api.Ctx) = { f( for (i <- 0 until ctx.args.length) @@ -75,12 +75,12 @@ object Task { private[define] class Mapped[+T, +V](source: Task[T], f: T => V) extends Task[V] { def evaluate(ctx: mill.api.Ctx) = f(ctx.arg(0)) - val inputs = List(source) + val inputs: Seq[Task[_]] = List(source) } private[define] class Zipped[+T, +V](source1: Task[T], source2: Task[V]) extends Task[(T, V)] { def evaluate(ctx: mill.api.Ctx) = (ctx.arg(0), ctx.arg(1)) - val inputs = List(source1, source2) + val inputs: Seq[Task[_]] = List(source1, source2) } } @@ -108,7 +108,7 @@ trait NamedTask[+T] extends Task[T] { def evaluate(ctx: mill.api.Ctx) = ctx.arg[T](0) val ctx = ctx0.withSegments(segments = ctx0.segments ++ Seq(ctx0.segment)) - val inputs = Seq(t) + val inputs: Seq[Task[_]] = Seq(t) def readWriterOpt: Option[upickle.default.ReadWriter[_]] = None diff --git a/main/src/mill/main/MainModule.scala b/main/src/mill/main/MainModule.scala index 11fcfebbc4f..cb5cb7c90f2 100644 --- a/main/src/mill/main/MainModule.scala +++ b/main/src/mill/main/MainModule.scala @@ -2,7 +2,6 @@ package mill.main import java.util.concurrent.LinkedBlockingQueue import mill.define.Target -import mill.main.BuildInfo import mill.api.{Ctx, Logger, PathRef, Result} import mill.define.{Command, NamedTask, Segments, Task} import mill.eval.{Evaluator, EvaluatorPaths, Terminal} @@ -31,7 +30,8 @@ object MainModule { targets: Seq[String], log: Logger, watch0: Watchable => Unit - )(f: Seq[(Any, Option[(RunScript.TaskName, ujson.Value)])] => ujson.Value) = { + )(f: Seq[(Any, Option[(RunScript.TaskName, ujson.Value)])] => ujson.Value) + : Result[ujson.Value] = { RunScript.evaluateTasksNamed( // When using `show`, redirect all stdout of the evaluated tasks so the diff --git a/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSModuleSplitStyle.scala b/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSModuleSplitStyle.scala new file mode 100644 index 00000000000..b3874b699dc --- /dev/null +++ b/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSModuleSplitStyle.scala @@ -0,0 +1,18 @@ +package mill.scalajslib.worker + +// Separating ModuleSplitStyle in a standalone object avoids +// early classloading which fails in Scala.js versions where +// the classes don't exist +object ScalaJSModuleSplitStyle { + import org.scalajs.linker.interface.ModuleSplitStyle + object SmallModulesFor { + def apply(packages: List[String]): ModuleSplitStyle = + ModuleSplitStyle.SmallModulesFor(packages) + } + object FewestModules { + def apply(): ModuleSplitStyle = ModuleSplitStyle.FewestModules + } + object SmallestModules { + def apply(): ModuleSplitStyle = ModuleSplitStyle.SmallestModules + } +} diff --git a/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala b/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala index fe7050aa710..1df6144ff87 100644 --- a/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala +++ b/scalajslib/worker/1/src/mill/scalajslib/worker/ScalaJSWorkerImpl.scala @@ -330,20 +330,3 @@ class ScalaJSWorkerImpl extends ScalaJSWorkerApi { Seq(input) } } - -// Separating ModuleSplitStyle in a standalone object avoids -// early classloading which fails in Scala.js versions where -// the classes don't exist -object ScalaJSModuleSplitStyle { - import org.scalajs.linker.interface.ModuleSplitStyle - object SmallModulesFor { - def apply(packages: List[String]): ModuleSplitStyle = - ModuleSplitStyle.SmallModulesFor(packages) - } - object FewestModules { - def apply(): ModuleSplitStyle = ModuleSplitStyle.FewestModules - } - object SmallestModules { - def apply(): ModuleSplitStyle = ModuleSplitStyle.SmallestModules - } -} diff --git a/scalalib/api/src/mill/scalalib/api/ZincWorkerUtil.scala b/scalalib/api/src/mill/scalalib/api/ZincWorkerUtil.scala index a4fe90c8c72..44800ee4290 100644 --- a/scalalib/api/src/mill/scalalib/api/ZincWorkerUtil.scala +++ b/scalalib/api/src/mill/scalalib/api/ZincWorkerUtil.scala @@ -24,10 +24,10 @@ trait ZincWorkerUtil { val suffix = if (sources) "-sources.jar" else ".jar" lazy val dir = if (sources) "srcs" else "jars" - def mavenStyleMatch(fname: String) = + def mavenStyleMatch(fname: String): Boolean = fname.startsWith(s"$name-$versionPrefix") && fname.endsWith(suffix) - def ivyStyleMatch(p: os.Path) = { + def ivyStyleMatch(p: os.Path): Boolean = { val fname = s"$name$suffix" p.segments.toSeq match { case _ :+ v :+ `dir` :+ `fname` if v.startsWith(versionPrefix) => true @@ -52,7 +52,7 @@ trait ZincWorkerUtil { val NightlyVersion = raw"""(\d+)\.(\d+)\.(\d+)-bin-[a-f0-9]*""".r val TypelevelVersion = raw"""(\d+)\.(\d+)\.(\d+)-bin-typelevel.*""".r - def scalaBinaryVersion(scalaVersion: String) = scalaVersion match { + def scalaBinaryVersion(scalaVersion: String): String = scalaVersion match { case Scala3EarlyVersion(milestone) => s"3.0.0-$milestone" case Scala3Version(_, _) => "3" case ReleaseVersion(major, minor, _) => s"$major.$minor" @@ -65,7 +65,7 @@ trait ZincWorkerUtil { private val ScalaJSFullVersion = """^([0-9]+)\.([0-9]+)\.([0-9]+)(-.*)?$""".r - def scalaJSBinaryVersion(scalaJSVersion: String) = scalaJSVersion match { + def scalaJSBinaryVersion(scalaJSVersion: String): String = scalaJSVersion match { case _ if scalaJSVersion.startsWith("0.6.") => throw new Exception("Scala.js 0.6 is not supported") case ScalaJSFullVersion(major, minor, patch, suffix) => @@ -75,7 +75,7 @@ trait ZincWorkerUtil { major } - def scalaJSWorkerVersion(scalaJSVersion: String) = scalaJSVersion match { + def scalaJSWorkerVersion(scalaJSVersion: String): String = scalaJSVersion match { case _ if scalaJSVersion.startsWith("0.6.") => throw new Exception("Scala.js 0.6 is not supported") case ScalaJSFullVersion(major, _, _, _) => @@ -84,7 +84,7 @@ trait ZincWorkerUtil { private val ScalaNativeFullVersion = """^([0-9]+)\.([0-9]+)\.([0-9]+)(-.*)?$""".r - def scalaNativeBinaryVersion(version: String) = version match { + def scalaNativeBinaryVersion(version: String): String = version match { case ScalaNativeFullVersion(major, minor, patch, suffix) => if (suffix != null && patch == "0") version @@ -92,7 +92,7 @@ trait ZincWorkerUtil { s"$major.$minor" } - def scalaNativeWorkerVersion(version: String) = version match { + def scalaNativeWorkerVersion(version: String): String = version match { case ScalaNativeFullVersion(major, minor, _, _) => s"$major.$minor" } @@ -110,7 +110,7 @@ trait ZincWorkerUtil { Versions.millCompilerBridgeScalaVersions.split(",").toSet /** @return true if the compiler bridge can be downloaded as an already compiled jar */ - def isBinaryBridgeAvailable(scalaVersion: String) = + def isBinaryBridgeAvailable(scalaVersion: String): Boolean = if (millCompilerBridgeScalaVersions.contains(scalaVersion)) true else scalaVersion match { case DottyNightlyVersion(major, minor, _, _) => diff --git a/scalalib/src/mill/scalalib/JavaModule.scala b/scalalib/src/mill/scalalib/JavaModule.scala index a26b5f19c31..96b073359dc 100644 --- a/scalalib/src/mill/scalalib/JavaModule.scala +++ b/scalalib/src/mill/scalalib/JavaModule.scala @@ -1,7 +1,6 @@ package mill package scalalib -import scala.annotation.nowarn import coursier.Repository import coursier.core.Dependency import coursier.core.Resolution @@ -699,8 +698,6 @@ trait JavaModule */ def ivyDepsTree(args: IvyDepsTreeArgs = IvyDepsTreeArgs()): Command[Unit] = { - val dependsOnModules = args.whatDependsOn.map(ModuleParser.javaOrScalaModule(_)) - val (invalidModules, validModules) = args.whatDependsOn.map(ModuleParser.javaOrScalaModule(_)).partitionMap(identity) @@ -961,7 +958,6 @@ trait JavaModule /** * @param all If `true` fetches also source dependencies */ - @nowarn("msg=pure expression does nothing") override def prepareOffline(all: Flag): Command[Unit] = { val tasks = if (all.value) Seq( diff --git a/scalalib/src/mill/scalalib/ScalaModule.scala b/scalalib/src/mill/scalalib/ScalaModule.scala index 819eaa2e4b8..2071117b7ce 100644 --- a/scalalib/src/mill/scalalib/ScalaModule.scala +++ b/scalalib/src/mill/scalalib/ScalaModule.scala @@ -1,7 +1,6 @@ package mill package scalalib -import scala.annotation.nowarn import mill.api.{DummyInputStream, JarManifest, PathRef, Result, SystemStreams, internal} import mill.main.BuildInfo import mill.util.{Jvm, Util} @@ -480,7 +479,6 @@ trait ScalaModule extends JavaModule with TestModule.ScalaModuleBase { outer => /** * @param all If `true` , fetches also sources, Ammonite and compiler dependencies. */ - @nowarn("msg=pure expression does nothing") override def prepareOffline(all: Flag): Command[Unit] = { val ammonite = resolvedAmmoniteReplIvyDeps val tasks = diff --git a/scalalib/src/mill/scalalib/ZincWorkerModule.scala b/scalalib/src/mill/scalalib/ZincWorkerModule.scala index 661760f5066..0ce66219da3 100644 --- a/scalalib/src/mill/scalalib/ZincWorkerModule.scala +++ b/scalalib/src/mill/scalalib/ZincWorkerModule.scala @@ -1,6 +1,5 @@ package mill.scalalib -import scala.annotation.nowarn import coursier.Repository import mainargs.Flag import mill.Agg @@ -47,7 +46,6 @@ trait ZincWorkerModule extends mill.Module with OfflineSupportModule { self: Cou def zincLogDebug: T[Boolean] = T.input(T.ctx().log.debugEnabled) def worker: Worker[ZincWorkerApi] = T.worker { - val ctx = T.ctx() val jobs = T.ctx() match { case j: Ctx.Jobs => j.jobs case _ => 1 @@ -169,14 +167,12 @@ trait ZincWorkerModule extends mill.Module with OfflineSupportModule { self: Cou } else dep } - @nowarn("msg=pure expression does nothing") override def prepareOffline(all: Flag): Command[Unit] = T.command { super.prepareOffline(all)() classpath() () } - @nowarn("msg=pure expression does nothing") def prepareOfflineCompiler(scalaVersion: String, scalaOrganization: String): Command[Unit] = T.command { classpath() diff --git a/scalalib/src/mill/scalalib/bsp/BuildScAwareness.scala b/scalalib/src/mill/scalalib/bsp/BuildScAwareness.scala index 75aab2c797d..199c4f6f001 100644 --- a/scalalib/src/mill/scalalib/bsp/BuildScAwareness.scala +++ b/scalalib/src/mill/scalalib/bsp/BuildScAwareness.scala @@ -13,7 +13,6 @@ trait BuildScAwareness { private val MatchDep = """^import ([$]ivy[.]`([^`]+)`).*""".r private val MatchFile = """^import ([$]file[.]([^,]+)).*""".r - private val MatchShebang = """^(#!.*)$""".r def parseAmmoniteImports(buildScript: os.Path): Seq[Included] = { val queue = mutable.Queue.empty[os.Path] @@ -25,8 +24,8 @@ trait BuildScAwareness { while (queue.nonEmpty) { val file = queue.dequeue() val inclusions = os.read.lines(file).collect { - case m @ MatchDep(_, dep) => IncludedDep(dep, file) - case m @ MatchFile(_, include) => + case MatchDep(_, dep) => IncludedDep(dep, file) + case MatchFile(_, include) => val pat = include.split("[.]").map { case "^" => os.up case x => os.rel / x diff --git a/scalalib/src/mill/scalalib/dependency/versions/VersionParser.scala b/scalalib/src/mill/scalalib/dependency/versions/VersionParser.scala index 4dd540bd702..d4cb56c816f 100644 --- a/scalalib/src/mill/scalalib/dependency/versions/VersionParser.scala +++ b/scalalib/src/mill/scalalib/dependency/versions/VersionParser.scala @@ -1,6 +1,7 @@ package mill.scalalib.dependency.versions -import fastparse._, NoWhitespace._ +import fastparse._ +import fastparse.NoWhitespace._ private[dependency] object VersionParser { diff --git a/scalalib/src/mill/scalalib/internal/ModuleUtils.scala b/scalalib/src/mill/scalalib/internal/ModuleUtils.scala index 44cfd244285..dc991285ea6 100644 --- a/scalalib/src/mill/scalalib/internal/ModuleUtils.scala +++ b/scalalib/src/mill/scalalib/internal/ModuleUtils.scala @@ -1,6 +1,6 @@ package mill.scalalib.internal -import mill.api.{BuildScriptException, experimental} +import mill.api.BuildScriptException import mill.define.{Module, Segments} import scala.annotation.tailrec