Add Gradle recipe RemoveEmptyBuildscriptBlock#8329
Open
sullis wants to merge 5 commits into
Open
Conversation
Contributor
Author
|
LGTM |
…sing `ListUtils.map` Both `G.CompilationUnit` and `K.CompilationUnit` dispatch through `adapt()`, so a single `JavaIsoVisitor` traverses Groovy and Kotlin scripts and the two visitors collapse into one. The top-level statement list still needs a per-language cast, since `JavaSourceFile` exposes `getClasses()` rather than `getStatements()`. `ListUtils.filter` cannot express the removal, because the removed block's prefix has to transfer to the statement taking its place. `ListUtils.map` can, following the `AtomicReference<Space>` idiom in `ChangeMethodAccessLevelVisitor`. The first removed prefix wins, so consecutive empty blocks do not leave the successor with a separator prefix at the start of the file; a new test covers that.
…ldscript` blocks A Kotlin script's `K.CompilationUnit` always holds its body in a single block, for `build.gradle.kts` and `settings.gradle.kts` alike, so that branch could never see a `buildscript` invocation and never fired. Kotlin's top level is reached through `visitBlock` instead; only Groovy keeps its statements on the compilation unit. Both traversals are load-bearing, which no test previously showed: without the recursion in `visitBlock` an empty `buildscript` nested in another block survives in Kotlin, and without visiting a Groovy compilation unit's children it survives in Groovy. Add a test per language so neither can regress unnoticed.
Groovy gives the last statement of a closure an implicit `return`, so a `buildscript` block written there is a `J.Return` rather than a `J.MethodInvocation` and the name check never matched it. The emptiness check already unwrapped that, making the two disagree: the same block counted as empty when nested inside another block, yet was never removed when it sat last. Kotlin has no such wrapper, so it removed the block either way. Share the unwrapping between both checks so the two agree, and take the prefix from the outer statement so the `return` keeps supplying the whitespace it owns.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new recipe
org.openrewrite.gradle.RemoveEmptyBuildscriptBlockthat removesbuildscriptblocks which contribute nothing to the build, and wires it intoorg.openrewrite.gradle.GradleBestPractices.Behavior
build.gradle(.kts)andsettings.gradle(.kts)(Groovy and Kotlin DSL).buildscriptblock whose closure body is empty, or contains only other empty blocks (e.g. an emptydependenciesorrepositoriesblock).nullfrom the method invocation visit, so the following statement inherits the removed block's prefix — otherwise removing the first statement in a file leaves its leading blank lines behind.Tests
RemoveEmptyBuildscriptBlockTestcovers the empty block, a block containing only empty nested blocks, the Kotlin DSL, a block between other statements, and the negative cases (real dependencies, a comment in the block, a comment in a nested block).GradleBestPracticesTestgains a case verifying the recipe runs as part of the composite recipe.