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

Prohibiting automatic .toString in string interpolation (like s"$someclass"). #394

Open
kszilagyi opened this issue Aug 12, 2017 · 9 comments
Labels

Comments

@kszilagyi
Copy link

Hi,

I am about to start to write this actually but I saw the process is to create an issue first.

Why is it good?

Basically the similar justification as StringPlusAny. I made the mistake a couple of times myself, putting a wrapper case class into a string (into the DB!) and forgetting that it won't be what you would expect. I will provide an other interpolator which does allow this for logging though. But the default should be safe and people should explicitly decide that they don't want safe interpolation when it's not important.

Please let me know if you think this is not a good idea or have any suggestions about it.

@ritschwumm
Copy link
Contributor

@tim-zh
Copy link
Contributor

tim-zh commented Aug 12, 2017

Have a look at #388

@kszilagyi
Copy link
Author

Ah, cool, thanks!

@ritschwumm
Copy link
Contributor

i wonder whether disallowing the s interpolator completely wouldn't be the better solution

@fommil
Copy link
Contributor

fommil commented Nov 26, 2017

I think scalafix may be able to implement this by adding a Disable rule for the default s interpolator. You may then require use of your own interpolator named s or an alternative such as show which uses the Show instance.

@ritschwumm
Copy link
Contributor

well, i have wartremover in all my builds already - should i additionally need scalafix for something that quite definitely falls into the range of things wartremover exists for to prevent?

@fommil
Copy link
Contributor

fommil commented Nov 26, 2017

scalafix will replace wartremover, eventually.

@xuwei-k xuwei-k added the wart label Sep 20, 2018
@devkat
Copy link

devkat commented Dec 3, 2018

scalafix will replace wartremover, eventually.

What is the background of this statement? Is this opinion shared by the other Wartremover contributors? TIA for a brief explanation!

@som-snytt
Copy link

I love the confluence of "warn" and "wart" in "es warnt". Which I think maybe in southern U.S. english is also good for "was not".

In Scala 2, it was still possible to subvert s with an implicit, but I guess no longer in Scala 3.

I've been intending since the time of this thread to learn something about scalafix, but, among many other things to do, have not yet made the time. Maybe I was waiting for it to supplant wartremover?

To be fair, Scala 3 is not yet done removing warts from the language.

(I stopped by while researching features for -Wtostring-interpolated.)

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

No branches or pull requests

7 participants