Skip to content

Scala parser: wrap non-Expression J trees in S.StatementExpression at visitUnknown fallbacks#7385

Merged
jkschneider merged 1 commit intomainfrom
scala-parser-stmt-expr-fallbacks
Apr 15, 2026
Merged

Scala parser: wrap non-Expression J trees in S.StatementExpression at visitUnknown fallbacks#7385
jkschneider merged 1 commit intomainfrom
scala-parser-stmt-expr-fallbacks

Conversation

@jkschneider
Copy link
Copy Markdown
Member

Summary

  • Mirrors the then branch's expression-wrapping fallback in visitIf's else branch so if (x) a else b with expression operands (e.g. if (flag) "yes" else "no") parses without throwing UnsupportedOperationException.
  • Adds case j: J => new S.StatementExpression(Tree.randomId(), j) fallbacks at ~25 visitUnknown sites so Scala idioms where an expression appears in an expression-required slot (but maps to a non-Expression J) no longer crash the parser. Affected visitors include: visitWhileDo, visitForDo, visitReturn, visitThrow, visitMatchImpl, visitAssign, visitInfixOp / visitBinaryOperation / visitInfixMethodCall, visitPrefixOp / visitPostfixOp, visitParentheses, visitSelect, visitTyped, visitTypeApply (asInstanceOf/isInstanceOf), visitAppliedTypeTree, method-call target, ArrayAccess, NewArray/NewArrayWithType, NewClassWithArgs, annotation args, tuple elements. Try body/finalizer also now accept Statement (matching existing ParsedTry handling).
  • Fixes ScalaParser.sourcePathFromSourceText — was hard-coded to "file.scala", causing multi-file tests to collide on a single path and garble each other on print round-trip. Extracts package + class name via a shared derivedRelativePath helper (logic reused from parse()).

Motivation

Integration-testing prethink quality recipes against Scala sources surfaced two concrete blockers:

  1. UnsupportedOperationException: Unmapped Scala AST node: If at [116..141] source=if (flag) "yes" else "no" — the elsep visit in visitIf was missing the expression-wrapping fallback that thenp already had.
  2. Two-file test case with distinct packages collided on src/main/scala/file.scala, producing garbled print output.

The else fix exposed that the same defensive pattern was missing at many other sites. The fallback is mechanical: if visitTree returns a J that isn't exactly an Expression (e.g. a Statement like J.If used as a value in Scala), wrap it via S.StatementExpression (which implements both Expression and Statement). This keeps existing behavior for the happy path and replaces a crash with a round-trippable LST in edge cases.

Test plan

  • 7 new regression tests in ControlFlowTest and CompilationUnitTest:
    • ifElseExpressionLiteralOperands, ifElseExpressionIdentifierOperands
    • returnWithIfExpression, assignmentWithIfExpression
    • binaryOpWithIfExpressionOperand, tupleWithIfExpressionElements, throwWithIfExpression
    • whileWithUnitBody
    • multipleFilesGetDistinctPaths
  • Full rewrite-scala test suite (~400 tests across 30+ classes) passes with 0 regressions.

ScalaTreeVisitor: mirror the else branch's expression-wrapping fallback
that the then branch already had, so `if (x) a else b` with expression
operands parses (closes asymmetry that threw UnsupportedOperationException).
Add `case j: J => new S.StatementExpression(Tree.randomId(), j)` fallbacks
at ~25 visitUnknown sites where the parser previously crashed on any J
that wasn't exactly an Expression/Statement — covering visitWhileDo
body/cond, visitForDo body/iterable, visitReturn, visitThrow,
visitMatchImpl selector, visitAssign lhs/rhs, visitInfixOp /
visitBinaryOperation / visitInfixMethodCall operands, visitPrefixOp /
visitPostfixOp operand, visitParentheses inner, visitTyped,
visitTypeApply asInstanceOf/isInstanceOf, visitAppliedTypeTree args,
method call target, ArrayAccess array/index, NewArray /
NewArrayWithType elements, NewClassWithArgs constructor args,
annotation args, tuple elements. Try body/finalizer now also accept
Statement, matching the existing ParsedTry behavior.

ScalaParser.sourcePathFromSourceText: was hard-coded to "file.scala"
so every source in a multi-file test collided on the same path and
clobbered each other on print round-trip. Extract package + class
name via a shared derivedRelativePath helper (reusing logic from
parse()) so each source gets a distinct path.

7 new regression tests covering if-else-as-expression (literal/ident
operands), return/throw/assign with if-rhs, binary/tuple/paren with
if-operand, while-with-unit-body, and multi-file distinct paths. Full
rewrite-scala suite passes.
@jkschneider jkschneider force-pushed the scala-parser-stmt-expr-fallbacks branch from 5d9b68b to 3fd7156 Compare April 15, 2026 15:47
@jkschneider jkschneider merged commit 916cb78 into main Apr 15, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Apr 15, 2026
@jkschneider jkschneider deleted the scala-parser-stmt-expr-fallbacks branch April 15, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant