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
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ 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]
### Added
* `scalafmt` integration now has a configuration option `majorScalaVersion` that allows you to configure the Scala version that gets resolved from the maven artifact ([#1283](https://github.com/diffplug/spotless/pull/1283))
* Converted `scalafmt` integration to use a compile-only source set (fixes [#524](https://github.com/diffplug/spotless/issues/524))
### Changes
* Add the `ktlint` rule in error messages when `ktlint` fails to apply a fix ([#1279](https://github.com/diffplug/spotless/pull/1279))
* Bump default `scalafmt` to latest `3.0.8` -> `3.5.9` (removed support for pre-`3.0.0`) ([#1283](https://github.com/diffplug/spotless/pull/1283))

## [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
109 changes: 24 additions & 85 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,108 +34,52 @@ 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

private static final String DEFAULT_SCALA_MAJOR_VERSION = "2.13";
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:";
static final String MAVEN_COORDINATE = "org.scalameta:scalafmt-core_";

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

public static FormatterStep create(String version, Provisioner provisioner, @Nullable File configFile) {
Objects.requireNonNull(version, "version");
Objects.requireNonNull(provisioner, "provisioner");
return create(version, defaultScalaMajorVersion(), provisioner, configFile);
}

public static FormatterStep create(String version, @Nullable String scalaMajorVersion, Provisioner provisioner, @Nullable File configFile) {
String finalScalaMajorVersion = scalaMajorVersion == null ? DEFAULT_SCALA_MAJOR_VERSION : scalaMajorVersion;

return FormatterStep.createLazy(NAME,
() -> new State(version, provisioner, configFile),
() -> new State(JarState.from(MAVEN_COORDINATE + finalScalaMajorVersion + ":" + version, provisioner), configFile),
State::createFormat);
}

public static String defaultVersion() {
return DEFAULT_VERSION;
}

public static String defaultScalaMajorVersion() {
return DEFAULT_SCALA_MAJOR_VERSION;
}

static final class State implements Serializable {
private static final long serialVersionUID = 1L;

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,54 @@
/*
* 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 ScalafmtConfig config;

public ScalafmtFormatterFunc(FileSignature configSignature) throws Exception {
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();
}
}

@Override
public String apply(String input) {
return Scalafmt.format(input, config, Set$.MODULE$.empty()).get();
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
}
}
3 changes: 3 additions & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).

## [Unreleased]
### Added
* `scalafmt` integration now has a configuration option `majorScalaVersion` that allows you to configure the Scala version that gets resolved from the maven artifact ([#1283](https://github.com/diffplug/spotless/pull/1283))
### Changes
* Add the `ktlint` rule in error messages when `ktlint` fails to apply a fix ([#1279](https://github.com/diffplug/spotless/pull/1279))
* Bump default `scalafmt` to latest `3.0.8` -> `3.5.9` (removed support for pre-`3.0.0`) ([#1283](https://github.com/diffplug/spotless/pull/1283))

## [6.9.1] - 2022-08-10
### Fixed
Expand Down
4 changes: 2 additions & 2 deletions plugin-gradle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,8 @@ spotless {
```gradle
spotless {
scala {
// version and configFile are both optional
scalafmt('2.6.1').configFile('scalafmt.conf')
// version and configFile, majorScalaVersion are all optional
scalafmt('3.5.9').configFile('scalafmt.conf').majorScalaVersion('2.13')
```

<a name="applying-to-cc-sources"></a>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2020 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 Down Expand Up @@ -48,21 +48,30 @@ public ScalaFmtConfig scalafmt(String version) {
public class ScalaFmtConfig {
final String version;
@Nullable
String scalaMajorVersion;
@Nullable
Object configFile;

ScalaFmtConfig(String version) {
this.version = Objects.requireNonNull(version);
addStep(createStep());
}

public void configFile(Object configFile) {
public ScalaFmtConfig configFile(Object configFile) {
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
this.configFile = Objects.requireNonNull(configFile);
replaceStep(createStep());
return this;
}

public ScalaFmtConfig scalaMajorVersion(String scalaMajorVersion) {
this.scalaMajorVersion = Objects.requireNonNull(scalaMajorVersion);
replaceStep(createStep());
return this;
}

private FormatterStep createStep() {
File resolvedConfigFile = configFile == null ? null : getProject().file(configFile);
return ScalaFmtStep.create(version, provisioner(), resolvedConfigFile);
return ScalaFmtStep.create(version, scalaMajorVersion, provisioner(), resolvedConfigFile);
}
}

Expand Down
3 changes: 3 additions & 0 deletions plugin-maven/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).

## [Unreleased]
### Added
* `scalafmt` integration now has a configuration option `majorScalaVersion` that allows you to configure the Scala version that gets resolved from the maven artifact ([#1283](https://github.com/diffplug/spotless/pull/1283))
### Changes
* Add the `ktlint` rule in error messages when `ktlint` fails to apply a fix ([#1279](https://github.com/diffplug/spotless/pull/1279))
* Bump default `scalafmt` to latest `3.0.8` -> `3.5.9` (removed support for pre-`3.0.0`) ([#1283](https://github.com/diffplug/spotless/pull/1283))

## [2.24.1] - 2022-08-10
### Fixed
Expand Down
3 changes: 2 additions & 1 deletion plugin-maven/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,9 @@ Groovy-Eclipse formatting errors/warnings lead per default to a build failure. T

```xml
<scalafmt>
<version>2.0.1</version> <!-- optional -->
<version>3.5.9</version> <!-- optional -->
<file>${project.basedir}/scalafmt.conf</file> <!-- optional -->
<majorScalaVersion>2.13</majorScalaVersion> <!-- optional -->
</scalafmt>
```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016 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 Down Expand Up @@ -32,10 +32,14 @@ public class Scalafmt implements FormatterStepFactory {
@Parameter
private String version;

@Parameter
private String scalaMajorVersion;

@Override
public FormatterStep newFormatterStep(FormatterStepConfig config) {
String scalafmtVersion = version != null ? version : ScalaFmtStep.defaultVersion();
String scalafmtScalaMajorVersion = scalaMajorVersion != null ? scalaMajorVersion : ScalaFmtStep.defaultScalaMajorVersion();
File configFile = config.getFileLocator().locateFile(file);
return ScalaFmtStep.create(scalafmtVersion, config.getProvisioner(), configFile);
return ScalaFmtStep.create(scalafmtVersion, scalafmtScalaMajorVersion, config.getProvisioner(), configFile);
}
}

This file was deleted.

Loading