diffplug / spotless

Keep your code spotless
Apache License 2.0
4.55k stars 458 forks source link

`spotlessCheck` could add comments and suggested fix to GitHub PR #655

Open nedtwigg opened 4 years ago

nedtwigg commented 4 years ago

Per @rompic's great idea on gitter.

https://github.com/jenkinsci/violation-comments-to-github-plugin

rompic commented 4 years ago

Violation-comments to github uses https://github.com/tomasbjerre/violations-lib Which says in the read me: Missing a format? Open an issue here!

I may also be able to have a look in a few days/weeks

tomasbjerre commented 4 years ago

Or you can add an option in Spotless to create a report on one of the formats supported by violations-lib.

Meemaw commented 4 years ago

Integration with https://github.com/reviewdog/reviewdog would also be nice

roxspring commented 3 years ago

The reviewdog diagnostic format is designed to associate diagnostic messages with a code and a severity against sections of files. Correct me if I've misunderstood but I don't think Spotless has a variety of messages / codes / severities to offer. We'd be left to identify the suggested changes and produce uniform generic message / code / severity for each of them.

It appears that reviewdog also supports a diff input and given that Spotless is built around a simple Function<String, String> model, I wonder whether this might be a more appropriate integration point, producing a .diff file of changes (possibly reusing some diff logic from jgit), and leaving reviewdog to process this into PR comments etc.

nedtwigg commented 3 years ago

The reviewdog data format includes a suggestions with replacementText field, which can accomplish the same things as the diff format. IMO it's a bit easier to generate than .patch, because you don't have to futz with the context lines which allow normal patches to be applied even if the line numbers change.

I agree that the messages/codes/severities will all be the same, except for failures. Spotless is designed around Function<String, String>, but sometimes a formatter will instead throw an Exception (which we consider to be an internal failure) or an Error (which we consider to be an intentional error message for the user).

In particular, ktlint throws errors for lots of things which don't fit the Function<String, String> model. I'm not a fan of linters of this type, but some people are, and it would be nice for Spotless to support them.

If you make a PR which adds a spotlessDiff task, I'd be happy to merge it, but I'm not sure what the correct behavior should be if a formatter errors out on some specific files. IMO, it is just as easy to integrate with the reviewdog format as the diff format, but with the added simplicity that you can always generate a complete report even for formatters which throw lots of errors like the very popular ktlint.

nedtwigg commented 2 years ago

A PR against #1097 which pipes Lint.java into either https://github.com/Contrast-Security-OSS/java-sarif (add it to lib-extra) or any other reviewdog format which supports "suggestions" would be very welcome. We need to figure out which fields Lint needs, and that's a tug of war between