enhance rules and tests for FinalCaseClasses, ImplicitValueClasses, and ImplicitParamDefaults; improve error messages and test coverage

Author: halotukozak

.idea/codeStyles/Project.xml

@@ -13,7 +13,7 @@ class ImplicitParamDefaults(g: Global) extends AnalyzerRule(g, "implicitParamDef
         if (paramList.nonEmpty && paramList.head.mods.hasFlag(Flag.IMPLICIT)) {
           paramList.foreach { param =>
             if (param.rhs != EmptyTree) {
-              report(param.pos, "Do not declare default values for implicit parameters")
+              report(param.pos, "Implicit parameters should not have default values")
             }
           }
         }

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

@@ -10,10 +10,7 @@ class ImplicitValueClasses(g: Global) extends AnalyzerRule(g, "implicitValueClas
 
   private lazy val defaultClasses = Set[Symbol](AnyClass, AnyValClass, ObjectClass)
 
-  private lazy val reportOnNestedClasses = argument match {
-    case null => false
-    case a => a.toBoolean
-  }
+  private lazy val reportOnNestedClasses = ImplicitValueClasses.Options.fromString(argument).reportOnNestedClasses
 
   def analyze(unit: CompilationUnit): Unit = unit.body.foreach {
     case cd: ClassDef if cd.mods.hasFlag(Flag.IMPLICIT) =>
@@ -51,3 +48,17 @@ class ImplicitValueClasses(g: Global) extends AnalyzerRule(g, "implicitValueClas
     case _ =>
   }
 }
+
+object ImplicitValueClasses {
+  private sealed abstract class Options(val reportOnNestedClasses: Boolean)
+  private object Options {
+    private case object All extends Options(reportOnNestedClasses = true)
+    private case object TopLevelOnly extends Options(reportOnNestedClasses = false)
+
+    def fromString(s: String): Options = s match {
+      case "all" => All
+      case "top-level-only" | null => TopLevelOnly
+      case _ => throw new IllegalArgumentException(s"Unknown ImplicitValueClasses option: $s")
+    }
+  }
+}

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

@@ -3,7 +3,7 @@ package analyzer
 
 import org.scalatest.funsuite.AnyFunSuite
 
-class CatchThrowableTest extends AnyFunSuite with AnalyzerTest {
+final class CatchThrowableTest extends AnyFunSuite with AnalyzerTest {
   test("catching Throwable should be rejected") {
     assertErrors(1,
       //language=Scala

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

@@ -3,42 +3,59 @@ package analyzer
 
 import org.scalatest.funsuite.AnyFunSuite
 
-class FinalCaseClassesTest extends AnyFunSuite with AnalyzerTest {
-  test("case classes should be marked as final") {
-    assertErrors(2,
+final class FinalCaseClassesTest extends AnyFunSuite with AnalyzerTest {
+  test("final case class should pass") {
+    assertNoErrors(
       //language=scala
       """
-        |object whatever {
-        |  // This should pass - final case class
-        |  final case class GoodCaseClass(x: Int, y: String) {
-        |    def double: Int = x * 2
-        |  }
-        |
-        |  // This should fail - case class not marked as final
-        |  case class BadCaseClass1(x: Int, y: String) {
-        |    def double: Int = x * 2
-        |  }
-        |
-        |  // This should fail - another case class not marked as final
-        |  case class BadCaseClass2[T](x: T, y: String) {
-        |    def double: String = y * 2
-        |  }
-        |
-        |  // Regular class - should not be affected
-        |  class RegularClass(val x: Int, val y: String) {
-        |    def double: Int = x * 2
-        |  }
-        |
-        |  // Regular class with case-like constructor - should not be affected
-        |  class RegularClass2(x: Int, y: String) {
-        |    def double: Int = x * 2
-        |  }
+        |final case class GoodCaseClass(x: Int, y: String) {
+        |  def double: Int = x * 2
+        |}
+      """.stripMargin)
+  }
+
+  test("case class not marked as final should fail") {
+    assertErrors(1,
+      //language=scala
+      """
+        |case class BadCaseClass1(x: Int, y: String) {
+        |  def double: Int = x * 2
+        |}
+      """.stripMargin)
+  }
+
+  test("generic case class not marked as final should fail") {
+    assertErrors(1,
+      //language=scala
+      """
+        |case class BadCaseClass2[T](x: T, y: String) {
+        |  def double: String = y * 2
+        |}
+      """.stripMargin)
+  }
+
+  test("regular class should not be affected") {
+    assertNoErrors(
+      //language=scala
+      """
+        |class RegularClass(val x: Int, val y: String) {
+        |  def double: Int = x * 2
+        |}
+      """.stripMargin)
+  }
+
+  test("regular class with case-like constructor should not be affected") {
+    assertNoErrors(
+      //language=scala
+      """
+        |class RegularClass2(x: Int, y: String) {
+        |  def double: Int = x * 2
         |}
       """.stripMargin)
   }
 
   // SI-4440 https://github.com/scala/bug/issues/4440
-  test("should not be affected due to SI-4440") {
+  test("inner case class in trait should not be affected due to SI-4440") {
     assertNoErrors(
       //language=scala
       """
@@ -47,13 +64,20 @@ class FinalCaseClassesTest extends AnyFunSuite with AnalyzerTest {
         |    def double: Int = x * 2
         |  }
         |}
-        |
+      """.stripMargin
+    )
+  }
+
+  test("inner case class in class should not be affected due to SI-4440") {
+    assertNoErrors(
+      //language=scala
+      """
         |class Outer2 {
         |  case class Inner(x: Int, y: String) {
         |    def double: Int = x * 2
         |  }
         |}
-        """.stripMargin
+      """.stripMargin
     )
   }
 
@@ -68,7 +92,7 @@ class FinalCaseClassesTest extends AnyFunSuite with AnalyzerTest {
         |  val jeden = SealedCaseClass(1)
         |  val dwa = SealedCaseClass(2)
         |}
-  """.stripMargin
+      """.stripMargin
     )
   }
-}
+}

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

@@ -3,37 +3,54 @@ package analyzer
 
 import org.scalatest.funsuite.AnyFunSuite
 
-class FinalValueClassesTest extends AnyFunSuite with AnalyzerTest {
-  test("value classes should be marked as final") {
-    assertErrors(2,
+final class FinalValueClassesTest extends AnyFunSuite with AnalyzerTest {
+  test("final value class should pass") {
+    assertNoErrors(
+      //language=scala
       """
-        |object whatever {
-        |  // This should pass - final value class
-        |  final class GoodValueClass(val x: Int) extends AnyVal {
-        |    def double: Int = x * 2
-        |  }
-        |
-        |  // This should fail - value class not marked as final
-        |  class BadValueClass1(val x: Int) extends AnyVal {
-        |    def double: Int = x * 2
-        |  }
-        |
-        |  // This should fail - another value class not marked as final
-        |  class BadValueClass2[T <: Int](val x: T) extends AnyVal {
-        |    def double: Int = x * 2
-        |  }
-        |
-        |  // Regular class extending AnyVal but not marked as final - should not be affected
-        |  // because it has multiple parameters
-        |  class RegularClass(val x: Int, val y: Int) {
-        |    def double: Int = x * 2
-        |  }
-        |
-        |  // Regular class not extending AnyVal - should not be affected
-        |  class RegularClass2(val x: Int) {
-        |    def double: Int = x * 2
-        |  }
+        |final class GoodValueClass(val x: Int) extends AnyVal {
+        |  def double: Int = x * 2
         |}
       """.stripMargin)
   }
-}
+
+  test("value class not marked as final should fail") {
+    assertErrors(1,
+      //language=scala
+      """
+        |class BadValueClass1(val x: Int) extends AnyVal {
+        |  def double: Int = x * 2
+        |}
+      """.stripMargin)
+  }
+
+  test("generic value class not marked as final should fail") {
+    assertErrors(1,
+      //language=scala
+      """
+        |class BadValueClass2[T <: Int](val x: T) extends AnyVal {
+        |  def double: Int = x * 2
+        |}
+      """.stripMargin)
+  }
+
+  test("regular class with multiple parameters should not be affected") {
+    assertNoErrors(
+      //language=scala
+      """
+        |class RegularClass(val x: Int, val y: Int) {
+        |  def double: Int = x * 2
+        |}
+      """.stripMargin)
+  }
+
+  test("regular class not extending AnyVal should not be affected") {
+    assertNoErrors(
+      //language=scala
+      """
+        |class RegularClass2(val x: Int) {
+        |  def double: Int = x * 2
+        |}
+      """.stripMargin)
+  }
+}