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

Fix ktlint 0.48 #1456

Merged
merged 13 commits into from
Jan 8, 2023
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ 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]
### Fixed
* Support `ktlint` 0.48+ new rule disabling syntax ([#1456](https://github.com/diffplug/spotless/pull/1456)) fixes ([#1444](https://github.com/diffplug/spotless/issues/1444))
### Changes
* Bump the dev version of Gradle from `7.5.1` to `7.6` ([#1409](https://github.com/diffplug/spotless/pull/1409))
* We also removed the no-longer-required dependency `org.codehaus.groovy:groovy-xml`
Expand All @@ -18,6 +20,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
* `StepHarness` now operates on `Formatter` rather than a `FormatterStep`
* `StepHarnessWithFile` now takes a `ResourceHarness` in its constructor to handle the file manipulation parts
* Standardized that we test exception *messages*, not types, which will ease the transition to linting later on
* Bump default `ktlint` version to latest `0.47.1` -> `0.48.1` ([#1456](https://github.com/diffplug/spotless/pull/1456))

## [2.31.1] - 2023-01-02
### Fixed
Expand Down
2 changes: 2 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# To fix metaspace errors
org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=1024m -Dfile.encoding=UTF-8
org.gradle.parallel=true
org.gradle.caching=true
name=spotless
description=Spotless - keep your code spotless with Gradle
org=diffplug
Expand Down
16 changes: 11 additions & 5 deletions lib/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ versionCompatibility {
'0.45.2',
'0.46.0',
'0.47.0',
'0.48.0',
'0.48.1',
Copy link
Contributor

@davidburstrom davidburstrom Jan 6, 2023

Choose a reason for hiding this comment

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

If 0.48.0 and 0.48.1 are binary compatible (and as far as I can see from https://github.com/pinterest/ktlint/releases they are), I'd recommend that you keep the adapter version set to 0.48.0. Otherwise it appears as if the same adapter should be used for versions 0.47.x and 0.48.0 inclusive (see manual for version-compatibility-gradle-plugin). In the test works0_48_1 you would still verify that disabling works. If 0.48.0 is truly not supported, then it needs to be reflected in the code that selects the adapter.

Copy link
Contributor Author

@Sineaggi Sineaggi Jan 6, 2023

Choose a reason for hiding this comment

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

They are compatible, I've undone the renaming change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool!

]
targetSourceSetName = 'ktlint'
}
Expand Down Expand Up @@ -91,9 +91,9 @@ dependencies {
compatKtLint0Dot47Dot0CompileOnly 'com.pinterest.ktlint:ktlint-core:0.47.0'
compatKtLint0Dot47Dot0CompileOnly 'com.pinterest.ktlint:ktlint-ruleset-experimental:0.47.0'
compatKtLint0Dot47Dot0CompileOnly 'com.pinterest.ktlint:ktlint-ruleset-standard:0.47.0'
compatKtLint0Dot48Dot0CompileOnly 'com.pinterest.ktlint:ktlint-core:0.48.0'
compatKtLint0Dot48Dot0CompileOnly 'com.pinterest.ktlint:ktlint-ruleset-experimental:0.48.0'
compatKtLint0Dot48Dot0CompileOnly 'com.pinterest.ktlint:ktlint-ruleset-standard:0.48.0'
compatKtLint0Dot48Dot1CompileOnly 'com.pinterest.ktlint:ktlint-core:0.48.1'
compatKtLint0Dot48Dot1CompileOnly 'com.pinterest.ktlint:ktlint-ruleset-experimental:0.48.1'
compatKtLint0Dot48Dot1CompileOnly 'com.pinterest.ktlint:ktlint-ruleset-standard:0.48.1'

String VER_SCALAFMT="3.6.1"
scalafmtCompileOnly "org.scalameta:scalafmt-core_2.13:$VER_SCALAFMT"
Expand All @@ -105,10 +105,16 @@ dependencies {
flexmarkCompileOnly 'com.vladsch.flexmark:flexmark-all:0.62.2'
}

configurations.named('testCompatKtLint0Dot48Dot1Implementation').configure {
extendsFrom(configurations.testImplementation, configurations.compatKtLint0Dot48Dot1CompileOnly)
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I can see, you should be able to declare the testImplementation dependencies as testCommonImplementation (https://github.com/davidburstrom/version-compatibility-gradle-plugin#compatibility-adapter-test-suites) so that they will be exported automatically to both testImplementation and testCompatKtLint0Dot48Dot1Implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding exposing the compileOnly libraries it makes perfect sense. I've had some thoughts about making the plugin export that automatically to the test implementation classpath, as it's likely that you'd want to refer to that version of the library as part of the test suite. Do you think it would make sense to automatically add a configuration called something like compatXYZSpecific that compatXYZCompileOnly and testCompatXYZImplementation would extend from? "Specific" is up for discussion.

Copy link
Contributor

@davidburstrom davidburstrom Jan 6, 2023

Choose a reason for hiding this comment

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

🤔 Having a compatXYZSpecific configuration would be quite flexible since everything is modular in nature. If you actually want a different library on the test classpath, you simply don't need to add anything to compatXYZSpecific, but rather to the downstream configurations directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Sineaggi Do you have any thoughts about my info-dump here? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davidburstrom I had missed the testCommonImplementation configuration when looking thru the docs.

Also, I like the idea of the compatXYZSpecific. compatXYZCompileOnlyAndTestImplementation maybe? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll see what I can do :) Naming is hard...

Copy link
Contributor

@davidburstrom davidburstrom Jan 7, 2023

Choose a reason for hiding this comment

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

I've published version 0.4.0 of the plugin now, with support for compatXYZCompileAndTestOnly. See https://github.com/davidburstrom/version-compatibility-gradle-plugin#compatibility-adapters.

}

// we'll hold the core lib to a high standard
spotbugs { reportLevel = 'low' } // low|medium|high (low = sensitive to even minor mistakes)

test { useJUnitPlatform() }
tasks.withType(Test).configureEach {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good (other than also being required) :)

useJUnitPlatform()
}

jar {
for (glue in NEEDS_GLUE) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package com.diffplug.spotless.glue.ktlint.compat;

import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
Expand All @@ -23,31 +26,53 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import com.pinterest.ktlint.core.KtLint;
import com.pinterest.ktlint.core.KtLintRuleEngine;
import com.pinterest.ktlint.core.LintError;
import com.pinterest.ktlint.core.Rule;
import com.pinterest.ktlint.core.RuleProvider;
import com.pinterest.ktlint.core.api.DefaultEditorConfigProperties;
import com.pinterest.ktlint.core.api.EditorConfigDefaults;
import com.pinterest.ktlint.core.api.EditorConfigOverride;
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties;
import com.pinterest.ktlint.core.api.editorconfig.CodeStyleEditorConfigPropertyKt;
import com.pinterest.ktlint.core.api.editorconfig.DisabledRulesEditorConfigPropertyKt;
import com.pinterest.ktlint.core.api.editorconfig.EditorConfigProperty;
import com.pinterest.ktlint.core.api.editorconfig.IndentSizeEditorConfigPropertyKt;
import com.pinterest.ktlint.core.api.editorconfig.IndentStyleEditorConfigPropertyKt;
import com.pinterest.ktlint.core.api.editorconfig.InsertFinalNewLineEditorConfigPropertyKt;
import com.pinterest.ktlint.core.api.editorconfig.MaxLineLengthEditorConfigPropertyKt;
import com.pinterest.ktlint.core.api.editorconfig.RuleExecutionEditorConfigPropertyKt;
import com.pinterest.ktlint.ruleset.experimental.ExperimentalRuleSetProvider;
import com.pinterest.ktlint.ruleset.standard.StandardRuleSetProvider;

import kotlin.Pair;
import kotlin.Unit;
import kotlin.jvm.functions.Function2;

public class KtLintCompat0Dot48Dot0Adapter implements KtLintCompatAdapter {
public class KtLintCompat0Dot48Dot1Adapter implements KtLintCompatAdapter {

private static final List<EditorConfigProperty<?>> DEFAULT_EDITOR_CONFIG_PROPERTIES;

static {
List<EditorConfigProperty<?>> list = new ArrayList<>();
list.add(CodeStyleEditorConfigPropertyKt.getCODE_STYLE_PROPERTY());
//noinspection deprecation
list.add(DisabledRulesEditorConfigPropertyKt.getDISABLED_RULES_PROPERTY());
//noinspection KotlinInternalInJava,deprecation
list.add(DisabledRulesEditorConfigPropertyKt.getKTLINT_DISABLED_RULES_PROPERTY());
list.add(IndentStyleEditorConfigPropertyKt.getINDENT_STYLE_PROPERTY());
list.add(IndentSizeEditorConfigPropertyKt.getINDENT_SIZE_PROPERTY());
list.add(InsertFinalNewLineEditorConfigPropertyKt.getINSERT_FINAL_NEWLINE_PROPERTY());
list.add(MaxLineLengthEditorConfigPropertyKt.getMAX_LINE_LENGTH_PROPERTY());
DEFAULT_EDITOR_CONFIG_PROPERTIES = Collections.unmodifiableList(list);
}

static class FormatterCallback implements Function2<LintError, Boolean, Unit> {
@Override
public Unit invoke(LintError lint, Boolean corrected) {
if (!corrected) {
KtLintCompatReporting.report(lint.getLine(), lint.getCol(), lint.getRuleId(), lint.getDetail());
}
return null;
return Unit.INSTANCE;
}
}

Expand All @@ -66,25 +91,18 @@ public String format(final String text, final String name, final boolean isScrip

EditorConfigOverride editorConfigOverride;
if (editorConfigOverrideMap.isEmpty()) {
editorConfigOverride = EditorConfigOverride.Companion.getEmptyEditorConfigOverride();
editorConfigOverride = EditorConfigOverride.Companion.getEMPTY_EDITOR_CONFIG_OVERRIDE();
} else {
editorConfigOverride = createEditorConfigOverride(allRuleProviders.stream().map(
RuleProvider::createNewRuleInstance).collect(
Collectors.toList()),
editorConfigOverrideMap);
}

return KtLint.INSTANCE.format(new KtLint.ExperimentalParams(
name,
text,
allRuleProviders,
userData,
Sineaggi marked this conversation as resolved.
Show resolved Hide resolved
formatterCallback,
isScript,
false,
EditorConfigDefaults.Companion.getEmptyEditorConfigDefaults(),
editorConfigOverride,
false));
EditorConfigDefaults editorConfigDefaults = EditorConfigDefaults.Companion.getEMPTY_EDITOR_CONFIG_DEFAULTS();
boolean isInvokedFromCli = false;
KtLintRuleEngine ktLintRuleEngine = new KtLintRuleEngine(allRuleProviders, editorConfigDefaults, editorConfigOverride, isInvokedFromCli);
return ktLintRuleEngine.format(text, Paths.get(name), formatterCallback);
}

/**
Expand All @@ -98,7 +116,7 @@ private static EditorConfigOverride createEditorConfigOverride(final List<Rule>

// Create a mapping of properties to their names based on rule properties and default properties
Map<String, EditorConfigProperty<?>> supportedProperties = Stream
.concat(ruleProperties, DefaultEditorConfigProperties.INSTANCE.getEditorConfigProperties().stream())
.concat(ruleProperties, DEFAULT_EDITOR_CONFIG_PROPERTIES.stream())
.distinct()
.collect(Collectors.toMap(EditorConfigProperty::getName, property -> property));

Expand All @@ -109,6 +127,18 @@ private static EditorConfigOverride createEditorConfigOverride(final List<Rule>
EditorConfigProperty<?> property = supportedProperties.get(entry.getKey());
if (property != null) {
return new Pair<>(property, entry.getValue());
} else if (entry.getKey().startsWith("ktlint_")) {
String[] parts = entry.getKey().substring(7).split("_", 2);
if (parts.length == 1) {
// convert ktlint_{ruleset} to {ruleset}
String qualifiedRuleId = parts[0];
property = RuleExecutionEditorConfigPropertyKt.createRuleSetExecutionEditorConfigProperty(qualifiedRuleId);
} else {
// convert ktlint_{ruleset}_{rulename} to {ruleset}:{rulename}
String qualifiedRuleId = parts[0] + ":" + parts[1];
property = RuleExecutionEditorConfigPropertyKt.createRuleExecutionEditorConfigProperty(qualifiedRuleId);
}
return new Pair<>(property, entry.getValue());
} else {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot45Dot2Adapter;
import com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot46Dot0Adapter;
import com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot47Dot0Adapter;
import com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot48Dot0Adapter;
import com.diffplug.spotless.glue.ktlint.compat.KtLintCompat0Dot48Dot1Adapter;
import com.diffplug.spotless.glue.ktlint.compat.KtLintCompatAdapter;

public class KtlintFormatterFunc implements FormatterFunc.NeedsFile {
Expand All @@ -44,7 +44,7 @@ public KtlintFormatterFunc(String version, boolean isScript, boolean useExperime
int minorVersion = Integer.parseInt(version.split("\\.")[1]);
if (minorVersion >= 48) {
// ExperimentalParams lost two constructor arguments, EditorConfigProperty moved to its own class
this.adapter = new KtLintCompat0Dot48Dot0Adapter();
this.adapter = new KtLintCompat0Dot48Dot1Adapter();
} else if (minorVersion == 47) {
// rename RuleSet to RuleProvider
this.adapter = new KtLintCompat0Dot47Dot0Adapter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class KtLintStep {
// prevent direct instantiation
private KtLintStep() {}

private static final String DEFAULT_VERSION = "0.48.0";
private static final String DEFAULT_VERSION = "0.48.1";
static final String NAME = "ktlint";
static final String PACKAGE_PRE_0_32 = "com.github.shyiko";
static final String PACKAGE = "com.pinterest";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package com.diffplug.spotless.glue.ktlint.compat;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Map;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class KtLintCompat0Dot48Dot1AdapterTest {
@Test
public void testDefaults(@TempDir Path path) throws IOException {
KtLintCompat0Dot48Dot1Adapter KtLintCompat0Dot48Dot1Adapter = new KtLintCompat0Dot48Dot1Adapter();
try (InputStream is = KtLintCompat0Dot48Dot1AdapterTest.class.getResourceAsStream("/empty_class_body.kt")) {
Files.copy(is, path.resolve("empty_class_body.kt"));
}
String text = new String(Files.readAllBytes(path.resolve("empty_class_body.kt")));

Map<String, String> userData = new HashMap<>();

Map<String, Object> editorConfigOverrideMap = new HashMap<>();

String formatted = KtLintCompat0Dot48Dot1Adapter.format(text, "empty_class_body.kt", false, false, userData, editorConfigOverrideMap);
assertEquals("class empty_class_body\n", formatted);
}

@Test
public void testEditorConfigCanDisable(@TempDir Path path) throws IOException {
KtLintCompat0Dot48Dot1Adapter KtLintCompat0Dot48Dot1Adapter = new KtLintCompat0Dot48Dot1Adapter();
try (InputStream is = KtLintCompat0Dot48Dot1AdapterTest.class.getResourceAsStream("/fails_no_semicolons.kt")) {
Files.copy(is, path.resolve("fails_no_semicolons.kt"));
}
String text = new String(Files.readAllBytes(path.resolve("fails_no_semicolons.kt")));

Map<String, String> userData = new HashMap<>();

Map<String, Object> editorConfigOverrideMap = new HashMap<>();
editorConfigOverrideMap.put("ktlint_standard_no-semi", "disabled");

String formatted = KtLintCompat0Dot48Dot1Adapter.format(text, "fails_no_semicolons.kt", false, false, userData, editorConfigOverrideMap);
assertEquals("class fails_no_semicolons {\n\tval i = 0;\n}\n", formatted);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class empty_class_body {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class fails_no_semicolons {
val i = 0;
}
3 changes: 3 additions & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
## [Unreleased]
### Fixed
* Prevent tool configurations from being resolved outside project ([#1447](https://github.com/diffplug/spotless/pull/1447) fixes [#1215](https://github.com/diffplug/spotless/issues/1215))
* Support `ktlint` 0.48+ new rule disabling syntax ([#1456](https://github.com/diffplug/spotless/pull/1456)) fixes ([#1444](https://github.com/diffplug/spotless/issues/1444))
### Changes
* Bump default `ktlint` version to latest `0.47.1` -> `0.48.1` ([#1456](https://github.com/diffplug/spotless/pull/1456))

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

## [Unreleased]
### Fixed
* Support `ktlint` 0.48+ new rule disabling syntax ([#1456](https://github.com/diffplug/spotless/pull/1456)) fixes ([#1444](https://github.com/diffplug/spotless/issues/1444))
### Changes
* Bump default `ktlint` version to latest `0.47.1` -> `0.48.1` ([#1456](https://github.com/diffplug/spotless/pull/1456))

## [2.29.0] - 2023-01-02
### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ void works0_47_1() {
}

@Test
void works0_48_0() {
FormatterStep step = KtLintStep.create("0.48.0", TestProvisioner.mavenCentral());
void works0_48_1() {
FormatterStep step = KtLintStep.create("0.48.1", TestProvisioner.mavenCentral());
StepHarnessWithFile.forStep(this, step)
.testResource("kotlin/ktlint/basic.dirty", "kotlin/ktlint/basic.clean")
.testResourceExceptionMsg("kotlin/ktlint/unsolvable.dirty").isEqualTo("Error on line: 1, column: 1\n" +
Expand Down