Merge pull request #2859 from armanbilge/issue/2858

Fix `IO#syncStep` double-execution
typelevel · Mar 7, 2022 · 4171faf · 4171faf
2 parents d640e49 + af3c602
commit 4171faf
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 5 deletions.
diff --git a/core/shared/src/main/scala/cats/effect/IO.scala b/core/shared/src/main/scala/cats/effect/IO.scala
@@ -843,28 +843,28 @@ sealed abstract class IO[+A] private () extends IOPlatform[A] {
 
         case IO.Map(ioe, f, _) =>
           interpret(ioe).map {
-            case Left(_) => Left(io)
+            case Left(io) => Left(io.map(f))
             case Right(a) => Right(f(a))
           }
 
         case IO.FlatMap(ioe, f, _) =>
           interpret(ioe).flatMap {
-            case Left(_) => SyncIO.pure(Left(io))
+            case Left(io) => SyncIO.pure(Left(io.flatMap(f)))
             case Right(a) => interpret(f(a))
           }
 
         case IO.Attempt(ioe) =>
           interpret(ioe)
             .map {
-              case Left(_) => Left(io)
+              case Left(io) => Left(io.attempt)
               case Right(a) => Right(a.asRight[Throwable])
             }
             .handleError(t => Right(t.asLeft[IO[B]]))
 
         case IO.HandleErrorWith(ioe, f, _) =>
           interpret(ioe)
             .map {
-              case Left(_) => Left(io)
+              case Left(io) => Left(io.handleErrorWith(f))
               case Right(a) => Right(a)
             }
             .handleErrorWith(t => interpret(f(t)))
@@ -919,7 +919,7 @@ object IO extends IOCompanionPlatform with IOLowPriorityImplicits {
    * Newtype encoding for an `IO` datatype that has a `cats.Applicative` capable of doing
    * parallel processing in `ap` and `map2`, needed for implementing `cats.Parallel`.
    *
-   * For converting back and forth you can use either the `Parallel[IO]` instance or 
+   * For converting back and forth you can use either the `Parallel[IO]` instance or
    * the methods `cats.effect.kernel.Par.ParallelF.apply` for wrapping any `IO` value and
    * `cats.effect.kernel.Par.ParallelF.value` for unwrapping it.
    *

diff --git a/tests/shared/src/test/scala/cats/effect/IOSpec.scala b/tests/shared/src/test/scala/cats/effect/IOSpec.scala
@@ -1342,6 +1342,50 @@ class IOSpec extends BaseSpec with Discipline with IOPlatformSpecification {
             case Right(_) => SyncIO.raiseError[Unit](new RuntimeException("Boom!"))
           } must completeAsSync(())
       }
+
+      "should not execute effects twice for map (#2858)" in ticked { implicit ticker =>
+        var i = 0
+        val io = (IO(i += 1) *> IO.cede).void.syncStep.unsafeRunSync() match {
+          case Left(io) => io
+          case Right(_) => IO.unit
+        }
+        io must completeAs(())
+        i must beEqualTo(1)
+      }
+
+      "should not execute effects twice for flatMap (#2858)" in ticked { implicit ticker =>
+        var i = 0
+        val io = (IO(i += 1) *> IO.cede *> IO.unit).syncStep.unsafeRunSync() match {
+          case Left(io) => io
+          case Right(_) => IO.unit
+        }
+        io must completeAs(())
+        i must beEqualTo(1)
+      }
+
+      "should not execute effects twice for attempt (#2858)" in ticked { implicit ticker =>
+        var i = 0
+        val io = (IO(i += 1) *> IO.cede).attempt.void.syncStep.unsafeRunSync() match {
+          case Left(io) => io
+          case Right(_) => IO.unit
+        }
+        io must completeAs(())
+        i must beEqualTo(1)
+      }
+
+      "should not execute effects twice for handleErrorWith (#2858)" in ticked {
+        implicit ticker =>
+          var i = 0
+          val io = (IO(i += 1) *> IO.cede)
+            .handleErrorWith(_ => IO.unit)
+            .syncStep
+            .unsafeRunSync() match {
+            case Left(io) => io
+            case Right(_) => IO.unit
+          }
+          io must completeAs(())
+          i must beEqualTo(1)
+      }
     }
 
     "fiber repeated yielding test" in real {