Skip to content

Statements must return unit #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
blast-hardcheese opened this issue May 19, 2019 · 8 comments
Open

Statements must return unit #23

blast-hardcheese opened this issue May 19, 2019 · 8 comments

Comments

@blast-hardcheese
Copy link

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)
            }
          )
    }
  )
}
@oleg-py
Copy link
Owner

oleg-py commented May 21, 2019

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.

@blast-hardcheese
Copy link
Author

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

@blast-hardcheese
Copy link
Author

@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.

@oleg-py
Copy link
Owner

oleg-py commented Jul 20, 2019

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

@blast-hardcheese
Copy link
Author

Oh, excellent! I'll try right now!

@blast-hardcheese
Copy link
Author

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.

@blast-hardcheese
Copy link
Author

Thanks!

@oleg-py
Copy link
Owner

oleg-py commented Jul 20, 2019

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

@oleg-py oleg-py reopened this Jul 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants