Add checkstyle auto-fix tooling by sercuzz8 · Pull Request #9017 · apache/storm

sercuzz8 · 2026-08-28T16:03:59Z

After this comment, implement a groovy tool to auto fix checkstyle violation to avoid post hoc fixes

reiabreu · 2026-09-05T02:51:20Z

@GGraziadei are you happy that this fulfils your request?

GGraziadei · 2026-09-05T08:32:08Z

Hi @sercuzz8,
Good attempt! However, this plugin doesn’t fix all the warnings.
Have you tried to add more rules for org.codehaus.gmavenplus? Have you tried a different tool (with a parser support)?
Otherwise, I propose we keep the code changes and formatting, but exclude this plugin from mvn validate.

GGraziadei · 2026-09-18T20:44:42Z

Hi @sercuzz8 any updates on this PR?

sercuzz8 · 2026-09-18T21:35:55Z

Hello @GGraziadei,
I am sorry for the delay. I have been trying to either expand this solution or use other tools (Openrewrite, Spotless) to eradicate warning.
Unfortunately I have not managed to overcome all the warnings, including those regarding line length as breaking them sometimes breaks compilation in the process.
Given these conditions, I suppose that the best idea would be to remove this plugin from the validate, as you said.
What do you think?

sercuzz8 · 2026-09-19T14:21:04Z

Update: I took the need for a rebase as an occasion to improve the implementation.
It now uses Openrewrite as much as possible and complements with the original groovy script.
Some violations can't be solved mechanically (missing Javadoc, long lines being unbreakable URLs) so I removed the script from mvn validate.
The second commit contains the result of the run of the command

GGraziadei · 2026-09-20T10:52:39Z

Hi @sercuzz8 very good progress on this side. Thanks!
The next steps:

Migrate from Groovy to OpenRewrite

Please proceed according to this table if a OpenRewrite rule is available.

Checkstyle module	Groovy fixer today	OpenRewrite recipe	Coverage	Notes
`NeedBraces`	yes (fallback)	`org.openrewrite.staticanalysis.NeedBraces`	Full	already in `rewrite.yml`
`MultipleVariableDeclarations`	yes (fallback)	`org.openrewrite.staticanalysis.MultipleVariableDeclarations`	Full	already in `rewrite.yml`
`ModifierOrder`	yes (fallback)	`org.openrewrite.staticanalysis.ModifierOrder`	Full	already in `rewrite.yml`
`UpperEll`	yes (fallback)	`org.openrewrite.staticanalysis.UpperCaseLiteralSuffixes`	Full	already in `rewrite.yml`
`ArrayTypeStyle`	yes (fallback)	`org.openrewrite.staticanalysis.UseJavaStyleArrayDeclarations`	Full	already in `rewrite.yml`
`WhitespaceAround`, `WhitespaceAfter`, `NoWhitespaceBefore`	yes	`org.openrewrite.java.format.Spaces`	Style	`SpacesStyle` must mirror the ruleset; whole-file reformat
`ParenPad`, `MethodParamPad`, `GenericWhitespace`	yes	`org.openrewrite.java.format.Spaces`	Style	same `SpacesStyle`
`NoWhitespaceBeforeCaseDefaultColon`	yes	`org.openrewrite.java.format.Spaces`	Partial (verify)	not sure the style has a knob for `case X :`
`RegexpSinglelineJava` (empty-block `{}` spacing)	yes	`org.openrewrite.java.format.Spaces`	Partial (verify)	regex-based rule, no 1:1 recipe
`FileTabCharacter`	yes	`org.openrewrite.java.format.NormalizeTabsOrSpaces`	Full
`Indentation`	yes	`org.openrewrite.java.format.TabsAndIndents`	Style	`TabsAndIndentsStyle` with `basicOffset` and continuation indent
`CommentsIndentation`	yes	`org.openrewrite.java.format.TabsAndIndents`	Partial	comments follow the enclosing indent, not every case Checkstyle checks
`EmptyLineSeparator`	yes	`org.openrewrite.java.format.BlankLines`	Style	`BlankLinesStyle`
`LeftCurly`, `RightCurly`	yes	`org.openrewrite.java.format.WrappingAndBraces`	Style	the `RightCurly alone` regression the PR saw comes from the Checkstyle-derived style, not the recipe; a custom `WrappingAndBracesStyle` avoids it
`NoLineWrap`	yes	`org.openrewrite.java.format.WrappingAndBraces`	Partial	package/import wraps only
`AnnotationLocation`	yes	`org.openrewrite.java.format.WrappingAndBraces`	Partial	annotations on their own line for types/methods; field and parameter cases not covered
`OneStatementPerLine`	yes	—	None	small custom visitor
`OperatorWrap`	yes	`org.openrewrite.staticanalysis.OperatorWrap`	Full	has an `nl`/`eol` option; use `nl` to match the ruleset
`SeparatorWrap`	yes	—	None	comma/dot wrap position; custom visitor
`LineLength`	yes (comments, arguments, `&&`/`\|\|`/`+`, call chains, `=`, string split)	—	None	OpenRewrite has no column-based wrapping; this is the largest Groovy fixer and stays
`CustomImportOrder`	yes	`org.openrewrite.java.OrderImports`	Style	`ImportLayoutStyle` with the ruleset's group order; set `classCountToUseStarImport` and `nameCountToUseStarImport` very high, which removes the "collapses to star imports" problem the PR hit
`AvoidStarImport`	yes (expands via module classpath)	`org.openrewrite.java.RemoveUnusedImports`	Partial (verify)	unfolds star imports only with type attribution, i.e. the module's dependencies resolvable, the same constraint the Groovy has
`IllegalTokenText`, `AvoidEscapedUnicodeCharacters`	yes	—	None	trivial custom recipes
`TodoComment`, single-line comment space (`MatchXpath`)	yes	—	None	trivial custom recipes
`OverloadMethodsDeclarationOrder`	yes	—	None	custom visitor reordering class members
`ConstructorsDeclarationGrouping`	yes	—	None	same visitor
`MissingSwitchDefault`	yes (adds `default: break;`)	`org.openrewrite.staticanalysis.DefaultComesLast`	Partial	only reorders an existing `default`; adding one needs a custom recipe
`JavadocLeadingAsteriskAlign`, `JavadocMissingLeadingAsterisk`	yes	—	None	the Javadoc LST exists, but no formatting recipes ship
`JavadocContentLocation`, `JavadocParagraph`, `JavadocTagContinuationIndentation`	yes	—	None	custom Javadoc visitors
`RequireEmptyLineBeforeBlockTagGroup`, `AtclauseOrder`, `InvalidJavadocPosition`	yes	—	None	custom Javadoc visitors
`SummaryJavadoc` (missing period, lowercase first word)	yes	—	None	custom Javadoc visitor

Fix the license note

The correct license note is this one https://www.apache.org/legal/src-headers.html#headers
Please evict to add additional <p> or other characters and uniform between the whole codebase.

Trasform this PR in a GitHub action

The amount of code touched by this PR is too large to review. Could you drop the source fixes from this PR and keep only the tooling, and then add a GitHub Actions workflow (workflow_dispatch) that:

takes the module to fix as an input;
runs the auto-fix on that module;
pushes the result to a checkstyle-fix/<module> branch, so we can open one PR per module against master.

OpenRewrite is preferred: everything it has a recipe for (fully or partly) runs there, driven by a style in rewrite.yml that mirrors storm_checkstyle.xml. The Groovy script only keeps the rules with no recipe (LineLength, OneStatementPerLine, SeparatorWrap, Javadoc rules, member ordering, trivial text rules). A second OpenRewrite execution re-indents after the Groovy stage. A Groovy script normalizes license headers to the ASF template, leaving third-party works alone. Wired into the root and module poms without binding it to validate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…per module Takes the module as input, runs OpenRewrite, the Groovy stage and the re-indent pass plus the license header normalization on it, and pushes the result to checkstyle-fix/<module> so one pull request per module can be opened against master. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

GGraziadei · 2026-09-20T14:01:40Z

+          distribution: temurin
+          java-version: 25
+      - name: Build the module and the modules it depends on (the Groovy stage resolves its classpath)
+        run: mvn --batch-mode install -DskipTests -Dcheckstyle.skip=true -pl "$MODULE" -am


I advice to execute a checkstyle before and after including the results inside the job artifact of GitHub action.

GGraziadei

Hi @sercuzz8 good progress, thanks!

Some Claude's findings in addition to the inline comments

normalize-license-headers.groovy:74 — greedy # header match deletes non-license comments

fixHash matches the whole leading run of # lines (/\A(?:[ \t]*#[^\n]*(?:\n|\z))+/) and
replaces all of it with the template when it contains the license. Any comment glued to the
header is lost. Concrete cases in the repo: integration-test/config/install-storm.sh loses
# $1 is the storm binary zip file, and integration-test/run-it.sh loses its
# -*- compile-command: ... -*- line. A # -*- coding: -*- line in a Python file would go the
same way. Since the workflow commits and pushes this step automatically, the loss would go
unnoticed. The match should stop at the first blank line or at the end of the license text.

checkstyle-autofix.groovy:762 (illegalTokenText) — \u0020 / \040 turned into a raw space

ESCAPES[32] = ' ' rewrites \u0020 and \040 as a literal space. IllegalTokenText is
configured for TEXT_BLOCK_CONTENT and its format includes 20 / 040. Inside a text block a
trailing \u0020 is the idiomatic way to preserve a trailing space; a raw space is stripped by
text-block processing, so the string value changes. No text blocks exist in Storm today, so this
is latent, but the fixer should skip that escape (or skip text blocks altogether).

checkstyle-autofix.groovy:1305-1315 (splitString) — a split can cut a 3-digit octal escape

Safe cut points advance 2 chars for any non-\u escape, so a 3-digit octal escape such as \123
can be cut into "…\12" + "3…", which changes the string (newline followed by '3' instead of
'S'). There are no octal escapes in Storm sources today, but the scanner should consume up to
three octal digits after the backslash.

.github/workflows/checkstyle-autofix.yaml:51 — modules without the plugins run as silent no-ops

storm-webapp, storm-submit-tools and storm-multilang/python declare
maven-checkstyle-plugin but were not given the two new plugins. Dispatching the workflow for
them passes the "module exists" check, rewrite:run@… and gmavenplus:execute@… run with no
configuration ("no active recipes" / "no scripts"), and the job ends with "Nothing to fix". It
would be better to either add the plugins to those modules or fail the job when the executions
are not configured.

pom.xml:1176 — misleading comment on the gmavenplus pluginManagement

The comment says the fixer "Runs at validate, before the checkstyle", but both executions are bound to <phase>none</phase> (correctly, per the README). Please update the comment so the next maintainer is not misled.

GGraziadei · 2026-09-20T14:03:39Z

+        run: mvn --batch-mode -pl "$MODULE" -Dcheckstyle.skip=true org.codehaus.gmavenplus:gmavenplus-plugin:execute@normalize-license-headers
+      - name: Push the result to checkstyle-fix/<module>
+        run: |
+          BRANCH="checkstyle-fix/$MODULE"


This action executes checkout form master. Consider to create a branch for each execution of the action. Once merged the PR it will be deleted.

GGraziadei · 2026-09-20T14:04:43Z

+          git checkout -b "$BRANCH"
+          git commit -m "Checkstyle auto-fix of $MODULE"
+          git push --force origin "$BRANCH"
+          echo "Pushed $BRANCH; open a pull request against master from it." >> "$GITHUB_STEP_SUMMARY"


Please execute again the checkstyle reporting the missing warnings, if any.
The GitHub action can create a PR directly.

GGraziadei · 2026-09-20T14:07:57Z

+ * written and the build fails if a file would change.
+ */
+
+final List<String> TEXT = '''\


The code base currently uses two versions of the ASF header, both accepted by RAT.
The text is identical; they only differ in where the lines wrap.
Could the normalization accept both forms and leave them alone? Rewriting one into
the other touches ~1300 Java files and 40 poms and pollutes git blame on every file
for no legal or functional gain.

sercuzz8 force-pushed the sketch-solution branch from a81038a to 17796cd Compare August 28, 2026 16:23

sercuzz8 force-pushed the sketch-solution branch 2 times, most recently from 129bcfb to 5035de3 Compare September 19, 2026 14:16

sercuzz8 and others added 2 commits September 20, 2026 14:22

sercuzz8 force-pushed the sketch-solution branch from 5035de3 to 495ca08 Compare September 20, 2026 12:26

GGraziadei added this to the 3.2.0 milestone Sep 20, 2026

GGraziadei reviewed Sep 20, 2026

View reviewed changes

GGraziadei requested changes Sep 20, 2026

View reviewed changes

Navigation Menu

Add checkstyle auto-fix tooling - #9017

Add checkstyle auto-fix tooling#9017
sercuzz8 wants to merge 2 commits into
apache:masterfrom
sercuzz8:sketch-solution

sercuzz8 commented Aug 28, 2026 •

edited

Loading

Uh oh!

reiabreu commented Sep 5, 2026

Uh oh!

GGraziadei commented Sep 5, 2026

Uh oh!

GGraziadei commented Sep 18, 2026

Uh oh!

sercuzz8 commented Sep 18, 2026 •

edited

Loading

Uh oh!

sercuzz8 commented Sep 19, 2026

Uh oh!

GGraziadei commented Sep 20, 2026

Uh oh!

GGraziadei Sep 20, 2026

Uh oh!

GGraziadei left a comment

Uh oh!

GGraziadei Sep 20, 2026

Uh oh!

GGraziadei Sep 20, 2026

Uh oh!

GGraziadei Sep 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sercuzz8 commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reiabreu commented Sep 5, 2026

Uh oh!

GGraziadei commented Sep 5, 2026

Uh oh!

GGraziadei commented Sep 18, 2026

Uh oh!

sercuzz8 commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sercuzz8 commented Sep 19, 2026

Uh oh!

GGraziadei commented Sep 20, 2026

Migrate from Groovy to OpenRewrite

Fix the license note

Trasform this PR in a GitHub action

Uh oh!

GGraziadei Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

GGraziadei left a comment

Choose a reason for hiding this comment

Uh oh!

GGraziadei Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

GGraziadei Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

GGraziadei Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sercuzz8 commented Aug 28, 2026 •

edited

Loading

sercuzz8 commented Sep 18, 2026 •

edited

Loading