Skip to content

Commit c01d891

Browse files
halotukozakhalotukozak
authored
add NothingAsFunctionArgument rule to the Analyzer Plugin (#585)
* add ThrownExceptionNotInMonixScope rule to the Analyzer Plugin * change to all function parameters * add some `flatMap` cases * Nothing is a special bottom type; it is a subtype of every other type. The Scala compiler loves to infer Nothing as a generic type but that is almost always incorrect. Explicit type arguments should be used instead. * Revert "Nothing is a special bottom type; it is a subtype of every other type. The Scala compiler loves to infer Nothing as a generic type but that is almost always incorrect. Explicit type arguments should be used instead." This reverts commit 9622a3f. * handle multiple arguments, not only the first one * add more tests * add more tests * add and fix test * lil enhancement * better error message * Refactor `ThrownExceptionNotInFunctionTest` using `AnyWordSpec` and `ScalaInterpolator` for improved readability and maintainability. * Enhance `ThrownExceptionNotInFunctionTest` for clarity and accuracy in exception detection scenarios * renaming + docs * Rename `ThrowAsFunctionArgument` to `NothingAsFunctionArgument`, update related documentation and tests
1 parent 3f29426 commit c01d891

File tree

4 files changed

+309
-21
lines changed

4 files changed

+309
-21
lines changed

‎analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ final class AnalyzerPlugin(val global: Global) extends Plugin { plugin =>
5656
new Any2StringAdd(global),
5757
new ThrowableObjects(global),
5858
new DiscardedMonixTask(global),
59+
new NothingAsFunctionArgument(global),
5960
new BadSingletonComponent(global),
6061
new ConstantDeclarations(global),
6162
new BasePackage(global),

‎analyzer/src/main/scala/com/avsystem/commons/analyzer/NothingAsFunctionArgument.scala

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package com.avsystem.commons
2+
package analyzer
3+
4+
import scala.tools.nsc.Global
5+
6+
final class NothingAsFunctionArgument(g: Global) extends AnalyzerRule(g, "nothingAsFunctionArgument") {
7+
8+
import global.*
9+
10+
def analyze(unit: CompilationUnit): Unit = unit.body.foreach(analyzeTree {
11+
case Apply(f: Tree, args: List[Tree]) =>
12+
args.zip(f.tpe.params).foreach {
13+
case (arg, param) if definitions.isFunctionType(param.tpe) && arg.tpe <:< definitions.NothingTpe =>
14+
report(arg.pos,
15+
s"""
16+
|A value of type `Nothing` was passed where a function is expected.
17+
|If you intended to throw an exception, wrap it in a function literal (e.g. `_ => throw ex` instead of `throw ex`).
18+
|If you are using a mocking framework, provide a mock function with the correct type (e.g. `any[${show(param.tpe)}]`).
19+
|""".stripMargin
20+
)
21+
case (_, _) =>
22+
}
23+
})
24+
}

‎analyzer/src/test/scala/com/avsystem/commons/analyzer/NothingAsFunctionArgumentTest.scala

Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
package com.avsystem.commons
2+
package analyzer
3+
4+
import org.scalatest.wordspec.AnyWordSpec
5+
6+
final class NothingAsFunctionArgumentTest extends AnyWordSpec with AnalyzerTest {
7+
settings.pluginOptions.value ++= List("AVSystemAnalyzer:-discardedMonixTask")
8+
9+
"The ThrownExceptionNotInFunction rule" should {
10+
"detect improper usage of thrown exceptions" when {
11+
12+
"an exception is thrown directly in a simple function argument" in assertErrors(1,
13+
scala"""
14+
|def ex: Exception = ???
15+
|
16+
|def f0(x1: Int => Int): Unit = ???
17+
|
18+
|// This is incorrect: throwing an exception instead of passing a function
19+
|f0(throw ex)
20+
|
21+
|// This is correct: passing a valid function
22+
|f0(identity)
23+
|""".stripMargin
24+
)
25+
26+
Seq(
27+
("Option[_]", "map"),
28+
("Option[_]", "flatMap"),
29+
("List[_]", "map"),
30+
("Seq[_]", "map"),
31+
("Set[_]", "map"),
32+
("Map[_, _]", "map"),
33+
("scala.concurrent.Future[_]", "map"),
34+
("scala.concurrent.Future[_]", "flatMap"),
35+
("scala.util.Try[_]", "map"),
36+
("Either[_, _]", "map"),
37+
("monix.eval.Task[_]", "map"),
38+
("monix.eval.Task[_]", "flatMap"),
39+
("com.avsystem.commons.misc.Opt[_]", "map"),
40+
("String => Int", "andThen"),
41+
("String => Nothing", "andThen"),
42+
("Nothing => Nothing", "andThen"),
43+
("String => Int", "compose[Any]"),
44+
("Seq[_]", "foreach"),
45+
).foreach { case (typeName, methodName) =>
46+
val definition =
47+
//language=Scala
48+
s"""
49+
|def sth: $typeName = ???
50+
|def ex: Exception = ???
51+
|
52+
|implicit val ec: scala.concurrent.ExecutionContext = ??? // Required for Future
53+
|""".stripMargin
54+
55+
s"an exception is thrown directly in the $methodName method of $typeName" which {
56+
57+
"occurs within the method body" in assertErrors(1,
58+
scala"""
59+
|$definition
60+
|sth.$methodName(throw ex)
61+
|""".stripMargin
62+
)
63+
64+
"occurs within a code block" in assertErrors(1,
65+
scala"""
66+
|$definition
67+
|{
68+
| println(""); sth.$methodName(throw ex)
69+
|}
70+
|""".stripMargin
71+
)
72+
73+
"occurs within both branches of an if expression" in assertErrors(2,
74+
scala"""
75+
|$definition
76+
|if (true) sth.$methodName(throw ex) else sth.$methodName(throw ex)
77+
|""".stripMargin
78+
)
79+
80+
"occurs within all sections of a try-catch-finally block" in assertErrors(3,
81+
scala"""
82+
|$definition
83+
|try sth.$methodName(throw ex) catch {
84+
| case _: Exception => sth.$methodName(throw ex)
85+
|} finally sth.$methodName(throw ex)
86+
|""".stripMargin
87+
)
88+
89+
"occurs within a while loop" in assertErrors(1,
90+
scala"""
91+
|$definition
92+
|while (true) sth.$methodName(throw ex)
93+
|""".stripMargin
94+
)
95+
96+
"occurs within a do-while loop" in assertErrors(1,
97+
scala"""
98+
|$definition
99+
|do sth.$methodName(throw ex) while (true)
100+
|""".stripMargin
101+
)
102+
103+
"occurs within a function invocation" in assertErrors(1,
104+
scala"""
105+
|$definition
106+
|Seq(1, 2, 3).foreach(_ => sth.$methodName(throw ex))
107+
|""".stripMargin
108+
)
109+
110+
"does not occur when the exception is thrown inside a function literal" in assertNoErrors(
111+
scala"""
112+
|$definition
113+
|
114+
|sth.$methodName(_ => throw ex)
115+
|
116+
|{
117+
| println(""); sth.$methodName(_ => throw ex)
118+
|}
119+
|
120+
|if (true) sth.$methodName(_ => throw ex) else sth.$methodName(_ => throw ex)
121+
|
122+
|try sth.$methodName(_ => throw ex) catch {
123+
| case _: Exception => sth.$methodName(_ => throw ex)
124+
|} finally sth.$methodName(_ => throw ex)
125+
|
126+
|Seq(1, 2, 3).foreach(_ => sth.$methodName(_ => throw ex))
127+
|
128+
|while (true) sth.$methodName(_ => throw ex)
129+
|
130+
|do sth.$methodName(_ => throw ex) while (true)
131+
|""".stripMargin
132+
)
133+
}
134+
}
135+
136+
"multiple-parameter functions are tested for thrown exceptions" in {
137+
val definition =
138+
//language=Scala
139+
"""
140+
|def ex: Exception = ???
141+
|
142+
|def f1(x1: Int => Int, x2: String => String): Unit = ???
143+
|def f2(x1: Int => Int)(x2: String => String): Unit = ???
144+
|def f3(x1: Int => Int)(x2: Int)(x3: String => String): Unit = ???
145+
|def f4(x1: Int, x2: Int, x3: String => String): Unit = ???
146+
|""".stripMargin
147+
148+
assertErrors(13,
149+
scala"""
150+
|$definition
151+
|
152+
|f1(throw ex, throw ex)
153+
|f1(identity, throw ex)
154+
|f1(throw ex, identity)
155+
|
156+
|f2(throw ex)(throw ex)
157+
|f2(identity)(throw ex)
158+
|f2(throw ex)(identity)
159+
|
160+
|f3(throw ex)(42)(throw ex)
161+
|f3(throw ex)(42)(identity)
162+
|f3(identity)(42)(throw ex)
163+
|
164+
|f4(42, 42, throw ex)
165+
|""".stripMargin
166+
)
167+
168+
assertNoErrors(
169+
scala"""$definition
170+
|
171+
|f1(identity, identity)
172+
|f2(identity)(identity)
173+
|f3(identity)(42)(identity)
174+
|f4(42, 42, identity)
175+
|""".stripMargin
176+
)
177+
}
178+
179+
"methods with parameters in a class context are tested" in {
180+
val definition =
181+
//language=Scala
182+
"""
183+
|class A {
184+
| def f1(x1: Int => Int, x2: String => String): Unit = ???
185+
| def f2(x1: Int => Int)(x2: String => String): Unit = ???
186+
| def f3(x1: Int => Int)(x2: Int)(x3: String => String): Unit = ???
187+
| def f4(x1: Int, x2: Int, x3: String => String): Unit = ???
188+
|}
189+
|final val a = new A
190+
|""".stripMargin
191+
192+
assertErrors(15,
193+
scala"""
194+
|$definition
195+
|
196+
|a.f1(throw ex, throw ex)
197+
|a.f1(identity, throw ex)
198+
|a.f1(throw ex, identity)
199+
|
200+
|a.f2(throw ex)(throw ex)
201+
|a.f2(identity)(throw ex)
202+
|a.f2(throw ex)(identity)
203+
|
204+
|a.f3(throw ex)(42)(throw ex)
205+
|a.f3(throw ex)(42)(identity)
206+
|a.f3(identity)(42)(throw ex)
207+
|a.f4(42, 42, throw ex)
208+
|
209+
|""".stripMargin
210+
)
211+
212+
assertNoErrors(
213+
scala"""
214+
|$definition
215+
|
216+
|a.f1(identity, identity)
217+
|a.f2(identity)(identity)
218+
|a.f3(identity)(42)(identity)
219+
|a.f4(42, 42, identity)
220+
|""".stripMargin
221+
)
222+
}
223+
224+
"an exception is thrown directly in a class constructor parameter" in {
225+
assertErrors(9,
226+
scala"""
227+
|def ex: Exception = ???
228+
|
229+
|class A(f: Int => Int)
230+
|new A(throw ex)
231+
|
232+
|class B(f: Int => Int)(g: Int => Int)
233+
|new B(throw ex)(identity)
234+
|new B(identity)(throw ex)
235+
|new B(throw ex)(throw ex)
236+
|
237+
|class C(f: Int => Int, g: Int => Int)
238+
|new C(throw ex, identity)
239+
|new C(identity, throw ex)
240+
|new C(throw ex, throw ex)
241+
|""".stripMargin
242+
)
243+
}
244+
245+
"detects passing a function that throws an exception (Nothing)" in {
246+
assertErrors(1,
247+
scala"""
248+
|def throwEx: Nothing = ???
249+
|
250+
|def f0(x1: Int => Int): Unit = ???
251+
|
252+
|// Incorrect usage: passing a thrown exception instead of a function
253+
|f0(throwEx)
254+
|
255+
|// Correct usage: passing a valid function
256+
|f0(identity)
257+
|""".stripMargin
258+
)
259+
}
260+
}
261+
}
262+
}

0 commit comments

Comments
 (0)