From 0c9c5d2c3dcb9418772a3cb58a52b974b0974e1e Mon Sep 17 00:00:00 2001 From: Grzegorz Oledzki Date: Sun, 24 Nov 2019 15:02:56 +0100 Subject: [PATCH 1/6] UT for same variable name in sibling if branches --- .../inspections/VariableShadowingTest.scala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala b/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala index b028aad6..645a2d9e 100644 --- a/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala +++ b/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala @@ -116,6 +116,22 @@ 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 + } } } } From 7e70ab0b6664170eb89e4a27513cd2de7d3fbbc8 Mon Sep 17 00:00:00 2001 From: Grzegorz Oledzki Date: Sun, 24 Nov 2019 21:17:06 +0100 Subject: [PATCH 2/6] UT for same variable name in sibling match cases --- .../inspections/VariableShadowingTest.scala | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala b/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala index 645a2d9e..671cf205 100644 --- a/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala +++ b/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala @@ -132,6 +132,24 @@ class VariableShadowingTest 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 + } } } } From 7b8c383e505baa19b0fb5d8abddca7bb0be0b636 Mon Sep 17 00:00:00 2001 From: Grzegorz Oledzki Date: Mon, 25 Nov 2019 20:11:57 +0100 Subject: [PATCH 3/6] UT for not visiting the same match case twice --- .../inspections/VariableShadowingTest.scala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala b/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala index 671cf205..f1cec9ff 100644 --- a/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala +++ b/src/test/scala/com/sksamuel/scapegoat/inspections/VariableShadowingTest.scala @@ -150,6 +150,22 @@ class VariableShadowingTest 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 + } } } } From 5647219e2919bffed8149f445f6ed2f332ee7f68 Mon Sep 17 00:00:00 2001 From: Grzegorz Oledzki Date: Mon, 25 Nov 2019 20:14:35 +0100 Subject: [PATCH 4/6] Fixing multiple visits of the same match case --- .../sksamuel/scapegoat/inspections/VariableShadowing.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala index 68a40386..6005de11 100644 --- a/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala +++ b/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala @@ -41,13 +41,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) } } } } -} \ No newline at end of file +} From 9017278718a0398314d9927c9d19b24c46f3f629 Mon Sep 17 00:00:00 2001 From: Grzegorz Oledzki Date: Mon, 25 Nov 2019 20:23:26 +0100 Subject: [PATCH 5/6] Adding support for Blocks --- .../com/sksamuel/scapegoat/inspections/VariableShadowing.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala b/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala index 6005de11..19ffaebb 100644 --- a/src/main/scala/com/sksamuel/scapegoat/inspections/VariableShadowing.scala +++ b/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)) => From 11375812237a6d5281bdf647129dc6ab57c49ed9 Mon Sep 17 00:00:00 2001 From: Grzegorz Oledzki Date: Mon, 25 Nov 2019 20:23:47 +0100 Subject: [PATCH 6/6] Enabling VariableShadowing --- README.md | 3 ++- src/main/scala/com/sksamuel/scapegoat/ScapegoatConfig.scala | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 39f11814..0e6ca8c1 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,7 @@ For instructions on suppressing warnings by file, by inspection or by line see [ ### Inspections -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) +There are currently 118 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 | |------|-------------------|---------------| @@ -258,6 +258,7 @@ There are currently 117 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 | diff --git a/src/main/scala/com/sksamuel/scapegoat/ScapegoatConfig.scala b/src/main/scala/com/sksamuel/scapegoat/ScapegoatConfig.scala index d07e9734..ffeafa10 100644 --- a/src/main/scala/com/sksamuel/scapegoat/ScapegoatConfig.scala +++ b/src/main/scala/com/sksamuel/scapegoat/ScapegoatConfig.scala @@ -135,6 +135,7 @@ object ScapegoatConfig extends App { new UseLog10, new UseLog1P, new UseSqrt, + new VariableShadowing, new VarClosure, new VarCouldBeVal, new WhileTrue,