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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_interpolated_string_literals_are_no.md
@@ -0,0 +1 @@
* [#10006](https://github.com/rubocop/rubocop/pull/10006): Interpolated string literals are no longer frozen since Ruby 3.0. ([@splattael][])
15 changes: 14 additions & 1 deletion lib/rubocop/cop/mixin/frozen_string_literal.rb
Expand Up @@ -8,14 +8,27 @@ module FrozenStringLiteral

FROZEN_STRING_LITERAL = '# frozen_string_literal:'
FROZEN_STRING_LITERAL_ENABLED = '# frozen_string_literal: true'
FROZEN_STRING_LITERAL_TYPES = %i[str dstr].freeze
FROZEN_STRING_LITERAL_TYPES_RUBY27 = %i[str dstr].freeze
FROZEN_STRING_LITERAL_TYPES_RUBY30 = %i[str].freeze

private_constant :FROZEN_STRING_LITERAL_TYPES_RUBY27, :FROZEN_STRING_LITERAL_TYPES_RUBY30

def frozen_string_literal_comment_exists?
leading_comment_lines.any? { |line| MagicComment.parse(line).valid_literal_value? }
end

private

def frozen_string_literal?(node)
literal_types = if target_ruby_version >= 3.0
FROZEN_STRING_LITERAL_TYPES_RUBY30
else
FROZEN_STRING_LITERAL_TYPES_RUBY27
end

literal_types.include?(node.type) && frozen_string_literals_enabled?
end

def frozen_string_literals_enabled?
ruby_version = processed_source.ruby_version
return false unless ruby_version
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/frozen_string_literal_comment.rb
Expand Up @@ -141,7 +141,7 @@ def last_special_comment(processed_source)

def frozen_string_literal_comment(processed_source)
processed_source.find_token do |token|
token.text.start_with?(FrozenStringLiteral::FROZEN_STRING_LITERAL)
token.text.start_with?(FROZEN_STRING_LITERAL)
end
end

Expand Down
11 changes: 5 additions & 6 deletions lib/rubocop/cop/style/mutable_constant.rb
Expand Up @@ -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.

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

# string literals when `# frozen-string-literal: true` is used.
#
# @example EnforcedStyle: literals (default)
# # bad
# CONST = [1, 2, 3]
Expand Down Expand Up @@ -151,8 +154,8 @@ def check(value)
range_enclosed_in_parentheses = range_enclosed_in_parentheses?(value)
return unless mutable_literal?(value) ||
target_ruby_version <= 2.7 && range_enclosed_in_parentheses
return if FROZEN_STRING_LITERAL_TYPES.include?(value.type) &&
frozen_string_literals_enabled?

return if frozen_string_literal?(value)
return if shareable_constant_value?(value)

add_offense(value) { |corrector| autocorrect(corrector, value) }
Expand Down Expand Up @@ -183,10 +186,6 @@ def immutable_literal?(node)
frozen_regexp_or_range_literals?(node) || node.immutable_literal?
end

def frozen_string_literal?(node)
FROZEN_STRING_LITERAL_TYPES.include?(node.type) && frozen_string_literals_enabled?
end

def shareable_constant_value?(node)
return false if target_ruby_version < 3.0

Expand Down
7 changes: 4 additions & 3 deletions lib/rubocop/cop/style/redundant_freeze.rb
Expand Up @@ -7,6 +7,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
# string literals when `# frozen-string-literal: true` is used.
#
# @example
# # bad
# CONST = 1.freeze
Expand Down Expand Up @@ -37,9 +40,7 @@ def immutable_literal?(node)
node = strip_parenthesis(node)

return true if node.immutable_literal?

return true if FROZEN_STRING_LITERAL_TYPES.include?(node.type) &&
frozen_string_literals_enabled?
return true if frozen_string_literal?(node)

target_ruby_version >= 3.0 && (node.regexp_type? || node.range_type?)
end
Expand Down
122 changes: 65 additions & 57 deletions spec/rubocop/cop/style/mutable_constant_spec.rb
Expand Up @@ -80,6 +80,69 @@
it_behaves_like 'immutable objects', "::ENV['foo']"
end

shared_examples '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
# the long run. So these tests are left with a provisional value of 4.0.
if RuboCop::TargetRuby.supported_versions.include?(4.0)
context 'when the target ruby version >= 4.0' do
let(:ruby_version) { 4.0 }

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

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

it_behaves_like 'immutable objects', '"#{a}"'
end

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

it_behaves_like 'immutable objects', '"#{a}"'
end
end
end

context 'Ruby 3.0 or higher', :ruby30 do
context 'when the frozen string literal comment is missing' do
it_behaves_like 'mutable objects', '"#{a}"'
end

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

it_behaves_like 'mutable objects', '"#{a}"'
end

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

it_behaves_like 'mutable objects', '"#{a}"'
end
end

context 'Ruby 2.7 or lower', :ruby27 do
context 'when the frozen string literal comment is missing' do
it_behaves_like 'mutable objects', '"#{a}"'
end

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

it_behaves_like 'immutable objects', '"#{a}"'
end

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

it_behaves_like 'mutable objects', '"#{a}"'
end
end
end

context 'Strict: false' do
let(:cop_config) { { 'EnforcedStyle' => 'literals' } }

Expand Down Expand Up @@ -294,48 +357,7 @@
end
end

context 'when the constant is a frozen 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
# the long run. So these tests are left with a provisional value of 4.0.
if RuboCop::TargetRuby.supported_versions.include?(4.0)
context 'when the target ruby version >= 4.0' do
let(:ruby_version) { 4.0 }

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

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

it_behaves_like 'immutable objects', '"#{a}"'
end

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

it_behaves_like 'immutable objects', '"#{a}"'
end
end
end

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

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

it_behaves_like 'immutable objects', '"#{a}"'
end

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

it_behaves_like 'mutable objects', '"#{a}"'
end
end
it_behaves_like 'string literal'
end

context 'Strict: true' do
Expand Down Expand Up @@ -558,20 +580,6 @@ def assignment?
RUBY
end

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

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

it_behaves_like 'immutable objects', '"#{a}"'
end

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

it_behaves_like 'mutable objects', '"#{a}"'
end
it_behaves_like 'string literal'
end
end
40 changes: 30 additions & 10 deletions spec/rubocop/cop/style/redundant_freeze_spec.rb
Expand Up @@ -49,7 +49,7 @@
expect_no_offenses('TOP_TEST = Something.new.freeze')
end

context 'when the receiver is a frozen string literal' do
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
# the long run. So these tests are left with a provisional value of 4.0.
Expand All @@ -75,20 +75,40 @@
end
end

context 'when the frozen string literal comment is missing' do
it_behaves_like 'mutable objects', '"#{a}"'
end
context 'Ruby 3.0 or higher', :ruby30 do
context 'when the frozen string literal comment is missing' do
it_behaves_like 'mutable objects', '"#{a}"'
end

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

context 'when the frozen string literal comment is true' do
let(:prefix) { '# frozen_string_literal: true' }
it_behaves_like 'mutable objects', '"#{a}"'
end

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

it_behaves_like 'immutable objects', '"#{a}"'
it_behaves_like 'mutable objects', '"#{a}"'
end
end

context 'when the frozen string literal comment is false' do
let(:prefix) { '# frozen_string_literal: false' }
context 'Ruby 2.7 or lower', :ruby27 do
context 'when the frozen string literal comment is missing' do
it_behaves_like 'mutable objects', '"#{a}"'
end

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

it_behaves_like 'immutable objects', '"#{a}"'
end

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

it_behaves_like 'mutable objects', '"#{a}"'
it_behaves_like 'mutable objects', '"#{a}"'
end
end

describe 'Regexp and Range literals' do
Expand Down