Skip to content

Commit 88a8815

Browse files
leonard84paulk-asert
leonard84
authored and
paulk-asert
committed
GROOVY-12144: Fix AstNodeToScriptAdapter decompiler rendering fidelity
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)
1 parent 63903d9 commit 88a8815

3 files changed

Lines changed: 130 additions & 12 deletions

File tree

‎subprojects/groovy-console/src/main/groovy/groovy/console/ui/AstNodeToScriptAdapter.groovy‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,9 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
391391
}
392392
first = false
393393
print it.name
394+
if (!it.placeholder && !it.wildcard) {
395+
visitGenerics it.type?.genericsTypes
396+
}
394397
if (it.upperBounds) {
395398
print ' extends '
396399
boolean innerFirst = true
@@ -692,6 +695,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
692695
print '?'
693696
}
694697
print '.'
698+
visitGenerics expression.genericsTypes
695699
Expression method = expression.method
696700
if (method instanceof ConstantExpression) {
697701
visitConstantExpression(method, true)
@@ -728,7 +732,11 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
728732
void visitBinaryExpression(BinaryExpression expression) {
729733
expression?.leftExpression?.visit this
730734
if (!(expression.rightExpression instanceof EmptyExpression) || expression.operation.type != Types.ASSIGN) {
731-
print " $expression.operation.text "
735+
if (expression?.operation?.text == '[') {
736+
print expression.safe ? '?[' : ' [ '
737+
} else {
738+
print " $expression.operation.text "
739+
}
732740
expression.rightExpression.visit this
733741

734742
if (expression?.operation?.text == '[') {
@@ -759,6 +767,8 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
759767
if (expression?.parameters) {
760768
visitParameters(expression?.parameters)
761769
print ' ->'
770+
} else if (expression?.parameters == null) {
771+
print ' ->'
762772
}
763773
printLineBreak()
764774
indented {
@@ -792,7 +802,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
792802
void visitRangeExpression(RangeExpression expression) {
793803
print '('
794804
expression?.from?.visit this
795-
print '..'
805+
print expression.inclusive ? '..' : '..<'
796806
expression?.to?.visit this
797807
print ')'
798808
}
@@ -805,7 +815,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
805815
} else if (expression?.isSafe()) {
806816
print '?'
807817
}
808-
print '.'
818+
print expression instanceof AttributeExpression ? '.@' : '.'
809819
if (expression?.property instanceof ConstantExpression) {
810820
visitConstantExpression((ConstantExpression) expression?.property, true)
811821
} else {
@@ -830,6 +840,18 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
830840
print "'$escaped'"
831841
} else {
832842
print expression.value
843+
// re-append the literal's type suffix so re-parsing yields the same type
844+
// (Integer and BigDecimal are the literal defaults and need no suffix)
845+
def value = expression.value
846+
if (value instanceof Long) {
847+
print 'L'
848+
} else if (value instanceof Float) {
849+
print 'F'
850+
} else if (value instanceof Double) {
851+
print 'D'
852+
} else if (value instanceof BigInteger) {
853+
print 'G'
854+
}
833855
}
834856
}
835857

@@ -1031,7 +1053,9 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
10311053

10321054
@Override
10331055
void visitShortTernaryExpression(ElvisOperatorExpression expression) {
1034-
visitTernaryExpression(expression)
1056+
expression?.booleanExpression?.visit this
1057+
print ' ?: '
1058+
expression?.falseExpression?.visit this
10351059
}
10361060

10371061
@Override

‎subprojects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstNodeToScriptAdapter.groovy‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,9 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
400400
}
401401
first = false
402402
print it.name
403+
if (!it.placeholder && !it.wildcard) {
404+
visitGenerics it.type?.genericsTypes
405+
}
403406
if (it.upperBounds) {
404407
print ' extends '
405408
boolean innerFirst = true
@@ -701,6 +704,7 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
701704
print '?'
702705
}
703706
print '.'
707+
visitGenerics expression.genericsTypes
704708
Expression method = expression.method
705709
if (method instanceof ConstantExpression) {
706710
visitConstantExpression(method, true)
@@ -737,7 +741,11 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
737741
void visitBinaryExpression(BinaryExpression expression) {
738742
expression?.leftExpression?.visit this
739743
if (!(expression.rightExpression instanceof EmptyExpression) || expression.operation.type != Types.ASSIGN) {
740-
print " $expression.operation.text "
744+
if (expression?.operation?.text == '[') {
745+
print expression.safe ? '?[' : ' [ '
746+
} else {
747+
print " $expression.operation.text "
748+
}
741749
expression.rightExpression.visit this
742750

743751
if (expression?.operation?.text == '[') {
@@ -769,6 +777,8 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
769777
if (expression?.parameters) {
770778
visitParameters(expression?.parameters)
771779
print ' ->'
780+
} else if (expression?.parameters == null) {
781+
print ' ->'
772782
}
773783
printLineBreak()
774784
indented {
@@ -802,7 +812,7 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
802812
void visitRangeExpression(RangeExpression expression) {
803813
print '('
804814
expression?.from?.visit this
805-
print '..'
815+
print expression.inclusive ? '..' : '..<'
806816
expression?.to?.visit this
807817
print ')'
808818
}
@@ -815,7 +825,7 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
815825
} else if (expression?.isSafe()) {
816826
print '?'
817827
}
818-
print '.'
828+
print expression instanceof AttributeExpression ? '.@' : '.'
819829
if (expression?.property instanceof ConstantExpression) {
820830
visitConstantExpression((ConstantExpression) expression?.property, true)
821831
} else {
@@ -840,6 +850,18 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
840850
print "'$escaped'"
841851
} else {
842852
print expression.value
853+
// re-append the literal's type suffix so re-parsing yields the same type
854+
// (Integer and BigDecimal are the literal defaults and need no suffix)
855+
def value = expression.value
856+
if (value instanceof Long) {
857+
print 'L'
858+
} else if (value instanceof Float) {
859+
print 'F'
860+
} else if (value instanceof Double) {
861+
print 'D'
862+
} else if (value instanceof BigInteger) {
863+
print 'G'
864+
}
843865
}
844866
}
845867

@@ -1043,7 +1065,9 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
10431065

10441066
@Override
10451067
void visitShortTernaryExpression(ElvisOperatorExpression expression) {
1046-
visitTernaryExpression(expression)
1068+
expression?.booleanExpression?.visit this
1069+
print ' ?: '
1070+
expression?.falseExpression?.visit this
10471071
}
10481072

10491073
@Override

‎subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy‎

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
159159
String result = compileToScript(script, CompilePhase.CLASS_GENERATION)
160160
assert result.contains('public class Tree<V> extends java.lang.Object implements groovy.lang.GroovyObject')
161161
assert result.contains('private java.lang.Object<V> value') // todo: is Object<V> correct? How do you know?
162-
assert result.contains('private java.util.List<Tree> branches') // should the <? extends V> be dropped?
163-
assert result.contains('branches = new java.util.ArrayList<Tree>()') // should the <? extends V> be dropped?
162+
assert result.contains('private java.util.List<Tree<? extends java.lang.Object<V>>> branches') // GROOVY-12144: nested generics now preserved
163+
assert result.contains('branches = new java.util.ArrayList<Tree<? extends java.lang.Object<V>>>()') // GROOVY-12144: nested generics now preserved
164164
assert result.contains('public Tree(java.lang.Object<V> value)') // again, is this correct?
165165
assert result.contains(' public java.lang.Object<V> getValue()') // is this correct?
166166
assert result.contains('public void setValue(java.lang.Object<V> value)')
@@ -322,7 +322,8 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
322322
String result = compileToScript(script, CompilePhase.CLASS_GENERATION)
323323
assert result.contains('private static final transient java.util.logging.Logger log')
324324
assert result.contains("log = java.util.logging.Logger.getLogger('Event')")
325-
assert result.contains('return log.isLoggable(java.util.logging.Level.FINE) ? log.fine(this.someMethod()) : null')
325+
// GROOVY-12144: @Log builds Level.FINE as an AttributeExpression, now rendered .@FINE
326+
assert result.contains('return log.isLoggable(java.util.logging.Level.@FINE) ? log.fine(this.someMethod()) : null')
326327
}
327328

328329
void testFieldDeclarationWithValue() {
@@ -709,7 +710,7 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
709710
foo ?: 'y'"""
710711
String result = compileToScript(script, CompilePhase.SEMANTIC_ANALYSIS)
711712
assert result.contains("true || false ? 'y' : 'n'")
712-
assert result.contains("foo ? foo : 'y'")
713+
assert result.contains("foo ?: 'y'")
713714
}
714715

715716
void testWhileLoop() {
@@ -984,4 +985,73 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
984985
assert result =~ /(?s)oi:.*?\{.*?v \+= 2.*?\}/
985986
}
986987

988+
// GROOVY-12144
989+
void testNestedGenerics() {
990+
String result = compileToScript('Map<String, List<Integer>> nested() { null }')
991+
assert result.contains('java.util.Map<String, List<Integer>> nested()')
992+
}
993+
994+
// GROOVY-12144 -- 3.0: only inclusive `1..5` and right-exclusive `1..<5` exist
995+
void testRangeExpressionExclusiveBounds() {
996+
String result = compileToScript('''def a = 1..5
997+
def b = 1..<5''')
998+
assert result.contains('(1..5)')
999+
assert result.contains('(1..<5)')
1000+
}
1001+
1002+
// GROOVY-12144
1003+
void testElvisOperator() {
1004+
String result = compileToScript('def a = c ?: d')
1005+
assert result.contains('c ?: d')
1006+
assert !result.contains('c ? c : d')
1007+
}
1008+
1009+
// GROOVY-12144
1010+
void testAttributeExpression() {
1011+
String result = compileToScript('''def a = other.@order
1012+
def b = foos*.@order
1013+
def c = other?.@order''')
1014+
assert result.contains('other .@order')
1015+
assert result.contains('foos *.@order')
1016+
assert result.contains('other ?.@order')
1017+
}
1018+
1019+
// GROOVY-12144
1020+
void testClosureParameterArrow() {
1021+
String result = compileToScript('''def implicitIt = { it * 2 }
1022+
def noArgs = { -> 'x' }''')
1023+
assert result.contains('{ ->')
1024+
assert result =~ /\{\s*it \* 2/
1025+
}
1026+
1027+
// GROOVY-12144
1028+
// 3.0 renders VariableExpressions with a trailing space (e.g. `list `), so safe-index
1029+
// access reads `list ?[0]`; verified to still re-parse/re-render as a safe index.
1030+
void testSafeIndexAccess() {
1031+
String result = compileToScript('''def a = list?[0]
1032+
def b = map?['key']
1033+
list?[1] = 42''')
1034+
assert result.contains('list ?[0]')
1035+
assert result.contains("map ?['key']")
1036+
assert result.contains('list ?[1] = 42')
1037+
}
1038+
1039+
// GROOVY-12144
1040+
void testNumericLiteralSuffixes() {
1041+
String result = compileToScript('''def a = 42L
1042+
def b = 2.5f
1043+
def c = 3.5d
1044+
def d = 10G''')
1045+
assert result.contains('= 42L')
1046+
assert result.contains('= 2.5F')
1047+
assert result.contains('= 3.5D')
1048+
assert result.contains('= 10G')
1049+
}
1050+
1051+
// GROOVY-12144
1052+
void testExplicitMethodTypeArguments() {
1053+
String result = compileToScript('def a = Collections.<String>emptyList()')
1054+
assert result.contains('java.util.Collections.<String>emptyList()')
1055+
}
1056+
9871057
}

0 commit comments

Comments
 (0)