Skip to content

Commit d861b8c

Browse files
committed
enhance CatchThrowable rule to reject catching Throwable with pattern matching and add corresponding tests
1 parent f37a935 commit d861b8c

File tree

2 files changed

+41
-4
lines changed

2 files changed

+41
-4
lines changed

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

+9-4
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,15 @@ class CatchThrowable(g: Global) extends AnalyzerRule(g, "catchThrowable", Level.
2525
def analyze(unit: CompilationUnit): Unit =
2626
unit.body.foreach {
2727
case t: Try =>
28-
t.catches.foreach { case cas@CaseDef(pat, _, _) =>
29-
if (pat.tpe != null && pat.tpe =:= throwableTpe && !isNonFatalPattern(pat)) {
30-
report(cas.pos, "Catching Throwable is discouraged, catch specific exceptions instead")
31-
}
28+
t.catches.foreach {
29+
case cas@CaseDef(Alternative(trees), _, _) =>
30+
if (trees.exists(_.tpe =:= throwableTpe)) {
31+
report(cas.pos, "Catching Throwable is discouraged, catch specific exceptions instead")
32+
}
33+
case cas@CaseDef(pat, _, _) =>
34+
if (pat.tpe != null && pat.tpe =:= throwableTpe && !isNonFatalPattern(pat)) {
35+
report(cas.pos, "Catching Throwable is discouraged, catch specific exceptions instead")
36+
}
3237
}
3338
case _ =>
3439
}

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

+32
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,36 @@ class CatchThrowableTest extends AnyFunSuite with AnalyzerTest {
8989
|}
9090
""".stripMargin)
9191
}
92+
93+
test("catching non-Throwable with pattern match should be allowed") {
94+
assertNoErrors(
95+
//language=Scala
96+
"""
97+
|object Test {
98+
| def test(): Unit = {
99+
| try {
100+
| println("test")
101+
| } catch {
102+
| case _: IndexOutOfBoundsException | _: NullPointerException => println("OK!")
103+
| }
104+
| }
105+
|}
106+
""".stripMargin)
107+
}
108+
109+
test("catching Throwable with pattern match should be rejected") {
110+
assertErrors(1,
111+
//language=Scala
112+
"""
113+
|object Test {
114+
| def test(): Unit = {
115+
| try {
116+
| println("test")
117+
| } catch {
118+
| case _: IndexOutOfBoundsException | _: Throwable => println("Not OK!")
119+
| }
120+
| }
121+
|}
122+
""".stripMargin)
123+
}
92124
}

0 commit comments

Comments
 (0)