https://kotlinlang.org logo
Title
w

wakingrufus

04/27/2023, 10:12 PM
I am working on ktlint-gradle compatibility for 0.49, and I have some questions
First, I am wondering about Baseline
it looks like it was moved to cli, which I don't see published to maven central
but I need that api to read the baseline file and use it to filter the linting errors found in our various gradle custom reporters
unless there is another way to do this now?
s

Sha Sha Chu

04/27/2023, 10:28 PM
hm...that may be a mistake (it not being in maven central)
actually there's nothing to publish in there except for
Baseline
. we may just need to move it back into ktlint-core
i'm not sure why it was moved into ktlint-cli
w

wakingrufus

04/27/2023, 10:36 PM
Ok that would be great
s

Sha Sha Chu

04/27/2023, 10:51 PM
hrm. the dependencies are slightly tangled. i'll need to take a look at this tomorrow
w

wakingrufus

04/27/2023, 10:56 PM
Thanks. No rush of course. I appreciate you taking the time
s

Sha Sha Chu

04/27/2023, 10:59 PM
of course. I think I may end up breaking into a separate ktlint-baseline module if I can't find a more logical place for it
perhaps i can merge and you can check with the SNAPSHOT to see if it gets what you need?
w

wakingrufus

04/28/2023, 10:14 PM
I had one question on the PR, but overall looks good. Id be happy to test against the snapshot and let you know
s

Sha Sha Chu

04/28/2023, 10:26 PM
lol some bad copypasta
w

wakingrufus

04/29/2023, 1:06 AM
that's what i suspected :)
p

Paul Woitaschek

04/30/2023, 6:45 AM
Life would be so much easier if the gradle plugin was part of ktlint
s

Sha Sha Chu

05/01/2023, 6:03 PM
@wakingrufus thanks for your patience; I just merged the PR. let me know if the SNAPSHOT is working
@Paul Woitaschek it's not out of the question; it was mostly us trying to figure out the resourcing. for now we're collaborating much more closely to try to minimize disruption
w

wakingrufus

05/01/2023, 6:07 PM
Thanks, ill take a look at the snapshot.
p

Paul Woitaschek

05/01/2023, 6:07 PM
That's great to hear. With whom do you try to collaborate?
w

wakingrufus

05/01/2023, 6:07 PM
which repo is the snapshot published to?
jonathan and john, who are the current primary maintainers of this plugin https://github.com/JLLeitschuh/ktlint-gradle
w

wakingrufus

05/01/2023, 6:10 PM
@Paul Woitaschek regarding combining the projects: in the gradle ecosystem, there is a convention of keeping plugin version decoupled from the tool version. for example, you can use the latest version of ktlint-gradle with any version of ktlint. This is to allow users to use the right plugin for gradle compatibility that they need, without forcing down new behavior from the external tool, or being stuck on an old version of the tool in some cases. merging the ktlint and ktlint-gradle projects would not really simplify this, as all the work we do in ktlint-gradle to decouple from a single compile target of ktlint would still need to done
s

Sha Sha Chu

05/01/2023, 6:12 PM
I am hopeful the churn will lessen once we have a 1.0 release and more of a commitment to stop constantly breaking the API :)
p

Paul Woitaschek

05/01/2023, 6:13 PM
Do you think there is really the need for that? This is a thing that doesn't ship as part of a library. I don't care about compatibility here, to me these just belong together.
w

wakingrufus

05/01/2023, 6:15 PM
need might be a strong word, but yes I think there is value in that, and it is how practically every other gradle plugin for external tooling works, so it would be unexpected if this one did not work that way
@Sha Sha Chu the snapshot is lookign good. Ill let you know if i have any other issues
p

Paul Dingemans

05/09/2023, 9:00 AM
Due to holidays, I have missed this thread. I would like to discuss the necessity of exposing the Baseline functionality by Ktlint. The baseline functionality is something that is entirely encapsulated in the Ktlint CLI. The KtlintRuleEngine has no knowledge about the Baseline concept. So, I am wondering why anybody else would need to know about the Baseline. From the perspective of Ktlint CLI, the baseline should not be tempered with by any other tools. So my counter questions are: 1. Do you invoke Ktlint CLI or do you invoke the KtLintRuleEngine directly? 2. Wat kind of reporters are you meaning with
filter the linting errors found in our various gradle custom reporters
? When using Ktlint CLI, the linting errors which are stored in the
baseline.xml
are already filtered out before calling the reporters. 3. When calling KtLintRuleEngine directly, why do you want to filter out results formerly stored in the baseline of Ktlint CLI? I would like to understand this as exposing the Baseline does not seem logical to me at this moment. Also exposing it as a separate artifact does feel a bit weird when I look at the result architecture diagram.
s

Sha Sha Chu

05/09/2023, 4:05 PM
I would think the baseline should be something that is accessible to reporters, not the cli specifically. it just says whether or not a specific issue is already known. but obviously your understanding of the overall arch is much deeper than mine
p

Paul Dingemans

05/09/2023, 4:16 PM
Ktlint CLI collects all lint errors from the
lint
and
format
tasks of
KtLintRuleEngine
. After filtering out all lint errors that exists in
baseline.xml
the remaining lint errors are sent to the reporters.
s

Sha Sha Chu

05/09/2023, 4:23 PM
i think i'm likely just misunderstanding, but how do API consumers use the baseline then? @wakingrufus FYI
p

Paul Dingemans

05/09/2023, 4:25 PM
That is exactly my question as well.
w

wakingrufus

05/09/2023, 8:33 PM
wea re calling KtLintRuleEngine directly
we use baseline to serialize out the baseline and then read it in when generating reports, so we can remove baselined errors form the lint output when generating the reports
all of these operations deal with files to maximize gradle caching
I am open to delegating to other APIs in ktlint if they are available for this though
p

Paul Dingemans

05/10/2023, 4:19 PM
So basically you’re replicating functionality which is part of Ktlint CLI. That is fine and understandable from viewpoint to maximize Gradle caching. But as you already replicate functionality regarding filtering out baseline errors before sending it to the reporters, I would say that replicating (e.g. copy paste the Baseline class) to your project would be best. There seems to be little reason that the
baseline.xml
as used by Ktlint CLI should be identical to the baseline that is used by the Gradle plugin. Or, is it essential because users of the Gradle Plugin also use Ktlint CLI for the same project? From the viewpoint of segregation of concerns, I would advocate that both Ktlint CLI and the Gradle plugin use custom (independent) implementations of the Baseline.
w

wakingrufus

05/10/2023, 8:28 PM
I am trying to figure this out, im still rather new to the codebase... but it seems we are using Reporter/ReporterV2 with
ktlint-reporter-baseline
to create the baseline. so we are delegating to that for serialization, so it would make sense to also delegate for deserialization, but for that we need the
com.pinterest.ktlint.cli.api.loadBaseline
method right? doing this would make our baselines continue to be compatible with CLI. that is useful when, for example, devs are using the plugin locally, but the ci process might be invoking the cli by other means
I could copy-paste Baseline.kt to my project I suppose, but if the format ever changes, I would break
p

Paul Dingemans

05/11/2023, 2:54 PM
I am trying to figure this out, im still rather new to the codebase... but it seems we are using Reporter/ReporterV2 with
ktlint-reporter-baseline
to create the baseline.
This is actually a very good remark and the solution to the problem. The Baseline class should move the
ktlint-reporter-baseline
. As a result, both the producer and consumer of the
baseline.xml
are in the same module. As both the plugin and
ktlint-cli
already depend on the
ktlint-reporter-baseline
it will work well for both of us. The API stays clean and logical.
PR is merged. I plan to release
0.49.1
this weekend.
w

wakingrufus

05/11/2023, 8:27 PM
awesome thanks so much!\
p

Paul Dingemans

05/12/2023, 3:31 PM
Well, lets thanks everybody in participating in this thread. Only together we build great stuff.
w

wakingrufus

05/17/2023, 8:49 PM
p

Paul Dingemans

05/19/2023, 5:48 PM
Cool! But maybe you can wait a little before releasing. There are two issues of potential relevance. I will send more details tomorrow.
w

wakingrufus

05/19/2023, 5:49 PM
Ok thanks for the heads up. I still have a couple things to improve in my PR before we merge anyway
p

Paul Dingemans

05/20/2023, 8:59 AM
So yesterday, I have promised to send more details. A Spotless developer mentioned in issue #2042 that the introduction of the value classes
RuleId
and
RuleSetId
lead to Java interoperability problem in Spotless. This problem will be solved by replacing the value classes by data classes. Technically, this is a breaking change (
0.50.x
). I would guess, that the impact for
ktlint-gradle
is a simple recompilation. But releasing in
0.50.x
would mean an additional layer of incompatibility that you might want to avoid by skipping the
0.49.x
. For a normal user, e.g. a non-API consumer, a
0.50.x
does not make sense at it contains no functional improvements and only a few bugfixes. For Spotless the
0.50.x
release is not pressing as they can work around the problem with reflection but want to get rid of that. From a ktlint perspective, I would prefer to add some functional changes and then release
0.50.x
somewhere late in June. But, I can also imagine that from
ktlint-gradle
perspective a
0.49.2
release would be preferred although a patch release should not be used for a breaking change. What would your preference be from the
ktlint-gradle
perspective? Then there is another noteworthy issue #1981 about a deprecation in the upcoming Kotlin 1.9 version which will cause some rules to break. To assist API Consumers in finding those problems before the actual release, I have created a fix that allows you to disable the registration of the offending extension point to find all problems and mitigate them ahead in time. This issue does not necessarily need to be released immediately, because the snapshot version can be used (after merge #2044) to analyse if detekt rules require to be changed.
w

wakingrufus

05/21/2023, 5:55 PM
Ok thanks for the update. From our perspective, there isnt a huge difference in 0.49.2 vs 0.50.0 since we look at the semantic version down to the patch version anyway (there are certain patch versions we don't support and tell people to upgrade to the fixed version. 0.49.0->0.49.1 will be an example of that.) We also are already sharing implementations across multiple minor versions. For example, the old rule provider api is used for 0.45.x, 0.46.x, 0.46.x, etc. So I'd lean slightly toward a 0.50.0 version since we dont yet have different impl across patch versions, just a "block list" of certain patch versions. But either is fine for us. Thanks for running it by us.
p

Paul Dingemans

05/21/2023, 6:46 PM
Ok, for now there is no pressing need from any party to release this fix a.s.a.p. So it will be deployed with the next release. Up until now there is no need for another patch release on the
0.49.x
release.