Skip to content

Commit 7edce9a

Browse files
halotukozakhalotukozak
committed
ImplicitValueClasses rule: divide test into several ones, add Universal Trait and inheriting classes support. If class is nested, only info. Add nested classes support
1 parent b189364 commit 7edce9a

File tree

5 files changed

+226
-40
lines changed

5 files changed

+226
-40
lines changed

‎.idea/codeStyles/Project.xml‎

Lines changed: 0 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ abstract class AnalyzerRule(val global: Global, val name: String, defaultLevel:
3232
pos: Position,
3333
message: String,
3434
category: WarningCategory = WarningCategory.Lint,
35-
site: Symbol = NoSymbol
35+
site: Symbol = NoSymbol,
36+
level: Level = this.level
3637
): Unit =
3738
level match {
3839
case Level.Off =>

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,42 @@ import scala.tools.nsc.Global
66
class ImplicitValueClasses(g: Global) extends AnalyzerRule(g, "implicitValueClasses", Level.Warn) {
77

88
import global.*
9+
import definitions.*
910

10-
private lazy val anyValTpe = typeOf[AnyVal]
11+
private lazy val defaultClasses = Set[Symbol](AnyClass, AnyValClass, ObjectClass)
12+
13+
private lazy val reportOnNestedClasses = argument match {
14+
case null => false
15+
case a => a.toBoolean
16+
}
17+
18+
private lazy val message = "Implicit classes should always extend AnyVal to become value classes" +
19+
(if (reportOnNestedClasses) ". Nested classes should be extracted to top-level objects" else "")
1120

1221
def analyze(unit: CompilationUnit): Unit = unit.body.foreach {
1322
case cd: ClassDef if cd.mods.hasFlag(Flag.IMPLICIT) =>
1423
val tpe = cd.symbol.typeSignature
1524
val primaryCtor = tpe.member(termNames.CONSTRUCTOR).alternatives.find(_.asMethod.isPrimaryConstructor)
1625
val paramLists = primaryCtor.map(_.asMethod.paramLists)
17-
val hasExactlyOneParam = paramLists.forall(lists => lists.size == 1 && lists.head.size == 1)
26+
val inheritsAnyVal = tpe.baseClasses contains AnyValClass
27+
28+
def inheritsOtherClass = tpe.baseClasses.exists { cls =>
29+
def isDefault = defaultClasses contains cls
30+
def isUniversalTrait = cls.isTrait && cls.superClass == AnyClass
31+
32+
cls != cd.symbol && !isDefault && !isUniversalTrait
33+
}
34+
def hasExactlyOneParam = paramLists.forall(lists => lists.size == 1 && lists.head.size == 1)
35+
36+
if (!inheritsAnyVal && !inheritsOtherClass && hasExactlyOneParam) {
37+
def isNestedClass =
38+
//implicit classes are always nested classes, so we want to check if the outer class's an object
39+
/*cd.symbol.isNestedClass &&*/ !cd.symbol.isStatic
1840

19-
if (!tpe.baseClasses.contains(anyValTpe.typeSymbol) && hasExactlyOneParam) {
20-
report(cd.pos, "Implicit classes should always extend AnyVal to become value classes")
41+
if (reportOnNestedClasses || !isNestedClass)
42+
report(cd.pos, message)
43+
else
44+
report(cd.pos, message, level = Level.Info)
2145
}
2246
case _ =>
2347
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class ImplicitTypesTest extends AnyFunSuite with AnalyzerTest {
1313
| implicit def conv(x: Int) = x.toString
1414
| implicit def conv2(x: Int): String = x.toString
1515
| implicit object objekt
16-
| implicit class wtf(x: Int)
16+
| implicit final class wtf(val x: Int) extends AnyVal
1717
|}
1818
""".stripMargin)
1919
}

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

Lines changed: 195 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,217 @@
11
package com.avsystem.commons
22
package analyzer
33

4+
import org.scalatest.Suites
45
import org.scalatest.funsuite.AnyFunSuite
56

6-
class ImplicitValueClassesTest extends AnyFunSuite with AnalyzerTest {
7-
test("implicit classes should extend AnyVal") {
8-
assertErrors(2,
7+
class DefaultImplicitValueClassesSuite extends AnyFunSuite with AnalyzerTest {
8+
test("implicit final class extending AnyVal should pass") {
9+
assertNoErrors(
10+
//language=Scala
911
"""
1012
|object whatever {
11-
| // This should pass - implicit class extending AnyVal
12-
| implicit class GoodImplicitClass(val x: Int) extends AnyVal {
13+
| implicit final class GoodImplicitClass(val x: Int) extends AnyVal {
1314
| def double: Int = x * 2
1415
| }
15-
|
16-
| // This should fail - implicit class not extending AnyVal
17-
| implicit class BadImplicitClass1(val x: Int) {
16+
|}
17+
""".stripMargin)
18+
}
19+
20+
test("implicit class not extending AnyVal should fail") {
21+
assertErrors(1,
22+
//language=Scala
23+
"""
24+
|object whatever {
25+
| implicit final class BadImplicitClass(val x: Int) {
1826
| def double: Int = x * 2
1927
| }
20-
|
21-
| // This should fail - another implicit class not extending AnyVal
22-
| implicit class BadImplicitClass2[T <: Int](val x: T) {
28+
|}
29+
""".stripMargin)
30+
}
31+
32+
test("implicit class with type parameter not extending AnyVal should fail") {
33+
assertErrors(1,
34+
//language=Scala
35+
"""
36+
|object whatever {
37+
| implicit final class BadImplicitClass[T <: Int](val x: T) {
2338
| def double: Int = x * 2
2439
| }
25-
|
26-
| // Regular class - should not be affected
40+
|}
41+
""".stripMargin)
42+
}
43+
44+
test("regular class should not be affected") {
45+
assertNoErrors(
46+
//language=Scala
47+
"""
48+
|object whatever {
2749
| class RegularClass(val x: Int) {
2850
| def double: Int = x * 2
2951
| }
52+
|}
53+
""".stripMargin)
54+
}
55+
56+
test("implicit class with implicit parameter should not be affected") {
57+
assertNoErrors(
58+
//language=Scala
59+
"""
60+
|object whatever {
61+
| implicit final class ImplicitClassWithImplicitParameter(val x: Int)(implicit dummy: DummyImplicit) {
62+
| def double: Int = x * 2
63+
| }
64+
|}
65+
""".stripMargin)
66+
}
67+
68+
test("implicit class extending other classes should not be affected") {
69+
assertNoErrors(
70+
//language=Scala
71+
"""
72+
|object whatever {
73+
| class SomeClass
74+
|
75+
| implicit final class GoodImplicitClass1(val x: Int) extends SomeClass {
76+
| def double: Int = x * 2
77+
| }
3078
|
31-
| // implicit class with implicit parameter - should not be affected
32-
| implicit class ImplicitClassWithImplicitParameter(val x: Int)(implicit dummy: DummyImplicit) {
79+
| trait SomeTrait
80+
| implicit final class GoodImplicitClass2(val x: Int) extends SomeTrait {
3381
| def double: Int = x * 2
3482
| }
3583
|}
3684
""".stripMargin)
3785
}
38-
}
86+
87+
test("implicit class extending AnyVal with traits should be handled correctly") {
88+
//language=Scala
89+
assertErrors(1,
90+
"""
91+
|object whatever {
92+
| trait AnyTrait extends Any
93+
| implicit final class GoodImplicitClass(val x: Int) extends AnyVal with AnyTrait {
94+
| def double: Int = x * 2
95+
| }
96+
| implicit final class BadImplicitClass(val x: Int) extends AnyTrait {
97+
| def double: Int = x * 2
98+
| }
99+
|}
100+
""".stripMargin)
101+
}
102+
103+
test("nested implicit class not extending AnyVal should pass") {
104+
assertNoErrors(
105+
//language=Scala
106+
"""
107+
|object whatever {
108+
| class Outer {
109+
| implicit final class NestedImplicitClass(val x: Int) {
110+
| def double: Int = x * 2
111+
| }
112+
| }
113+
|}
114+
""".stripMargin)
115+
}
116+
}
117+
118+
class NestedImplicitValueClassesSuite extends AnyFunSuite with AnalyzerTest {
119+
settings.pluginOptions.value ++= List("AVSystemAnalyzer:+implicitValueClasses:true")
120+
121+
test("nested implicit class not extending AnyVal should fail") {
122+
assertErrors(1,
123+
"""
124+
|object whatever {
125+
| class Outer {
126+
| implicit final class GoodNestedImplicitClass(val x: Int) {
127+
| def double: Int = x * 2
128+
| }
129+
| }
130+
|}
131+
""".stripMargin)
132+
}
133+
134+
135+
test("nested implicit class with type parameter not extending AnyVal should fail") {
136+
assertErrors(1,
137+
"""
138+
|object whatever {
139+
| class Outer {
140+
| implicit final class BadNestedImplicitClass[T <: Int](val x: T) {
141+
| def double: Int = x * 2
142+
| }
143+
| }
144+
|}
145+
""".stripMargin)
146+
}
147+
148+
test("deeply nested implicit class not extending AnyVal should fail") {
149+
assertErrors(1,
150+
"""
151+
|object whatever {
152+
| class Outer {
153+
| class Inner {
154+
| implicit final class BadDeeplyNestedImplicitClass(val x: Int) {
155+
| def double: Int = x * 2
156+
| }
157+
| }
158+
| }
159+
|}
160+
""".stripMargin)
161+
}
162+
163+
test("regular class should not be affected") {
164+
assertNoErrors(
165+
"""
166+
|object whatever {
167+
| class Outer {
168+
| class RegularClass(val x: Int) {
169+
| def double: Int = x * 2
170+
| }
171+
| }
172+
|}
173+
""".stripMargin)
174+
}
175+
176+
test("implicit class extending other classes should not be affected") {
177+
assertNoErrors(
178+
"""
179+
|object whatever {
180+
| class Outer {
181+
| class SomeClass
182+
|
183+
| implicit final class GoodImplicitClass1(val x: Int) extends SomeClass {
184+
| def double: Int = x * 2
185+
| }
186+
|
187+
| trait SomeTrait
188+
| implicit final class GoodImplicitClass2(val x: Int) extends SomeTrait {
189+
| def double: Int = x * 2
190+
| }
191+
| }
192+
|}
193+
""".stripMargin)
194+
}
195+
196+
test("implicit class extending AnyVal with traits should be handled correctly") {
197+
assertErrors(1,
198+
"""
199+
|object whatever {
200+
| class Outer {
201+
| trait AnyTrait extends Any
202+
| implicit final class GoodImplicitClass(val x: Int) extends AnyVal with AnyTrait {
203+
| def double: Int = x * 2
204+
| }
205+
| implicit final class BadImplicitClass(val x: Int) extends AnyTrait {
206+
| def double: Int = x * 2
207+
| }
208+
| }
209+
|}
210+
""".stripMargin)
211+
}
212+
}
213+
214+
class ImplicitValueClassesTest extends Suites(
215+
new DefaultImplicitValueClassesSuite,
216+
new NestedImplicitValueClassesSuite
217+
)

0 commit comments

Comments
 (0)