Add a new wart: NonPrimitiveInStringInterpolation

[y-yu]

• We can use the non-~~String~~ primitive value in the string interpolation such as s"hello $caseClass" where val caseClass = Dummy("world")
• It sometimes creates runtime bugs which are very difficult to find out
  • For example, I changed a case class field type from String to an other type, then most of places I have to fix were pointed out as compilation errors
  • However the field was used in string interpolations, and I couldn't find all of them out 😢
• So I make a wart NonPrimitiveInStringInterpolation, which does not allow us to use string interpolations that contain non-~~String~~ primitive value as the arguments
• Does it make sense? 🤔
• This PR could be related:
  • https://github.com/wartremover/wartremover/issues/394
  • https://github.com/wartremover/wartremover/pull/388
  • https://github.com/wartremover/wartremover/issues/447
• UPDATE: December 16th
  • I made this change to allow some primitive types rather than only String
  • As @reibitto saying, I realized it's more convenient 🙇‍♀️

[reibitto]

I've found myself wanting a rule for this too. Restricting this to just String might be a little too much though? Primitive types seem mostly alright to me. I don't know... maybe it's a fine default though.

If you could define a Set of types that are allowed in the config, it might cover more cases. Like, I might want to say "allow only String and all primitives except Float/Double" (since I might want to make sure floating-point numbers are always explicitly formatted).

[y-yu]

Primitive types seem mostly alright to me

I thought it would be better to allow CharSequence rather than String... I also think that it doesn't make sense to allow us to use all primitive types. As you said, some people cannot determine if it can be OK to use Float and Double in string interpolations. The only type we can use in string interpolations is String, which is very clear and simple to understand. I don't know it's good or not but I prefer clearness and simpleness than extensibility.

[y-yu]

I welcome discussions about what types should be allowed 🤔

[reibitto]

I don't think there's a definite answer, but I think you're right that String/CharSequence might be the "safest" default. I'm only bringing up the idea of customizable sets of types because I think it would be a shame if this PR gets closed due to no consensus being reached. Even if I would prefer some primitive types to be included too, I'd rather use this String-only rule as is than not having it at all.

[y-yu]

By the way, how can we provide settings/configurations to wart? 🤔

[reibitto]

Oh, hmm, there might not be a built-in way of customizing rules. Sorry, I assumed there was. Using properties or something isn't exactly ideal...

If there is no nice way of doing it currently, I guess that kind of eliminates my suggestion. Oh well, carry on. 😅

[y-yu]

I thought https://github.com/wartremover/wartremover-contrib/pull/55/files#diff-bbbae801164db9c1ce1a89a3fe34c5c7R34 👈 this is the result of a new optimization provided by Scala2.13 🤔 If it's true, this wart maybe give us true negative decisions due to the optimization. I think it's a problem.

[y-yu]

I think the optimization is up to FastStringInterpolator https://github.com/scala/scala/pull/7678

• https://github.com/scala/scala/blob/bb894b2f925fbc698845a92d84b4ba260bfcb733/src/compiler/scala/tools/reflect/FastStringInterpolator.scala#L23-L24

[y-yu]

@xuwei-k Could you please check this PR? 🙇

[y-yu]

I changed that it also allows us to use the other primitive types, not only String. I used it, and I think it would be better. Thanks @reibitto for your suggestion.

[xuwei-k]

• https://github.com/scala/scala/pull/10776 🤔

[som-snytt]

Thanks for linking. I will look for more ideas over here.