Fix #93 Wart for implicit toString in String concatenation and interpolation

[tim-zh]

No description provided.

[pmpfr]

This would be really great - it's so easy to write s"blah${someWrapperType}blah" when you mean s"blah${someWrapperType.stringRepr}blah". I have a suggestion to keep string interpolation useful without reducing safety though. I think in the context of string interpolation, you should allow not just String but also Byte Short, Int, Long and Char. Essentially because there can't be any sort of surprise or unexpected behaviour that these would all be converted to their obvious string representation (and Char to the expected one-Char String).

I would suggest keeping the ban on Float, Double, Boolean in string interpolation though.

What do you think, @tim-zh ?

[tim-zh]

@pmpfr Why these types? It could be s"$blah{StringRepr(x)}blah" for any primitive.

[pmpfr]

I'm not sure I understand your question. You can certainly always define a StringRepr function taking your type and returning a String, and nothing is stopping you from doing that (neither now, nor after your fix as-is, nor after my suggestion). You can also write s"$blah${x.toString}$blah" if you want to.

What I am saying is that you shouldn't need to do that for String, Byte, Short, Int, Long or Char whenever you want the "natural" String representation of those types.

My suggestion is that for those types, there is no reasonable possibility of actually meaning a different string representation than the one .toString gives you. Of course there are possible alternative string representations of all of them, including String itself, but I don't think there's a "correctness issue" allowing them because it would be unreasonable to be surprised that you don't get those implementation by default. By perhaps you disagree there?

If you are questioning why I exclude Float, Double and Char, it's because for Float and Double the choice of representation is arbitrary (e.g. number of decimal places) and lossy, and in the case of Boolean, there's not really an obvious standard (if forced, you'd probably guess "true/"false", but you could have expected 1/0 or T/F or ...

[ClaireNeveu]

I like those exceptions (otherwise the s interpolator is basically useless) but I would also include Boolean.

[tim-zh]

If you are questioning why I exclude Float, Double and Char

Yes. "Natural" representation is a vague thing. On the one hand, any type has a standard one, which is result of toString. On the other hand, you may want a special representation for each type and take a "natural" one as a bug.

[ClaireNeveu]

I mean, ideally s would take some type-class like Show but it doesn't and we should be pragramatic here. The representations of most primitives tend to be common among many purposes. 1 is almost always represented as "1" whether output to logs, a template, a CLI, etc. We normally prevent implicit conversion to string for Int because the + operator is ambiguous and has a different return type depending on the arguments; in the context of s we always expect things to be converted to a string and the result type to a string.

I'm sure we've all hit a bug where we meant to output "foo" but actually output "Some(foo)" but it's less common that you really wanted "IV" instead of "4". We can always add a wart that bans s in contrib.

[pmpfr]

I suppose for me it comes down to this. I don't think it's reasonable for Wartremover to stop people writing { val i = 4; s"blah${i}blah" } hoping for "blahIVblah" but I do think it's reasonable to prevent { val n = Name("Peter"); s"blah${n}blah" } hoping for "blahPeterblah".

The reason for the difference being that I think there are a small number of types which people are "born knowing" their toString representation. There's so little room for doubt that it's reasonable (IMO) to rely on it in production code, that they're round-trip safe and suitable for machine parsing. None of that is true (IMO) for any other toString implementation.

So I want wartremover to warn me about any other implicit .toString calls even in string interpolation. But I don't want it to warn me about (i:Int).toString calls in string interpolation, because I couldn't reasonably have meant anything else. Anyway, that's all I have.

[som-snytt]

Interesting idea! Retronym has a PR to convert interpolation to string + in refchecks, where the backend emits efficient string builder etc. That's ok bc the plug-in runs earlier. It makes me think current scheme allowing only string args is best as it avoids boxing, perhaps. But I also think it effectively outlaws s & raw. It would break all my debug strings. Probably I should use a debug interpolator. The idea to allow int and char is ok but wouldn't enable me to use it. As mentioned everyone should use prettyprinting typeclass.

The check for f interpolation or arbitrary format strings would be tricky albeit doable.

[som-snytt]

I quite forgot that I'd left some famous last words here.

Standard interpolators are now intrinsic, expanded at typer in Scala 2 and much later in Scala 3.

If I want 1024 as "1,024", I especially like the idea of f"$n%d" looking for a typeclass. Currently you get an expected type and applicable conversions. In addition, it should skip String.format and just concatenate directly.