diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fb31d825..4ed4f3795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Master (Unreleased) +- Fix `RSpec/SortMetadata` cop to sort strings and variables before symbols in metadata arguments. ([@cbliard]) + ## 3.0.5 (2024-09-07) - Fix false-negative and error for `RSpec/MetadataStyle` when non-literal args are used in metadata in `EnforceStyle: hash`. ([@cbliard]) diff --git a/lib/rubocop/cop/rspec/mixin/metadata.rb b/lib/rubocop/cop/rspec/mixin/metadata.rb index b5dd448c1..325747433 100644 --- a/lib/rubocop/cop/rspec/mixin/metadata.rb +++ b/lib/rubocop/cop/rspec/mixin/metadata.rb @@ -47,15 +47,12 @@ def on_metadata(_symbols, _hash) private def on_metadata_arguments(metadata_arguments) - *symbols, last = metadata_arguments - hash = nil - case last&.type - when :hash - hash = last - when :sym - symbols << last + if metadata_arguments.last&.hash_type? + *metadata_arguments, hash = metadata_arguments + on_metadata(metadata_arguments, hash) + else + on_metadata(metadata_arguments, nil) end - on_metadata(symbols, hash) end end end diff --git a/lib/rubocop/cop/rspec/sort_metadata.rb b/lib/rubocop/cop/rspec/sort_metadata.rb index f34f0ade1..fc878bbe9 100644 --- a/lib/rubocop/cop/rspec/sort_metadata.rb +++ b/lib/rubocop/cop/rspec/sort_metadata.rb @@ -23,20 +23,30 @@ class SortMetadata < Base MSG = 'Sort metadata alphabetically.' - def on_metadata(symbols, hash) + # @!method match_ambiguous_trailing_metadata?(node) + def_node_matcher :match_ambiguous_trailing_metadata?, <<~PATTERN + (send _ _ _ ... !{hash sym str dstr xstr}) + PATTERN + + def on_metadata(args, hash) pairs = hash&.pairs || [] - return if sorted?(symbols, pairs) + args = args[...-1] if ambiguous_trailing_arg?(args.last) + return if sorted?(args, pairs) - crime_scene = crime_scene(symbols, pairs) + crime_scene = crime_scene(args, pairs) add_offense(crime_scene) do |corrector| - corrector.replace(crime_scene, replacement(symbols, pairs)) + corrector.replace(crime_scene, replacement(args, pairs)) end end private - def crime_scene(symbols, pairs) - metadata = symbols + pairs + def ambiguous_trailing_arg?(arg) + arg && match_ambiguous_trailing_metadata?(arg.parent) + end + + def crime_scene(args, pairs) + metadata = args + pairs range_between( metadata.first.source_range.begin_pos, @@ -44,26 +54,23 @@ def crime_scene(symbols, pairs) ) end - def replacement(symbols, pairs) - (sort_symbols(symbols) + sort_pairs(pairs)).map(&:source).join(', ') + def replacement(args, pairs) + (sort_args(args) + sort_pairs(pairs)).map(&:source).join(', ') end - def sorted?(symbols, pairs) - symbols == sort_symbols(symbols) && pairs == sort_pairs(pairs) + def sorted?(args, pairs) + args == sort_args(args) && pairs == sort_pairs(pairs) end def sort_pairs(pairs) pairs.sort_by { |pair| pair.key.source.downcase } end - def sort_symbols(symbols) - symbols.sort_by do |symbol| - if symbol.str_type? || symbol.sym_type? - symbol.value.to_s.downcase - else - symbol.source.downcase - end - end + def sort_args(args) + symbols, args = args.partition(&:sym_type?) + strings, args = args.partition(&:str_type?) + symbols.sort_by! { |symbol| symbol.value.to_s.downcase } + strings + args + symbols end end end diff --git a/spec/rubocop/cop/rspec/sort_metadata_spec.rb b/spec/rubocop/cop/rspec/sort_metadata_spec.rb index c6129a366..465140217 100644 --- a/spec/rubocop/cop/rspec/sort_metadata_spec.rb +++ b/spec/rubocop/cop/rspec/sort_metadata_spec.rb @@ -23,6 +23,50 @@ RUBY end + it 'registers an offense when a symbol metadata is before second docstring ' \ + 'argument' do + expect_offense(<<~RUBY) + RSpec.describe 'Something', :a, :z, 'second docstring' do + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Sort metadata alphabetically. + end + RUBY + + expect_correction(<<~RUBY) + RSpec.describe 'Something', 'second docstring', :a, :z do + end + RUBY + end + + it 'registers an offense when a symbol metadata is before a variable ' \ + 'argument' do + expect_offense(<<~RUBY) + RSpec.describe 'Something', :a, :z, variable, foo: :bar do + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sort metadata alphabetically. + end + RUBY + + expect_correction(<<~RUBY) + RSpec.describe 'Something', variable, :a, :z, foo: :bar do + end + RUBY + end + + it 'does not register an offense when a symbol metadata is before a ' \ + 'variable argument being the last argument as it could be a hash' do + expect_no_offenses(<<~RUBY) + RSpec.describe 'Something', :a, :z, some_hash do + end + RUBY + end + + it 'does not register an offense when using a second level description ' \ + 'not in alphabetical order with symbol metadata' do + expect_no_offenses(<<~RUBY) + RSpec.describe 'Something', 'second docstring', :a, :b do + end + RUBY + end + it 'does not register an offense when using only a hash of metadata ' \ 'with keys in alphabetical order' do expect_no_offenses(<<~RUBY) @@ -105,22 +149,22 @@ RUBY expect_correction(<<~RUBY) - it 'Something', :a, 'B', variable, baz: Snafu.new, foo: ->(x) { bar(x) }, Identifier.sample => true, key => {} do + it 'Something', 'B', variable, :a, baz: Snafu.new, foo: ->(x) { bar(x) }, Identifier.sample => true, key => {} do end RUBY end it 'registers an offense only when example or group has a block' do expect_offense(<<~RUBY) - shared_examples 'a difficult situation', 'B', :a do |x, y| - ^^^^^^^ Sort metadata alphabetically. + shared_examples 'a difficult situation', :b, :a do |x, y| + ^^^^^^ Sort metadata alphabetically. end include_examples 'a difficult situation', 'value', 'another value' RUBY expect_correction(<<~RUBY) - shared_examples 'a difficult situation', :a, 'B' do |x, y| + shared_examples 'a difficult situation', :a, :b do |x, y| end include_examples 'a difficult situation', 'value', 'another value'