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 #1946.

Co-authored-by: Phil Pirozhkov <pirj@users.noreply.github.com>
  • Loading branch information
cbliard and pirj committed Jan 9, 2025
1 parent 162d91a commit c2e861f
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 30 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.3.0 (2024-12-12)

- Deprecate `top_level_group?` method from `TopLevelGroup` mixin as all of its callers were intentionally removed from `Rubocop/RSpec`. ([@corsonknowles])
Expand Down
6 changes: 6 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -5735,6 +5735,8 @@ end
Sort RSpec metadata alphabetically.
Only the trailing metadata is sorted.
[#examples-rspecsortmetadata]
=== Examples
Expand All @@ -5749,6 +5751,10 @@ it 'works', :b, :a, foo: 'bar', baz: true
describe 'Something', :a, :b
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
----
[#references-rspecsortmetadata]
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
4 changes: 2 additions & 2 deletions lib/rubocop/rspec/description_extractor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ def documented_constant
end

def cop_subclass?
yardoc.superclass.path == RSPEC_COP_CLASS_NAME ||
yardoc.superclass.path == RUBOCOP_COP_CLASS_NAME
[RSPEC_COP_CLASS_NAME,
RUBOCOP_COP_CLASS_NAME].include?(yardoc.superclass.path)
end

def abstract?
Expand Down
83 changes: 71 additions & 12 deletions spec/rubocop/cop/rspec/sort_metadata_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,80 @@
it 'does not register an offense when using only symbol metadata ' \
'in alphabetical order' do
expect_no_offenses(<<~RUBY)
RSpec.describe 'Something', :a, :b do
describe 'Something', :a, :b do
end
RUBY
end

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

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

it 'does not register an offense for a symbol metadata before a non-symbol ' \
'argument' do
expect_no_offenses(<<~RUBY)
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)
describe 'Something', :z, :a, variable, :c, :b do
^^^^^^ Sort metadata alphabetically.
end
RUBY

expect_correction(<<~RUBY)
describe 'Something', :z, :a, variable, :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)
describe 'Something', :z, :a, some_hash do
^^^^^^ Sort metadata alphabetically.
end
RUBY

expect_correction(<<~RUBY)
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)
describe 'Something', 'second docstring', :a, :b do
end
RUBY
end

it 'registers an offense when using a second level description ' \
'and metadata not in alphabetical order' do
expect_offense(<<~RUBY)
describe 'Something', 'second docstring', :b, :a do
^^^^^^ Sort metadata alphabetically.
end
RUBY

expect_correction(<<~RUBY)
describe 'Something', 'second docstring', :a, :b do
end
RUBY
end
Expand Down Expand Up @@ -100,27 +159,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 All @@ -129,14 +188,14 @@

it 'registers an offense also when the metadata is not on one single line' do
expect_offense(<<~RUBY)
RSpec.describe 'Something', :foo, :bar,
^^^^^^^^^^^ Sort metadata alphabetically.
describe 'Something', :foo, :bar,
^^^^^^^^^^^ Sort metadata alphabetically.
baz: 'goo' do
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe 'Something', :bar, :foo, baz: 'goo' do
describe 'Something', :bar, :foo, baz: 'goo' do
end
RUBY
end
Expand Down

0 comments on commit c2e861f

Please sign in to comment.