diff --git a/CHANGES.md b/CHANGES.md index 751749585f..446698d7ed 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,7 @@ This document is intended for Spotless developers. We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +* Converted `scalafmt` integration to use a compile-only source set. (fixes [#524](https://github.com/diffplug/spotless/issues/524)) ## [2.28.1] - 2022-08-10 ### Fixed diff --git a/lib/build.gradle b/lib/build.gradle index e14e5c6438..0b3f4555b2 100644 --- a/lib/build.gradle +++ b/lib/build.gradle @@ -12,7 +12,8 @@ def NEEDS_GLUE = [ 'ktfmt', 'ktlint', 'flexmark', - 'diktat' + 'diktat', + 'scalafmt' ] for (glue in NEEDS_GLUE) { sourceSets.register(glue) { @@ -51,6 +52,9 @@ dependencies { ktlintCompileOnly "com.pinterest.ktlint:ktlint-ruleset-experimental:$VER_KTLINT" ktlintCompileOnly "com.pinterest.ktlint:ktlint-ruleset-standard:$VER_KTLINT" + String VER_SCALAFMT="3.5.9" + scalafmtCompileOnly "org.scalameta:scalafmt-core_2.13:$VER_SCALAFMT" + String VER_DIKTAT = "1.2.3" diktatCompileOnly "org.cqfn.diktat:diktat-rules:$VER_DIKTAT" diff --git a/lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java b/lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java index 8c8e4ccb2d..8f96e1bfae 100644 --- a/lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java +++ b/lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java @@ -18,13 +18,8 @@ import java.io.File; import java.io.IOException; import java.io.Serializable; -import java.lang.reflect.Method; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; +import java.lang.reflect.Constructor; import java.util.Collections; -import java.util.Objects; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.annotation.Nullable; @@ -38,13 +33,8 @@ public class ScalaFmtStep { // prevent direct instantiation private ScalaFmtStep() {} - - private static final Pattern VERSION_PRE_2_0 = Pattern.compile("[10]\\.(\\d+)\\.\\d+"); - private static final Pattern VERSION_PRE_3_0 = Pattern.compile("2\\.(\\d+)\\.\\d+"); - private static final String DEFAULT_VERSION = "3.0.8"; + private static final String DEFAULT_VERSION = "3.5.9"; static final String NAME = "scalafmt"; - static final String MAVEN_COORDINATE_PRE_2_0 = "com.geirsson:scalafmt-core_2.11:"; - static final String MAVEN_COORDINATE_PRE_3_0 = "org.scalameta:scalafmt-core_2.11:"; static final String MAVEN_COORDINATE = "org.scalameta:scalafmt-core_2.13:"; public static FormatterStep create(Provisioner provisioner) { @@ -52,10 +42,8 @@ public static FormatterStep create(Provisioner provisioner) { } public static FormatterStep create(String version, Provisioner provisioner, @Nullable File configFile) { - Objects.requireNonNull(version, "version"); - Objects.requireNonNull(provisioner, "provisioner"); return FormatterStep.createLazy(NAME, - () -> new State(version, provisioner, configFile), + () -> new State(JarState.from(MAVEN_COORDINATE + version, provisioner), configFile), State::createFormat); } @@ -69,78 +57,16 @@ static final class State implements Serializable { final JarState jarState; final FileSignature configSignature; - State(String version, Provisioner provisioner, @Nullable File configFile) throws IOException { - String mavenCoordinate; - Matcher versionMatcher; - if ((versionMatcher = VERSION_PRE_2_0.matcher(version)).matches()) { - mavenCoordinate = MAVEN_COORDINATE_PRE_2_0; - } else if ((versionMatcher = VERSION_PRE_3_0.matcher(version)).matches()) { - mavenCoordinate = MAVEN_COORDINATE_PRE_3_0; - } else { - mavenCoordinate = MAVEN_COORDINATE; - } - - this.jarState = JarState.from(mavenCoordinate + version, provisioner); + State(JarState jarState, @Nullable File configFile) throws IOException { + this.jarState = jarState; this.configSignature = FileSignature.signAsList(configFile == null ? Collections.emptySet() : Collections.singleton(configFile)); } FormatterFunc createFormat() throws Exception { - ClassLoader classLoader = jarState.getClassLoader(); - - // scalafmt returns instances of formatted, we get result by calling get() - Class formatted = classLoader.loadClass("org.scalafmt.Formatted"); - Method formattedGet = formatted.getMethod("get"); - - // this is how we actually do a format - Class scalafmt = classLoader.loadClass("org.scalafmt.Scalafmt"); - Class scalaSet = classLoader.loadClass("scala.collection.immutable.Set"); - - Object defaultScalaFmtConfig = scalafmt.getMethod("format$default$2").invoke(null); - Object emptyRange = scalafmt.getMethod("format$default$3").invoke(null); - Method formatMethod = scalafmt.getMethod("format", String.class, defaultScalaFmtConfig.getClass(), scalaSet); - - // now we just need to parse the config, if any - Object config; - if (configSignature.files().isEmpty()) { - config = defaultScalaFmtConfig; - } else { - File file = configSignature.getOnlyFile(); - - Class optionCls = classLoader.loadClass("scala.Option"); - Class configCls = classLoader.loadClass("org.scalafmt.config.Config"); - Class scalafmtCls = classLoader.loadClass("org.scalafmt.Scalafmt"); - - Object configured; - - try { - // scalafmt >= 1.6.0 - Method parseHoconConfig = scalafmtCls.getMethod("parseHoconConfig", String.class); - - String configStr = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8); - - configured = parseHoconConfig.invoke(null, configStr); - } catch (NoSuchMethodException e) { - // scalafmt >= v0.7.0-RC1 && scalafmt < 1.6.0 - Method fromHocon = configCls.getMethod("fromHoconString", String.class, optionCls); - Object fromHoconEmptyPath = configCls.getMethod("fromHoconString$default$2").invoke(null); - - String configStr = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8); - - configured = fromHocon.invoke(null, configStr, fromHoconEmptyPath); - } - - config = invokeNoArg(configured, "get"); - } - return input -> { - Object resultInsideFormatted = formatMethod.invoke(null, input, config, emptyRange); - return (String) formattedGet.invoke(resultInsideFormatted); - }; + final ClassLoader classLoader = jarState.getClassLoader(); + final Class formatterFunc = classLoader.loadClass("com.diffplug.spotless.glue.scalafmt.ScalafmtFormatterFunc"); + final Constructor constructor = formatterFunc.getConstructor(FileSignature.class); + return (FormatterFunc) constructor.newInstance(this.configSignature); } } - - private static Object invokeNoArg(Object obj, String toInvoke) throws Exception { - Class clazz = obj.getClass(); - Method method = clazz.getMethod(toInvoke); - return method.invoke(obj); - } } diff --git a/lib/src/scalafmt/java/com/diffplug/spotless/glue/scalafmt/ScalafmtFormatterFunc.java b/lib/src/scalafmt/java/com/diffplug/spotless/glue/scalafmt/ScalafmtFormatterFunc.java new file mode 100644 index 0000000000..17040c790b --- /dev/null +++ b/lib/src/scalafmt/java/com/diffplug/spotless/glue/scalafmt/ScalafmtFormatterFunc.java @@ -0,0 +1,41 @@ +package com.diffplug.spotless.glue.scalafmt; + +import com.diffplug.spotless.FileSignature; +import com.diffplug.spotless.FormatterFunc; + +import org.scalafmt.Scalafmt; +import org.scalafmt.config.ScalafmtConfig; +import org.scalafmt.config.ScalafmtConfig$; + +import scala.collection.immutable.Set$; + +import java.io.File; +import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; + +public class ScalafmtFormatterFunc implements FormatterFunc { + private final FileSignature configSignature; + + public ScalafmtFormatterFunc(FileSignature configSignature) { + this.configSignature = configSignature; + } + + @Override + public String apply(String input) throws Exception { + ScalafmtConfig config; + if (configSignature.files().isEmpty()) { + // Note that reflection is used here only because Scalafmt has a method called + // default which happens to be a reserved Java keyword. The only way to call + // such methods is by reflection, see + // https://vlkan.com/blog/post/2015/11/20/scala-method-with-java-reserved-keyword/ + Method method = ScalafmtConfig$.MODULE$.getClass().getDeclaredMethod("default"); + config = (ScalafmtConfig) method.invoke(ScalafmtConfig$.MODULE$); + } else { + File file = configSignature.getOnlyFile(); + String configStr = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8); + config = Scalafmt.parseHoconConfig(configStr).get(); + } + return Scalafmt.format(input, config, Set$.MODULE$.empty()).get(); + } +} diff --git a/testlib/src/main/resources/scala/scalafmt/basic.cleanWithCustomConf_1.1.0 b/testlib/src/main/resources/scala/scalafmt/basic.cleanWithCustomConf_1.1.0 deleted file mode 100644 index 98bf69b7af..0000000000 --- a/testlib/src/main/resources/scala/scalafmt/basic.cleanWithCustomConf_1.1.0 +++ /dev/null @@ -1,24 +0,0 @@ -@foobar("annot", { - val x = 2 - val y = 2 // y=2 - x + y -}) -object a - extends b - with c { - def foo[ - T: Int#Double#Triple, - R <% String]( - @annot1 - x: Int @annot2 = - 2, - y: Int = 3) - : Int = { - "match" match { - case 1 | 2 => - 3 - case 2 => - 2 - } - } -} diff --git a/testlib/src/main/resources/scala/scalafmt/basic.cleanWithCustomConf_2.0.1 b/testlib/src/main/resources/scala/scalafmt/basic.cleanWithCustomConf_2.0.1 deleted file mode 100644 index fc8267fb75..0000000000 --- a/testlib/src/main/resources/scala/scalafmt/basic.cleanWithCustomConf_2.0.1 +++ /dev/null @@ -1,25 +0,0 @@ -@foobar("annot", { - val x = 2 - val y = 2 // y=2 - x + y -}) -object a - extends b - with c { - def foo[ - T: Int#Double#Triple, - R <% String - ]( - @annot1 - x: Int @annot2 = - 2, - y: Int = 3 - ): Int = { - "match" match { - case 1 | 2 => - 3 - case 2 => - 2 - } - } -} diff --git a/testlib/src/main/resources/scala/scalafmt/basic.clean_1.1.0 b/testlib/src/main/resources/scala/scalafmt/basic.clean_1.1.0 deleted file mode 100644 index 922a2ccbb9..0000000000 --- a/testlib/src/main/resources/scala/scalafmt/basic.clean_1.1.0 +++ /dev/null @@ -1,16 +0,0 @@ -@foobar("annot", { - val x = 2 - val y = 2 // y=2 - x + y -}) -object a extends b with c { - def foo[T: Int#Double#Triple, R <% String](@annot1 - x: Int @annot2 = 2, - y: Int = 3): Int = { - "match" match { - case 1 | 2 => - 3 - case 2 => 2 - } - } -} diff --git a/testlib/src/main/resources/scala/scalafmt/basic.clean_2.0.1 b/testlib/src/main/resources/scala/scalafmt/basic.clean_2.0.1 deleted file mode 100644 index b838dfea65..0000000000 --- a/testlib/src/main/resources/scala/scalafmt/basic.clean_2.0.1 +++ /dev/null @@ -1,18 +0,0 @@ -@foobar("annot", { - val x = 2 - val y = 2 // y=2 - x + y -}) -object a extends b with c { - def foo[T: Int#Double#Triple, R <% String]( - @annot1 - x: Int @annot2 = 2, - y: Int = 3 - ): Int = { - "match" match { - case 1 | 2 => - 3 - case 2 => 2 - } - } -} diff --git a/testlib/src/main/resources/scala/scalafmt/basicPost2.0.0.clean b/testlib/src/main/resources/scala/scalafmt/basicPost2.0.0.clean deleted file mode 100644 index b838dfea65..0000000000 --- a/testlib/src/main/resources/scala/scalafmt/basicPost2.0.0.clean +++ /dev/null @@ -1,18 +0,0 @@ -@foobar("annot", { - val x = 2 - val y = 2 // y=2 - x + y -}) -object a extends b with c { - def foo[T: Int#Double#Triple, R <% String]( - @annot1 - x: Int @annot2 = 2, - y: Int = 3 - ): Int = { - "match" match { - case 1 | 2 => - 3 - case 2 => 2 - } - } -} diff --git a/testlib/src/main/resources/scala/scalafmt/basicPost2.0.0.cleanWithCustomConf b/testlib/src/main/resources/scala/scalafmt/basicPost2.0.0.cleanWithCustomConf deleted file mode 100644 index fc8267fb75..0000000000 --- a/testlib/src/main/resources/scala/scalafmt/basicPost2.0.0.cleanWithCustomConf +++ /dev/null @@ -1,25 +0,0 @@ -@foobar("annot", { - val x = 2 - val y = 2 // y=2 - x + y -}) -object a - extends b - with c { - def foo[ - T: Int#Double#Triple, - R <% String - ]( - @annot1 - x: Int @annot2 = - 2, - y: Int = 3 - ): Int = { - "match" match { - case 1 | 2 => - 3 - case 2 => - 2 - } - } -} diff --git a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java index 0f42252d36..ce850bade9 100644 --- a/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/scala/ScalaFmtStepTest.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; import java.lang.reflect.InvocationTargetException; +import java.util.NoSuchElementException; import org.junit.jupiter.api.Test; @@ -34,38 +35,16 @@ class ScalaFmtStepTest extends ResourceHarness { @Test void behaviorDefaultConfig() throws Exception { - StepHarness.forStep(ScalaFmtStep.create("1.1.0", TestProvisioner.mavenCentral(), null)) - .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basic.clean_1.1.0"); - StepHarness.forStep(ScalaFmtStep.create("2.0.1", TestProvisioner.mavenCentral(), null)) - .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basic.clean_2.0.1"); StepHarness.forStep(ScalaFmtStep.create("3.0.0", TestProvisioner.mavenCentral(), null)) .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basic.clean_3.0.0"); } @Test void behaviorCustomConfig() throws Exception { - StepHarness.forStep(ScalaFmtStep.create("1.1.0", TestProvisioner.mavenCentral(), createTestFile("scala/scalafmt/scalafmt.conf"))) - .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basic.cleanWithCustomConf_1.1.0"); - StepHarness.forStep(ScalaFmtStep.create("2.0.1", TestProvisioner.mavenCentral(), createTestFile("scala/scalafmt/scalafmt.conf"))) - .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basic.cleanWithCustomConf_2.0.1"); StepHarness.forStep(ScalaFmtStep.create("3.0.0", TestProvisioner.mavenCentral(), createTestFile("scala/scalafmt/scalafmt.conf"))) .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basic.cleanWithCustomConf_3.0.0"); } - @Test - void behaviorDefaultConfigVersion_2_0_0() throws Exception { - FormatterStep step = ScalaFmtStep.create("2.0.0", TestProvisioner.mavenCentral(), null); - StepHarness.forStep(step) - .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basicPost2.0.0.clean"); - } - - @Test - void behaviorCustomConfigVersion_2_0_0() throws Exception { - FormatterStep step = ScalaFmtStep.create("2.0.0", TestProvisioner.mavenCentral(), createTestFile("scala/scalafmt/scalafmt.conf")); - StepHarness.forStep(step) - .testResource("scala/scalafmt/basic.dirty", "scala/scalafmt/basicPost2.0.0.cleanWithCustomConf"); - } - @Test void behaviorDefaultConfigVersion_3_0_0() throws Exception { FormatterStep step = ScalaFmtStep.create("3.0.0", TestProvisioner.mavenCentral(), null); @@ -83,7 +62,7 @@ void behaviorCustomConfigVersion_3_0_0() throws Exception { @Test void equality() throws Exception { new SerializableEqualityTester() { - String version = "0.5.1"; + String version = "3.5.9"; File configFile = null; @Override @@ -91,7 +70,7 @@ protected void setupTest(API api) throws IOException { // same version == same api.areDifferentThan(); // change the version, and it's different - version = "0.5.0"; + version = "3.5.8"; api.areDifferentThan(); // add a config file, and its different configFile = createTestFile("scala/scalafmt/scalafmt.conf"); @@ -113,18 +92,10 @@ void invalidConfiguration() throws Exception { File invalidConfFile = createTestFile("scala/scalafmt/scalafmt.invalid.conf"); Provisioner provisioner = TestProvisioner.mavenCentral(); - InvocationTargetException exception; - - exception = assertThrows(InvocationTargetException.class, - () -> StepHarness.forStep(ScalaFmtStep.create("1.1.0", provisioner, invalidConfFile)).test("", "")); - assertThat(exception.getTargetException().getMessage()).contains("Invalid fields: invalidScalaFmtConfigField"); - - exception = assertThrows(InvocationTargetException.class, - () -> StepHarness.forStep(ScalaFmtStep.create("2.0.1", provisioner, invalidConfFile)).test("", "")); - assertThat(exception.getTargetException().getMessage()).contains("Invalid field: invalidScalaFmtConfigField"); + NoSuchElementException exception; - exception = assertThrows(InvocationTargetException.class, + exception = assertThrows(NoSuchElementException.class, () -> StepHarness.forStep(ScalaFmtStep.create("3.0.0", provisioner, invalidConfFile)).test("", "")); - assertThat(exception.getTargetException().getMessage()).contains("found option 'invalidScalaFmtConfigField' which wasn't expected"); + assertThat(exception.getMessage()).contains("found option 'invalidScalaFmtConfigField' which wasn't expected"); } }