Fix several Scala parse-print idempotency issues found on Apache Flink#8174
Merged
Conversation
Five distinct parser bugs surfaced when parsing the Apache Flink codebase
(Scala 2) with rewrite-scala:
- Argument prefix doubling in `new Foo(...)` calls: whitespace after a comma
was captured into argPrefix without advancing the cursor past it, so the
argument's visit consumed it again into a nested prefix. Visible for
J.TypeCast arguments (`asInstanceOf`), where withPrefix can't replace the
nested copy.
- Backticks dropped from package segments (`package a.b.`trait``): the package
expression was rebuilt from compiler name strings, which strip backticks.
Now derived from the source span.
- Trailing `;` dropped: dotty extends some statement spans over the trailing
`;`, which visitBlock's forward scan couldn't see; and a braced block's
final expression had no semicolon handling at all.
- `enum`/`given` used as identifiers (valid Scala 2) produced error trees:
parseOne now retries a file under `-source 3.0-migration` (where dotty's
scanner demotes enum/given/erased to identifiers) when the default
3.6-migration parse has syntax errors.
- Scala 2 `do { ... } while (cond)` printed garbled: dotty desugars it to
`WhileDo(Block(body, cond), ())`; now detected and mapped to J.DoWhileLoop.
Verified by round-tripping all 1017 Scala files in apache/flink: zero parse
errors, zero print diffs.
Claude-Session: https://claude.ai/code/session_01R2LCngRXpEq7cuF6wu8XmE
doesNotLeakTempDirectories compared before/after snapshots of the shared java.io.tmpdir, so rewrite-scala temp dirs created (and later deleted) by concurrently running test classes were flagged as leaks. Poll until transient dirs disappear; genuine leaks persist and still fail the test. Claude-Session: https://claude.ai/code/session_01R2LCngRXpEq7cuF6wu8XmE
A backticked package segment like `trait` is source syntax only — the package name is `a.b.trait`. Build the package expression with TypeTree.build's escape overload so the identifier's simple name is the bare name with a Quoted marker, and render the backticks from the marker in ScalaPrinter. J.Package.getPackageName() now returns the real package name, so recipes matching on it work. Claude-Session: https://claude.ai/code/session_01R2LCngRXpEq7cuF6wu8XmE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Parsing the Apache Flink codebase (Scala 2) with
rewrite-scalaproduced 44 parse failures ("is not print idempotent") across 42 files. Triaging them revealed five distinct parser bugs. All 1017 Scala files in apache/flink now round-trip with zero parse errors and zero print diffs.Summary
new Foo(...)calls — whitespace after a comma was captured intoargPrefixwithout advancing the cursor past it, so the argument's own visit consumed it again into a nested prefix. Ordinary arguments masked this becausewithPrefixoverwrites the prefix at the same level, but forJ.TypeCastarguments (asInstanceOf) the duplicate sits on the inner qualifier, printingnew MyPojo(a.asInstanceOf[Int], b)with a doubled space (or a doubled newline+indent for multi-line argument lists). The cursor now advances to the argument start, as the first-argument branch already did.package a.b.`trait`printed without backticks because the package expression was rebuilt from compiler name strings, which strip them.ScalaASTConverternow derives the package name from thepidsource span, for both the file-header and braced-package forms.;dropped — two defects invisitBlock: (a) dotty extends some statement spans to include the trailing;(e.g. when aval's rhs sits on its own line), which the forward scan couldn't see; the absorbed;is now recognized the same wayconsumeTrailingSemicolondoes, including a cursor catch-up so the;can't leak into whitespace; (b) a braced block's final expression had no semicolon handling at all — its;is now consumed and preserved via theSemicolonmarker. Braceless blocks (e.g. match-case bodies) are excluded: there the;is the enclosing construct's separator and is preserved there.enum/givenas identifiers produced error trees — valid Scala 2 names are hard keywords in Scala 3, so files likedef qualifyEnum(enum: Enum[_])printed with<error>fragments.ScalaCompilerBridge.parseOnenow retries a file under-source 3.0-migration— where dotty's scanner demotesenum/given/erasedto identifiers — whenever the default3.6-migrationparse has syntax errors, keeping the first attempt if the retry doesn't parse cleanly either. Each attempt parses under its own reporter so a failed attempt's errors don't surface on a file the retry parsed fine. A global source-version switch was not an option:3.0-migrationdisables ≥3.1 parser features such asfewerBracescolon-arguments, which this parser supports.do { ... } while (cond)printed garbled — dotty desugars do-while intoWhileDo(Block(body, cond), ()), which printed aswhiledo {{ ... } while (...)}do. The desugared shape (span starting at thedokeyword) is now detected and mapped toJ.DoWhileLoop.Test plan
TypeCastTest#castAsFirstOfMultipleArguments,NewClassTest#newClassWithMultiLineChainedArgument,CompilationUnitTest#packageWithBacktickedSegment,BlockTest#trailingSemicolonAfterMultiLineInitializer,Scala2CompatTest#enumAsIdentifier/#givenAsIdentifier,ControlFlowTest#doWhileLoop./gradlew :rewrite-scala:testgreenhttps://claude.ai/code/session_01R2LCngRXpEq7cuF6wu8XmE