Adding inspection for immediate rethrows (#652)

* Adding inspection for immediate rethrows * Inspections now available when throwables/exceptions are immediately rethrown in a catch block implementation. * Adding inspection * Updating README.md
scapegoat-scala · May 22, 2022 · a578ad6 · a578ad6
1 parent 4a0e2ac
commit a578ad6
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 1 deletion.
diff --git a/README.md b/README.md
@@ -149,7 +149,7 @@ For instructions on suppressing warnings by file, by inspection or by line see [
 
 ### Inspections
 
-There are currently 119 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 120 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 |
 |---------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|
@@ -167,6 +167,7 @@ There are currently 119 inspections. An overview list is given, followed by a mo
 | BoundedByFinalType                    | Looks for types with upper bounds of a final type                                                                                                            | Warning       |
 | BrokenOddness                         | checks for a % 2 == 1 for oddness because this fails on negative numbers                                                                                     | Warning       |
 | CatchException                        | Checks for try blocks that catch Exception                                                                                                                   | Warning       |
+| CatchExceptionImmediatelyRethrown     | Checks for try-catch blocks that immediately rethrow caught exceptions.                                                                                      | Warning       |
 | CatchFatal                            | Checks for try blocks that catch fatal exceptions: VirtualMachineError, ThreadDeath, InterruptedException, LinkageError, ControlThrowable                    | Warning       |
 | CatchNpe                              | Checks for try blocks that catch null pointer exceptions                                                                                                     | Error         |
 | CatchThrowable                        | Checks for try blocks that catch Throwable                                                                                                                   | Warning       |

diff --git a/src/main/scala/com/sksamuel/scapegoat/Inspections.scala b/src/main/scala/com/sksamuel/scapegoat/Inspections.scala
@@ -40,6 +40,7 @@ object Inspections extends App {
       new BoundedByFinalType,
       new BrokenOddness,
       new CatchException,
+      new CatchExceptionImmediatelyRethrown,
       new CatchFatal,
       new CatchNpe,
       new CatchThrowable,

diff --git a/...cala/com/sksamuel/scapegoat/inspections/exception/CatchExceptionImmediatelyRethrown.scala b/...cala/com/sksamuel/scapegoat/inspections/exception/CatchExceptionImmediatelyRethrown.scala
@@ -0,0 +1,45 @@
+package com.sksamuel.scapegoat.inspections.exception
+
+import com.sksamuel.scapegoat._
+
+class CatchExceptionImmediatelyRethrown
+    extends Inspection(
+      text = "Caught Exception Immediately Rethrown",
+      defaultLevel = Levels.Warning,
+      description = "Checks for try-catch blocks that immediately rethrow caught exceptions.",
+      explanation = "Immediately re-throwing a caught exception is equivalent to not catching it at all."
+    ) {
+
+  def inspector(context: InspectionContext): Inspector =
+    new Inspector(context) {
+
+      override def postTyperTraverser: context.Traverser =
+        new context.Traverser {
+          import context.global._
+
+          private def exceptionHandlers(cases: List[CaseDef]): List[(String, Tree)] = {
+            cases.collect {
+              // matches t : Exception
+              case CaseDef(Bind(name, Typed(_, tpt)), _, body) if tpt.tpe =:= typeOf[Exception] =>
+                (name.toString(), body)
+              // matches t : Throwable
+              case CaseDef(Bind(name, Typed(_, tpt)), _, body) if tpt.tpe =:= typeOf[Throwable] =>
+                (name.toString(), body)
+            }
+          }
+
+          override def inspect(tree: Tree): Unit = {
+            tree match {
+              case Try(_, catches, _) =>
+                val exceptionBodies = exceptionHandlers(catches)
+                exceptionBodies.collect {
+                  case (exceptionName, Throw(Ident(name))) if name.toString() == exceptionName =>
+                    context.warn(tree.pos, self)
+                }
+              case _ =>
+                continue(tree)
+            }
+          }
+        }
+    }
+}
diff --git a/.../com/sksamuel/scapegoat/inspections/exception/CatchExceptionImmediatelyRethrownTest.scala b/.../com/sksamuel/scapegoat/inspections/exception/CatchExceptionImmediatelyRethrownTest.scala
@@ -0,0 +1,62 @@
+package com.sksamuel.scapegoat.inspections.exception
+
+import com.sksamuel.scapegoat.InspectionTest
+
+class CatchExceptionImmediatelyRethrownTest extends InspectionTest {
+
+  override val inspections = Seq(new CatchExceptionImmediatelyRethrown)
+
+  "catch exception immediately throw" - {
+    "should report warning" in {
+
+      val testCode = """object Test {
+                        try {
+                          val x = 1
+                        } catch {
+                          case foo : Exception =>
+                            throw foo
+                        }
+                    } """.stripMargin
+
+      compileCodeSnippet(testCode)
+      compiler.scapegoat.feedback.warnings.size shouldBe 1
+    }
+  }
+
+  "catch throwable immediately throw" - {
+    "should report warning" in {
+
+      val testCode = """object Test {
+                        try {
+                          val x = 1
+                        } catch {
+                          case foo : Throwable =>
+                            throw foo
+                        }
+                    } """.stripMargin
+
+      compileCodeSnippet(testCode)
+      compiler.scapegoat.feedback.warnings.size shouldBe 1
+    }
+
+  }
+
+  "catch throwable and exception immediately throw" - {
+    "should report all warnings" in {
+
+      val testCode = """object Test {
+                        try {
+                          val x = 1
+                        } catch {
+                          case foo : Throwable =>
+                            throw foo
+                          case bar : Exception =>
+                            throw bar
+                        }
+                    } """.stripMargin
+
+      compileCodeSnippet(testCode)
+      compiler.scapegoat.feedback.warnings.size shouldBe 2
+    }
+  }
+}