From 22b6e0f9c7484fd9420fa7fcfbcb69052510f7a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Kozak?= Date: Tue, 15 Apr 2025 18:52:32 +0200 Subject: [PATCH 01/14] add ImplicitValueClasses rule to enforce extending AnyVal for implicit classes --- .../commons/analyzer/AnalyzerPlugin.scala | 1 + .../analyzer/ImplicitValueClasses.scala | 24 ++++++++++++ .../analyzer/ImplicitValueClassesTest.scala | 38 +++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitValueClasses.scala create mode 100644 analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitValueClassesTest.scala 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..7e749211c 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,7 @@ final class AnalyzerPlugin(val global: Global) extends Plugin { plugin => new BadSingletonComponent(global), new ConstantDeclarations(global), new BasePackage(global), + new ImplicitValueClasses(global), ) private lazy val rulesByName = rules.map(r => (r.name, r)).toMap 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..f322a0291 --- /dev/null +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitValueClasses.scala @@ -0,0 +1,24 @@ +package com.avsystem.commons +package analyzer + +import scala.tools.nsc.Global + +class ImplicitValueClasses(g: Global) extends AnalyzerRule(g, "implicitValueClasses", 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.IMPLICIT) => + val tpe = cd.symbol.typeSignature + val primaryCtor = tpe.member(termNames.CONSTRUCTOR).alternatives.find(_.asMethod.isPrimaryConstructor) + val paramLists = primaryCtor.map(_.asMethod.paramLists) + val hasExactlyOneParam = paramLists.forall(lists => lists.size == 1 && lists.head.size == 1) + + if (!tpe.baseClasses.contains(anyValTpe.typeSymbol) && hasExactlyOneParam) { + report(cd.pos, "Implicit classes should always extend AnyVal to become value classes") + } + case _ => + } +} 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..76214bb37 --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitValueClassesTest.scala @@ -0,0 +1,38 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class ImplicitValueClassesTest extends AnyFunSuite with AnalyzerTest { + test("implicit classes should extend AnyVal") { + assertErrors(2, + """ + |object whatever { + | // This should pass - implicit class extending AnyVal + | implicit class GoodImplicitClass(val x: Int) extends AnyVal { + | def double: Int = x * 2 + | } + | + | // This should fail - implicit class not extending AnyVal + | implicit class BadImplicitClass1(val x: Int) { + | def double: Int = x * 2 + | } + | + | // This should fail - another implicit class not extending AnyVal + | implicit class BadImplicitClass2[T <: Int](val x: T) { + | def double: Int = x * 2 + | } + | + | // Regular class - should not be affected + | class RegularClass(val x: Int) { + | def double: Int = x * 2 + | } + | + | // implicit class with implicit parameter - should not be affected + | implicit class ImplicitClassWithImplicitParameter(val x: Int)(implicit dummy: DummyImplicit) { + | def double: Int = x * 2 + | } + |} + """.stripMargin) + } +} \ No newline at end of file From e3c37cc9262359492bcc0169c0d1d0aa8e68fd93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Kozak?= Date: Tue, 15 Apr 2025 19:04:59 +0200 Subject: [PATCH 02/14] add FinalValueClasses rule to enforce marking value classes as final --- .../commons/analyzer/AnalyzerPlugin.scala | 1 + .../commons/analyzer/FinalValueClasses.scala | 21 ++++++++++ .../analyzer/FinalValueClassesTest.scala | 39 +++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 analyzer/src/main/scala/com/avsystem/commons/analyzer/FinalValueClasses.scala create mode 100644 analyzer/src/test/scala/com/avsystem/commons/analyzer/FinalValueClassesTest.scala 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 7e749211c..8483df1fd 100644 --- a/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala @@ -60,6 +60,7 @@ final class AnalyzerPlugin(val global: Global) extends Plugin { plugin => new ConstantDeclarations(global), new BasePackage(global), new ImplicitValueClasses(global), + new FinalValueClasses(global), ) private lazy val rulesByName = rules.map(r => (r.name, r)).toMap 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/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 From b189364ad211b135a8cc22a882f376bb3265f72a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Kozak?= Date: Tue, 15 Apr 2025 19:16:08 +0200 Subject: [PATCH 03/14] add ImplicitParamDefaults rule to prevent default values for implicit parameters --- .../commons/analyzer/AnalyzerPlugin.scala | 1 + .../analyzer/ImplicitParamDefaults.scala | 23 +++++++++ .../analyzer/ImplicitParamDefaultsTest.scala | 49 +++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 analyzer/src/main/scala/com/avsystem/commons/analyzer/ImplicitParamDefaults.scala create mode 100644 analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitParamDefaultsTest.scala 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 8483df1fd..4e6614d25 100644 --- a/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala +++ b/analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala @@ -61,6 +61,7 @@ final class AnalyzerPlugin(val global: Global) extends Plugin { plugin => new BasePackage(global), new ImplicitValueClasses(global), new FinalValueClasses(global), + new ImplicitParamDefaults(global), ) private lazy val rulesByName = rules.map(r => (r.name, r)).toMap 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/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..9449219d9 --- /dev/null +++ b/analyzer/src/test/scala/com/avsystem/commons/analyzer/ImplicitParamDefaultsTest.scala @@ -0,0 +1,49 @@ +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, + """ + |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 From 7edce9a9ed0bbd5275c80942b92127609ef4ac17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Kozak?= Date: Thu, 17 Apr 2025 13:48:28 +0200 Subject: [PATCH 04/14] ImplicitValueClasses rule: divide test into several ones, add Universal Trait and inheriting classes support. If class is nested, only info. Add nested classes support --- .idea/codeStyles/Project.xml | 18 -- .../commons/analyzer/AnalyzerRule.scala | 3 +- .../analyzer/ImplicitValueClasses.scala | 32 ++- .../commons/analyzer/ImplicitTypesTest.scala | 2 +- .../analyzer/ImplicitValueClassesTest.scala | 211 ++++++++++++++++-- 5 files changed, 226 insertions(+), 40 deletions(-) diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index cb14e62c6..095539431 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -1,23 +1,5 @@ - - - -