Merge pull request #1416 from stremlenye/master

Add the CLI option to fetch only recently changed files for formating.

bin/_scalafmt

@@ -16,7 +16,7 @@ _scalafmt() {
     '--assume-filename[when using --stdin, use --assume-filename to hint to scalafmt that the input is an .sbt file]:value:' \
     '--test[test for mis-formatted code, exits with status 1 on failure]' \
     '--migrate2hocon[migrate .scalafmt CLI style configuration to hocon style configuration in .scalafmt.conf]:value:' \
-    '--diff[if set, only format edited files in git diff against master]' \
+    '--mode diff[if set, only format edited files in git diff against master]' \
     '--diff-branch[if set, only format edited files in git diff against provided branch]:value:' \
     '--build-info[prints build information]' \
     '--quiet[do not print out stuff to console]' \

scalafmt-cli/src/main/scala/org/scalafmt/cli/CliArgParser.scala

@@ -15,7 +15,8 @@ object CliArgParser {
        |         # 2. .scalafmt.conf inside root directory of current git repo
        |         # 3. no configuration, default style
        |scalafmt --test # throw exception on mis-formatted files, won't write to files.
-       |scalafmt --diff # Format all files that were edited in git diff against master branch.
+       |scalafmt --mode diff # Format all files that were edited in git diff against master branch.
+       |scalafmt --mode changed # Format files listed in `git status` (latest changes against previous commit.
        |scalafmt --diff-branch 2.x # same as --diff, except against branch 2.x
        |scalafmt --stdin # read from stdin and print to stdout
        |scalafmt --stdin --assume-filename foo.sbt < foo.sbt # required when using --stdin to format .sbt files.
@@ -135,12 +136,23 @@ object CliArgParser {
           """migrate .scalafmt CLI style configuration to hocon style configuration in .scalafmt.conf"""
         )
       opt[Unit]("diff")
-        .action((_, c) => c.copy(diff = Some("master")))
-        .text("If set, only format edited files in git diff against master.")
+        .action((_, c) => c.copy(mode = Option(DiffFiles("master"))))
+        .text(
+          s"""Format files listed in `git diff` against master.
+             |Deprecated: use --mode diff instead""".stripMargin
+        )
+      opt[FileFetchMode]("mode")
+        .action((m, c) => c.copy(mode = Option(m)))
+        .text(
+          s"""Sets the files to be formatted fetching mode.
+             |Options:
+             |        diff - format files listed in `git diff` against master
+             |        changed - format files listed in `git status` (latest changes against previous commit)""".stripMargin
+        )
       opt[String]("diff-branch")
         .action((branch, c) => c.copy(diff = Some(branch)))
         .text(
-          "If set, only format edited files in git diff against provided branch."
+          "If set, only format edited files in git diff against provided branch. Has no effect if mode set to `changed`."
         )
       opt[Unit]("build-info")
         .action({

scalafmt-cli/src/main/scala/org/scalafmt/cli/CliOptions.scala

@@ -45,6 +45,7 @@ object CliOptions {
       config = style
     )
   }
+
   private def getConfigJFile(file: AbsoluteFile): AbsoluteFile =
     file / ".scalafmt.conf"
 
@@ -85,6 +86,7 @@ case class CliOptions(
     debug: Boolean = false,
     git: Option[Boolean] = None,
     nonInteractive: Boolean = false,
+    mode: Option[FileFetchMode] = None,
     diff: Option[String] = None,
     assumeFilename: String = "stdin.scala", // used when read from stdin
     migrate: Option[AbsoluteFile] = None,
@@ -145,11 +147,8 @@ case class CliOptions(
 
   val inPlace: Boolean = writeMode == Override
 
-  val fileFetchMode: FileFetchMode = {
-    diff.map(DiffFiles).getOrElse {
-      if (isGit) GitFiles else RecursiveSearch
-    }
-  }
+  val fileFetchMode: FileFetchMode =
+    mode.orElse(Some(GitFiles).filter(_ => isGit)).getOrElse(RecursiveSearch)
 
   val files: Seq[AbsoluteFile] =
     if (customFiles.isEmpty)

scalafmt-cli/src/main/scala/org/scalafmt/cli/FileFetchMode.scala

@@ -1,10 +1,24 @@
 package org.scalafmt.cli
 
+import scopt.Read
+
 /**
   * Determines how we fetch files for formatting
   */
 sealed trait FileFetchMode
 
+object FileFetchMode {
+
+  /**
+    * The read instance is practically is not exhaustive due to the RecursiveSearch and GitFiles are the fallback used in the absence of
+    * other options
+    */
+  implicit val read: Read[FileFetchMode] = Read.reads {
+    case "diff" => DiffFiles("master")
+    case "changed" => ChangedFiles
+  }
+}
+
 /**
   * A simple recursive strategy where each directory is expanded
   */
@@ -21,3 +35,10 @@ final case object GitFiles extends FileFetchMode
   * When this is set, files passed via the cli are ignored.
   */
 final case class DiffFiles(branch: String) extends FileFetchMode
+
+/**
+  * A call to `git status --porcelain`
+  *
+  * When this is set, files passed via the cli are ignored.
+  */
+final case object ChangedFiles extends FileFetchMode

scalafmt-cli/src/main/scala/org/scalafmt/cli/ScalafmtCoreRunner.scala

@@ -32,7 +32,7 @@ object ScalafmtCoreRunner extends ScalafmtRunner {
         )
 
         val inputMethods = getInputMethods(options, Some(filterMatcher))
-        if (inputMethods.isEmpty && options.diff.isEmpty && !options.stdIn)
+        if (inputMethods.isEmpty && options.mode.isEmpty && !options.stdIn)
           throw NoMatchingFiles
 
         val counter = new AtomicInteger()

scalafmt-cli/src/main/scala/org/scalafmt/cli/ScalafmtDynamicRunner.scala

@@ -14,7 +14,7 @@ object ScalafmtDynamicRunner extends ScalafmtRunner {
       termDisplayMessage: String
   ): ExitCode = {
     val inputMethods = getInputMethods(options, None)
-    if (inputMethods.isEmpty && options.diff.isEmpty && !options.stdIn)
+    if (inputMethods.isEmpty && options.mode.isEmpty && !options.stdIn)
       throw NoMatchingFiles
 
     val counter = new AtomicInteger()

scalafmt-cli/src/main/scala/org/scalafmt/cli/ScalafmtRunner.scala

@@ -50,7 +50,8 @@ trait ScalafmtRunner {
       filter: Option[FilterMatcher]
   ): Seq[AbsoluteFile] = {
     def canFormat(f: AbsoluteFile): Boolean =
-      filter.map(_.matches(f)).getOrElse(true)
+      filter.forall(_.matches(f))
+
     val files = options.fileFetchMode match {
       case m @ (GitFiles | RecursiveSearch) =>
         val fetchFiles: AbsoluteFile => Seq[AbsoluteFile] =
@@ -63,8 +64,12 @@ trait ScalafmtRunner {
           // formatted regardless of what it is or where it is.
           case f => Seq(f)
         }
+
       case DiffFiles(branch) =>
         options.gitOps.diff(branch).filter(canFormat)
+
+      case ChangedFiles =>
+        options.gitOps.status.filter(canFormat)
     }
     val excludeRegexp = options.excludeFilterRegexp
     files.filter { f =>

scalafmt-core/shared/src/main/scala/org/scalafmt/util/AbsoluteFile.scala

@@ -5,6 +5,7 @@ import java.io.File
 /** Wrapper around java.io.File with an absolute path. */
 sealed abstract case class AbsoluteFile(jfile: File) {
   def path: String = jfile.getAbsolutePath
+  def exists: Boolean = jfile.exists()
   def /(other: String) = new AbsoluteFile(new File(jfile, other)) {}
 
   override def toString(): String = path

scalafmt-core/shared/src/main/scala/org/scalafmt/util/GitOps.scala

@@ -1,12 +1,12 @@
 package org.scalafmt.util
 
 import scala.sys.process.ProcessLogger
-import scala.util.Try
+import scala.util.{Failure, Success, Try}
 import scala.util.control.NonFatal
-
 import java.io.File
 
 trait GitOps {
+  def status: Seq[AbsoluteFile]
   def diff(branch: String): Seq[AbsoluteFile]
   def lsTree(dir: AbsoluteFile): Seq[AbsoluteFile]
   def rootDir: Option[AbsoluteFile]
@@ -50,7 +50,7 @@ class GitOpsImpl(private[util] val workingDirectory: AbsoluteFile)
           "--full-name",
           dir.path
         )
-      ).toOption.toSeq.flatten.map(f => rtDir / f)
+      ).getOrElse(Seq.empty).map(f => rtDir / f)
     }
 
   override def rootDir: Option[AbsoluteFile] = {
@@ -76,7 +76,48 @@ class GitOpsImpl(private[util] val workingDirectory: AbsoluteFile)
     )
     for {
       root <- rootDir.toSeq
-      path <- exec(cmd).toOption.toSeq.flatten
+      path <- exec(cmd).getOrElse(Seq.empty)
     } yield AbsoluteFile.fromFile(new File(path), root)
   }
+
+  override def status: Seq[AbsoluteFile] = {
+    val cmd = Seq(
+      "git",
+      "status",
+      "--porcelain"
+    )
+    for {
+      root <- rootDir.toSeq
+      statusLine <- exec(cmd).getOrElse(Seq.empty)
+      file <- getFileFromGitStatusLine(statusLine).toOption.toSeq
+    } yield AbsoluteFile.fromFile(file, root)
+  }.filter(_.exists)
+
+  private final val renameStatusCode = "R"
+  private final val renameStatusArrowDelimiter = "-> "
+
+  /*
+    Method extracts path to changed file from the singular line of the `git status --porcelain` output.
+   (see https://git-scm.com/docs/git-status#_short_format)
+   */
+  private def extractPathPart(s: String): Try[String] =
+    Try(
+      Option(s)
+      // Checks if the line status states the file was renamed (E.g: `R  ORIG_PATH -> PATH`)
+        .filter(_.substring(0, 2).contains(renameStatusCode))
+        // takes the part of the string after the `-> ` character sequence
+        .map(_.split(renameStatusArrowDelimiter).last)
+        // fallback for the regular status line (E.g.: `XY PATH`)
+        // Drops the status codes by splitting on white spaces then taking the tail of the result
+        // Restores spaces in the path by merging the tail back with white space separator
+        .getOrElse(s.trim.split(' ').tail.mkString(" "))
+        .trim
+    )
+
+  private def trimQuotes(s: String): String =
+    s.replaceAll("^\"|\"$", "")
+
+  private def getFileFromGitStatusLine(s: String): Try[File] =
+    extractPathPart(s)
+      .map(pathRaw => new File(trimQuotes(pathRaw)))
 }

scalafmt-tests/src/test/scala/org/scalafmt/cli/CliTest.scala

@@ -71,6 +71,7 @@ abstract class AbstractCliTest extends FunSuite with DiffAssertions {
 
   def gimmeConfig(string: String): ScalafmtConfig =
     Config.fromHoconString(string).get
+
   def noArgTest(
       input: AbsoluteFile,
       expected: String,
@@ -314,11 +315,12 @@ trait CliTestBehavior { this: AbstractCliTest =>
     }
 
     test(
-      s"scalafmt (no matching files) is okay with --diff and --stdin: $label"
+      s"scalafmt (no matching files) is okay with --mode diff and --stdin: $label"
     ) {
       val diff = getConfig(
         Array(
-          "--diff",
+          "--mode",
+          "diff",
           "--config-str",
           s"""{version="$version",style=IntelliJ}"""
         )
@@ -370,9 +372,10 @@ trait CliTestBehavior { this: AbstractCliTest =>
       noArgTest(
         input,
         expected,
-        Seq(Array.empty[String], Array("--diff"))
+        Seq(Array.empty[String], Array("--mode", "diff"))
       )
     }
+
     test(s"scalafmt (no arg, no config): $label") {
       noArgTest(
         string2dir(
@@ -459,7 +462,7 @@ trait CliTestBehavior { this: AbstractCliTest =>
     }
 
     test(
-      s"includeFilters are ignored for full paths but NOT ignore for passed directories: $label"
+      s"includeFilters are ignored for full paths but NOT test for passed directories: $label"
     ) {
       val root =
         string2dir(

scalafmt-tests/src/test/scala/org/scalafmt/cli/FakeGitOps.scala

@@ -8,5 +8,6 @@ class FakeGitOps(root: AbsoluteFile) extends GitOps {
   override def lsTree(dir: AbsoluteFile): Vector[AbsoluteFile] =
     FileOps.listFiles(dir)
   override def rootDir: Option[AbsoluteFile] = Some(root)
+  override def status: Seq[AbsoluteFile] = lsTree(root)
   override def diff(branch: String): Seq[AbsoluteFile] = lsTree(root)
 }