Address feedback

typelevel · Sep 3, 2018 · b4c5080 · b4c5080
1 parent 46f2307
commit b4c5080
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 82 deletions.
diff --git a/core/src/main/scala/cats/data/EitherT.scala b/core/src/main/scala/cats/data/EitherT.scala
@@ -3,7 +3,6 @@ package data
 
 import cats.Bifunctor
 import cats.instances.either._
-import cats.instances.option._
 import cats.syntax.either._
 
 /**
@@ -515,28 +514,6 @@ private[data] abstract class EitherTInstances extends EitherTInstances1 {
       def defer[A](fa: => EitherT[F, L, A]): EitherT[F, L, A] =
         EitherT(F.defer(fa.value))
     }
-
-  implicit def catsDataTraverseFilterForEitherT[F[_], L](implicit F0: TraverseFilter[F]): TraverseFilter[EitherT[F, L, ?]] =
-    new EitherTFunctorFilter[F, L] with TraverseFilter[EitherT[F, L, ?]] {
-      implicit def F: FunctorFilter[F] = F0
-      def traverse: Traverse[EitherT[F, L, ?]] = catsDataTraverseForEitherT[F, L](F0.traverse)
-
-      def traverseFilter[G[_], A, B]
-      (fa: EitherT[F, L, A])
-      (f: A => G[Option[B]])
-      (implicit G: Applicative[G]): G[EitherT[F, L, B]] =
-        G.map(
-          F0.traverseFilter[G, Either[L, A], Either[L, B]](fa.value) {
-            case l@Left(_) => G.pure(Option(l.rightCast[B]))
-            case Right(a) => G.map(f(a))(_.map(Either.right))
-          })(EitherT(_))
-
-      override def filterA[G[_], A]
-      (fa: EitherT[F, L, A])
-      (f: A => G[Boolean])
-      (implicit G: Applicative[G]): G[EitherT[F, L, A]] =
-        G.map(F0.filterA(fa.value)(_.fold(_ => G.pure(true), f)))(EitherT[F, L, A])
-    }
 }
 
 private[data] abstract class EitherTInstances1 extends EitherTInstances2 {
@@ -568,9 +545,6 @@ private[data] abstract class EitherTInstances1 extends EitherTInstances2 {
       override def ensureOr[A](fa: EitherT[F, L, A])(error: (A) => L)(predicate: (A) => Boolean): EitherT[F, L, A] =
         fa.ensureOr(error)(predicate)(F)
     }
-
-  implicit def catsDataFunctorFilterForEitherT[F[_], L](implicit F0: FunctorFilter[F]): FunctorFilter[EitherT[F, L, ?]] =
-    new EitherTFunctorFilter[F, L] { implicit def F = F0 }
 }
 
 private[data] abstract class EitherTInstances2 extends EitherTInstances3 {
@@ -730,24 +704,3 @@ private[data] sealed trait EitherTOrder[F[_], L, A] extends Order[EitherT[F, L,
 
   override def compare(x: EitherT[F, L, A], y: EitherT[F, L, A]): Int = x compare y
 }
-
-private[data] sealed trait EitherTFunctorFilter[F[_], E] extends FunctorFilter[EitherT[F, E, ?]] {
-  implicit def F: FunctorFilter[F]
-
-  override def functor: Functor[EitherT[F, E, ?]] = EitherT.catsDataFunctorForEitherT[F, E](F.functor)
-
-  def mapFilter[A, B](fa: EitherT[F, E, A])(f: (A) => Option[B]): EitherT[F, E, B] =
-    EitherT[F, E, B](F.mapFilter(fa.value)(_.traverse(f)))
-
-  override def collect[A, B](fa: EitherT[F, E, A])(f: PartialFunction[A, B]): EitherT[F, E, B] = {
-    EitherT[F, E, B](F.mapFilter(fa.value)(_.traverse(f.lift)))
-  }
-
-  override def flattenOption[A](fa: EitherT[F, E, Option[A]]): EitherT[F, E, A] = {
-    EitherT[F, E, A](F.flattenOption[Either[E, A]](F.functor.map(fa.value)(Traverse[Either[E, ?]].sequence[Option, A])))
-  }
-
-  override def filter[A](fa: EitherT[F, E, A])(f: (A) => Boolean): EitherT[F, E, A] = {
-    EitherT[F, E, A](F.filter(fa.value)(_.forall(f)))
-  }
-}
diff --git a/core/src/main/scala/cats/data/Nested.scala b/core/src/main/scala/cats/data/Nested.scala
@@ -59,8 +59,6 @@ private[data] sealed abstract class NestedInstances extends NestedInstances0 {
       implicit val F: Traverse[F] = F0
       implicit val G: TraverseFilter[G] = G0
     }
-
-
 }
 
 private[data] sealed abstract class NestedInstances0 extends NestedInstances1 {

diff --git a/core/src/main/scala/cats/data/OptionT.scala b/core/src/main/scala/cats/data/OptionT.scala
@@ -1,7 +1,7 @@
 package cats
 package data
 
-import cats.instances.option.{catsStdInstancesForOption => optionInstance}
+import cats.instances.option.{catsStdInstancesForOption => optionInstance, catsStdTraverseFilterForOption}
 import cats.syntax.either._
 
 /**
@@ -235,19 +235,25 @@ private[data] sealed abstract class OptionTInstances extends OptionTInstances0 {
         OptionT(F.defer(fa.value))
     }
 
-  implicit def optionTFunctorFilter[F[_]: Functor]: FunctorFilter[OptionT[F, ?]] = {
-    new FunctorFilter[OptionT[F, ?]] {
-      override val functor: Functor[OptionT[F, ?]] = OptionT.catsDataFunctorForOptionT[F]
+  implicit def catsDateTraverseFilterForOptionT[F[_]](implicit F0: Traverse[F]): TraverseFilter[OptionT[F, ?]] =
+    new OptionTFunctorFilter[F] with TraverseFilter[OptionT[F, ?]] {
+      implicit def F: Functor[F] = F0
 
-      override def mapFilter[A, B](fa: OptionT[F, A])(f: (A) => Option[B]): OptionT[F, B] = fa.subflatMap(f)
+      val traverse: Traverse[OptionT[F, ?]] = OptionT.catsDataTraverseForOptionT[F]
 
-      override def collect[A, B](fa: OptionT[F, A])(f: PartialFunction[A, B]): OptionT[F, B] = fa.subflatMap(f.lift)
+      def traverseFilter[G[_], A, B](fa: OptionT[F, A])
+        (f: A => G[Option[B]])
+        (implicit G: Applicative[G]): G[OptionT[F, B]] =
+          G.map(Traverse[F].traverse[G, Option[A], Option[B]](fa.value) {
+            oa => TraverseFilter[Option].traverseFilter(oa)(f)
+          })(OptionT[F, B])
 
-      override def flattenOption[A](fa: OptionT[F, Option[A]]): OptionT[F, A] = fa.subflatMap(identity)
+      override def filterA[G[_], A](fa: OptionT[F, A])
+        (f: A => G[Boolean])
+        (implicit G: Applicative[G]): G[OptionT[F, A]] =
+          G.map(Traverse[F].traverse(fa.value)(TraverseFilter[Option].filterA[G, A](_)(f)))(OptionT[F, A])
 
-      override def filter[A](fa: OptionT[F, A])(f: (A) => Boolean): OptionT[F, A] = fa.filter(f)
     }
-  }
 }
 
 private[data] sealed abstract class OptionTInstances0 extends OptionTInstances1 {
@@ -265,6 +271,9 @@ private[data] sealed abstract class OptionTInstances0 extends OptionTInstances1
 
   implicit def catsDataPartialOrderForOptionT[F[_], A](implicit F0: PartialOrder[F[Option[A]]]): PartialOrder[OptionT[F, A]] =
     new OptionTPartialOrder[F, A] { implicit val F = F0 }
+
+  implicit def catsDateFunctorFilterForOptionT[F[_]](implicit F0: Functor[F]): FunctorFilter[OptionT[F, ?]] =
+    new OptionTFunctorFilter[F] { implicit val F = F0 }
 }
 
 private[data] sealed abstract class OptionTInstances1 extends OptionTInstances2 {
@@ -401,6 +410,20 @@ private[data] sealed trait OptionTPartialOrder[F[_], A] extends PartialOrder[Opt
   override def partialCompare(x: OptionT[F, A], y: OptionT[F, A]): Double = x partialCompare y
 }
 
+private[data] sealed trait OptionTFunctorFilter[F[_]] extends FunctorFilter[OptionT[F, ?]] {
+  implicit def F: Functor[F]
+
+  def functor: Functor[OptionT[F, ?]] = OptionT.catsDataFunctorForOptionT[F]
+
+  def mapFilter[A, B](fa: OptionT[F, A])(f: (A) => Option[B]): OptionT[F, B] = fa.subflatMap(f)
+
+  override def collect[A, B](fa: OptionT[F, A])(f: PartialFunction[A, B]): OptionT[F, B] = fa.subflatMap(f.lift)
+
+  override def flattenOption[A](fa: OptionT[F, Option[A]]): OptionT[F, A] = fa.subflatMap(identity)
+
+  override def filter[A](fa: OptionT[F, A])(f: (A) => Boolean): OptionT[F, A] = fa.filter(f)
+}
+
 private[data] sealed trait OptionTOrder[F[_], A] extends Order[OptionT[F, A]] with OptionTPartialOrder[F, A]{
   override implicit def F: Order[F[Option[A]]]
 

diff --git a/core/src/main/scala/cats/instances/option.scala b/core/src/main/scala/cats/instances/option.scala
@@ -136,13 +136,13 @@ trait OptionInstancesBinCompat0 {
 
     def traverseFilter[G[_], A, B](fa: Option[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Option[B]] =
       fa match {
-        case _: None.type => G.pure(Option.empty[B])
+        case None => G.pure(Option.empty[B])
         case Some(a) => f(a)
       }
 
     override def filterA[G[_], A](fa: Option[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Option[A]] =
       fa match {
-        case _: None.type => G.pure(Option.empty[A])
+        case None => G.pure(Option.empty[A])
         case Some(a) => G.map(f(a))(b => if (b) Some(a) else None)
       }
 

diff --git a/tests/src/test/scala/cats/tests/EitherTSuite.scala b/tests/src/test/scala/cats/tests/EitherTSuite.scala
@@ -47,28 +47,6 @@ class EitherTSuite extends CatsSuite {
 
   }
 
-  {
-    //If a TraverseFilter for F is defined
-    implicit val F = ListWrapper.traverseFilter
-
-    checkAll("EitherT[ListWrapper, Int, ?]",
-      TraverseFilterTests[EitherT[ListWrapper, Int, ?]].traverseFilter[Int, Int, Int])
-    checkAll("TraverseFilter[EitherT[ListWrapper, Int, ?]]",
-      SerializableTests.serializable(TraverseFilter[EitherT[ListWrapper, Int, ?]]))
-
-  }
-
-  {
-    //If a functorFilter for F is defined
-    implicit val F = ListWrapper.functorFilter
-
-    checkAll("EitherT[ListWrapper, Int, ?]",
-      FunctorFilterTests[EitherT[ListWrapper, Int, ?]].functorFilter[Int, Int, Int])
-    checkAll("FunctorFilter[EitherT[ListWrapper, Int, ?]]",
-      SerializableTests.serializable(FunctorFilter[EitherT[ListWrapper, Int, ?]]))
-
-  }
-
   {
     //if a Monad is defined
 

diff --git a/tests/src/test/scala/cats/tests/OptionTSuite.scala b/tests/src/test/scala/cats/tests/OptionTSuite.scala
@@ -13,6 +13,29 @@ class OptionTSuite extends CatsSuite {
   checkAll("OptionT[Eval, ?]", FunctorFilterTests[OptionT[Eval, ?]].functorFilter[Int, Int, Int])
 
 
+  {
+    //If a Functor for F is defined
+    implicit val F = ListWrapper.functor
+
+    checkAll("OptionT[ListWrapper, ?]",
+      FunctorFilterTests[OptionT[ListWrapper, ?]].functorFilter[Int, Int, Int])
+    checkAll("FunctorFilter[OptionT[ListWrapper, ?]]",
+      SerializableTests.serializable(FunctorFilter[OptionT[ListWrapper, ?]]))
+
+  }
+
+
+  {
+    //If a Traverse for F is defined
+    implicit val F = ListWrapper.traverse
+
+    checkAll("OptionT[ListWrapper, ?]",
+      TraverseFilterTests[OptionT[ListWrapper, ?]].traverseFilter[Int, Int, Int])
+    checkAll("TraverseFilter[OptionT[ListWrapper, ?]]",
+      SerializableTests.serializable(TraverseFilter[OptionT[ListWrapper, ?]]))
+
+  }
+
   {
     implicit val F = ListWrapper.eqv[Option[Int]]