Fix nailgun integration.

Added test to ensure the integration continues to work.

.drone.yml

@@ -2,6 +2,9 @@
 build:
   image: olafurpg/scalafix:0.0.1
   commands:
+    - apt-get install nailgun
+    - alias ng=ng-nailgun
+    - export PATH=$PATH:/root/local/bin
     - pip install --user codecov
     - ./bin/testAll.sh
     # I can't be bothered to hide this token.

bin/nailgun_integration_test.sh

@@ -0,0 +1,42 @@
+#!/usr/bin/env bash
+set -e
+
+echo "Running nailgun integration test..."
+
+sbt "cli/pack"
+cwd=$(pwd)
+cd cli/target/pack
+make install
+cd $cwd
+
+echoOriginalContents() {
+  cat << EOF
+object app {
+  def main() {
+    println(1)
+  }
+}
+EOF
+}
+
+echoExpectedContents() {
+  cat << EOF
+object app {
+  def main(): Unit = {
+    println(1)
+  }
+}
+EOF
+}
+original=target/original.scala
+expected=target/expected.scala
+
+mkdir -p target
+echoOriginalContents > ${original}
+echoExpectedContents > ${expected}
+./bin/scalafix_ng -i ${original}
+
+# Assert there is no diff
+diff ${original} ${expected}
+
+echo "Done!"

bin/scalafix_ng

@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+set -e
+
+function assert-installed() {
+  binary=$1
+  command -v ${binary} >/dev/null 2>&1 || { echo >&2 "Missing dependency ${binary}, exiting."; exit 1; }
+}
+
+NAILGUN="ng"
+if  ! type "${NAILGUN}" > /dev/null; then
+  NAILGUN="ng-nailgun"
+fi
+
+assert-installed $NAILGUN
+assert-installed scalafix_ng_server
+assert-installed entr
+
+startNailgun() {
+  echo "Starting nailgun..."
+  scalafix_ng_server &
+  sleep 1
+  $NAILGUN ng-alias scalafix scalafix.cli.Cli
+}
+
+
+if [ "$1" = "restart" ]; then
+  $NAILGUN ng-stop || true
+  sleep 1
+  startNailgun
+else
+  $NAILGUN ng-stats &> /dev/null || startNailgun
+  $NAILGUN scalafix "$@"
+fi

bin/testAll.sh

@@ -1,7 +1,10 @@
 #!/bin/bash
 set -e
 
-sbt clean coverage test
-sbt "; publishLocal ; scripted"
-sbt coverageAggregate
+#sbt clean coverage test
+#sbt "; publishLocal ; scripted"
+#sbt coverageAggregate
+
+# Integration tests
+./bin/nailgun_integration_test.sh
 

build.sbt

@@ -122,12 +122,12 @@ lazy val cli = project
     moduleName := "scalafix-cli",
     packJvmOpts := Map(
       "scalafix" -> jvmOptions,
-      "scalafix_ng" -> jvmOptions
+      "scalafix_ng_server" -> jvmOptions
     ),
     mainClass in assembly := Some("scalafix.cli.Cli"),
     packMain := Map(
       "scalafix" -> "scalafix.cli.Cli",
-      "scalafix_ng" -> "com.martiansoftware.nailgun.NGServer"
+      "scalafix_ng_server" -> "com.martiansoftware.nailgun.NGServer"
     ),
     libraryDependencies ++= Seq(
       "com.github.scopt"           %% "scopt"         % "3.5.0",

cli/src/main/scala/scalafix/cli/Cli.scala

@@ -17,6 +17,13 @@ import caseapp._
 import caseapp.core.Messages
 import com.martiansoftware.nailgun.NGContext
 
+case class CommonOptions(
+    @Hidden workingDirectory: String = System.getProperty("user.dir"),
+    @Hidden out: PrintStream = System.out,
+    @Hidden in: InputStream = System.in,
+    @Hidden err: PrintStream = System.err
+)
+
 @AppName("scalafix")
 @AppVersion(scalafix.Versions.nightly)
 @ProgName("scalafix")
@@ -36,16 +43,12 @@ case class ScalafixOptions(
     @HelpMessage(
       "If true, prints out debug information."
     ) debug: Boolean = false,
-    @Hidden workingDirectory: String = System.getProperty("user.dir"),
-    @Hidden out: PrintStream = System.out,
-    @Hidden in: InputStream = System.in,
-    @Hidden err: PrintStream = System.err
+    @Recurse common: CommonOptions = CommonOptions()
 ) extends App {
+
   Cli.runOn(this)
 }
 
-object ScalafixOptions {}
-
 object Cli extends AppOf[ScalafixOptions] {
   val helpMessage: String = Messages[ScalafixOptions].withHelp.helpMessage
 
@@ -56,24 +59,24 @@ object Cli extends AppOf[ScalafixOptions] {
       case FixResult.Success(code) =>
         if (config.inPlace) {
           FileOps.writeFile(file, code)
-        } else config.out.write(code.getBytes)
+        } else config.common.out.write(code.getBytes)
       case FixResult.Failure(e) =>
-        config.err.write(s"Failed to fix $file. Cause: $e".getBytes)
+        config.common.err.write(s"Failed to fix $file. Cause: $e".getBytes)
       case e: FixResult.ParseError =>
         if (config.files.contains(file)) {
           // Only log if user explicitly specified that file.
-          config.err.write(e.toString.getBytes())
+          config.common.err.write(e.toString.getBytes())
         }
     }
   }
 
   def runOn(config: ScalafixOptions): Unit = {
     config.files.foreach { pathStr =>
       val path = new File(pathStr)
-      val workingDirectory = new File(config.workingDirectory)
+      val workingDirectory = new File(config.common.workingDirectory)
       val realPath: File =
         if (path.isAbsolute) path
-        else new File(config.workingDirectory, path.getPath)
+        else new File(config.common.workingDirectory, path.getPath)
       if (realPath.isDirectory) {
         val filesToFix: GenSeq[String] = {
           val files =
@@ -84,13 +87,13 @@ object Cli extends AppOf[ScalafixOptions] {
         val logger = new TermDisplay(new OutputStreamWriter(System.out))
         logger.init()
         val msg = "Running scalafix..."
-        logger.downloadingArtifact(msg, workingDirectory)
-        logger.downloadLength(msg, filesToFix.length, 0)
+        logger.startTask(msg, workingDirectory)
+        logger.taskLength(msg, filesToFix.length, 0)
         val counter = new AtomicInteger()
         filesToFix.foreach { x =>
           handleFile(new File(x), config)
           val progress = counter.incrementAndGet()
-          logger.downloadProgress(msg, progress)
+          logger.taskProgress(msg, progress)
         }
         logger.stop()
       } else {
@@ -106,16 +109,20 @@ object Cli extends AppOf[ScalafixOptions] {
       case Left(x) => Left(x)
     }
 
-  def runMain(args: Seq[String], init: ScalafixOptions): Unit = {
+  def runMain(args: Seq[String], commonOptions: CommonOptions): Unit = {
     parse(args) match {
-      case _ => System.exit(1)
+      case Right(options) =>
+        runOn(options.copy(common = commonOptions))
+      case Left(error) =>
+        commonOptions.err.println(error)
+        System.exit(1)
     }
   }
 
   def nailMain(nGContext: NGContext): Unit = {
     runMain(
       nGContext.getArgs,
-      ScalafixOptions(
+      CommonOptions(
         workingDirectory = nGContext.getWorkingDirectory,
         out = nGContext.out,
         in = nGContext.in,

cli/src/main/scala/scalafix/cli/TermDisplay.scala

@@ -431,24 +431,10 @@ object TermDisplay {
 object Cache {
   trait Logger {
     def foundLocally(url: String, f: File): Unit = {}
-
-    def downloadingArtifact(url: String, file: File): Unit = {}
-
-    @deprecated("Use / override the variant with 3 arguments instead")
-    def downloadLength(url: String, length: Long): Unit = {}
-    def downloadLength(url: String,
-                       totalLength: Long,
-                       alreadyDownloaded: Long): Unit = {
-      downloadLength(url, totalLength)
-    }
-
-    def downloadProgress(url: String, downloaded: Long): Unit = {}
-
-    def downloadedArtifact(url: String, success: Boolean): Unit = {}
+    def startTask(url: String, file: File): Unit = {}
+    def taskProgress(url: String, downloaded: Long): Unit = {}
+    def completedTask(url: String, success: Boolean): Unit = {}
     def checkingUpdates(url: String, currentTimeOpt: Option[Long]): Unit = {}
-    def checkingUpdatesResult(url: String,
-                              currentTimeOpt: Option[Long],
-                              remoteTimeOpt: Option[Long]): Unit = {}
   }
 }
 
@@ -469,20 +455,20 @@ class TermDisplay(
     updateThread.end()
   }
 
-  override def downloadingArtifact(url: String, file: File): Unit =
+  override def startTask(msg: String, file: File): Unit =
     updateThread.newEntry(
-      url,
+      msg,
       DownloadInfo(0L,
                    0L,
                    None,
                    System.currentTimeMillis(),
                    updateCheck = false),
-      s"Downloading $url\n"
+      s"$msg\n"
     )
 
-  override def downloadLength(url: String,
-                              totalLength: Long,
-                              alreadyDownloaded: Long): Unit = {
+  def taskLength(url: String,
+                 totalLength: Long,
+                 alreadyDownloaded: Long): Unit = {
     val info = updateThread.infos.get(url)
     assert(info != null)
     val newInfo = info match {
@@ -496,7 +482,7 @@ class TermDisplay(
 
     updateThread.update()
   }
-  override def downloadProgress(url: String, downloaded: Long): Unit = {
+  override def taskProgress(url: String, downloaded: Long): Unit = {
     val info = updateThread.infos.get(url)
     assert(info != null)
     val newInfo = info match {
@@ -510,7 +496,7 @@ class TermDisplay(
     updateThread.update()
   }
 
-  override def downloadedArtifact(url: String, success: Boolean): Unit =
+  override def completedTask(url: String, success: Boolean): Unit =
     updateThread.removeEntry(url, success, s"Downloaded $url\n")(x => x)
 
   override def checkingUpdates(url: String,
@@ -521,25 +507,4 @@ class TermDisplay(
       s"Checking $url\n"
     )
 
-  override def checkingUpdatesResult(
-      url: String,
-      currentTimeOpt: Option[Long],
-      remoteTimeOpt: Option[Long]
-  ): Unit = {
-    // Not keeping a message on-screen if a download should happen next
-    // so that the corresponding URL doesn't appear twice
-    val newUpdate = remoteTimeOpt.exists { remoteTime =>
-      currentTimeOpt.forall { currentTime =>
-        currentTime < remoteTime
-      }
-    }
-
-    updateThread.removeEntry(url, !newUpdate, s"Checked $url\n") {
-      case info: CheckUpdateInfo =>
-        info.copy(remoteTimeOpt = remoteTimeOpt, isDone = true)
-      case _ =>
-        throw new Exception(s"Incoherent display state for $url")
-    }
-  }
-
 }

cli/src/test/scala/scalafix/cli/CliSuite.scala

@@ -9,19 +9,7 @@ import java.io.PrintStream
 
 import org.scalatest.FunSuite
 
-class CliSuite extends FunSuite with DiffAssertions {
-
-  println(Cli.helpMessage)
-
-  test("testMain") {
-    val expected = ScalafixOptions(
-      files = List("foo", "bar"),
-      inPlace = true
-    )
-    val Right(obtained) = Cli.parse(Seq("-i", "foo", "bar"))
-    assertEqual(obtained, expected)
-  }
-
+object BasicTest {
   val original = """|object Main {
                     |  def foo() {
                     |    println(1)
@@ -35,6 +23,23 @@ class CliSuite extends FunSuite with DiffAssertions {
                     |}
                  """.stripMargin
 
+}
+
+class CliSuite extends FunSuite with DiffAssertions {
+
+  println(Cli.helpMessage)
+
+  import BasicTest._
+
+  test("testMain") {
+    val expected = ScalafixOptions(
+      files = List("foo", "bar"),
+      inPlace = true
+    )
+    val Right(obtained) = Cli.parse(Seq("-i", "foo", "bar"))
+    assertEqual(obtained, expected)
+  }
+
   test("write fix to file") {
     val file = File.createTempFile("prefix", ".scala")
     FileOps.writeFile(file, original)
@@ -49,7 +54,9 @@ class CliSuite extends FunSuite with DiffAssertions {
     val baos = new ByteArrayOutputStream()
     Cli.runOn(
       ScalafixOptions(
-        out = new PrintStream(baos),
+        common = CommonOptions(
+          out = new PrintStream(baos)
+        ),
         files = List(file.getAbsolutePath)
       ))
     assertNoDiff(FileOps.readFile(file), original)