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 #8743

Conversation

sambostock
Copy link
Contributor

@sambostock sambostock commented Sep 17, 2020

As per Ruby Feature #17104, interpolated string literals will no longer be frozen when frozen string literals are enabled.

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

This PR makes those changes.


Before submitting the PR make sure the following are checked:

  • 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.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

Fixes #8703

Interpolated and non-interpolated strings will be frozen differently.
Splitting their tests will make the change clearer.
As per https://bugs.ruby-lang.org/issues/17104, interpolated string
literals will no longer be frozen when frozen string literals are
enabled.

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
@bbatsov
Copy link
Collaborator

bbatsov commented Sep 23, 2020

Reading the conversation in the ticket it seems it's not clear if that change will be in Ruby 3.0 (despite the changes that were merged), so let's wait for the final release before changing anything.

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 4, 2020

I'll close the PR for now in the interest of reducing the number of outstanding PRs. We'll revisit it down the road.

@bbatsov bbatsov closed this Nov 4, 2020
@marcandre
Copy link
Contributor

interest of reducing the number of outstanding PRs

FWIW, I don't see the intrinsic interest in this.

@mttkay
Copy link
Contributor

mttkay commented Aug 10, 2021

@bbatsov @marcandre Since that change has indeed made it into Ruby 3, should we consider re-opening this?

We are running into the same issue in GitLab, and are now considering to either disable this Cop or patch this in RuboCop itself.

https://gitlab.com/gitlab-org/gitlab-styles/-/merge_requests/87

@splattael
Copy link
Contributor

@mttkay

Since that change has indeed made it into Ruby 3, should we consider re-opening this?

I went ahead and created #10006 which addresses interpolated strings in Ruby 3.0 or higher.

splattael added a commit to splattael/rubocop that referenced this pull request Aug 23, 2021
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
bbatsov pushed a commit that referenced this pull request Aug 23, 2021
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 #8743
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Style/RedundantFreeze and string literals
5 participants