Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paulk-asert merged 1 commit into from
Jul 11, 2026
+130 −12
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions subprojects/groovy-console/src/main/groovy/groovy/console/ui/AstNodeToScriptAdapter.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,9 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
}
first = false
print it.name
if (!it.placeholder && !it.wildcard) {
visitGenerics it.type?.genericsTypes
}
if (it.upperBounds) {
print ' extends '
boolean innerFirst = true
Expand Down Expand Up @@ -692,6 +695,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
print '?'
}
print '.'
visitGenerics expression.genericsTypes
Expression method = expression.method
if (method instanceof ConstantExpression) {
visitConstantExpression(method, true)
Expand Down Expand Up @@ -728,7 +732,11 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
void visitBinaryExpression(BinaryExpression expression) {
expression?.leftExpression?.visit this
if (!(expression.rightExpression instanceof EmptyExpression) || expression.operation.type != Types.ASSIGN) {
print " $expression.operation.text "
if (expression?.operation?.text == '[') {
print expression.safe ? '?[' : ' [ '
} else {
print " $expression.operation.text "
}
expression.rightExpression.visit this

if (expression?.operation?.text == '[') {
Expand Down Expand Up @@ -759,6 +767,8 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
if (expression?.parameters) {
visitParameters(expression?.parameters)
print ' ->'
} else if (expression?.parameters == null) {
print ' ->'
}
printLineBreak()
indented {
Expand Down Expand Up @@ -792,7 +802,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
void visitRangeExpression(RangeExpression expression) {
print '('
expression?.from?.visit this
print '..'
print expression.inclusive ? '..' : '..<'
expression?.to?.visit this
print ')'
}
Expand All @@ -805,7 +815,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
} else if (expression?.isSafe()) {
print '?'
}
print '.'
print expression instanceof AttributeExpression ? '.@' : '.'
if (expression?.property instanceof ConstantExpression) {
visitConstantExpression((ConstantExpression) expression?.property, true)
} else {
Expand All @@ -830,6 +840,18 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
print "'$escaped'"
} else {
print expression.value
// re-append the literal's type suffix so re-parsing yields the same type
// (Integer and BigDecimal are the literal defaults and need no suffix)
def value = expression.value
if (value instanceof Long) {
print 'L'
} else if (value instanceof Float) {
print 'F'
} else if (value instanceof Double) {
print 'D'
} else if (value instanceof BigInteger) {
print 'G'
}
}
}

Expand Down Expand Up @@ -1031,7 +1053,9 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati

@Override
void visitShortTernaryExpression(ElvisOperatorExpression expression) {
visitTernaryExpression(expression)
expression?.booleanExpression?.visit this
print ' ?: '
expression?.falseExpression?.visit this
}

@Override
Expand Down
32 changes: 28 additions & 4 deletions ...jects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstNodeToScriptAdapter.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
}
first = false
print it.name
if (!it.placeholder && !it.wildcard) {
visitGenerics it.type?.genericsTypes
}
if (it.upperBounds) {
print ' extends '
boolean innerFirst = true
Expand Down Expand Up @@ -701,6 +704,7 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
print '?'
}
print '.'
visitGenerics expression.genericsTypes
Expression method = expression.method
if (method instanceof ConstantExpression) {
visitConstantExpression(method, true)
Expand Down Expand Up @@ -737,7 +741,11 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
void visitBinaryExpression(BinaryExpression expression) {
expression?.leftExpression?.visit this
if (!(expression.rightExpression instanceof EmptyExpression) || expression.operation.type != Types.ASSIGN) {
print " $expression.operation.text "
if (expression?.operation?.text == '[') {
print expression.safe ? '?[' : ' [ '
} else {
print " $expression.operation.text "
}
expression.rightExpression.visit this

if (expression?.operation?.text == '[') {
Expand Down Expand Up @@ -769,6 +777,8 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
if (expression?.parameters) {
visitParameters(expression?.parameters)
print ' ->'
} else if (expression?.parameters == null) {
print ' ->'
}
printLineBreak()
indented {
Expand Down Expand Up @@ -802,7 +812,7 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
void visitRangeExpression(RangeExpression expression) {
print '('
expression?.from?.visit this
print '..'
print expression.inclusive ? '..' : '..<'
expression?.to?.visit this
print ')'
}
Expand All @@ -815,7 +825,7 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
} else if (expression?.isSafe()) {
print '?'
}
print '.'
print expression instanceof AttributeExpression ? '.@' : '.'
if (expression?.property instanceof ConstantExpression) {
visitConstantExpression((ConstantExpression) expression?.property, true)
} else {
Expand All @@ -840,6 +850,18 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy
print "'$escaped'"
} else {
print expression.value
// re-append the literal's type suffix so re-parsing yields the same type
// (Integer and BigDecimal are the literal defaults and need no suffix)
def value = expression.value
if (value instanceof Long) {
print 'L'
} else if (value instanceof Float) {
print 'F'
} else if (value instanceof Double) {
print 'D'
} else if (value instanceof BigInteger) {
print 'G'
}
}
}

Expand Down Expand Up @@ -1043,7 +1065,9 @@ class AstNodeToScriptVisitor extends PrimaryClassNodeOperation implements Groovy

@Override
void visitShortTernaryExpression(ElvisOperatorExpression expression) {
visitTernaryExpression(expression)
expression?.booleanExpression?.visit this
print ' ?: '
expression?.falseExpression?.visit this
}

@Override
Expand Down
78 changes: 74 additions & 4 deletions ...ojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
String result = compileToScript(script, CompilePhase.CLASS_GENERATION)
assert result.contains('public class Tree<V> extends java.lang.Object implements groovy.lang.GroovyObject')
assert result.contains('private java.lang.Object<V> value') // todo: is Object<V> correct? How do you know?
assert result.contains('private java.util.List<Tree> branches') // should the <? extends V> be dropped?
assert result.contains('branches = new java.util.ArrayList<Tree>()') // should the <? extends V> be dropped?
assert result.contains('private java.util.List<Tree<? extends java.lang.Object<V>>> branches') // GROOVY-12144: nested generics now preserved
assert result.contains('branches = new java.util.ArrayList<Tree<? extends java.lang.Object<V>>>()') // GROOVY-12144: nested generics now preserved
assert result.contains('public Tree(java.lang.Object<V> value)') // again, is this correct?
assert result.contains(' public java.lang.Object<V> getValue()') // is this correct?
assert result.contains('public void setValue(java.lang.Object<V> value)')
Expand Down Expand Up @@ -322,7 +322,8 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
String result = compileToScript(script, CompilePhase.CLASS_GENERATION)
assert result.contains('private static final transient java.util.logging.Logger log')
assert result.contains("log = java.util.logging.Logger.getLogger('Event')")
assert result.contains('return log.isLoggable(java.util.logging.Level.FINE) ? log.fine(this.someMethod()) : null')
// GROOVY-12144: @Log builds Level.FINE as an AttributeExpression, now rendered .@FINE
assert result.contains('return log.isLoggable(java.util.logging.Level.@FINE) ? log.fine(this.someMethod()) : null')
}

void testFieldDeclarationWithValue() {
Expand Down Expand Up @@ -709,7 +710,7 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
foo ?: 'y'"""
String result = compileToScript(script, CompilePhase.SEMANTIC_ANALYSIS)
assert result.contains("true || false ? 'y' : 'n'")
assert result.contains("foo ? foo : 'y'")
assert result.contains("foo ?: 'y'")
}

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

// GROOVY-12144
void testNestedGenerics() {
String result = compileToScript('Map<String, List<Integer>> nested() { null }')
assert result.contains('java.util.Map<String, List<Integer>> nested()')
}

// GROOVY-12144 -- 3.0: only inclusive `1..5` and right-exclusive `1..<5` exist
void testRangeExpressionExclusiveBounds() {
String result = compileToScript('''def a = 1..5
def b = 1..<5''')
assert result.contains('(1..5)')
assert result.contains('(1..<5)')
}

// GROOVY-12144
void testElvisOperator() {
String result = compileToScript('def a = c ?: d')
assert result.contains('c ?: d')
assert !result.contains('c ? c : d')
}

// GROOVY-12144
void testAttributeExpression() {
String result = compileToScript('''def a = other.@order
def b = foos*.@order
def c = other?.@order''')
assert result.contains('other .@order')
assert result.contains('foos *.@order')
assert result.contains('other ?.@order')
}

// GROOVY-12144
void testClosureParameterArrow() {
String result = compileToScript('''def implicitIt = { it * 2 }
def noArgs = { -> 'x' }''')
assert result.contains('{ ->')
assert result =~ /\{\s*it \* 2/
}

// GROOVY-12144
// 3.0 renders VariableExpressions with a trailing space (e.g. `list `), so safe-index
// access reads `list ?[0]`; verified to still re-parse/re-render as a safe index.
void testSafeIndexAccess() {
String result = compileToScript('''def a = list?[0]
def b = map?['key']
list?[1] = 42''')
assert result.contains('list ?[0]')
assert result.contains("map ?['key']")
assert result.contains('list ?[1] = 42')
}

// GROOVY-12144
void testNumericLiteralSuffixes() {
String result = compileToScript('''def a = 42L
def b = 2.5f
def c = 3.5d
def d = 10G''')
assert result.contains('= 42L')
assert result.contains('= 2.5F')
assert result.contains('= 3.5D')
assert result.contains('= 10G')
}

// GROOVY-12144
void testExplicitMethodTypeArguments() {
String result = compileToScript('def a = Collections.<String>emptyList()')
assert result.contains('java.util.Collections.<String>emptyList()')
}

}
Loading