Skip to content

Fix NoGuava* collection recipes crashing on Groovy sources - #1210

Merged
timtebeek merged 1 commit into
mainfrom
tim/guangzhou-v3
Aug 20, 2026
Merged

Fix NoGuava* collection recipes crashing on Groovy sources#1210
timtebeek merged 1 commit into
mainfrom
tim/guangzhou-v3

Conversation

@timtebeek

@timtebeek timtebeek commented Aug 20, 2026

Copy link
Copy Markdown
Member

What

org.openrewrite.java.migrate.guava.NoGuavaMapsNewHashMap throws on Groovy sources:

java.lang.IllegalArgumentException: Expected a template that would generate exactly one statement
to replace one statement, but generated 0. Template:
new HashMap<>(__P__.<java.util.Map>/*__p0__*/p())
...
  org.openrewrite.java.migrate.guava.NoGuavaMapsNewHashMap$1.visitMethodInvocation(NoGuavaMapsNewHashMap.java:69)
  org.openrewrite.java.migrate.guava.NoGuavaMapsNewHashMap_1_GroovyVisitor.visitMethodInvocation(...)

Found via the Moderne SaaS flagship run of Java best practices against the "Netflix + Spring" org, where it puts Netflix/metacat into RepositoryRecipeRunError on every run. The trigger is metacat-thrift/src/test/groovy/com/netflix/metacat/thrift/CatalogThriftHiveMetastoreSpec.groovy — a Spock spec containing Maps.newHashMap(ImmutableMap.of(...)), which a sibling recipe in the same composite (NoGuavaImmutableMapOf) has already rewritten to Maps.newHashMap(Map.of(...)) by the time this recipe sees it.

Why

The templates are marked .contextSensitive(). That makes JavaTemplate reconstruct and re-parse the enclosing block as Java before splicing in the replacement. On a Groovy LST that block routinely contains constructs with no Java equivalent — def result = ..., timer.record(_, _) >> {}, registry.createId(_ as String) >> id — so the reconstructed source does not parse, parseBlockStatements returns zero statements, and maybeReplaceStatement throws.

None of these templates reference anything from the surrounding scope: they are self-contained expressions (new HashMap<>(#{any(java.util.Map)})) whose only external dependency is an import they already declare explicitly. Context sensitivity was never needed here, and dropping it also lets the templates be cached.

  • I went with the real fix rather than a language guard (cf. openrewrite/rewrite@e9c478e / rewrite-static-analysis#105, which caught RecipeRunException and skipped) — the replacement works correctly on Groovy once the template is context-free, so there is no reason to skip those sources.

Scope

The same unnecessary .contextSensitive() appears across the whole NoGuava* collection-factory family, all with the identical latent crash, so they are fixed together:

  • NoGuavaListsNewArrayList, NoGuavaListsNewCopyOnWriteArrayList, NoGuavaListsNewLinkedList
  • NoGuavaMapsNewHashMap, NoGuavaMapsNewLinkedHashMap, NoGuavaMapsNewTreeMap
  • NoGuavaSetsNewConcurrentHashSet, NoGuavaSetsNewHashSet, NoGuavaSetsNewLinkedHashSet

NoGuavaDirectExecutor is deliberately left alone — its Runnable::run method reference does lose type attribution without context (MemberReference type is missing or malformed), so .contextSensitive() is load-bearing there.

Testing

Added NoGuavaMapsNewHashMapTest#replaceWithNewHashMapWithMapInGroovy, a groovy(...) source spec modelled on the metacat block (Spock when:/then:, mock interactions, multi-line argument) that reproduces the crash before this change. It needs a GroovyParser with Guava on its classpath, and an explicit testImplementation("org.openrewrite:rewrite-groovy") so org.openrewrite.groovy.Assertions is no longer picked up only transitively.

All 218 tests in org.openrewrite.java.migrate.guava pass.

`Maps.newHashMap(someMap)` inside a Spock spec threw `IllegalArgumentException:
Expected a template that would generate exactly one statement to replace one
statement, but generated 0`.

The templates were marked `.contextSensitive()`, which makes `JavaTemplate`
re-parse the enclosing block as Java before splicing in the replacement. On a
Groovy LST that block can contain constructs with no Java equivalent (`def`,
`>> {}`, `_ as String`), so it fails to parse and zero statements come back.

None of these templates reference anything from the surrounding scope — they
are self-contained expressions with explicit imports — so context sensitivity
was never needed. `NoGuavaDirectExecutor` is left as-is: its `Runnable::run`
method reference does lose type attribution without context.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Aug 20, 2026
@timtebeek
timtebeek merged commit 55ccd1f into main Aug 20, 2026
1 check passed
@timtebeek
timtebeek deleted the tim/guangzhou-v3 branch August 20, 2026 10:03
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant