diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala index eacfa6ef5..e02902508 100644 --- a/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala @@ -59,6 +59,12 @@ final class AnalyzerPlugin(val global: Global) extends Plugin { plugin => new BadSingletonComponent(global), new ConstantDeclarations(global), new BasePackage(global), + new ImplicitValueClasses(global), + new FinalValueClasses(global), + new FinalCaseClasses(global), + new ImplicitParamDefaults(global), + new CatchThrowable(global), + new ImplicitFunctionParams(global), ) private lazy val rulesByName = rules.map(r => (r.name, r)).toMap diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala index d5b42f0f0..33a1b0a09 100644 --- a/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala @@ -32,7 +32,8 @@ abstract class AnalyzerRule(val global: Global, val name: String, defaultLevel: pos: Position, message: String, category: WarningCategory = WarningCategory.Lint, - site: Symbol = NoSymbol + site: Symbol = NoSymbol, + level: Level = this.level ): Unit = level match { case Level.Off => diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/CatchThrowable.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/CatchThrowable.scala new file mode 100644 index 000000000..a61e3b24e --- /dev/null +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/CatchThrowable.scala @@ -0,0 +1,31 @@ +package com.avsystem.commons +package analyzer + +import scala.tools.nsc.Global + +class CatchThrowable(g: Global) extends AnalyzerRule(g, "catchThrowable", Level.Warn) { + + import global.* + + private lazy val throwableTpe = typeOf[Throwable] + + private def isCustomExtractor(tree: Tree): Boolean = tree match { + case UnApply(Apply(Select(_, TermName("unapply")), _), _) => true + case _ => false + } + + private def checkTree(pat: Tree): Unit = if (pat.tpe != null && pat.tpe =:= throwableTpe && !isCustomExtractor(pat)) { + report(pat.pos, "Catching Throwable is discouraged, catch specific exceptions instead") + } + + def analyze(unit: CompilationUnit): Unit = + unit.body.foreach { + case t: Try => + t.catches.foreach { + case CaseDef(Alternative(trees), _, _) => trees.foreach(checkTree) + case CaseDef(Bind(_, Alternative(trees)), _, _) => trees.foreach(checkTree) + case CaseDef(pat, _, _) => checkTree(pat) + } + case _ => + } +} diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/FinalCaseClasses.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/FinalCaseClasses.scala new file mode 100644 index 000000000..3b5361461 --- /dev/null +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/FinalCaseClasses.scala @@ -0,0 +1,24 @@ +package com.avsystem.commons +package analyzer + +import com.avsystem.commons.analyzer.Level.Info + +import scala.tools.nsc.Global + +class FinalCaseClasses(g: Global) extends AnalyzerRule(g, "finalCaseClasses", Level.Warn) { + + import global.* + + def analyze(unit: CompilationUnit): Unit = unit.body.foreach { + case cd: ClassDef if !cd.mods.hasFlag(Flag.FINAL | Flag.SEALED) && cd.mods.hasFlag(Flag.CASE) => + // Skip case classes defined inside traits (SI-4440) + val isInner = cd.symbol.isStatic + + if (isInner) { + report(cd.pos, "Case classes should be marked as final") + } else { + report(cd.pos, "Case classes should be marked as final. Due to the SI-4440 bug, it cannot be done here. Consider moving the case class to the companion object", level = Info) + } + case _ => + } +} diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/FinalValueClasses.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/FinalValueClasses.scala new file mode 100644 index 000000000..d6054e242 --- /dev/null +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/FinalValueClasses.scala @@ -0,0 +1,21 @@ +package com.avsystem.commons +package analyzer + +import scala.tools.nsc.Global + +class FinalValueClasses(g: Global) extends AnalyzerRule(g, "finalValueClasses", Level.Warn) { + + import global.* + + private lazy val anyValTpe = typeOf[AnyVal] + + def analyze(unit: CompilationUnit): Unit = unit.body.foreach { + case cd: ClassDef if !cd.mods.hasFlag(Flag.FINAL) => + val tpe = cd.symbol.typeSignature + + if (tpe.baseClasses.contains(anyValTpe.typeSymbol) ) { + report(cd.pos, "Value classes should be marked as final") + } + case _ => + } +} diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitFunctionParams.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitFunctionParams.scala new file mode 100644 index 000000000..073954653 --- /dev/null +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitFunctionParams.scala @@ -0,0 +1,24 @@ +package com.avsystem.commons +package analyzer + +import scala.tools.nsc.Global + +class ImplicitFunctionParams(g: Global) extends AnalyzerRule(g, "implicitFunctionParams", Level.Warn) { + + import global.* + + def analyze(unit: CompilationUnit): Unit = unit.body.foreach { + case dd: DefDef => + dd.vparamss.foreach { paramList => + if (paramList.nonEmpty && paramList.head.mods.hasFlag(Flag.IMPLICIT)) { + paramList.foreach { param => + val paramTpe = param.tpt.tpe + if (paramTpe != null && (definitions.isFunctionType(paramTpe) || definitions.isPartialFunctionType(paramTpe))) { + report(param.pos, "Implicit parameters should not have any function type") + } + } + } + } + case _ => + } +} diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitParamDefaults.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitParamDefaults.scala new file mode 100644 index 000000000..3a04cb933 --- /dev/null +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitParamDefaults.scala @@ -0,0 +1,23 @@ +package com.avsystem.commons +package analyzer + +import scala.tools.nsc.Global + +class ImplicitParamDefaults(g: Global) extends AnalyzerRule(g, "implicitParamDefaults", Level.Warn) { + + import global.* + + def analyze(unit: CompilationUnit): Unit = unit.body.foreach { + case dd: DefDef => + dd.vparamss.foreach { paramList => + 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") + } + } + } + } + case _ => + } +} \ No newline at end of file diff --git a/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitValueClasses.scala b/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitValueClasses.scala new file mode 100644 index 000000000..5e61e192f --- /dev/null +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitValueClasses.scala @@ -0,0 +1,53 @@ +package com.avsystem.commons +package analyzer + +import scala.tools.nsc.Global + +class ImplicitValueClasses(g: Global) extends AnalyzerRule(g, "implicitValueClasses", Level.Warn) { + + import global.* + import definitions.* + + private lazy val defaultClasses = Set[Symbol](AnyClass, AnyValClass, ObjectClass) + + private lazy val reportOnNestedClasses = argument match { + case null => false + case a => a.toBoolean + } + + def analyze(unit: CompilationUnit): Unit = unit.body.foreach { + case cd: ClassDef if cd.mods.hasFlag(Flag.IMPLICIT) => + val tpe = cd.symbol.typeSignature + val primaryCtor = tpe.member(termNames.CONSTRUCTOR).alternatives.find(_.asMethod.isPrimaryConstructor) + val paramLists = primaryCtor.map(_.asMethod.paramLists) + val inheritsAnyVal = tpe.baseClasses contains AnyValClass + + def inheritsOtherClass = tpe.baseClasses.exists { cls => + def isDefault = defaultClasses contains cls + def isUniversalTrait = cls.isTrait && cls.superClass == AnyClass + + cls != cd.symbol && !isDefault && !isUniversalTrait + } + def hasExactlyOneParam = paramLists.exists(lists => lists.size == 1 && lists.head.size == 1) + + def paramIsValueClass = paramLists.exists { lists => + /* lists.nonEmpty && lists.head.nonEmpty && */ + lists.head.head.typeSignature.typeSymbol.isDerivedValueClass + } + + if (!inheritsAnyVal && !inheritsOtherClass && hasExactlyOneParam && !paramIsValueClass) { + val isNestedClass = + //implicit classes are always nested classes, so we want to check if the outer class's an object + /*cd.symbol.isNestedClass &&*/ !cd.symbol.isStatic + + val message = "Implicit classes should always extend AnyVal to become value classes" + + (if (isNestedClass) ". Nested classes should be extracted to top-level objects" else "") + + if (reportOnNestedClasses || !isNestedClass) + report(cd.pos, message) + else + report(cd.pos, message, level = Level.Info) + } + case _ => + } +} diff --git a/analyzer/src/test/scala/com/avsystem/commons/analyzer/CatchThrowableTest.scala b/analyzer/src/test/scala/com/avsystem/commons/analyzer/CatchThrowableTest.scala new file mode 100644 index 000000000..c17b4d69a --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/CatchThrowableTest.scala @@ -0,0 +1,136 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class CatchThrowableTest extends AnyFunSuite with AnalyzerTest { + test("catching Throwable should be rejected") { + assertErrors(1, + //language=Scala + """ + |object Test { + | def test(): Unit = { + | try { + | println("test") + | } catch { + | case t: Throwable => println(t) + | } + | } + |} + """.stripMargin) + } + + test("catching specific exceptions should be allowed") { + assertNoErrors( + //language=Scala + """ + |object Test { + | def test(): Unit = { + | 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, + //language=Scala + """ + |object Test { + | def test(): Unit = { + | 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, + //language=Scala + """ + |object Test { + | def test(): Unit = { + | try println("test") + | catch { + | case e: Exception => try println("test") + | catch { + | case e: Throwable => println(e) + | } + | } + | } + |} + """.stripMargin) + } + + test("catching Throwable using custom extractor should be allowed") { + assertNoErrors( + //language=Scala + """ + |object Test extends com.avsystem.commons.CommonAliases { + | object custom { + | def unapply(t: Throwable): Option[IllegalArgumentException] = t match { + | case e: IllegalArgumentException => Some(e) + | 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) + | } + | } + |} + """.stripMargin) + } + + test("catching non-Throwable with pattern match should be allowed") { + assertNoErrors( + //language=Scala + """ + |object Test { + | def test(): Unit = { + | try { + | println("test") + | } catch { + | case _: IndexOutOfBoundsException | _: NullPointerException => println("OK!") + | } + | try { + | println("test") + | } catch { + | case e@(_: IndexOutOfBoundsException | _: NullPointerException) => println("OK!") + | } + | } + |} + """.stripMargin) + } + + test("catching Throwable with pattern match should be rejected") { + assertErrors(1, + //language=Scala + """ + |object Test { + | def test(): Unit = { + | try { + | println("test") + | } catch { + | case _: IndexOutOfBoundsException | _: Throwable => println("Not OK!") + | } + | } + |} + """.stripMargin) + } +} diff --git a/analyzer/src/test/scala/com/avsystem/commons/analyzer/FinalCaseClassesTest.scala b/analyzer/src/test/scala/com/avsystem/commons/analyzer/FinalCaseClassesTest.scala new file mode 100644 index 000000000..a48f4fd33 --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/FinalCaseClassesTest.scala @@ -0,0 +1,74 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class FinalCaseClassesTest extends AnyFunSuite with AnalyzerTest { + test("case classes should be marked as final") { + assertErrors(2, + //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 + | } + |} + """.stripMargin) + } + + // SI-4440 https://github.com/scala/bug/issues/4440 + test("should not be affected due to SI-4440") { + assertNoErrors( + //language=scala + """ + |trait Outer { + | case class Inner(x: Int, y: String) { + | def double: Int = x * 2 + | } + |} + | + |class Outer2 { + | case class Inner(x: Int, y: String) { + | def double: Int = x * 2 + | } + |} + """.stripMargin + ) + } + + test("sealed case class should not be affected") { + assertNoErrors( + //language=scala + """ + |sealed case class SealedCaseClass(x: Int) { + | def double: Int = x * 2 + |} + |object SealedCaseClass { + | val jeden = SealedCaseClass(1) + | val dwa = SealedCaseClass(2) + |} + """.stripMargin + ) + } +} \ No newline at end of file diff --git a/analyzer/src/test/scala/com/avsystem/commons/analyzer/FinalValueClassesTest.scala b/analyzer/src/test/scala/com/avsystem/commons/analyzer/FinalValueClassesTest.scala new file mode 100644 index 000000000..47cbdfb4b --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/FinalValueClassesTest.scala @@ -0,0 +1,39 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class FinalValueClassesTest extends AnyFunSuite with AnalyzerTest { + test("value classes should be marked as final") { + assertErrors(2, + """ + |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 + | } + |} + """.stripMargin) + } +} \ No newline at end of file diff --git a/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitFunctionParamsTest.scala b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitFunctionParamsTest.scala new file mode 100644 index 000000000..8d47cdc93 --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitFunctionParamsTest.scala @@ -0,0 +1,137 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class ImplicitFunctionParamsTest extends AnyFunSuite with AnalyzerTest { + test("implicit parameters should not be function types or partial functions") { + assertErrors(8, + //language=Scala + """ + |object whatever { + | // This should pass - regular parameter with function type + | def goodMethod1(f: Int => String): Unit = ??? + | + | // This should pass - regular parameter with partial function type + | def goodMethod2(pf: PartialFunction[Int, String]): Unit = ??? + | + | // This should pass - implicit parameter with non-function type + | def goodMethod3(implicit s: String): Unit = ??? + | + | // This should fail - implicit parameter with function type + | def badMethod1(implicit f: Int => String): Unit = ??? + | + | // This should fail - implicit parameter with function type in second parameter list + | def badMethod2(x: Int)(implicit f: Int => String): Unit = ??? + | + | // This should fail - implicit parameter with partial function type + | def badMethod3(implicit pf: PartialFunction[Int, String]): Unit = ??? + | + | // This should fail - implicit parameter with partial function type in second parameter list + | def badMethod4(x: Int)(implicit pf: PartialFunction[Int, String]): Unit = ??? + | + | // This should pass - regular class parameter with function type + | class GoodClass1(f: Int => String) + | + | // This should pass - regular class parameter with partial function type + | class GoodClass2(pf: PartialFunction[Int, String]) + | + | // This should pass - implicit class parameter with non-function type + | class GoodClass3(implicit s: String) + | + | // This should fail - implicit class parameter with function type + | class BadClass1(implicit f: Int => String) + | + | // This should fail - implicit class parameter with function type in second parameter list + | class BadClass2(x: Int)(implicit f: Int => String) + | + | // This should fail - implicit class parameter with partial function type + | class BadClass3(implicit pf: PartialFunction[Int, String]) + | + | // This should fail - implicit class parameter with partial function type in second parameter list + | class BadClass4(x: Int)(implicit pf: PartialFunction[Int, String]) + |} + """.stripMargin) + } + + test("implicit parameters with SAM types should pass") { + assertCompiles( + //language=Scala + """ + |object whatever { + | // Define a SAM type (Single Abstract Method) + | trait IntToString { + | def apply(i: Int): String + | } + | + | // This should pass - regular parameter with SAM type + | def goodMethod1(f: IntToString): Unit = ??? + | + | // This should pass - implicit parameter with SAM type + | def goodMethod2(implicit f: IntToString): Unit = ??? + | + | // This should pass - implicit parameter with SAM type in second parameter list + | def goodMethod3(x: Int)(implicit f: IntToString): Unit = ??? + | + | // This should pass - regular class parameter with SAM type + | class GoodClass1(f: IntToString) + | + | // This should pass - implicit class parameter with SAM type + | class GoodClass2(implicit f: IntToString) + | + | // This should pass - implicit class parameter with SAM type in second parameter list + | class GoodClass3(x: Int)(implicit f: IntToString) + |} + """.stripMargin) + } + + test("implicit parameters should not be function types with multiple parameters") { + assertErrors(8, + //language=Scala + """ + |object whatever { + | // This should pass - regular parameter with Function2 type + | def goodMethod1(f: (Int, String) => Boolean): Unit = ??? + | + | // This should pass - regular parameter with Function3 type + | def goodMethod2(f: (Int, String, Double) => Boolean): Unit = ??? + | + | // This should pass - implicit parameter with non-function type + | def goodMethod3(implicit s: String): Unit = ??? + | + | // This should fail - implicit parameter with Function2 type + | def badMethod1(implicit f: (Int, String) => Boolean): Unit = ??? + | + | // This should fail - implicit parameter with Function2 type in second parameter list + | def badMethod2(x: Int)(implicit f: (Int, String) => Boolean): Unit = ??? + | + | // This should fail - implicit parameter with Function3 type + | def badMethod3(implicit f: (Int, String, Double) => Boolean): Unit = ??? + | + | // This should fail - implicit parameter with Function3 type in second parameter list + | def badMethod4(x: Int)(implicit f: (Int, String, Double) => Boolean): Unit = ??? + | + | // This should pass - regular class parameter with Function2 type + | class GoodClass1(f: (Int, String) => Boolean) + | + | // This should pass - regular class parameter with Function3 type + | class GoodClass2(f: (Int, String, Double) => Boolean) + | + | // This should pass - implicit class parameter with non-function type + | class GoodClass3(implicit s: String) + | + | // This should fail - implicit class parameter with Function2 type + | class BadClass1(implicit f: (Int, String) => Boolean) + | + | // This should fail - implicit class parameter with Function2 type in second parameter list + | class BadClass2(x: Int)(implicit f: (Int, String) => Boolean) + | + | // This should fail - implicit class parameter with Function3 type + | class BadClass3(implicit f: (Int, String, Double) => Boolean) + | + | // This should fail - implicit class parameter with Function3 type in second parameter list + | class BadClass4(x: Int)(implicit f: (Int, String, Double) => Boolean) + |} + """.stripMargin) + } +} diff --git a/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitParamDefaultsTest.scala b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitParamDefaultsTest.scala new file mode 100644 index 000000000..05a153a17 --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitParamDefaultsTest.scala @@ -0,0 +1,50 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class ImplicitParamDefaultsTest extends AnyFunSuite with AnalyzerTest { + test("implicit parameters should not have default values") { + assertErrors(6, + //language=Scala + """ + |object whatever { + | // This should pass - implicit parameter without default value + | def goodMethod1(implicit s: Scheduler): Unit = ??? + | + | // This should pass - regular parameter with default value + | def goodMethod2(s: Scheduler = Scheduler.global): Unit = ??? + | + | // This should fail - implicit parameter with default value + | def badMethod1(implicit s: Scheduler = Scheduler.global): Unit = ??? + | + | // This should fail - implicit parameter with default value + | def badMethod(sth: Int)(implicit s: Scheduler = Scheduler.global): Unit = ??? + | + | // This should fail - another implicit parameter with default value + | def badMethod2[T](x: T)(implicit s: Scheduler = Scheduler.global): T = ??? + | + | // This should pass - implicit parameter without default value + | class GoodClass1(implicit s: Scheduler) + | + | // This should pass - regular parameter with default value + | class GoodClass2(s: Scheduler = Scheduler.global) + | + | // This should fail - implicit parameter with default value + | class BadClass1(implicit s: Scheduler = Scheduler.global) + | + | // This should fail - implicit parameter with default value + | class BadClass2(sth: Int)(implicit s: Scheduler = Scheduler.global) + | + | // This should fail - another implicit parameter with default value + | class BadClass3[T](x: T)(implicit s: Scheduler = Scheduler.global) + | + | // Dummy classes for the test + | class Scheduler + | object Scheduler { + | val global = new Scheduler + | } + |} + """.stripMargin) + } +} \ No newline at end of file diff --git a/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitTypesTest.scala b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitTypesTest.scala index f7d07a83f..d17c8601d 100644 --- a/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitTypesTest.scala +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitTypesTest.scala @@ -13,7 +13,7 @@ class ImplicitTypesTest extends AnyFunSuite with AnalyzerTest { | implicit def conv(x: Int) = x.toString | implicit def conv2(x: Int): String = x.toString | implicit object objekt - | implicit class wtf(x: Int) + | implicit final class wtf(val x: Int) extends AnyVal |} """.stripMargin) } diff --git a/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitValueClassesTest.scala b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitValueClassesTest.scala new file mode 100644 index 000000000..3001128c0 --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitValueClassesTest.scala @@ -0,0 +1,230 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class ImplicitValueClassesSuite extends AnyFunSuite with AnalyzerTest { + test("implicit final class extending AnyVal should pass") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | implicit final class GoodImplicitClass(val x: Int) extends AnyVal { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } + + test("implicit class not extending AnyVal should fail") { + assertErrors(1, + //language=Scala + """ + |object whatever { + | implicit final class BadImplicitClass(val x: Int) { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } + + test("implicit class with type parameter not extending AnyVal should fail") { + assertErrors(1, + //language=Scala + """ + |object whatever { + | implicit final class BadImplicitClass[T <: Int](val x: T) { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } + + test("regular class should not be affected") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | class RegularClass(val x: Int) { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } + + test("implicit class with implicit parameter should not be affected") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | implicit final class ImplicitClassWithImplicitParameter(val x: Int)(implicit dummy: DummyImplicit) { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } + + test("implicit class extending other classes should not be affected") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | class SomeClass + | + | implicit final class GoodImplicitClass1(val x: Int) extends SomeClass { + | def double: Int = x * 2 + | } + | + | trait SomeTrait + | implicit final class GoodImplicitClass2(val x: Int) extends SomeTrait { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } + + test("implicit class extending AnyVal with traits should be handled correctly") { + //language=Scala + assertErrors(1, + """ + |object whatever { + | trait AnyTrait extends Any + | implicit final class GoodImplicitClass(val x: Int) extends AnyVal with AnyTrait { + | def double: Int = x * 2 + | } + | implicit final class BadImplicitClass(val x: Int) extends AnyTrait { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } + + test("nested implicit class not extending AnyVal should pass") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | class Outer { + | implicit final class NestedImplicitClass(val x: Int) { + | def double: Int = x * 2 + | } + | } + |} + """.stripMargin) + } + + test("implicit class for value class should not be affected") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | implicit final class ValueClass(x: com.avsystem.commons.misc.Timestamp) { + | def sth: Long = x.millis + | } + |} + """.stripMargin) + } +} + +class NestedImplicitValueClassesSuite extends AnyFunSuite with AnalyzerTest { + settings.pluginOptions.value ++= List("AVSystemAnalyzer:+implicitValueClasses:true") + + test("nested implicit class not extending AnyVal should fail") { + assertErrors(1, + //language=Scala + """ + |object whatever { + | class Outer { + | implicit final class GoodNestedImplicitClass(val x: Int) { + | def double: Int = x * 2 + | } + | } + |} + """.stripMargin) + } + + + test("nested implicit class with type parameter not extending AnyVal should fail") { + assertErrors(1, + """ + | //language=Scala + | + |object whatever { + | class Outer { + | implicit final class BadNestedImplicitClass[T <: Int](val x: T) { + | def double: Int = x * 2 + | } + | } + |} + """.stripMargin) + } + + test("deeply nested implicit class not extending AnyVal should fail") { + assertErrors(1, + //language=Scala + """ + |object whatever { + | class Outer { + | class Inner { + | implicit final class BadDeeplyNestedImplicitClass(val x: Int) { + | def double: Int = x * 2 + | } + | } + | } + |} + """.stripMargin) + } + + test("regular class should not be affected") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | class Outer { + | class RegularClass(val x: Int) { + | def double: Int = x * 2 + | } + | } + |} + """.stripMargin) + } + + test("implicit class extending other classes should not be affected") { + assertNoErrors( + //language=Scala + """ + |object whatever { + | class Outer { + | class SomeClass + | + | implicit final class GoodImplicitClass1(val x: Int) extends SomeClass { + | def double: Int = x * 2 + | } + | + | trait SomeTrait + | implicit final class GoodImplicitClass2(val x: Int) extends SomeTrait { + | def double: Int = x * 2 + | } + | } + |} + """.stripMargin) + } + + test("implicit class extending AnyVal with traits should be handled correctly") { + assertErrors(1, + //language=Scala + """ + |object whatever { + | class Outer { + | trait AnyTrait extends Any + | implicit final class GoodImplicitClass(val x: Int) extends AnyVal with AnyTrait { + | def double: Int = x * 2 + | } + | implicit final class BadImplicitClass(val x: Int) extends AnyTrait { + | def double: Int = x * 2 + | } + | } + |} + """.stripMargin) + } +} diff --git a/docs/Analyzer.md b/docs/Analyzer.md index 5ef4d58fc..0ea099bd1 100644 --- a/docs/Analyzer.md +++ b/docs/Analyzer.md @@ -31,6 +31,13 @@ Here's a list of currently supported rules: | `throwableObjects` | warning | Makes sure that objects are never used as `Throwable`s (unless they have stack traces disabled) | | `constantDeclarations` | off | Checks if constants are always declared as `final val`s with `UpperCamelCase` and no explicit type annotation for literal values | | `basePackage` | warning | Checks if all sources are within the specified base package | +| `catchThrowable` | warning | Makes sure that code does not catch `Throwable` directly, which can hide critical errors like `OutOfMemoryError` | +| `finalValueClasses` | warning | Makes sure that value classes are marked final. | +| `finalCaseClasses` | warning | Makes sure that case classes are marked final. Does not affect inner classes. | +| `implicitParamDefaults` | warning | Makes sure that default values are not defined for implicit parameters | +| `implicitValueClasses` | warning | Makes sure that implicit classes extend `AnyVal` (when applicable). Nested classes check can be enabled by passing the `true` argument. | +| `implicitFunctionParams` | warning | Makes sure that implicit parameters are not function types or partial functions | + Rules may be enabled and disabled in `build.sbt` with Scala compiler options: