Skip to content

Commit

Permalink
[Fix rubocop#9328] Honour shareable_constant_value magic comment
Browse files Browse the repository at this point in the history
[Fix rubocop#9328] Resolve review comments

[Fix rubocop#9328] Removing nil check, refactoring spec

[Fix rubocop#9328] Tests for code next to inline comments

[Fix rubocop#9328] Move module to mutable_constant
  • Loading branch information
thearjunmdas committed Aug 21, 2021
1 parent ed7aa5c commit 4a13649
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 2 deletions.
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][])
47 changes: 45 additions & 2 deletions lib/rubocop/cop/style/mutable_constant.rb
Expand Up @@ -53,6 +53,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

include ShareableConstantValue
include FrozenStringLiteral
include ConfigurableEnforcedStyle
extend AutoCorrector
Expand Down Expand Up @@ -85,18 +122,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 +167,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']"
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

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
^^^^^^^^^ 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
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

0 comments on commit 4a13649

Please sign in to comment.