Fix RSpec/SortMetadata cop to sort strings and variables first

Fixes #1946.

Symbols in metadata are processed by RSpec only when they are positioned
last, meaning the other parameter types must be positioned before the
symbols. RSpec `context`/`describe` accepts second non-symbol argument
as an additional description which is why strings are sorted first.

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])

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

lib/rubocop/cop/rspec/sort_metadata.rb

@@ -23,47 +23,54 @@ 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,
             metadata.last.source_range.end_pos
           )
         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

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'