Add gradle thirdPartyAudit to precommit tasks

[rmuir]

Currently, we do all kinds of checks on our own code, but dependencies are no different. The computer does not care if you wrote it, or dragged it in via a dependency.

This task fails for two situations (implemented with forbidden apis checks):

1. dependency classpath is incomplete: fails the build if third party dependencies refer to other classes that do not exist. Really, when classes are missing, this is just asking for trouble e.g. NoClassDefFoundError, but in some cases its actually intentional (e.g. optional dependency of a dependency). In those cases, this check can be disabled with thirdPartyAudit.lenient = true
2. use of internal JDK classes: fails the build, unless exact third party class doing this is excluded in thirdPartyAudit.excludes. Wildcards are explicitly not permitted in exclusion lists.

We can expand in the future, e.g. add additional forbidden apis beyond just internal JDK apis for 3rd party jars if we want. Probably better to worry about sorting out the messes discovered here first (I don't fix em, i just document what they are).

[nik9000]

thirdPartyAudit.lenient = true

thirdParyAudit.ignoreClassNotFound = true? lenient leaves a lot to the imagination.

[rmuir]

That is intentional, because of this problem, we have to hide all forbidden APIs warnings (otherwise the thousands of missing classes would overflow your screen).

So yes, it should be leaving stuff to the imagination, like "what else could be wrong".

[rjernst]

LGTM

[nik9000]

So yes, it should be leaving stuff to the imagination, like "what else could be wrong".

Got it.

[uschindler]

The unzipping is not nice, but looks fine. The alternative would be to not use forbiddenapis ANT task, but instead instantiate the forbiddenapis Checker class directly, pass custom logger with INFO and ERROR, but no WARN, and finally feed all classes from a ZipInputStream or similar to prevent the "Ant O(n^2) zipfileset" bug. Backside: You need a bit more code to fix the Checker initialization (classloader, load classes from ZIP stream)

+1

[nik9000]

Tried it locally as well. LGTM.

[rmuir]

Instead of lenient i would also be fine with CLASSES_ARE_MISSING

Basically if we have this situation, anything goes. I thought lenient would make it sound appropriately negative, but something in all-upper-case like that would be fine too. :)

[rmuir]

I am working on improving it further, to eventually remove lenient. I think there should only be excludes, that is where you list classes, if classes are wrong in any way, including if they are missing. I also have some new checks to add that reveal additional problems.

This is better as it always "keeps tabs", which is what we need. It does require doing a little hack to parse the forbidden apis warnings :)

The me, the point of these checks right now is to have nice documentation of issues for every module, not necessarily even to try to fix them. We have to at least know what is happening and what code is or is not running!