Merge pull request #1404 from eugeneius/index_by_index_with_numblock

Update `Rails/IndexBy` and `Rails/IndexWith` to support numbered block parameters
rubocop · Jan 2, 2025 · 851f382 · 851f382
2 parents 43fb400 + 15605e1
commit 851f382
Show file tree

Hide file tree

Showing 6 changed files with 180 additions and 25 deletions.
diff --git a/changelog/fix_index_by_index_with_numblock.md b/changelog/fix_index_by_index_with_numblock.md
@@ -0,0 +1 @@
+* [#1404](https://github.com/rubocop/rubocop-rails/pull/1404): Update `Rails/IndexBy` and `Rails/IndexWith` to support numbered block parameters. ([@eugeneius][])
diff --git a/lib/rubocop/cop/mixin/index_method.rb b/lib/rubocop/cop/mixin/index_method.rb
@@ -6,7 +6,7 @@ module Cop
     module IndexMethod # rubocop:disable Metrics/ModuleLength
       RESTRICT_ON_SEND = %i[each_with_object to_h map collect []].freeze
 
-      def on_block(node) # rubocop:todo InternalAffairs/NumblockHandler
+      def on_block(node)
         on_bad_each_with_object(node) do |*match|
           handle_possible_offense(node, match, 'each_with_object')
         end
@@ -18,6 +18,8 @@ def on_block(node) # rubocop:todo InternalAffairs/NumblockHandler
         end
       end
 
+      alias on_numblock on_block
+
       def on_send(node)
         on_bad_map_to_h(node) do |*match|
           handle_possible_offense(node, match, 'map { ... }.to_h')
@@ -153,6 +155,8 @@ def set_new_method_name(new_method_name, corrector)
         end
 
         def set_new_arg_name(transformed_argname, corrector)
+          return if block_node.numblock_type?
+
           corrector.replace(block_node.arguments, "|#{transformed_argname}|")
         end
 

diff --git a/lib/rubocop/cop/rails/index_by.rb b/lib/rubocop/cop/rails/index_by.rb
@@ -29,29 +29,45 @@ class IndexBy < Base
         PATTERN
 
         def_node_matcher :on_bad_to_h, <<~PATTERN
-          (block
-            (call _ :to_h)
-            (args (arg $_el))
-            (array $_ (lvar _el)))
+          {
+            (block
+              (call _ :to_h)
+              (args (arg $_el))
+              (array $_ (lvar _el)))
+            (numblock
+              (call _ :to_h) $1
+              (array $_ (lvar :_1)))
+          }
         PATTERN
 
         def_node_matcher :on_bad_map_to_h, <<~PATTERN
           (call
-            (block
-              (call _ {:map :collect})
-              (args (arg $_el))
-              (array $_ (lvar _el)))
+            {
+              (block
+                (call _ {:map :collect})
+                (args (arg $_el))
+                (array $_ (lvar _el)))
+              (numblock
+                (call _ {:map :collect}) $1
+                (array $_ (lvar :_1)))
+            }
             :to_h)
         PATTERN
 
         def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
           (send
             (const {nil? cbase} :Hash)
             :[]
-            (block
-              (call _ {:map :collect})
-              (args (arg $_el))
-              (array $_ (lvar _el))))
+            {
+              (block
+                (call _ {:map :collect})
+                (args (arg $_el))
+                (array $_ (lvar _el)))
+              (numblock
+                (call _ {:map :collect}) $1
+                (array $_ (lvar :_1)))
+            }
+          )
         PATTERN
 
         private

diff --git a/lib/rubocop/cop/rails/index_with.rb b/lib/rubocop/cop/rails/index_with.rb
@@ -32,29 +32,45 @@ class IndexWith < Base
         PATTERN
 
         def_node_matcher :on_bad_to_h, <<~PATTERN
-          (block
-            (call _ :to_h)
-            (args (arg $_el))
-            (array (lvar _el) $_))
+          {
+            (block
+              (call _ :to_h)
+              (args (arg $_el))
+              (array (lvar _el) $_))
+            (numblock
+              (call _ :to_h) $1
+              (array (lvar :_1) $_))
+          }
         PATTERN
 
         def_node_matcher :on_bad_map_to_h, <<~PATTERN
           (call
-            (block
-              (call _ {:map :collect})
-              (args (arg $_el))
-              (array (lvar _el) $_))
+            {
+              (block
+                (call _ {:map :collect})
+                (args (arg $_el))
+                (array (lvar _el) $_))
+              (numblock
+                (call _ {:map :collect}) $1
+                (array (lvar :_1) $_))
+            }
             :to_h)
         PATTERN
 
         def_node_matcher :on_bad_hash_brackets_map, <<~PATTERN
           (send
             (const {nil? cbase} :Hash)
             :[]
-            (block
-              (call _ {:map :collect})
-              (args (arg $_el))
-              (array (lvar _el) $_)))
+            {
+              (block
+                (call _ {:map :collect})
+                (args (arg $_el))
+                (array (lvar _el) $_))
+              (numblock
+                (call _ {:map :collect}) $1
+                (array (lvar :_1) $_))
+            }
+          )
         PATTERN
 
         private

diff --git a/spec/rubocop/cop/rails/index_by_spec.rb b/spec/rubocop/cop/rails/index_by_spec.rb
@@ -182,4 +182,63 @@
       RUBY
     end
   end
+
+  context 'numbered parameters' do
+    it 'registers an offense for `map { ... }.to_h`' do
+      expect_offense(<<~RUBY)
+        x.map { [_1.to_sym, _1] }.to_h
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `map { ... }.to_h`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.index_by { _1.to_sym }
+      RUBY
+    end
+
+    context 'when values are transformed' do
+      it 'does not register an offense for `map { ... }.to_h`' do
+        expect_no_offenses(<<~RUBY)
+          x.map { [_1.to_sym, foo(_1)] }.to_h
+        RUBY
+      end
+    end
+
+    it 'registers an offense for Hash[map { ... }]' do
+      expect_offense(<<~RUBY)
+        Hash[x.map { [_1.to_sym, _1] }]
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `Hash[map { ... }]`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.index_by { _1.to_sym }
+      RUBY
+    end
+
+    context 'when the referenced numbered parameter is not _1' do
+      it 'does not register an offense for Hash[map { ... }]' do
+        expect_no_offenses(<<~RUBY)
+          Hash[x.map { [_1.to_sym, _2] }]
+        RUBY
+      end
+    end
+
+    it 'registers an offense for `to_h { ... }`' do
+      expect_offense(<<~RUBY)
+        x.to_h { [_1.to_sym, _1] }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `to_h { ... }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.index_by { _1.to_sym }
+      RUBY
+    end
+
+    context 'when a numbered parameter other than _1 is referenced in the key' do
+      it 'does not register an offense for `to_h { ... }`' do
+        expect_no_offenses(<<~RUBY)
+          x.to_h { [_2.to_sym, _1] }
+        RUBY
+      end
+    end
+  end
 end
diff --git a/spec/rubocop/cop/rails/index_with_spec.rb b/spec/rubocop/cop/rails/index_with_spec.rb
@@ -155,6 +155,65 @@
         RUBY
       end
     end
+
+    context 'numbered parameters' do
+      it 'registers an offense for `map { ... }.to_h`' do
+        expect_offense(<<~RUBY)
+          x.map { [_1, _1.to_sym] }.to_h
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `map { ... }.to_h`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          x.index_with { _1.to_sym }
+        RUBY
+      end
+
+      context 'when keys are transformed' do
+        it 'does not register an offense for `map { ... }.to_h`' do
+          expect_no_offenses(<<~RUBY)
+            x.map { [foo(_1), _1.to_sym] }.to_h
+          RUBY
+        end
+      end
+
+      it 'registers an offense for Hash[map { ... }]' do
+        expect_offense(<<~RUBY)
+          Hash[x.map { [_1, _1.to_sym] }]
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `Hash[map { ... }]`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          x.index_with { _1.to_sym }
+        RUBY
+      end
+
+      context 'when the referenced numbered parameter is not _1' do
+        it 'does not register an offense for Hash[map { ... }]' do
+          expect_no_offenses(<<~RUBY)
+            Hash[x.map { [_2, _1.to_sym] }]
+          RUBY
+        end
+      end
+
+      it 'registers an offense for `to_h { ... }`' do
+        expect_offense(<<~RUBY)
+          x.to_h { [_1, _1.to_sym] }
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `to_h { ... }`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          x.index_with { _1.to_sym }
+        RUBY
+      end
+
+      context 'when a numbered parameter other than _1 is referenced in the value' do
+        it 'does not register an offense for `to_h { ... }`' do
+          expect_no_offenses(<<~RUBY)
+            x.to_h { [_1, _2.to_sym] }
+          RUBY
+        end
+      end
+    end
   end
 
   context 'when using Rails 5.2 or older', :rails52 do
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		* [#1404](https://github.com/rubocop/rubocop-rails/pull/1404): Update `Rails/IndexBy` and `Rails/IndexWith` to support numbered block parameters. ([@eugeneius][])