HairyFotr / linter

Static Analysis Compiler Plugin for Scala
Apache License 2.0
268 stars 34 forks source link

Questioning UnnecessaryElseBranch when the "then" branch always returns. #49

Closed TomasMikula closed 8 years ago

TomasMikula commented 8 years ago

The UnnecessaryElseBranch warning is issued when

"the then branch always returns"

What is the reasoning behind issuing this warning? When the if condition fails, the else branch is executed, so it doesn't seem unnecessary to me.

This is one of the unit tests for UnnecessaryElseBranch:

should("""
  def test(): Any = {
    if(util.Random.nextBoolean) {
      println("foo"); return 5; println("foo2");
    } else {
      println("foo3"); println("foo4");
    }
  }""")

I don't see why the else branch would be unnecessary here.

HairyFotr commented 8 years ago

It's a style warning that the code in the else block could be moved outside the if to decrease code nesting, like this:

noLint("""
  def test(): Any = {
    if(util.Random.nextBoolean) {
      println("foo"); return 5; println("foo2");
    }
    println("foo3"); println("foo4");
  }
}""")

I'll change the text of the warning, as it really doesn't explain this, and add this example to the test cases. Thanks for reporting!

TomasMikula commented 8 years ago

Oh, OK, thanks!