Incorrect rewriting of code from significant indentation to non-significant indentation based syntax

[eloots]

Compiler version

3.0.2 and higher

Minimized code

I haven't managed to come up with a minimised example, but I created this repo that illustrates the issue and that pinpoints the Scala version that introduced the problem (note that I tried this a long time back with RC releases of Scala 3.0.0 and the GA release of Scala 3; that version didn't have this issue).

Output

When rewriting syntax from significant indentation based syntax to non-significant indentation based syntax, the compiler generates code where in some spots, a closing curly brace is missing. In the code provided in the linked repo, this occurs 12 times.

For example, in file src/main/scala/org/lunatechlabs/dotty/SudokuSolverMain.scala, notice the missing } for method main:

object SudokuSolverMain {

  def main(args: Array[String]): Unit = {

    val system = ActorSystem[NotUsed](Main(), "sudoku-solver-system")

    println(s"${GREEN}Hit RETURN to stop solver${RESET}")
    StdIn.readLine()
    system.terminate()
}

Expectation

The compiler should rewrite code correctly. In fact, it should be possible to go "full circle" through the different syntax version as noted in the README in the linked repo.

[mbovel]

Thanks for reporting and for the great reproduction!

What seems mostly relevant to me is this commit where we can see what braces are missing.

Have you noticed any pattern in which braces are missing? The commit message answers my question:

I didn't discover a specific pattern in the spots where the closing '}' are omitted. So, it's not obvious to create minimised reproductions...

[mbovel]

Minimized to (rewrite.scala):

object A:
    object B:
        def a = 2

Works with scala-cli compile --scala-version="3.0.0" -rewrite -no-indent rewrite.scala, fails with --scala-version="3.2.2":

object A {
    object B {
        def a = 2
}

[eloots]

@mbovel

Have you noticed any pattern in which braces are missing?

Yes, as you noted, I didn't manage to find a pattern... The good news is that you found at least one minimal reproduction!

For the cases reported, there's probably more to it than meets the eye; for example, the problem occurs in if ... then ... else constructs and try ... catch ... expressions.

[mbovel]

I could indeed reproduce with other constructs. For example:

def a =
    def b =
        def c = 2

becomes:

def a = {
    def b = {
        def c = 2
}

What problem have you observed with an if ... then ... else? Shouldn't -rewrite -no-indent leave them untouched?

[eloots]

What problem have you observed with an if ... then ... else? Shouldn't -rewrite -no-indent leave them untouched?

In src/main/scala/org/lunatechlabs/dotty/sudoku/SudokuDetailProcessor.scala, line 75, there's a missing } on the else branch of the nested if:

    Behaviors.receiveMessage {
      case Update(cellUpdates, replyTo) if !fullyReduced =>
        val previousState = state
        val updatedState = mergeState(state, cellUpdates)
        if updatedState == previousState && cellUpdates != cellUpdatesEmpty then {
          replyTo ! SudokuDetailUnchanged
          Behaviors.same
        }
        else {
          val transformedUpdatedState = reductionRuleTwo(reductionRuleOne(updatedState))
          if transformedUpdatedState == state then {
            replyTo ! SudokuDetailUnchanged
            Behaviors.same
          }
          else {
            val updateSender = implicitly[UpdateSender[DetailType]]
            updateSender.sendUpdate(id, stateChanges(state, transformedUpdatedState))(replyTo)
            operational(id, transformedUpdatedState, isFullyReduced(transformedUpdatedState))
        }
  ...

[eloots]

So, if there is a pattern, it seems to be linked to nested constructs...

[mbovel]

Minimization with if-then-else:

def f =
    if true then
        val x = 3
        if (false)
            x
        else
            val y = 4
            y

becomes:

def f =
    if true then {
        val x = 3
        if (false)
            x
        else {
            val y = 4
            y
        // Missing closing brace
    }

So, if there is a pattern, it seems to be linked to nested constructs...

Yup.

[eloots]

Here's another case; with try/catch nested in a def:

Minimised to (nested5.scala):

def b =
  try
    val n2 = 21
    val n1 = 4
    n2 / n1
  catch
    case _ => 4

def a =
  val n2 = 21
  try
    val n1 = 4
    n2 / n1
  catch
    case _ => 4

scala-cli compile --scala-version="3.0.2" -rewrite -no-indent nested5.scala gives:

// Method 'b' is correctly rewritten
def b =
  try {
    val n2 = 21
    val n1 = 4
    n2 / n1
  }
  catch {
    case _ => 4
  }



// Method 'a' isn't. The difference with 'b' is the moving of the 'n2' value declaration
def a = {
  val n2 = 21
  try {
    val n1 = 4
    n2 / n1
  }
  catch {
    case _ => 4
  // Missing closing brace
}

[mbovel]

@eloots I would be interested to have your thoughts on https://contributors.scala-lang.org/t/could-we-remove-syntax-rewriting-options-from-dotty/6178.

[adpi2]

Found the root cause of this issue in https://github.com/lampepfl/dotty/pull/12954/files#diff-530622276a63ddb56dd8a34bb76b163adc2881f92952dedabe3a0046fc0332e8R23-R26

[som-snytt]

I'll follow up what I broke. Thanks for the diagnosis. I'll update here if I don't understand how rewrite works. This is modulo whether scalac should be rewriting. (I think it's a great feature.)

[adpi2]

@som-snytt I actually have a fix that I am ready to push. I'll assign you as reviewer.