scalameta / scalafmt

Code formatter for Scala
http://scalameta.org/scalafmt
Apache License 2.0
1.42k stars 276 forks source link

Scalafmt 3.5.2 with scala3 dialect formats code that is not valid Scala 3 #3188

Closed valencik closed 2 years ago

valencik commented 2 years ago

Related to https://github.com/scalameta/scalameta/issues/2727

We can see the issue happening here: https://github.com/scalameta/munit/pull/518

(I'm really sorry if my previous issue, linked above, didn't have enough details and inevitably caused us to end up here. :grimacing: )

Configuration (required)

version = "3.5.2"
runner.dialect = scala3

Command-line parameters (required)

When I run scalafmt via CLI like this: scalafmt -c .scalafmt.conf test.scala

Steps

Given code like this:

package munit.internal

import munit.Clue
import scala.quoted._
import scala.language.experimental.macros

object MacroCompat {

  trait ClueMacro {
    inline implicit def generate[T](value: T): Clue[T] = ${ clueImpl('value) }
    implicit def generate[T](value: T): Clue[T] = macro MacroCompatScala2.clueImpl
  }

}

Problem

Scalafmt formats code like this:

package munit.internal

import munit.Clue
import scala.quoted._
import scala.language.experimental.macros

object MacroCompat {

  trait ClueMacro {
    inline implicit def generate[T](value: T): Clue[T] = ${ clueImpl('value) }
    implicit def generate[T](value: T): Clue[T] =
      macro MacroCompatScala2.clueImpl
  }

}

Note the macro MacroCompatScala2.clueImpl on a new line.

Once formatted, scala 3.1.2 will not parse this code, yielding:

[error] -- [E103] Syntax Error: /home/andrew/src/scalafmttest/test.scala:12:6 ----------
[error] 12 |      macro MacroCompatScala2.clueImpl
[error]    |      ^^^^^
[error]    |      Illegal start of statement

Expectation

The output format should be able to parsed by Scala 3

kitbellew commented 2 years ago

@valencik i thought we had discussed in the previous issue whether macros can in fact be defined on a new line, to understand if it's a parser vs formatter problem. so, are you confirming that the compiler does not like it?

can you check these options, both using scala2 and scala3 compilers, and let me know:

implicit def generate[T](value: T): Clue[T] = macro MacroCompatScala2.clueImpl
implicit def generate[T](value: T): Clue[T] = macro
  MacroCompatScala2.clueImpl
implicit def generate[T](
  value: T
): Clue[T] = macro MacroCompatScala2.clueImpl
implicit def generate[T](value: T)
  : Clue[T] = macro MacroCompatScala2.clueImpl
tgodzik commented 2 years ago

@valencik i thought we had discussed in the previous issue whether macros can in fact be defined on a new line, to understand if it's a parser vs formatter problem. so, are you confirming that the compiler does not like it?

can you check these options, both using scala2 and scala3 compilers, and let me know:

I checked and the compiler handles it correctly. did we already bump the scalameta version to the one with the fix?

kitbellew commented 2 years ago

@tgodzik for the formatter, #3187

tgodzik commented 2 years ago

Ach, right. This seems to be an error within Scala 3, Scala 2 allows the newline. We might want to not add the newline in this case.

kitbellew commented 2 years ago

@valencik if you're no longer interested in this, should we close it?

valencik commented 2 years ago

Oh, sorry, I'm still interested. This does still current affect munit: https://github.com/scalameta/munit/pull/518

I though @tgodzik had correctly identified the problem though.

Currently scalafmt inserts a newline in the scala3 dialect when it shouldn't as Scala 3 does not allow that.

tgodzik commented 2 years ago

Yeah, it seems to be a bug in Dotty, where it counts the newline as an opening of a block, which doesn't happen in Scala 2. We need leave it as is in that case.

kitbellew commented 2 years ago

@valencik i thought we had discussed in the previous issue whether macros can in fact be defined on a new line, to understand if it's a parser vs formatter problem. so, are you confirming that the compiler does not like it?

can you check these options, both using scala2 and scala3 compilers, and let me know:

implicit def generate[T](value: T): Clue[T] = macro MacroCompatScala2.clueImpl
implicit def generate[T](value: T): Clue[T] = macro
  MacroCompatScala2.clueImpl
implicit def generate[T](
  value: T
): Clue[T] = macro MacroCompatScala2.clueImpl
implicit def generate[T](value: T)
  : Clue[T] = macro MacroCompatScala2.clueImpl

@valencik this is what i need

tgodzik commented 2 years ago

can you check these options, both using scala2 and scala3 compilers, and let me know:

I checked it myself and the issue is in the Scala 3 compiler, that this is not allowed. Newline that is.

kitbellew commented 2 years ago

@tgodzik which of the options above are allowed by the compiler?

tgodzik commented 2 years ago

All of them seem to work, so it's ok if no newline is present between = and the macro keyword

kitbellew commented 1 year ago

@valencik @atry @tgodzik dotty doesn't seem to define macro as a valid keyword: https://dotty.epfl.ch/docs/internals/syntax.html

This page also doesn't mention it: https://docs.scala-lang.org/scala3/reference/metaprogramming/macros.html

is this an undocumented feature of scala3?

valencik commented 1 year ago

I don't really know, but I did come across this:

https://github.com/lampepfl/dotty/blob/ce1ce99b6b2e67d4d2ca05f6732e3eb0ee692f1f/compiler/src/dotty/tools/dotc/parsing/Tokens.scala#L180

also note the "TODO remove"