Kotlin / dataframe

Structured data processing in Kotlin
https://kotlin.github.io/dataframe/overview.html
Apache License 2.0
761 stars 48 forks source link

Updating the linter #364

Open Jolanrensen opened 1 year ago

Jolanrensen commented 1 year ago

The linter plugin we use (https://github.com/jeremymailen/kotlinter-gradle) can be updated to v3.14.0.

However, two things:

zaleslaw commented 1 year ago

Probably we need a strong DETEKT rules here

Jolanrensen commented 1 month ago

Relevant issue: https://github.com/pinterest/ktlint/issues/2675 (for accepting this[{ notation }])

PR: https://github.com/pinterest/ktlint/pull/2677

Jolanrensen commented 1 month ago

After that PR is merged, the linter can be updated relatively easily.

Jolanrensen commented 1 week ago

I've been tweaking some settings in .editorconfig to what I think looks best (and most in-line with DataFrame): https://github.com/Kotlin/dataframe/blob/k2-linter-attempt/.editorconfig

We just need to figure out how to NOT run on generated-sources by default (as it's way to heavy) but allow the linter to format generated-sources from github actions instead.

Jolanrensen commented 6 days ago

Some updates:

I've had difficulties using Kotlinter due to its limited customization options. After trying Spotless, which also wasn't fit for the job, I found https://github.com/JLLeitschuh/ktlint-gradle, which is also used by kotlin-jupyer.

This branch contains a working project with that plugin: https://github.com/Kotlin/dataframe/tree/k2-ktlint-jlleitschuh-attempt.

In principle, it works the same as Kotlinter; it takes the .editorconfig file as source of truth, but, if we need it, we can programmatically override certain rules. It's ktlint-version independent. It can auto-check and -format when needed and it creates tasks for each individual source set. Since the generated-sources are no sourceSet, it will not run on them by default, which is great :) We can still create a custom ktlint format task for the generated sources though, as I did in :core:ktlintFormatGeneratedSources in the branch. This can run automatically after processKDocsMain and format the generated-sources folder :) (with outputReadOnly = false of couse).

If people want, they can run addKtlintFormatGitPreCommitHook, which will do exactly that, however, let's keep this optional for now.

Finally, the plugin has baseline support https://github.com/JLLeitschuh/ktlint-gradle?tab=readme-ov-file#baseline-support. This means, in theory, we can make the linter only format files that are touched. While this would lower the impact of enabling the new linter, I feel an opportunity to clean up the library like this will not come around soun again.