mikaelgrev / miglayout

Official MiG Layout for Swing, SWT and JavaFX
miglayout.com
423 stars 76 forks source link

Consider adding ErrorProne for capturing bad coding patterns during the build #95

Closed vlsi closed 1 year ago

vlsi commented 1 year ago

See https://errorprone.info/

For instance, it could have detected default locale issue: https://errorprone.info/bugpattern/StringCaseLocaleUsage

I would not make a PR though as I don't want spending time on pleasing Maven

tbee commented 1 year ago

Interesting. Yet another code analyzer. What makes errorprone different form e.g. sonarqube? Or is it just the fact that it in integrated into the compiler?

And please don't tell me you're into pleasing Gradle instead? ;-)

vlsi commented 1 year ago

Errorprone is fast, it provides immediate feedback, and its failures are actionable. The defaults for Errorprone are great. Sonarqube is slower, and I think it would be much harder to configure.

In other words, errorprone is something you just configure, and you can let it to fail builds on violations

vlsi commented 1 year ago

If you think something else is better, that is fine with me

tbee commented 1 year ago

No, it's interesting. But the number of issues it finds are fairly small, according to comparison websites. So I was just curious why you suggested it.

I've configured it; it doesn't fail in on the default settings, but MigLayout has a lot of warnings, so the relevant ones by error-prone are missed in the masses. Forcing the StringCaseLocaleCheck to become an error (and reintroduced one) makes it fail. It at least prevents new mistakes of this type, so I'll leave it in.

tbee commented 1 year ago

I'll also configure snyk, see what it has to say.

tbee commented 1 year ago

Thanks!

vlsi commented 1 year ago

Have you enabled just StringCaseLocaleUsage?

In my experience, all or almost all the “error by default” checks in errorprone make total sense. You are right the number of checks is not high, but they are great, and they produce meaningful messages on failures

tbee commented 1 year ago

I did not find a flag to enable all.

https://errorprone.info/docs/flags

vlsi commented 1 year ago

I did not find a flag to enable all.

I missed that https://errorprone.info/bugpattern/StringCaseLocaleUsage is "warning by default".

Typically I configure projects with -Werror, so StringCaseLocaleUsage reports with no extra configuration.

tbee commented 1 year ago

If I do that I need to fix A LOT of stuff. Maybe someday.

vlsi commented 1 year ago

I am not sure it would really take long to fix as IDEs have automatic fixes for many issues. I've fixed several issues (see https://github.com/mikaelgrev/miglayout/pull/96), however, then Maven stopped working, so I'll switch to something else.

tbee commented 1 year ago

I'm not sure having -Werror active at all times is something I'm very happy with. But time will tell. The project is in maintenance mode, that may make a difference