Fix ktlint 0.48 #1456

Sineaggi · 2023-01-04T21:18:44Z

We use ktlint's new rule execution api to construct rules that support the format in ktlint's documentation.

So for example, ktlint_standard will disable all the standard rules and ktlint_standard_no-semi would only disable the no semicolon rule.

Fixed a few deprecation warnings and added tests to verify the new rules work.

Bumps ktlint to 0.48.1

Sineaggi · 2023-01-04T21:27:19Z

For the two rules that have deprecation (for removal) warnings,

//noinspection deprecation
list.add(DisabledRulesEditorConfigPropertyKt.getDISABLED_RULES_PROPERTY());
//noinspection KotlinInternalInJava,deprecation
list.add(DisabledRulesEditorConfigPropertyKt.getKTLINT_DISABLED_RULES_PROPERTY());

we'll simply be removing them from the list of rules when they're removed in the next version of ktlint.

...t0Dot48Dot1/java/com/diffplug/spotless/glue/ktlint/compat/KtLintCompat0Dot48Dot1Adapter.java

Sineaggi · 2023-01-04T22:27:37Z

ktlint's new (non-deprecated) formatting api will always load the .editorconfig. In this case, the .editorconfig in the root folder is changing the behavior of the tests.

davidburstrom · 2023-01-06T16:22:17Z

lib/build.gradle

@@ -34,7 +34,7 @@ versionCompatibility {
 				'0.45.2',
 				'0.46.0',
 				'0.47.0',
-				'0.48.0',
+				'0.48.1',


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.

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

davidburstrom · 2023-01-06T16:27:54Z

lib/build.gradle

@@ -105,10 +105,16 @@ dependencies {
 	flexmarkCompileOnly 'com.vladsch.flexmark:flexmark-all:0.62.2'
 }

+configurations.named('testCompatKtLint0Dot48Dot1Implementation').configure {
+	extendsFrom(configurations.testImplementation, configurations.compatKtLint0Dot48Dot1CompileOnly)


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.

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.

🤔 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.

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

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

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

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

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.

eirnym · 2023-01-07T13:32:27Z

I've started the #1442 to introduce editorconfig support earlier and feel we need to join our efforts

nedtwigg · 2023-01-07T16:44:18Z

I just did a local test-merge between this PR and 1442

Add editor config file option to maven and gradle #1442

and luckily the only conflict was a manageable one in KtLintCompat0Dot48Dot0Adapter. In my estimation, the other PR, #1442, is the harder one, so normally I would let that one merge first.

But, in this case, @davidburstrom is the original author and maintainer of the KtlintStep, and he has been engaged in this PR. So I the order of merging will be:

this PR when @davidburstrom comments with "this PR is ready to merge"
then Add editor config file option to maven and gradle #1442 when @eirnym has had a chance to fix the merge conflicts. Might be a good idea for you to review that PR too if you have time @davidburstrom, it is high quality and fixes the long-requested problem of .editorconfig integration

Sorry @eirnym, you got sort of a rough break to write a good and long-requested feature and then get blocked. 🤷

eirnym · 2023-01-07T16:53:51Z

@nedtwigg Thanks, this sounds like a good plan, as soon this PR will be merged, I'll merge main branch.
@davidburstrom I'd be happy if you review my PR as well

PS: I kind'a expected to have some time break as well ;P

davidburstrom · 2023-01-07T19:12:30Z

lib/build.gradle

@@ -105,10 +105,16 @@ dependencies {
 	flexmarkCompileOnly 'com.vladsch.flexmark:flexmark-all:0.62.2'
 }

+configurations.named('testCompatKtLint0Dot48Dot0Implementation').configure {


This should no longer be necessary with version 0.4.0 of the version-compatibility-plugin, if the compatKtLint0Dot48Dot0CompileOnly is changed to ompatKtLint0Dot48Dot0CompileAndTestOnly.

I've used the latest plugin to clean this up. Thanks for the new configuration!

No problem, I'm happy to improve the plugin :) I also improved the plugin documentation, fyi.

davidburstrom · 2023-01-07T19:13:01Z

lib/build.gradle

 // 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 {


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

davidburstrom · 2023-01-07T19:30:13Z

...t48Dot0/java/com/diffplug/spotless/glue/ktlint/compat/KtLintCompat0Dot48Dot0AdapterTest.java

+	@Test
+	public void testDefaults(@TempDir Path path) throws IOException {
+		KtLintCompat0Dot48Dot0Adapter ktLintCompat0Dot48Dot0Adapter = new KtLintCompat0Dot48Dot0Adapter();
+		try (InputStream is = KtLintCompat0Dot48Dot0AdapterTest.class.getResourceAsStream("/empty_class_body.kt")) {


What would you say about extracting this into a utility function? As far as I can tell, it's just a fairly condensed way of getting a String from a Java resource, but it's still verbose when it's duplicated.

I could inline the input, considering I inlined the formatting result.

EDIT: Utility function ended up being easier.

Even easier :)

davidburstrom · 2023-01-07T19:35:47Z

this PR when @davidburstrom comments with "this PR is ready to merge"

I'm not sure if I should consider myself author/maintainer, but I'm happy to chime in. I don't have anything other than minor suggestions. After @Sineaggi has had a chance to look at those and do as he feels about them, I think this PR is ready to be merged.

davidburstrom · 2023-01-07T19:45:45Z

@eirnym I've looked through #1442 and it looks good. Let me know when the merge conflicts have been addressed, and then I'll have another go.

eirnym · 2023-01-07T20:24:03Z

@davidburstrom sweet! I'll address merge conflicts as soon as possible after this PR is merged.

davidburstrom · 2023-01-08T12:03:20Z

"this PR is ready to merge"

this PR is ready to merge

Nicely done @Sineaggi!

Jolley71717 · 2023-01-12T22:20:58Z

I'm excited to see this get deployed.

We've had some super nasty output from the java.lang.IllegalArgumentException: Can not create an EditorConfigOverride without properties. Use 'emptyEditorConfigOverride' instead

Thanks again for merging it.

nedtwigg · 2023-01-14T23:13:56Z

Published in plugin-gradle 6.13.0 and plugin-maven 2.30.0.

…2.0 (mulk/mulkcms2!16) This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [com.diffplug.spotless:spotless-maven-plugin](https://github.com/diffplug/spotless) | build | minor | `2.31.0` -> `2.32.0` | --- ### Release Notes <details> <summary>diffplug/spotless</summary> ### [`v2.32.0`](https://github.com/diffplug/spotless/blob/HEAD/CHANGES.md#2320---2023-01-13) ##### Added - Add option `editorConfigFile` for `ktLint` [#142](diffplug/spotless#142) - **POTENTIALLY BREAKING** `ktlint` step now modifies license headers. Make sure to put `licenseHeader` *after* `ktlint`. - Added `skipLinesMatching` option to `licenseHeader` to support formats where license header cannot be immediately added to the top of the file (e.g. xml, sh). ([#1441](diffplug/spotless#1441)). - Add YAML support through Jackson ([#1478](diffplug/spotless#1478)) - Added support for npm-based [ESLint](https://eslint.org/)-formatter for javascript and typescript ([#1453](diffplug/spotless#1453)) - Better suggested messages when user's default is set by JVM limitation. ([#995](diffplug/spotless#995)) ##### Fixed - Support `ktlint` 0.48+ new rule disabling syntax ([#1456](diffplug/spotless#1456)) fixes ([#1444](diffplug/spotless#1444)) - Fix subgroups leading catch all matcher. ##### Changes - Bump default version for `prettier` from `2.0.5` to `2.8.1` ([#1453](diffplug/spotless#1453)) - Bump the dev version of Gradle from `7.5.1` to `7.6` ([#1409](diffplug/spotless#1409)) - We also removed the no-longer-required dependency `org.codehaus.groovy:groovy-xml` - Breaking changes to Spotless' internal testing infrastructure `testlib` ([#1443](diffplug/spotless#1443)) - `ResourceHarness` no longer has any duplicated functionality which was also present in `StepHarness` - `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](diffplug/spotless#1456)) - Switch our publishing infrastructure from CircleCI to GitHub Actions ([#1462](diffplug/spotless#1462)). - Help wanted for moving our tests too ([#1472](diffplug/spotless#1472)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Sineaggi added 4 commits January 4, 2023 14:12

Fix ktlint 0.48

6dc01fa

Add change to changelog

4550b66

Add link to issue in changelog

2936d3c

Update all change logs

41c3fe9

twarner-sofi reviewed Jan 4, 2023

View reviewed changes

...t0Dot48Dot1/java/com/diffplug/spotless/glue/ktlint/compat/KtLintCompat0Dot48Dot1Adapter.java Outdated Show resolved Hide resolved

Run spotless

954e6b5

Sineaggi added 3 commits January 4, 2023 15:34

Use older format function that doesn't load .editorconfig

1a19da1

Fix UTF-8 encoder errors

11971ab

Remove org.gradle settings

b853169

davidburstrom reviewed Jan 6, 2023

View reviewed changes

Sineaggi added 2 commits January 6, 2023 10:45

Change back to 0.48.0

b6e9073

Use testCommonImplementation configuration

6b2e2c3

davidburstrom reviewed Jan 7, 2023

View reviewed changes

Sineaggi added 2 commits January 7, 2023 14:10

Use latest version-compatibility plugin

778f485

Factor out helper method

3409754

Merge branch 'main' into fix-ktlint-48

2eebed3

nedtwigg enabled auto-merge January 8, 2023 16:58

nedtwigg merged commit 308c7cc into diffplug:main Jan 8, 2023

Sineaggi deleted the fix-ktlint-48 branch January 8, 2023 21:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ktlint 0.48 #1456

Fix ktlint 0.48 #1456

Sineaggi commented Jan 4, 2023 •

edited

Loading

Sineaggi commented Jan 4, 2023 •

edited

Loading

Sineaggi commented Jan 4, 2023

davidburstrom Jan 6, 2023 •

edited

Loading

Sineaggi Jan 6, 2023 •

edited

Loading

davidburstrom Jan 6, 2023

davidburstrom Jan 6, 2023

davidburstrom Jan 6, 2023

davidburstrom Jan 6, 2023 •

edited

Loading

davidburstrom Jan 6, 2023

Sineaggi Jan 6, 2023

davidburstrom Jan 7, 2023

davidburstrom Jan 7, 2023 •

edited

Loading

eirnym commented Jan 7, 2023

nedtwigg commented Jan 7, 2023 •

edited

Loading

eirnym commented Jan 7, 2023

davidburstrom Jan 7, 2023

Sineaggi Jan 7, 2023

davidburstrom Jan 7, 2023

davidburstrom Jan 7, 2023

davidburstrom Jan 7, 2023

Sineaggi Jan 7, 2023 •

edited

Loading

davidburstrom Jan 7, 2023

davidburstrom commented Jan 7, 2023

davidburstrom commented Jan 7, 2023

eirnym commented Jan 7, 2023

davidburstrom commented Jan 8, 2023

Jolley71717 commented Jan 12, 2023

nedtwigg commented Jan 14, 2023

Fix ktlint 0.48 #1456

Fix ktlint 0.48 #1456

Conversation

Sineaggi commented Jan 4, 2023 • edited Loading

Sineaggi commented Jan 4, 2023 • edited Loading

Sineaggi commented Jan 4, 2023

davidburstrom Jan 6, 2023 • edited Loading

Choose a reason for hiding this comment

Sineaggi Jan 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidburstrom Jan 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidburstrom Jan 7, 2023 • edited Loading

Choose a reason for hiding this comment

eirnym commented Jan 7, 2023

nedtwigg commented Jan 7, 2023 • edited Loading

eirnym commented Jan 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sineaggi Jan 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidburstrom commented Jan 7, 2023

davidburstrom commented Jan 7, 2023

eirnym commented Jan 7, 2023

davidburstrom commented Jan 8, 2023

Jolley71717 commented Jan 12, 2023

nedtwigg commented Jan 14, 2023

Sineaggi commented Jan 4, 2023 •

edited

Loading

Sineaggi commented Jan 4, 2023 •

edited

Loading

davidburstrom Jan 6, 2023 •

edited

Loading

Sineaggi Jan 6, 2023 •

edited

Loading

davidburstrom Jan 6, 2023 •

edited

Loading

davidburstrom Jan 7, 2023 •

edited

Loading

nedtwigg commented Jan 7, 2023 •

edited

Loading

Sineaggi Jan 7, 2023 •

edited

Loading