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:

  1. 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 passing indent_size and disabled_rules within userData map. The migration path for indent_size seemed to be hidden but I was able to identify DefaultEditorConfigProperties.indentSizeProperty which looked legitimate and worked in my case, but I couldn’t find a replacement for disabled_rules. The fact ExperimentalParams still takes userData 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?
  2. It seems like there is a behavior change introduced in 0.45.0, where values passed as userData using the now deprecated lint 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 overall 0.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 seen ASTNode.getEditorConfigValue(editorConfigProperty) or EDITOR_CONFIG_USER_DATA_KEY mentioned there 😬 )
  3. I noticed isUnitTestContext = true gets passed when calling any of KtLint#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… 👀 image

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

Most upvoted comments

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?

Thank you for all your work @paul-dingemans and recognize you’re shepherding inherited code with some wrinkles.

Tnx!

With this in mind, could we move forward with more confidence as @mateuszkwiecinski implies – ExperimentalParams could just become the new Params and remove any alpha status?

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:

  • remove current deprecations including ‘Params’
  • deprecating/removal ExperimentalParams. I have not yet decided on the new name but the structureborobably won’t change
  • be able to get a list of resolved editorconfig files and other ktlint configuration in use (nice to have new request – sometimes have to bust gradle cache if configuration has changed or otherwise have some insight into what ktlint is configured to do).

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