GGraziadei commented on PR #9017:
URL: https://github.com/apache/storm/pull/9017#issuecomment-5749342800
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:
1. takes the module to fix as an input;
2. runs the auto-fix on that module;
3. pushes the result to a `checkstyle-fix/<module>` branch, so we can open
one PR per module against `master`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
