spotless icon indicating copy to clipboard operation
spotless copied to clipboard

KTlint reporter arguments

Open JustinTullgren opened this issue 6 years ago • 9 comments
trafficstars

Hi,

I was wondering if it is possible to specify the reporter for ktlint? The userData map doesn't seem to send cli args but just editor config args, though I could be misunderstanding.

I have tried a few variations:

kotlin {
        ktlint(Versions.KT_LINT)
            .userData(mapOf("color" to "", "--reporter" to "checkstyle"))
}
kotlin {
        ktlint(Versions.KT_LINT)
            .userData(mapOf("color" to "", "reporter" to "checkstyle"))
}

If this is supported I would appreciate a pointer on how to accomplish this. If it is not I would request a feature enhancement for it.

Thanks!

JustinTullgren avatar Feb 08 '19 01:02 JustinTullgren

Here's the code for anyone who wants to contribute a PR: https://github.com/diffplug/spotless/blob/b38ee5a7b9f47bfe6a38749a19d09a751044b6eb/lib/src/main/java/com/diffplug/spotless/kotlin/KtLintStep.java

nedtwigg avatar Feb 08 '19 01:02 nedtwigg

Thanks @nedtwigg for the quick reply. I am traveling this week but if no one volunteers by the time I get back I will take a stab at it. Thanks

JustinTullgren avatar Feb 08 '19 01:02 JustinTullgren

Closing due to inactivity, happy to reopen on request.

nedtwigg avatar Aug 20 '19 06:08 nedtwigg

@nedtwigg any updates here? I want to get the ktlint reports to pass it to other tool.

Ahmed-Abdelmeged avatar Dec 22 '20 16:12 Ahmed-Abdelmeged

No changes. Happy to take a PR for this feature, but as of now no one has plans to work on this.

nedtwigg avatar Dec 22 '20 19:12 nedtwigg

I never did a plugin dev before but I will try my best to make this PR. Thanks! @nedtwigg

Ahmed-Abdelmeged avatar Dec 22 '20 19:12 Ahmed-Abdelmeged

Hi, @nedtwigg after a lot of investigation. KtlintStep uses the KtLint.fromat() internally and access it via reflection. And most of Ktlint useful functionally like reporters are a CLI commands written here. https://github.com/pinterest/ktlint/blob/master/ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt. I tried to port them to Spotless but that was too much work and felt like writing Ktlint internals again but in Java! From my understanding of how Spotless internally work (Correct me if I'm wrong) You can't just call the library CLI commands instead of access its classes and call it via reflection. Any suggestions?

Ahmed-Abdelmeged avatar Jan 04 '21 10:01 Ahmed-Abdelmeged

Spotless is just a Function<String, String>, and that's all (see CONTRIBUTING.md).

  • The fastest way to implement this is to pass a string to a JVM function, and get a string back
  • But we also have steps (e.g. clang and black) where we pass a string on stdin and get the result on stdout, which requires a spawning a whole new process for every single file
  • The npm steps (e.g. prettier) even start a node.js server and pass data over localhost

For a JVM tool like ktlint, you get a huge speed improvement by calling a JVM function over and over so that the JIT can warm up. If you spawn a new process for every file, it will be very slow. But you'll still get buildcache and up-to-date caching, so it's not insane.

So you can absolutely build a ktlint step which calls java -jar ktlint.jar --stdin and then pass the file content on stdin, and parse the output on stdout, and wrap that whole mess as a Function<String, String>. But if ktlint doesn't have the public APIs that you need, it would probably be easier to get ktlint to accept a PR which adds them. Presumably ktlint is useful in lots of IDE-like scenarios, do they really intend the only API to be the CLI? If they do, I would at least have a way to pass arbitrary streams to use as stdin and stdout. Then you don't have to spawn a new JVM for every file, and can instead call ktlint.main(argsarray) and reuse the streams (something like this).

If the existing ktlint() step is too restrictive, I'd be open to adding a ktlintCli() or something which has the tradeoff that it is slower, but has more features. But mostly I think it would be best to get ktlint to commit to a public API so that an ecosystem can exist around it.

nedtwigg avatar Jan 04 '21 19:01 nedtwigg

Now that #1012 has merged, it should be easier to build this if anybody wants to contribute a PR.

nedtwigg avatar Dec 05 '21 00:12 nedtwigg