Add frozen_string_literal pragma to generated files

[klyonrad]

Up for discussion obviously

Is your feature request related to a problem? Please describe.

I'm always frustrated when the files from a generator are generated do result in linting tools (mostly rubocop) screaming at me.

I know obviously that completely adhering to one style is obviously a bit pointless. But the first one rubocop offense top of the file. String literals are not frozen. And a rspec test file completely full of string literals. This is not a Style issue, it is a technical issue.

I think it would help developer UX when that would not be necessary to change after the generator execution.

Describe the solution you'd like

Template files start with the now very common frozen_string_literal: true pragma.

Describe alternatives you've considered

• Excluding the cop check from spec directory. Uncool
• Freeze strings by default when executing the ruby interpreter? yeah, not happening. To cumbersome, too brittle.

Additional context

It was even decided that string literals will not be frozen by default with Ruby 3. So the problem will not go away automatically when the rails projects are getting updated to ruby 3

See: https://bugs.ruby-lang.org/issues/11473#note-53

[pirj]

Is there any evidence that freezing literals in spec files gains any benefit in terms of memory consumption or execution time? Is there a case that a string object might be accidentally mutated and have side effects on other examples?

Related: rspec/rspec-core#2264 rspec/rspec-core#2662

[pirj]

Some prefer it with this pragma, some - without.

I'm not aware of generator integration with style configuration, it would be barely possible to yield results that would appease style checkers. With RuboCop though, it should be possible to extend generators to run rubocop --safe-auto-correct on generated files. Would it work for you?

[klyonrad]

Is there any evidence that freezing literals in spec files gains any benefit in terms of memory consumption or execution time?

In my opinion this does not matter, which is why I would not bother with making a benchmark.

Freezing string literals is objectively better, it reduces less memory; it enforces to use some more explicit code, etc. It is what the ruby community should strive for as a whole.

Some prefer it with this pragma, some - without.

Why would anyone prefer that unless for some annoying compatibility reason? Besides, we are talking about generated, emtpy, shallow test files.

EDIT: I reaaaally don't want to sound rude or anything, hopefully I didn't accidentally. I am just wondering what the problem is with the frozen string literals

[pirj]

I did some benchmarks back in the day when optimizing a huge test suite, and couldn't find any improvements in memory consumption with that pragma turned on.

One of the real-life reasons to turn it off is two meaningless lines, and I'd really love to see a good justification for that. It's not that two lines are a big deal, but what's on the other side of the scales?

[JonRowe]

👋 I don't think you sound rude, but there is back story you are missing as to why we don't do this currently. RSpec is semantically versioned and we are also catering to the lowest common denominator, we want everyone to be able to use RSpec to test things. We haven't historically recommended it because it has to work out of the box, sure its frustrating when things aren't setup the way you want, but imagine the frustration a new comer has when things don't work at all out of the box. Also remember rubocop is not the defacto ruby standard, it's their set of opinions.

So prior to the upcoming release it was a breaking change for us to introduce this to generated code, people may not have been able to use it. I'm open to changing this in RSpec-Rails 4 provided its tested as working, as I'm not sure Rails is 100% frozen string literal safe.

[pirj]

@klyonrad What are your thoughts on this?

[klyonrad]

Oh sorry, did not realize that were questions to me. I guess we can close this issue. I mean, I didn't actually expect that anyone says "yeah let's do it" but I thought it would be nice to document the reasonings here.

I still think that it wouldn't be thaaaat big of deal but given that not even the rails generators add the pragma comment (also super annoying when I just generate a DB migration 😁) I would say it's reasonable to keep it as it is (or postpone this to version 5.x or something. Although I would argue that rspec-rails can be a bit more "bleeding edge than rails 😉.

See for example https://github.com/rails/rails/blob/master/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb.tt

Also remember rubocop is not the defacto ruby standard, it's their set of opinions.

As mentioned, this is not about rubocop, but more about the ruby ecosystem. I interpret the history of the https://bugs.ruby-lang.org/issues/11473 in the way that freezing the string literals is still a good idea but Matz is only afraid about compatibility (gems and projects). So by raising awareness to this stuff the ruby ecosystem might be more carefully with string literals and maybe at some point in the future Matz would feel more confident in changing the default after all.

One of the real-life reasons to turn it off is two meaningless lines

If we would have the case that those lines become meaningless with Ruby 3, then yeah the lines have would become meaningless in the future. But how it looks now is that they will not become meaningless.

I'm not sure Rails is 100% frozen string literal safe.

I find it very hard to believe that actually rails itself would have a problem with frozen strings - they started caring about it too in their codebase. It is not unlikely that the codebases of people would break in some way - but when that happens it would not be to much of a dealbreaker to catch it in tests, right? Remember, we're not talking about applying frozen_string_literal: true to every existing file without people noticing, but to adding it to new files, the very early stage of development.

[JonRowe]

Remember, we're not talking about applying frozen_string_literal: true to every existing file without people noticing, but to adding it to new files, the very early stage of development.

Remember that new files interact with existing files, and the frozen strings can and do migrate through to existing code. This is the compatibility problem.

[pirj]

Closing since there's no clear way on how to proceed. At least not adding a pragma is less harmful, so let's keep it this way for now.

[JonRowe]

I think there is a way to proceed, we can make this opt-in with a configuration option

[pirj]

@klyonrad Would you like to tackle on this option?

[klyonrad]

Yeah, but more like around April or May

[klyonrad]

I think there is a way to proceed, we can make this opt-in with a configuration option

Hmm. I tried fiddling around with the rspec configuration object (something like RSpec.configuration.frozen_string_pragma?) but it looks to me that this is not available in the generators. Adding it to the require list in lib/generators/rspec.rb breaks completely.

@JonRowe Or was your suggestion meant as (yet another) command line flag?

[JonRowe]

It'd probably have to be a command line flag I'm afraid (that seems to be how generators work)