ktlint: `0.45.0` backwards incompatibility: `KtLint.Params#userData` gets ignored, unclear migration path + more feedback
Initially reported on Kotlin slack: https://kotlinlang.slack.com/archives/CKS3XG0LS/p1647897839331439 but I was asked to create issue here.
Expected Behavior
0.45.0
is backward compatible with 0.44.0
, or has clear migration path
Observed Behavior
I’ll copy-paste what I wrote on Slack:
- Can someone share what’s the future of
userData
map, does it only change its purpose, or will it be removed completely at some point? The code I was upgrading had been passingindent_size
anddisabled_rules
withinuserData
map. The migration path forindent_size
seemed to be hidden but I was able to identifyDefaultEditorConfigProperties.indentSizeProperty
which looked legitimate and worked in my case, but I couldn’t find a replacement fordisabled_rules
. The factExperimentalParams
still takesuserData
suggests that’s still the “recommended” option, am I wrong here? What’s the proper way to pass rules that I expect to be disabled? - It seems like there is a behavior change introduced in
0.45.0
, where values passed asuserData
using the now deprecatedlint
method are ignored. Maybe I misread the release notes, but they had vibe you strongly advise to migrate now, because later it will be more difficult, but in overall0.45.0
is still backwards compatible. If breaking the compatibility was intended, then I’ll share my personal opinion: I think it would be better for api consumers if you removed the method already in this release so api consumers would get a compile-time error, instead of runtime one (or bug reports from end users 😛). If breaking the compatibility wasn’t intended, then FYI it did, at least the case described above 😛 And just for context: I attempted to bump ktlint in Gradle Plugin, so I might be missing something here, as the release notes weren’t 100% clear for me (for example: I’ve never seenASTNode.getEditorConfigValue(editorConfigProperty)
orEDITOR_CONFIG_USER_DATA_KEY
mentioned there 😬 ) - I noticed
isUnitTestContext = true
gets passed when calling any ofKtLint#lint
methods. It doesn’t break anything, but looks suspicious, so I thought it might be worth pointing out, especially when seeing todos like this 👇 AFAIU both ktlint-gradle and kotlinter-gradle use lint and format in regular usage, not in tests soo… 👀
Feel free to split them into 3 separate issues if it’s more convenient for you 😃
Steps to Reproduce
Run KtLint.lint(KtLint.Params(userData = mapOf("indent_size" to "5"), ...))
on a file with respective indentation and observe the lint reporting failures.
Your Environment
- Version of ktlint used: 0.45.1
- Relevant parts of the
.editorconfig
settings: none - Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): I tried to upgrade kotlinter-gradle and the tests failed: https://github.com/jeremymailen/kotlinter-gradle/pull/243/files I had to switch to the
ExperimentalParams
class which introduced even more confusion - hence all the questions above - Version of Gradle used (if applicable): it’s not
- Operating System and version:
macos-latest
,windows-latest
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 3
- Comments: 15 (6 by maintainers)
Commits related to this issue
- Fix backward incompatibility issues released via KtLint 0.45.0 and 0.45.1. * Deprecate data class 'Params' as it only provides the userData parameter which prohibits API Consumers to split the '.ed... — committed to paul-dingemans/ktlint by paul-dingemans 2 years ago
- Update changelog Closes #1434 — committed to paul-dingemans/ktlint by paul-dingemans 2 years ago
- Fix backward incompatibility issues released via KtLint 0.45.0 and 0.45.1 (#1442) * Fix backward incompatibility issues released via KtLint 0.45.0 and 0.45.1. * Deprecate data class 'Params' as it... — committed to pinterest/ktlint by paul-dingemans 2 years ago
This change has made KtLint unusable in essentially every third party plugin for it (gradle plugins, etc) because they all relied on userData. While I understand it’s complicated, I think it should be temporarily restored until there’s a proper path forward for integrations
0.45.2 is released. Thanks everyone!
Thnx! @shashachu Would you be so kind to release current master as version 0.45.2?
Tnx!
In next release (0.46.0), I will definitely clean up a lot of the alpha and beta markings. I am sure that more changes will follow in the future but both maintainers and consumers now need an API which is less bloated. Hopefully this also sheds light on the next steps. Having said that, I expect following to be included in the release:
Please file a separate issue. But please make more clear what you need. It can take a while before it will be implemented. My own backlog is also full of stuff that I want to achieve. And as winter is almost gone, I will have less time to spend.
tbh, given that ktlint is still not at 1.0.0, I think it’s probably fine to just break API and document the migration path properly. I’d pretty much prefer a compile/runtime exception rather than this hidden behavior of
userData