[Fix #9836] Fix incorrect corrections for Layout/HashAlignment when…

… a `kwsplat` node is on the same line as a `pair` node with table style.

Keyword splats were previously treated as pairs using colon style, which some hacks in the Alignment classes to handle them. However, this did not account for all possibilities and would cause previous pairs on the same line as the `kwsplat` to be removed by autocorrection.

Instead, now a special `KeywordSplatAlignment` type was added to handle `kwsplat`s. The previously behaviour about `kwsplat`s was retained (they will still be aligned with the beginning of the rest of the hash, regardless of enforced style), and the hacks in `KeyAlignment` and `TableAlignment` for `kwsplat`s were removed.

changelog/fix_fix_incorrect_corrections_for.md

@@ -0,0 +1 @@
+* [#9836](https://github.com/rubocop/rubocop/issues/9836): Fix incorrect corrections for `Layout/HashAlignment` when a `kwsplat` node is on the same line as a `pair` node with table style. ([@dvandersluis][])

config/default.yml

@@ -772,7 +772,7 @@ Layout/HashAlignment:
   Enabled: true
   AllowMultipleStyles: true
   VersionAdded: '0.49'
-  VersionChanged: '0.77'
+  VersionChanged: '<<next>>'
   # Alignment of entries using hash rocket as separator. Valid values are:
   #
   # key - left alignment of keys

lib/rubocop/cop/layout/hash_alignment.rb

@@ -185,7 +185,9 @@ class HashAlignment < Base
                      SeparatorAlignment => 'Align the separators of a hash ' \
                        'literal if they span more than one line.',
                      TableAlignment => 'Align the keys and values of a hash ' \
-                       'literal if they span more than one line.' }.freeze
+                       'literal if they span more than one line.',
+                     KeywordSplatAlignment => 'Align keyword splats with the ' \
+                       'rest of the hash if it spans more than one line.' }.freeze
 
         def on_send(node)
           return if double_splat?(node)
@@ -247,7 +249,14 @@ def check_pairs(node)
         end
 
         def add_offences
+          kwsplat_offences = offences_by.delete(KeywordSplatAlignment)
+          register_offences_with_format(kwsplat_offences, KeywordSplatAlignment)
+
           format, offences = offences_by.min_by { |_, v| v.length }
+          register_offences_with_format(offences, format)
+        end
+
+        def register_offences_with_format(offences, format)
           (offences || []).each do |offence|
             add_offense(offence, message: MESSAGES[format]) do |corrector|
               delta = column_deltas[alignment_for(offence).first.class][offence]
@@ -275,7 +284,9 @@ def ignore_hash_argument?(node)
         end
 
         def alignment_for(pair)
-          if pair.hash_rocket?
+          if pair.kwsplat_type?
+            [KeywordSplatAlignment.new]
+          elsif pair.hash_rocket?
             alignment_for_hash_rockets
           else
             alignment_for_colons

lib/rubocop/cop/mixin/hash_alignment_styles.rb

@@ -43,7 +43,7 @@ def separator_delta(pair)
         end
 
         def value_delta(pair)
-          return 0 if pair.kwsplat_type? || pair.value_on_new_line?
+          return 0 if pair.value_on_new_line?
 
           correct_value_column = pair.loc.operator.end.column + 1
           actual_value_column = pair.value.loc.column
@@ -108,8 +108,6 @@ def hash_rocket_delta(first_pair, current_pair)
         end
 
         def value_delta(first_pair, current_pair)
-          return 0 if current_pair.kwsplat_type?
-
           correct_value_column = first_pair.key.loc.column +
                                  current_pair.delimiter(true).length +
                                  max_key_width
@@ -139,6 +137,19 @@ def value_delta(first_pair, current_pair)
           first_pair.value_delta(current_pair)
         end
       end
+
+      # Handles calculation of deltas for `kwsplat` nodes.
+      # This is a special case that just ensures the kwsplat is aligned with the rest of the hash
+      # since a `kwsplat` does not have a key, separator or value.
+      class KeywordSplatAlignment
+        def deltas(first_pair, current_pair)
+          if Util.begins_its_line?(current_pair.source_range)
+            { key: first_pair.key_delta(current_pair) }
+          else
+            {}
+          end
+        end
+      end
     end
   end
 end

spec/rubocop/cop/layout/hash_alignment_spec.rb

@@ -540,7 +540,7 @@ def example
       expect_offense(<<~RUBY)
         Hash(foo: 'bar',
                **extra_params
-               ^^^^^^^^^^^^^^ Align the keys of a hash literal if they span more than one line.
+               ^^^^^^^^^^^^^^ Align keyword splats with the rest of the hash if it spans more than one line.
         )
       RUBY
 
@@ -555,7 +555,7 @@ def example
       expect_offense(<<~RUBY)
         {foo: 'bar',
                **extra_params
-               ^^^^^^^^^^^^^^ Align the keys of a hash literal if they span more than one line.
+               ^^^^^^^^^^^^^^ Align keyword splats with the rest of the hash if it spans more than one line.
         }
       RUBY
 
@@ -1127,4 +1127,144 @@ def self.scenarios_order
   it 'register no offense for yield without args' do
     expect_no_offenses('yield')
   end
+
+  context 'with `EnforcedColonStyle`: `table`' do
+    let(:cop_config) do
+      {
+        'EnforcedColonStyle' => 'table'
+      }
+    end
+
+    context 'and misaligned keys' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<~RUBY)
+          foo ab: 1,
+              c: 2
+              ^^^^ Align the keys and values of a hash literal if they span more than one line.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo ab: 1,
+              c:  2
+        RUBY
+      end
+    end
+
+    context 'when the only item is a kwsplat' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          foo({**rest})
+        RUBY
+      end
+    end
+
+    context 'and a double splat argument after a hash key' do
+      it 'registers an offense on the misaligned key and corrects' do
+        expect_offense(<<~RUBY)
+          foo ab: 1,
+              c: 2, **rest
+              ^^^^ Align the keys and values of a hash literal if they span more than one line.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo ab: 1,
+              c:  2, **rest
+        RUBY
+      end
+    end
+
+    context 'and aligned keys but a double splat argument after' do
+      it 'does not register an offense on the `kwsplat`' do
+        expect_no_offenses(<<~RUBY)
+          foo a: 1,
+              b: 2, **rest
+        RUBY
+      end
+    end
+
+    context 'and a misaligned double splat argument' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<~RUBY)
+          foo a: 1,
+                **rest
+                ^^^^^^ Align keyword splats with the rest of the hash if it spans more than one line.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo a: 1,
+              **rest
+        RUBY
+      end
+    end
+  end
+
+  context 'with `EnforcedHashRocketStyle`: `table`' do
+    let(:cop_config) do
+      {
+        'EnforcedHashRocketStyle' => 'table'
+      }
+    end
+
+    context 'and misaligned keys' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<~RUBY)
+          foo :ab => 1,
+              :c => 2
+              ^^^^^^^ Align the keys and values of a hash literal if they span more than one line.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo :ab => 1,
+              :c  => 2
+        RUBY
+      end
+    end
+
+    context 'when the only item is a kwsplat' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          foo({**rest})
+        RUBY
+      end
+    end
+
+    context 'and a double splat argument after a hash key' do
+      it 'registers an offense on the misaligned key and corrects' do
+        expect_offense(<<~RUBY)
+          foo :ab => 1,
+              :c => 2, **rest
+              ^^^^^^^ Align the keys and values of a hash literal if they span more than one line.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo :ab => 1,
+              :c  => 2, **rest
+        RUBY
+      end
+    end
+
+    context 'and aligned keys but a double splat argument after' do
+      it 'does not register an offense on the `kwsplat`' do
+        expect_no_offenses(<<~RUBY)
+          foo :a => 1,
+              :b => 2, **rest
+        RUBY
+      end
+    end
+
+    context 'and a misaligned double splat argument' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<~RUBY)
+          foo :a => 1,
+                **rest
+                ^^^^^^ Align keyword splats with the rest of the hash if it spans more than one line.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo :a => 1,
+              **rest
+        RUBY
+      end
+    end
+  end
 end