Merge pull request #266 from mccartney/mccartney-variable-shadowing

Variable shadowing

README.md

@@ -139,7 +139,7 @@ For instructions on suppressing warnings by file, by inspection or by line see [
 
 ### Inspections
 
-There are currently 116 inspections. An overview list is given, followed by a more detailed description of each inspection after the list (todo: finish rest of detailed descriptions)
+There are currently 117 inspections. An overview list is given, followed by a more detailed description of each inspection after the list (todo: finish rest of detailed descriptions)
 
 | Name | Brief Description | Default Level |
 |------|-------------------|---------------|
@@ -257,6 +257,7 @@ There are currently 116 inspections. An overview list is given, followed by a mo
 | UseSqrt | Checks for use of `math.pow` for calculating `math.sqrt` | Info |
 | VarClosure | Finds closures that reference var | Warning |
 | VarCouldBeVal | Checks for `var`s that could be declared as `val`s | Warning |
+| VariableShadowing | Checks for multiple uses of the variable name in nested scopes | Warning |
 | WhileTrue | Checks for code that uses a `while(true)` or `do { } while(true)` block. | Warning |
 | ZeroNumerator | Checks for dividing by 0 by a number, eg `0 / x` which will always return `0` | Warning |
 

src/main/scala/com/sksamuel/scapegoat/ScapegoatConfig.scala

@@ -134,6 +134,7 @@ object ScapegoatConfig extends App {
     new UseLog10,
     new UseLog1P,
     new UseSqrt,
+    new VariableShadowing,
     new VarClosure,
     new VarCouldBeVal,
     new WhileTrue,

src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala

@@ -25,6 +25,8 @@ class VariableShadowing extends Inspection("Variable shadowing", Levels.Warning)
 
       override def inspect(tree: Tree): Unit = {
         tree match {
+          case Block(_, _) =>
+            enter(); continue(tree); exit()
           case ClassDef(_, _, _, Template(_, _, body)) =>
             enter(); continue(tree); exit()
           case ModuleDef(_, _, Template(_, _, body)) =>
@@ -41,13 +43,12 @@ class VariableShadowing extends Inspection("Variable shadowing", Levels.Warning)
             cases.foreach {
               case CaseDef(Bind(name, _), _, _) =>
                 if (isDefined(name.toString)) warn(tree)
-                continue(tree)
-              case _ =>
-                continue(tree)
+              case _ => // do nothing
             }
+            continue(tree)
           case _ => continue(tree)
         }
       }
     }
   }
-}
+}

src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala

@@ -116,6 +116,56 @@ class VariableShadowingTest
         compileCodeSnippet(code)
         compiler.scapegoat.feedback.warnings.size shouldBe 0
       }
+
+      "when two if branches define the same variable" in {
+        val code =
+            """class Test {
+            |  if (1 > 0) {
+            |    val something = 4
+            |    println(something+1)
+            |  } else {
+            |    val something = 2
+            |    println(something+2)
+            |  }
+            |}""".stripMargin
+
+        compileCodeSnippet(code)
+        compiler.scapegoat.feedback.warnings.size shouldBe 0
+      }
+
+      "when two sibling cases define the same local variable" in {
+        val code =
+            """class Test {
+            |  val possibility: Option[Int] = Some(3) 
+            |  possibility match {
+            |    case Some(x) => 
+            |      val y = x + 1    
+            |      println(y)
+            |    case None =>
+            |      val y = 0
+            |      println(y)     
+            |  }
+            |}""".stripMargin
+
+        compileCodeSnippet(code)
+        compiler.scapegoat.feedback.warnings.size shouldBe 0
+      }
+
+      "when visiting a match case, especially not visit it twice" in {
+        val code =
+            """class Test {
+            |  val possibility: Option[Int] = Some(3)
+            |  possibility match {
+            |    case Some(x) =>
+            |      val y = x + 1
+            |      println(y)
+            |    case None => println("None")
+            |  }
+            |}""".stripMargin
+
+        compileCodeSnippet(code)
+        compiler.scapegoat.feedback.warnings.size shouldBe 0
+      }
     }
   }
 }