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

[Fix rubocop#9328] Honour shareable_constant_value magic comment #9929

Merged
merged 3 commits into from Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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.md
Expand Up @@ -5812,3 +5812,4 @@
[@AirWick219]: https://github.com/AirWick219
[@markburns]: https://github.com/markburns
[@gregfletch]: https://github.com/gregfletch
[@thearjunmdas]: https://github.com/thearjunmdas
@@ -0,0 +1 @@
* [#9328](https://github.com/rubocop/rubocop/issues/9328): Recognize shareable_constant_value magic comment. ([@thearjunmdas][], [@caalberts][])
52 changes: 50 additions & 2 deletions lib/rubocop/cop/style/mutable_constant.rb
Expand Up @@ -14,6 +14,11 @@ module Style
# positives. Luckily, there is no harm in freezing an already
# frozen object.
#
# From Ruby 3.0, this cop honours the magic comment
thearjunmdas marked this conversation as resolved.
Show resolved Hide resolved
# 'shareable_constant_value'. When this magic comment is set to any
# acceptable value other than none, it will suppress the offenses
# raised by this cop. It enforces frozen state.
#
# NOTE: Regexp and Range literals are frozen objects since Ruby 3.0.
#
# @example EnforcedStyle: literals (default)
Expand Down Expand Up @@ -53,6 +58,43 @@ module Style
# end
# end.freeze
class MutableConstant < Base
# Handles magic comment shareable_constant_value with O(n ^ 2) complexity
# n - number of lines in the source
# Iterates over all lines before a CONSTANT
# until it reaches shareable_constant_value
module ShareableConstantValue
module_function

def recent_shareable_value?(node)
shareable_constant_comment = magic_comment_in_scope node
return false if shareable_constant_comment.nil?

shareable_constant_value = MagicComment.parse(shareable_constant_comment)
.shareable_constant_value
shareable_constant_value_enabled? shareable_constant_value
end

# Identifies the most recent magic comment with valid shareable constant values
# thats in scope for this node
def magic_comment_in_scope(node)
processed_source_till_node(node).reverse_each.find do |line|
MagicComment.parse(line).valid_shareable_constant_value?
end
end

private

def processed_source_till_node(node)
processed_source.lines[0..(node.last_line - 1)]
end

def shareable_constant_value_enabled?(value)
%w[literal experimental_everything experimental_copy].include? value
end
end
private_constant :ShareableConstantValue
marcandre marked this conversation as resolved.
Show resolved Hide resolved

include ShareableConstantValue
include FrozenStringLiteral
include ConfigurableEnforcedStyle
extend AutoCorrector
Expand Down Expand Up @@ -85,18 +127,18 @@ def strict_check(value)
return if immutable_literal?(value)
return if operation_produces_immutable_object?(value)
return if frozen_string_literal?(value)
return if shareable_constant_value?(value)

add_offense(value) { |corrector| autocorrect(corrector, value) }
end

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 shareable_constant_value?(value)

add_offense(value) { |corrector| autocorrect(corrector, value) }
end
Expand Down Expand Up @@ -130,6 +172,12 @@ 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

recent_shareable_value? node
end

def frozen_regexp_or_range_literals?(node)
target_ruby_version >= 3.0 && (node.regexp_type? || node.range_type?)
end
Expand Down
72 changes: 72 additions & 0 deletions spec/rubocop/cop/style/mutable_constant_spec.rb
Expand Up @@ -43,6 +43,43 @@
end
end

shared_examples 'literals that are frozen' do |o|
let(:prefix) { o }

it_behaves_like 'immutable objects', '[1, 2, 3]'
it_behaves_like 'immutable objects', '%w(a b c)'
it_behaves_like 'immutable objects', '{ a: 1, b: 2 }'
it_behaves_like 'immutable objects', "'str'"
it_behaves_like 'immutable objects', '"top#{1 + 2}"'
it_behaves_like 'immutable objects', '1'
it_behaves_like 'immutable objects', '1.5'
it_behaves_like 'immutable objects', ':sym'
it_behaves_like 'immutable objects', 'FOO + BAR'
it_behaves_like 'immutable objects', 'FOO - BAR'
it_behaves_like 'immutable objects', "'foo' + 'bar'"
it_behaves_like 'immutable objects', "ENV['foo']"
it_behaves_like 'immutable objects', "::ENV['foo']"
marcandre marked this conversation as resolved.
Show resolved Hide resolved
end

shared_examples 'literals that are not frozen' do |o|
let(:prefix) { o }

it_behaves_like 'mutable objects', '[1, 2, 3]'
it_behaves_like 'mutable objects', '%w(a b c)'
it_behaves_like 'mutable objects', '{ a: 1, b: 2 }'
it_behaves_like 'mutable objects', "'str'"
it_behaves_like 'mutable objects', '"top#{1 + 2}"'

it_behaves_like 'immutable objects', '1'
it_behaves_like 'immutable objects', '1.5'
it_behaves_like 'immutable objects', ':sym'
it_behaves_like 'immutable objects', 'FOO + BAR'
it_behaves_like 'immutable objects', 'FOO - BAR'
it_behaves_like 'immutable objects', "'foo' + 'bar'"
it_behaves_like 'immutable objects', "ENV['foo']"
it_behaves_like 'immutable objects', "::ENV['foo']"
end

thearjunmdas marked this conversation as resolved.
Show resolved Hide resolved
context 'Strict: false' do
let(:cop_config) { { 'EnforcedStyle' => 'literals' } }

Expand Down Expand Up @@ -157,6 +194,34 @@
RUBY
end
end

context 'when using shareable_constant_value' do
it_behaves_like 'literals that are frozen', '# shareable_constant_value: literal'
it_behaves_like 'literals that are frozen', '# shareable_constant_value: experimental_everything'
it_behaves_like 'literals that are frozen', '# shareable_constant_value: experimental_copy'
it_behaves_like 'literals that are not frozen', '# shareable_constant_value: none'
end

it 'raises offense when shareable_constant_value is specified as an inline comment' do
expect_offense(<<~RUBY)
X = [1, 2, 3] # shareable_constant_value: literal
Copy link
Collaborator

Choose a reason for hiding this comment

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

At some point we'll need a lint cop to check for this. :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes, sense. Since it's an experimental feature, maybe we need not do it right away.

^^^^^^^^^ Freeze mutable objects assigned to constants.
Y = [4, 5, 6]
^^^^^^^^^ Freeze mutable objects assigned to constants.
RUBY
end

it 'raises offense only for shareable_constant_value as none when set in the order of: literal, none and experimental_everything' do
expect_offense(<<~RUBY)
# shareable_constant_value: literal
X = [1, 2, 3]
# shareable_constant_value: none
Y = [4, 5, 6]
^^^^^^^^^ Freeze mutable objects assigned to constants.
# shareable_constant_value: experimental_everything
Z = [7, 8, 9]
RUBY
thearjunmdas marked this conversation as resolved.
Show resolved Hide resolved
end
end

context 'Ruby 2.7 or lower', :ruby27 do
Expand Down Expand Up @@ -220,6 +285,13 @@
RUBY
end
end

context 'when using shareable_constant_values' do
it_behaves_like 'literals that are not frozen', '# shareable_constant_value: literal'
it_behaves_like 'literals that are not frozen', '# shareable_constant_value: experimental_everything'
it_behaves_like 'literals that are not frozen', '# shareable_constant_value: experimental_copy'
it_behaves_like 'literals that are not frozen', '# shareable_constant_value: none'
end
end

context 'when the constant is a frozen string literal' do
Expand Down
35 changes: 35 additions & 0 deletions spec/rubocop/magic_comment_spec.rb
Expand Up @@ -4,6 +4,7 @@
shared_examples 'magic comment' do |comment, expectations = {}|
encoding = expectations[:encoding]
frozen_string = expectations[:frozen_string_literal]
shareable_constant_value = expectations[:shareable_constant_value]

it "returns #{encoding.inspect} for encoding when comment is #{comment}" do
expect(described_class.parse(comment).encoding).to eql(encoding)
Expand All @@ -12,6 +13,12 @@
it "returns #{frozen_string.inspect} for frozen_string_literal when comment is #{comment}" do
expect(described_class.parse(comment).frozen_string_literal).to eql(frozen_string)
end

it "returns #{shareable_constant_value.inspect} for shareable_constant_value " \
"when comment is #{comment}" do
expect(described_class.parse(comment)
.shareable_constant_value).to eql(shareable_constant_value)
end
end

include_examples 'magic comment', '#'
Expand Down Expand Up @@ -46,6 +53,34 @@

include_examples 'magic comment', '# fRoZeN-sTrInG_lItErAl: true', frozen_string_literal: true

include_examples 'magic comment', '# shareable_constant_value: literal', shareable_constant_value: 'literal'

include_examples 'magic comment', '# shareable_constant_value:literal', shareable_constant_value: 'literal'

include_examples 'magic comment', '# shareable-constant-value: literal', shareable_constant_value: 'literal'

include_examples 'magic comment', '# SHAREABLE-CONSTANT-VALUE: literal', shareable_constant_value: 'literal'

include_examples 'magic comment', '# sHaReaBLE-CoNstANT-ValUE: literal', shareable_constant_value: 'literal'

include_examples 'magic comment', '# shareable_constant_value: none', shareable_constant_value: 'none'

include_examples 'magic comment', '# xyz shareable_constant_value: literal'

include_examples 'magic comment', '# xyz shareable_constant_value: literal xyz'

include_examples(
'magic comment',
'# shareable_constant_value: experimental_everything',
shareable_constant_value: 'experimental_everything'
)

include_examples(
'magic comment',
'# shareable_constant_value: experimental_copy',
shareable_constant_value: 'experimental_copy'
)

include_examples 'magic comment',
'# -*- frozen-string-literal: true -*-',
frozen_string_literal: true
Expand Down