oleg-py/better-monadic-for

Statements must return unit

Opened this issue · 8 comments

The following causes some noise during migration to this plugin:

import cats.implicits._
case class Foo(a: Long)
for {
  Foo(a) <- Either.right[String, Foo](Foo(1))
  b <- Either.right(Foo(2))
  Foo(c) = b
} yield a + c

yields

scala> for { Foo(a) <- Either.right[String, Foo](Foo(1)); b <- Either.right(Foo(2)); Foo(c) = b } yield a + c
<console>:17: warning: [wartremover:NonUnitStatements] Statements must return Unit
       for { Foo(a) <- Either.right[String, Foo](Foo(1)); b <- Either.right(Foo(2)); Foo(c) = b } yield a + c
                                                                                        ^
res1: scala.util.Either[String,Long] = Right(3)

Walking through the tree, I can't even see where this error is coming from. By spacing out the above code, I can narrow it down to:

<console>:20: warning: [wartremover:NonUnitStatements] Statements must return Unit
         Foo(c) = b
            ^

Any pointers or insight would be appreciated. This is already excellent, just looking to learn.

The simplified tree from the SBT console is:

object $iw extends scala.AnyRef {
  def <init>() = {
    super.<init>();
    ()
  };
  val res1 = Either.right[String, Foo](Foo(1)).withFilter((check$ifrefutable$1) =>
    check$ifrefutable$1: @scala.unchecked match {
      case Foo((a @ _)) => true
      case _ => false
    }
  )
  .flatMap((x$4) =>
    x$4: @scala.unchecked match {
      case Foo((a @ _)) =>
        Either.right(Foo(2))
          .map((b) => {
            <synthetic> <artifact> private[this] val x$2 = b: @scala.unchecked match {
              case (x$1 @ Foo((c @ _))) => scala.Tuple2(x$1, c)
            };
            val x$1 = x$2._1;
            val c = x$2._2;
            scala.Tuple2(b, x$1)
          })
          .map((x$3) =>
            x$3: @scala.unchecked match {
              case scala.Tuple2((b @ _), Foo((c @ _))) => a.$plus(c)
            }
          )
    }
  )
}

That tree looks like one between parser and bm4 first phase, so it won't tell you anything. What you want to check is tree right after typer or bm4-typer

It's probably caused by tuple removal feature, which, to support -Ywarn-unused:vars and side-effects, has to wrap stuff in locally (see comment in the source, and there is no stdlib function that gives you an ability to discard value.

So, your best bet is probably disabling that feature by adding -P:bm4:no-tupling:n to scalac options. Or, if that doesn't help, try to figure out which features are causing the issues.

I'll take a look if I can add a SuppressWarning for WartRemover just to a block later on, but don't hold your breath on that for now.

Thanks for the explanation, I'll give it a shot!

@oleg-py So, with bm4 defaults:

[warn] .../src/main/scala/Test1.scala:6:12: [wartremover:NonUnitStatements] Statements must return Unit
[warn]     Foo(a) <- Either.right[String, Foo](Foo(1))
[warn]            ^
[warn] .../src/main/scala/Test1.scala:8:8: [wartremover:NonUnitStatements] Statements must return Unit
[warn]     Foo(c) = b
[warn]        ^

with -P:bm4:no-tupling:n, it reduces to:

[warn] .../src/main/scala/Test1.scala:6:12: [wartremover:NonUnitStatements] Statements must return Unit
[warn]     Foo(a) <- Either.right[String, Foo](Foo(1))
[warn]            ^

Neither -P:bm4:no-filtering:n nor -P:bm4:implicit-patterns:n have any impact on the original example code, and -P:bm4:no-map-id:n makes everything stop compiling.

Closing with no resolution isn't super great, imo -- if you've got some advice on how I can help resolve this, I'm willing to do the work, as this is preventing me from making warnings errors.

Sorry, it appears to be fixed in 0.3.1, please try updating

Oh, excellent! I'll try right now!

OK, so it appears that the fix resolves my real warnings! The one thing that's baffling is:

[warn] .../src/main/scala/Test1.scala:6:12: [wartremover:NonUnitStatements] Statements must return Unit
[warn]     Foo(a) <- Either.right[String, Foo](Foo(1))
[warn]            ^

still exists with 0.3.1. I'm not concerned about this, though it may cause false negatives in tests when trying to create stubs that match the required types.

Thanks!

That's odd. I'll leave it open to look into later.