Skip to content

GROOVY-12144: Fix AstNodeToScriptAdapter decompiler rendering fidelity (GROOVY_3_0_X backport)#2686

Merged
paulk-asert merged 1 commit into
apache:GROOVY_3_0_Xfrom
leonard84:GROOVY-12144-backport-GROOVY_3_0_X
Jul 11, 2026
Merged

GROOVY-12144: Fix AstNodeToScriptAdapter decompiler rendering fidelity (GROOVY_3_0_X backport)#2686
paulk-asert merged 1 commit into
apache:GROOVY_3_0_Xfrom
leonard84:GROOVY-12144-backport-GROOVY_3_0_X

Conversation

@leonard84

Copy link
Copy Markdown
Contributor

This backports the eight decompiler-fidelity fixes from master (#2682, commit 4b27025962) to GROOVY_3_0_X. AstNodeToScriptVisitor rendered several constructs so they did not round-trip; each fix is localized to one visitor method:

  1. Nested generic type arguments dropped (Map<String, List<Integer>>Map<String, List>) — visitGenerics recurses into concrete args.
  2. Range bound exclusions dropped — visitRangeExpression.
  3. Elvis rendered as a duplicated ternary (c ?: dc ? c : d, evaluating c twice) — visitShortTernaryExpression emits ?:.
  4. Attribute access lost the at-sign (obj.@fobj.f) — visitPropertyExpression emits .@ for an AttributeExpression.
  5. Explicit zero-arg closures lost the arrow ({ -> ... }{ ... }) — visitClosureExpression distinguishes a null parameter list from Parameter.EMPTY_ARRAY.
  6. Safe index access lost safe navigation (list?[0]list[0]) — visitBinaryExpression.
  7. Numeric literal type suffixes dropped (42L/2.5f/3.5d/10G) — visitConstantExpression re-appends L/F/D/G.
  8. Explicit method-call type arguments dropped (Collections.<String>emptyList()) — visitMethodCallExpression.

One regression test per defect (GroovyTestCase style, matching the existing test class). Existing tests reconciled to the corrected output: testTernaryOperaters (elvis), testGenericsInMethods (nested generics now preserved), testLogAnnotation (@Log's Level.FINE is an AttributeExpression, now renders Level.@FINE).

Branch adaptations for 3.0:

  • Range (fix 2): 3.0 has no left-exclusive range syntax and RangeExpression exposes only isInclusive(), so visitRangeExpression emits the ..< right-exclusive form; the two left-exclusive cases from master'"'"'s range test are dropped.
  • Two class copies: the same 8 fixes are applied to both groovy.console.ui.AstNodeToScriptAdapter (exercised by the test) and the legacy duplicate groovy.inspect.swingui.AstNodeToScriptAdapter.
  • Safe index: 3.0 renders VariableExpressions with a trailing space, so safe-index output reads e.g. list ?[0] (verified to still re-parse/re-render as a safe index).

Tests: AstNodeToScriptAdapterTest 69/69 (forced recompile+run, JDK 11).

Master PR: #2682


🤖 Prepared with Claude Code (Opus 4.8) on behalf of @leonard84.

GROOVY_3_0_X backport of master commit 4b27025. Applies the eight
visitor fixes (nested generics, range bounds, elvis, attribute access,
explicit closure arrow, safe index access, numeric literal suffixes,
explicit method type arguments) to both copies of the class on this
branch: groovy.console.ui.AstNodeToScriptAdapter (exercised by the test)
and the legacy duplicate groovy.inspect.swingui.AstNodeToScriptAdapter.

Adaptations for 3.0:
 - Range fix uses RangeExpression.isInclusive() and emits the `..<`
   right-exclusive form; 3.0 has no left-exclusive range syntax, so the
   two left-exclusive cases from master's range test are dropped.
 - Safe index access renders `?[`; 3.0 renders VariableExpressions with a
   trailing space, so output reads e.g. `list ?[0]` (verified to still
   re-parse and re-render as a safe index).

Adds one regression test per defect (GroovyTestCase style). Reconciles
existing tests to the branch's corrected output: testTernaryOperaters
(elvis), testGenericsInMethods (nested generics now preserved),
testLogAnnotation (@log's Level.FINE AttributeExpression now renders
.@fine).

Assisted-by: Claude Code (Opus 4.8)
@paulk-asert paulk-asert merged commit 88a8815 into apache:GROOVY_3_0_X Jul 11, 2026
14 checks passed
@paulk-asert

Copy link
Copy Markdown
Contributor

We aren't necessarily planning another 3.x release but I guess no harm in having this in place of we do.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants