Skip to content

Commit

Permalink
Fix RSpec/SortMetadata cop to limit sorting to trailing metadata
Browse files Browse the repository at this point in the history
Metadata processed by RSpec is:
- the last argument when it's a hash
- the trailing arguments when they are symbols

Only this metadata is sorted by this cop.

If the second argument to a `context`/`describe` block is used as an
additional description, it is not sorted anymore. This fixes rubocop#1946.
  • Loading branch information
cbliard committed Oct 10, 2024
1 parent a95f2f0 commit 621c304
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 21 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Master (Unreleased)

- Fix `RSpec/SortMetadata` cop to limit sorting to trailing metadata arguments. ([@cbliard])

## 3.1.0 (2024-10-01)

- Add `RSpec/StringAsInstanceDoubleConstant` to check for and correct strings used as instance_doubles. ([@corsonknowles])
Expand Down
13 changes: 5 additions & 8 deletions lib/rubocop/cop/rspec/mixin/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 22 additions & 8 deletions lib/rubocop/cop/rspec/sort_metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ module Cop
module RSpec
# Sort RSpec metadata alphabetically.
#
# Only the trailing metadata is sorted.
#
# @example
# # bad
# describe 'Something', :b, :a
Expand All @@ -16,15 +18,24 @@ module RSpec
# context 'Something', baz: true, foo: 'bar'
# it 'works', :a, :b, baz: true, foo: 'bar'
#
# # good, trailing metadata is sorted
# describe 'Something', 'description', :a, :b, :z
# context 'Something', :z, variable, :a, :b
class SortMetadata < Base
extend AutoCorrector
include Metadata
include RangeHelp

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 || []
symbols = trailing_symbols(args)
return if sorted?(symbols, pairs)

crime_scene = crime_scene(symbols, pairs)
Expand All @@ -35,6 +46,15 @@ def on_metadata(symbols, hash)

private

def trailing_symbols(args)
args = args[...-1] if last_arg_could_be_a_hash?(args)
args.reverse.take_while(&:sym_type?).reverse
end

def last_arg_could_be_a_hash?(args)
args.last && match_ambiguous_trailing_metadata?(args.last.parent)
end

def crime_scene(symbols, pairs)
metadata = symbols + pairs

Expand All @@ -57,13 +77,7 @@ def sort_pairs(pairs)
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
symbols.sort_by { |symbol| symbol.value.to_s.downcase }
end
end
end
Expand Down
58 changes: 53 additions & 5 deletions spec/rubocop/cop/rspec/sort_metadata_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,54 @@
RUBY
end

it 'does not register an offense for a symbol metadata before a non-symbol ' \
'argument' do
expect_no_offenses(<<~RUBY)
RSpec.describe 'Something', :z, :a, 'string' do
end
RSpec.describe 'Something', :z, :a, variable, foo: :bar do
end
RUBY
end

it 'registers an offense only for trailing symbol metadata not in ' \
'alphabetical order' do
expect_offense(<<~RUBY)
RSpec.describe 'Something', :z, :a, 'string', :c, :b do
^^^^^^ Sort metadata alphabetically.
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe 'Something', :z, :a, 'string', :b, :c do
end
RUBY
end

it 'registers an offense when a symbol metadata not in alphabetical order ' \
'is before a variable argument being the last argument ' \
'as it could be a hash' do
expect_offense(<<~RUBY)
RSpec.describe 'Something', :z, :a, some_hash do
^^^^^^ Sort metadata alphabetically.
end
RUBY

expect_correction(<<~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)
Expand Down Expand Up @@ -100,27 +148,27 @@
'and the hash values are complex objects' do
expect_offense(<<~RUBY)
it 'Something', variable, 'B', :a, key => {}, foo: ->(x) { bar(x) }, Identifier.sample => true, baz: Snafu.new do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sort metadata alphabetically.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sort metadata alphabetically.
end
RUBY

expect_correction(<<~RUBY)
it 'Something', :a, 'B', variable, baz: Snafu.new, foo: ->(x) { bar(x) }, Identifier.sample => true, key => {} do
it 'Something', variable, 'B', :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', :z, :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', 'B', :a, :z do |x, y|
end
include_examples 'a difficult situation', 'value', 'another value'
Expand Down

0 comments on commit 621c304

Please sign in to comment.