False positive "Empty if statement"

[flipsi]

Unlike #125, I think scapegoat really finds false positives for the Empty if statement warning.

When defining a function like this:

  def checkFoo(things: Future[Seq[String]]): Future[Unit] = {
    things.map(ts => if (ts.contains("foo")) () else throw new Exception("No foo!"))
  }

scapegoat will throw this warning:

[warning] com.mycom.foobar.File.scala:42: Empty if statement
          if (ts.contains[String]("foo"))
  ()
else
  throw new scala.`package`.Exception("No foo!")

The function is supposed to return Future.unit in case monadic things does contain "foo" and a failed future otherwise.

[BalmungSan]

I do not think you should use map for a function that throws an exception.

Maybe this would be better. (Not sure if that would fix the warning)

def checkFoo(things: Future[Seq[String]]): Future[Unit] =
  things.transform {
    case Success(ts) =>
      if (ts.contains("foo"))
        Success(())
      else
        Failure(new Exception("No foo!"))
    case f => f
  }

[flipsi]

@BalmungSan Thanks for your answer! Maybe you are right and this should be written in another way. In fact, we could write

def checkFoo(things: Future[Seq[String]]): Future[Unit] = {
    things.transformWith {
        case Success(ts) => if (ts.contains("foo")) Future.unit else Future.failed(new Exception("No foo!"))
        case Failure(e)  => Future.failed(e)
  }

and work around the issue. But the way I see it, transform/transformWith should be used when the failure case needs to be handled explicitly, which is not the case here.

Anyway, this is not the point. The point is: The if statement is not empty, but scapegoat reports it as empty. I consider this a false positive.

[xplosunn]

I tried to fix this bug but couldn't because at the build step that Scapegoat runs, the if's empty block has been compiled into a unit literal, so I have no way to distinguish between your code and an if with an empty block.