Feature #21269
closed
Opt in to raise for modification of chilled strings returned from `Symbol#to_s`
Description
Ref https://bugs.ruby-lang.org/issues/20350
begin
:foo.to_s << "bar"
puts "ok sym"
rescue
puts "fail sym"
end
begin
"foo" << "bar"
puts "ok str"
rescue
puts "failed str"
end
Output with RUBYOPT="--enable=frozen-string-literal -W":
test.rb:2: warning: string returned by :foo.to_s will be frozen in the future
ok sym
failed str
--enable=frozen-string-literal does something for string literals, I would like the same for Symbol#to_s. I would suggest simply reusing --enable=frozen-string-literal but its name seems very clear about being about string literals, which doesn't really fit for what's going on with symbols here.
For convenience, it would be very nice to have just a single switch for this though.
Updated by byroot (Jean Boussier) 2 months ago
I understand the ask, and that's something we considered with @etienne when we implemented it, but I thought it was too weird for --enable=frozen-string-literal to impact something that isn't a literal.
A pattern I use a lot for this sort of things is to turn warnings into errors globally by decorating Warning.warn, for instance: https://github.com/rails/rails/blob/66732971111a62e5940268e1daf7d413c72a234f/tools/strict_warnings.rb
Some test frameworks like minitest have this feature built-in now: https://github.com/minitest/minitest/blob/4dcad89a788e56f8abb30952ed839a41f1b3ec6e/lib/minitest/error_on_warning.rb
Updated by Earlopain (Earlopain _) 2 months ago
Initially I thought it was a bug that it didn't impact symbols but like you said, the current existing switch it rather explicit in what it is about. I know about overwriting warn and it's something I already do in my own projects (didn't know about minitest though, nice one).
I stumbled over this while contributing to an external project. The warning prints the symbol name the string originated from which can be useful, here I had many places with the same symbol so I wanted it to just raise and see what happens. I guess I can just overwrite warn for this, now that I was reminded of it again, but I am much more familiar with the command switch.
Considering that this was the first time I actually encountered this warning, I'd say it's probably not worth it to add something that needs to be maintained for however long. If you said to just roll it into the existing switch, that would have been ok. But we seem to both agree that it is not really the right place.
All that said, feel free to close this.
Updated by byroot (Jean Boussier) 2 months ago
- Status changed from Open to Rejected