Replace kotlin public data classes with Poko compiler plugin generated ones

[mateuszkwiecinski]

Fixes #2133

Description

This is a quick attempt to replace public data classes with equals/hashCode/toString methods generated by Poko compiler plugin.
From my understanding the alternative are:

1. Generate equals/hashCode/toString manually via IDE - but this is the least preferred option, since it's super easy to forget updating their implementation, when modifying the class
2. Go through the code and identify all pieces of code relying on equals/hashCode(calls like==, List#distinct()orCollection#contains` etc) and then refactor the implementation to use custom comparators.
3. Keep using data classes, but never modify them

I'm opening this PR to gather some feedback, show the diff as a proof how it can be done, and maybe someone has any better idea 😉

Checklist

Before submitting the PR, please check following (checks which are not relevant may be ignored):

• Commit message are well written. In addition to a short title, the commit message also explain why a change is made.
• At least one commit message contains a reference Closes #<xxx> or Fixes #<xxx> (replace<xxx> with issue number)
• Tests are added
• KtLint format has been applied on source code itself and violations are fixed
• CHANGELOG.md is updated
• PR description added

Documentation is updated. See difference between snapshot and release documentation

• Snapshot documentation in case documentation is to be released together with a code change
• Release documentation in case documentation is related to a released version of ktlint and has to be published as soon as the change is merged to master
• In case of adding a new rule, it needs to be added to experimental rules documentation

[paul-dingemans]

This worked out differently than I expected beforehand when just (quickly) reading the Poko documentation. Overall, the code changes do not seem too intrusive to me.

Would it be possible to define some gradle task that fails the build it it finds something like "public data class" in the source code? Because, I really believe that I will forget about not using data class in public API.

Also nice to see that the binary-compatibility-validator requires changes.

[mateuszkwiecinski]

Would it be possible to define some gradle task that fails the build it it finds something like "public data class" in the source code? Because, I really believe that I will forget about not using data class in public API.

I did a bit of research, but I couldn't find any standard solution. If I had to come up with some custom ideas then:

1. cheapest: have a bash script that iterates through all .api files and fails if it finds a line with component1() function signature (may produce false negatives if someone adds function named component1(), but that feels rather unlikely scenario)
2. probably most proper one: have a custom ktlint rule that fails if public data class is used, that should be doable, right?
3. Not have extra checks, and pay extra attention when reviewing binary validator diffs

[paul-dingemans]

Would it be possible to define some gradle task that fails the build it it finds something like "public data class" in the source code? Because, I really believe that I will forget about not using data class in public API.

I did a bit of research, but I couldn't find any standard solution. If I had to come up with some custom ideas then:

1. cheapest: have a bash script that iterates through all .api files and fails if it finds a line with component1() function signature (may produce false negatives if someone adds function named component1(), but that feels rather unlikely scenario)

Alternative to scanning .api files we could also grep the source itself for public data class or protected data class. But this also quite hacky.

2. probably most proper one: have a custom ktlint rule that fails if public data class is used, that should be doable, right?

It would be possible, but it would be a rule that should not be exposed because public data classes are valid to be used in case ktlint is used on a project that is not exposing an API.

3. Not have extra checks, and pay extra attention when reviewing binary validator diffs

True, except that I made this remark because I know that I going to forget about it. Also, it doesn't warn a contributor about making this mistake.

[edit] Last days, I slowly come to the believe that it is okay, to have no additional check. We can document this as a FAQ for ktlint developers.

[mateuszkwiecinski]

Alternative to scanning .api files we could also grep the source itself for public data class or protected data class. But this also quite hacky.

Scanning .api files feels much safer since it is somewhat guaranteed it checks only the public API surface (scanning sources is susceptible to data classes used within non-published project, within test sources or within code comments)

It would be possible, but it would be a rule that should not be exposed

Absolutely 👍 Assuming this project used one of the gradle plugins, my idea was to do something like:

dependencies {
    ktlintRuleSet(project(":completely-internal-ruleset-just-for-ktlint-project-not-published-anywhere"))

but with CLI, I'd have to produce .jar file first, and only then append it somewhere to CLI invocation? idk, I haven't worked with CLI 👀

Anyway, in the first iteration, I can propose a bash script that runs on CI. If the bash script turns out unreliable and/or if I find enough motivation - I'll try to come up with a project-specific Rule for ktlint, as a replacement for the bash script.

Here's how a failure will look like:

[paul-dingemans]

For now, let's proceed with the current check you have added.