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

ktLint 0.34 changes not compatible with spotless #419

Closed
Tolriq opened this issue Jul 16, 2019 · 12 comments · Fixed by #469

Comments

@Tolriq
Copy link

commented Jul 16, 2019

Last version of ktLint 0.34 seems to have changes that are not compatible with spotless.

Execution failed for task ':utils:spotlessKotlin'.
> java.lang.NoSuchMethodException: com.pinterest.ktlint.core.KtLint.format(java.lang.String, java.lang.Iterable, java.util.Map, kotlin.jvm.functions.Function2)
@nedtwigg nedtwigg added the bug label Jul 16, 2019
@nedtwigg

This comment has been minimized.

Copy link
Member

commented Jul 16, 2019

Bummer :( Some relevant PR's for anyone who wants a guide for how to fix this:

@shashachu

This comment has been minimized.

Copy link

commented Jul 27, 2019

Sorry about this. I made a change to clean up the API for the lint and format APIs and moved editorconfig parsing out of Main.kt and into ktlint-core. I'll be sure to reach out the next time we make breaking API changes. pinterest/ktlint#503

@nedtwigg

This comment has been minimized.

Copy link
Member

commented Jul 29, 2019

No biggie, I'll be happy to merge a PR whenever someone gets around to it. FWIW, Kotlin has some great tools to allow you to refactor without breaking downstream clients: https://dzone.com/articles/deprecated-annotation-in-kotlin

@abhinavnair

This comment has been minimized.

Copy link

commented Aug 22, 2019

@shashachu Any updates on this?

@nedtwigg

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

Please refrain from "bump comments" unless you have new information to offer. Versions of ktlint >= 0.34 will not work with Spotless until either or both of the following happen:

  1. someone updates Spotless to support the new API
  2. ktlint adds the old methods back in a new version, perhaps with @Deprecated annotations

If you want to help with option 1, there are example PRs in my first comment that describe exactly how to do it. If you want to help with option 2, the PR which changed the KtLint API is also in this issue.

Spotless is about people scratching their own itch. At some point, someone wanted KtLint support, so they built it. If it works, and has a test case, then we will merge it. We'll make sure that those testcases keep working forever, which we have done. We don't promise to follow every API change of every formatter we support - this would force us to be much choosier about letting people merge new features in, which would be a bummer. It should be (and has been) easier to add things to Spotless over time, not harder.

@shashachu

This comment has been minimized.

Copy link

commented Sep 6, 2019

@shashachu Any updates on this?

Sorry, I wasn't aware that this was expected to be fixed on the ktlint side. I'd rather not add the old APIs back (in fact it'd be pretty difficult), so if this could be fixed on the Spotless side, that feels easier. The functionality is available, it's just been moved out of Main and into ktlint-core.

@nedtwigg

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

I wasn't aware that this was expected to be fixed on the ktlint side

It's opensource, we're all volunteers, nobody is expected to do anything! We've had cases in the past where a formatter broke their public API on accident, and then put the old methods back with @Deprecated for compatibility in a follow-up release. You don't have to do this work, but someone who wants KtLint to maintain a stable API could do it, which is why I offered it up.

I'd rather not add the old APIs back

Fair enough, option 2 is off the table. Option 1 is probably 2 hrs of work, max, even if you haven't work on Spotless before. Happy to merge a PR from anyone. We support a ton of formatters, and we eagerly help people to merge changes they would like to see. We do not take proactive responsibility for adapting to breaking changes across the entire ecosystem that we support.

tbroyer added a commit to gwtproject/gwt-events that referenced this issue Sep 29, 2019
due to diffplug/spotless#409 and diffplug/spotless#419

Update to Ktlint 0.34.2 and enable experimental rules.
@ZacSweers

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

I'm working on a PR for this

@ZacSweers

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

Side note - I don't really understand the backward compatibility requirement for a formatter that doesn't have a stable API yet. Makes this quite brittle :/. Especially as the APIs being reflected are Kotlin, so much of reflected APIs are implementation details of the Kotlin compiler's outputs.

ZacSweers added a commit to ZacSweers/spotless that referenced this issue Oct 6, 2019
Resolves diffplug#419
@ZacSweers

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

@nedtwigg

This comment has been minimized.

Copy link
Member

commented Oct 6, 2019

Awesome, thanks very much! As a side note, the reflection isn't for backwards compatibility, it's for classpath isolation. We have formatters for <java | kotlin | scala | sql | groovy | javascript | flow | typeScript | css | scss | less | jsx | vue | graphql | json | yaml | markdown | license headers | anything> using <gradle | maven | anything>, and they do not agree which version of guava is best (for example). A better way than reflection would be to have src/main/ktlint-0.34/ and src/main/ktlint-0.33/ and they could each be compiled against their respective lib. But so far, each single PR has found it easier to write their little bit of reflection than to figure out the gradle-foo to make this work.

@nedtwigg nedtwigg closed this in #469 Oct 7, 2019
@nedtwigg

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

Fixed in in 3.25.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.