Skip to content

Commit c2e861f

Browse files
cbliardpirj
cbliard
and
pirj
committed
Fix RSpec/SortMetadata cop to limit sorting to trailing metadata
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>
1 parent 162d91a commit c2e861f

File tree

6 files changed

+108
-30
lines changed

6 files changed

+108
-30
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## Master (Unreleased)
44

5+
- Fix `RSpec/SortMetadata` cop to limit sorting to trailing metadata arguments. ([@cbliard])
6+
57
## 3.3.0 (2024-12-12)
68

79
- Deprecate `top_level_group?` method from `TopLevelGroup` mixin as all of its callers were intentionally removed from `Rubocop/RSpec`. ([@corsonknowles])

‎docs/modules/ROOT/pages/cops_rspec.adoc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5735,6 +5735,8 @@ end
57355735
57365736
Sort RSpec metadata alphabetically.
57375737
5738+
Only the trailing metadata is sorted.
5739+
57385740
[#examples-rspecsortmetadata]
57395741
=== Examples
57405742
@@ -5749,6 +5751,10 @@ it 'works', :b, :a, foo: 'bar', baz: true
57495751
describe 'Something', :a, :b
57505752
context 'Something', baz: true, foo: 'bar'
57515753
it 'works', :a, :b, baz: true, foo: 'bar'
5754+
5755+
# good, trailing metadata is sorted
5756+
describe 'Something', 'description', :a, :b, :z
5757+
context 'Something', :z, variable, :a, :b
57525758
----
57535759
57545760
[#references-rspecsortmetadata]

‎lib/rubocop/cop/rspec/mixin/metadata.rb

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,12 @@ def on_metadata(_symbols, _hash)
4747
private
4848

4949
def on_metadata_arguments(metadata_arguments)
50-
*symbols, last = metadata_arguments
51-
hash = nil
52-
case last&.type
53-
when :hash
54-
hash = last
55-
when :sym
56-
symbols << last
50+
if metadata_arguments.last&.hash_type?
51+
*metadata_arguments, hash = metadata_arguments
52+
on_metadata(metadata_arguments, hash)
53+
else
54+
on_metadata(metadata_arguments, nil)
5755
end
58-
on_metadata(symbols, hash)
5956
end
6057
end
6158
end

‎lib/rubocop/cop/rspec/sort_metadata.rb

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ module Cop
55
module RSpec
66
# Sort RSpec metadata alphabetically.
77
#
8+
# Only the trailing metadata is sorted.
9+
#
810
# @example
911
# # bad
1012
# describe 'Something', :b, :a
@@ -16,15 +18,24 @@ module RSpec
1618
# context 'Something', baz: true, foo: 'bar'
1719
# it 'works', :a, :b, baz: true, foo: 'bar'
1820
#
21+
# # good, trailing metadata is sorted
22+
# describe 'Something', 'description', :a, :b, :z
23+
# context 'Something', :z, variable, :a, :b
1924
class SortMetadata < Base
2025
extend AutoCorrector
2126
include Metadata
2227
include RangeHelp
2328

2429
MSG = 'Sort metadata alphabetically.'
2530

26-
def on_metadata(symbols, hash)
31+
# @!method match_ambiguous_trailing_metadata?(node)
32+
def_node_matcher :match_ambiguous_trailing_metadata?, <<~PATTERN
33+
(send _ _ _ ... !{hash sym str dstr xstr})
34+
PATTERN
35+
36+
def on_metadata(args, hash)
2737
pairs = hash&.pairs || []
38+
symbols = trailing_symbols(args)
2839
return if sorted?(symbols, pairs)
2940

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

3647
private
3748

49+
def trailing_symbols(args)
50+
args = args[...-1] if last_arg_could_be_a_hash?(args)
51+
args.reverse.take_while(&:sym_type?).reverse
52+
end
53+
54+
def last_arg_could_be_a_hash?(args)
55+
args.last && match_ambiguous_trailing_metadata?(args.last.parent)
56+
end
57+
3858
def crime_scene(symbols, pairs)
3959
metadata = symbols + pairs
4060

@@ -57,13 +77,7 @@ def sort_pairs(pairs)
5777
end
5878

5979
def sort_symbols(symbols)
60-
symbols.sort_by do |symbol|
61-
if symbol.str_type? || symbol.sym_type?
62-
symbol.value.to_s.downcase
63-
else
64-
symbol.source.downcase
65-
end
66-
end
80+
symbols.sort_by { |symbol| symbol.value.to_s.downcase }
6781
end
6882
end
6983
end

‎lib/rubocop/rspec/description_extractor.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ def documented_constant
6262
end
6363

6464
def cop_subclass?
65-
yardoc.superclass.path == RSPEC_COP_CLASS_NAME ||
66-
yardoc.superclass.path == RUBOCOP_COP_CLASS_NAME
65+
[RSPEC_COP_CLASS_NAME,
66+
RUBOCOP_COP_CLASS_NAME].include?(yardoc.superclass.path)
6767
end
6868

6969
def abstract?

‎spec/rubocop/cop/rspec/sort_metadata_spec.rb

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,80 @@
44
it 'does not register an offense when using only symbol metadata ' \
55
'in alphabetical order' do
66
expect_no_offenses(<<~RUBY)
7-
RSpec.describe 'Something', :a, :b do
7+
describe 'Something', :a, :b do
88
end
99
RUBY
1010
end
1111

1212
it 'registers an offense when using only symbol metadata, ' \
1313
'but not in alphabetical order' do
1414
expect_offense(<<~RUBY)
15-
RSpec.describe 'Something', :b, :a do
16-
^^^^^^ Sort metadata alphabetically.
15+
describe 'Something', :b, :a do
16+
^^^^^^ Sort metadata alphabetically.
1717
end
1818
RUBY
1919

2020
expect_correction(<<~RUBY)
21-
RSpec.describe 'Something', :a, :b do
21+
describe 'Something', :a, :b do
22+
end
23+
RUBY
24+
end
25+
26+
it 'does not register an offense for a symbol metadata before a non-symbol ' \
27+
'argument' do
28+
expect_no_offenses(<<~RUBY)
29+
describe 'Something', :z, :a, variable, foo: :bar do
30+
end
31+
RUBY
32+
end
33+
34+
it 'registers an offense only for trailing symbol metadata not in ' \
35+
'alphabetical order' do
36+
expect_offense(<<~RUBY)
37+
describe 'Something', :z, :a, variable, :c, :b do
38+
^^^^^^ Sort metadata alphabetically.
39+
end
40+
RUBY
41+
42+
expect_correction(<<~RUBY)
43+
describe 'Something', :z, :a, variable, :b, :c do
44+
end
45+
RUBY
46+
end
47+
48+
it 'registers an offense when a symbol metadata not in alphabetical order ' \
49+
'is before a variable argument being the last argument ' \
50+
'as it could be a hash' do
51+
expect_offense(<<~RUBY)
52+
describe 'Something', :z, :a, some_hash do
53+
^^^^^^ Sort metadata alphabetically.
54+
end
55+
RUBY
56+
57+
expect_correction(<<~RUBY)
58+
describe 'Something', :a, :z, some_hash do
59+
end
60+
RUBY
61+
end
62+
63+
it 'does not register an offense when using a second level description ' \
64+
'not in alphabetical order with symbol metadata' do
65+
expect_no_offenses(<<~RUBY)
66+
describe 'Something', 'second docstring', :a, :b do
67+
end
68+
RUBY
69+
end
70+
71+
it 'registers an offense when using a second level description ' \
72+
'and metadata not in alphabetical order' do
73+
expect_offense(<<~RUBY)
74+
describe 'Something', 'second docstring', :b, :a do
75+
^^^^^^ Sort metadata alphabetically.
76+
end
77+
RUBY
78+
79+
expect_correction(<<~RUBY)
80+
describe 'Something', 'second docstring', :a, :b do
2281
end
2382
RUBY
2483
end
@@ -100,27 +159,27 @@
100159
'and the hash values are complex objects' do
101160
expect_offense(<<~RUBY)
102161
it 'Something', variable, 'B', :a, key => {}, foo: ->(x) { bar(x) }, Identifier.sample => true, baz: Snafu.new do
103-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sort metadata alphabetically.
162+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sort metadata alphabetically.
104163
end
105164
RUBY
106165

107166
expect_correction(<<~RUBY)
108-
it 'Something', :a, 'B', variable, baz: Snafu.new, foo: ->(x) { bar(x) }, Identifier.sample => true, key => {} do
167+
it 'Something', variable, 'B', :a, baz: Snafu.new, foo: ->(x) { bar(x) }, Identifier.sample => true, key => {} do
109168
end
110169
RUBY
111170
end
112171

113172
it 'registers an offense only when example or group has a block' do
114173
expect_offense(<<~RUBY)
115-
shared_examples 'a difficult situation', 'B', :a do |x, y|
116-
^^^^^^^ Sort metadata alphabetically.
174+
shared_examples 'a difficult situation', 'B', :z, :a do |x, y|
175+
^^^^^^ Sort metadata alphabetically.
117176
end
118177
119178
include_examples 'a difficult situation', 'value', 'another value'
120179
RUBY
121180

122181
expect_correction(<<~RUBY)
123-
shared_examples 'a difficult situation', :a, 'B' do |x, y|
182+
shared_examples 'a difficult situation', 'B', :a, :z do |x, y|
124183
end
125184
126185
include_examples 'a difficult situation', 'value', 'another value'
@@ -129,14 +188,14 @@
129188

130189
it 'registers an offense also when the metadata is not on one single line' do
131190
expect_offense(<<~RUBY)
132-
RSpec.describe 'Something', :foo, :bar,
133-
^^^^^^^^^^^ Sort metadata alphabetically.
191+
describe 'Something', :foo, :bar,
192+
^^^^^^^^^^^ Sort metadata alphabetically.
134193
baz: 'goo' do
135194
end
136195
RUBY
137196

138197
expect_correction(<<~RUBY)
139-
RSpec.describe 'Something', :bar, :foo, baz: 'goo' do
198+
describe 'Something', :bar, :foo, baz: 'goo' do
140199
end
141200
RUBY
142201
end

0 commit comments

Comments
 (0)