Repository navigation
refactor(runtime-small): idiomatic Groovy cleanup of the J2G output - #16681
Open
borinquenkid wants to merge 15 commits into
Open
borinquenkid wants to merge 15 commits into
borinquenkid wants to merge 15 commits into
Conversation
…-groovy-datamapping-validation
…tor/java-to-groovy-grails-datamapping-core
…ctor/java-to-groovy-grails-encoder
…-to-groovy-runtime-small
…o-groovy-datamapping-validation
…ctor/java-to-groovy-grails-datamapping-core
…actor/java-to-groovy-grails-encoder
…a-to-groovy-runtime-small
…rogress conversion idiomatic
Idiomatic-Groovy pass over the Java-to-Groovy conversion output for
grails-datasource, grails-logging, grails-quartz and grails-startup-progress.
Behaviour-preserving only: no public API, no test-visible behaviour change
except where noted.
Applied:
- Property syntax for getters/setters without an isX/getX collision: ~90 sites
(Connection.readOnly, Trigger.jobKey/startTime, JobDetail.key, GrailsJobClass
group/concurrent/durability/requestsRecovery/description/fullName, Method
parameterTypes/name, HttpServletRequest/Response, HttpExchange, FilterRegistrationBean,
ConfigurableApplicationContext/WebServerApplicationContext/ConfigurableListableBeanFactory,
StartupProgress phase/reporting, StartupStep name/id/parentId/tags, BeanDefinition
singleton/abstract/lazyInit, InetAddress anyLocalAddress/hostAddress, Throwable
cause/message, Class name).
- String `.equals(` -> `==`: ~20 sites (job group/description defaults via Groovy
truth, request methods, tag-key comparisons, status/report path comparisons).
- `==`-equality for reference checks and `in` for `Collection`/array membership:
a handful of sites (StartupProgress bean-name tracking, StartupProgressPage's
format=json query check).
- Null-check -> truthy where the type is a non-falsy bean/closure/object
(JobDetail, Closure, JobExecutionException, PersistenceContextInterceptor,
BeanCreationStep/TaskStep `enclosing`), never where the type is a falsy
String/Collection/Number/Boolean.
- Flow-typed instanceof narrowing to drop a redundant cast (ConfigurableApplicationContext
in StartupProgressRunListener.ready(), InvocationTargetException's targetException
in GrailsJobFactory).
- Collection literals and `<<` for statement-position adds (`List<String> kept = []`,
`names << name`, `commandLine << address.toString()`, `kept << parameter`).
- GString instead of `+` concatenation into a String-typed log message slot
(ExceptionPrinterJobListener, SessionBinderJobListener).
- `value?.toString()` for the null-collapsing ternary in Quartz's StringEditor.
Deliberately left (Tier B exclusions / out of scope):
- LoggingTransformer.groovy: kept `classNode.getNodeMetaData(Slf4j) != null` and
`logField != null` as explicit null checks rather than Groovy truth, per the
file's own caution about AST-node comparisons; `getField`/`getNodeMetaData`/
`putNodeMetaData` kept as calls (parameterized, not simple accessors).
- DefaultGrailsJobClass.execute()/execute(JobExecutionContext): left the
`getMetaClass().invokeMethod(getReferenceInstance(), ...)` dynamic dispatch
untouched; it is the documented, intentional reason the class is `@POJO`.
- DefaultGrailsJobClass.isConcurrent/isSessionRequired/isDurability/
isRequestsRecovery/isEnabled and JobDetailFactoryBean's `name`/`group` null
checks: kept the explicit `== null` ternary/ifs, since the value is a Boolean
or String where Elvis/truthy would also treat a legitimate `false`/`""` as
"absent".
- CustomTriggerFactoryBean/JobDetailFactoryBean/SessionBinderJobListener's
private-field setters (`setJobDetail`, `setTriggerClass`, `setJobClass`,
`setApplicationName`, `setPersistenceInterceptor`/`getPersistenceInterceptor`):
left as explicit methods rather than bare properties, since converting would
add a public getter that was not there before (API-surface change), or (for
the Javadoc'd session-binder pair) drop the method-level Javadoc.
- StartupProgressProperties.groovy: left every getX/setX pair untouched; each
carries explanatory Javadoc, and the class is bound by Spring Boot
`@ConfigurationProperties` reflection, where touching the accessors is not
worth the risk for a style-only pass.
- Files already idiomatic, so unchanged: ReadOnlyDriverManagerDataSourceSpec,
LoggingTransformerSkipSpec, GrailsJobClass, GrailsJobClassConstants,
GrailsJobClassConstantsSpec, ExceptionPrinterJobListenerSpec,
SessionBinderJobListenerSpec, StartupProgressLifecycle,
StartupProgressProperties, StartupProgressSpec.
Tests: added one gap-filling case to DefaultGrailsJobClassEdgeCasesSpec
("a missing or blank group falls back to the default"), mirroring the
existing blank-description case, since the `!group` Groovy-truth rewrite
needed coverage for the blank-string branch that the out-of-scope
DefaultGrailsJobClassSpec does not exercise. No other test changed.
Verification: compileGroovy/compileTestGroovy and full `test` green for all
four modules (grails-datasource, grails-logging, grails-quartz,
grails-startup-progress), StartupProgressSpec run twice with zero failures
both times; codeStyle (checkstyle + CodeNarc) clean for all four modules;
javap dynamic-dispatch audit clean on every changed class, including the
`@POJO` DefaultGrailsJobClass (no `implements GroovyObject`, no
setGroovyObjectProperty/getGroovyObjectProperty/invokeMethodN introduced; the
one pre-existing invokeMethod call is the untouched execute() dispatch).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…untime-small # Conflicts: # grails-logging/src/main/groovy/org/grails/compiler/logging/LoggingTransformer.groovy # grails-startup-progress/src/main/groovy/org/apache/grails/startup/StartupProgressApplicationStartup.groovy
borinquenkid
marked this pull request as ready for review
October 20, 2026 02:09
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.
Stacks on #16590 (java-to-groovy conversion for
runtime-small).Idiom pass over that conversion's output: the conversion PR keeps the original Java semicolons and double-quoted strings, under a
@SuppressWarnings('UnnecessarySemicolon')header and aTODO(groovyism-cleanup)comment, so git still recognizes each.java->.groovypair as a rename (see matrei's review on #16576). This PR removes that scaffolding and applies the Groovy idioms CodeNarc expects.Verification
CI note: this stacked branch inherits the temporary
CHANGEME FOR ACCEPTANCEworkflow-trigger commit from its base, so no CI runs on it. That commit must be reverted, and CI green, before this PR merges (blocker).🤖 Generated with Claude Code