Open kubukoz opened 3 years ago
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.
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
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.
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.
Scala 2 does look for implicitNotFound
in parents and we decided to keep consistent with this behaviour https://github.com/lampepfl/dotty/issues/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
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.
There is a suggestion for future linting https://github.com/scala/bug/issues/11653#issuecomment-516245681
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. https://github.com/fthomas/refined/issues/661#issuecomment-515936825
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.
Got bit by this again, it's super confusing - any chance we can get this prioritized?
Compiler version
3.0.0-RC1
Minimized code
https://github.com/kubukoz/given-repro
Output
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 hasMonadError[IO, Throwable]
, soMonad[IO]
is definitely there.