leonard84 opened a new pull request, #2682:
URL: https://github.com/apache/groovy/pull/2682
Upstreams eight rendering-fidelity fixes to the Groovy Console's AST
decompiler (`AstNodeToScriptAdapter` / `AstNodeToScriptVisitor`), plus a
prerequisite fix that re-enables its test class which a JUnit 5 migration had
silently disabled.
Two commits, two JIRAs (12145 first, as it's a prerequisite):
## GROOVY-12145 — re-enable `AstNodeToScriptAdapterTest`
The "Convert to JUnit5" migration (`a1897e0811`) removed `extends
GroovyTestCase` but never added `@Test`. Since the build uses
`useJUnitPlatform()`, JUnit Platform discovers tests only by `@Test`, so all 63
tests silently stopped executing — a `--tests` filter reports *"No tests found
for given includes"*. The `import org.junit.jupiter.api.Test` was also
mis-placed inside a triple-quoted script string.
This commit adds `@Test` to every method, moves the import to the top, and
refreshes two assertions whose expected output had drifted while the tests were
dark (`@Immutable`'s hashCode accumulator `Object`→`Integer`; `@Lazy` now emits
`@Internal` before the backing field).
> GROOVY-12145 also surveys a broader fallout of the same migration (~195
dead test methods across ~32 files). This PR fixes only the `groovy-console`
instance, which is the prerequisite for the fidelity tests below; the full
survey is attached to the JIRA.
## GROOVY-12144 — decompiler rendering fidelity
`AstNodeToScriptVisitor` rendered several constructs so they did not
round-trip — re-parsing produced a different type or different semantics. Eight
independent defects, each fixed in a single visitor method:
| # | Construct | Rendered before | Rendered after |
|---|---|---|---|
| 1 | nested generic type arguments | `Map<String, List>` | `Map<String,
List<Integer>>` |
| 2 | range boundary exclusions | `(1..5)` (all) | `(1..<5)` / `(1<..5)` /
`(1<..<5)` |
| 3 | elvis operator | `c ? c : d` (evaluates `c` twice) | `c ?: d` |
| 4 | attribute (direct field) access | `obj.f` | `obj.@f` / `obj*.@f` /
`obj?.@f` |
| 5 | explicit zero-arg closure | `{ ... }` (gains implicit `it`) | `{ ->
... }` |
| 6 | safe index access | `list[0]` | `list?[0]` |
| 7 | numeric literal type suffixes | `42` / `2.5` / `3.5` / `10` | `42L` /
`2.5F` / `3.5D` / `10G` |
| 8 | explicit method-call type arguments | `Collections.emptyList()` |
`Collections.<String>emptyList()` |
Defects 3, 5, 6 and 7 changed the *meaning* of the recovered source, not
just its appearance. Each defect has a regression test (marked `//
GROOVY-12144`) that fails before the fix and passes after.
**Reviewer note:** fix 4 makes `AttributeExpression` render faithfully
everywhere. `@Log` builds `Level.FINE` as an `AttributeExpression`, so it now
renders `Level.@FINE` — a faithful view of the actual AST node (previously
mis-shown as property access); `testLogAnnotation` is updated accordingly. Two
existing tests are updated to the corrected output (`testTernaryOperaters`;
`testGenericsInMethods`, whose own comments had asked whether the nested type
args should be dropped).
These fidelity issues were originally found and fixed downstream in Spock's
transpiler (derived from this class; both projects are Apache-2.0), and this
upstreams them.
## Testing
`./gradlew :groovy-console:test` → **138/138 green**. The GROOVY-12145
commit stands on its own (63/63 with the unmodified adapter), and the
GROOVY-12144 commit adds the eight regression tests on top.
---
🤖 Prepared with Claude Code (Opus 4.8); commits carry `Assisted-by` trailers
per the ASF generative-tooling guidance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
