diff --git a/config/default.yml b/config/default.yml index a1f307e792e..fe2be42deb7 100644 --- a/config/default.yml +++ b/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.' diff --git a/lib/rubocop/cop/lint/symbol_conversion.rb b/lib/rubocop/cop/lint/symbol_conversion.rb index c443aab8f89..3faf652c9ea 100644 --- a/lib/rubocop/cop/lint/symbol_conversion.rb +++ b/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 `%s` instead.' + MSG_CONSISTENCY = 'Symbol hash key should be quoted for consistency; ' \ + 'use `%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,7 +120,7 @@ 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 @@ -68,6 +128,10 @@ def properly_quoted?(source, value) 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 diff --git a/spec/rubocop/cop/lint/symbol_conversion_spec.rb b/spec/rubocop/cop/lint/symbol_conversion_spec.rb index 115bb130ea0..8a72240652e 100644 --- a/spec/rubocop/cop/lint/symbol_conversion_spec.rb +++ b/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