Either.catchOnly and -Yexplicit-nulls are not playing well together

[joan38]

Hi,

Either.catchOnly does not seem usable with -Yexplicit-nulls enabled on Scala 3.5.2:

[error] -- [E057] Type Mismatch Error: /.../CaseAppParsers.scala:18:17 
[error] 18 |      .catchOnly[NumberFormatException](Duration(s))
[error]    |                 ^
[error]    |Type argument NumberFormatException does not conform to lower bound Null
[error]    |
[error]    | longer explanation available when compiling with `-explain`
[error] two errors found

Is there something that can be done at the library level?

Thanks

[johnynek]

looks like the type signature requires it?

cats/core/src/main/scala/cats/syntax/either.scala

Line 398 in 1cc04ec

def catchOnly[T >: Null <: Throwable]: CatchOnlyPartiallyApplied[T] =

I don't know why that >: Null was put in there. Maybe someone else knows.

Here is the full code:

  final private[syntax] class CatchOnlyPartiallyApplied[T](private val dummy: Boolean = true) extends AnyVal {
    def apply[A](f: => A)(implicit CT: ClassTag[T], NT: NotNull[T]): Either[T, A] =
      try {
        Right(f)
      } catch {
        case t if CT.runtimeClass.isInstance(t) =>
          Left(t.asInstanceOf[T])
      }
  }
}

I have no idea why the NotNull[T] was added, but also we need >: Null. Seems like all we should require is that T <: AnyRef to be able to use runtimeClass.isInstance but that's true of any exception...

[satorg]

@johnynek

I have no idea why the NotNull[T] was added, but also we need >: Null

I was wondering once too and asked pretty much the same question in Discord's scala-users.

@s5bug helped me to figure it out (thank you!).

I still feel it is not obvious from the code at all and perhaps there should be an explanation in scaladocs for that method.

[s5bug]

For posterity & archival, the >: Null bound is to prevent unhelpful inference of Nothing (i.e. in the case of catchOnly { 123 }). It ensures that a type argument is explicitly applied.

[johnynek]

That’s unfortunate that it breaks explicit null though.

Not sure it’s the right trade if we could get explicit null.

[satorg]

There is a discussion about the catchOnly method itself in #4616.
It seems this and similar methods do not exactly meet the Cats convention for methods to be "safe" and "pure" by default. Otherwise, such methods should be marked as "unsafe".

So perhaps(!) the right way to address it would be to create a new bunch of methods like catchOnlyUnsafe and such and make sure they work on both Scala2 and Scala3 with/without explicit nulls syntax. Then we could gradually deprecate and decommission the old impure ones.

[armanbilge]

There is a discussion about the catchOnly method itself in #4616.

Ah yes, thanks for the reminder. I think my opinion is that catchOnly should be deprecated without replacement.

[satorg]

Arman decided to raise the stakes 😄