Issue #18673: Organize openrewrite staticanalysis composite recipes by groups as it is done on openrewrite website#18676
Conversation
0a20a85 to
a28a20a
Compare
| - org.openrewrite.java.recipes.JavaRecipeBestPractices | ||
| - org.openrewrite.java.recipes.RecipeTestingBestPractices | ||
| - org.openrewrite.maven.BestPractices | ||
| # Composite Recipes |
There was a problem hiding this comment.
no please make full dedication like assumed here:
There was a problem hiding this comment.
kindly consider:
---
type: specs.openrewrite.org/v1beta/recipe
name: org.checkstyle.AllAutoFixes
displayName: Auto Fixes
description: List of auto fixes from different providers.
recipeList:
- org.checkstyle.CodeCleanup
- org.checkstyle.autofix.CheckstyleAutoFix:
violationReportPath: target/cs_errors.xml
configurationPath: config/checkstyle-checks.xml
propertiesPath: config/openrewrite-recipes-checkstyle.properties
- org.openrewrite.java.RemoveUnusedImports
- org.openrewrite.java.format.NormalizeFormat
- org.openrewrite.java.format.NormalizeLineBreaks
- org.openrewrite.java.format.RemoveTrailingWhitespace
- org.openrewrite.java.migrate.UpgradeToJava21
- org.openrewrite.java.migrate.lang.StringRulesRecipes
- org.openrewrite.java.migrate.util.MigrateInflaterDeflaterToClose
- org.openrewrite.java.migrate.util.ReplaceStreamCollectWithToList
- org.openrewrite.java.migrate.util.SequencedCollection
- org.openrewrite.java.recipes.JavaRecipeBestPractices
- org.openrewrite.java.recipes.RecipeTestingBestPractices
- org.openrewrite.maven.BestPractices
- org.openrewrite.staticanalysis.BufferedWriterCreationRecipes
- org.openrewrite.staticanalysis.InlineVariable
- org.openrewrite.staticanalysis.JavaApiBestPractices
- org.openrewrite.staticanalysis.LowercasePackage
- org.openrewrite.staticanalysis.MissingOverrideAnnotation
- org.openrewrite.staticanalysis.ModifierOrder
- org.openrewrite.staticanalysis.NoFinalizer
- org.openrewrite.staticanalysis.NoToStringOnStringType
- org.openrewrite.staticanalysis.RemoveUnusedLocalVariables
- org.openrewrite.staticanalysis.RemoveUnusedPrivateFields
- org.openrewrite.staticanalysis.RemoveUnusedPrivateMethods
- org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference
- org.openrewrite.staticanalysis.SimplifyTernaryRecipes
- org.openrewrite.staticanalysis.URLEqualsHashCodeRecipes
- org.openrewrite.staticanalysis.UnnecessaryCloseInTryWithResources
- org.openrewrite.staticanalysis.UnnecessaryExplicitTypeArguments
- org.openrewrite.staticanalysis.UnnecessaryReturnAsLastStatement
- org.openrewrite.staticanalysis.UseLambdaForFunctionalInterface
- tech.picnic.errorprone.refasterrules.BigDecimalRulesRecipes
- tech.picnic.errorprone.refasterrules.CharSequenceRulesRecipes
- tech.picnic.errorprone.refasterrules.ClassRulesRecipes
- tech.picnic.errorprone.refasterrules.CollectionRulesRecipes
- tech.picnic.errorprone.refasterrules.ComparatorRulesRecipes
- tech.picnic.errorprone.refasterrules.FileRulesRecipes
- tech.picnic.errorprone.refasterrules.MapRulesRecipes
- tech.picnic.errorprone.refasterrules.MicrometerRulesRecipes
- tech.picnic.errorprone.refasterrules.MockitoRulesRecipes
- tech.picnic.errorprone.refasterrules.PatternRulesRecipes
- tech.picnic.errorprone.refasterrules.PreconditionsRulesRecipes
- tech.picnic.errorprone.refasterrules.PrimitiveRulesRecipes
- tech.picnic.errorprone.refasterrules.StreamRulesRecipes
- tech.picnic.errorprone.refasterrules.TimeRulesRecipes
---
type: specs.openrewrite.org/v1beta/recipe
name: org.checkstyle.CodeCleanup
displayName: Code cleanup
recipeList:
- org.openrewrite.java.ShortenFullyQualifiedTypeReferences
- org.openrewrite.java.SimplifySingleElementAnnotation
- org.openrewrite.java.format.EmptyNewlineAtEndOfFile
- org.openrewrite.java.format.MethodParamPad
- org.openrewrite.java.format.PadEmptyForLoopComponents
- org.openrewrite.staticanalysis.ChainStringBuilderAppendCalls
- org.openrewrite.staticanalysis.CustomImportOrder
- org.openrewrite.staticanalysis.DefaultComesLast
- org.openrewrite.staticanalysis.EmptyBlock
- org.openrewrite.staticanalysis.EqualsAvoidsNull
- org.openrewrite.staticanalysis.ExplicitInitialization
- org.openrewrite.staticanalysis.FallThrough
- org.openrewrite.staticanalysis.FinalizePrivateFields
- org.openrewrite.staticanalysis.ForLoopControlVariablePostfixOperators
- org.openrewrite.staticanalysis.HideUtilityClassConstructor
- org.openrewrite.staticanalysis.NeedBraces
- org.openrewrite.staticanalysis.OperatorWrap
- org.openrewrite.staticanalysis.ReplaceStringBuilderWithString
- org.openrewrite.staticanalysis.ReplaceThreadRunWithThreadStart
- org.openrewrite.staticanalysis.TypecastParenPad
- org.openrewrite.staticanalysis.UnnecessaryParentheses
# - org.openrewrite.java.format.NoWhitespaceAfter # incompatible
# - org.openrewrite.java.format.NoWhitespaceBefore # incompatible
---There was a problem hiding this comment.
we should use name, the same as in "Yaml Recipe List" tab at
https://docs.openrewrite.org/recipes/staticanalysis/codecleanup
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.staticanalysis.CodeCleanup
displayName: Code cleanup
description: |
Automatically cleanup code, e.g. remove unnecessary parentheses, simplify expressions.
There was a problem hiding this comment.
I have replaced ~20 individual org.openrewrite.staticanalysis.* recipes with the single composite org.openrewrite.staticanalysis.CodeCleanup recipe.
Removed duplicates (recipes that are already included in CodeCleanup)
Used the exact name as shown in the OpenRewrite documentation's "Yaml Recipe List" tab
There was a problem hiding this comment.
@Brijeshthummar02 , please use above suggestion and use name name: org.openrewrite.staticanalysis.CodeCleanup to make it clear it is list from openrewrite, not us. And only comments on disablement are from us.
There was a problem hiding this comment.
@romani i made changes but - Using the same name as org.openrewrite.staticanalysis.CodeCleanup causes a runtime failure due to duplicate recipe keys.
What should i do?
we can keep the OpenRewrite recipe in the recipeList but rename the wrapper recipe to avoid the conflict.
Something like name: org.checkstyle.OpenRewriteCodeCleanup??
|
@romani do i need to make any further changes? |
11a6a8c to
5e98610
Compare
|
@romani The CI shows multiple |
romani
left a comment
There was a problem hiding this comment.
Do you see that new violations comes from previously inactive recipes ?
If yes. We need to make copy list of what they have and explicitly comment out what we not ready to apply for now. We can discuss separately if this is temporary disablement or permanent
Items
| displayName: Auto Fixes | ||
| description: List of auto fixes from different providers. | ||
| recipeList: | ||
| - org.checkstyle.autofix.CheckstyleAutoFix: |
There was a problem hiding this comment.
Let put above this line a comment # composite recipes
And move org.openrewrite.staticanalysis.CodeCleanup to be under org.checkstyle.autofix.CheckstyleAutoFix
| propertiesPath: config/openrewrite-recipes-checkstyle.properties | ||
| # Code Cleanup includes: EmptyNewlineAtEndOfFile, PadEmptyForLoopComponents, | ||
| # ShortenFullyQualifiedTypeReferences, SimplifySingleElementAnnotation | ||
| - org.openrewrite.java.RemoveUnusedImports |
There was a problem hiding this comment.
Please put above ground individual recipes a comment "# individual recipes"
Yes — the new violations are coming from previously inactive recipes, mainly introduced via the composite |
5e98610 to
596fb16
Compare
|
@romani .
So there is nothing to explicitly disable right now. |
| configurationPath: config/checkstyle-checks.xml | ||
| propertiesPath: config/openrewrite-recipes-checkstyle.properties | ||
| recipeList: | ||
| - org.openrewrite.staticanalysis.CodeCleanup |
There was a problem hiding this comment.
Not sure why you put recipeList under other recipeList.
In my mind it should be like
recipeList:
# composite recipes
- org.checkstyle.autofix.CheckstyleAutoFix
...
- org.openrewrite.staticanalysis.CodeCleanup
And in same file we have
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.staticanalysis.CodeCleanup
That is copy from web and some recipe are explicitly commented out
596fb16 to
8613821
Compare
| - org.openrewrite.staticanalysis.CodeCleanup | ||
| # individual recipes | ||
| # Code Cleanup includes: EmptyNewlineAtEndOfFile, PadEmptyForLoopComponents, | ||
| # ShortenFullyQualifiedTypeReferences, SimplifySingleElementAnnotation |
There was a problem hiding this comment.
this is hard to underatand please consider SOLID design principles regarding S meaning: SRP SOC
… recipes by groups as it is done on openrewrite website checkstyle#18676
8613821 to
a6c01f3
Compare
| --- | ||
| type: specs.openrewrite.org/v1beta/recipe | ||
| name: org.checkstyle.AllAutoFixes | ||
| name: org.openrewrite.staticanalysis.CodeCleanup |
There was a problem hiding this comment.
this list is our list, as it has org.checkstyle.autofix.CheckstyleAutoFix:
create new in this file new block:
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.staticanalysis.CodeCleanup
There was a problem hiding this comment.
I am not getting why we are adding a empty new block.
There was a problem hiding this comment.
it isnot empty I just copied top 3 line for you to hint, but you should take all from openrewrite
863fc2d to
64f1271
Compare
64f1271 to
464188e
Compare
Issue #18673
I made grouping of the composite recipes (BufferedWriterCreationRecipes, SimplifyTernaryRecipes, URLEqualsHashCodeRecipes) together with appropriate comments, and separating them from the non-composite recipes.