Merge pull request #1471 from koic/support_itblock_in_rails_cops

koic · web-flow · commit 2420d70ed919 · 2025-03-29T02:48:42.000+09:00
Support `it` block parameter in `Rails` cops
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -83,7 +83,7 @@ jobs:
               -e "/gem 'rubocop-performance',/d" \
               -e "/gem 'rubocop-rspec',/d" -i Gemfile
           cat << EOF > Gemfile.local
-            gem 'rubocop', '1.72.1' # Specify the oldest supported RuboCop version
+            gem 'rubocop', '1.75.0' # Specify the oldest supported RuboCop version
           EOF
       - uses: ruby/setup-ruby@v1
         with:
diff --git a/changelog/new_support_itblock_in_rails_cops.md b/changelog/new_support_itblock_in_rails_cops.md
@@ -0,0 +1 @@
+* [#1471](https://github.com/rubocop/rubocop-rails/pull/1471): Support `it` block parameter in `Rails` cops. ([@koic][])
diff --git a/lib/rubocop/cop/mixin/index_method.rb b/lib/rubocop/cop/mixin/index_method.rb
@@ -58,7 +58,7 @@ def set_new_method_name(new_method_name, corrector)
         end
 
         def set_new_arg_name(transformed_argname, corrector)
-          return if block_node.numblock_type?
+          return unless block_node.block_type?
 
           corrector.replace(block_node.arguments, "|#{transformed_argname}|")
         end
@@ -84,6 +84,7 @@ def on_block(node)
       end
 
       alias on_numblock on_block
+      alias on_itblock on_block
 
       def on_send(node)
         on_bad_map_to_h(node) do |*match|
diff --git a/lib/rubocop/cop/rails/index_by.rb b/lib/rubocop/cop/rails/index_by.rb
@@ -37,6 +37,9 @@ class IndexBy < Base
             (numblock
               (call _ :to_h) $1
               (array $_ (lvar :_1)))
+            (itblock
+              (call _ :to_h) $:it
+              (array $_ (lvar :it)))
           }
         PATTERN
 
@@ -50,6 +53,9 @@ class IndexBy < Base
               (numblock
                 (call _ {:map :collect}) $1
                 (array $_ (lvar :_1)))
+              (itblock
+                (call _ {:map :collect}) $:it
+                (array $_ (lvar :it)))
             }
             :to_h)
         PATTERN
@@ -66,6 +72,9 @@ class IndexBy < Base
               (numblock
                 (call _ {:map :collect}) $1
                 (array $_ (lvar :_1)))
+              (itblock
+                (call _ {:map :collect}) $:it
+                (array $_ (lvar :it)))
             }
           )
         PATTERN
diff --git a/lib/rubocop/cop/rails/index_with.rb b/lib/rubocop/cop/rails/index_with.rb
@@ -40,6 +40,9 @@ class IndexWith < Base
             (numblock
               (call _ :to_h) $1
               (array (lvar :_1) $_))
+            (itblock
+              (call _ :to_h) $:it
+              (array (lvar :it) $_))
           }
         PATTERN
 
@@ -53,6 +56,9 @@ class IndexWith < Base
               (numblock
                 (call _ {:map :collect}) $1
                 (array (lvar :_1) $_))
+              (itblock
+                (call _ {:map :collect}) $:it
+                (array (lvar :it) $_))
             }
             :to_h)
         PATTERN
@@ -69,6 +75,9 @@ class IndexWith < Base
               (numblock
                 (call _ {:map :collect}) $1
                 (array (lvar :_1) $_))
+              (itblock
+                (call _ {:map :collect}) $:it
+                (array (lvar :it) $_))
             }
           )
         PATTERN
diff --git a/lib/rubocop/cop/rails/pluck.rb b/lib/rubocop/cop/rails/pluck.rb
@@ -59,6 +59,7 @@ class Pluck < Base
           (any_block (call _ {:map :collect}) $_argument (send lvar :[] $_key))
         PATTERN
 
+        # rubocop:disable Metrics/AbcSize
         def on_block(node)
           return if node.each_ancestor(:any_block).any?
 
@@ -68,20 +69,25 @@ def on_block(node)
             match = if node.block_type?
                       block_argument = argument.children.first.source
                       use_block_argument_in_key?(block_argument, key)
-                    else # numblock
-                      argument == 1 && use_block_argument_in_key?('_1', key)
+                    elsif node.numblock_type?
+                      use_block_argument_in_key?('_1', key)
+                    else # itblock
+                      use_block_argument_in_key?('it', key)
                     end
             next unless match
 
             register_offense(node, key)
           end
         end
+        # rubocop:enable Metrics/AbcSize
         alias on_numblock on_block
+        alias on_itblock on_block
 
         private
 
         def use_one_block_argument?(argument)
-          return true if argument == 1 # Checks for numbered argument `_1`.
+          # Checks for numbered argument `_1` or `it block parameter.
+          return true if [1, :it].include?(argument)
 
           argument.respond_to?(:one?) && argument.one?
         end
diff --git a/lib/rubocop/cop/rails/redundant_receiver_in_with_options.rb b/lib/rubocop/cop/rails/redundant_receiver_in_with_options.rb
@@ -85,18 +85,22 @@ def on_block(node)
         end
 
         alias on_numblock on_block
+        alias on_itblock on_block
 
         private
 
         def autocorrect(corrector, send_node, node)
           corrector.remove(send_node.receiver)
           corrector.remove(send_node.loc.dot)
-          corrector.remove(block_argument_range(send_node)) unless node.numblock_type?
+          corrector.remove(block_argument_range(send_node)) if node.block_type?
         end
 
+        # rubocop:disable Metrics/AbcSize
         def redundant_receiver?(send_nodes, node)
           proc = if node.numblock_type?
                    ->(n) { n.receiver.lvar_type? && n.receiver.source == '_1' }
+                 elsif node.itblock_type?
+                   ->(n) { n.receiver.lvar_type? && n.receiver.source == 'it' }
                  else
                    return false if node.arguments.empty?
 
@@ -106,6 +110,7 @@ def redundant_receiver?(send_nodes, node)
 
           send_nodes.all?(&proc)
         end
+        # rubocop:enable Metrics/AbcSize
 
         def block_argument_range(node)
           block_node = node.each_ancestor(:block).first
diff --git a/lib/rubocop/cop/rails/reversible_migration.rb b/lib/rubocop/cop/rails/reversible_migration.rb
@@ -205,6 +205,7 @@ def on_block(node)
         end
 
         alias on_numblock on_block
+        alias on_itblock on_block
 
         private
 
diff --git a/rubocop-rails.gemspec b/rubocop-rails.gemspec
@@ -37,6 +37,6 @@ Gem::Specification.new do |s|
   # Rack::Utils::SYMBOL_TO_STATUS_CODE, which is used by HttpStatus cop, was
   # introduced in rack 1.1
   s.add_dependency 'rack', '>= 1.1'
-  s.add_dependency 'rubocop', '>= 1.72.1', '< 2.0'
+  s.add_dependency 'rubocop', '>= 1.75.0', '< 2.0'
   s.add_dependency 'rubocop-ast', '>= 1.38.0', '< 2.0'
 end
diff --git a/spec/rubocop/cop/rails/create_table_with_timestamps_spec.rb b/spec/rubocop/cop/rails/create_table_with_timestamps_spec.rb
@@ -64,6 +64,17 @@
     RUBY
   end
 
+  it 'does not register an offense when including timestamps in itblock', :ruby34, unsupported_on: :parser do
+    expect_no_offenses <<~RUBY
+      create_table :users do
+        it.string :name
+        it.string :email
+
+        it.timestamps
+      end
+    RUBY
+  end
+
   it 'does not register an offense when including timestamps with `to_proc` syntax' do
     expect_no_offenses <<~RUBY
       create_table :users, &:timestamps
diff --git a/spec/rubocop/cop/rails/index_by_spec.rb b/spec/rubocop/cop/rails/index_by_spec.rb
@@ -262,4 +262,68 @@
       end
     end
   end
+
+  context '`it` parameter', :ruby34, unsupported_on: :parser do
+    it 'registers an offense for `map { ... }.to_h`' do
+      expect_offense(<<~RUBY)
+        x.map { [it.to_sym, it] }.to_h
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `map { ... }.to_h`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.index_by { it.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 { [it.to_sym, foo(it)] }.to_h
+        RUBY
+      end
+    end
+
+    it 'registers an offense for Hash[map { ... }]' do
+      expect_offense(<<~RUBY)
+        Hash[x.map { [it.to_sym, it] }]
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `Hash[map { ... }]`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.index_by { it.to_sym }
+      RUBY
+    end
+
+    context 'when the referenced `it` parameter is not it' do
+      it 'does not register an offense for Hash[map { ... }]' do
+        expect_no_offenses(<<~RUBY)
+          Hash[x.map { [it.to_sym, y] }]
+        RUBY
+      end
+    end
+
+    it 'registers an offense for `to_h { ... }`' do
+      expect_offense(<<~RUBY)
+        x.to_h { [it.to_sym, it] }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `to_h { ... }`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        x.index_by { it.to_sym }
+      RUBY
+    end
+
+    context 'when `it` parameter other than `it` is referenced in the key' do
+      it 'registers an offense for `to_h { ... }`' do
+        expect_offense(<<~RUBY)
+          x.to_h { [y.to_sym, it] }
+          ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_by` over `to_h { ... }`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          x.index_by { y.to_sym }
+        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
@@ -261,6 +261,70 @@
         end
       end
     end
+
+    context '`it` block parameter', :ruby34, unsupported_on: :parser do
+      it 'registers an offense for `map { ... }.to_h`' do
+        expect_offense(<<~RUBY)
+          x.map { [it, it.to_sym] }.to_h
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `map { ... }.to_h`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          x.index_with { it.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(it), it.to_sym] }.to_h
+          RUBY
+        end
+      end
+
+      it 'registers an offense for Hash[map { ... }]' do
+        expect_offense(<<~RUBY)
+          Hash[x.map { [it, it.to_sym] }]
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `Hash[map { ... }]`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          x.index_with { it.to_sym }
+        RUBY
+      end
+
+      context 'when the referenced `it` parameter is not `it`' do
+        it 'does not register an offense for Hash[map { ... }]' do
+          expect_no_offenses(<<~RUBY)
+            Hash[x.map { [y, it.to_sym] }]
+          RUBY
+        end
+      end
+
+      it 'registers an offense for `to_h { ... }`' do
+        expect_offense(<<~RUBY)
+          x.to_h { [it, it.to_sym] }
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `to_h { ... }`.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          x.index_with { it.to_sym }
+        RUBY
+      end
+
+      context 'when an `it` parameter other than `it` is referenced in the value' do
+        it 'registers an offense for `to_h { ... }`' do
+          expect_offense(<<~RUBY)
+            x.to_h { [it, y.to_sym] }
+            ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `index_with` over `to_h { ... }`.
+          RUBY
+
+          expect_correction(<<~RUBY)
+            x.index_with { y.to_sym }
+          RUBY
+        end
+      end
+    end
   end
 
   context 'when using Rails 5.2 or older', :rails52 do
diff --git a/spec/rubocop/cop/rails/output_spec.rb b/spec/rubocop/cop/rails/output_spec.rb
@@ -161,6 +161,12 @@
     RUBY
   end
 
+  it 'does not register an offense when io method is called with `it` parameter', :ruby34, unsupported_on: :parser do
+    expect_no_offenses(<<~RUBY)
+      obj.write { do_something(it) }
+    RUBY
+  end
+
   it 'does not register an offense when a method is ' \
      'safe navigation called to a local variable with the same name as a print method' do
     expect_no_offenses(<<~RUBY)
diff --git a/spec/rubocop/cop/rails/pluck_spec.rb b/spec/rubocop/cop/rails/pluck_spec.rb
@@ -129,6 +129,31 @@
         end
       end
 
+      context 'when using Ruby 3.4 or newer', :ruby34, unsupported_on: :parser do
+        context 'when using `it` block parameter' do
+          context "when `#{method}` can be replaced with `pluck`" do
+            it 'registers an offense' do
+              expect_offense(<<~RUBY, method: method)
+                x.%{method} { it[:foo] }
+                  ^{method}^^^^^^^^^^^^^ Prefer `pluck(:foo)` over `%{method} { it[:foo] }`.
+              RUBY
+
+              expect_correction(<<~RUBY)
+                x.pluck(:foo)
+              RUBY
+            end
+          end
+
+          context 'when the `it` argument is used in `[]`' do
+            it 'does not register an offense' do
+              expect_no_offenses(<<~RUBY)
+                x.#{method} { it[foo...it.to_something] }
+              RUBY
+            end
+          end
+        end
+      end
+
       context "when `#{method}` is used in block" do
         it 'does not register an offense' do
           expect_no_offenses(<<~RUBY)
diff --git a/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb b/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb
@@ -329,6 +329,12 @@
         RUBY
       end
 
+      it "does not register an offense when using `#{method}` with `it` block", :ruby34, unsupported_on: :parser do
+        expect_no_offenses(<<~RUBY)
+          User.all.#{method} { it.do_something }
+        RUBY
+      end
+
       it "does not register an offense when using `#{method}` with symbol block" do
         expect_no_offenses(<<~RUBY)
           User.all.#{method}(&:do_something)
diff --git a/spec/rubocop/cop/rails/redundant_receiver_in_with_options_spec.rb b/spec/rubocop/cop/rails/redundant_receiver_in_with_options_spec.rb
diff --git a/spec/rubocop/cop/rails/relative_date_constant_spec.rb b/spec/rubocop/cop/rails/relative_date_constant_spec.rb
diff --git a/spec/rubocop/cop/rails/reversible_migration_spec.rb b/spec/rubocop/cop/rails/reversible_migration_spec.rb
diff --git a/spec/rubocop/cop/rails/save_bang_spec.rb b/spec/rubocop/cop/rails/save_bang_spec.rb
diff --git a/spec/rubocop/cop/rails/transaction_exit_statement_spec.rb b/spec/rubocop/cop/rails/transaction_exit_statement_spec.rb
diff --git a/spec/rubocop/cop/rails/uniq_before_pluck_spec.rb b/spec/rubocop/cop/rails/uniq_before_pluck_spec.rb
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+* [#1471](https://github.com/rubocop/rubocop-rails/pull/1471): Support `it` block parameter in `Rails` cops. ([@koic][])