Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
valencik opened this issue Apr 18, 2022 · 13 comments · Fixed by #3199
Closed

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

valencik opened this issue Apr 18, 2022 · 13 comments · Fixed by #3199

Comments

@valencik
Copy link

valencik commented Apr 18, 2022

Related to scalameta/scalameta#2727

We can see the issue happening here: scalameta/munit#518

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

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
Copy link
Collaborator

@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
Copy link
Contributor

tgodzik commented Apr 19, 2022

@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
Copy link
Collaborator

@tgodzik for the formatter, #3187

@tgodzik
Copy link
Contributor

tgodzik commented Apr 20, 2022

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
Copy link
Collaborator

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

@valencik
Copy link
Author

Oh, sorry, I'm still interested. This does still current affect munit: scalameta/munit#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
Copy link
Contributor

tgodzik commented Apr 29, 2022

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
Copy link
Collaborator

@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
Copy link
Contributor

tgodzik commented Apr 29, 2022

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
Copy link
Collaborator

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

@tgodzik
Copy link
Contributor

tgodzik commented Apr 29, 2022

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

@kitbellew
Copy link
Collaborator

@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
Copy link
Author

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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants