Add frozen string literal comments

[tegandbiscuits]

I saw #2264 and thought adding the frozen string literal comments would be a good way to ensure RSpec works with frozen string literal enabled.

This also makes rubocop enforce the comment is present. I wasn't sure if it should go in .rubocop_rspec_base.yml or just .rubocop.yml, but can move it.

I also added a test to be sure that other peoples specs don't have to have frozen string literal enabled to use RSpec.

[pirj]

Thanks!

How does this relate to #2268? Do all the runnable RSpec have frozen literals now? Is rspec-support --enable-frozen-string-literal compatible too?

In response to #2264 it turns out freezing string literals won't be the default for Ruby 3.0 (see this issue).

There's no harm in this change, but are there any benefits performance-wise or from memory consumption standpoint?

Are you up to make sure rspec is --enable-frozen-string-literal compatible for those projects that would like to enable this project-wise?

[pirj]

What is the purpose of this test?

[tegandbiscuits]

I might have misperceived some of the comments in the other issues, but it looked like some people had concern if this would break existing specs. I added this one just to prove a test that would break with frozen string literal enabled still won't break without it.

That said, I don't think there's really much value in keeping it, so don't have a problem removing it.

[tegandbiscuits]

I think it’s basically the same change as #2268, except it satisfies this comment by setting up rubocop.

I haven’t checked any of the other rspec projects, but I’ll take a look to make sure they’re also compatible.
Would that just be setting up rspec-dev, and running all the tests? Should I also add # frozen_string_literal: true to those as well?

My understanding is that enabling frozen string literals should have a bit of a performance increase, but I’m not sure if it’ll be significant.
I’ll get some benchmarks of running the test suite.

[JonRowe]

Hi, thanks for taking the time to contribute this, as per #2264 this is a change I'm wary of making.

There are two sides to frozen string support, one is ensuring that RSpec is compatible with code which uses frozen strings (which is what #2425 helps with), and the other is ensuring that if RSpec uses frozen strings that it does not break specs outside of our control.

The problem with the later, and why I'm hesitant to merge this, is we have no control over how our strings are used. Its entirely possible for RSpec to emit strings which are then mutated, which this PR would cause to throw errors, this is thus really a breaking change for those test suites. (This also applies to anything extending / using RSpec).

Its not feasible for us to test all these scenarios and the features are not the right place to do that any how, but this needs some more thorough testing (one of the reasons why it never went anywhere).

To be clear, at the moment I don't want to merge this, but I'm willing to continue a discussion about it.

[tegandbiscuits]

Test compatibility is understandable, and I think I have a better sense of the problem now than when I first read over those issues you linked.

I think it's fair if you want to close this pull request, since it looks like there isn't a significant performance difference (I haven't collected benchmarks myself yet), and frozen string literals aren't going to be enabled by default in Ruby 3.

Related to ensuring frozen string literal compatibility, would running the specs with --enable-frozen-string-literal in the CI be enough?

[JonRowe]

Your welcome to add a step to our CI doing that, you can see we have templates in rspec/rspec-dev to handle that across the repos.

[pirj]

@NRauh are you still up to do this?

[tegandbiscuits]

I'm still up to adding this in the other projects, though I haven't done much more than this. I was unsure if this change would be wanted or not.

Feel free to close this pull request if it's not something that would really be wanted, otherwise I'll try and work on it this or next week.

[pirj]

@NRauh Are you up to make sure other RSpec repositories are tolerant to frozen string literals?

[tegandbiscuits]

Hi, no probably not. I've been really busy, so in theory I'd be up for it, but time wise I probably won't be able to get to it soon.

I'm going to go ahead and close this to reduce clutter for you all.