Replace Checkstyle with Error Prone

[ThoSap]

Checkstyle is removed from the build. Error Prone takes over the rules it enforced, and adds a large set it never had.
82 checks now run at ERROR, up from 2.

Every check was measured before it was enabled. No production logic changed.

See https://errorprone.info/bugpatterns for rule descriptions.

---

Why

It is cumbersome for new PR contributors to setup a GitHub Token just so they can download the AboutBits Java Checkstyle Config and compile the project.
I also saw in PR #60 that new contributors will easily get things wrong which could be prevented by something like Error Prone, for example the Varifier rule I meantioned in the aforementioned PR.

Error Prone was already wired in and running 54 checks at ERROR by default since the very start of the project.
Moving the rules there removes the blocker, removes a build tool, and removes the GitHub Packages credentials that only the Checkstyle config needed.

Also the Checkstyle upgrade was blocked.
Checkstyle 13.9.0 removed the JavadocStyle module, and the shared config it.aboutbits:java-checkstyle-config still declares it in every published tag, RC2 through RC5.
An unknown module name is a config load failure, not a violation, so the suppressions file cannot absorb it. Both config/checkstyle/*.xml files are regenerated from the artifact on every run, so the fix could not live in this repository.

What the Checkstyle config actually enforced

The baseline is smaller than the module count suggests. RC5 declared 62 active modules:

• 6 Javadoc modules enforced nothing. The bundled checkstyle-suppressions-default.xml carries
  <suppress checks="Javadoc" files="."/>, a substring regex over every file. JavadocMethod,
  JavadocType, JavadocVariable, JavadocStyle, MissingJavadocMethod and
  InvalidJavadocPosition were all off.
• 1 module was dead config. IllegalInstantiation has an empty classes property by default, so it reported nothing.
• 4 more were relaxed in tests: MethodName, VisibilityModifier, FileTabCharacter,
  FileLength.

Real baseline: 55 enforced rules on main sources.

Coverage

	Count
✅ Fully replaced	17
🟡 Partially replaced	7
⚪ Was dead config	1
❌ Gap	37

✅ Fully replaced (17)

Checkstyle module	Error Prone check
AvoidStarImport	WildcardImport
UnusedImports	RemoveUnusedImports
ParameterNumber (max 7)	TooManyParameters, limit pinned to 7
NeedBraces	MissingBraces
EqualsHashCode	EqualsHashCode (ERROR by default)
InnerAssignment	AssignmentExpression
MissingSwitchDefault	MissingDefault
MultipleVariableDeclarations	MultiVariableDeclaration
HideUtilityClassConstructor	PrivateConstructorForUtilityClass
InterfaceIsType	InterfaceWithOnlyStatics
ArrayTypeStyle	MixedArrayDimensions
UpperEll	LongLiteralLowerCaseSuffix
JspecifyOnTopLevelTypes (custom)	RequireExplicitNullMarking - exact same rule
InvalidJavadocPosition	AnnotationPosition, NotJavadoc, AlmostJavadoc
JavadocMethod	InvalidParam, InvalidThrows, InvalidLink, InvalidInlineTag, MalformedInlineTag
JavadocType	InvalidBlockTag, MissingSummary
JavadocStyle	MissingSummary, EscapedEntity, UnescapedEntity, UnrecognisedJavadocTag

🟡 Partially replaced (7)

Checkstyle module	Error Prone	What is narrower
ConstantName	ConstantField	Inverse rule. It demands static final for a CONSTANT_CASE name, but does not force a constant to be CONSTANT_CASE
RedundantModifier	UnnecessaryFinal	Locals and parameters only. Not public on interface members
EmptyBlock	EmptyCatch, EmptyIf	No general empty-block rule
SimplifyBooleanExpression	BooleanLiteral, ComplexBooleanConstant	Overlapping, not identical
VisibilityModifier	MutablePublicArray, ProtectedMembersInFinalClass	No "fields must be private" rule
JspecifyAnnotationOrder (custom)	AnnotationPosition	Same rule for type-use annotations, which is the case that matters. It found the one real violation in RoleReconcilerTest
JspecifyInlineTypeUse (custom)	AnnotationPosition	As above

❌ The 37 gaps

A. Layout - 16 modules, the one real loss

EmptyForIteratorPad, GenericWhitespace, MethodParamPad, NoWhitespaceAfter,
NoWhitespaceBefore, OperatorWrap, ParenPad, TypecastParenPad, WhitespaceAfter,
WhitespaceAround, LeftCurly, RightCurly, ModifierOrder, FileTabCharacter,
NewlineAtEndOfFile, RegexpSingleline (trailing spaces).

Error Prone matches on the javac AST and never sees the characters. The right owner is a formatter, which rewrites instead of reporting.
Follow-up: Spotless with google-java-format or palantir-java-format.

B. Naming - 8 modules, a deliberate decision

TypeName, MethodName, MemberName, LocalVariableName, LocalFinalVariableName,
ParameterName, StaticVariableName, PackageName.

IdentifierName covers all eight and is deliberately not enabled. It also enforces the Google acronym rule, so it demands 23 renames of public types and members: SQLUtil → SqlUtil,
CRStatus → CrStatus, PostgreSQLContextFactory → PostgreSqlContextFactory, getDSLContext → getDslContext.
That is a naming-policy change, not a tooling swap, and it belongs in its own PR.

C. Remaining 13, low value

Module	Note
MethodLength (max 150)	No counterpart. Two methods carried a suppression for it
FileLength	No counterpart
EmptyStatement	UnnecessarySemicolon is the counterpart but stays off: Lombok makes it report every @Getter and @Setter, 1547 times over 39 positions
SimplifyBooleanReturn	No counterpart
FinalClass	No counterpart
AvoidNestedBlocks	No counterpart
IllegalImport (sun.*)	No counterpart
RedundantImport	No counterpart. Duplicate and java.lang imports; the IDE flags these
TodoComment	No counterpart
Translation	No counterpart. .properties key parity
JavadocVariable	Required Javadoc on every field. Was suppressed anyway
MissingJavadocMethod	Error Prone's MissingJavadoc exists only at HEAD, not in 2.50.0. Was suppressed anyway
JspecifyMapStructMapperAnnotation (custom)	No counterpart, and moot: the project has no MapStruct

On the "linting versus style" framing

The two tools do not divide that way, and a reviewer should not read the change that way.

Error Prone is not linting only.
Of the 82 checks now at ERROR, 12 carry Google's own StandardTags.STYLE tag, read from the @BugPattern annotations in the 2.50.0 sources:

ConstantField, EmptyCatch, MissingBraces, MissingOverride, MixedArrayDimensions,
MultiVariableDeclaration, MultipleTopLevelClasses, PackageLocation, RemoveUnusedImports,
SwitchDefault, UnnecessaryStaticImport, WildcardImport

Four more carry FRAGILE_CODE, and 64 carry no tag, because the attribute is optional.
Varifier, UnnecessaryParentheses, TooManyParameters and LongLiteralLowerCaseSuffix are pure style too.

Checkstyle was not style only. Its coding category held 8 modules and design held 4. Those are correctness and API rules.

The real axis is what each tool can see:

	Error Prone	Checkstyle
Input	The typed javac AST - types, symbols, dataflow, and a parsed Javadoc AST	The token stream and an untyped AST, including whitespace, raw text and comments
Can express	Bug rules and style rules, as long as the rule lives in the AST	Layout rules, and semantic rules weakly, without type information
Cannot express	Anything depending on characters and positions: indentation, padding, brace placement, tabs	Anything needing a type, a symbol or dataflow

There is a quality difference worth stating: Checkstyle judges without type information.
Its EqualsHashCode matches on method names. Error Prone's resolves types and symbols. Same rule name, stronger guarantee.

What the migration gains

1. Javadoc goes from zero rules to 15 at ERROR. The blanket suppression meant Checkstyle enforced nothing there. This is the largest single gain, and it serves the /// markdown comments directly.
2. 82 checks at ERROR against a baseline of 55. Roughly 46 have no Checkstyle counterpart at all:
   CheckReturnValue, ReferenceEquality, MissingOverride, UnusedVariable, Varifier, UseEnumSwitch, the varargs family, and the rest.
3. Failures arrive at compile time. Checkstyle was a separate task a developer could forget. The pre-commit hook now runs compileJava compileTestJava.
4. Per-rule severity, and per-source-set relaxation through the Gradle DSL, replacing the suppressions XML.
5. No registry credentials. The shared config was the only artifact from GitHub Packages. Contributors no longer need a token, and both workflows lost their credential env blocks.
6. One duplicated rule removed. RequireExplicitNullMarking and the custom
   JspecifyOnTopLevelTypes enforced the same rule twice.

Configuration layout

The block is grouped by topic, and alphabetical inside each group:

Group	Checks
Nullness	5
Javadoc	15
Imports	4
Naming and source layout	5
Blocks, statements and switches	9
Class design	5
Correctness	23
Redundant code and house style	16
Turned off	1
Total	82 at ERROR, 1 at OFF

Of the 82, 43 rise from WARNING and 35 rise from DISABLED. Two (NullAway, RequireExplicitNullMarking) come from the NullAway artifact, whose defaults are WARNING and SUGGESTION.

Review notes

• Two entries are redundant today. CheckReturnValue and SelfComparison are already ERROR by default in Error Prone 2.50.0. They are listed so the policy is explicit and survives a release that lowers a default. Say the word and they come out.
• UnnecessarySemicolon is deliberately off. It replaces Checkstyle EmptyStatement, but Lombok makes it fire on every @Getter and @Setter: 1547 reports from 39 source positions.
• MissingOverride must stay at ERROR. NullAway needs it for the exhaustive override checks.
• lombok.checkReturnValueAnnotation = lombok pairs with CheckReturnValue at ERROR. Verified:
  Error Prone matches the annotation by simple name, so @lombok.CheckReturnValue is honoured. It generates nothing today, because the codebase has no @With and no @Builder.

Source changes

Five files, all mechanical:

• 6 obsolete @SuppressWarnings("checkstyle:MethodLength") entries removed from GrantReconciler, GrantService, GrantReconcilerTest and HelmTest. The java:S3776 entries stay.
• RoleReconcilerTest:
  private @Nullable <T> T getRoleFlagValue( → private <T> @Nullable T getRoleFlagValue(.
  A type-use annotation belongs after the type parameter. This is the only violation any newly enabled check found.

Build and infrastructure changes

File	Change
build.gradle.kts	The checkstyle plugin, apply(plugin = "checkstyle"), the tasks.withType<Checkstyle> block, the checkstyleConfig configuration, the checkstyleExtractConfig task and the checkstyle { } block are gone. The Error Prone block replaces them
gradle/libs.versions.toml	checkstyleConfig and checkstyle removed, versions and library entries
settings.gradle.kts	The GitHub Packages repository block, 27 lines. It served the it.aboutbits group only
.gitignore	config/, the suppressions exception and !.idea/checkstyle-idea.xml removed
.githooks/pre-commit	checkstyleMain checkstyleTest → compileJava compileTestJava
.github/workflows/test.yml, release.yml	The GITHUB_USER_NAME and GITHUB_ACCESS_TOKEN env blocks, which fed the removed repository
README.md	The registry setup section, 13 lines
deleted	config/ and .idea/checkstyle-idea.xml

Test scope

• ./gradlew --rerun-tasks :operator:compileJava :operator:compileTestJava :generated:compileJava - 0 Error Prone findings.
• ./gradlew build -x test - successful.
• Two identical compileJava compileTestJava runs - configuration cache reused.
• ./gradlew tasks --all | grep -i checkstyle - nothing.
• Dependency resolution succeeds without the GitHub Packages repository, which confirms nothing else came from it.
• Severity was proven live, not assumed: with UnnecessarySemicolon temporarily at ERROR the build failed with error: [UnnecessarySemicolon].

Method

Every candidate was compiled at WARN across all three source sets with the warning cap raised, and only checks at zero violations were promoted. Four measurement rounds, 83 candidates. Two produced findings: UnnecessarySemicolon (excluded) and AnnotationPosition (one violation, fixed).

[ThoSap]

I a call I mentioned this to @SirCotare and he in favor of doing this, especially as it now it is less cumbersome for new contributors to start the project and we do not really love Checkstyle.

I will also create a research task for Spotless and will open a RFC.