From ccebfb16ef6fcf781bfbe16455a55501fea8d1f3 Mon Sep 17 00:00:00 2001 From: sercuzz8 Date: Sun, 18 Oct 2026 20:14:10 +0200 Subject: [PATCH] Fix the indentation the checkstyle auto-fix gets wrong OpenRewrite's TabsAndIndents re-indent pass puts array initializer elements at the continuation indent and misplaces the bodies of some anonymous classes and enum constants, which Checkstyle's Indentation check rejects (27 errors on storm-client). Replace that pass with the checkstyle-autofix-indentation execution, which runs the Groovy stage again with only new Indentation and CommentsIndentation fixers. They move each reported line (and the rest of a reported declaration header, or a whole block comment) to the level Checkstyle's message expects. The auto-fix still runs three passes. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/checkstyle-autofix.yaml | 4 +- pom.xml | 32 ++++--- rewrite.yml | 10 -- storm-checkstyle/README.md | 11 ++- .../resources/storm/checkstyle-autofix.groovy | 96 ++++++++++++++++++- 5 files changed, 121 insertions(+), 32 deletions(-) diff --git a/.github/workflows/checkstyle-autofix.yaml b/.github/workflows/checkstyle-autofix.yaml index 267ff585ad8..d7c5b44cc6f 100644 --- a/.github/workflows/checkstyle-autofix.yaml +++ b/.github/workflows/checkstyle-autofix.yaml @@ -76,12 +76,12 @@ jobs: run: mvn --batch-mode install -DskipTests -Dcheckstyle.skip=true -pl "$MODULE" -am - name: Checkstyle report before the fix run: ./dev-tools/gitact/checkstyle-report.sh before - - name: Auto-fix the module (OpenRewrite, Groovy, OpenRewrite re-indent) + - name: Auto-fix the module (OpenRewrite, Groovy, Groovy indentation) run: | mvn --batch-mode -pl "$MODULE" -Dcheckstyle.skip=true \ org.openrewrite.maven:rewrite-maven-plugin:run@checkstyle-autofix-openrewrite \ org.codehaus.gmavenplus:gmavenplus-plugin:execute@checkstyle-autofix \ - org.openrewrite.maven:rewrite-maven-plugin:run@checkstyle-reindent-openrewrite + org.codehaus.gmavenplus:gmavenplus-plugin:execute@checkstyle-autofix-indentation - name: Normalize the license headers run: mvn --batch-mode -pl "$MODULE" -Dcheckstyle.skip=true org.codehaus.gmavenplus:gmavenplus-plugin:execute@normalize-license-headers - name: Checkstyle report after the fix diff --git a/pom.xml b/pom.xml index 7fc37e9e417..c7c2e029481 100644 --- a/pom.xml +++ b/pom.xml @@ -1167,19 +1167,6 @@ - - - checkstyle-reindent-openrewrite - none - - run - - - - org.apache.storm.checkstyle.Reindent - - - @@ -1238,6 +1225,25 @@ + + + checkstyle-autofix-indentation + none + + execute + + + + + + + + autofixChecks + Indentation,CommentsIndentation + + + + normalize-license-headers diff --git a/rewrite.yml b/rewrite.yml index 727881b27e2..2087ba0852e 100644 --- a/rewrite.yml +++ b/rewrite.yml @@ -66,16 +66,6 @@ recipeList: - org.openrewrite.java.OrderImports: removeUnused: false --- -# Second pass, run after checkstyle-autofix.groovy: its LineLength wraps leave continuation lines -# at the wrong indent. Only TabsAndIndents runs here; Spaces would turn the `{}` the Groovy stage -# writes for empty blocks back into `{ }`. -type: specs.openrewrite.org/v1beta/recipe -name: org.apache.storm.checkstyle.Reindent -displayName: Re-indent after the Groovy stage -description: Indentation only. -recipeList: - - org.openrewrite.java.format.TabsAndIndents ---- type: specs.openrewrite.org/v1beta/style name: org.apache.storm.checkstyle.Style displayName: Style mirroring storm_checkstyle.xml diff --git a/storm-checkstyle/README.md b/storm-checkstyle/README.md index 630718545f2..3a8cad50468 100644 --- a/storm-checkstyle/README.md +++ b/storm-checkstyle/README.md @@ -34,6 +34,7 @@ restored to its original text. | `NeedBraces`, `MultipleVariableDeclarations`, `ModifierOrder`, `UpperEll`, `ArrayTypeStyle`, `FileTabCharacter`, `OperatorWrap` | OpenRewrite (dedicated recipes) | | `WhitespaceAround`, `WhitespaceAfter`, `NoWhitespaceBefore`, `ParenPad`, `MethodParamPad`, `GenericWhitespace` | OpenRewrite (`Spaces`, style `org.apache.storm.checkstyle.Style`) | | `Indentation`, `EmptyLineSeparator`, `CustomImportOrder` | OpenRewrite (`TabsAndIndents`, `BlankLines`, `OrderImports`, same style) | +| `Indentation`, `CommentsIndentation` (what `TabsAndIndents` gets wrong: array initializer elements, bodies of some anonymous classes and enum constants) | Groovy, third pass only (`checkstyle-autofix-indentation`): each reported line is moved to the level Checkstyle's message expects | | `AvoidStarImport` | OpenRewrite (`RemoveUnusedImports`; needs type attribution, i.e. the module's dependencies resolvable) | | `MissingSwitchDefault` | OpenRewrite (`DefaultComesLast`: only moves an existing `default` last, it does not add a missing one) | | `LeftCurly`, `RightCurly`, `NoLineWrap`, `AnnotationLocation` | OpenRewrite (`WrappingAndBraces`), partial: it does not move `{`, `catch`/`finally` or every annotation; what is left is not chased in Groovy | @@ -95,14 +96,16 @@ introduces star imports) and `TypecastParenPad` (turns `(T) x` into `(T)x`, viol ## Running it -Three passes: OpenRewrite, the Groovy stage, then the OpenRewrite re-indent pass (the Groovy -`LineLength` wraps leave continuation lines mis-indented; the pass runs `TabsAndIndents` only, since -`Spaces` would turn the `{}` of empty blocks back into `{ }`). Review the result with `git diff`: +Three passes: OpenRewrite, the Groovy stage, then the Groovy stage again with only the +`Indentation`/`CommentsIndentation` fixers. The last pass re-indents what the Groovy `LineLength` +wraps leave mis-indented and what `TabsAndIndents` gets wrong (array initializer elements, bodies of +some anonymous classes and enum constants); an OpenRewrite re-indent there would bring the latter +back. Review the result with `git diff`: ```sh mvn org.openrewrite.maven:rewrite-maven-plugin:run@checkstyle-autofix-openrewrite \ org.codehaus.gmavenplus:gmavenplus-plugin:execute@checkstyle-autofix \ - org.openrewrite.maven:rewrite-maven-plugin:run@checkstyle-reindent-openrewrite \ + org.codehaus.gmavenplus:gmavenplus-plugin:execute@checkstyle-autofix-indentation \ -pl '!storm-shaded-deps' -Dcheckstyle.skip=true ``` diff --git a/storm-checkstyle/src/main/resources/storm/checkstyle-autofix.groovy b/storm-checkstyle/src/main/resources/storm/checkstyle-autofix.groovy index 295a011553a..2331c16797a 100644 --- a/storm-checkstyle/src/main/resources/storm/checkstyle-autofix.groovy +++ b/storm-checkstyle/src/main/resources/storm/checkstyle-autofix.groovy @@ -53,6 +53,8 @@ * JavadocContentLocation, JavadocParagraph, JavadocTagContinuationIndentation, * RequireEmptyLineBeforeBlockTagGroup, AtclauseOrder, SummaryJavadoc * (missing period, lowercase first word), InvalidJavadocPosition + * Indentation Indentation, CommentsIndentation - only in the `checkstyle-autofix-indentation` + * execution (autofixChecks), the pass after this one * * Deliberately not fixed - each needs a decision or knowledge a text edit can't have: * Naming rules and AbbreviationAsWordInName (renames need cross-file symbol resolution), @@ -1120,6 +1122,79 @@ private static final Map ESCAPES = [ } // ---- indentation ------------------------------------------------------------ + // Only run by the `checkstyle-autofix-indentation` execution, after the default run: it + // re-indents the continuation lines the LineLength wraps leave mis-indented, and what + // OpenRewrite's TabsAndIndents gets wrong (array initializer elements at the continuation + // indent, bodies of some anonymous classes and enum constants misplaced). + // Each fixer moves the reported line to the level Checkstyle's message asks for. + + static int indentation(Src s, int off, String msg, int[] ls) { + Matcher m = msg =~ /incorrect indentation level (\d+), expected level should be (?:one of the following: )?(\d+(?:, \d+)*)/ + if (!m.find()) { + return NONE + } + int lineStart = s.lineStart(off) + int current = m.group(1).toInteger() + // the reported token must open its line, at the level the message says + if (s.firstNonWs(lineStart) != off || off - lineStart != current) { + return NONE + } + // of the accepted levels, the one closest to where the line is + int target = m.group(2).split(', ').collect { + it.toInteger() + }.min { a, b -> + (Math.abs(a - current) <=> Math.abs(b - current)) ?: (a <=> b) + } + int end = s.lineEnd(off) + if (tokenOf(msg)?.endsWith(' modifier')) { + // only the first line of a declaration is reported (its annotation, say): the rest of + // its header, up to the `{` or `;`, moves with it + int depth = 0 + int i = off + while (i < s.length() && !(depth == 0 && s.isCode(i) && (s.charAt(i) == '{' as char || s.charAt(i) == ';' as char))) { + if (s.isCode(i) && s.charAt(i) == '(' as char) { + depth++ + } else if (s.isCode(i) && s.charAt(i) == ')' as char) { + depth-- + } + i++ + } + end = s.lineEnd(Math.min(i, s.length())) + } + s.replace(lineStart, end, shiftLines(s.slice(lineStart, end), target - current)) + lineStart + } + + static int commentsIndentation(Src s, int off, String msg, int[] ls) { + Matcher m = msg =~ /incorrect indentation level (\d+), expected is (\d+)/ + if (!m.find()) { + return NONE + } + int lineStart = s.lineStart(off) + if (s.firstNonWs(lineStart) != off || off - lineStart != m.group(1).toInteger()) { + return NONE + } + int delta = m.group(2).toInteger() - m.group(1).toInteger() + int end = s.lineEnd(off) + if (s.has(off, '/*')) { + // a block comment moves as a whole, so its ' *' lines stay aligned + int close = s.indexOf('*/', off + 2) + if (close < 0) { + return NONE + } + end = s.lineEnd(close) + } + s.replace(lineStart, end, shiftLines(s.slice(lineStart, end), delta)) + lineStart + } + + /** Moves every line of text right (delta > 0) or left by delta columns, never past its indent. */ + static String shiftLines(String text, int delta) { + text.split('\n', -1).collect { String l -> + int lead = l.length() - l.replaceFirst(/^[ \t]+/, '').length() + delta >= 0 ? spaces(delta) + l : l.substring(Math.min(lead, -delta)) + }.join('\n') + } // ---- line length ------------------------------------------------------------ @@ -1460,7 +1535,17 @@ Map fixers = [ AtclauseOrder : Fixers.&atclauseOrder, SummaryJavadoc : Fixers.&summaryJavadoc, InvalidJavadocPosition : Fixers.&invalidJavadocPosition, + Indentation : Fixers.&indentation, + CommentsIndentation : Fixers.&commentsIndentation, ] +// Indentation is left out of the default run so that it is fixed once the other fixers' edits +// (LineLength wraps above all) are in: the `checkstyle-autofix-indentation` execution sets +// `autofixChecks` to run only the indentation fixers, after the default run. +Set indentationChecks = ['Indentation', 'CommentsIndentation'] as Set +Set selectedChecks = binding.hasVariable('autofixChecks') && autofixChecks + ? (autofixChecks.toString().split(',')*.trim().findAll { it } as Set) + : fixers.keySet() - indentationChecks +fixers = fixers.subMap(selectedChecks) final int MAX_ROUNDS = 25 @@ -1510,6 +1595,11 @@ Map originalText = [:] Map beforeRound = [:] Map initialCount = [:] Map latestCount = [:] +// the safety net counts every violation, or only the selected checks' when autofixChecks is set +// (re-indenting a line can push it past LineLength, which is not this pass's to judge) +def counted = { List vs -> + binding.hasVariable('autofixChecks') && autofixChecks ? vs.count { selectedChecks.contains(it.check) } : vs.size() +} Map appliedByCheck = [:].withDefault { 0 } @@ -1530,9 +1620,9 @@ while (!pending.isEmpty() && round < MAX_ROUNDS) { List next = [] pending.each { File f -> List vs = violations[f.absolutePath] ?: [] - latestCount[f] = vs.size() + latestCount[f] = counted(vs) if (!initialCount.containsKey(f)) { - initialCount[f] = vs.size() + initialCount[f] = counted(vs) } if (unparsable.contains(f.absolutePath)) { if (beforeRound.containsKey(f)) { @@ -1593,7 +1683,7 @@ while (!pending.isEmpty() && round < MAX_ROUNDS) { if (!pending.isEmpty()) { audit(pending) pending.each { File f -> - latestCount[f] = (violations[f.absolutePath] ?: []).size() + latestCount[f] = counted(violations[f.absolutePath] ?: []) } }