Pattern decomposition failing in for comprehension for Either

[Andrapyre]

Compiler version

3.2.0. Also tried 3.3.0-RC2 and found the same bug.

Cats Effect version

3.4.8

Minimized code

import cats.effect.IO

val someIOOp: IO[(Int, String)] = IO.pure((1,"sample"))

for {
  (num, str) <- someIOOp
} yield ()

Output

[error] 44 |      (num, str) <- someIOOp
[error]    |                    ^^^^^^^^
[error]    |       value withFilter is not a member of cats.effect.IO[(Int, String)]

Expectation

Expected that decomposition would succeed inside the for comprehension and that num and str would be parsed to Int and String respectively. The compiler parses a normal flatMap call correctly. So the following correctly returns the number:

someIOOp.flatMap { (num, str) =>
  IO(num)
}

It's just decomposition inside the for comprehension that fails for the IO. This bug does not affect Option, but it does affect Either, since Either lacks the .withFilter method.

[Andrapyre]

Current workaround with an extension like so:

object IOOps {
  extension [A](op: IO[A])
    def withFilter(func: A => Boolean): IO[A] = op
}

[soronpo]

Can you provide a minimization without the cats dependency?

[Andrapyre]

@soronpo Sure. The below will not compile, as tuple decomposition in for comprehensions also fails with Either or any other class that lacks the .withFilter method.

val someEitherOp: Either[Exception, (Int, String)] = Right((1,"sample"))

for {
  (num, str) <- someEitherOp
} yield ()

Interesting note: Option possesses the .withFilter method, but this was apparently introduced only for for comprehensions (see here). Is it still necessary for classes to have a .withFilter method in order to use for comprehensions? Getting rid of this would be great - otherwise we would need to add the .withFilter method to Either or any other type (such as IO) that is regularly used in for comprehensions. As it is, a for comprehension should be the same as a series of flatMap/map calls and both flatMap and map methods decompose tuples properly. See below:

someEitherOp.flatMap((num, _) => Right(num))

someEitherOp.map((num, _) => num)

.withFilter for for comprehensions seems to be a legacy hack and it would be great if we could remove it, but I don't know the scala code base well enough to know how feasible that is.

[kubukoz]

Seeing this too, with Either, in 3.3.0.

[Andrapyre]

@anatoliykmetyuk , can we have an update on this?

[nicolasstucki]

val someEitherOp: Either[Exception, (Int, String)] = Right((1,"sample"))
for {
 (num, str) <- someEitherOp
} yield ()

The documentation of https://www.scala-lang.org/api/2.13.6/scala/util/Either.html states that

// Guard expressions are not supported:
for {
  i <- right1
  if i > 0
} yield i
// error: value withFilter is not a member of Right[Double,Int]

// Similarly, refutable patterns are not supported:
for (x: Int <- right1) yield x
// error: value withFilter is not a member of Right[Double,Int]

which probably also covers this case.

I assume that there might be a fundamental reason for this to not be implemented in the standard library. If not this is something that would need to change in the Scala 2 standard library directly.

[Andrapyre]

@nicolasstucki , thanks for the link, as it's helpful to show some of the issues that .withFilter (or its absense) is solving for. Why is tuple decomposition linked to guards? There is no guard in my original example - it's just decomposition of a known structure - nothing should fail. As stated above, this decomposition works in a simple .flatMap call, but not in a for comprehension. The example in the link you provided does not work as a .flatMap call because it does not return an Either value if the guard condition is not met. See the following does not compile:

Altered Code:

val right1: Either[Exception, (Int, String)] = Right((1,"sample"))

right1.flatMap(res => {
  if (res._1 > 0) {
    res
  }
})

Output

Found:    Unit
Required: Either[Any, Any]
Maybe you are missing an else part for the conditional?
      if (res._1 > 0) {

[Andrapyre]

@nicolasstucki , in short, because tuple decomposition does work in a .flatMap call, but not a for comprehension, I believe it should still be labeled a bug.

[nicolasstucki]

because this does work in a .flatMap call, but not a for comprehension, I believe it should still be labeled a bug.

That for comprehension desugars into a withFilter and map because of the pattern. It is not equivalent to the flatMap at this abstraction level.

The spec explicitly states

The translation scheme is as follows. In a first step, every generator p <- e, where p
is not irrefutable for the type of e is replaced by

p <- e.withFilter { case p => true; case _ => false }

Therefore it is not a bug.

[nicolasstucki]

Maybe you should just use

def test(either: Either[Exception, (Int, String)]) =
  for {
-    (num, str) <- either
+    (num, str) <- either.toOption
  } yield ()

[som-snytt]

There is discussion of filterOrElse on the PR for 2.12 and possible future enhancements for 2.14.

scala/scala#5135

In short, you need a Left value for filtering.

Deprecated right projection has filter which is really filterToOption. So toOption is the implied idiom.

[Andrapyre]

@nicolasstucki , a few questions:

1. What is the main use case for the .filter method and why would the pattern in this instance call for reverting to it and .map instead of a normal .flatMap call? As a normal user, this feels unintuitive.

2. Option basically hardcodes the .filter method so that it always returns true and it does it specifically to make for comprehensions work (see link above). Why is this not possible with Either?

[nicolasstucki]

1. What is the main use case for the .filter method and why would the pattern in this instance call for reverting to it and .map instead of a normal .flatMap call? As a normal user, this feels unintuitive.

Not sure. There are some concerns about laziness in the withFilter that might play an important role in this.

2. Option basically hardcodes the .filter method so that it always returns true and it does it specifically to make for comprehensions work (see link above). Why is this not possible with Either?

This issue is that Either does not have a bias. (num, str) <- either could extract the Left or the Right, we do not have a way to know that during the desugaring.

[Andrapyre]

@nicolasstucki , since Scala 2.12 Either is right-biased and works fine in for comprehensions. The issue is not Either's bias, but the fact that it is not destructuring a Set properly in a for comprehension as we would expect it to. Could we do one of the following:

1. Create a default for .withFilter functionality in a for comprehension for all types, so that if a type does not possess the .withFilter method the for comprehension proceeds as if the .withFilter method returned true? (Ok solution)

2. Alter the for comprehension pattern so that .withFilter is not called at all in an instance like this (why should it be, it does nothing here) and instead revert to a normal .flatMap call? (Ideal solution)

3. Hardcode .withFilter to return true for Either (least preferable, as it will need to be done for all other relevant types as well. Still, better than nothing).

[kubukoz]

Do we actually still need this issue in the presence of -source:future ? The OP sample with IO and the Either one both work fine under that flag.