Skip to content

Commit

Permalink
[Fix rubocop#11977] Add new global StringLiteralsFrozenByDefault op…
Browse files Browse the repository at this point in the history
…tion

While Ruby continues to not have this the default, user can
opt into it using the `RUBYOPT` environment variable.
To make RuboCop behave correctly when doing that, offer a config value to say
how literals behave by default.

The most egregious cop `Style/FrozenStringLiteralComment` can already be configured
appropriately by setting `EnforcedStyle` to `never`.
  • Loading branch information
Earlopain authored and bbatsov committed Aug 21, 2024
1 parent 4e88589 commit 5631ce2
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#13077](https://github.com/rubocop/rubocop/pull/13077): Add new global `StringLiteralsFrozenByDefault` option for correct analysis with `RUBYOPT=--enable=frozen-string-literal`. ([@earlopain][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ AllCops:
rubocop-rspec_rails: [rspec-rails]
# Enable/Disable checking the methods extended by Active Support.
ActiveSupportExtensionsEnabled: false
# Future version of Ruby will freeze string literals by default.
# This allows to opt in early, for example when enabled through RUBYOPT.
# For now this will behave as if set to false but in future ruby versions
# (likely 4.0) it will be true by default.
StringLiteralsFrozenByDefault: ~

#################### Bundler ###############################

Expand Down
15 changes: 15 additions & 0 deletions docs/modules/ROOT/pages/configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1001,3 +1001,18 @@ This is off by default, but can be enabled by the `ActiveSupportExtensionsEnable
AllCops:
ActiveSupportExtensionsEnabled: true
----

== Opting into globally frozen string literals

Ruby continues to move into the direction of having all string literals frozen by default.
Ruby 3.4 for example will show a warning if a non-frozen string literal from a file without
the frozen string literal magic comment gets modified. By starting ruby with the environment
variable `RUBYOPT` set to `--enable=frozen-string-literal` you can opt into that behaviour today.
For RuboCop to provide accurate analysis you must also configure the `StringLiteralsFrozenByDefault`
option.

[source,yaml]
----
AllCops:
StringLiteralsFrozenByDefault: true
----
4 changes: 4 additions & 0 deletions lib/rubocop/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ def active_support_extensions_enabled?
for_all_cops['ActiveSupportExtensionsEnabled']
end

def string_literals_frozen_by_default?
for_all_cops['StringLiteralsFrozenByDefault']
end

def file_to_include?(file)
relative_file_path = path_relative_to_config(file)

Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ def active_support_extensions_enabled?
@config.active_support_extensions_enabled?
end

def string_literals_frozen_by_default?
@config.string_literals_frozen_by_default?
end

def relevant_file?(file)
return false unless target_satisfies_all_gem_version_requirements?
return true unless @config.clusivity_config_for_badge?(self.class.badge)
Expand Down
23 changes: 16 additions & 7 deletions lib/rubocop/cop/mixin/frozen_string_literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,28 @@ def frozen_string_literals_enabled?
ruby_version = processed_source.ruby_version
return false unless ruby_version

# Check if a magic string literal comment specifies what to do
magic_comments = leading_comment_lines.filter_map { |line| MagicComment.parse(line) }
if (literal_magic_comment = magic_comments.find(&:frozen_string_literal_specified?))
return literal_magic_comment.frozen_string_literal?
end

# TODO: Ruby officially abandon making frozen string literals default
# for Ruby 3.0.
# https://bugs.ruby-lang.org/issues/11473#note-53
# Whether frozen string literals will be the default after Ruby 3.0
# or not is still unclear as of January 2019.
# Whether frozen string literals will be the default after Ruby 4.0
# or not is still unclear as of July 2024.
# It may be necessary to add this code in the future.
#
# return true if ruby_version >= 3.1
# return ruby_version >= 4.0 if string_literals_frozen_by_default?.nil?
#
# And the above `ruby_version >= 3.1` is undecided whether it will be
# Ruby 3.1, 3.2, 4.0 or others.
# See https://bugs.ruby-lang.org/issues/8976#note-41 for details.
leading_comment_lines.any? { |line| MagicComment.parse(line).frozen_string_literal? }
# And the above `ruby_version >= 4.0` is undecided whether it will be
# Ruby 4.0 or others.
# See https://bugs.ruby-lang.org/issues/20205 for details.
# For now, offer a configuration value to override behavior is using RUBYOPT.
return false if string_literals_frozen_by_default?.nil?

string_literals_frozen_by_default?
end

def frozen_string_literals_disabled?
Expand Down
6 changes: 4 additions & 2 deletions lib/rubocop/cop/style/empty_literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ def correction(node)
def frozen_strings?
return true if frozen_string_literals_enabled?

frozen_string_cop_enabled = config.for_cop('Style/FrozenStringLiteral')['Enabled']
frozen_string_cop_enabled && !frozen_string_literals_disabled?
frozen_string_cop_enabled = config.for_cop('Style/FrozenStringLiteralComment')['Enabled']
frozen_string_cop_enabled &&
!frozen_string_literals_disabled? &&
string_literals_frozen_by_default?.nil?
end
end
end
Expand Down
87 changes: 84 additions & 3 deletions spec/rubocop/cop/style/empty_literal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def foo
end

describe 'Empty String', :config do
let(:other_cops) { { 'Style/FrozenStringLiteral' => { 'Enabled' => false } } }
let(:other_cops) { { 'Style/FrozenStringLiteralComment' => { 'Enabled' => false } } }

it 'registers an offense for String.new()' do
expect_offense(<<~RUBY)
Expand Down Expand Up @@ -287,8 +287,8 @@ def foo
end
end

context 'when Style/FrozenStringLiteral is enabled' do
let(:other_cops) { { 'Style/FrozenStringLiteral' => { 'Enabled' => true } } }
context 'when Style/FrozenStringLiteralComment is enabled' do
let(:other_cops) { { 'Style/FrozenStringLiteralComment' => { 'Enabled' => true } } }

context 'and there is no magic comment' do
it 'does not register an offense' do
Expand All @@ -313,5 +313,86 @@ def foo
end
end
end

context 'when `AllCops/StringLiteralsFrozenByDefault: true`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'StringLiteralsFrozenByDefault' => true })
end

context 'when the frozen string literal comment is missing' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
test = String.new
RUBY
end
end

context 'when the frozen string literal comment is true' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
# frozen_string_literal: true
test = String.new
RUBY
end
end

context 'when the frozen string literal comment is false' do
it 'registers an offense' do
expect_offense(<<~RUBY)
# frozen_string_literal: false
test = String.new
^^^^^^^^^^ Use string literal `''` instead of `String.new`.
RUBY

expect_correction(<<~RUBY)
# frozen_string_literal: false
test = ''
RUBY
end
end
end

context 'when `AllCops/StringLiteralsFrozenByDefault: false`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'StringLiteralsFrozenByDefault' => false })
end

context 'when the frozen string literal comment is missing' do
it 'registers an offense' do
expect_offense(<<~RUBY)
test = String.new
^^^^^^^^^^ Use string literal `''` instead of `String.new`.
RUBY

expect_correction(<<~RUBY)
test = ''
RUBY
end
end

context 'when the frozen string literal comment is true' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
# frozen_string_literal: true
test = String.new
RUBY
end
end

context 'when the frozen string literal comment is false' do
it 'registers an offense' do
expect_offense(<<~RUBY)
# frozen_string_literal: false
test = String.new
^^^^^^^^^^ Use string literal `''` instead of `String.new`.
RUBY

expect_correction(<<~RUBY)
# frozen_string_literal: false
test = ''
RUBY
end
end
end
end
end
44 changes: 44 additions & 0 deletions spec/rubocop/cop/style/redundant_freeze_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,50 @@
expect_no_offenses('TOP_TEST = Something.new.freeze')
end

context 'when `AllCops/StringLiteralsFrozenByDefault: true`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'StringLiteralsFrozenByDefault' => true })
end

context 'when the frozen string literal comment is missing' do
it_behaves_like 'immutable objects', '""'
end

context 'when the frozen string literal comment is true' do
let(:prefix) { '# frozen_string_literal: true' }

it_behaves_like 'immutable objects', '""'
end

context 'when the frozen string literal comment is false' do
let(:prefix) { '# frozen_string_literal: false' }

it_behaves_like 'mutable objects', '""'
end
end

context 'when `AllCops/StringLiteralsFrozenByDefault: false`' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'StringLiteralsFrozenByDefault' => false })
end

context 'when the frozen string literal comment is missing' do
it_behaves_like 'mutable objects', '""'
end

context 'when the frozen string literal comment is true' do
let(:prefix) { '# frozen_string_literal: true' }

it_behaves_like 'immutable objects', '""'
end

context 'when the frozen string literal comment is false' do
let(:prefix) { '# frozen_string_literal: false' }

it_behaves_like 'mutable objects', '""'
end
end

context 'when the receiver is a string literal' do
# TODO : It is not yet decided when frozen string will be the default.
# It has been abandoned in the Ruby 3.0 period, but may default in
Expand Down

0 comments on commit 5631ce2

Please sign in to comment.