Skip to content

Commit

Permalink
Backports for a cats-1.6.x patch (#2861)
Browse files Browse the repository at this point in the history
* Change MonadErrorOps#reject so it no longer runs effects twice (#2810)

* Add regression test for MonadErrorOps bug

* Change MonadErrorOps#reject so it no longer re-runs the effect it is called on. Fixes #2809

* Fix Order.max and Oder.min description comments (#2842)

Changed description to better fit their implentation

* Make WrappedMutableMapBase extend Serializable (#2784)

We are running into Spark `Task not serializable` issues when a closure
that executes on a Spark executor node involves a `Map` that is created
via running `foldMap` on a `List`. This commit makes the
`WrappedMutableMap` hierarchy extend `Serializable` and chex that the
cerealization works (this test failed before extending `Serializable`).

* Optimize productR in Apply (#2728)
  • Loading branch information
rossabaker authored Jun 3, 2019
1 parent 64b83d0 commit de0056b
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 7 deletions.
2 changes: 1 addition & 1 deletion core/src/main/scala/cats/Apply.scala
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ trait Apply[F[_]] extends Functor[F] with InvariantSemigroupal[F] with ApplyArit
*
*/
def productR[A, B](fa: F[A])(fb: F[B]): F[B] =
map2(fa, fb)((_, b) => b)
ap(map(fa)(_ => (b: B) => b))(fb)

/**
* Compose two actions, discarding any value produced by the second.
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/cats/syntax/monadError.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ final class MonadErrorOps[F[_], E, A](private val fa: F[A]) extends AnyVal {
*/
def reject(pf: PartialFunction[A, E])(implicit F: MonadError[F, E]): F[A] =
F.flatMap(fa) { a =>
pf.andThen(F.raiseError[A]).applyOrElse(a, (_: A) => fa)
pf.andThen(F.raiseError[A] _).applyOrElse(a, F.pure)
}

def adaptError(pf: PartialFunction[E, E])(implicit F: MonadError[F, E]): F[A] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package compat

import scala.collection.mutable

abstract private[kernel] class WrappedMutableMapBase[K, V](m: mutable.Map[K, V]) extends Map[K, V] {
abstract private[kernel] class WrappedMutableMapBase[K, V](m: mutable.Map[K, V]) extends Map[K, V] with Serializable {
def +[V2 >: V](kv: (K, V2)): Map[K, V2] = m.toMap + kv
def -(key: K): Map[K, V] = m.toMap - key
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package compat

import scala.collection.mutable

abstract private[kernel] class WrappedMutableMapBase[K, V](m: mutable.Map[K, V]) extends Map[K, V] {
abstract private[kernel] class WrappedMutableMapBase[K, V](m: mutable.Map[K, V]) extends Map[K, V] with Serializable {
def updated[V2 >: V](key: K, value: V2): Map[K, V2] = m.toMap + ((key, value))
def remove(key: K): Map[K, V] = m.toMap - key
}
4 changes: 2 additions & 2 deletions kernel/src/main/scala/cats/kernel/Order.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ trait Order[@sp A] extends Any with PartialOrder[A] { self =>
def partialCompare(x: A, y: A): Double = compare(x, y).toDouble

/**
* If x <= y, return x, else return y.
* If x < y, return x, else return y.
*/
def min(x: A, y: A): A = if (lt(x, y)) x else y

/**
* If x >= y, return x, else return y.
* If x > y, return x, else return y.
*/
def max(x: A, y: A): A = if (gt(x, y)) x else y

Expand Down
6 changes: 6 additions & 0 deletions tests/src/test/scala/cats/tests/MapSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import cats.laws.discipline.{
}
import cats.laws.discipline.arbitrary._
import cats.arrow.Compose
import cats.kernel.instances.StaticMethods.wrapMutableMap

class MapSuite extends CatsSuite {
implicit val iso = SemigroupalTests.Isomorphisms.invariant[Map[Int, ?]]
Expand Down Expand Up @@ -38,4 +39,9 @@ class MapSuite extends CatsSuite {
map.show should ===(implicitly[Show[Map[Int, String]]].show(map))
}
}

{
val m = wrapMutableMap(scala.collection.mutable.Map(1 -> "one", 2 -> "two"))
checkAll("WrappedMutableMap", SerializableTests.serializable(m))
}
}
6 changes: 5 additions & 1 deletion tests/src/test/scala/cats/tests/RegressionSuite.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package cats
package tests

import cats.data.{Const, NonEmptyList}
import cats.data.{Const, NonEmptyList, StateT}
import scala.collection.mutable
import scala.collection.immutable.SortedMap
class RegressionSuite extends CatsSuite {
Expand Down Expand Up @@ -157,4 +157,8 @@ class RegressionSuite extends CatsSuite {

}

test("#2809 MonadErrorOps.reject runs effects only once") {
val program = StateT.modify[Either[Throwable, ?], Int](_ + 1).reject { case _ if false => new Throwable }
program.runS(0).right.get should ===(1)
}
}

0 comments on commit de0056b

Please sign in to comment.