Skip to content

Commit

Permalink
Fix macro crash when handling sealed case classes (#634)
Browse files Browse the repository at this point in the history
Fixes #628

Sealed classes can be instantiated directly. Therefore, when generating
the typeTag, I included the sealed class itself. This ensures that the
macro correctly handles sealed classes without subclasses. To check
whether it’s a sealed class and not a trait or abstract class, I used
the following condition:
 ```
 sealedParents.find(_ == tpe.typeSymbol)
```
Since `trait` and `abstract class` cannot be instantiated, I believe this approach works. However, I could also explicitly check that the symbol is not a trait or an abstract class, if needed.

What do you think?
  • Loading branch information
nox213 authored Sep 28, 2024
1 parent c7a77d9 commit 004ed7e
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,14 @@ object Macros {
fail(tpe, _),
)


val sealedClassSymbol: Option[Symbol] = sealedParents.find(_ == tpe.typeSymbol)
val segments =
sealedParents
sealedClassSymbol.toList.map(_.fullName.split('.')) ++
sealedParents
.flatMap(_.asClass.knownDirectSubclasses)
.map(_.fullName.split('.'))


// -1 because even if there is only one subclass, and so no name segments
// are needed to differentiate between them, we want to keep at least
// the rightmost name segment
Expand Down
7 changes: 5 additions & 2 deletions upickle/implicits/src-3/upickle/implicits/macros.scala
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,15 @@ def tagNameImpl0[T](transform: String => String)(using Quotes, Type[T]): Expr[St
inline def shortTagName[T]: String = ${ shortTagNameImpl[T] }
def shortTagNameImpl[T](using Quotes, Type[T]): Expr[String] =
import quotes.reflect._
val sym = TypeTree.of[T].symbol
val sealedClassSymbol = if (TypeRepr.of[T].baseClasses.contains(TypeRepr.of[T].typeSymbol))
Some(TypeRepr.of[T].typeSymbol.fullName.split('.'))
else None
val segments = TypeRepr.of[T].baseClasses
.filter(_.flags.is(Flags.Sealed))
.flatMap(_.children)
.filter(_.flags.is(Flags.Case))
.map(_.fullName.split('.'))
.map(_.fullName.split('.')) ++
sealedClassSymbol.toList

val identicalSegmentCount = Range(0, segments.map(_.length).max - 1)
.takeWhile(i => segments.map(_.lift(i)).distinct.size == 1)
Expand Down
9 changes: 9 additions & 0 deletions upickle/test/src/upickle/MacroTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import upickle.default.{read, write, ReadWriter => RW}

case class Trivial(a: Int = 1)

sealed case class SealedClass(i: Int, s: String)
object SealedClass {
implicit val rw: RW[SealedClass] = upickle.default.macroRW
}

case class KeyedPerson(
@upickle.implicits.key("first_name") firstName: String = "N/A",
@upickle.implicits.key("last_name") lastName: String)
Expand Down Expand Up @@ -872,5 +877,9 @@ object MacroTests extends TestSuite {
)

}

test("sealedClass"){
assert(write(SealedClass(3, "Hello")) == """{"$type":"SealedClass","i":3,"s":"Hello"}""")
}
}
}

0 comments on commit 004ed7e

Please sign in to comment.