[
https://issues.apache.org/jira/browse/GROOVY-12144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095285#comment-18095285
]
ASF GitHub Bot commented on GROOVY-12144:
-----------------------------------------
leonard84 opened a new pull request, #2686:
URL: https://github.com/apache/groovy/pull/2686
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 ?: d` → `c ? c : d`,
evaluating `c` twice) — `visitShortTernaryExpression` emits `?:`.
4. Attribute access lost the at-sign (`obj.@f` → `obj.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 `VariableExpression`s 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: https://github.com/apache/groovy/pull/2682
---
🤖 Prepared with Claude Code (Opus 4.8) on behalf of @leonard84.
> AstNodeToScriptAdapter decompiler drops or corrupts several constructs when
> rendering AST back to source
> --------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12144
> URL: https://issues.apache.org/jira/browse/GROOVY-12144
> Project: Groovy
> Issue Type: Bug
> Components: groovy-console
> Reporter: Leonard Brünings
> Priority: Major
>
> h2. Description
> {{groovy.console.ui.AstNodeToScriptAdapter}} (class
> {{{}AstNodeToScriptVisitor{}}}) turns a compiled AST back into Groovy source;
> it backs the Groovy Console's AST browser and is usable from the command
> line. Several node kinds are rendered incorrectly, producing source that does
> not round-trip - it either fails to re-parse, or re-parses to a *different*
> program (changed type or changed semantics).
> Eight distinct rendering defects are present on {{{}master{}}}. They are
> independent and each has a one-line cause in a single visitor method.
> || # ||Construct||Input||Rendered now (wrong)||Should render||
> |1|Nested generic type arguments|{{Map<String, List<Integer>>}}|{{Map<String,
> List>}}|{{Map<String, List<Integer>>}}|
> |2|Range boundary exclusions|{{1..<5}} / {{1<..5}} / {{1<..<5}}|{{(1..5)}}
> (all three)|{{(1..<5)}} / {{(1<..5)}} / {{(1<..<5)}}|
> |3|Elvis operator|{{c ?: d}}|{{c ? c : d}} (evaluates {{c}} twice)|{{c ?: d}}|
> |4|Attribute (direct field) access|{{obj.@f}} / {{obj*.@f}} /
> {{obj?.@f}}|{{obj.f}} / {{obj*.f}} / {{obj?.f}} (silently becomes property
> access)|{{obj.@f}} / {{obj*.@f}} / {{obj?.@f}}|
> |5|Explicit zero-arg closure|{\{ { -> foo() }}}\|{{{}{ foo() }{}}} (re-parses
> with an implicit {{{}it{}}})\|\{{{ -> foo() }
> }}|
> |6|Safe index access|{{{{{}list?[0]{}}}}}|{{list[0]}}|{{list?[0]}}|
> |7|Numeric literal type suffixes|{{42L}} / {{2.5f}} / {{3.5d}} /
> {{10G}}|{{42}} / {{2.5}} / {{3.5}} / {{10}} (re-parses as
> {{{}Integer{}}}/{{{}BigDecimal{}}})|{{42L}} / {{2.5F}} / {{3.5D}} / {{10G}}|
> |8|Explicit method-call type
> arguments|{{Collections.<String>emptyList()}}|{{Collections.emptyList()}}|{{Collections.<String>emptyList()}}|
> Defects 3, 5, 6 and 7 change the *meaning* of the recovered source, not just
> its appearance.
> h3. Root cause (per defect)
> * *1* - {{visitGenerics}} prints {{it.name}} only and never recurses into a
> concrete type argument's own generics.
> * *2* - {{visitRangeExpression}} prints {{'..'}} unconditionally, ignoring
> {{RangeExpression.isExclusiveLeft()}} / {{{}isExclusiveRight(){}}}.
> * *3* - {{visitShortTernaryExpression}} delegates to
> {{{}visitTernaryExpression{}}}, duplicating the condition.
> * *4* - {{visitAttributeExpression}} delegates to
> {{{}visitPropertyExpression{}}}, which always prints {{{}'.'{}}}.
> * *5* - {{visitClosureExpression}} treats {{parameters == null}} (explicit
> \{{{} \{ -> }
> {}}}) the same as {{parameters == Parameter.EMPTY_ARRAY}} (implicit
> {{{}it{}}}); only the latter should omit the arrow.
> * *6* - {{visitBinaryExpression}} detects the {{LEFT_SQUARE_BRACKET}} access
> but prints a hardcoded {{{}'['{}}}, ignoring {{{}isSafe(){}}}.
> * *7* - {{visitConstantExpression}} prints {{value.toString()}} with no type
> suffix.
> * *8* - {{visitMethodCallExpression}} ignores
> {{{}MethodCallExpression.getGenericsTypes(){}}}.
> h2. Steps to reproduce
> {code:groovy}
> import groovy.console.ui.AstNodeToScriptAdapter
> import org.codehaus.groovy.control.CompilePhase
> def src = 'def x = Collections.<String>emptyList(); def y = 1..<5; def z =
> a?.@f'
> println new AstNodeToScriptAdapter().compileToScript(src,
> CompilePhase.SEMANTIC_ANALYSIS.phaseNumber)
> // observe: Collections.emptyList(), (1..5), a.f -- all three wrong
> {code}
> h2. Suggested fix
> One localized change per visitor method (all in
> {{{}subprojects/groovy-console/src/main/groovy/groovy/console/ui/AstNodeToScriptAdapter.groovy{}}}),
> plus one regression test per defect in {{{}AstNodeToScriptAdapterTest{}}}.
> *Note for reviewers:* fix 4 makes {{AttributeExpression}} render faithfully
> as {{{}.@{}}}. This changes the rendered output of AST-transform-generated
> code that builds attribute nodes - e.g. {{@Log}} generates {{Level.@FINE}}
> for its {{isLoggable}} guard. That output is now a faithful representation of
> the actual AST node (previously mis-shown as property access
> {{{}Level.FINE{}}}); {{{}testLogAnnotation{}}}'s expectation is updated
> accordingly.
> h2. Notes
> * These fidelity bugs were originally found and fixed downstream in Spock's
> transpiler (a fork of this class); this ticket upstreams them.
> * Both projects are Apache-2.0 and the Spock file derives from this one, so
> the contribution is clean.
> * Depends on the test class actually running - see the companion ticket
> GROOVY-12145 on the JUnit 5 migration having left
> {{AstNodeToScriptAdapterTest}} unexecuted (commit {{{}a1897e0811{}}}).
> {panel:title=Provenance}
> Assisted-by: Claude Code (Opus 4.8). Contributor is responsible for
> correctness, licensing, and style.
> {panel}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
