From 9fbf40d570a62cb7cfda504779297df4b8798d4e Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Mon, 12 Aug 2024 08:17:21 +0200 Subject: [PATCH] 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. --- CHANGELOG.md | 2 + docs/modules/ROOT/pages/cops_rspec.adoc | 6 ++ lib/rubocop/cop/rspec/mixin/metadata.rb | 13 ++--- lib/rubocop/cop/rspec/sort_metadata.rb | 30 +++++++--- spec/rubocop/cop/rspec/sort_metadata_spec.rb | 58 ++++++++++++++++++-- 5 files changed, 88 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93499ceaf..8f0f659eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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]) diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index 2aff167af..801a2e98a 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -5339,6 +5339,8 @@ end Sort RSpec metadata alphabetically. +Only the trailing metadata is sorted. + === Examples [source,ruby] @@ -5352,6 +5354,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 diff --git a/lib/rubocop/cop/rspec/mixin/metadata.rb b/lib/rubocop/cop/rspec/mixin/metadata.rb index b5dd448c1..325747433 100644 --- a/lib/rubocop/cop/rspec/mixin/metadata.rb +++ b/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 diff --git a/lib/rubocop/cop/rspec/sort_metadata.rb b/lib/rubocop/cop/rspec/sort_metadata.rb index f34f0ade1..2e9411a44 100644 --- a/lib/rubocop/cop/rspec/sort_metadata.rb +++ b/lib/rubocop/cop/rspec/sort_metadata.rb @@ -5,6 +5,8 @@ module Cop module RSpec # Sort RSpec metadata alphabetically. # + # Only the trailing metadata is sorted. + # # @example # # bad # describe 'Something', :b, :a @@ -16,6 +18,9 @@ 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 @@ -23,8 +28,14 @@ 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 || [] + symbols = trailing_symbols(args) return if sorted?(symbols, pairs) crime_scene = crime_scene(symbols, pairs) @@ -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 @@ -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 diff --git a/spec/rubocop/cop/rspec/sort_metadata_spec.rb b/spec/rubocop/cop/rspec/sort_metadata_spec.rb index c6129a366..3ba7a2e14 100644 --- a/spec/rubocop/cop/rspec/sort_metadata_spec.rb +++ b/spec/rubocop/cop/rspec/sort_metadata_spec.rb @@ -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) @@ -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'