com-lihaoyi / cask

Cask: a Scala HTTP micro-framework. Cask makes it easy to set up a website, backend server, or REST API using Scala
https://com-lihaoyi.github.io/cask/
Other
529 stars 57 forks source link

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

Open keynmol opened 2 weeks ago

keynmol commented 2 weeks ago
//> 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 commented 1 week ago

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

jodersky commented 1 week ago

@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 commented 1 week ago

I could reproduce the error without cask and opened a bug report on the scala3 repository: https://github.com/scala/scala3/issues/21666

jodersky commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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