From 5f5e962cc126e09597dd1516003b33c8e442d948 Mon Sep 17 00:00:00 2001 From: arjun Date: Mon, 12 Jul 2021 08:22:26 +0530 Subject: [PATCH] [Fix rubocop#9328] Honour shareable_constant_value magic comment [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 --- CHANGELOG.md | 1 + ...ecognize_shareable_constant_value_magic.md | 1 + lib/rubocop/cop/style/mutable_constant.rb | 47 +++++++++++- lib/rubocop/magic_comment.rb | 2 +- .../cop/style/mutable_constant_spec.rb | 72 +++++++++++++++++++ spec/rubocop/magic_comment_spec.rb | 35 +++++++++ 6 files changed, 155 insertions(+), 3 deletions(-) create mode 100644 changelog/change_recognize_shareable_constant_value_magic.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 557795dee8b..e1e8bba76f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5743,3 +5743,4 @@ [@AirWick219]: https://github.com/AirWick219 [@markburns]: https://github.com/markburns [@gregfletch]: https://github.com/gregfletch +[@thearjunmdas]: https://github.com/thearjunmdas diff --git a/changelog/change_recognize_shareable_constant_value_magic.md b/changelog/change_recognize_shareable_constant_value_magic.md new file mode 100644 index 00000000000..c0cccef66b5 --- /dev/null +++ b/changelog/change_recognize_shareable_constant_value_magic.md @@ -0,0 +1 @@ +* [#9328](https://github.com/rubocop/rubocop/issues/9328): Recognize shareable_constant_value magic comment. ([@thearjunmdas][]) diff --git a/lib/rubocop/cop/style/mutable_constant.rb b/lib/rubocop/cop/style/mutable_constant.rb index 7d22323c091..725c8122a50 100644 --- a/lib/rubocop/cop/style/mutable_constant.rb +++ b/lib/rubocop/cop/style/mutable_constant.rb @@ -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 @@ -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 @@ -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 diff --git a/lib/rubocop/magic_comment.rb b/lib/rubocop/magic_comment.rb index baa70679837..91f111c2289 100644 --- a/lib/rubocop/magic_comment.rb +++ b/lib/rubocop/magic_comment.rb @@ -166,7 +166,7 @@ def extract_frozen_string_literal end def extract_shareable_constant_value - match('shareable[_-]constant[_-]values') + match('shareable[_-]constant[_-]value') end end diff --git a/spec/rubocop/cop/style/mutable_constant_spec.rb b/spec/rubocop/cop/style/mutable_constant_spec.rb index 800714b1e2d..4c60c24f3ba 100644 --- a/spec/rubocop/cop/style/mutable_constant_spec.rb +++ b/spec/rubocop/cop/style/mutable_constant_spec.rb @@ -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' } } @@ -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 @@ -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 diff --git a/spec/rubocop/magic_comment_spec.rb b/spec/rubocop/magic_comment_spec.rb index 241aeafc2d0..c216562fac1 100644 --- a/spec/rubocop/magic_comment_spec.rb +++ b/spec/rubocop/magic_comment_spec.rb @@ -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) @@ -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', '#' @@ -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