Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scalafmt support #308

Merged
merged 9 commits into from
May 6, 2018
39 changes: 39 additions & 0 deletions scalalib/src/mill/scalalib/scalafmt/ScalafmtModule.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package mill.scalalib.scalafmt

import ammonite.ops._
import mill._
import mill.define.{Command, Sources}
import mill.scalalib._

trait ScalafmtModule extends ScalaModule {

def reformat(): Command[Unit] = T.command {
ScalafmtWorkerModule
.worker()
.reformat(
filesToFormat(sources()),
scalafmtConfig().head,
scalafmtDeps().map(_.path)
)
}

def scalafmtVersion: T[String] = "1.5.1"

def scalafmtConfig: Sources = T.sources(pwd / ".scalafmt.conf")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have an alternative for Sources for a single file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so; we could add one if it comes up repeatedly, but for now just picking the first item in the Agg[PathRef] may be fine


def scalafmtDeps: T[Agg[PathRef]] = T {
Lib.resolveDependencies(
scalaWorker.repositories,
Lib.depToDependency(_, "2.12.4"),
Seq(ivy"com.geirsson::scalafmt-cli:${scalafmtVersion()}")
)
}

private def filesToFormat(sources: Seq[PathRef]) = {
for {
pathRef <- sources if exists(pathRef.path)
file <- ls.rec(pathRef.path) if file.isFile && file.ext == "scala"
} yield PathRef(file)
}

}
58 changes: 58 additions & 0 deletions scalalib/src/mill/scalalib/scalafmt/ScalafmtWorker.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package mill.scalalib.scalafmt

import ammonite.ops.{Path, exists}
import mill._
import mill.define.{Discover, ExternalModule, Worker}
import mill.modules.Jvm
import mill.util.Ctx

import scala.collection.mutable

object ScalafmtWorkerModule extends ExternalModule {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't immediately clear that worker must live in a separate module to be a single one in a build.
Also, I copy-pasted extends ExternalModule from scala and scalajs workers. Why should it be an ExternalModule, not just a Module ?
When I tried extends Module complilation failed with:

[error] /Users/rockjam/projects/mill/scalalib/src/mill/scalalib/scalafmt/ScalafmtWorker.scala:11:54: Modules, Targets and Commands can only be defined within a mill Module
[error] private[scalafmt] object ScalafmtWorker extends mill.Module {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rockjam being an ExternalModule allows a task myTask to be run via mill.scalalib.scalafmt.ScalafmtWorkerModule/myTask. Normal Module objects must be defined in the build.sc file and be run via their object selector from the root of build.sc.

If you want to let people run ScalaFmt without needing to extend anything, you could make a task in the ExternalModule that lets people run e.g. mill.scalalib.scalafmt.ScalafmtWorkerModule/fmt __.sources to format the sources of all their ScalaModules. See how the mill.scalalib.PublishModule/publishAll command works as an example.

As for being separate Mill submodules, we only needed to put things like ScalaWorker/ScalaJSWorker in there because we needed code that runs with some heavyweight dependencies on the classpath that might collide (e.g. different Scala.js versions): the dependencies are isolated within a classloader and interacted with via reflection. If you don't have any heavyweight dependencies, or you do but you can just interact with them directly via reflection/subprocesses, then there's no need to isolate them in a separate Mill build module

def worker: Worker[ScalafmtWorker] = T.worker { new ScalafmtWorker() }

lazy val millDiscover = Discover[this.type]
}

private[scalafmt] class ScalafmtWorker {
private val reformatted: mutable.Map[Path, Int] = mutable.Map.empty
private var configSig: Int = 0

def reformat(input: Seq[PathRef],
scalafmtConfig: PathRef,
scalafmtClasspath: Agg[Path])(implicit ctx: Ctx): Unit = {
val toFormat =
if (scalafmtConfig.sig != configSig) input
else
input.filterNot(ref => reformatted.get(ref.path).contains(ref.sig))

if (toFormat.nonEmpty) {
ctx.log.info(s"Formatting ${toFormat.size} Scala sources")
reformatAction(toFormat.map(_.path),
scalafmtConfig.path,
scalafmtClasspath)
reformatted ++= toFormat.map { ref =>
val updRef = PathRef(ref.path)
updRef.path -> updRef.sig
}
configSig = scalafmtConfig.sig
} else {
ctx.log.info(s"Everything is formatted already")
}
}

private val cliFlags = Seq("--non-interactive", "--quiet")

private def reformatAction(toFormat: Seq[Path],
config: Path,
classpath: Agg[Path])(implicit ctx: Ctx) = {
val configFlags =
if (exists(config)) Seq("--config", config.toString) else Seq.empty
Jvm.subprocess(
"org.scalafmt.cli.Cli",
classpath,
mainArgs = toFormat.map(_.toString) ++ configFlags ++ cliFlags
)
}

}