Skip to content

add NothingAsFunctionArgument rule to the Analyzer Plugin #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d8f04ff
add ThrownExceptionNotInMonixScope rule to the Analyzer Plugin
halotukozak Jun 12, 2024
8e4fa2e
change to all function parameters
halotukozak Jun 14, 2024
8aeed88
add some `flatMap` cases
halotukozak Jun 18, 2024
9622a3f
Nothing is a special bottom type; it is a subtype of every other type…
halotukozak Jun 19, 2024
4b554f2
Revert "Nothing is a special bottom type; it is a subtype of every ot…
halotukozak Jun 19, 2024
4f836b1
handle multiple arguments, not only the first one
halotukozak Jun 19, 2024
e259d63
add more tests
halotukozak Jun 19, 2024
ae2b07a
add more tests
halotukozak Jun 19, 2024
da13377
add and fix test
halotukozak Jun 24, 2024
0c3b54c
lil enhancement
halotukozak Jun 24, 2024
0a59f61
Merge branch 'refs/heads/master' into 500-handle-thrown-exception-out…
halotukozak Jul 19, 2024
3fea6b1
Merge branch 'master' into 500-handle-thrown-exception-outside-the-mo…
halotukozak Jun 20, 2025
4a7b996
better error message
halotukozak Jun 20, 2025
9f38ad9
Merge remote-tracking branch 'origin/master' into 500-handle-thrown-e…
halotukozak Jun 25, 2025
a515f6f
Refactor `ThrownExceptionNotInFunctionTest` using `AnyWordSpec` and `…
halotukozak Jun 25, 2025
64c9db9
Enhance `ThrownExceptionNotInFunctionTest` for clarity and accuracy i…
halotukozak Jun 25, 2025
370e4fd
renaming + docs
halotukozak Jun 25, 2025
6222088
Rename `ThrowAsFunctionArgument` to `NothingAsFunctionArgument`, upda…
halotukozak Jun 25, 2025
f9e48dd
Merge remote-tracking branch 'origin/master' into 500-handle-thrown-e…
halotukozak Jun 26, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ final class AnalyzerPlugin(val global: Global) extends Plugin { plugin =>
new Any2StringAdd(global),
new ThrowableObjects(global),
new DiscardedMonixTask(global),
new NothingAsFunctionArgument(global),
new BadSingletonComponent(global),
new ConstantDeclarations(global),
new BasePackage(global),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.avsystem.commons
package analyzer

import scala.tools.nsc.Global

final class NothingAsFunctionArgument(g: Global) extends AnalyzerRule(g, "nothingAsFunctionArgument") {

import global.*

def analyze(unit: CompilationUnit): Unit = unit.body.foreach(analyzeTree {
case Apply(f: Tree, args: List[Tree]) =>
args.zip(f.tpe.params).foreach {
case (arg, param) if definitions.isFunctionType(param.tpe) && arg.tpe <:< definitions.NothingTpe =>
report(arg.pos,
s"""
|A value of type `Nothing` was passed where a function is expected.
|If you intended to throw an exception, wrap it in a function literal (e.g. `_ => throw ex` instead of `throw ex`).
|If you are using a mocking framework, provide a mock function with the correct type (e.g. `any[${show(param.tpe)}]`).
|""".stripMargin
)
case (_, _) =>
}
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
package com.avsystem.commons
package analyzer

import org.scalatest.wordspec.AnyWordSpec

final class NothingAsFunctionArgumentTest extends AnyWordSpec with AnalyzerTest {
settings.pluginOptions.value ++= List("AVSystemAnalyzer:-discardedMonixTask")

"The ThrownExceptionNotInFunction rule" should {
"detect improper usage of thrown exceptions" when {

"an exception is thrown directly in a simple function argument" in assertErrors(1,
scala"""
|def ex: Exception = ???
|
|def f0(x1: Int => Int): Unit = ???
|
|// This is incorrect: throwing an exception instead of passing a function
|f0(throw ex)
|
|// This is correct: passing a valid function
|f0(identity)
|""".stripMargin
)

Seq(
("Option[_]", "map"),
("Option[_]", "flatMap"),
("List[_]", "map"),
("Seq[_]", "map"),
("Set[_]", "map"),
("Map[_, _]", "map"),
("scala.concurrent.Future[_]", "map"),
("scala.concurrent.Future[_]", "flatMap"),
("scala.util.Try[_]", "map"),
("Either[_, _]", "map"),
("monix.eval.Task[_]", "map"),
("monix.eval.Task[_]", "flatMap"),
("com.avsystem.commons.misc.Opt[_]", "map"),
("String => Int", "andThen"),
("String => Nothing", "andThen"),
("Nothing => Nothing", "andThen"),
("String => Int", "compose[Any]"),
("Seq[_]", "foreach"),
).foreach { case (typeName, methodName) =>
val definition =
//language=Scala
s"""
|def sth: $typeName = ???
|def ex: Exception = ???
|
|implicit val ec: scala.concurrent.ExecutionContext = ??? // Required for Future
|""".stripMargin

s"an exception is thrown directly in the $methodName method of $typeName" which {

"occurs within the method body" in assertErrors(1,
scala"""
|$definition
|sth.$methodName(throw ex)
|""".stripMargin
)

"occurs within a code block" in assertErrors(1,
scala"""
|$definition
|{
| println(""); sth.$methodName(throw ex)
|}
|""".stripMargin
)

"occurs within both branches of an if expression" in assertErrors(2,
scala"""
|$definition
|if (true) sth.$methodName(throw ex) else sth.$methodName(throw ex)
|""".stripMargin
)

"occurs within all sections of a try-catch-finally block" in assertErrors(3,
scala"""
|$definition
|try sth.$methodName(throw ex) catch {
| case _: Exception => sth.$methodName(throw ex)
|} finally sth.$methodName(throw ex)
|""".stripMargin
)

"occurs within a while loop" in assertErrors(1,
scala"""
|$definition
|while (true) sth.$methodName(throw ex)
|""".stripMargin
)

"occurs within a do-while loop" in assertErrors(1,
scala"""
|$definition
|do sth.$methodName(throw ex) while (true)
|""".stripMargin
)

"occurs within a function invocation" in assertErrors(1,
scala"""
|$definition
|Seq(1, 2, 3).foreach(_ => sth.$methodName(throw ex))
|""".stripMargin
)

"does not occur when the exception is thrown inside a function literal" in assertNoErrors(
scala"""
|$definition
|
|sth.$methodName(_ => throw ex)
|
|{
| println(""); sth.$methodName(_ => throw ex)
|}
|
|if (true) sth.$methodName(_ => throw ex) else sth.$methodName(_ => throw ex)
|
|try sth.$methodName(_ => throw ex) catch {
| case _: Exception => sth.$methodName(_ => throw ex)
|} finally sth.$methodName(_ => throw ex)
|
|Seq(1, 2, 3).foreach(_ => sth.$methodName(_ => throw ex))
|
|while (true) sth.$methodName(_ => throw ex)
|
|do sth.$methodName(_ => throw ex) while (true)
|""".stripMargin
)
}
}

"multiple-parameter functions are tested for thrown exceptions" in {
val definition =
//language=Scala
"""
|def ex: Exception = ???
|
|def f1(x1: Int => Int, x2: String => String): Unit = ???
|def f2(x1: Int => Int)(x2: String => String): Unit = ???
|def f3(x1: Int => Int)(x2: Int)(x3: String => String): Unit = ???
|def f4(x1: Int, x2: Int, x3: String => String): Unit = ???
|""".stripMargin

assertErrors(13,
scala"""
|$definition
|
|f1(throw ex, throw ex)
|f1(identity, throw ex)
|f1(throw ex, identity)
|
|f2(throw ex)(throw ex)
|f2(identity)(throw ex)
|f2(throw ex)(identity)
|
|f3(throw ex)(42)(throw ex)
|f3(throw ex)(42)(identity)
|f3(identity)(42)(throw ex)
|
|f4(42, 42, throw ex)
|""".stripMargin
)

assertNoErrors(
scala"""$definition
|
|f1(identity, identity)
|f2(identity)(identity)
|f3(identity)(42)(identity)
|f4(42, 42, identity)
|""".stripMargin
)
}

"methods with parameters in a class context are tested" in {
val definition =
//language=Scala
"""
|class A {
| def f1(x1: Int => Int, x2: String => String): Unit = ???
| def f2(x1: Int => Int)(x2: String => String): Unit = ???
| def f3(x1: Int => Int)(x2: Int)(x3: String => String): Unit = ???
| def f4(x1: Int, x2: Int, x3: String => String): Unit = ???
|}
|final val a = new A
|""".stripMargin

assertErrors(15,
scala"""
|$definition
|
|a.f1(throw ex, throw ex)
|a.f1(identity, throw ex)
|a.f1(throw ex, identity)
|
|a.f2(throw ex)(throw ex)
|a.f2(identity)(throw ex)
|a.f2(throw ex)(identity)
|
|a.f3(throw ex)(42)(throw ex)
|a.f3(throw ex)(42)(identity)
|a.f3(identity)(42)(throw ex)
|a.f4(42, 42, throw ex)
|
|""".stripMargin
)

assertNoErrors(
scala"""
|$definition
|
|a.f1(identity, identity)
|a.f2(identity)(identity)
|a.f3(identity)(42)(identity)
|a.f4(42, 42, identity)
|""".stripMargin
)
}

"an exception is thrown directly in a class constructor parameter" in {
assertErrors(9,
scala"""
|def ex: Exception = ???
|
|class A(f: Int => Int)
|new A(throw ex)
|
|class B(f: Int => Int)(g: Int => Int)
|new B(throw ex)(identity)
|new B(identity)(throw ex)
|new B(throw ex)(throw ex)
|
|class C(f: Int => Int, g: Int => Int)
|new C(throw ex, identity)
|new C(identity, throw ex)
|new C(throw ex, throw ex)
|""".stripMargin
)
}

"detects passing a function that throws an exception (Nothing)" in {
assertErrors(1,
scala"""
|def throwEx: Nothing = ???
|
|def f0(x1: Int => Int): Unit = ???
|
|// Incorrect usage: passing a thrown exception instead of a function
|f0(throwEx)
|
|// Correct usage: passing a valid function
|f0(identity)
|""".stripMargin
)
}
}
}
}
Loading