From 8408933667393efcfeee1d0ba84862e7c119f539 Mon Sep 17 00:00:00 2001 From: Morgen Peschke Date: Mon, 10 Jun 2024 16:04:19 -0700 Subject: [PATCH 1/3] Add bridge between ApplicativeError and ApplicativeThrow --- .../main/scala/cats/ApplicativeError.scala | 32 +++++++++++++++++- .../test/scala/cats/tests/EitherSuite.scala | 33 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/ApplicativeError.scala b/core/src/main/scala/cats/ApplicativeError.scala index 7582fca4ed..b3eedc7936 100644 --- a/core/src/main/scala/cats/ApplicativeError.scala +++ b/core/src/main/scala/cats/ApplicativeError.scala @@ -21,7 +21,7 @@ package cats -import cats.ApplicativeError.CatchOnlyPartiallyApplied +import cats.ApplicativeError.{CatchOnlyAsPartiallyApplied, CatchOnlyPartiallyApplied} import cats.data.{EitherT, Validated} import cats.data.Validated.{Invalid, Valid} @@ -287,6 +287,26 @@ trait ApplicativeError[F[_], E] extends Applicative[F] { def catchOnly[T >: Null <: Throwable]: CatchOnlyPartiallyApplied[T, F, E] = new CatchOnlyPartiallyApplied[T, F, E](this) + /** + * Often E can be created from Throwable. Here we try to call pure or + * catch, adapt into E, and raise. + * + * Exceptions that cannot be adapted to E will be propagated + */ + def catchNonFatalAs[A](adaptIfPossible: Throwable => Option[E])(a: => A): F[A] = + try pure(a) + catch { + case e if NonFatal(e) => adaptIfPossible(e).map(raiseError[A]).getOrElse(throw e) + } + + /** + * Evaluates the specified block, catching exceptions of the specified type and attempting to map them to `E`. + * + * Uncaught exceptions and those that cannot be adapted to E will be propagated. + */ + def catchOnlyAs[T >: Null <: Throwable]: CatchOnlyAsPartiallyApplied[T, F, E] = + new CatchOnlyAsPartiallyApplied[T, F, E](this) + /** * If the error type is Throwable, we can convert from a scala.util.Try */ @@ -383,6 +403,16 @@ object ApplicativeError { } } + final private[cats] class CatchOnlyAsPartiallyApplied[T >: Null <: Throwable, F[_], E]( + private val F: ApplicativeError[F, E] + ) extends AnyVal { + def apply[A](adaptIfPossible: T => Option[E])(f: => A)(implicit CT: ClassTag[T], NT: NotNull[T]): F[A] = + try F.pure(f) + catch { + case CT(t) => adaptIfPossible(t).map(F.raiseError[A]).getOrElse(throw t) + } + } + /** * lift from scala.Option[A] to a F[A] * diff --git a/tests/shared/src/test/scala/cats/tests/EitherSuite.scala b/tests/shared/src/test/scala/cats/tests/EitherSuite.scala index eca7b226e7..79f6d3a80f 100644 --- a/tests/shared/src/test/scala/cats/tests/EitherSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/EitherSuite.scala @@ -23,6 +23,7 @@ package cats.tests import cats._ import cats.data.{EitherT, NonEmptyChain, NonEmptyList, NonEmptySet, NonEmptyVector, Validated} +import cats.syntax.option._ import cats.syntax.bifunctor._ import cats.kernel.laws.discipline.{EqTests, MonoidTests, OrderTests, PartialOrderTests, SemigroupTests} import cats.laws.discipline._ @@ -136,6 +137,38 @@ class EitherSuite extends CatsSuite { assert(Either.catchNonFatal(throw new Throwable("blargh")).isLeft) } + test("ApplicativeError instance catchNonFatalAs maps exceptions to E") { + val res = ApplicativeError[Either[String, *], String].catchNonFatalAs(_.getMessage.some)("foo".toInt) + assert(res === Left("For input string: \"foo\"")) + } + + test("ApplicativeError instance catchNonFatalAs propagates unmappable exceptions") { + val _ = intercept[NumberFormatException] { + ApplicativeError[Either[String, *], String].catchNonFatalAs(_ => none[String])("foo".toInt) + } + } + + test("ApplicativeError instance catchOnlyAs maps exceptions to E") { + val res = + ApplicativeError[Either[String, *], String] + .catchOnlyAs[NumberFormatException](_.getMessage.some)("foo".toInt) + assert(res === Left("For input string: \"foo\"")) + } + + test("ApplicativeError instance catchOnlyAs propagates unmappable exceptions") { + val _ = intercept[NumberFormatException] { + ApplicativeError[Either[String, *], String] + .catchOnlyAs[NumberFormatException](_ => none[String])("foo".toInt) + } + } + + test("ApplicativeError instance catchOnlyAs propagates non-matching exceptions") { + val _ = intercept[NumberFormatException] { + ApplicativeError[Either[String, *], String] + .catchOnlyAs[IndexOutOfBoundsException](_.getMessage.some)("foo".toInt) + } + } + test("fromTry is left for failed Try") { forAll { (t: Try[Int]) => assert(t.isFailure === (Either.fromTry(t).isLeft)) From 366d77644d1f8c8413f8fdaab216c709d9f2a253 Mon Sep 17 00:00:00 2001 From: Morgen Peschke Date: Tue, 11 Jun 2024 10:57:24 -0700 Subject: [PATCH 2/3] Tighten mapping requirements of catchOnlyAs Also expands scaladocs --- core/src/main/scala/cats/ApplicativeError.scala | 14 +++++++++----- .../src/test/scala/cats/tests/EitherSuite.scala | 13 +++---------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/core/src/main/scala/cats/ApplicativeError.scala b/core/src/main/scala/cats/ApplicativeError.scala index b3eedc7936..a2991c65cf 100644 --- a/core/src/main/scala/cats/ApplicativeError.scala +++ b/core/src/main/scala/cats/ApplicativeError.scala @@ -291,7 +291,7 @@ trait ApplicativeError[F[_], E] extends Applicative[F] { * Often E can be created from Throwable. Here we try to call pure or * catch, adapt into E, and raise. * - * Exceptions that cannot be adapted to E will be propagated + * Exceptions that cannot be adapted to E will be propagated outside of `F` */ def catchNonFatalAs[A](adaptIfPossible: Throwable => Option[E])(a: => A): F[A] = try pure(a) @@ -300,9 +300,13 @@ trait ApplicativeError[F[_], E] extends Applicative[F] { } /** - * Evaluates the specified block, catching exceptions of the specified type and attempting to map them to `E`. + * Evaluates the specified block, catching exceptions of the specified type and maps them to `E`. * - * Uncaught exceptions and those that cannot be adapted to E will be propagated. + * Uncaught exceptions will be propagated outside of `F` + * + * Note: `catchOnlyAs` assumes that, if a specific exception type `T` is expected, there + * exists a mapping from `T` to `E`. If this is not the case, consider either manually + * rethrowing inside the mapping function, or using `catchNonFatalAs` */ def catchOnlyAs[T >: Null <: Throwable]: CatchOnlyAsPartiallyApplied[T, F, E] = new CatchOnlyAsPartiallyApplied[T, F, E](this) @@ -406,10 +410,10 @@ object ApplicativeError { final private[cats] class CatchOnlyAsPartiallyApplied[T >: Null <: Throwable, F[_], E]( private val F: ApplicativeError[F, E] ) extends AnyVal { - def apply[A](adaptIfPossible: T => Option[E])(f: => A)(implicit CT: ClassTag[T], NT: NotNull[T]): F[A] = + def apply[A](adapt: T => E)(f: => A)(implicit CT: ClassTag[T], NT: NotNull[T]): F[A] = try F.pure(f) catch { - case CT(t) => adaptIfPossible(t).map(F.raiseError[A]).getOrElse(throw t) + case CT(t) => F.raiseError(adapt(t)) } } diff --git a/tests/shared/src/test/scala/cats/tests/EitherSuite.scala b/tests/shared/src/test/scala/cats/tests/EitherSuite.scala index 79f6d3a80f..94fa09c59d 100644 --- a/tests/shared/src/test/scala/cats/tests/EitherSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/EitherSuite.scala @@ -148,24 +148,17 @@ class EitherSuite extends CatsSuite { } } - test("ApplicativeError instance catchOnlyAs maps exceptions to E") { + test("ApplicativeError instance catchOnlyAs maps exceptions of the specified type to E") { val res = ApplicativeError[Either[String, *], String] - .catchOnlyAs[NumberFormatException](_.getMessage.some)("foo".toInt) + .catchOnlyAs[NumberFormatException](_.getMessage)("foo".toInt) assert(res === Left("For input string: \"foo\"")) } - test("ApplicativeError instance catchOnlyAs propagates unmappable exceptions") { - val _ = intercept[NumberFormatException] { - ApplicativeError[Either[String, *], String] - .catchOnlyAs[NumberFormatException](_ => none[String])("foo".toInt) - } - } - test("ApplicativeError instance catchOnlyAs propagates non-matching exceptions") { val _ = intercept[NumberFormatException] { ApplicativeError[Either[String, *], String] - .catchOnlyAs[IndexOutOfBoundsException](_.getMessage.some)("foo".toInt) + .catchOnlyAs[IndexOutOfBoundsException](_.getMessage)("foo".toInt) } } From e29d1d6dd62508d18c99012fd09591d9196cb528 Mon Sep 17 00:00:00 2001 From: Morgen Peschke Date: Thu, 13 Jun 2024 11:32:23 -0700 Subject: [PATCH 3/3] Add catch*Unsafe variants --- .../main/scala/cats/ApplicativeError.scala | 100 ++++++++++++++++-- .../test/scala/cats/tests/EitherSuite.scala | 44 ++++++-- .../src/test/scala/cats/tests/TrySuite.scala | 14 +++ 3 files changed, 143 insertions(+), 15 deletions(-) diff --git a/core/src/main/scala/cats/ApplicativeError.scala b/core/src/main/scala/cats/ApplicativeError.scala index a2991c65cf..d4b20efb2c 100644 --- a/core/src/main/scala/cats/ApplicativeError.scala +++ b/core/src/main/scala/cats/ApplicativeError.scala @@ -21,7 +21,12 @@ package cats -import cats.ApplicativeError.{CatchOnlyAsPartiallyApplied, CatchOnlyPartiallyApplied} +import cats.ApplicativeError.{ + CatchOnlyAsPartiallyApplied, + CatchOnlyAsUnsafePartiallyApplied, + CatchOnlyPartiallyApplied, + CatchOnlySafePartiallyApplied +} import cats.data.{EitherT, Validated} import cats.data.Validated.{Invalid, Valid} @@ -264,6 +269,8 @@ trait ApplicativeError[F[_], E] extends Applicative[F] { /** * Often E is Throwable. Here we try to call pure or catch * and raise. + * + * Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated */ def catchNonFatal[A](a: => A)(implicit ev: Throwable <:< E): F[A] = try pure(a) @@ -274,6 +281,8 @@ trait ApplicativeError[F[_], E] extends Applicative[F] { /** * Often E is Throwable. Here we try to call pure or catch * and raise + * + * Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated */ def catchNonFatalEval[A](a: Eval[A])(implicit ev: Throwable <:< E): F[A] = try pure(a.value) @@ -282,34 +291,84 @@ trait ApplicativeError[F[_], E] extends Applicative[F] { } /** - * Evaluates the specified block, catching exceptions of the specified type. Uncaught exceptions are propagated. + * Evaluates the specified block, catching exceptions of the specified type. + * + * This method is considered unsafe because uncaught exceptions are propagated + * outside of `F`. While the name does not currently reflect this, this may change + * at a future date. */ def catchOnly[T >: Null <: Throwable]: CatchOnlyPartiallyApplied[T, F, E] = new CatchOnlyPartiallyApplied[T, F, E](this) + /** + * Evaluates the specified block, catching exceptions of the specified type. + * + * Exceptions of type `T` are raised in `F`, other non-fatal exceptions are raised in `G` + * + * Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated + */ + def catchOnlySafe[G[_], T >: Null <: Throwable]: CatchOnlySafePartiallyApplied[T, G, F, E] = + new CatchOnlySafePartiallyApplied[T, G, F, E](this) + /** * Often E can be created from Throwable. Here we try to call pure or * catch, adapt into E, and raise. * - * Exceptions that cannot be adapted to E will be propagated outside of `F` + * Exceptions that cannot be adapted to E and raised in `F` will be raised in `G` + * + * Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated + */ + def catchNonFatalAs[G[_], A](adaptIfPossible: Throwable => Option[E])(a: => A)(implicit + G: ApplicativeThrow[G] + ): G[F[A]] = + try G.pure(pure(a)) + catch { + case t if NonFatal(t) => + adaptIfPossible(t).fold(G.raiseError[F[A]](t))(e => G.pure(raiseError[A](e))) + } + + /** + * Often E can be created from Throwable. Here we try to call pure or + * catch, adapt into E, and raise. + * + * This method is considered unsafe because exceptions that cannot be + * adapted to E will be rethrown outside of `F` + * + * Note: fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated */ - def catchNonFatalAs[A](adaptIfPossible: Throwable => Option[E])(a: => A): F[A] = + def catchNonFatalAsUnsafe[A](adaptIfPossible: Throwable => Option[E])(a: => A): F[A] = try pure(a) catch { case e if NonFatal(e) => adaptIfPossible(e).map(raiseError[A]).getOrElse(throw e) } /** - * Evaluates the specified block, catching exceptions of the specified type and maps them to `E`. + * Evaluates the specified block, catching exceptions of the specified type. * - * Uncaught exceptions will be propagated outside of `F` + * Caught exceptions are mapped to `E` and raised in `F`. + * Uncaught non-fatal exceptions will be raised in `G`. + * Fatal exceptions (as defined by `scala.util.control.NonFatal`) will be propagated * * Note: `catchOnlyAs` assumes that, if a specific exception type `T` is expected, there * exists a mapping from `T` to `E`. If this is not the case, consider either manually * rethrowing inside the mapping function, or using `catchNonFatalAs` */ - def catchOnlyAs[T >: Null <: Throwable]: CatchOnlyAsPartiallyApplied[T, F, E] = - new CatchOnlyAsPartiallyApplied[T, F, E](this) + def catchOnlyAs[G[_], T >: Null <: Throwable]: CatchOnlyAsPartiallyApplied[T, G, F, E] = + new CatchOnlyAsPartiallyApplied[T, G, F, E](this) + + /** + * Evaluates the specified block, catching exceptions of the specified type. + * + * Caught exceptions are mapped to `E` and raised in `F`. + * + * This method is considered unsafe because uncaught exceptions will be propagated outside of `F` + * + * Note: `catchOnlyAsUnsafe` assumes that, if a specific exception type `T` is expected, there + * exists a mapping from `T` to `E`. If this is not the case, consider either manually + * rethrowing inside the mapping function, or using `catchNonFatalAsUnsafe` + */ + def catchOnlyAsUnsafe[T >: Null <: Throwable]: CatchOnlyAsUnsafePartiallyApplied[T, F, E] = + new CatchOnlyAsUnsafePartiallyApplied[T, F, E](this) /** * If the error type is Throwable, we can convert from a scala.util.Try @@ -407,7 +466,30 @@ object ApplicativeError { } } - final private[cats] class CatchOnlyAsPartiallyApplied[T >: Null <: Throwable, F[_], E]( + final private[cats] class CatchOnlySafePartiallyApplied[T, G[_], F[_], E](private val F: ApplicativeError[F, E]) + extends AnyVal { + def apply[A]( + f: => A + )(implicit CT: ClassTag[T], NT: NotNull[T], ev: Throwable <:< E, G: ApplicativeThrow[G]): G[F[A]] = + try G.pure(F.pure(f)) + catch { + case t if CT.runtimeClass.isInstance(t) => G.pure(F.raiseError(t)) + case t if NonFatal(t) => G.raiseError(t) + } + } + + final private[cats] class CatchOnlyAsPartiallyApplied[T >: Null <: Throwable, G[_], F[_], E]( + private val F: ApplicativeError[F, E] + ) extends AnyVal { + def apply[A](adapt: T => E)(f: => A)(implicit CT: ClassTag[T], NT: NotNull[T], G: ApplicativeThrow[G]): G[F[A]] = + try G.pure(F.pure(f)) + catch { + case CT(t) => G.pure(F.raiseError(adapt(t))) + case t if NonFatal(t) => G.raiseError[F[A]](t) + } + } + + final private[cats] class CatchOnlyAsUnsafePartiallyApplied[T >: Null <: Throwable, F[_], E]( private val F: ApplicativeError[F, E] ) extends AnyVal { def apply[A](adapt: T => E)(f: => A)(implicit CT: ClassTag[T], NT: NotNull[T]): F[A] = diff --git a/tests/shared/src/test/scala/cats/tests/EitherSuite.scala b/tests/shared/src/test/scala/cats/tests/EitherSuite.scala index 94fa09c59d..590c38b914 100644 --- a/tests/shared/src/test/scala/cats/tests/EitherSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/EitherSuite.scala @@ -138,27 +138,59 @@ class EitherSuite extends CatsSuite { } test("ApplicativeError instance catchNonFatalAs maps exceptions to E") { - val res = ApplicativeError[Either[String, *], String].catchNonFatalAs(_.getMessage.some)("foo".toInt) + val res = + ApplicativeError[Either[String, *], String] + .catchNonFatalAs[Either[Throwable, *], Int](_.getMessage.some)("foo".toInt) + .leftMap(0 -> _.getMessage) + assert(res === Right(Left("For input string: \"foo\""))) + } + + test("ApplicativeError instance catchNonFatalAs raises unmappable exceptions in G") { + val res = + ApplicativeError[Either[String, *], String] + .catchNonFatalAs[Either[Throwable, *], Int](_ => none[String])("foo".toInt) + .leftMap(0 -> _.getMessage) + assert(res === Left(0 -> "For input string: \"foo\"")) + } + + test("ApplicativeError instance catchNonFatalAsUnsafe maps exceptions to E") { + val res = ApplicativeError[Either[String, *], String].catchNonFatalAsUnsafe(_.getMessage.some)("foo".toInt) assert(res === Left("For input string: \"foo\"")) } - test("ApplicativeError instance catchNonFatalAs propagates unmappable exceptions") { + test("ApplicativeError instance catchNonFatalAsUnsafe rethrows unmappable exceptions") { val _ = intercept[NumberFormatException] { - ApplicativeError[Either[String, *], String].catchNonFatalAs(_ => none[String])("foo".toInt) + ApplicativeError[Either[String, *], String].catchNonFatalAsUnsafe(_ => none[String])("foo".toInt) } } test("ApplicativeError instance catchOnlyAs maps exceptions of the specified type to E") { val res = ApplicativeError[Either[String, *], String] - .catchOnlyAs[NumberFormatException](_.getMessage)("foo".toInt) + .catchOnlyAs[Either[Throwable, *], NumberFormatException](_.getMessage)("foo".toInt) + .leftMap(0 -> _.getMessage) + assert(res === Right(Left("For input string: \"foo\""))) + } + + test("ApplicativeError instance catchOnlyAs raises non-matching exceptions in G") { + val res = + ApplicativeError[Either[String, *], String] + .catchOnlyAs[Either[Throwable, *], IndexOutOfBoundsException](_.getMessage)("foo".toInt) + .leftMap(0 -> _.getMessage) + assert(res === Left(0 -> "For input string: \"foo\"")) + } + + test("ApplicativeError instance catchOnlyAsUnsafe maps exceptions of the specified type to E") { + val res = + ApplicativeError[Either[String, *], String] + .catchOnlyAsUnsafe[NumberFormatException](_.getMessage)("foo".toInt) assert(res === Left("For input string: \"foo\"")) } - test("ApplicativeError instance catchOnlyAs propagates non-matching exceptions") { + test("ApplicativeError instance catchOnlyAsUnsafe propagates non-matching exceptions") { val _ = intercept[NumberFormatException] { ApplicativeError[Either[String, *], String] - .catchOnlyAs[IndexOutOfBoundsException](_.getMessage)("foo".toInt) + .catchOnlyAsUnsafe[IndexOutOfBoundsException](_.getMessage)("foo".toInt) } } diff --git a/tests/shared/src/test/scala/cats/tests/TrySuite.scala b/tests/shared/src/test/scala/cats/tests/TrySuite.scala index 8208cfad11..156706dd39 100644 --- a/tests/shared/src/test/scala/cats/tests/TrySuite.scala +++ b/tests/shared/src/test/scala/cats/tests/TrySuite.scala @@ -103,6 +103,20 @@ class TrySuite extends CatsSuite { } } + test("catchOnlySafe works") { + forAll { (e: Either[String, Int]) => + val str = e.fold(identity, _.toString) + val res = MonadThrow[Try].catchOnlySafe[Try, NumberFormatException](str.toInt) + // the above should never raise an exception in the outer try + assertEquals(res.toEither.map(_.isSuccess), Right(e.isRight), clue(res)) + } + } + + test("catchOnlySafe only raise the specified type in Try") { + val res = MonadThrow[Try].catchOnlySafe[Try, UnsupportedOperationException]("str".toInt) + assert(res.isFailure, clue(res)) + } + test("fromTry works") { forAll { (t: Try[Int]) => assert((MonadThrow[Try].fromTry(t)) === t)