Skip to content

refactor(grails-bootstrap): idiomatic Groovy cleanup of the J2G output - #16676

Open
borinquenkid wants to merge 7 commits into
refactor/java-to-groovy-grails-bootstrapfrom
groovyism/grails-bootstrap
Open

borinquenkid wants to merge 7 commits into
refactor/java-to-groovy-grails-bootstrapfrom
groovyism/grails-bootstrap

Conversation

@borinquenkid

Copy link
Copy Markdown
Member

Stacks on #16578 (java-to-groovy conversion for grails-bootstrap).

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 a TODO(groovyism-cleanup) comment, so git still recognizes each .java -> .groovy pair as a rename (see matrei's review on #16576). This PR removes that scaffolding and applies the Groovy idioms CodeNarc expects.

Verification

  • compile and CodeNarc of the module's own projects pass on this branch (the tip carries the 10.0.x merge of the conversion PR)
  • no tests were run locally for this stacked cleanup (compile and CodeNarc only); CI on stacked PRs is disabled, see below

CI note: this stacked branch inherits the temporary CHANGEME FOR ACCEPTANCE workflow-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

borinquenkid and others added 4 commits October 16, 2026 08:32
…matic Groovy

Idiomatic-Groovy cleanup of the Java-to-Groovy conversion output for
grails-bootstrap. No behaviour or public API change.

Applied:
- Property syntax for trivial getters/setters (safe receivers only):
  Throwable.cause/stackTrace, File.name/absolutePath/canonicalPath,
  Resource.URL/file/filename, StackTraceElement.className/fileName/lineNumber,
  JarURLConnection.jarFile/jarFileURL/jarEntry, URI.schemeSpecificPart,
  BuildEvent.target/priority/message, Target.name, Thread.contextClassLoader,
  Ant BuildLogger/Project setters.
- Safe navigation / Elvis for null-guard-then-call and ternary-with-null-check
  patterns (GrailsConsole, FileSystemResourceLoader, ResourceLocator,
  CandidateListCompletionHandler).
- Collection literals ([], [:], [] as Set) replacing no-arg
  ArrayList/LinkedHashMap/HashSet constructors; sized/copy-constructor forms
  left untouched.
- list.add/Set.add -> <<, map.get -> map[k] (bracket reads resolve statically);
  map.put deliberately kept as .put(k, v) - see "left unchanged" below.
- .equals( ) -> == for Strings; str.length() == 0 / ''.equals(x) -> isEmpty().
- in for Collection.contains and Map.containsKey.
- any{}/every{} replacing classic break-and-flag / early-return boolean loops
  (MetaClassRegistryCleaner.createAndRegister, DefaultStackTraceFilterer
  .isApplicationClass, CandidateListCompletionHandler.startsWith).
- Sentinel identity checks (NO_CUSTOM_METACLASS, NULL_RESOURCE) converted to
  SENTINEL.is(x) per the equals/identity audit.
- Dropped a redundant instanceof-guarded cast (PathMatchingResourcePatternResolver
  .doFindPathMatchingJarResources) relying on STC flow typing.
- SAM coercion for an anonymous FileFilter (ResourceLocator).
- join(';') replacing an index loop that built a separator-joined string
  (ConsoleAnsi.flushStyles).
- Misc: Arrays.asList(single) -> list literal, new String[]{..} -> as String[],
  list.toArray(new T[size]) -> as T[], list.get(i)/list[i].

Left unchanged (Tier B/C exclusions):
- map[k] = v (putAt) kept as .put(k, v): javap showed Groovy's bracket-assignment
  on Map compiles to a dynamic ScriptBytecodeAdapter.invokeMethodN under
  @CompileStatic in this toolchain, even though bracket-read (getAt) resolves
  statically - the exact dynamic-fallback regression the groovyisms skill's
  javap audit step exists to catch. Reverted every map[k] = v back to .put
  across MetaClassRegistryCleaner, ResourceLocator, the two WatchService
  watchers, PollingDirectoryWatcher and AntPathMatcher; kept the (confirmed
  static) bracket reads.
- getResourceLoader()/getPathMatcher()/getDefaultResourceLoader() self-calls
  (PathMatchingResourcePatternResolver, ResourceLocator) left as explicit method
  calls: these are overridable public/protected getters on non-final classes
  with real subclasses elsewhere in the repo, so collapsing them to bare
  property access risks losing virtual dispatch.
- Resource.getURL() left alone where chained purely for GrailsResourceUtils
  argument passing is already minimal; getFile()/getURL() were converted only
  where Resource's interface (grails-gradle's copy) confirmed no isFile()/isURL()
  pairing risk.
- AntPathMatcher.doMatch's indexed "all elements are '**'" loops: these fit the
  any/every idiom, but rewriting them to `(start..end).every {}` is unsafe in
  general (a descending Groovy IntRange iterates rather than being empty) and
  verifying the specific invariant that makes it safe here is not worth the
  risk in a hand-ported path-matching algorithm; left as indexed for loops,
  only the inner .equals -> == was applied.
- Side-effect-only for-loops with no early exit were left as for-loops rather
  than converted to .each{}, to keep the diff to idiom substitutions rather
  than control-flow rewrites.
- GrailsEclipseConsole, ConsoleLogger, GrailsConsoleBuildListener,
  GrailsConsolePrintStream/GrailsConsoleErrorPrintStream, the plugin
  metadata annotations/interfaces/exceptions, and
  MetaClassRegistryCleanerSpec: no leftover Java shapes to convert.

Verification: compileGroovy/compileCliGroovy clean; :grails-bootstrap:test
197/0 (0 skipped); codenarcMain/codenarcCli 0 violations (codenarcTest/
codenarcTestCli skipped, no ruleset); javap dynamic-dispatch audit on every
changed class (including the cli sourceset and anonymous inner classes)
shows zero invokeMethodN/getGroovyObjectProperty/setGroovyObjectProperty.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@borinquenkid
borinquenkid marked this pull request as ready for review October 20, 2026 02:09
Copilot AI balanced review requested due to automatic review settings October 20, 2026 02:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants