Enable the javac -Xlint categories that Error Prone cannot see - #67
Open
ThoSap wants to merge 3 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the Checkstyle removal #66
Error Prone reads the typed javac AST, so it cannot see everything the compiler sees.
This PR turns on the 14 lint categories that only javac can check, and fixes the 4 warnings they surfaced.
The build now compiles with 0 warnings and 0 notes.
Why
Error Prone and
javaclint look at different things:The 82 Error Prone checks cover the AST side well. This PR closes the other side.
It deliberately does not enable a category that an Error Prone check already owns, so no problem is reported twice.
Where both can check a rule, Error Prone wins: it has per-check severity, so a rule fails the build on its own, while a javac lint category can only warn unless
-Werrorpromotes every warning at once.The 14 enabled categories
Code quality (11)
deprecationremovaluncheckedcastrawtypesdivzerothis-escapeidentitysynchronizationis now a deprecated alias. Error Prone'sValueClassIdentityexists only at HEAD, not in 2.50.0text-blocksdangling-doc-comments///markdown commentsrestrictedMemorySegment.reinterpret,System.loadandSystem.loadLibrary. Zero use today, so this is pure insuranceBuild hygiene (3)
options--releaseor source/target pairpathoutput-file-clashThe categories left off, and why
A. Error Prone already owns it - 11 categories
Enabling these in javac would report the same problem twice, and at a lower severity. Every Error Prone check below runs at ERROR.
emptyEmptyIfdep-annDepAnnfallthroughFallThroughfinallyFinallylossy-conversionsNarrowingCompoundAssignmentoverridesOverridesstaticStaticQualifiedUsingExpressionauxiliaryclassMultipleTopLevelClasses,ClassNameoverloadsAmbiguousMethodReference,FunctionalInterfaceClashvarargsExplicitArrayForVarargs,PrimitiveArrayPassedToVarargsMethod, plus 4 varargs checks that are ERROR by defaulttryTryWithResourcesVariable,MustBeClosedCheckerThere is also a severity argument. javac has no per-category severity: a lint category can only warn unless
-Werrorpromotes every warning at once. An Error Prone check fails the build on its own.B. Third-party noise - 3 categories
classfilemicroprofile-config-api-3.1.jar(35) andmicroprofile-health-api-4.0.1.jar(2). They reference OSGi annotations -org.osgi.annotation.bundle.Requirement,Requirements,versioning.Version- that are not on the compile classpath. Not our code, not fixable here.serialGrant,Role,Schema,Database,DefaultPrivilege,ClusterConnection. They inheritSerializablefrom the fabric8CustomResource, but they travel as JSON and are never Java-serialized. AserialVersionUIDon each would be pure ceremonyprocessingC. Not applicable - 8 categories
module,exports,opens,requires-automatic,requires-transitive-automaticandmissing-explicit-ctorall need a named module.The project has no
module-info.java.incubatingneeds an incubator module, andpreviewneeds a preview language feature.Neither is used, and
previewis on by default anyway.D. Deliberate aliases and near-misses
synchronizationis not listed.It is a deprecated alias of
identitysince JDK 25, and the toolchain's own--help-lintsays so.identityis listed instead.strictfpis not listed.It flags an unnecessary
strictfpmodifier, but all floating-point has been strict since Java 17, so the modifier has been meaningless for four releases.For it to fire, somebody would have to type a keyword that does nothing, in a codebase with zero
floatanddoubledeclarations.It is harmless to add and guards a mistake nobody here can make.
The 4 warnings that were fixed
removal- 1GrantReconcilerTest:SQLDataType.BIGINT.identity(true)→generatedByDefaultAsIdentity().The jOOQ method is deprecated and marked for removal.
Worth noting for a reviewer:
removalis on by default, so this warning was already visible before this PR. Nobody had looked.deprecation- 3HelmTest:Serialization.yamlMapper()twice andSerialization.jsonMapper()once, replaced by aKubernetesSerializationinstance held in a constant, withunmarshal(...).Reading also moves from
FiletoFiles.readStringandFiles.newInputStream.unchecked- 1ClusterConnectionReconcilerErrorTest: the mock creation carried//noinspection unchecked, an IntelliJ-only comment that javac ignores.It is now a real
@SuppressWarnings("unchecked")on a local variable declaration, with the value assigned to the field afterwards, because the annotation cannot sit on an assignment expression.Review notes
removalis redundant today, the same way two Error Prone entries are. javac enables it by default. It is listed so the intent is explicit and survives a JDK that changes the default.-Werroris now viable, and is not in this PR. The build is at zero warnings, so the ratchet would hold today. It is a separate decision, because javac promotes every warning, including a deprecation that arrives with the next jOOQ or fabric8 bump, and including any Error Prone check left at WARNING.-Xlintlist because a check inerrorprone.argsalready fails the build for it.Test scope
./gradlew --rerun-tasks :operator:compileJava :operator:compileTestJava :generated:compileJava- BUILD SUCCESSFUL, 0 warnings, 0 notes.-Xlint:allon the JDK 25 Amazon Corretto toolchain across all three source sets, with the warning cap raised so nothing was truncated. The category list came from that toolchain's ownjavac --help-lint, not from documentation.