From 3fa21f4337349b45b5c0a0e2a0a8425066d051b9 Mon Sep 17 00:00:00 2001 From: Cody Allen Date: Sat, 31 Oct 2015 07:34:03 -0400 Subject: [PATCH 1/3] Don't infer Null in Xor.fromTryCatch I also renamed `fromTryCatch` to `catching`, because it's a bit less verbose and I like the sound of it a little more. I can change it back if people would prefer though. --- core/src/main/scala/cats/NotNull.scala | 24 +++++++++++++++++++ core/src/main/scala/cats/data/Xor.scala | 19 ++++++++++----- .../src/test/scala/cats/tests/XorTests.scala | 4 ++-- 3 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 core/src/main/scala/cats/NotNull.scala diff --git a/core/src/main/scala/cats/NotNull.scala b/core/src/main/scala/cats/NotNull.scala new file mode 100644 index 0000000000..1822a66108 --- /dev/null +++ b/core/src/main/scala/cats/NotNull.scala @@ -0,0 +1,24 @@ +package cats + +/** + * An instance of `NotNull[A]` indicates that `A` does not have a static type + * of `Null`. + * + * This can be useful in preventing `Null` from being inferred when a type + * parameter is omitted. + */ +sealed trait NotNull[A] + +object NotNull { + /** + * Since NotNull is just a marker trait with no functionality, it's safe to + * reuse a single instance of it. This helps prevent unnecessary allocations. + */ + private[this] val singleton: NotNull[Any] = new NotNull[Any] {} + + implicit def ambiguousNull1: NotNull[Null] = throw new Exception("An instance of NotNull[Null] was used. This should never happen. Both ambiguousNull1 and ambiguousNull2 should always be in scope if one of them is.") + + implicit def ambiguousNull2: NotNull[Null] = ambiguousNull1 + + implicit def notNull[A]: NotNull[A] = singleton.asInstanceOf[NotNull[A]] +} diff --git a/core/src/main/scala/cats/data/Xor.scala b/core/src/main/scala/cats/data/Xor.scala index 6e8cea7973..7066f0999e 100644 --- a/core/src/main/scala/cats/data/Xor.scala +++ b/core/src/main/scala/cats/data/Xor.scala @@ -217,20 +217,27 @@ trait XorFunctions { * the resulting `Xor`. Uncaught exceptions are propagated. * * For example: {{{ - * val result: NumberFormatException Xor Int = fromTryCatch[NumberFormatException] { "foo".toInt } + * val result: NumberFormatException Xor Int = catching[NumberFormatException] { "foo".toInt } * }}} */ - def fromTryCatch[T >: Null <: Throwable]: FromTryCatchAux[T] = - new FromTryCatchAux[T] + def catching[T >: Null <: Throwable]: CatchingAux[T] = + new CatchingAux[T] - final class FromTryCatchAux[T] private[XorFunctions] { - def apply[A](f: => A)(implicit T: ClassTag[T]): T Xor A = + final class CatchingAux[T] private[XorFunctions] { + def apply[A](f: => A)(implicit CT: ClassTag[T], NT: NotNull[T]): T Xor A = try { right(f) } catch { - case t if T.runtimeClass.isInstance(t) => + case t if CT.runtimeClass.isInstance(t) => left(t.asInstanceOf[T]) } + } + + def catchingNonFatal[A](f: => A): Throwable Xor A = + try { + right(f) + } catch { + case scala.util.control.NonFatal(t) => left(t) } /** diff --git a/tests/src/test/scala/cats/tests/XorTests.scala b/tests/src/test/scala/cats/tests/XorTests.scala index 369424c3a6..0616979797 100644 --- a/tests/src/test/scala/cats/tests/XorTests.scala +++ b/tests/src/test/scala/cats/tests/XorTests.scala @@ -33,12 +33,12 @@ class XorTests extends CatsSuite { checkAll("? Xor ?", BifunctorTests[Xor].bifunctor[Int, Int, Int, String, String, String]) test("fromTryCatch catches matching exceptions") { - assert(Xor.fromTryCatch[NumberFormatException]{ "foo".toInt }.isInstanceOf[Xor.Left[NumberFormatException]]) + assert(Xor.catching[NumberFormatException]{ "foo".toInt }.isInstanceOf[Xor.Left[NumberFormatException]]) } test("fromTryCatch lets non-matching exceptions escape") { val _ = intercept[NumberFormatException] { - Xor.fromTryCatch[IndexOutOfBoundsException]{ "foo".toInt } + Xor.catching[IndexOutOfBoundsException]{ "foo".toInt } } } From c6d3ea446024bcca85174adc8155e415cc834423 Mon Sep 17 00:00:00 2001 From: Cody Allen Date: Tue, 3 Nov 2015 18:33:09 -0500 Subject: [PATCH 2/3] Rename Xor methods to catchOnly and catchAll And also use a slightly ridiculous name for one of the ambiguous NotNull[Null] instances to steer people in the right direction. --- core/src/main/scala/cats/NotNull.scala | 6 ++++-- core/src/main/scala/cats/data/Xor.scala | 4 ++-- docs/src/main/tut/traverse.md | 2 +- docs/src/main/tut/xor.md | 11 +++++++++-- .../src/test/scala/cats/tests/XorTests.scala | 19 +++++++++++++++---- 5 files changed, 31 insertions(+), 11 deletions(-) diff --git a/core/src/main/scala/cats/NotNull.scala b/core/src/main/scala/cats/NotNull.scala index 1822a66108..60ecf0ca6a 100644 --- a/core/src/main/scala/cats/NotNull.scala +++ b/core/src/main/scala/cats/NotNull.scala @@ -16,9 +16,11 @@ object NotNull { */ private[this] val singleton: NotNull[Any] = new NotNull[Any] {} - implicit def ambiguousNull1: NotNull[Null] = throw new Exception("An instance of NotNull[Null] was used. This should never happen. Both ambiguousNull1 and ambiguousNull2 should always be in scope if one of them is.") + private[this] def ambiguousException: Exception = new Exception("An instance of NotNull[Null] was used. This should never happen. Both ambiguous NotNull[Null] instances should always be in scope if one of them is.") - implicit def ambiguousNull2: NotNull[Null] = ambiguousNull1 + implicit def `If you are seeing this, you probably need to add an explicit type parameter somewhere, beause Null is being inferred.`: NotNull[Null] = throw ambiguousException + + implicit def ambiguousNull2: NotNull[Null] = throw ambiguousException implicit def notNull[A]: NotNull[A] = singleton.asInstanceOf[NotNull[A]] } diff --git a/core/src/main/scala/cats/data/Xor.scala b/core/src/main/scala/cats/data/Xor.scala index 7066f0999e..b737fd9508 100644 --- a/core/src/main/scala/cats/data/Xor.scala +++ b/core/src/main/scala/cats/data/Xor.scala @@ -220,7 +220,7 @@ trait XorFunctions { * val result: NumberFormatException Xor Int = catching[NumberFormatException] { "foo".toInt } * }}} */ - def catching[T >: Null <: Throwable]: CatchingAux[T] = + def catchOnly[T >: Null <: Throwable]: CatchingAux[T] = new CatchingAux[T] final class CatchingAux[T] private[XorFunctions] { @@ -233,7 +233,7 @@ trait XorFunctions { } } - def catchingNonFatal[A](f: => A): Throwable Xor A = + def catchNonFatal[A](f: => A): Throwable Xor A = try { right(f) } catch { diff --git a/docs/src/main/tut/traverse.md b/docs/src/main/tut/traverse.md index 767a4816b2..cef281b1a5 100644 --- a/docs/src/main/tut/traverse.md +++ b/docs/src/main/tut/traverse.md @@ -93,7 +93,7 @@ import cats.std.list._ import cats.syntax.traverse._ def parseIntXor(s: String): Xor[NumberFormatException, Int] = - Xor.fromTryCatch[NumberFormatException](s.toInt) + Xor.catchOnly[NumberFormatException](s.toInt) def parseIntValidated(s: String): ValidatedNel[NumberFormatException, Int] = Validated.fromTryCatch[NumberFormatException](s.toInt).toValidatedNel diff --git a/docs/src/main/tut/xor.md b/docs/src/main/tut/xor.md index 358a4fcee4..6c329c7839 100644 --- a/docs/src/main/tut/xor.md +++ b/docs/src/main/tut/xor.md @@ -340,13 +340,20 @@ val xor: Xor[NumberFormatException, Int] = } ``` -However, this can get tedious quickly. `Xor` provides a `fromTryCatch` method on its companion object +However, this can get tedious quickly. `Xor` provides a `catchOnly` method on its companion object that allows you to pass it a function, along with the type of exception you want to catch, and does the above for you. ```tut val xor: Xor[NumberFormatException, Int] = - Xor.fromTryCatch[NumberFormatException]("abc".toInt) + Xor.catchOnly[NumberFormatException]("abc".toInt) +``` + +If you want to catch all (non-fatal) throwables, you can use `catchNonFatal`. + +```tut +val xor: Xor[Throwable, Int] = + Xor.catchNonFatal("abc".toInt) ``` ## Additional syntax diff --git a/tests/src/test/scala/cats/tests/XorTests.scala b/tests/src/test/scala/cats/tests/XorTests.scala index 0616979797..aa4589c5cb 100644 --- a/tests/src/test/scala/cats/tests/XorTests.scala +++ b/tests/src/test/scala/cats/tests/XorTests.scala @@ -32,13 +32,24 @@ class XorTests extends CatsSuite { checkAll("? Xor ?", BifunctorTests[Xor].bifunctor[Int, Int, Int, String, String, String]) - test("fromTryCatch catches matching exceptions") { - assert(Xor.catching[NumberFormatException]{ "foo".toInt }.isInstanceOf[Xor.Left[NumberFormatException]]) + test("catchOnly catches matching exceptions") { + assert(Xor.catchOnly[NumberFormatException]{ "foo".toInt }.isInstanceOf[Xor.Left[NumberFormatException]]) } - test("fromTryCatch lets non-matching exceptions escape") { + test("catchOnly lets non-matching exceptions escape") { val _ = intercept[NumberFormatException] { - Xor.catching[IndexOutOfBoundsException]{ "foo".toInt } + Xor.catchOnly[IndexOutOfBoundsException]{ "foo".toInt } + } + } + + test("catchNonFatal catches non-fatal exceptions") { + assert(Xor.catchNonFatal{ "foo".toInt }.isLeft) + assert(Xor.catchNonFatal{ new Throwable("blargh") }.isLeft) + } + + test("catchOnly lets non-matching exceptions escape") { + val _ = intercept[NumberFormatException] { + Xor.catchOnly[IndexOutOfBoundsException]{ "foo".toInt } } } From 5411e9366b15d765d4c86bed603c3ff274b8e5c4 Mon Sep 17 00:00:00 2001 From: Cody Allen Date: Tue, 3 Nov 2015 20:08:08 -0500 Subject: [PATCH 3/3] Validated.catchOnly and catchAll --- core/src/main/scala/cats/data/Validated.scala | 16 +++++++++++----- docs/src/main/tut/traverse.md | 2 +- .../test/scala/cats/tests/ValidatedTests.scala | 13 +++++++++---- tests/src/test/scala/cats/tests/XorTests.scala | 8 +------- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/core/src/main/scala/cats/data/Validated.scala b/core/src/main/scala/cats/data/Validated.scala index a7d998f35f..f35f318f01 100644 --- a/core/src/main/scala/cats/data/Validated.scala +++ b/core/src/main/scala/cats/data/Validated.scala @@ -228,22 +228,28 @@ trait ValidatedFunctions { * the resulting `Validated`. Uncaught exceptions are propagated. * * For example: {{{ - * val result: Validated[NumberFormatException, Int] = fromTryCatch[NumberFormatException] { "foo".toInt } + * val result: Validated[NumberFormatException, Int] = catchOnly[NumberFormatException] { "foo".toInt } * }}} */ - def fromTryCatch[T >: Null <: Throwable]: FromTryCatchPartiallyApplied[T] = new FromTryCatchPartiallyApplied[T] + def catchOnly[T >: Null <: Throwable]: CatchOnlyPartiallyApplied[T] = new CatchOnlyPartiallyApplied[T] - final class FromTryCatchPartiallyApplied[T] private[ValidatedFunctions] { - def apply[A](f: => A)(implicit T: ClassTag[T]): Validated[T, A] = { + final class CatchOnlyPartiallyApplied[T] private[ValidatedFunctions] { + def apply[A](f: => A)(implicit T: ClassTag[T], NT: NotNull[T]): Validated[T, A] = try { valid(f) } catch { case t if T.runtimeClass.isInstance(t) => invalid(t.asInstanceOf[T]) } - } } + def catchNonFatal[A](f: => A): Validated[Throwable, A] = + try { + valid(f) + } catch { + case scala.util.control.NonFatal(t) => invalid(t) + } + /** * Converts a `Try[A]` to a `Validated[Throwable, A]`. */ diff --git a/docs/src/main/tut/traverse.md b/docs/src/main/tut/traverse.md index cef281b1a5..f4de95ce4a 100644 --- a/docs/src/main/tut/traverse.md +++ b/docs/src/main/tut/traverse.md @@ -96,7 +96,7 @@ def parseIntXor(s: String): Xor[NumberFormatException, Int] = Xor.catchOnly[NumberFormatException](s.toInt) def parseIntValidated(s: String): ValidatedNel[NumberFormatException, Int] = - Validated.fromTryCatch[NumberFormatException](s.toInt).toValidatedNel + Validated.catchOnly[NumberFormatException](s.toInt).toValidatedNel val x1 = List("1", "2", "3").traverseU(parseIntXor) val x2 = List("1", "abc", "3").traverseU(parseIntXor) diff --git a/tests/src/test/scala/cats/tests/ValidatedTests.scala b/tests/src/test/scala/cats/tests/ValidatedTests.scala index 1e3383201e..3c9ae4d103 100644 --- a/tests/src/test/scala/cats/tests/ValidatedTests.scala +++ b/tests/src/test/scala/cats/tests/ValidatedTests.scala @@ -26,16 +26,21 @@ class ValidatedTests extends CatsSuite { Applicative[Validated[String, ?]].ap2(Invalid("1"), Invalid("2"))(Valid(plus)) should === (Invalid("12")) } - test("fromTryCatch catches matching exceptions") { - assert(Validated.fromTryCatch[NumberFormatException]{ "foo".toInt }.isInstanceOf[Invalid[NumberFormatException]]) + test("catchOnly catches matching exceptions") { + assert(Validated.catchOnly[NumberFormatException]{ "foo".toInt }.isInstanceOf[Invalid[NumberFormatException]]) } - test("fromTryCatch lets non-matching exceptions escape") { + test("catchOnly lets non-matching exceptions escape") { val _ = intercept[NumberFormatException] { - Validated.fromTryCatch[IndexOutOfBoundsException]{ "foo".toInt } + Validated.catchOnly[IndexOutOfBoundsException]{ "foo".toInt } } } + test("catchNonFatal catches non-fatal exceptions") { + assert(Validated.catchNonFatal{ "foo".toInt }.isInvalid) + assert(Validated.catchNonFatal{ throw new Throwable("blargh") }.isInvalid) + } + test("fromTry is invalid for failed try"){ forAll { t: Try[Int] => t.isFailure should === (Validated.fromTry(t).isInvalid) diff --git a/tests/src/test/scala/cats/tests/XorTests.scala b/tests/src/test/scala/cats/tests/XorTests.scala index aa4589c5cb..b8f6ebecf2 100644 --- a/tests/src/test/scala/cats/tests/XorTests.scala +++ b/tests/src/test/scala/cats/tests/XorTests.scala @@ -44,13 +44,7 @@ class XorTests extends CatsSuite { test("catchNonFatal catches non-fatal exceptions") { assert(Xor.catchNonFatal{ "foo".toInt }.isLeft) - assert(Xor.catchNonFatal{ new Throwable("blargh") }.isLeft) - } - - test("catchOnly lets non-matching exceptions escape") { - val _ = intercept[NumberFormatException] { - Xor.catchOnly[IndexOutOfBoundsException]{ "foo".toInt } - } + assert(Xor.catchNonFatal{ throw new Throwable("blargh") }.isLeft) } test("fromTry is left for failed Try") {