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

Endpoint compilation errors point to initialize() method on Scala 3 #141

Open
keynmol opened this issue Sep 27, 2024 · 6 comments
Open

Endpoint compilation errors point to initialize() method on Scala 3 #141

keynmol opened this issue Sep 27, 2024 · 6 comments

Comments

@keynmol
Copy link

keynmol commented Sep 27, 2024

//> using dep com.lihaoyi::cask::0.9.4
//> using scala 3.5.1

object OptimizerServer extends cask.MainRoutes {
  @cask.get("/")
  def index() =
    (1, 2)

  initialize()
}

image

Compiling project (Scala 3.5.1, JVM (17))
[error] ./test.scala:9:3
[error] can't convert scala.Tuple2 to a response
[error]   initialize()
[error]   ^^^^^^^^^^^^
Error compiling project (Scala 3.5.1, JVM (17))
@bishabosha
Copy link

the initialize macro needs to be updated to report its errors with explicit positions

@jodersky
Copy link
Member

jodersky commented Sep 27, 2024

@bishabosha, I think the macro is already reporting the error with an explicit position

When I try a similar example, I get this error:

object Routes extends cask.Routes:

  @cask.get("/foo")
  def foo() =
    (1,1)

  initialize()
[error] 102 |  initialize()
[error]     |  ^^^^^^^^^^^^
[error]     |can't convert scala.Tuple2 to a response
[error]     |---------------------------------------------------------------------------
[error]     |Inline stack trace
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     |This location contains code that was inlined from Routes.scala:99
[error]  99 |  def foo() =
[error]     |      ^
[error]      ---------------------------------------------------------------------------
[error] one error found
1 targets failed

So it appears that the inner error is somehow hidden in the examples above. Could it be an issue with the way the editor displays the errors?

On a side note, I think the error message definitely has room for improvement "can't convert X to a response" is not very useful. When I ported the macro to scala3, I didn't make this a priority, but I think it would be nice to detail what the expected response type actually is, and also include the implicit lookups that were attempted.

@lolgab
Copy link
Member

lolgab commented Sep 27, 2024

I could reproduce the error without cask and opened a bug report on the scala3 repository: scala/scala3#21666

@jodersky
Copy link
Member

As an alternative solution. we could simply also include the method's position in the error message. This way it's at least obvious what part of the code is logically causing the error, regardless of the position of the actual compile error.

For example, here's a quick snippet to replace in the above mentioned macro:

report.error(s"error in route definition `def ${method.name}` (at ${method.pos.get}): the method's return type ${rtp.show} cannot be converted to the expected endpoint response type ${innerReturnedTpt.show}", method.pos.get)

with this change, the error message would look something like this:

[error] -- Error: /home/jodersky/p/cask/example/decorated/app/src/Decorated.scala:7:12 -
[error] 7 |  initialize()
[error]   |  ^^^^^^^^^^^^
[error]   |error in route definition `def hello` (at /home/jodersky/p/cask/example/decorated/app/src/Decorated.scala:<79..79>): the method's return type scala.Tuple2[scala.Int, scala.Int] cannot be converted to the expected response type cask.model.Response[cask.model.Response.Data]
[error]   |-----------------------------------------------------------------------------
[error]   |Inline stack trace
[error]   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]   |This location contains code that was inlined from Decorated.scala:6
[error] 6 |  def hello(world: String) = (1, 1)
[error]   |      ^
[error]    -----------------------------------------------------------------------------
[error] one error found
1 targets failed

@lihaoyi
Copy link
Member

lihaoyi commented Sep 27, 2024

I think reporting the method position in the error message would be great, that way we get both the initialize() position and the def position, whereas previously we only got the def position which was always a bit mysterious.

One issue would be whether it works nicely in IDEs, but the command-line terror report should be great

@bishabosha
Copy link

bishabosha commented Sep 28, 2024

the bits from the inline stack trace can be included as DiagnosticRelatedInformation in a BSP diagnostic, which I think scala 3 does emit and sbt supports

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

No branches or pull requests

5 participants