[Fix rubocop#9621] Add consistent style to Lint/SymbolConversion …

…to require all symbol keys in a hash to use the same convention.

The `consistent` style works the same as the previous default style (now called `strict`), in that symbol conversions should be explicit. However, in the case of a hash with a mix of symbol keys requiring quoting and not, this style now enforces that all keys are quoted for consistency.

config/default.yml

@@ -2063,6 +2063,10 @@ Lint/SymbolConversion:
   Description: 'Checks for unnecessary symbol conversions.'
   Enabled: pending
   VersionAdded: '1.9'
+  EnforcedStyle: strict
+  SupportedStyles:
+    - strict
+    - consistent
 
 Lint/Syntax:
   Description: 'Checks for syntax errors.'

lib/rubocop/cop/lint/symbol_conversion.rb

@@ -6,6 +6,12 @@ module Lint
       # This cop checks for uses of literal strings converted to
       # a symbol where a literal symbol could be used instead.
       #
+      # There are two possible styles for this cop.
+      # `strict` (default) will register an offense for any incorrect usage.
+      # `consistent` additionally requires hashes to use the same style for
+      # every symbol key (ie. if any symbol key needs to be quoted it requires
+      # all keys to be quoted).
+      #
       # @example
       #   # bad
       #   'string'.to_sym
@@ -21,10 +27,49 @@ module Lint
       #   :underscored_symbol
       #   :'hyphenated-string'
       #
+      # @example EnforcedStyle: strict (default)
+      #
+      #   # bad
+      #   {
+      #     'a': 1,
+      #     "b": 2,
+      #     'c-d': 3
+      #   }
+      #
+      #   # good (don't quote keys that don't require quoting)
+      #   {
+      #     a: 1,
+      #     b: 2,
+      #     'c-d': 3
+      #   }
+      #
+      # @example EnforcedStyle: consistent
+      #
+      #   # bad
+      #   {
+      #     a: 1,
+      #     'b-c': 2
+      #   }
+      #
+      #   # good (quote all keys if any need quoting)
+      #   {
+      #     'a': 1,
+      #     'b-c': 2
+      #   }
+      #
+      #   # good (no quoting required)
+      #   {
+      #     a: 1,
+      #     b: 2
+      #   }
+      #
       class SymbolConversion < Base
         extend AutoCorrector
+        include ConfigurableEnforcedStyle
 
         MSG = 'Unnecessary symbol conversion; use `%<correction>s` instead.'
+        MSG_CONSISTENCY = 'Symbol hash key should be quoted for consistency; ' \
+          'use `%<correction>s` instead.'
         RESTRICT_ON_SEND = %i[to_sym intern].freeze
 
         def on_send(node)
@@ -35,7 +80,7 @@ def on_send(node)
         end
 
         def on_sym(node)
-          return if properly_quoted?(node.source, node.value.inspect)
+          return if ignored_node?(node) || properly_quoted?(node.source, node.value.inspect)
 
           # `alias` arguments are symbols but since a symbol that requires
           # being quoted is not a valid method identifier, it can be ignored
@@ -51,6 +96,21 @@ def on_sym(node)
           register_offense(node, correction: node.value.inspect)
         end
 
+        def on_hash(node)
+          # For `EnforcedStyle: strict`, hash keys are evaluated in `on_sym`
+          return unless style == :consistent
+
+          keys = node.keys.select(&:sym_type?)
+
+          if keys.any? { |key| requires_quotes?(key) }
+            correct_inconsistent_hash_keys(keys)
+          else
+            # If there are no symbol keys requiring quoting,
+            # treat the hash like `EnforcedStyle: strict`.
+            keys.each { |key| correct_hash_key(key) }
+          end
+        end
+
         private
 
         def register_offense(node, correction:, message: format(MSG, correction: correction))
@@ -60,14 +120,18 @@ def register_offense(node, correction:, message: format(MSG, correction: correct
         end
 
         def properly_quoted?(source, value)
-          return true if !source.match?(/['"]/) || value.end_with?('=')
+          return true if style == :strict && (!source.match?(/['"]/) || value.end_with?('='))
 
           source == value ||
             # `Symbol#inspect` uses double quotes, but allow single-quoted
             # symbols to work as well.
             source.tr("'", '"') == value
         end
 
+        def requires_quotes?(sym_node)
+          sym_node.value.inspect.match?(/^:".*?"|=$/)
+        end
+
         def in_alias?(node)
           node.parent&.alias_type?
         end
@@ -97,6 +161,29 @@ def correct_hash_key(node)
             message: format(MSG, correction: "#{correction}:")
           )
         end
+
+        def correct_inconsistent_hash_keys(keys)
+          keys.each do |key|
+            ignore_node(key)
+
+            next if requires_quotes?(key)
+            next if properly_quoted?(key.source, %("#{key.value}"))
+
+            correction = "#{quote_type}#{key.value}#{quote_type}"
+            register_offense(
+              key,
+              correction: correction,
+              message: format(MSG_CONSISTENCY, correction: "#{correction}:")
+            )
+          end
+        end
+
+        def quote_type
+          # Use the `Style/StringLiterals` configuration for quoting symbols
+          return '"' unless config.for_cop('Style/StringLiterals')['Enabled']
+
+          config.for_cop('Style/StringLiterals')['EnforcedStyle'] == 'single_quotes' ? "'" : '"'
+        end
       end
     end
   end

spec/rubocop/cop/lint/symbol_conversion_spec.rb

@@ -168,4 +168,154 @@
       RUBY
     end
   end
+
+  context 'EnforcedStyle: consistent' do
+    let(:string_literals_enabled) { true }
+    let(:string_literals_style) { 'single_quotes' }
+
+    let(:cop_config) { { 'EnforcedStyle' => 'consistent' } }
+    let(:other_cops) do
+      {
+        'Style/StringLiterals' => {
+          'Enabled' => string_literals_enabled,
+          'EnforcedStyle' => string_literals_style
+        }
+      }
+    end
+
+    context 'hash where no keys need to be quoted' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          {
+            a: 1,
+            b: 2
+          }
+        RUBY
+      end
+    end
+
+    context 'hash where keys are quoted but do not need to be' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          {
+            a: 1,
+            'b': 2
+            ^^^ Unnecessary symbol conversion; use `b:` instead.
+          }
+        RUBY
+
+        expect_correction(<<~RUBY)
+          {
+            a: 1,
+            b: 2
+          }
+        RUBY
+      end
+    end
+
+    context 'hash where there are keys needing quoting' do
+      it 'registers an offense for unquoted keys' do
+        expect_offense(<<~RUBY)
+          {
+            a: 1,
+            ^ Symbol hash key should be quoted for consistency; use `'a':` instead.
+            b: 2,
+            ^ Symbol hash key should be quoted for consistency; use `'b':` instead.
+            'c': 3,
+            'd-e': 4
+          }
+        RUBY
+
+        expect_correction(<<~RUBY)
+          {
+            'a': 1,
+            'b': 2,
+            'c': 3,
+            'd-e': 4
+          }
+        RUBY
+      end
+    end
+
+    context 'with a key with =' do
+      it 'requires symbols to be quoted' do
+        expect_offense(<<~RUBY)
+          {
+            'a=': 1,
+            b: 2
+            ^ Symbol hash key should be quoted for consistency; use `'b':` instead.
+          }
+        RUBY
+
+        expect_correction(<<~RUBY)
+          {
+            'a=': 1,
+            'b': 2
+          }
+        RUBY
+      end
+    end
+
+    context 'with different quote styles' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          {
+            'a': 1,
+            "b": 2,
+            "c-d": 3,
+            'e-f': 4
+          }
+        RUBY
+      end
+    end
+
+    context 'with a mix of string and symbol keys' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          {
+            'a' => 1,
+            'b-c': 2
+          }
+        RUBY
+      end
+    end
+
+    context 'quote style' do
+      let(:source) do
+        <<~RUBY
+          { a: 1, 'b-c': 2 }
+        RUBY
+      end
+
+      context 'when Style/StringLiterals is not enabled' do
+        let(:string_literals_enabled) { false }
+
+        it 'uses double quotes to correct' do
+          expect_correction(<<~RUBY, source: source)
+            { "a": 1, 'b-c': 2 }
+          RUBY
+        end
+      end
+
+      context 'when Style/StringLiterals uses single_quotes style' do
+        let(:string_literals_style) { 'single_quotes' }
+
+        it 'uses double quotes to correct' do
+          expect_correction(<<~RUBY, source: source)
+            { 'a': 1, 'b-c': 2 }
+          RUBY
+        end
+      end
+
+      context 'when Style/StringLiterals uses double_quotes style' do
+        let(:string_literals_style) { 'double_quotes' }
+
+        it 'uses double quotes to correct' do
+          expect_correction(<<~RUBY, source: source)
+            { "a": 1, 'b-c': 2 }
+          RUBY
+        end
+      end
+    end
+  end
 end