Repository navigation
refactor: convert grails-shell-cli to Groovy - #16585
Open
borinquenkid wants to merge 26 commits into
Open
borinquenkid wants to merge 26 commits into
borinquenkid wants to merge 26 commits into
Conversation
The public fields of FetchAllTaskSelectorsBuildAction.AllTasksModel became Groovy properties (private field plus accessors) when declared without 'public'; keep them public fields as in the Java original. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
borinquenkid
requested review from
jamesfredley and
jdaugherty
and
a balanced review from Copilot
October 11, 2026 01:27
…ovy-grails-shell-cli
…ovy-grails-shell-cli
…to-groovy-grails-shell-cli
…-to-groovy-grails-shell-cli
…va-to-groovy-grails-shell-cli
…y on 20 files Rebuilds these 20 grails-shell-cli files from the original .java content with a minimal mechanical conversion (public removed from methods/class decls, unnecessary groovy auto-imports dropped, safe double-quote -> single-quote conversions, unnecessary .class literal references removed) that deliberately keeps every semicolon, so git's default 50% rename-similarity threshold recognizes each as a rename of its .java predecessor instead of a delete+add, following the pattern proven on DefaultCommandLine.groovy (#16576) and already applied to grails-datastore-core and grails-spring. Each file adds a scoped @SuppressWarnings('UnnecessarySemicolon') with an explanatory TODO, to be removed by the stacked groovyism-cleanup PR. Four small files (GrailsCompilerAutoConfiguration, SpringBatchCompiler- AutoConfiguration, SpringIntegrationCompilerAutoConfiguration, SpringBootCompilerAutoConfiguration) stayed below the 50% threshold with semicolons alone, because their string-literal-dense bodies made the double-quote -> single-quote conversion (CodeNarc's UnnecessaryGString) proportionally too disruptive for their size; these also keep their original double-quoted strings under a documented, targeted @SuppressWarnings(['UnnecessarySemicolon', 'UnnecessaryGString']) exception, matching the GormProperties/KeyValueMappingFactory precedent in grails-datastore-core. Four files with nested/inner classes (GrailsApplicationCompiler- AutoConfiguration, ResourceMatcher, RunCommand, FetchAllTaskSelectorsBuildAction) needed one more fix: CodeNarc's @SuppressWarnings-on-class only suppresses UnnecessarySemicolon inside that class's own body, never on the file's package/import statements, which live outside any ClassNode. Moving a second @SuppressWarnings('UnnecessarySemicolon') onto the package declaration itself (a legal annotation target in Groovy, verified empirically) clears those violations file-wide. RunCommand's now-redundant explicit no-arg constructor and FetchAllTaskSelectorsBuildAction's missing blank-line-after-opening-brace were two further small, real CodeNarc findings fixed outright rather than suppressed. Verified: compileGroovy/compileTestGroovy clean, codenarcMain clean, and all 196 grails-shell-cli tests pass (0 failures, 0 errors, 2 pre-existing skips). All 20 files are now git-detected as R050+ renames against origin/10.0.x with git's default (no -M flag) similarity detection. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ning GlobalGrailsClassInjectorTransformationSpec's "a generated class missing from a hand-authored imports file is reported" fails on every GitHub-hosted runner (Ubuntu 21/25, macOS, Windows, Coverage) with an empty warning list, and passes locally in isolation, paired with every other base.dir-touching spec, and in the full suite at both local and CI fork counts. Static analysis rules out base.dir leakage (grails-core ships its own hand-authored imports file, so a leaked value would still warn), groovy.* property mutation, Spock parallelism and Groovy version drift (6.0.0 pinned on both sides). This inlines the compilation so the power assertion prints which branch of AutoConfigurationImportsWriter.register ran on the runner: whether the sibling was generated, whether the generated imports file was written (hand-authored file not found) or not (hand-authored branch taken), the collector's warning level and counts, and the directories involved. REVERT THIS COMMIT once the CI log has been read. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The stage-2 rebuild from the .java original omitted @CompileStatic on DefaultBeanConfiguration and DefaultRuntimeSpringConfiguration (the original has none) and turned this.setParent(...) into setParent(...). Without whole-class @CompileStatic a cold reflective call into setProperty() makes a further dynamic self-call that fails with "MetaClass.initialize() because mc is null", which broke BeanBuilder's property DSL: BeanBuilderTests testAbstractBeanDefinition, testAbstractBeanDefinitionWithClass and testInheritPropertiesFromAbstractBean failed with NPEs on PR #16581. Restores the annotation, the explanatory comment and the this. qualifier; similarity stays above the threshold (R073/R076). Verified with grails-spring 21/21, codenarc clean and grails-test-suite-uber BeanBuilderTests 33/33, the suite that exercises these classes and that the earlier local check skipped. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… dropped by the rename-fix The stage-2 rebuild from the .java original turned three reference-identity comparisons into ==/!=. In Groovy those call equals(), so: - PersistentList.set no longer marked the list dirty when an equal but different element replaced the old one, - AbstractSession.isDirty could treat a changed instance as clean when it equals() its cached copy, - Query.eq compared equality where the Java compared identity. They are .is() again, as in stage 1. KeyValueMappingFactory also lost its @CompileStatic (the original has none). Restoring it dropped similarity under 50%, so its imports and package line regain their semicolons under a package-level UnnecessarySemicolon suppression: R065. grails-datastore-core 949/949, codenarc clean. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…actor/java-to-groovy-grails-spring
…va-to-groovy-grails-shell-cli
…by the rename-fix 580bd5e rebuilt DefaultCommandLine from the .java original, which carries no annotations, and so lost the @CompileStatic stage 1 had put on the class and the @PackageScope on systemProperties, undeclaredOptions, declaredOptions and remainingArgs. Without them the four fields became public Groovy properties with generated accessors and the class fell back to dynamic dispatch. Both are restored; similarity stays at R069. grails-gradle-model: compile, codenarc and 432 tests pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…va-to-groovy-core-leaves
…-to-groovy-grails-bootstrap
…/java-to-groovy-grails-databinding-core
…efactor/java-to-groovy-grails-datastore-core
…actor/java-to-groovy-grails-spring
…va-to-groovy-grails-shell-cli
…orts warning" This reverts commit 72948a2.
…-to-groovy-grails-bootstrap
…/java-to-groovy-grails-databinding-core
…efactor/java-to-groovy-grails-datastore-core
…actor/java-to-groovy-grails-spring
…va-to-groovy-grails-shell-cli
✅ All tests passed ✅🏷️ Commit: 68b4680 Learn more about TestLens at testlens.app/docs. |
This branch has not been deployed
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
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.
What
Converts the remaining Java sources of
grails-shell-clito Groovy: 97.javafiles removed, 97.groovyfiles added (117 files changed, 4406 insertions(+), 4528 deletions(-)).Why
The maintainers asked on #16351 for the Java-to-Groovy migration to land one Gradle project at a time as a stack of PRs, bottom-up in dependency order. #16572 did
grails-data-hibernate7; this PR is 7 of 30 in the next stack (base:refactor/java-to-groovy-grails-spring). The end state of every file is taken from the9.0.x-java-to-groovybranch, which already carries the regression fixes found by CI.Commits:
Changed outside the converted sources
Java-facing API sweep
Every deleted
.javawas compared with its Groovy twin viajavap -v -protected(class declaration, public/protected fields, constructors, methods incl. modifiers/throws/generics, interface default methods, nested class modifiers, and the annotations callers see). Result:grails-shell-cli: 97 deleted .java, 4 difference(s) found and fixed in this PR (see thefix:commit), 0 leftSee also the notes below. Remaining informational entries (widened visibility, e.g. Groovy property accessors): 20.
Verification
classesandtestClasses(plusast/cli/testFixturessource sets where present) of the converted project(s) and of their direct dependents compile: ['grails-profiles-base']codeStyle,ratand a whole-repobuild -PskipTestsare run once on the tip of the stack.Notes
MIN_VALUE/MAX_VALUE/next()/previous(), property accessors appear for formerly package-private members, nested static classes with implicit constructors become public (they wereprotected), and generic type parameters that Groovy erases in constructors/methods (descriptors identical).static finalwithoutpubliccreates a property;==callsequals(); Closure/SAM coercions;@CompileStaticwith nested classes. All CI regression fixes found on the umbrella branch are included in the end state.🤖 Generated with Claude Code