Register an offense for Naming/BlockForwarding and `Style/Arguments…

…Forwarding` with Ruby >= 3.4 when the block is referenced in a block This worked around a bug in Ruby 3.3.0, where this syntax was accidentally disallowed. This check can now be reenabled for 3.4 where all versions accept it. Reference: * #12571 * #12577 * #12578 * https://bugs.ruby-lang.org/issues/20090
rubocop · Jan 23, 2025 · d766e91 · d766e91
1 parent 24bbe26
commit d766e91
Show file tree

Hide file tree

Showing 5 changed files with 238 additions and 106 deletions.
diff --git a/changelog/fix_false_negative_block_forwading_34.md b/changelog/fix_false_negative_block_forwading_34.md
@@ -0,0 +1 @@
+* [#13741](https://github.com/rubocop/rubocop/pull/13741): Register an offense for `Naming/BlockForwarding` and `Style/ArgumentsForwarding` with Ruby >= 3.4 when the block argument is referenced inside a block. This was previously disabled because of a bug in Ruby 3.3.0. ([@earlopain][])
diff --git a/lib/rubocop/cop/naming/block_forwarding.rb b/lib/rubocop/cop/naming/block_forwarding.rb
@@ -90,21 +90,11 @@ def block_argument_name_matched?(block_pass_node, last_argument)
           last_argument.source == block_pass_node.source
         end
 
-        # Prevents the following syntax error:
-        #
-        # # foo.rb
-        # def foo(&)
-        #   block_method do
-        #     bar(&)
-        #   end
-        # end
-        #
-        # $ ruby -vc foo.rb
-        # ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
-        # foo.rb: foo.rb:4: anonymous block parameter is also used within block (SyntaxError)
-        #
+        # Ruby 3.3.0 had a bug where accessing an anonymous block argument inside of a block
+        # was a syntax error in unambiguous cases: https://bugs.ruby-lang.org/issues/20090
+        # We disallow this also for earlier Ruby versions so that code is forwards compatible.
         def invalidates_syntax?(block_pass_node)
-          block_pass_node.each_ancestor(:block, :numblock).any?
+          target_ruby_version <= 3.3 && block_pass_node.each_ancestor(:block, :numblock).any?
         end
 
         def use_kwarg_in_method_definition?(node)

diff --git a/lib/rubocop/cop/style/arguments_forwarding.rb b/lib/rubocop/cop/style/arguments_forwarding.rb
@@ -191,9 +191,7 @@ def add_forward_all_offenses(node, send_classifications, forwardable_args)
 
           send_classifications.each do |send_node, c, forward_rest, forward_kwrest, forward_block_arg| # rubocop:disable Layout/LineLength
             if !forward_rest && !forward_kwrest && c != :all_anonymous
-              # Prevents `anonymous block parameter is also used within block (SyntaxError)` occurs
-              # in Ruby 3.3.0.
-              if outside_block?(forward_block_arg)
+              if allow_anonymous_forwarding_in_block?(forward_block_arg)
                 register_forward_block_arg_offense(!forward_rest, node.arguments, block_arg)
                 register_forward_block_arg_offense(!forward_rest, send_node, forward_block_arg)
               end
@@ -214,24 +212,22 @@ def add_forward_all_offenses(node, send_classifications, forwardable_args)
         # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
         def add_post_ruby_32_offenses(def_node, send_classifications, forwardable_args)
           return unless use_anonymous_forwarding?
-          return if send_inside_block?(send_classifications)
+          return unless all_forwarding_offenses_correctable?(send_classifications)
 
           rest_arg, kwrest_arg, block_arg = *forwardable_args
 
           send_classifications.each do |send_node, _c, forward_rest, forward_kwrest, forward_block_arg| # rubocop:disable Layout/LineLength
-            if outside_block?(forward_rest)
+            if allow_anonymous_forwarding_in_block?(forward_rest)
               register_forward_args_offense(def_node.arguments, rest_arg)
               register_forward_args_offense(send_node, forward_rest)
             end
 
-            if outside_block?(forward_kwrest)
+            if allow_anonymous_forwarding_in_block?(forward_kwrest)
               register_forward_kwargs_offense(!forward_rest, def_node.arguments, kwrest_arg)
               register_forward_kwargs_offense(!forward_rest, send_node, forward_kwrest)
             end
 
-            # Prevents `anonymous block parameter is also used within block (SyntaxError)` occurs
-            # in Ruby 3.3.0.
-            if outside_block?(forward_block_arg)
+            if allow_anonymous_forwarding_in_block?(forward_block_arg)
               register_forward_block_arg_offense(!forward_rest, def_node.arguments, block_arg)
               register_forward_block_arg_offense(!forward_rest, send_node, forward_block_arg)
             end
@@ -293,8 +289,23 @@ def redundant_named_arg(arg, config_name, keyword)
           redundant_arg_names.include?(arg.source) ? arg : nil
         end
 
-        def outside_block?(node)
+        # Checks if forwarding is uses both in blocks and outside of blocks.
+        # On Ruby 3.3.0, anonymous block forwarding in blocks can be is a syntax
+        # error, so we only want to register an offense if we can change all occurrences.
+        def all_forwarding_offenses_correctable?(send_classifications)
+          return true if target_ruby_version >= 3.4
+
+          send_classifications.none? do |send_node, *|
+            send_node.each_ancestor(:block, :numblock).any?
+          end
+        end
+
+        # Ruby 3.3.0 had a bug where accessing an anonymous block argument inside of a block
+        # was a syntax error in unambiguous cases: https://bugs.ruby-lang.org/issues/20090
+        # We disallow this also for earlier Ruby versions so that code is forwards compatible.
+        def allow_anonymous_forwarding_in_block?(node)
           return false unless node
+          return true if target_ruby_version >= 3.4
 
           node.each_ancestor(:block, :numblock).none?
         end
@@ -357,12 +368,6 @@ def use_anonymous_forwarding?
           cop_config.fetch('UseAnonymousForwarding', false)
         end
 
-        def send_inside_block?(send_classifications)
-          send_classifications.any? do |send_node, *|
-            send_node.each_ancestor(:block, :numblock).any?
-          end
-        end
-
         def add_parens_if_missing(node, corrector)
           return if parentheses?(node)
           return if node.send_type? && node.method?(:[])

diff --git a/spec/rubocop/cop/naming/block_forwarding_spec.rb b/spec/rubocop/cop/naming/block_forwarding_spec.rb
@@ -191,118 +191,160 @@ def foo(k: v, &block)
         RUBY
       end
 
-      it 'does not register an offense when using explicit block forwarding in block method' do
-        # Prevents the following syntax error:
-        #
-        # # foo.rb
-        # def foo(&)
-        #   block_method do
-        #     bar(&)
-        #   end
-        # end
-        #
-        # $ ruby -vc foo.rb
-        # ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
-        # foo.rb: foo.rb:4: anonymous block parameter is also used within block (SyntaxError)
-        #
-        expect_no_offenses(<<~RUBY)
-          def foo(&block)
-            block_method do
+      # Ruby 3.3.0 had a bug where accessing an anonymous block argument inside of a block itself
+      # was a syntax error: https://bugs.ruby-lang.org/issues/20090
+      context 'when accessing the block inside of a block' do
+        it 'does not register an offense when using explicit block forwarding in block method' do
+          expect_no_offenses(<<~RUBY)
+            def foo(&block)
+              block_method do
+                bar(&block)
+              end
+            end
+          RUBY
+        end
+
+        it 'does not register an offense when using explicit block forwarding in numbered block method' do
+          expect_no_offenses(<<~RUBY)
+            def foo(&block)
+              block_method do
+                bar(&block)
+                baz(_1)
+              end
+            end
+          RUBY
+        end
+
+        it 'does not register an offense when using explicit block forwarding in block method and after' do
+          expect_no_offenses(<<~RUBY)
+            def foo(&block)
+              block_method do
+                bar(&block)
+              end
+              baz(&block)
+            end
+          RUBY
+        end
+
+        it 'does not register an offense when using explicit block forwarding in block method and before' do
+          expect_no_offenses(<<~RUBY)
+            def foo(&block)
               bar(&block)
+              block_method do
+                baz(&block)
+              end
             end
+          RUBY
+        end
+      end
+
+      it 'does not register an offense when defining no arguments method' do
+        expect_no_offenses(<<~RUBY)
+          def foo
           end
         RUBY
       end
 
-      it 'does not register an offense when using explicit block forwarding in numbered block method' do
-        # Prevents the following syntax error:
-        #
-        # # foo.rb
-        # def foo(&)
-        #   block_method do
-        #     bar(&)
-        #   end
-        # end
-        #
-        # $ ruby -vc foo.rb
-        # ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
-        # foo.rb: foo.rb:4: anonymous block parameter is also used within block (SyntaxError)
-        #
+      it 'does not register an offense when assigning the block arg' do
         expect_no_offenses(<<~RUBY)
+          def example(&block)
+            block ||= -> { :foo }
+            bar(&block)
+          end
+
+          def example(&block)
+            block = proc {} unless block_given?
+            bar(&block)
+          end
+        RUBY
+      end
+    end
+
+    context 'Ruby >= 3.4', :ruby34 do
+      it 'registers an offense when using explicit block forwarding in block method' do
+        expect_offense(<<~RUBY)
           def foo(&block)
+                  ^^^^^^ Use anonymous block forwarding.
             block_method do
               bar(&block)
-              baz(_1)
+                  ^^^^^^ Use anonymous block forwarding.
+            end
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          def foo(&)
+            block_method do
+              bar(&)
             end
           end
         RUBY
       end
 
-      it 'does not register an offense when using explicit block forwarding in block method and after' do
-        # Prevents the following syntax error:
-        #
-        # # foo.rb
-        # def foo(&)
-        #   block_method do
-        #     bar(&)
-        #   end
-        # end
-        #
-        # $ ruby -vc foo.rb
-        # ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
-        # foo.rb: foo.rb:4: anonymous block parameter is also used within block (SyntaxError)
-        #
-        expect_no_offenses(<<~RUBY)
+      it 'registers an offense when using explicit block forwarding in numbered block method' do
+        expect_offense(<<~RUBY)
           def foo(&block)
+                  ^^^^^^ Use anonymous block forwarding.
             block_method do
               bar(&block)
+                  ^^^^^^ Use anonymous block forwarding.
+              baz(_1)
+            end
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          def foo(&)
+            block_method do
+              bar(&)
+              baz(_1)
             end
-            baz(&block)
           end
         RUBY
       end
 
-      it 'does not register an offense when using explicit block forwarding in block method and before' do
-        # Prevents the following syntax error:
-        #
-        # # foo.rb
-        # def foo(&)
-        #   block_method do
-        #     bar(&)
-        #   end
-        # end
-        #
-        # $ ruby -vc foo.rb
-        # ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22]
-        # foo.rb: foo.rb:4: anonymous block parameter is also used within block (SyntaxError)
-        #
-        expect_no_offenses(<<~RUBY)
+      it 'registers an offense when using explicit block forwarding in block method and after' do
+        expect_offense(<<~RUBY)
           def foo(&block)
-            bar(&block)
+                  ^^^^^^ Use anonymous block forwarding.
             block_method do
-              baz(&block)
+              bar(&block)
+                  ^^^^^^ Use anonymous block forwarding.
             end
+            baz(&block)
+                ^^^^^^ Use anonymous block forwarding.
           end
         RUBY
-      end
 
-      it 'does not register an offense when defining no arguments method' do
-        expect_no_offenses(<<~RUBY)
-          def foo
+        expect_correction(<<~RUBY)
+          def foo(&)
+            block_method do
+              bar(&)
+            end
+            baz(&)
           end
         RUBY
       end
 
-      it 'does not register an offense when assigning the block arg' do
-        expect_no_offenses(<<~RUBY)
-          def example(&block)
-            block ||= -> { :foo }
+      it 'registers an offense when using explicit block forwarding in block method and before' do
+        expect_offense(<<~RUBY)
+          def foo(&block)
+                  ^^^^^^ Use anonymous block forwarding.
             bar(&block)
+                ^^^^^^ Use anonymous block forwarding.
+            block_method do
+              baz(&block)
+                  ^^^^^^ Use anonymous block forwarding.
+            end
           end
+        RUBY
 
-          def example(&block)
-            block = proc {} unless block_given?
-            bar(&block)
+        expect_correction(<<~RUBY)
+          def foo(&)
+            bar(&)
+            block_method do
+              baz(&)
+            end
           end
         RUBY
       end
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		* [#13741](https://github.com/rubocop/rubocop/pull/13741): Register an offense for `Naming/BlockForwarding` and `Style/ArgumentsForwarding` with Ruby >= 3.4 when the block argument is referenced inside a block. This was previously disabled because of a bug in Ruby 3.3.0. ([@earlopain][])