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

Show null compatibility with scala string interpolator #4689

Open
BusyByte opened this issue Dec 19, 2024 · 3 comments
Open

Show null compatibility with scala string interpolator #4689

BusyByte opened this issue Dec 19, 2024 · 3 comments

Comments

@BusyByte
Copy link

BusyByte commented Dec 19, 2024

We have a lot of java things we interact and they can throw exceptions or have null in certain pojo fields to where if we use them in a string interpolator it will just print null

val foo = Foo(a, null)
log.info(s"found a foo ${foo.secondField)}")

val exception = new RuntimeException()
log.error(s"something happened ${exception.getMessage}", exception)

We tried to migrate to show but it blows up and does not give you a good error message to indicate where the problem is.
I asked in Discord and here's the feedback:

  1. don't use nulls (it's very hard to do this especially when integrating with a lot of java teams/libraries)
  2. it's a bug, file a bug (doing that here)
  3. write my own version of show SafeShow and use that.

I've done number 3 and created SafeShow but incorporating it into our legacy monolith is an effort itself so I thought I'd open this to see if anything could be done in cats itself. (I've basically duplicated the show syntax, interpolator and use the typename as required field for constructing the Show type class instance using SafeShow and log the typename and the exception when we recover from NPE before printing "null"). This is not something I really want to maintain although it's not that much code (less than 100 lines of main code). Also most of our libraries just use plain Show so we have to go and change all those as well.

The ability to adopt and use things from cats for newbies is also a hurdle. When do I use our safe show and when do we not.
It's a lot for newbies to absorb and makes the interop with Java stuff harder. It's also a hurdle when things that seem like they should be equivalent are not.

@satorg
Copy link
Contributor

satorg commented Dec 19, 2024

@BusyByte , thank you for the report!
Would you mind adding an example with Show that causes the issue please?

@BusyByte
Copy link
Author

BusyByte commented Dec 19, 2024

@satorg here you go

import cats.implicits._
case class Foo(firstField: String, secondField: String)
implicit val showFoo: Show[Foo] = Show.fromToString
val foo = Foo(a, null)
println(show"found a foo ${foo.secondField)}")

val exception = new RuntimeException()
println(show"something happened ${exception.getMessage}")

val message = foo.secondField.show
val anotherMessage = exception.getMessage.show

println(message)
println(anotherMessage)

val foo2: Foo = null
println(show"found a $foo2")

val message3 = foo2.show
println(message3)


@johnynek
Copy link
Contributor

Maybe a fix would be to make fromToString work correctly with null so it returns "null" or something. That should address the Java interop issue without changing behavior for people that set their own Show instances.

Or we could bake support for null into the macro.

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

No branches or pull requests

3 participants