Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

case object doesn't encode correctly #1114

Open
DesZhang opened this issue Jan 30, 2024 · 9 comments
Open

case object doesn't encode correctly #1114

DesZhang opened this issue Jan 30, 2024 · 9 comments
Labels

Comments

@DesZhang
Copy link

Recently I upgrade to 2.27.7 in my scala 3 project, the minimal example is:

final case class Error[+E <: ErrCode](code: E, msg: String)
given [E <: ErrCode](using JsonValueCodec[E]): JsonValueCodec[Error[E]] = JsonCodecMaker.make

sealed trait ErrCode
given JsonValueCodec[ErrCode] = JsonCodecMaker.make(
  CodecMakerConfig.withDiscriminatorFieldName(None)
)

@named("error type 1")
case object errType1 extends ErrCode
given JsonValueCodec[errType1.type] = JsonCodecMaker.make

@named("error type 2")
case object errType2 extends ErrCode

val err                  = Error(errType1, "msg")
val gErr: Error[ErrCode] = err

// {"code":{},"msg":"msg"}
val json1 = writeToString(err)
// {"code":"error type 1","msg":"msg"}
val json2 = writeToString(gErr)

obviously json1 is incorrect.

BTW: the library is amazing, thank you.

@plokhotnyuk
Copy link
Owner

plokhotnyuk commented Jan 30, 2024

Hi, @DesZhang! Thanks for your feedback and support!

I would say that jsoniter-scala-macros derives right codec according to types derived by scala compiler.

The root cause of issue that types are different for err and gErr values.

I've pasted your code to the module tests and got the following type hint for err:

image

Possible other workarounds to help scalac derive a proper type are using an explicit type when creating value or writing it :

writeToString(Error[ErrCode](errType1, "msg"))
writeToString[Error[ErrCode]](err)

Yet another solution could be removing (or commenting out) of a redundant codec for errType1.type type:

//given JsonValueCodec[errType1.type] = JsonCodecMaker.make

Then compiler will say that writeToString require an implicit parameter for JsonValueCodec[Error[errType1.type]]:

image

@DesZhang
Copy link
Author

@plokhotnyuk Thank you for quick response, the last part is exactly what I was trying to deal with. Do I have to widen the type explicitly every time so the compiler could use JsonValueCodec[ErrCode] to synthesize the instance for encoding?

and I'm still confused why value of code is empty in json1, I already had an instance of JsonValueCodec[errType1.type], and the compiler didn't complain, something like "error type 1" should be expected.

@plokhotnyuk
Copy link
Owner

plokhotnyuk commented Jan 30, 2024

The best option would be using of non-generic Error class and derivation of just one top-level codec for it:

final case class Error(code: ErrCode, msg: String)
given codec: JsonValueCodec[Error] = JsonCodecMaker.make(CodecMakerConfig.withDiscriminatorFieldName(None))

sealed trait ErrCode

@named("error type 1")
case object errType1 extends ErrCode

@named("error type 2")
case object errType2 extends ErrCode

val err         = Error(errType1, "msg")
val gErr: Error = err

// {"code":"error type 1","msg":"msg"}
println(writeToString(err))
// {"code":"error type 1","msg":"msg"}
println(writeToString(gErr))

@DesZhang
Copy link
Author

@plokhotnyuk thank you

@plokhotnyuk
Copy link
Owner

plokhotnyuk commented Jan 31, 2024

@DesZhang I have no confidence if proposed solutions were acceptable for you.

Yes, for your version of code snippet widening is required because the compiler tends to derive the narrowest type.

The codec generated by given JsonValueCodec[errType1.type] = JsonCodecMaker.make is out of any context and just serializes an empty JSON object to be distinguished from the default null value.

Sometime users report minimized code for their issue without description of the context, so my willing to get the simplest solution could be unacceptable or hard to grasp.

Have you migrated from Scala 2 or your code was initially created for other json library?

As example, if you migrate your code from circe or upickle codecs replacing them by jsoniter-scala codecs step by step, than it could be a long journey. The ideal approach would be to start from tests for top-level messages and derive only required jsoniter-scala codecs.

@DesZhang
Copy link
Author

DesZhang commented Mar 8, 2024

@plokhotnyuk thank you and sorry for late response. I used type parameter to make different types of Errors so that I could make precise error declaration in ZIO's error channel, like ZIO[Any, NotFoundError | PageError | NetworkError, A] .

I've started a new project with jsoniter-scala(2.28.3) , and type widening is ok for me, recently I found another compiler warning I don't know how to deal with.

sealed trait Error[+E <: Code] {
    val code: E
    val msg: String
}

@named("basic")
final case class BasicError[E <: Code](
    code: E,
    msg: String
) extends Error[E]

@named("line")
final case class LineError[E <: Code](
    code: E,
    ln: LineNumber,
    msg: String
) extends Error[E]

sealed trait Code

@named("0001")
case object notFound extends Code
type NotFound = Error[notFound.type]

@named("0002")
case object incomplete extends Code
type Incomplete = Error[incomplete.type]

// compiler warning: the type test for BasicError[Code] cannot be checked at runtime because
// its type arguments can't be determined from Error[Code]
given JsonValueCodec[Error[Code]] = JsonCodecMaker.make(
    CodecMakerConfig
          .withDiscriminatorFieldName(Some("tpe"))
          .withRequireDiscriminatorFirst(true)
)

The compiler(scala 3.4.0) complains about type test problem caused by type erasure, my guess is that pattern match happened somewhere in code generated by jsoniter-scala macro, since E has upper bound, can I safely assume the warning could be ignored? or what can I do to eliminate it?

Beside the question above, another strange thing is, I have almost the same error coding pattern in another project, by "almost" I mean they only have different class/trait names, and I got no warning at all, both projects have the same versions of jsoniter-scala and scala compiler.

@plokhotnyuk
Copy link
Owner

@DesZhang Thanks for your feedback!

Please peek the latest release with a fix for unwanted warning: https://github.com/plokhotnyuk/jsoniter-scala/releases/tag/v2.28.4

@DesZhang
Copy link
Author

DesZhang commented Mar 9, 2024

@plokhotnyuk wow,that was lightning fast, I'll try and report back asap

@DesZhang
Copy link
Author

@plokhotnyuk 2.28.4 works as expected, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants