Add checkerframework and errorprone

[juherr]

Try checkerframework and errorprone

[juherr]

@vlsi Any feedback about the 2 tools and how I added them in the build (even if I was inspired by what you did on pg-driver)?

@krmahadevan WDYT? They highlight a lot of minor issues but some potential problems too.

[krmahadevan]

@juherr - This is definitely a good addition. I am yet to explore both these tools. I have already seen error prone being used by jhipster when it scaffolds a new project based on my inputs.

Couple of questions:

• These highlight the problems in our code, but would it prevent TestNG from being used incorrectly by its consumers (especially the ones that use the TestNG apis to provide customised runners etc., ?)
• The CI has failed, mainly because of the discrepancies in our code, which you also highlighted. So for us to adopt these two tools, I believe we would need to iterate on all the failures and then fix them right ?

[juherr]

Thanks.

These highlight the problems in our code, but would it prevent TestNG from being used incorrectly by its consumers (especially the ones that use the TestNG apis to provide customised runners etc., ?)

As I understand, yes. It provides extra data in bytecode thanks to the annotations. The drawback is it is adding a new runtime dependency (because the annotations are stored in the bytecode).

The CI has failed, mainly because of the discrepancies in our code, which you also highlighted. So for us to adopt these two tools, I believe we would need to iterate on all the failures and then fix them right ?

To be honest, I hoped there were a bit less. That's why I sent the draft before fixing them all.

[vlsi]

"nullability" is not trivial, so it might take some time to properly annotate and identify nullable types. The general advice is assume "everything is not nullable by default" (==almost never put @NotNull), and annotate @Nullable where null values are allowed.

[vlsi]

In my experience, checkerframework nullability verification takes time, so I tend to activate it only when -PenableCheckerframework parameter is specified. Then checkerframework executes in a single CI job only.

See https://github.com/apache/jmeter/blob/c94db9741f0547235f9a67b1ece40389add6b742/build-logic/build-parameters/build.gradle.kts#L77 , and https://github.com/apache/jmeter/blob/c94db9741f0547235f9a67b1ece40389add6b742/build-logic/verification/src/main/kotlin/build-logic.style.gradle.kts#L42-L43 (conditional activation of .checkerframework.gradle.kts)

[juherr]

Thanks for the feedbacks @vlsi ❤️

[juherr]

Updated according to reviews.

All issues are not fixed yet.

Still 2 gradle issues: not succeeding in using the parameters plugin + not finding the way to merge package-info

[vlsi]

I would suggest merging errorprone first as it would require much fewer changes.

[juherr]

@vlsi Thanks for the review. I think your suggestion is a good strategy and I will make another PR with errorprone only.

Any idea why I'm not able to use build-parameters? The generated plugin is not found here https://github.com/testng-team/testng/pull/2941/files#diff-5bb00f6e0b0be0ec62ea3824c3888e27921683447cea9fb16bdd9f859f7881acR5