From 1942b8335ce8a04c12c6e8256279ef449e757730 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonard=20Br=C3=BCnings?= Date: Fri, 10 Jul 2026 13:40:29 +0200 Subject: [PATCH] GROOVY-12144: Fix AstNodeToScriptAdapter decompiler rendering fidelity GROOVY_3_0_X backport of master commit 4b27025962. 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) --- .../console/ui/AstNodeToScriptAdapter.groovy | 32 +++++++- .../swingui/AstNodeToScriptAdapter.groovy | 32 +++++++- .../ui/AstNodeToScriptAdapterTest.groovy | 78 ++++++++++++++++++- 3 files changed, 130 insertions(+), 12 deletions(-) diff --git a/subprojects/groovy-console/src/main/groovy/groovy/console/ui/AstNodeToScriptAdapter.groovy b/subprojects/groovy-console/src/main/groovy/groovy/console/ui/AstNodeToScriptAdapter.groovy index 6cbda0e88a7..7f2138ed320 100644 --- a/subprojects/groovy-console/src/main/groovy/groovy/console/ui/AstNodeToScriptAdapter.groovy +++ b/subprojects/groovy-console/src/main/groovy/groovy/console/ui/AstNodeToScriptAdapter.groovy @@ -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 @@ -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) @@ -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 == '[') { @@ -759,6 +767,8 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati if (expression?.parameters) { visitParameters(expression?.parameters) print ' ->' + } else if (expression?.parameters == null) { + print ' ->' } printLineBreak() indented { @@ -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 ')' } @@ -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 { @@ -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' + } } } @@ -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 diff --git a/subprojects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstNodeToScriptAdapter.groovy b/subprojects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstNodeToScriptAdapter.groovy index 41dc9b29eea..eb68b04677d 100644 --- a/subprojects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstNodeToScriptAdapter.groovy +++ b/subprojects/groovy-console/src/main/groovy/groovy/inspect/swingui/AstNodeToScriptAdapter.groovy @@ -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 @@ -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) @@ -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 == '[') { @@ -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 { @@ -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 ')' } @@ -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 { @@ -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' + } } } @@ -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 diff --git a/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy b/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy index 41e07348817..eafb3ea5b1c 100644 --- a/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy +++ b/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy @@ -159,8 +159,8 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase { String result = compileToScript(script, CompilePhase.CLASS_GENERATION) assert result.contains('public class Tree extends java.lang.Object implements groovy.lang.GroovyObject') assert result.contains('private java.lang.Object value') // todo: is Object correct? How do you know? - assert result.contains('private java.util.List branches') // should the be dropped? - assert result.contains('branches = new java.util.ArrayList()') // should the be dropped? + assert result.contains('private java.util.List>> branches') // GROOVY-12144: nested generics now preserved + assert result.contains('branches = new java.util.ArrayList>>()') // GROOVY-12144: nested generics now preserved assert result.contains('public Tree(java.lang.Object value)') // again, is this correct? assert result.contains(' public java.lang.Object getValue()') // is this correct? assert result.contains('public void setValue(java.lang.Object value)') @@ -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() { @@ -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() { @@ -984,4 +985,73 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase { assert result =~ /(?s)oi:.*?\{.*?v \+= 2.*?\}/ } + // GROOVY-12144 + void testNestedGenerics() { + String result = compileToScript('Map> nested() { null }') + assert result.contains('java.util.Map> 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.emptyList()') + assert result.contains('java.util.Collections.emptyList()') + } + }