Skip to content

Commit ced2478

Browse files
committed
Fix #1140 by throwing a compilation error in case of 'AnyVal' and one value classes with 'CodecMakerConfig.withInlineOneValueClasses(true)' are used as leaf classes of sum types
1 parent dda9ff1 commit ced2478

File tree

3 files changed

+131
-116
lines changed

3 files changed

+131
-116
lines changed

Diff for: jsoniter-scala-macros/shared/src/main/scala-2/com/github/plokhotnyuk/jsoniter_scala/macros/JsonCodecMaker.scala

+38-35
Original file line numberDiff line numberDiff line change
@@ -618,41 +618,6 @@ object JsonCodecMaker {
618618
case _ => false
619619
}
620620

621-
def adtLeafClasses(adtBaseTpe: Type): List[Type] = {
622-
def collectRecursively(tpe: Type): List[Type] = {
623-
val tpeClass = tpe.typeSymbol.asClass
624-
val leafTpes = tpeClass.knownDirectSubclasses.toList.flatMap { s =>
625-
val classSymbol = s.asClass
626-
val typeParams = classSymbol.typeParams
627-
val subTpe =
628-
if (typeParams.isEmpty) classSymbol.toType
629-
else {
630-
val typeParamsAndArgs = tpeClass.typeParams.map(_.toString).zip(tpe.typeArgs).toMap
631-
val typeArgs = typeParams.map(s => typeParamsAndArgs.getOrElse(s.toString, fail {
632-
s"Cannot resolve generic type(s) for `${classSymbol.toType}`. Please provide a custom implicitly accessible codec for it."
633-
}))
634-
classSymbol.toType.substituteTypes(typeParams, typeArgs)
635-
}
636-
if (isSealedClass(subTpe)) collectRecursively(subTpe)
637-
else if (isNonAbstractScalaClass(subTpe)) subTpe :: Nil
638-
else fail(if (s.isAbstract) {
639-
"Only sealed intermediate traits or abstract classes are supported. Please consider using of them " +
640-
s"for ADT with base '$adtBaseTpe' or provide a custom implicitly accessible codec for the ADT base."
641-
} else {
642-
"Only Scala classes & objects are supported for ADT leaf classes. Please consider using of them " +
643-
s"for ADT with base '$adtBaseTpe' or provide a custom implicitly accessible codec for the ADT base."
644-
})
645-
}
646-
if (isNonAbstractScalaClass(tpe)) leafTpes :+ tpe
647-
else leafTpes
648-
}
649-
650-
val classes = collectRecursively(adtBaseTpe).distinct
651-
if (classes.isEmpty) fail(s"Cannot find leaf classes for ADT base '$adtBaseTpe'. " +
652-
"Please add them or provide a custom implicitly accessible codec for the ADT base.")
653-
classes
654-
}
655-
656621
def companion(tpe: Type): Symbol = {
657622
val comp = tpe.typeSymbol.companion
658623
if (comp.isModule) comp
@@ -881,6 +846,44 @@ object JsonCodecMaker {
881846
(cfg.inlineOneValueClasses && isNonAbstractScalaClass(tpe) && !isCollection(tpe) && getClassInfo(tpe).fields.size == 1 ||
882847
tpe.typeSymbol.isClass && tpe.typeSymbol.asClass.isDerivedValueClass)
883848

849+
def adtLeafClasses(adtBaseTpe: Type): List[Type] = {
850+
def collectRecursively(tpe: Type): List[Type] = {
851+
val tpeClass = tpe.typeSymbol.asClass
852+
val leafTpes = tpeClass.knownDirectSubclasses.toList.flatMap { s =>
853+
val classSymbol = s.asClass
854+
val typeParams = classSymbol.typeParams
855+
val subTpe =
856+
if (typeParams.isEmpty) classSymbol.toType
857+
else {
858+
val typeParamsAndArgs = tpeClass.typeParams.map(_.toString).zip(tpe.typeArgs).toMap
859+
val typeArgs = typeParams.map(s => typeParamsAndArgs.getOrElse(s.toString, fail {
860+
s"Cannot resolve generic type(s) for `${classSymbol.toType}`. Please provide a custom implicitly accessible codec for it."
861+
}))
862+
classSymbol.toType.substituteTypes(typeParams, typeArgs)
863+
}
864+
if (isSealedClass(subTpe)) collectRecursively(subTpe)
865+
else if (isValueClass(subTpe)) {
866+
fail("'AnyVal' and one value classes with 'CodecMakerConfig.withInlineOneValueClasses(true)' are not " +
867+
s"supported as leaf classes for ADT with base '$adtBaseTpe'.")
868+
} else if (isNonAbstractScalaClass(subTpe)) subTpe :: Nil
869+
else fail(if (s.isAbstract) {
870+
"Only sealed intermediate traits or abstract classes are supported. Please consider using of them " +
871+
s"for ADT with base '$adtBaseTpe' or provide a custom implicitly accessible codec for the ADT base."
872+
} else {
873+
"Only Scala classes & objects are supported for ADT leaf classes. Please consider using of them " +
874+
s"for ADT with base '$adtBaseTpe' or provide a custom implicitly accessible codec for the ADT base."
875+
})
876+
}
877+
if (isNonAbstractScalaClass(tpe)) leafTpes :+ tpe
878+
else leafTpes
879+
}
880+
881+
val classes = collectRecursively(adtBaseTpe).distinct
882+
if (classes.isEmpty) fail(s"Cannot find leaf classes for ADT base '$adtBaseTpe'. " +
883+
"Please add them or provide a custom implicitly accessible codec for the ADT base.")
884+
classes
885+
}
886+
884887
def genReadKey(types: List[Type]): Tree = {
885888
val tpe = types.head
886889
val implKeyCodec = findImplicitKeyCodec(types)

Diff for: jsoniter-scala-macros/shared/src/main/scala-3/com/github/plokhotnyuk/jsoniter_scala/macros/JsonCodecMaker.scala

+84-81
Original file line numberDiff line numberDiff line change
@@ -755,87 +755,6 @@ object JsonCodecMaker {
755755
def isEnumOrModuleValue(tpe: TypeRepr): Boolean = tpe.isSingleton &&
756756
(tpe.typeSymbol.flags.is(Flags.Module) || tpe.termSymbol.flags.is(Flags.Enum))
757757

758-
def adtChildren(tpe: TypeRepr): Seq[TypeRepr] = { // TODO: explore yet one variant with mirrors
759-
def resolveParentTypeArg(child: Symbol, fromNudeChildTarg: TypeRepr, parentTarg: TypeRepr,
760-
binding: Map[String, TypeRepr]): Map[String, TypeRepr] =
761-
if (fromNudeChildTarg.typeSymbol.isTypeParam) { // TODO: check for paramRef instead ?
762-
val paramName = fromNudeChildTarg.typeSymbol.name
763-
binding.get(paramName) match
764-
case None => binding.updated(paramName, parentTarg)
765-
case Some(oldBinding) =>
766-
if (oldBinding =:= parentTarg) binding
767-
else fail(s"Type parameter $paramName in class ${child.name} appeared in the constructor of " +
768-
s"${tpe.show} two times differently, with ${oldBinding.show} and ${parentTarg.show}")
769-
} else if (fromNudeChildTarg <:< parentTarg) binding // TODO: assure parentTag is covariant, get covariance from type parameters
770-
else {
771-
(fromNudeChildTarg, parentTarg) match
772-
case (AppliedType(ctycon, ctargs), AppliedType(ptycon, ptargs)) =>
773-
ctargs.zip(ptargs).foldLeft(resolveParentTypeArg(child, ctycon, ptycon, binding)) { (b, e) =>
774-
resolveParentTypeArg(child, e._1, e._2, b)
775-
}
776-
case _ => fail(s"Failed unification of type parameters of ${tpe.show} from child $child - " +
777-
s"${fromNudeChildTarg.show} and ${parentTarg.show}")
778-
}
779-
780-
def resolveParentTypeArgs(child: Symbol, nudeChildParentTags: List[TypeRepr], parentTags: List[TypeRepr],
781-
binding: Map[String, TypeRepr]): Map[String, TypeRepr] =
782-
nudeChildParentTags.zip(parentTags).foldLeft(binding)((s, e) => resolveParentTypeArg(child, e._1, e._2, s))
783-
784-
tpe.typeSymbol.children.map { sym =>
785-
if (sym.isType) {
786-
if (sym.name == "<local child>") // problem - we have no other way to find this other return the name
787-
fail(s"Local child symbols are not supported, please consider change '${tpe.show}' or implement a " +
788-
"custom implicitly accessible codec")
789-
val nudeSubtype = TypeIdent(sym).tpe
790-
val tpeArgsFromChild = typeArgs(nudeSubtype.baseType(tpe.typeSymbol))
791-
nudeSubtype.memberType(sym.primaryConstructor) match
792-
case MethodType(_, _, resTp) => resTp
793-
case PolyType(names, bounds, resPolyTp) =>
794-
val targs = typeArgs(tpe)
795-
val tpBinding = resolveParentTypeArgs(sym, tpeArgsFromChild, targs, Map.empty)
796-
val ctArgs = names.map { name =>
797-
tpBinding.getOrElse(name, fail(s"Type parameter $name of $sym can't be deduced from " +
798-
s"type arguments of ${tpe.show}. Please provide a custom implicitly accessible codec for it."))
799-
}
800-
val polyRes = resPolyTp match
801-
case MethodType(_, _, resTp) => resTp
802-
case other => other // hope we have no multiple typed param lists yet.
803-
if (ctArgs.isEmpty) polyRes
804-
else polyRes match
805-
case AppliedType(base, _) => base.appliedTo(ctArgs)
806-
case AnnotatedType(AppliedType(base, _), annot) => AnnotatedType(base.appliedTo(ctArgs), annot)
807-
case _ => polyRes.appliedTo(ctArgs)
808-
case other => fail(s"Primary constructior for ${tpe.show} is not MethodType or PolyType but $other")
809-
} else if (sym.isTerm) Ref(sym).tpe
810-
else fail("Only Scala classes & objects are supported for ADT leaf classes. Please consider using of " +
811-
s"them for ADT with base '${tpe.show}' or provide a custom implicitly accessible codec for the ADT base. " +
812-
s"Failed symbol: $sym (fullName=${sym.fullName})\n")
813-
}
814-
}
815-
816-
def adtLeafClasses(adtBaseTpe: TypeRepr): Seq[TypeRepr] = {
817-
def collectRecursively(tpe: TypeRepr): Seq[TypeRepr] =
818-
val leafTpes = adtChildren(tpe).flatMap { subTpe =>
819-
if (isEnumOrModuleValue(subTpe) || subTpe =:= TypeRepr.of[None.type]) subTpe :: Nil
820-
else if (isSealedClass(subTpe)) collectRecursively(subTpe)
821-
else if (isNonAbstractScalaClass(subTpe)) subTpe :: Nil
822-
else fail(if (subTpe.typeSymbol.flags.is(Flags.Abstract) || subTpe.typeSymbol.flags.is(Flags.Trait) ) {
823-
"Only sealed intermediate traits or abstract classes are supported. Please consider using of them " +
824-
s"for ADT with base '${adtBaseTpe.show}' or provide a custom implicitly accessible codec for the ADT base."
825-
} else {
826-
"Only Scala classes & objects are supported for ADT leaf classes. Please consider using of them " +
827-
s"for ADT with base '${adtBaseTpe.show}' or provide a custom implicitly accessible codec for the ADT base."
828-
})
829-
}
830-
if (isNonAbstractScalaClass(tpe)) leafTpes :+ tpe
831-
else leafTpes
832-
833-
val classes = collectRecursively(adtBaseTpe).distinct
834-
if (classes.isEmpty) fail(s"Cannot find leaf classes for ADT base '${adtBaseTpe.show}'. " +
835-
"Please add them or provide a custom implicitly accessible codec for the ADT base.")
836-
classes
837-
}
838-
839758
def isOption(tpe: TypeRepr, types: List[TypeRepr]): Boolean = tpe <:< TypeRepr.of[Option[_]] &&
840759
(cfg.skipNestedOptionValues || !types.headOption.exists(_ <:< TypeRepr.of[Option[_]]))
841760

@@ -1122,6 +1041,90 @@ object JsonCodecMaker {
11221041
(cfg.inlineOneValueClasses && isNonAbstractScalaClass(tpe) && !isCollection(tpe) && getClassInfo(tpe).fields.size == 1 ||
11231042
tpe <:< TypeRepr.of[AnyVal])
11241043

1044+
def adtChildren(tpe: TypeRepr): Seq[TypeRepr] = { // TODO: explore yet one variant with mirrors
1045+
def resolveParentTypeArg(child: Symbol, fromNudeChildTarg: TypeRepr, parentTarg: TypeRepr,
1046+
binding: Map[String, TypeRepr]): Map[String, TypeRepr] =
1047+
if (fromNudeChildTarg.typeSymbol.isTypeParam) { // TODO: check for paramRef instead ?
1048+
val paramName = fromNudeChildTarg.typeSymbol.name
1049+
binding.get(paramName) match
1050+
case None => binding.updated(paramName, parentTarg)
1051+
case Some(oldBinding) =>
1052+
if (oldBinding =:= parentTarg) binding
1053+
else fail(s"Type parameter $paramName in class ${child.name} appeared in the constructor of " +
1054+
s"${tpe.show} two times differently, with ${oldBinding.show} and ${parentTarg.show}")
1055+
} else if (fromNudeChildTarg <:< parentTarg) binding // TODO: assure parentTag is covariant, get covariance from type parameters
1056+
else {
1057+
(fromNudeChildTarg, parentTarg) match
1058+
case (AppliedType(ctycon, ctargs), AppliedType(ptycon, ptargs)) =>
1059+
ctargs.zip(ptargs).foldLeft(resolveParentTypeArg(child, ctycon, ptycon, binding)) { (b, e) =>
1060+
resolveParentTypeArg(child, e._1, e._2, b)
1061+
}
1062+
case _ => fail(s"Failed unification of type parameters of ${tpe.show} from child $child - " +
1063+
s"${fromNudeChildTarg.show} and ${parentTarg.show}")
1064+
}
1065+
1066+
def resolveParentTypeArgs(child: Symbol, nudeChildParentTags: List[TypeRepr], parentTags: List[TypeRepr],
1067+
binding: Map[String, TypeRepr]): Map[String, TypeRepr] =
1068+
nudeChildParentTags.zip(parentTags).foldLeft(binding)((s, e) => resolveParentTypeArg(child, e._1, e._2, s))
1069+
1070+
tpe.typeSymbol.children.map { sym =>
1071+
if (sym.isType) {
1072+
if (sym.name == "<local child>") // problem - we have no other way to find this other return the name
1073+
fail(s"Local child symbols are not supported, please consider change '${tpe.show}' or implement a " +
1074+
"custom implicitly accessible codec")
1075+
val nudeSubtype = TypeIdent(sym).tpe
1076+
val tpeArgsFromChild = typeArgs(nudeSubtype.baseType(tpe.typeSymbol))
1077+
nudeSubtype.memberType(sym.primaryConstructor) match
1078+
case MethodType(_, _, resTp) => resTp
1079+
case PolyType(names, bounds, resPolyTp) =>
1080+
val targs = typeArgs(tpe)
1081+
val tpBinding = resolveParentTypeArgs(sym, tpeArgsFromChild, targs, Map.empty)
1082+
val ctArgs = names.map { name =>
1083+
tpBinding.getOrElse(name, fail(s"Type parameter $name of $sym can't be deduced from " +
1084+
s"type arguments of ${tpe.show}. Please provide a custom implicitly accessible codec for it."))
1085+
}
1086+
val polyRes = resPolyTp match
1087+
case MethodType(_, _, resTp) => resTp
1088+
case other => other // hope we have no multiple typed param lists yet.
1089+
if (ctArgs.isEmpty) polyRes
1090+
else polyRes match
1091+
case AppliedType(base, _) => base.appliedTo(ctArgs)
1092+
case AnnotatedType(AppliedType(base, _), annot) => AnnotatedType(base.appliedTo(ctArgs), annot)
1093+
case _ => polyRes.appliedTo(ctArgs)
1094+
case other => fail(s"Primary constructior for ${tpe.show} is not MethodType or PolyType but $other")
1095+
} else if (sym.isTerm) Ref(sym).tpe
1096+
else fail("Only Scala classes & objects are supported for ADT leaf classes. Please consider using of " +
1097+
s"them for ADT with base '${tpe.show}' or provide a custom implicitly accessible codec for the ADT base. " +
1098+
s"Failed symbol: $sym (fullName=${sym.fullName})\n")
1099+
}
1100+
}
1101+
1102+
def adtLeafClasses(adtBaseTpe: TypeRepr): Seq[TypeRepr] = {
1103+
def collectRecursively(tpe: TypeRepr): Seq[TypeRepr] =
1104+
val leafTpes = adtChildren(tpe).flatMap { subTpe =>
1105+
if (isEnumOrModuleValue(subTpe) || subTpe =:= TypeRepr.of[None.type]) subTpe :: Nil
1106+
else if (isSealedClass(subTpe)) collectRecursively(subTpe)
1107+
else if (isValueClass(subTpe)) {
1108+
fail("'AnyVal' and one value classes with 'CodecMakerConfig.withInlineOneValueClasses(true)' are not " +
1109+
s"supported as leaf classes for ADT with base '${adtBaseTpe.show}'.")
1110+
} else if (isNonAbstractScalaClass(subTpe)) subTpe :: Nil
1111+
else fail(if (subTpe.typeSymbol.flags.is(Flags.Abstract) || subTpe.typeSymbol.flags.is(Flags.Trait) ) {
1112+
"Only sealed intermediate traits or abstract classes are supported. Please consider using of them " +
1113+
s"for ADT with base '${adtBaseTpe.show}' or provide a custom implicitly accessible codec for the ADT base."
1114+
} else {
1115+
"Only Scala classes & objects are supported for ADT leaf classes. Please consider using of them " +
1116+
s"for ADT with base '${adtBaseTpe.show}' or provide a custom implicitly accessible codec for the ADT base."
1117+
})
1118+
}
1119+
if (isNonAbstractScalaClass(tpe)) leafTpes :+ tpe
1120+
else leafTpes
1121+
1122+
val classes = collectRecursively(adtBaseTpe).distinct
1123+
if (classes.isEmpty) fail(s"Cannot find leaf classes for ADT base '${adtBaseTpe.show}'. " +
1124+
"Please add them or provide a custom implicitly accessible codec for the ADT base.")
1125+
classes
1126+
}
1127+
11251128
def genReadKey[T: Type](types: List[TypeRepr], in: Expr[JsonReader])(using Quotes): Expr[T] =
11261129
val tpe = types.head
11271130
val implKeyCodec = findImplicitKeyCodec(types)

Diff for: jsoniter-scala-macros/shared/src/test/scala/com/github/plokhotnyuk/jsoniter_scala/macros/JsonCodecMakerSpec.scala

+9
Original file line numberDiff line numberDiff line change
@@ -2922,6 +2922,15 @@ class JsonCodecMakerSpec extends VerifyingSpec {
29222922
else "Type parameter A of class FooImpl can't be deduced from type arguments of Foo[[A >: scala.Nothing <: scala.Any] => Bar[A]]. Please provide a custom implicitly accessible codec for it."
29232923
})
29242924
}
2925+
"don't generate codecs when 'AnyVal' or one value classes with 'CodecMakerConfig.withInlineOneValueClasses(true)' are leaf types of the ADT base" in {
2926+
assert(intercept[TestFailedException](assertCompiles {
2927+
"""sealed trait X extends Any
2928+
|case class D(value: Double) extends X
2929+
|make[X](CodecMakerConfig.withInlineOneValueClasses(true))""".stripMargin
2930+
}).getMessage.contains {
2931+
"'AnyVal' and one value classes with 'CodecMakerConfig.withInlineOneValueClasses(true)' are not supported as leaf classes for ADT with base 'X'."
2932+
})
2933+
}
29252934
"don't generate codecs that cannot parse own output" in {
29262935
assert(intercept[TestFailedException](assertCompiles {
29272936
"JsonCodecMaker.make[Arrays](CodecMakerConfig.withRequireCollectionFields(true))"

0 commit comments

Comments
 (0)