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

Address frozen_string_literal no longer applying to interpolated strings in Ruby 3.0 or higher #10006

Merged
merged 3 commits into from Aug 23, 2021

Conversation

splattael
Copy link
Contributor

@splattael splattael commented Aug 12, 2021

Note, this PR is strongly inspired by #8743. Kudos to @sambostock ❤️


Since Ruby 3.0 or higher interpolated strings are not frozen automatically anymore.

Therefore, two changes must happen to how we treat interpolated literals

  • Style/MutableConstant should enforce freezing them
  • Style/RedundantFreeze should not consider it redundant to freeze them

The behaviour for Ruby 2.7 or lower is kept.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • [-] Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@splattael splattael force-pushed the ruby3-interpolated-string-literals branch 2 times, most recently from 4a94653 to 0bf81bb Compare August 12, 2021 16:01
@splattael splattael marked this pull request as ready for review August 12, 2021 16:02
@splattael
Copy link
Contributor Author

@sambostock As the original author of #8743 do you mind reviewing these changes? 🙏

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 17, 2021

The changes look good to me, but I think that this difference in behaviour should be mentioned in the docs for the two cops. I'm pretty sure most people don't know that there's a difference on Ruby 2.7 and 3.0. (I didn't know this myself)

@splattael
Copy link
Contributor Author

@bbatsov Good point about the docs, thanks 🙇

I'll add them and ping you again for another round of review 🙏

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 23, 2021

You'll also have to rebase on top of master, as there are now some conflicting changes in those files.

Use this helper method instead repeating the logic everywhere
to ease future changes.
@splattael splattael force-pushed the ruby3-interpolated-string-literals branch 2 times, most recently from 4287edd to 3dd5ae9 Compare August 23, 2021 08:48
@@ -21,6 +21,9 @@ module Style
#
# NOTE: Regexp and Range literals are frozen objects since Ruby 3.0.
#
# NOTE: From Ruby 3.0, this cop allows explicit freezing of interpolated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I wanted to link https://rubyreferences.github.io/rubychanges/3.0.html#interpolated-string-literals-are-no-longer-frozen-when--frozen-string-literal-true-is-used but it seems we are not linking to external resources in inline docs very often.

@splattael
Copy link
Contributor Author

@bbatsov Thanks for the heads-up about the rebase. Done ✔️

I've added added a note to Style/MutableConstant and Style/RedundantFreeze about the difference for interpolated string literals in Ruby 3.0.

Do you mind reviewing this PR again? 🙏

Since Ruby 3.0 or higher interpolated strings are not frozen
automatically anymore.

Therefore, two changes must happen to how we treat interpolated literals

* Style/MutableConstant should enforce freezing them
* Style/RedundantFreeze should not consider it redundant to freeze them

Strongly inspired by rubocop#8743
@splattael splattael force-pushed the ruby3-interpolated-string-literals branch from 3dd5ae9 to c537fc2 Compare August 23, 2021 08:53
@@ -21,6 +21,9 @@ module Style
#
# NOTE: Regexp and Range literals are frozen objects since Ruby 3.0.
Copy link
Contributor Author

@splattael splattael Aug 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems we are using different style to highlight notes. I went with NOTE: but would be happy to use "From Ruby 3.0, ..." pattern as seen above.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: is better.

@bbatsov bbatsov merged commit 8b8635a into rubocop:master Aug 23, 2021
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 23, 2021

Looks good. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants