This is an automated email from the ASF dual-hosted git repository. daniellansun pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
commit c5dc1a10f4c0c05a14fe43c2f24d1777c15a479d Author: Leonard Brünings <[email protected]> AuthorDate: Fri Jul 10 12:53:49 2026 +0200 GROOVY-12144: Fix AstNodeToScriptAdapter decompiler rendering fidelity AstNodeToScriptVisitor rendered several constructs incorrectly, producing source that did not round-trip -- it re-parsed to a different type or with changed semantics. Fix eight independent defects, each localized to a single visitor method: 1. nested generic type arguments dropped (Map<String, List<Integer>> rendered Map<String, List>) -- visitGenerics recurses into concrete args 2. range bound exclusions dropped (1..<5 / 1<..5 / 1<..<5 all rendered 1..5) -- visitRangeExpression honors isExclusiveLeft/isExclusiveRight 3. Elvis rendered as a duplicated ternary (c ?: d rendered c ? c : d, evaluating c twice) -- visitShortTernaryExpression emits ?: 4. attribute access lost the at-sign (obj.@f rendered obj.f) -- visitPropertyExpression emits .@ for an AttributeExpression 5. explicit zero-arg closures lost the arrow ({ -> ... } rendered { ... }, gaining an implicit it) -- visitClosureExpression treats a null parameter list distinctly from Parameter.EMPTY_ARRAY 6. safe index access lost safe navigation (list?[0] rendered list[0]) -- visitBinaryExpression honors isSafe() 7. numeric literal type suffixes dropped (42L/2.5f/3.5d/10G rendered as 42/2.5/3.5/10) -- visitConstantExpression re-appends L/F/D/G 8. explicit method-call type arguments dropped (Collections.<String>emptyList()) -- visitMethodCallExpression renders them Add one regression test per defect. Fix 4 makes AttributeExpression render faithfully everywhere: @Log builds Level.FINE as an AttributeExpression, so it now renders Level.@FINE (testLogAnnotation updated). Two existing tests are updated to the corrected output (testTernaryOperaters; testGenericsInMethods, whose comments had asked whether the nested type args should be dropped). These fidelity issues were found and fixed downstream in Spock's transpiler (derived from this class); this upstreams them. Assisted-by: Claude Code (Opus 4.8) --- .../console/ui/AstNodeToScriptAdapter.groovy | 28 ++++- .../console/ui/AstNodeToScriptAdapterTest.groovy | 126 ++++++++++++++++++++- 2 files changed, 147 insertions(+), 7 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 9f13c48a29..c86ccfb312 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 @@ -446,6 +446,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 @@ -754,6 +757,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati print '?' } print '.' + visitGenerics expression.genericsTypes Expression method = expression.method if (method instanceof ConstantExpression) { visitConstantExpression(method, true) @@ -799,7 +803,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati boolean isAssign = expression?.operation?.type == Types.ASSIGN boolean isAccess = expression?.operation?.type == Types.LEFT_SQUARE_BRACKET if (!isAssign || expression?.rightExpression !instanceof EmptyExpression) { - print isAccess ? '[' : " ${expression?.operation?.text} " + print isAccess ? (expression.safe ? '?[' : '[') : " ${expression?.operation?.text} " expression?.rightExpression?.visit this if (isAccess) { print ']' @@ -840,6 +844,8 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati if (expression?.parameters) { visitParameters expression?.parameters print ' ->' + } else if (expression?.parameters == null) { + print ' ->' } printLineBreak() indented { @@ -876,7 +882,9 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati void visitRangeExpression(RangeExpression expression) { print '(' expression?.from?.visit this + if (expression.exclusiveLeft) print '<' print '..' + if (expression.exclusiveRight) print '<' expression?.to?.visit this print ')' } @@ -890,7 +898,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 { @@ -919,6 +927,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' + } } } @@ -1145,7 +1165,9 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati /** Renders an Elvis expression. */ @Override void visitShortTernaryExpression(ElvisOperatorExpression expression) { - visitTernaryExpression expression + expression?.booleanExpression?.visit this + print ' ?: ' + expression?.falseExpression?.visit this } /** Renders a boolean expression. */ 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 92fa9f4f5b..0de3b0a0bd 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 @@ -181,8 +181,8 @@ final class AstNodeToScriptAdapterTest { 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') + assert result.contains('branches = new java.util.ArrayList<Tree<? extends java.lang.Object<V>>>()') 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)') @@ -409,7 +409,7 @@ final class AstNodeToScriptAdapterTest { 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') + assert result.contains('return log.isLoggable(java.util.logging.Level.@FINE) ? log.fine(this.someMethod()) : null') } @Test @@ -836,7 +836,7 @@ final class AstNodeToScriptAdapterTest { foo ?: 'y'""" String result = compileToScript(script) assert result.contains("true || false ? 'y' : 'n'") - assert result.contains("foo ? foo : 'y'") + assert result.contains("foo ?: 'y'") } @Test @@ -1159,4 +1159,122 @@ final class AstNodeToScriptAdapterTest { assert result.contains('(a as java.lang.String)') } + + // GROOVY-12144 + @Test + void testNestedGenerics() { + String script = ''' + Map<String, List<Integer>> nested() { null } + ''' + String result = compileToScript(script) + + assert result.contains('java.util.Map<String, List<Integer>> nested()') + } + + // GROOVY-12144 + @Test + void testRangeExpressionExclusiveBounds() { + String script = ''' + def a = 1..5 + def b = 1..<5 + def c = 1<..5 + def d = 1<..<5 + ''' + String result = compileToScript(script) + + assert result.contains('(1..5)') + assert result.contains('(1..<5)') + assert result.contains('(1<..5)') + assert result.contains('(1<..<5)') + } + + // GROOVY-12144 + @Test + void testElvisOperator() { + String script = ''' + def a = c ?: d + ''' + String result = compileToScript(script) + + assert result.contains('c ?: d') + assert !result.contains('c ? c : d') + } + + // GROOVY-12144 + @Test + void testAttributeExpression() { + String script = ''' + def a = other.@order + def b = foos*.@order + def c = other?.@order + ''' + String result = compileToScript(script) + + assert result.contains('other.@order') + assert result.contains('foos*.@order') + assert result.contains('other?.@order') + } + + // GROOVY-12144 + @Test + void testClosureParameterArrow() { + String script = ''' + def implicitIt = { it * 2 } + def noArgs = { -> 'x' } + ''' + String result = compileToScript(script) + + // explicit zero-argument closure keeps its arrow (otherwise it would gain an implicit 'it') + assert result.contains('{ ->') + // an implicit-'it' closure has no arrow + assert result =~ /\{\s*it \* 2/ + } + + // GROOVY-12144 + @Test + void testSafeIndexAccess() { + String script = ''' + def a = list?[0] + def b = map?['key'] + list?[1] = 42 + ''' + String result = compileToScript(script) + + assert result.contains('list?[0]') + assert result.contains("map?['key']") + assert result.contains('list?[1] = 42') + } + + // GROOVY-12144 + @Test + void testNumericLiteralSuffixes() { + String script = ''' + def a = 42L + def b = 2.5f + def c = 3.5d + def d = 10G + def e = 42 + def f = 2.5 + ''' + String result = compileToScript(script) + + assert result.contains('= 42L') + assert result.contains('= 2.5F') + assert result.contains('= 3.5D') + assert result.contains('= 10G') + // Integer and BigDecimal are the literal defaults and carry no suffix + assert result.contains('= 42\n') + assert result.contains('= 2.5\n') + } + + // GROOVY-12144 + @Test + void testExplicitMethodTypeArguments() { + String script = ''' + def a = Collections.<String>emptyList() + ''' + String result = compileToScript(script) + + assert result.contains('java.util.Collections.<String>emptyList()') + } }
