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

sourcePositionMapper of sbt doesn't work with DelegatingReporter of Dotty #14691

Closed
tototoshi opened this issue Mar 15, 2022 · 9 comments
Closed
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug

Comments

@tototoshi
Copy link

Compiler version

3.1.1

Minimized code

project/plugins.sbt

addSbtPlugin("com.typesafe.play" % "sbt-twirl" % "1.6.0-M1")

build.sbt

enablePlugins(SbtTwirl)
scalaVersion := "3.1.1"
crossScalaVersions := Seq("2.13.8", "3.1.1")

libraryDependencies := libraryDependencies.value.map {
  case module if module.name == "twirl-api" =>
    module.cross(CrossVersion.for3Use2_13)
  case module => module
}

src/main/twirl/error.scala.html

<h1>Hello @world</h1>

Output

toshi@/Users/toshi/tmp/twirl-example% sbt ++2.13.8 compile
...
[info] compiling 1 Scala source to /Users/toshi/tmp/twirl-example/target/scala-2.13/classes ...
[error] /Users/toshi/tmp/twirl-example/src/main/twirl/error.scala.html:1:12: not found: value world
[error] <h1>Hello @world</h1>
[error]            ^
[error] one error found
[error] (Compile / compileIncremental) Compilation failed
[error] Total time: 2 s, completed 2022/03/15 22:08:00
toshi@/Users/toshi/tmp/twirl-example% sbt ++3.1.1 compile
...
[info] compiling 1 Scala source to /Users/toshi/tmp/twirl-example/target/scala-3.1.1/classes ...
[error] -- [E006] Not Found Error: /Users/toshi/tmp/twirl-example/target/scala-3.1.1/twirl/main/html/error.template.scala:19:63
[error] 19 |Seq[Any](format.raw/*1.1*/("""<h1>Hello """),_display_(/*1.12*/world),format.raw/*1.17*/("""</h1>
[error]    |                                                               ^^^^^
[error]    |                                                        Not found: world
[error] one error found
[error] one error found
[error] (Compile / compileIncremental) Compilation failed
[error] Total time: 4 s, completed 2022/03/15 22:08:15

Expectation

When we write a template (.scala.html) for playframework/twirl, The template will be converted to a .scala file and compiled as a Scala object. If errors are found, Twirl shows them as template errors using sourcePositionMappers of sbt.
https://github.com/playframework/twirl/blob/2c064e741ff8771c68e6397518fc05cb16d61c67/sbt-twirl/src/main/scala/play/twirl/sbt/SbtTwirl.scala#L79

This works with Scala2 but doesn't work with Scala3. We see error messages directly from Dotty in Scala3.

I think this is because DelegatingReporter of Dotty passes a rendered parameter to xsbti.Reporter.
https://github.com/lampepfl/dotty/blob/d2ebd75a2e63e3dd885e7bad94472931d0bae296/sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java#L47

If the rendered parameter is passed, sbt will display the contents of the rendered parameter, and information other than the rendered parameter is ignored. The error information translated by sourcePositionMappers in sbt's ManagedLoggedReporter will also not be displayed.

If I modify the DelegatingReporter so that the rendered parameter is not passed to sbt, it is possible to display the same error message as when compiling with Scala2. However, the message stored in the rendered parameter is from Dotty and contains more detailed error messages, so I feel a bit uncomfortable making it completely invisible. What do you think?

@SethTisue
Copy link
Member

SethTisue commented Mar 15, 2022

I don't feel able to tackle this myself, but I do hope that a volunteer can be found, since it would help Play get onto Scala 3.

@smarter
Copy link
Member

smarter commented Mar 15, 2022

It seems that the API provided by zinc in xsbti doesn't expose the sourcePositionMappers stuff, so this will likely require some work on zinc. Alternatively someone could come up with a build-independent way to express source maps (e.g., an annotation that would be interpreted by the compiler). Either way this is likely to be a lot of work that will require significant effort from Someone™.

@smarter
Copy link
Member

smarter commented Mar 15, 2022

A temporary hack could be to turn off the use of rendered only if the filename ends with .template.scala and hope that only twirl uses that format...

@odersky odersky added area:reporting Error reporting including formatting, implicit suggestions, etc and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 15, 2022
@smarter
Copy link
Member

smarter commented Mar 15, 2022

I've opened sbt/zinc#1074.

@eed3si9n
Copy link
Member

I don't feel able to tackle this myself, but I do hope that a volunteer can be found, since it would help Play get onto Scala 3.

I'd be happy to assist anyone who wants to work on Zinc, if Zinc needs updates.

@brbrown25
Copy link

I don't feel able to tackle this myself, but I do hope that a volunteer can be found, since it would help Play get onto Scala 3.

I'd be happy to assist anyone who wants to work on Zinc, if Zinc needs updates.

I'm always happy to learn new things and help out, maybe we can find sometime to collaborate together on this :)

@mkurz
Copy link
Contributor

mkurz commented Jul 10, 2024

IMHO this issue is not relevant anymore. Based on the initial comment I created a reproducer and starting with sbt 1.7.0 the output is correct also when using Scala 3.
See https://github.com/mkurz/scala3-14691

# Works:
sbt --sbt-version 1.6.2 ++2.13.14 compile
# Fails:
sbt --sbt-version 1.6.2 ++3.3.3 compile

# Starting with sbt 1.7.0 it also works with Scala 3:
sbt --sbt-version 1.7.0 ++3.3.3 compile
# Check with sbt 1.10.1, works also:
sbt --sbt-version 1.10.1 ++3.3.3 compile

@tototoshi @brbrown25 @smarter @eed3si9n @SethTisue Can you confirm?

@brbrown25
Copy link

I'll give it a try once I get a moment

@tototoshi
Copy link
Author

The problem with Twirl and sbt have been resolved by the PR I submitted to sbt/zinc. If no other issues have been found, I believe this PR can be closed.
sbt/zinc#1082

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug
Projects
None yet
Development

No branches or pull requests

7 participants