Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RSpec/SortMetadata cop to sort strings and variables first #1948

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -5733,6 +5733,8 @@ end

Sort RSpec metadata alphabetically.

Only the trailing metadata is sorted.

[#examples-rspecsortmetadata]
=== Examples

Expand All @@ -5747,6 +5749,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
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
cbliard marked this conversation as resolved.
Show resolved Hide resolved
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
Loading