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

Convert scalafmt integration to use a compile-only sourceset #1283

Merged
Merged
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion lib/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ def NEEDS_GLUE = [
'ktfmt',
'ktlint',
'flexmark',
'diktat'
'diktat',
'scalafmt'
]
for (glue in NEEDS_GLUE) {
sourceSets.register(glue) {
Expand Down Expand Up @@ -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"

Expand Down
93 changes: 10 additions & 83 deletions lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 DiffPlug
* Copyright 2016-2022 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -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;

Expand All @@ -39,23 +34,17 @@ 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";
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
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:";
mdedetrich marked this conversation as resolved.
Show resolved Hide resolved

public static FormatterStep create(Provisioner provisioner) {
return create(defaultVersion(), provisioner, null);
}

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);
}

Expand All @@ -69,78 +58,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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright 2022 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.diffplug.spotless.glue.scalafmt;

import java.io.File;
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;

import org.scalafmt.Scalafmt;
import org.scalafmt.config.ScalafmtConfig;
import org.scalafmt.config.ScalafmtConfig$;

import com.diffplug.spotless.FileSignature;
import com.diffplug.spotless.FormatterFunc;

import scala.collection.immutable.Set$;

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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One way around this may be that instead of defining ScalafmtFormatterFunc as a Java source file, instead we can define it as a Scala source file (which with a basic class Java has no issues calling). Not sure if this is possible or even desirable

Copy link
Member

Choose a reason for hiding this comment

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

I'd be okay putting Scala into our codebase if it was really useful for some task, but this is small enough that I'd prefer the reflection. Note the little refactor I did in a75877c.

// 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();
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
}
}

This file was deleted.

This file was deleted.

16 changes: 0 additions & 16 deletions testlib/src/main/resources/scala/scalafmt/basic.clean_1.1.0

This file was deleted.

18 changes: 0 additions & 18 deletions testlib/src/main/resources/scala/scalafmt/basic.clean_2.0.1

This file was deleted.

18 changes: 0 additions & 18 deletions testlib/src/main/resources/scala/scalafmt/basicPost2.0.0.clean

This file was deleted.

This file was deleted.

2 changes: 2 additions & 0 deletions testlib/src/main/resources/scala/scalafmt/scalafmt.conf
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
version = 3.5.9
runner.dialect = scala213
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
style = defaultWithAlign # For pretty alignment.
maxColumn = 20 # For my teensy narrow display
Loading