[Question] How to add baseline to check just like ktlint does?

[shaobin0604]

Ktlint https://github.com/pinterest/ktlint

to add a baseline to check against prepend following args: "--baseline=ktlint-baseline.xml"

[jeremymailen]

Thank you @shaobin0604. It looks like that feature was merged 3 days ago and is not released yet.
It does seem like an appropriate feature to support when it comes out in a kotlinter 3.3.0.

Would you want to specify a baseline file globally in the kotlinter extension?
Or would you prefer to specify it in the task (at the sourceSet level)?

[shaobin0604]

Thank you @shaobin0604. It looks like that feature was merged 3 days ago and is not released yet.
It does seem like an appropriate feature to support when it comes out in a kotlinter 3.3.0.

Would you want to specify a baseline file globally in the kotlinter extension?
Or would you prefer to specify it in the task (at the sourceSet level)?

Thank you for your immediate response.

I prefer specifying a baseline file globally in the kotlinter extension. Thanks!

[Tapchicoma]

@jeremymailen FYI: pinterest/ktlint#1108 - with this change it should be easier to add baseline support

[jeremymailen]

Appreciate all the thoughts. One of the core ideas of kotlinter-gradle is simplicity, and I'd imagine this feature would lead to complexity in design and use. Perhaps someone might educate me on scenarios with bigger teams that might require baselining to introduce ktlint, but feels like a niche use case?
For now I will close and yeah, I suppose someone might change my mind on the topic and we could revive.

[dallasgutauckis]

The use case for baselining is when you're adding new ktlint rules via extension, such as Twitter's ktlint compose rules, and you have many preexisting rule violations.

In this scenario, it's preferable not to allow new violations of the rule (by adding the rules) but also allow the existing violations to enable delegation of fixing those violations to your organization's subteams, chipping away at the baseline violations and enabling smaller changes to be incrementally merged into the project thoughtfully and safely.

In SeatGeek's case, we have 126 lint errors when adding these specific rules that we don't want to/can't fix all at once. Still, we want to prevent future instances of those issues.

Would appreciate if baseline support can be added.

[dallasgutauckis]

For the time being, what I've done is registered a new task specific to the Compose rules by doing the following:

                // kotlinter-gradle doesn't support baselines (https://github.com/jeremymailen/kotlinter-gradle/issues/167), and we have a bunch of pre-existing
                // violations of the Twitter compose ktlint rules, so we register a separate task for testing while teams work on fixing violations to avoid
                // failing builds with violations using the normal ktlint task
                //
                // At time of writing, you could get the list of rules from the ktlint website using the following Javascript in the console from
                // https://pinterest.github.io/ktlint/rules/standard/:
                //
                // console.log("\"" + $$("p").filter(it => it.innerText.match(/Related rule/)).map(it => it.childNodes[1]).filter(it => it != undefined && it.localName == "a").map(it => it.innerText).join("\",\n\"") + "\"")
                project.tasks.register("lintCompose", LintTask::class.java) { task ->
                    group = "verification"
                    task.source(project.projectDir)
                    // Disable all default rules for this task, but don't disable the Twitter compose rules
                    task.disabledRules.addAll(
                        "annotation",
                        "argument-list-wrapping",
                        "chain-wrapping",
                        "enum-entry-name-case",
                        "filename",
                        "final-newline",
                        "import-ordering",
                        "indent",
                        "max-line-length",
                        "modifier-order",
                        "multiline-if-else",
                        "no-blank-line-before-rbrace",
                        "no-blank-lines-in-chained-method-calls",
                        "no-consecutive-blank-lines",
                        "no-empty-class-body",
                        "no-empty-first-line-in-method-block",
                        "no-line-break-after-else",
                        "no-line-break-before-assignment",
                        "no-multi-spaces",
                        "no-semi",
                        "no-trailing-spaces",
                        "no-unit-return",
                        "no-unused-imports",
                        "no-wildcard-imports",
                        "package-name",
                        "parameter-list-wrapping",
                        "string-template",
                        "trailing-comma-on-call-site",
                        "trailing-comma-on-declaration-site",
                        "wrapping",
                        "annotation-spacing",
                        "colon-spacing",
                        "comma-spacing",
                        "comment-spacing",
                        "curly-spacing",
                        "dot-spacing",
                        "double-colon-spacing",
                        "keyword-spacing",
                        "op-spacing",
                        "paren-spacing",
                        "range-spacing",
                        "spacing-around-angle-brackets",
                        "spacing-between-declarations-with-annotations",
                        "spacing-between-declarations-with-comments",
                        "unary-op-spacing"
                    )

                    task.exclude { fileTreeElement ->
                        fileTreeElement.file.absolutePath.contains(project.buildDir.absolutePath)
                    }
                }

Which will enable our folks to at least see what the Compose violations are without requiring them to manually add/remove the dependency for the rules.

Additionally, I've manually removed the main rules we have a lot or complex violations for:

                // This references ConfigurableKtLintTask instead of just LintTask because we also want to configure the formatKotlin task
                tasks.withType(ConfigurableKtLintTask::class.java).configureEach { task ->
                    if (task.name != "lintCompose") {
                        task.disabledRules.addAll(
                        ...
                        )

                        // Disable compose lint checks where we already have violations prior to Twitter Compose ktlint integration
                        // See lintCompose task for additional details
                        //
                        // This list of rules can be gathered from https://twitter.github.io/compose-rules/rules/ using the following in console:
                        // console.log("\"" + $$("p").filter(it => it.innerText.match(/Related rule/)).map(it => it.childNodes[1]).filter(it => it != undefined && it.localName == "a").map(it => it.innerText).join("\",\n\"") + "\"")
                        task.disabledRules.addAll(
                            "twitter-compose:remember-missing-check",
                            "twitter-compose:unstable-collections",
                            "twitter-compose:param-order-check",
                            "twitter-compose:modifier-missing-check",
                            "twitter-compose:modifier-reused-check"
                        )
                    }

                    task.exclude { fileTreeElement ->
                        fileTreeElement.file.absolutePath.contains(project.buildDir.absolutePath)
                    }
                }

We have this inside our own plugin so I can't give the full source, but wanted to share something in case someone comes across this before baseline support is added to kotlinter 🙏🏻

[nachtien]

I don't think my team/manager will let me format the entire codebase :(. That's preventing me from using Kotlinter.

[mateuszkwiecinski]

I see a couple of ways we could approach the challenge here. One of them is introducing baseline support gradually, as the first step we could only add baseline support for Lint/Format tasks and assume people would generate baseline themselves, using ktlint tool directly.

Given the above, I have built a PoC of a full baseline support (including file generation) here: https://github.com/jeremymailen/kotlinter-gradle/tree/baseline. It's far from ideal, it has a coupe of flaws (baselines overwrite each other, it relies on keeping lint and baseline generating task configs in sync), but I hoped it'll make it easier to grasp the complexity of an actual implementation, if we decide we want to add such.
Although, the final decision is up to @jeremymailen :)

[jeremymailen]

One could adopt ktlint incrementally by running it in warn only mode. Then switch to failing when warnings are reduced to the level where it is possible to fix them in one shot.

As a developer, not sure how I'd think about working in a baselined environment. I might edit a file which has existing different formatting. Should I make my new snippet old format or comply with ktlint because its failing and have it look different than the rest of the file?

[nachtien]

Yup I’m running into that issue with detekt. I’ll edit a file then all the lines get shifted and we gotta address them anyways. But at least we iterate on it slowly. If it was in warn mode then people are less inclined to fix anything.

…

On Thu, Nov 3, 2022 at 8:08 PM Jeremy Mailen ***@***.***> wrote: One could adopt ktlint incrementally by running it in warn only mode. Then switch to failing when warnings are reduced to the level where it is possible to fix them in one shot. As a developer, not sure how I'd think about working in a baselined environment. I might edit a file which has existing different formatting. Should I make my new snippet old format or comply with ktlint because its failing and have it look different than the rest of the file? — Reply to this email directly, view it on GitHub <#167 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA4P5TK564W37I6DCMRJDDWGR435ANCNFSM4T4GD55Q> . You are receiving this because you commented.Message ID: ***@***.***>

[dallasgutauckis]

In our case, we already have been using ktlint via this plugin for a while and have a lint-passing codebase, except now we wanted to add some new rules published by Twitter for Compose and we have a number of violations already-implemented that we do want to fix but can't via a single sweeping change. We need people to plan, develop, test, release, monitor those changes for correctness as well as format/usage.

Baselining makes sense here probably for the same well-thought-out reasons that ktlint supports it, checkstyle/Android linter does, etc

I'm definitely only thinking about supporting the baseline file param that ktlint CLI supports since there's no way to pass that param to ktlint otherwise.

[dallasgutauckis]

FWIW, I had to back out my implementation for separating out the tasks as it caused issues syncing the project for our team in Android Studio. I'm going to have to fork this project and maintain a new implementation that enables baselining as a result. Will keep this thread up to date with that so teams can have a solution.

[dallasgutauckis]

If you decide you don't want #302 in here, we will host the separate fork and make a separate plugin 👍🏻

[h-siddiqui]

Appreciate all the thoughts. One of the core ideas of kotlinter-gradle is simplicity, and I'd imagine this feature would lead to complexity in design and use. Perhaps someone might educate me on scenarios with bigger teams that might require baselining to introduce ktlint, but feels like a niche use case? For now I will close and yeah, I suppose someone might change my mind on the topic and we could revive.

I wanted to revive the topic of adding a baselining feature. I understand that the core principle of kotlinter-gradle is simplicity, but I think we can implement this feature in a way that stays true to that philosophy. By making the baseline feature optional, users who prefer not to use it won't see any change in the configuration or usage of the project.

I also wanted to touch on the concern that this feature might be for a niche audience. In my experience, baselining isn't really a niche use case, but more of a common scenario for development teams. Teams often need to enforce a policy of disallowing new warnings while working on a long-term plan to address and eliminate all existing ones. Adding the baseline feature would help these teams maintain code quality and enforce coding standards.

Considering these points, would you be open to reconsidering the implementation of the baselining feature? I believe it could be really beneficial for many users without compromising simplicity.