Automatic cleaning of the codebase

[blacelle]

I contribute on a Github application dedicated to the automatic cleaning of a code-base. I recently introduced into it the ability to apply SpringBoot java-formatter rules.

It also holds a set of automatic rules to clean quality issues (as they are reported by Checkstyle/PMD/etc). This set of rules is yet quite small, but growing.

Would it be of interest to enable such an application into this repository? SpringFormatter can be applied through various tools and IDE, but some users would benefit from not having to run these tools manually. Moreover, the automatic cleaning may help ensuring some quality standards (a step further existing checkstyle rules).

As a matter of example, here what would look like such an automatic cleaning step:
solven-eu#2

If relevant, the source of this GitHub application is available there:
https://github.com/solven-eu/cleanthat

[philwebb]

Thanks for the offer, but I'm not sure we want to add additional hooks to our GitHub repo. We generally prefer Gradle plugins so that we can run the checks locally before committing. We have source formatting and checkstyle already in our build.

If there's specific checks that your project does that you think we're missing, perhaps we could add them to our checkstyle rules.

[blacelle]

Fine @philwebb .

If there's specific checks that your project does

My project focused on automatic cleaning, not only checks. Still, I fully understand an additional Github hook is not satisfying. Out of curiosity, would it be interesting for SpringBoot if I provided a Gradle plugin? (which behavior would then be similar to https://github.com/spring-io/spring-javaformat code formatting)

If there's specific checks that your project does that you think we're missing

The referred PRs auto-cleaned multiple instances of https://rules.sonarsource.com/java/RSPEC-2976

[philwebb]

To be perfectly honest, I'm not a massive fan of automatically fixing problems detected by tools. Over the years we've had quite a few people raise pull-requests to fix things that have been detected by tools. There's been mixed success. Some of the issues were valid and we fixed them, but others missed subtleties in the code or were just rules that we didn't agree with.

My preference is generally to report the error during the build, but let the developer decide what the fix should be. That's one of the reasons I like our current checkstyle setup. It reports problems, but ultimately lets us decide how to fix or ignore them.

I think checkstyle and findbugs issues are different to formatting. Apply formatting rules doesn't actually change anything about the code itself.

[blacelle]

Thanks @philwebb , your feedback is very valuable.

Some of the issues were valid and we fixed them, but others missed subtleties in the code or were just rules that we didn't agree with.

I totally get this. This is one reason why such rules should be configurable just like checktyle/findbugs. Do you have, by any chance, any pointers/keywords to find back such issues? I would be happy to document such too-much-opinionated rules, and possibly implement them, but not activating them by default.

I think checkstyle and findbugs issues are different to formatting. Apply formatting rules doesn't actually change anything about the code itself.

I feel some rules changing the code could be considered like formatting. Indeed, it changes the runtime in most cases. But, as a first step for this project, I focus on rules which most developper will agree, and for which a project owner could decide given rules should apply in all cases (or be excluded a way or another). These rules turns to be style-rules (e.g. turning List.size() == 0 into List.isEmpty()), deprecation-rules (e.g. turn new Double(d) into Double.valueOf).

This rules would then be a way to forbid bad-style, automatically converting into equivalent good-style code (hence the parallel with formatting).

to fix things that have been detected by tools

Typically, here, the point here is mainly to fix issues reported by checkstyle/findbugs/PMD. Not to propose clumsy automatic-fixes to opinionated rules (to this regard, https://rules.sonarsource.com/java/RSPEC-2976 was probably not the best example).

[philwebb]

Do you have, by any chance, any pointers/keywords to find back such issues?

A few recent ones are #23111 #21530 #21995 #22637 #24817. I'm sure there are more. Often they're suggestions from IDE tooling.