Rewrite compiler flag on some generated code can break it #17456

ckipp01 · 2023-05-10T14:06:17Z

Discussed in #17453

^{Originally posted by PeuTit May 10, 2023}
Creating a new Play application with g8 and the play-scala-seed.g8 template.

sbt new playframework/play-scala-seed.g8 --branch 2.9.x

I was testing the rewrite capabilities of the compiler with the following options:

scalacOptions ++= Seq(
  "-rewrite",
  "-indent",
)

Launching a sbt shell & compiling the project, I'm facing the following errors:

[info] compiling 7 Scala sources and 1 Java source to /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/target/scala-3.3.0-RC5/classes ...
[error] /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/app/views/index.scala.html:4:28: Illegal start of toplevel definition
[error]   <h1>Welcome to Play!</h1>
[error]                            ^
[error] /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/app/views/main.scala.html:12:11: Illegal start of toplevel definition
[error]         @* Here's where we render the page title `String`. *@
[error]           ^
[error] /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/app/views/main.scala.html:25:8: render is already defined as method render in /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/target/scala-3.3.0-RC5/twirl/main/views/html/index.template.scala
[error]
[error] Note that overloaded methods must all be defined in the same group of toplevel definitions
[error] </html>
[error]        ^
[error] /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/app/views/index.scala.html:4:15: Not found: _display_:
[error]   <h1>Welcome to Play!</h1>
[error]               ^
[error] /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/app/views/index.scala.html:5:2: Not found: apply
[error] }
[error]  ^
[error] /Users/titouan/Documents/work/perso/scala/test-tmp-play-scala3/app/views/main.scala.html:11:9: Not found: _display_:
[error]     <head>
[error]         ^
[error] 6 errors found
[error] (Compile / compileIncremental) Compilation failed
[error] Total time: 1 s, completed May 10, 2023, 10:40:04 AM

Does anyone have ever faced this issue previously?

minimal example

Actually, I was able to reproduce this. Interesting one. So what happens is that on the first compile it will take the Scala code that gets generated from the index.scala.html and attempt to rewrite it. It does this, but outputs invalid code. For example here is what your index.scala.html really looks like when passed to the compiler:

package views.html

import _root_.play.twirl.api.TwirlFeatureImports._
import _root_.play.twirl.api.TwirlHelperImports._
import _root_.play.twirl.api.Html
import _root_.play.twirl.api.JavaScript
import _root_.play.twirl.api.Txt
import _root_.play.twirl.api.Xml
import models._
import controllers._
import play.api.i18n._
import views.html._
import play.api.templates.PlayMagic._
import play.api.mvc._
import play.api.data._

object index extends _root_.play.twirl.api.BaseScalaTemplate[play.twirl.api.HtmlFormat.Appendable,_root_.play.twirl.api.Format[play.twirl.api.HtmlFormat.Appendable]](play.twirl.api.HtmlFormat) with _root_.play.twirl.api.Template0[play.twirl.api.HtmlFormat.Appendable] {

  /**/
  def apply/*1.2*/():play.twirl.api.HtmlFormat.Appendable = {
    _display_ {
      {


Seq[Any](format.raw/*2.1*/("""
"""),_display_(/*3.2*/main("Welcome to Play")/*3.25*/ {_display_(Seq[Any](format.raw/*3.27*/("""
  """),format.raw/*4.3*/("""<h1>Welcome to Play!</h1>
""")))}),format.raw/*5.2*/("""
"""))
      }
    }
  }

  def render(): play.twirl.api.HtmlFormat.Appendable = apply()

  def f:(() => play.twirl.api.HtmlFormat.Appendable) = () => apply()

  def ref: this.type = this

}


              /*
                  -- GENERATED --
                  SOURCE: app/views/index.scala.html
                  HASH: f7d71d8a9f655317469a39627564796bcf2a545b
                  MATRIX: 722->1|818->4|845->6|876->29|915->31|944->34|1000->61
                  LINES: 21->1|26->2|27->3|27->3|27->3|28->4|29->5
                  -- GENERATED --
              */

This then gets rewritten to

package views.html

import _root_.play.twirl.api.TwirlFeatureImports._
import _root_.play.twirl.api.TwirlHelperImports._
import _root_.play.twirl.api.Html
import _root_.play.twirl.api.JavaScript
import _root_.play.twirl.api.Txt
import _root_.play.twirl.api.Xml
import models._
import controllers._
import play.api.i18n._
import views.html._
import play.api.templates.PlayMagic._
import play.api.mvc._
import play.api.data._

object index extends _root_.play.twirl.api.BaseScalaTemplate[play.twirl.api.HtmlFormat.Appendable,_root_.play.twirl.api.Format[play.twirl.api.HtmlFormat.Appendable]](play.twirl.api.HtmlFormat) with _root_.play.twirl.api.Template0[play.twirl.api.HtmlFormat.Appendable]:

  /**/
  def apply/*1.2*/():play.twirl.api.HtmlFormat.Appendable =
    _display_:


  Seq[Any](format.raw/*2.1*/("""
  """),_display_(/*3.2*/main("Welcome to Play")/*3.25*/ {_display_(Seq[Any](format.raw/*3.27*/("""
    """),format.raw/*4.3*/("""<h1>Welcome to Play!</h1>
  """)))}),format.raw/*5.2*/("""
  """))

    def render(): play.twirl.api.HtmlFormat.Appendable = apply()

    def f:(() => play.twirl.api.HtmlFormat.Appendable) = () => apply()

    def ref: this.type = this



                /*
                    -- GENERATED --
                    SOURCE: app/views/index.scala.html
                    HASH: f7d71d8a9f655317469a39627564796bcf2a545b
                    MATRIX: 722->1|818->4|845->6|876->29|915->31|944->34|1000->61
                    LINES: 21->1|26->2|27->3|27->3|27->3|28->4|29->5
                    -- GENERATED --
                */

Which has a whole handful of problems. The thing is, is that I don't even think we should be trying to rewrite these files, but I don't know if there is a good way to filter them out since Dotty I believe is just going to try and rewrite everything, whether it's generated code or not.

The text was updated successfully, but these errors were encountered:

ckipp01 · 2023-05-10T14:07:46Z

Alright so I ended up just moving this from the discussion into an issue since in reality it does produce broken code and this could bit uses if they try this out. In general I just don't think we should try to rewrite this, but I'm also not 100% sure how to avoid this. I'm not sure if there is some tag on a source when it's passed to the compiler indicating that it was generated or something, but if so we could detect that and not try to rewrite it I guess. I'm open for any idea/suggestions others might have here.

There's also the idea that in reality, this is valid code, so in reality rewriting should be able to work here.

ckipp01 · 2023-05-11T06:53:46Z

Alright, you can minimize this down to:

//> using scala 3.3.1-RC1-bin-20230510-d6c643c-NIGHTLY
//> using options -rewrite -indent

object example {
  def foo(a: Any) = ???

  def appply() = foo {
Seq(1, 2, 3)
  }
}

Running scala-cli compile . on this will result in:

//> using scala 3.3.1-RC1-bin-20230510-d6c643c-NIGHTLY
//> using options -rewrite -indent

object example:
  def foo(a: Any) = ???

  def appply() = foo:
Seq(1, 2, 3)

Which has broken indentation for the Seq(1, 2, 3).

adpi2 · 2023-05-12T12:17:42Z

In general it seems the compiler does not try to fix the indentation at all.

def f = {
val x = ""
x
}

is rewritten into:

def f =
val x = ""
x

I also found a few related/unrelated bugs/features:

The compiler does not rewrite the body if there is no '\n' after it:

class A {
  def foo = ""
}

class B {
  def bar = ""
} // no \n here

is rewritten to

class A:
  def foo = ""

class B {
  def bar = ""
}

It does not remove the braces of empty bodies class A {} or

class B {
}

ckipp01 added itype:bug area:rewriting tool labels May 10, 2023

adpi2 self-assigned this May 12, 2023

adpi2 linked a pull request May 16, 2023 that will close this issue

Patch indentation when removing braces (and other bug fixes in -indent -rewrite) #17522

Open

Kordyjan modified the milestone: 3.5.0 May 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite compiler flag on some generated code can break it #17456

Rewrite compiler flag on some generated code can break it #17456

ckipp01 commented May 10, 2023

ckipp01 commented May 10, 2023 •

edited

Loading

ckipp01 commented May 11, 2023

adpi2 commented May 12, 2023 •

edited

Loading

Rewrite compiler flag on some generated code can break it #17456

Rewrite compiler flag on some generated code can break it #17456

Comments

ckipp01 commented May 10, 2023

Discussed in #17453

ckipp01 commented May 10, 2023 • edited Loading

ckipp01 commented May 11, 2023

adpi2 commented May 12, 2023 • edited Loading

ckipp01 commented May 10, 2023 •

edited

Loading

adpi2 commented May 12, 2023 •

edited

Loading