diff --git a/changelog/change_change_lintsymbolconversion_to_only.md b/changelog/change_change_lintsymbolconversion_to_only.md new file mode 100644 index 00000000000..447c6758915 --- /dev/null +++ b/changelog/change_change_lintsymbolconversion_to_only.md @@ -0,0 +1 @@ +* [#9809](https://github.com/rubocop/rubocop/pull/9809): Change `Lint/SymbolConversion` to only quote with double quotes, since `Style/QuotedSymbols` can now correct those to the correct quotes as per configuration. ([@dvandersluis][]) diff --git a/changelog/new_add_stylequotedsymbols_to_enforce.md b/changelog/new_add_stylequotedsymbols_to_enforce.md new file mode 100644 index 00000000000..809b69f7ca7 --- /dev/null +++ b/changelog/new_add_stylequotedsymbols_to_enforce.md @@ -0,0 +1 @@ +* [#9793](https://github.com/rubocop/rubocop/issues/9793): Add `Style/QuotedSymbols` to enforce consistency in quoted symbols. ([@dvandersluis][]) diff --git a/config/default.yml b/config/default.yml index 1107290d5a6..9e79746ca22 100644 --- a/config/default.yml +++ b/config/default.yml @@ -2092,6 +2092,7 @@ Lint/SymbolConversion: Description: 'Checks for unnecessary symbol conversions.' Enabled: pending VersionAdded: '1.9' + VersionChanged: '<>' EnforcedStyle: strict SupportedStyles: - strict @@ -4186,6 +4187,16 @@ Style/Proc: VersionAdded: '0.9' VersionChanged: '0.18' +Style/QuotedSymbols: + Description: 'Use a consistent style for quoted symbols.' + Enabled: pending + VersionAdded: '<>' + EnforcedStyle: same_as_string_literals + SupportedStyles: + - same_as_string_literals + - single_quotes + - double_quotes + Style/RaiseArgs: Description: 'Checks the arguments passed to raise/fail.' StyleGuide: '#exception-class-messages' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 9e3235121d8..057fdee0f9b 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -119,6 +119,7 @@ require_relative 'rubocop/cop/mixin/statement_modifier' require_relative 'rubocop/cop/mixin/string_help' require_relative 'rubocop/cop/mixin/string_literals_help' +require_relative 'rubocop/cop/mixin/symbol_help' require_relative 'rubocop/cop/mixin/target_ruby_version' require_relative 'rubocop/cop/mixin/trailing_body' require_relative 'rubocop/cop/mixin/trailing_comma' @@ -550,6 +551,7 @@ require_relative 'rubocop/cop/style/perl_backrefs' require_relative 'rubocop/cop/style/preferred_hash_methods' require_relative 'rubocop/cop/style/proc' +require_relative 'rubocop/cop/style/quoted_symbols' require_relative 'rubocop/cop/style/raise_args' require_relative 'rubocop/cop/style/random_with_offset' require_relative 'rubocop/cop/style/redundant_argument' diff --git a/lib/rubocop/cop/lint/symbol_conversion.rb b/lib/rubocop/cop/lint/symbol_conversion.rb index b386bddc0e1..ef00d4bfd10 100644 --- a/lib/rubocop/cop/lint/symbol_conversion.rb +++ b/lib/rubocop/cop/lint/symbol_conversion.rb @@ -66,6 +66,7 @@ module Lint class SymbolConversion < Base extend AutoCorrector include ConfigurableEnforcedStyle + include SymbolHelp MSG = 'Unnecessary symbol conversion; use `%s` instead.' MSG_CONSISTENCY = 'Symbol hash key should be quoted for consistency; ' \ @@ -138,10 +139,6 @@ def in_percent_literal_array?(node) node.parent&.array_type? && node.parent&.percent_literal? end - def hash_key?(node) - node.parent&.pair_type? && node == node.parent.child_nodes.first - end - def correct_hash_key(node) # Although some operators can be converted to symbols normally # (ie. `:==`), these are not accepted as hash keys and will @@ -167,7 +164,7 @@ def correct_inconsistent_hash_keys(keys) next if requires_quotes?(key) next if properly_quoted?(key.source, %("#{key.value}")) - correction = "#{quote_type}#{key.value}#{quote_type}" + correction = %("#{key.value}") register_offense( key, correction: correction, @@ -175,13 +172,6 @@ def correct_inconsistent_hash_keys(keys) ) 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/lib/rubocop/cop/mixin/string_literals_help.rb b/lib/rubocop/cop/mixin/string_literals_help.rb index 5dc088d371e..c43c08dc176 100644 --- a/lib/rubocop/cop/mixin/string_literals_help.rb +++ b/lib/rubocop/cop/mixin/string_literals_help.rb @@ -4,12 +4,10 @@ module RuboCop module Cop # Common functionality for cops checking single/double quotes. module StringLiteralsHelp - include StringHelp - private - def wrong_quotes?(node) - src = node.source + def wrong_quotes?(src_or_node) + src = src_or_node.is_a?(RuboCop::AST::Node) ? src_or_node.source : src_or_node return false if src.start_with?('%', '?') if style == :single_quotes diff --git a/lib/rubocop/cop/mixin/symbol_help.rb b/lib/rubocop/cop/mixin/symbol_help.rb new file mode 100644 index 00000000000..2ab5a7d0a0c --- /dev/null +++ b/lib/rubocop/cop/mixin/symbol_help.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Classes that include this module just implement functions for working + # with symbol nodes. + module SymbolHelp + def hash_key?(node) + node.parent&.pair_type? && node == node.parent.child_nodes.first + end + end + end +end diff --git a/lib/rubocop/cop/style/quoted_symbols.rb b/lib/rubocop/cop/style/quoted_symbols.rb new file mode 100644 index 00000000000..e1f196e2a7c --- /dev/null +++ b/lib/rubocop/cop/style/quoted_symbols.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # Checks if the quotes used for quoted symbols match the configured defaults. + # By default uses the same configuration as `Style/StringLiterals`. + # + # String interpolation is always kept in double quotes. + # + # Note: `Lint/SymbolConversion` can be used in parallel to ensure that symbols + # are not quoted that don't need to be. This cop is for configuring the quoting + # style to use for symbols that require quotes. + # + # @example EnforcedStyle: same_as_string_literals (default) / single_quotes + # # bad + # :"abc-def" + # + # # good + # :'abc-def' + # :"#{str}" + # :"a\'b" + # + # @example EnforcedStyle: double_quotes + # # bad + # :'abc-def' + # + # # good + # :"abc-def" + # :"#{str}" + # :"a\'b" + class QuotedSymbols < Base + include ConfigurableEnforcedStyle + include SymbolHelp + include StringLiteralsHelp + extend AutoCorrector + + MSG_SINGLE = "Prefer single-quoted symbols when you don't need string interpolation " \ + 'or special symbols.' + MSG_DOUBLE = 'Prefer double-quoted symbols unless you need single quotes to ' \ + 'avoid extra backslashes for escaping.' + + def on_sym(node) + return unless quoted?(node) + + message = style == :single_quotes ? MSG_SINGLE : MSG_DOUBLE + + if wrong_quotes?(node) + add_offense(node, message: message) do |corrector| + opposite_style_detected + autocorrect(corrector, node) + end + else + correct_style_detected + end + end + + private + + def autocorrect(corrector, node) + str = if hash_key?(node) + # strip quotes + correct_quotes(node.source[1..-2]) + else + # strip leading `:` and quotes + ":#{correct_quotes(node.source[2..-2])}" + end + + corrector.replace(node, str) + end + + def correct_quotes(str) + if style == :single_quotes + to_string_literal(str) + else + str.inspect + end + end + + def style + return super unless super == :same_as_string_literals + + string_literals_config = config.for_cop('Style/StringLiterals') + return :single_quotes unless string_literals_config['Enabled'] + + string_literals_config['EnforcedStyle'].to_sym + end + + def alternative_style + (supported_styles - [style, :same_as_string_literals]).first + end + + def quoted?(sym_node) + sym_node.source.match?(/\A:?(['"]).*?\1\z/m) + end + + def wrong_quotes?(node) + return super if hash_key?(node) + + super(node.source[1..-1]) + end + end + end + end +end diff --git a/lib/rubocop/cop/style/string_literals.rb b/lib/rubocop/cop/style/string_literals.rb index 5cee8d8e19c..46ff41b5fa9 100644 --- a/lib/rubocop/cop/style/string_literals.rb +++ b/lib/rubocop/cop/style/string_literals.rb @@ -29,6 +29,7 @@ module Style class StringLiterals < Base include ConfigurableEnforcedStyle include StringLiteralsHelp + include StringHelp extend AutoCorrector MSG_INCONSISTENT = 'Inconsistent quote style.' diff --git a/lib/rubocop/cop/style/string_literals_in_interpolation.rb b/lib/rubocop/cop/style/string_literals_in_interpolation.rb index cd4193e6352..6b01b72549e 100644 --- a/lib/rubocop/cop/style/string_literals_in_interpolation.rb +++ b/lib/rubocop/cop/style/string_literals_in_interpolation.rb @@ -22,6 +22,7 @@ module Style class StringLiteralsInInterpolation < Base include ConfigurableEnforcedStyle include StringLiteralsHelp + include StringHelp extend AutoCorrector def autocorrect(corrector, node) diff --git a/spec/rubocop/cli/autocorrect_spec.rb b/spec/rubocop/cli/autocorrect_spec.rb index ca63408999d..df37422af5d 100644 --- a/spec/rubocop/cli/autocorrect_spec.rb +++ b/spec/rubocop/cli/autocorrect_spec.rb @@ -2051,4 +2051,33 @@ def my_func end RUBY end + + it 'consistently quotes symbol keys in a hash using `Lint/SymbolConversion` ' \ + 'with `EnforcedStyle: consistent` and `Style/QuotedSymbols`' do + source_file = Pathname('example.rb') + create_file(source_file, <<~RUBY) + { + a: 1, + b: 2, + 'c-d': 3 + } + RUBY + + create_file('.rubocop.yml', <<~YAML) + Lint/SymbolConversion: + EnforcedStyle: consistent + Style/QuotedSymbols: + EnforcedStyle: double_quotes + YAML + + status = cli.run(['--auto-correct', '--only', 'Lint/SymbolConversion,Style/QuotedSymbols']) + expect(status).to eq(0) + expect(source_file.read).to eq(<<~RUBY) + { + "a": 1, + "b": 2, + "c-d": 3 + } + RUBY + end end diff --git a/spec/rubocop/cop/lint/symbol_conversion_spec.rb b/spec/rubocop/cop/lint/symbol_conversion_spec.rb index 8a72240652e..810d985c11a 100644 --- a/spec/rubocop/cop/lint/symbol_conversion_spec.rb +++ b/spec/rubocop/cop/lint/symbol_conversion_spec.rb @@ -170,18 +170,7 @@ 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 @@ -218,9 +207,9 @@ expect_offense(<<~RUBY) { a: 1, - ^ Symbol hash key should be quoted for consistency; use `'a':` instead. + ^ Symbol hash key should be quoted for consistency; use `"a":` instead. b: 2, - ^ Symbol hash key should be quoted for consistency; use `'b':` instead. + ^ Symbol hash key should be quoted for consistency; use `"b":` instead. 'c': 3, 'd-e': 4 } @@ -228,8 +217,8 @@ expect_correction(<<~RUBY) { - 'a': 1, - 'b': 2, + "a": 1, + "b": 2, 'c': 3, 'd-e': 4 } @@ -243,14 +232,14 @@ { 'a=': 1, b: 2 - ^ Symbol hash key should be quoted for consistency; use `'b':` instead. + ^ Symbol hash key should be quoted for consistency; use `"b":` instead. } RUBY expect_correction(<<~RUBY) { 'a=': 1, - 'b': 2 + "b": 2 } RUBY end @@ -279,43 +268,5 @@ 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 diff --git a/spec/rubocop/cop/style/quoted_symbols_spec.rb b/spec/rubocop/cop/style/quoted_symbols_spec.rb new file mode 100644 index 00000000000..e6089267af3 --- /dev/null +++ b/spec/rubocop/cop/style/quoted_symbols_spec.rb @@ -0,0 +1,231 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::QuotedSymbols, :config do + shared_examples_for 'enforce single quotes' do + it 'accepts unquoted symbols' do + expect_no_offenses(<<~RUBY) + :a + RUBY + end + + it 'accepts single quotes' do + expect_no_offenses(<<~RUBY) + :'a' + RUBY + end + + it 'accepts double quotes with interpolation' do + expect_no_offenses(<<~'RUBY') + :"#{a}" + RUBY + end + + it 'accepts double quotes when interpolating an instance variable' do + expect_no_offenses(<<~'RUBY') + :"#@test" + RUBY + end + + it 'accepts double quotes when interpolating a global variable' do + expect_no_offenses(<<~'RUBY') + :"#$test" + RUBY + end + + it 'accepts double quotes when interpolating a class variable' do + expect_no_offenses(<<~'RUBY') + :"#@@test" + RUBY + end + + it 'accepts double quotes with escape sequences' do + expect_no_offenses(<<~RUBY) + :"a\nb" + RUBY + end + + it 'accepts single quotes with double quotes' do + expect_no_offenses(<<~RUBY) + :'"' + RUBY + end + + it 'accepts double quotes with single quotes' do + expect_no_offenses(<<~RUBY) + :"'" + RUBY + end + + it 'accepts single quotes with line breaks' do + expect_no_offenses(<<~RUBY) + :'a + bc' + RUBY + end + + it 'accepts double quotes with line breaks' do + expect_no_offenses(<<~RUBY) + :"a + bc" + RUBY + end + + it 'accepts double quotes when control characters are used' do + expect_no_offenses(<<~'RUBY') + :"\e" + RUBY + end + + it 'accepts double quotes when unicode control sequence is used' do + expect_no_offenses(<<~'RUBY') + :"Espa\u00f1a" + RUBY + end + + it 'accepts double quotes with some other special symbols' do + # "Substitutions in double-quoted symbols" + # http://www.ruby-doc.org/docs/ProgrammingRuby/html/language.html + expect_no_offenses(<<~'RUBY') + g = :"\x3D" + copyright = :"\u00A9" + RUBY + end + + it 'registers an offense and corrects for double quotes without interpolation' do + expect_offense(<<~RUBY) + :"a" + ^^^^ Prefer single-quoted symbols when you don't need string interpolation or special symbols. + RUBY + + expect_correction(<<~RUBY) + :'a' + RUBY + end + + it 'registers an offense and corrects for double quotes in hash keys' do + expect_offense(<<~RUBY) + { "a": value } + ^^^ Prefer single-quoted symbols when you don't need string interpolation or special symbols. + RUBY + + expect_correction(<<~RUBY) + { 'a': value } + RUBY + end + end + + shared_examples_for 'enforce double quotes' do + it 'accepts unquoted symbols' do + expect_no_offenses(<<~RUBY) + :a + RUBY + end + + it 'accepts double quotes' do + expect_no_offenses(<<~RUBY) + :"a" + RUBY + end + + it 'accepts double quotes with interpolation' do + expect_no_offenses(<<~'RUBY') + :"#{a}" + RUBY + end + + it 'accepts double quotes when interpolating an instance variable' do + expect_no_offenses(<<~'RUBY') + :"#@test" + RUBY + end + + it 'accepts double quotes when interpolating a global variable' do + expect_no_offenses(<<~'RUBY') + :"#$test" + RUBY + end + + it 'accepts double quotes when interpolating a class variable' do + expect_no_offenses(<<~'RUBY') + :"#@@test" + RUBY + end + + it 'accepts double quotes with escape sequences' do + expect_no_offenses(<<~RUBY) + :"a\nb" + RUBY + end + + it 'accepts single quotes with double quotes' do + expect_no_offenses(<<~RUBY) + :'"' + RUBY + end + + it 'accepts double quotes with single quotes' do + expect_no_offenses(<<~RUBY) + :"'" + RUBY + end + + it 'accepts single quotes with line breaks' do + expect_no_offenses(<<~RUBY) + :'a + bc' + RUBY + end + + it 'accepts double quotes with line breaks' do + expect_no_offenses(<<~RUBY) + :'a + bc' + RUBY + end + + it 'registers an offense for single quotes' do + expect_offense(<<~RUBY) + :'a' + ^^^^ Prefer double-quoted symbols unless you need single quotes to avoid extra backslashes for escaping. + RUBY + + expect_correction(<<~RUBY) + :"a" + RUBY + end + end + + context 'configured with `same_as_string_literals`' do + let(:cop_config) { { 'EnforcedStyle' => 'same_as_string_literals' } } + + context 'when Style/StringLiterals is configured with single_quotes' do + let(:other_cops) { { 'Style/StringLiterals' => { 'EnforcedStyle' => 'single_quotes' } } } + + it_behaves_like 'enforce single quotes' + end + + context 'when Style/StringLiterals is configured with double_quotes' do + let(:other_cops) { { 'Style/StringLiterals' => { 'EnforcedStyle' => 'double_quotes' } } } + + it_behaves_like 'enforce double quotes' + end + + context 'when Style/StringLiterals is disabled' do + let(:other_cops) { { 'Style/StringLiterals' => { 'Enabled' => false } } } + + it_behaves_like 'enforce single quotes' + end + end + + context 'configured with `single_quotes`' do + let(:cop_config) { { 'EnforcedStyle' => 'single_quotes' } } + + it_behaves_like 'enforce single quotes' + end + + context 'configured with `double_quotes`' do + let(:cop_config) { { 'EnforcedStyle' => 'double_quotes' } } + + it_behaves_like 'enforce double quotes' + end +end