Merge pull request #721 from AVSystem/720-not-catch-custom-handlers

improve handling of custom handler CaseDefs in CatchThrowable

Author: halotukozak

analyzer/src/main/scala/com/avsystem/commons/analyzer/CatchThrowable.scala

@@ -24,7 +24,9 @@ class CatchThrowable(g: Global) extends AnalyzerRule(g, "catchThrowable", Level.
         t.catches.foreach {
           case CaseDef(Alternative(trees), _, _) => trees.foreach(checkTree)
           case CaseDef(Bind(_, Alternative(trees)), _, _) => trees.foreach(checkTree)
-          case CaseDef(pat, _, _) => checkTree(pat)
+          // CaseDef generated from a custom handler has NoPosition
+          case cd@CaseDef(pat, _, _) if cd.pos != NoPosition => checkTree(pat)
+          case _ =>
         }
       case _ =>
     }

analyzer/src/test/scala/com/avsystem/commons/analyzer/CatchThrowableTest.scala

@@ -7,53 +7,47 @@ final class CatchThrowableTest extends AnyFunSuite with AnalyzerTest {
   test("catching Throwable should be rejected") {
     assertErrors(1,
       scala"""
-             |def test(): Unit = 
-             |  try {
-             |    println("test")
-             |  } catch {
-             |    case t: Throwable => println(t)
-             |  }
+             |try {
+             |  println("test")
+             |} catch {
+             |  case t: Throwable => println(t)
+             |}
              |""".stripMargin)
   }
 
   test("catching specific exceptions should be allowed") {
     assertNoErrors(
       scala"""
-             |def test(): Unit = 
-             |  try {
-             |    println("test")
-             |  } catch {
-             |    case e: Exception => println(e)
-             |    case e: RuntimeException => println(e)
-             |    case e: IllegalArgumentException => println(e)
-             |  }        
+             |try {
+             |  println("test")
+             |} catch {
+             |  case e: Exception => println(e)
+             |  case e: RuntimeException => println(e)
+             |  case e: IllegalArgumentException => println(e)
+             |}
              |""".stripMargin)
   }
 
   test("catching Throwable with other exceptions should be rejected") {
     assertErrors(1,
       scala"""
-             |def test(): Unit = {
-             |  try {
-             |    println("test")
-             |  } catch {
-             |    case e: IllegalArgumentException => println(e)
-             |    case t: Throwable => println(t)
-             |  }
+             |try {
+             |  println("test")
+             |} catch {
+             |  case e: IllegalArgumentException => println(e)
+             |  case t: Throwable => println(t)
              |}
              |""".stripMargin)
   }
 
   test("catching Throwable in nested catch block should be rejected") {
     assertErrors(1,
       scala"""
-             |def test(): Unit = {
-             |  try println("test")
+             |try println("test")
+             |catch {
+             |  case e: Exception => try println("test")
              |  catch {
-             |    case e: Exception => try println("test")
-             |    catch {
-             |      case e: Throwable => println(e)
-             |    }
+             |    case e: Throwable => println(e)
              |  }
              |}
              |""".stripMargin)
@@ -70,46 +64,55 @@ final class CatchThrowableTest extends AnyFunSuite with AnalyzerTest {
              |    case _ => None
              |  }
              |}
-             |def test(): Unit = {
-             |  try {
-             |    println("test")
-             |  } catch {
-             |    case custom(t) => println(t)
-             |    case NonFatal(t) => println(t)
-             |    case scala.util.control.NonFatal(t) => println(t)
-             |  }
+             |
+             |try {
+             |  println("test")
+             |} catch {
+             |  case custom(t) => println(t)
+             |  case NonFatal(t) => println(t)
+             |  case scala.util.control.NonFatal(t) => println(t)
              |}
              |""".stripMargin)
   }
 
   test("catching non-Throwable with pattern match should be allowed") {
     assertNoErrors(
       scala"""
-             |def test(): Unit = {
-             |  try {
-             |    println("test")
-             |  } catch {
-             |    case _: IndexOutOfBoundsException | _: NullPointerException => println("OK!")
-             |  }
-             |  try {
-             |    println("test")
-             |  } catch {
-             |    case e@(_: IndexOutOfBoundsException | _: NullPointerException) => println("OK!")
-             |  }
+             |try {
+             |  println("test")
+             |} catch {
+             |  case _: IndexOutOfBoundsException | _: NullPointerException => println("OK!")
              |}
-             |""".stripMargin)
+             |try {
+             |  println("test")
+             |} catch {
+             |  case e@(_: IndexOutOfBoundsException | _: NullPointerException) => println("OK!")
+             |}
+             |""".stripMargin
+    )
   }
 
   test("catching Throwable with pattern match should be rejected") {
     assertErrors(1,
       scala"""
-             |def test(): Unit = {
-             |  try {
-             |    println("test")
-             |  } catch {
-             |    case _: IndexOutOfBoundsException | _: Throwable => println("Not OK!")
-             |  }
+             |try {
+             |  println("test")
+             |} catch {
+             |  case _: IndexOutOfBoundsException | _: Throwable => println("Not OK!")
              |}
              |""".stripMargin)
   }
+
+  test("catching Throwable using custom handler should be allowed") {
+    assertNoErrors(
+      scala"""
+             |object CustomHandler {
+             |  def apply[T](): PartialFunction[Throwable, T] = ???
+             |}
+             |
+             |try {
+             |  println("test")
+             |} catch CustomHandler()
+             |""".stripMargin)
+  }
 }