From 4a13649905603c08175992108c8136f03e481ba3 Mon Sep 17 00:00:00 2001 From: arjun Date: Mon, 12 Jul 2021 08:22:26 +0530 Subject: [PATCH 1/3] [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 +++++++++++- .../cop/style/mutable_constant_spec.rb | 72 +++++++++++++++++++ spec/rubocop/magic_comment_spec.rb | 35 +++++++++ 5 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 changelog/change_recognize_shareable_constant_value_magic.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 6821acb06ad..481ad8e5c9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5812,3 +5812,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/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 8ce7afa3d1e..567cf544bf3 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 From a89c08ba3a7242a10e4c9f9acd26d529511c2a5f Mon Sep 17 00:00:00 2001 From: arjun Date: Sun, 22 Aug 2021 06:59:03 +0530 Subject: [PATCH 2/3] [Fix rubocop#9328] Add co-author, simple documentation --- changelog/change_recognize_shareable_constant_value_magic.md | 2 +- lib/rubocop/cop/style/mutable_constant.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/changelog/change_recognize_shareable_constant_value_magic.md b/changelog/change_recognize_shareable_constant_value_magic.md index c0cccef66b5..eed14490de6 100644 --- a/changelog/change_recognize_shareable_constant_value_magic.md +++ b/changelog/change_recognize_shareable_constant_value_magic.md @@ -1 +1 @@ -* [#9328](https://github.com/rubocop/rubocop/issues/9328): Recognize shareable_constant_value magic comment. ([@thearjunmdas][]) +* [#9328](https://github.com/rubocop/rubocop/issues/9328): Recognize shareable_constant_value magic comment. ([@thearjunmdas][], [@caalberts][]) diff --git a/lib/rubocop/cop/style/mutable_constant.rb b/lib/rubocop/cop/style/mutable_constant.rb index 725c8122a50..cba0d859a2e 100644 --- a/lib/rubocop/cop/style/mutable_constant.rb +++ b/lib/rubocop/cop/style/mutable_constant.rb @@ -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 + # '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) From e5258d44d3c29a13f3ba2399e506145ca820ce05 Mon Sep 17 00:00:00 2001 From: arjun Date: Sun, 22 Aug 2021 12:34:07 +0530 Subject: [PATCH 3/3] [Fix rubocop#9328] Added some examples to the cop --- lib/rubocop/cop/style/mutable_constant.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/rubocop/cop/style/mutable_constant.rb b/lib/rubocop/cop/style/mutable_constant.rb index cba0d859a2e..a950db84879 100644 --- a/lib/rubocop/cop/style/mutable_constant.rb +++ b/lib/rubocop/cop/style/mutable_constant.rb @@ -57,6 +57,21 @@ module Style # puts 1 # end # end.freeze + # + # @example + # # Magic comment - shareable_constant_value: literal + # + # # bad + # CONST = [1, 2, 3] + # + # # good + # # shareable_constant_value: literal + # CONST = [1, 2, 3] + # + # NOTE: This special directive helps to create constants + # that hold only immutable objects, or Ractor-shareable + # constants. - ruby docs + # class MutableConstant < Base # Handles magic comment shareable_constant_value with O(n ^ 2) complexity # n - number of lines in the source