Invalid error message on missing given instance

[kubukoz]

Compiler version

3.0.0-RC1

Minimized code

https://github.com/kubukoz/given-repro

// requires cats-effect 3.0.0-RC2 or 2.3.3
// "org.typelevel" %% "cats-effect" % "2.3.3"

import cats.MonadError
import cats.effect.IO

sealed trait Failure extends Throwable

def demo[F[_]](using MonadError[F, Failure]) = ???

def demo2 = demo[IO]

Output

[E] [E1] src/main/scala/Main.scala:8:21
[E]      Could not find an instance of Monad for cats.effect.IO
[E]      L8: def demo2 = demo[IO]
[E]                              ^
[E] src/main/scala/Main.scala: L8 [E1]

Expectation

Something along the lines of

Could not find an instance of MonadError[[a] =>> F[a], Failure] for cats.effect.IO

More context

The typeclass hierarchy is: MonadError[F[_], E] <: Monad[F]. Cats IO has MonadError[IO, Throwable], so Monad[IO] is definitely there.

[kubukoz]

I wasn't able to reproduce this without the dependency (with fake types for MonadError etc.), so it might have something to do with implicit priority etc.

[smarter]

This message comes from the @implicitNotFound of Monad: https://github.com/typelevel/cats/blob/6bd3e00c50ad1d30a3cd4b55fc967ea0c1c1310c/core/src/main/scala/cats/Monad.scala#L15
We intentionally look for @implicitNotFound in parents, but I guess Scala 2 doesn't? Maybe we should revert to the Scala 2 behavior here to avoid confusion /cc @prolativ

[kubukoz]

oh, that would explain why I wasn't getting it to work. I think that behavior of looking up the annotation in parents would be quite confusing if someone didn't know this is happening. And I'm not entirely sure it's helpful if you know 😅 after all, in this case the message is a lie.

[djspiewak]

I think there's also a fair argument to be made that the implicitNotFound in question is doing more harm than good even on Scala 2.

[prolativ]

Scala 2 does look for implicitNotFound in parents and we decided to keep consistent with this behaviour #10098.
The difference is that in Scala 2 the message from implicitNotFound does not override the default error message but is appended to it so for a Scala 2 version of the reported snippet the error is something like

could not find implicit value for parameter ev: cats.MonadError[cats.effect.IO,Failure] (Could not find an instance of Monad for cats.effect.IO)

So the questions are:

1. Should we prepend the default error message as Scala 2 does? This gives you some additional information if you know how this message is constructed but might be quite confusing otherwise, as in the example above. I believe the intention of displaying only the message from the annotation was that if as the author of a library you consistently annotate all the types that are used as implicit parameters, the users of the library might not even need to (fully) understand what an implicit is (especially that we now use given/using instead in most places) - the custom message can contain information about what needs to be done to make the code compile (e.g. add a specific import).
2. Should we revert to the previous behaviour and not inherit the implicitNotFound annotation? Or maybe we should require that if a parent class is annotated its children should be always explicitly annotated as well. Java seems to have some generic mechanisms for controlling inheritance of annotations (@Inherited meta-annotation) but I'm not sure if/how this currently works for scala as well

[smarter]

Should we prepend the default error message as Scala 2 does?

Because existing implicitNotFound messages are designed with this behavior in mind, I think it makes sense to replicate it yes, we can revisit this later.

[som-snytt]

There is a suggestion for future linting scala/bug#11653 (comment)

I think the idea was to detect whether the parent message applies (type params align).

I don't know whether messages are designed with this behavior in mind. It is more of a back-up to report "as much info as possible" when annotations are missing. Lukas suggested warning when the "overriding" annotation is omitted. fthomas/refined#661 (comment)

[kubukoz]

1. Should we prepend the default error message as Scala 2 does? This gives you some additional information if you know how this message is constructed but might be quite confusing otherwise, as in the example above. I believe the intention of displaying only the message from the annotation was that if as the author of a library you consistently annotate all the types that are used as implicit parameters, the users of the library might not even need to (fully) understand what an implicit is (especially that we now use `given`/`using` instead in most places) - the custom message can contain information about what needs to be done to make the code compile (e.g. add a specific import).

@prolativ

I think this makes the most sense.

[kubukoz]

Got bit by this again, it's super confusing - any chance we can get this prioritized?