Skip to content

Commit

Permalink
Added option to only format changed files (#50)
Browse files Browse the repository at this point in the history
* feature/1.0.3: added option to only format changes from a git branch

* updated maven version in build file

* post review changes
  • Loading branch information
SimonJPegg authored Jan 19, 2020
1 parent 2bd1b47 commit 3260add
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 20 deletions.
6 changes: 3 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ jdk:
- openjdk8

before_install:
- wget http://apache.mirror.gtcomm.net/maven/maven-3/3.6.2/binaries/apache-maven-3.6.2-bin.tar.gz
- tar xzvf apache-maven-3.6.2-bin.tar.gz
- export PATH=`pwd`/apache-maven-3.6.2/bin:$PATH
- wget http://apache.mirror.gtcomm.net/maven/maven-3/3.6.3/binaries/apache-maven-3.6.3-bin.tar.gz
- tar xzvf apache-maven-3.6.3-bin.tar.gz
- export PATH=`pwd`/apache-maven-3.6.3/bin:$PATH

install:
mvn --settings .maven.xml clean test -B -V
Expand Down
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ A wrapper that allows the use of the [Scalafmt](https://github.com/scalameta/sca

Add the following snippet to your pom.

Note: `version.scala.binary` refers to major releases of scala ie. 2.11 or 2.12 (scalafmt-dynamic doesn't currently support 2.13).
Note: `version.scala.binary` refers to major releases of scala ie. 2.11, 2.12 or 2.13.


```xml
<plugin>
<groupId>org.antipathy</groupId>
<artifactId>mvn-scalafmt_${version.scala.binary}</artifactId>
<version>1.0.1</version>
<version>1.0.3</version>
<configuration>
<configLocation>${project.basedir}/.scalafmt.conf</configLocation> <!-- path to config -->
<skipTestSources>false</skipTestSources> <!-- (Optional) skip formatting test sources -->
Expand All @@ -29,6 +30,8 @@ Note: `version.scala.binary` refers to major releases of scala ie. 2.11 or 2.12
<param>${project.basedir}/src/test/scala</param>
</testSourceDirectories>
<validateOnly>false</validateOnly> <!-- check formatting without changing files -->
<onlyChangedFiles>true</onlyChangedFiles> <!-- only format (staged) files that have been changed from the specified git branch -->
<branch>master</branch> <!-- The git branch to check against -->
</configuration>
<executions>
<execution>
Expand Down
6 changes: 3 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<groupId>org.antipathy</groupId>
<artifactId>mvn-scalafmt_${version.scala.major}</artifactId>
<packaging>maven-plugin</packaging>
<version>1.0.2</version>
<version>1.0.3</version>
<name>ScalaFmt Maven Plugin (${version.scala.major})</name>
<description>Maven plugin for ScalaFmt</description>
<url>https://github.com/SimonJPegg/mvn_scalafmt</url>
Expand Down Expand Up @@ -35,7 +35,7 @@
<properties>
<version.commonsio>2.6</version.commonsio>
<version.commonslang>3.8</version.commonslang>
<version.commonsvalidator>1.5.1</version.commonsvalidator>
<version.commonsvalidator>1.6</version.commonsvalidator>
<version.java>1.8</version.java>
<version.maven.annotations>3.5.2</version.maven.annotations>
<version.maven.plugin.compiler>3.8.0</version.maven.plugin.compiler>
Expand All @@ -51,7 +51,7 @@
<version.scala.major>2.13</version.scala.major>
<version.scala.minor>.1</version.scala.minor>
<version.scala.xml>1.2.0</version.scala.xml>
<version.scalafmt.dynamic>2.2.1</version.scalafmt.dynamic>
<version.scalafmt.dynamic>2.3.2</version.scalafmt.dynamic>
<version.scalatest>3.0.8</version.scalatest>

<maven.compiler.source>${version.java}</maven.compiler.source>
Expand Down
22 changes: 20 additions & 2 deletions src/main/java/org/antipathy/mvn_scalafmt/FormatMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.project.MavenProject;

import java.io.File;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -29,10 +31,17 @@ public class FormatMojo extends AbstractMojo {
private boolean respectVersion;
@Parameter(property = "format.validateOnly", defaultValue = "false")
private boolean validateOnly;
@Parameter(property = "format.onlyChangedFiles", defaultValue = "false")
private boolean onlyChangedFiles;
@Parameter(property = "format.branch", defaultValue = "master")
private String branch;
@Parameter(readonly = true, defaultValue = "${project}")
private MavenProject project;


public void execute() throws MojoExecutionException {

ArrayList<File> sources = new ArrayList<>();
List<File> sources = new ArrayList<>();

if (!skipSources) {
sources.addAll(sourceDirectories);
Expand All @@ -47,7 +56,16 @@ public void execute() throws MojoExecutionException {
}
if (sources.size() > 0) {
try {
Summary result = ScalaFormatter.apply(configLocation,getLog(),respectVersion, validateOnly).format(sources);

Summary result = ScalaFormatter.apply(
configLocation,
getLog(),
respectVersion,
validateOnly,
onlyChangedFiles,
branch,
project.getBasedir()
).format(sources);
getLog().info(result.toString());
if (validateOnly && result.unformattedFiles() != 0) {
throw new MojoExecutionException("Scalafmt: Unformatted files found");
Expand Down
35 changes: 30 additions & 5 deletions src/main/scala/org/antipathy/mvn_scalafmt/ScalaFormatter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.antipathy.mvn_scalafmt.logging.MavenLogReporter
import org.antipathy.mvn_scalafmt.model.{FormatResult, Summary}
import org.apache.maven.plugin.logging.Log
import org.scalafmt.interfaces.Scalafmt
import org.antipathy.mvn_scalafmt.builder.ChangedFilesBuilder

import scala.jdk.CollectionConverters._

Expand All @@ -18,6 +19,7 @@ import scala.jdk.CollectionConverters._
*/
class ScalaFormatter(
sourceBuilder: Builder[Seq[File], Seq[File]],
changedFilesBuilder: Builder[Seq[File], Seq[File]],
fileFormatter: Formatter[File, FormatResult],
writer: Writer[Seq[FormatResult], Summary]
) extends Formatter[JList[File], Summary] {
Expand All @@ -29,26 +31,49 @@ class ScalaFormatter(
*/
override def format(sourceDirectories: JList[File]): Summary = {
val sources = sourceBuilder.build(sourceDirectories.asScala.toSeq)
val formattedSources = sources.map(fileFormatter.format)
val sourcesToFormat = changedFilesBuilder.build(sources)
val formattedSources = sourcesToFormat.map(fileFormatter.format)
writer.write(formattedSources)
}
}

object ScalaFormatter {

def apply(configLocation: String, log: Log, respectVersion: Boolean, testOnly: Boolean): ScalaFormatter = {
val config = LocalConfigBuilder(log).build(configLocation)
val sourceBuilder = new SourceFileSequenceBuilder(log)
/**
* Create a new ScalaFormatter instance
* @param configLocation The location of the scalafmt.conf
* @param log The maven logger
* @param respectVersion should we respect the version in the scalafmt.conf
* @param testOnly should files be reformatted
* @param onlyChangedFiles Should only changed files be formatted
* @param branch The branch to compare against for changed files
* @param workingDirectory The project working directory
* @return a new ScalaFormatter instance
*/
def apply(
configLocation: String,
log: Log,
respectVersion: Boolean,
testOnly: Boolean,
onlyChangedFiles: Boolean,
branch: String,
workingDirectory: File
): ScalaFormatter = {
val config = LocalConfigBuilder(log).build(configLocation)
val sourceBuilder = new SourceFileSequenceBuilder(log)
val changedFilesBuilder = ChangedFilesBuilder(log, onlyChangedFiles, branch, workingDirectory)

val scalafmt = Scalafmt
.create(this.getClass.getClassLoader)
.withReporter(new MavenLogReporter(log))
.withRespectVersion(respectVersion)
val sourceFormatter = new SourceFileFormatter(config, scalafmt, log)

val fileWriter = if (testOnly) {
new TestResultLogWriter(log)
} else {
new FormattedFilesWriter(log)
}
new ScalaFormatter(sourceBuilder, sourceFormatter, fileWriter)
new ScalaFormatter(sourceBuilder, changedFilesBuilder, sourceFormatter, fileWriter)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package org.antipathy.mvn_scalafmt.builder

import java.io.File
import org.apache.maven.plugin.logging.Log
import scala.sys.process.ProcessLogger
import scala.util.{Try, Success, Failure}

/**
* Class for building a list of files that have changed from a specified git branch
* @param log The maven logger
* @param diff Should only changed files be returned
* @param branch the git branch to compare against
* @param changeFunction Function to identify changed files
*/
class ChangedFilesBuilder(log: Log, diff: Boolean, branch: String, changeFunction: () => String)
extends Builder[Seq[File], Seq[File]] {

/**
* Build a list of files that have changed in git from the specified input
*
* @param input The input to build from
* @return A list of changed files.
*/
override def build(input: Seq[File]): Seq[File] =
if (diff) {
log.info(s"Checking for files changed from $branch")
Try {
val names: Seq[String] =
Predef.augmentString(changeFunction()).linesIterator.toSeq
val changedFiles = names.map(new File(_).getAbsolutePath)
changedFiles.foreach { f =>
log.info(s"Changed from $branch: $f")
}
changedFiles.map(new File(_)).filter { f =>
val path = f.getAbsolutePath
path.endsWith("scala") ||
path.endsWith("sc") ||
path.endsWith("sbt")

}
} match {
case Success(value) => value
case Failure(e) =>
log.error("Could not obtain list of changed files", e)
throw e
}
} else {
input
}

}

object ChangedFilesBuilder {

def apply(log: Log, diff: Boolean, branch: String, workingDirectory: File): ChangedFilesBuilder = {

def command(branch: String): String = s"git diff --name-only --diff-filter=d $branch"
val logger: ProcessLogger = sys.process.ProcessLogger(_ => (), err => log.error(err))
def processFunction: () => String = () => {
sys.process.Process(command(branch), workingDirectory).!!(logger)
}
new ChangedFilesBuilder(log, diff, branch, processFunction)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ class SourceFileSequenceBuilder(log: Log) extends Builder[Seq[File], Seq[File]]
None
}
}

files.flatMap(file => FileUtils.listFiles(file, Array("scala", "sc", "sbt"), true).asScala)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class FormattedFilesWriter(log: Log) extends Writer[Seq[FormatResult], Summary]
Summary(
input.length,
unformattedFiles.length,
build(FileSummaryRequest(input, "Correctly formatted: ", "Reformatted: "))
build(FileSummaryRequest(input, "Correctly formatted", "Reformatted"))
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ package org.antipathy.mvn_scalafmt.model
*/
case class FileSummary(name: String, details: String) {

override def toString: String = s"$name: $details"
override def toString: String = s"$details: $name"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package org.antipathy.mvn_scalafmt.builder

import org.scalatest.{FlatSpec, GivenWhenThen, Matchers}
import java.io.File
import org.apache.maven.plugin.logging.SystemStreamLog

class ChangedFilesBuilderSpec extends FlatSpec with GivenWhenThen with Matchers {

behavior of "ChangedFilesBuilder"

it should "Identify files that have changed from master" in {
val log = new SystemStreamLog
val sourceDirs = Seq("src/test/scala", "src/main/scala").map(new File(_))
val sources = new SourceFileSequenceBuilder(log).build(sourceDirs)
val changedFiles = Seq(
"/mvn_scalafmt/src/main/scala/org/antipathy/mvn_scalafmt/builder/ChangedFilesBuilder.scala",
"/mvn_scalafmt/src/main/scala/org/antipathy/mvn_scalafmt/builder/SourceFileSequenceBuilder.scala",
"/mvn_scalafmt/src/test/scala/org/antipathy/mvn_scalafmt/builder/ChangedFilesBuilderSpec.scala",
"/mvn_scalafmt/src/test/scala/org/antipathy/mvn_scalafmt/builder/LocalConfigBuilderSpec.scala"
)

val changeFunction = () => changedFiles.mkString(System.lineSeparator())

val result = new ChangedFilesBuilder(log, true, "master", changeFunction).build(sources)
result should be(changedFiles.map(new File(_)))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ class LocalConfigBuilderSpec extends FlatSpec with GivenWhenThen with Matchers {

builder.build(path)

val result = scala.io.Source.fromFile(new File(".scalafmt.conf")).getLines().mkString(System.lineSeparator())

val source = scala.io.Source.fromFile(new File(".scalafmt.conf"))
val result = source.getLines().mkString(System.lineSeparator())
source.close()
result.trim should be(expectedContent.trim)
}

Expand Down

0 comments on commit 3260add

Please sign in to comment.