Add support for `itblock` node for Ruby 3.4 #365

Earlopain · 2025-03-19T11:00:38Z

In tests, prism will always be used for Ruby 3.4+ and tests that don't pass with parser get marked as broken.

Needs whitequark/parser#1071, draft for now. The minimum parser version must be bumped in this PR, as otherwise itblock_type? method is not available.

koic · 2025-03-20T08:51:22Z

Parser 3.3.7.2 has been released. Hopefully, this can move things forward.
https://rubygems.org/gems/parser/versions/3.3.7.2

Earlopain · 2025-03-20T09:05:11Z

That should indeed be very useful, thanks! I bumped the required parser version to pull that in.

koic · 2025-03-20T10:11:38Z

lib/rubocop/ast/node/block_node.rb

        max_param = children[1]
        1.upto(max_param).map do |i|
          ArgNode.new(:arg, [:"_#{i}"])
        end.freeze
      end
+
+      def it_arguments


Nits, considering the it syntax, it is probably singular, isn't it?

Suggested change

def it_arguments

def it_argument

True, but it returns an array so I was a bit conflicted.

koic · 2025-03-20T10:11:39Z

lib/rubocop/ast/node/block_node.rb

        max_param = children[1]
        1.upto(max_param).map do |i|
          ArgNode.new(:arg, [:"_#{i}"])
        end.freeze
      end
+
+      def it_arguments
+        [ArgNode.new(:arg, [:it])].freeze


It may be possible to consider making it a constant.

That works, sure

koic · 2025-03-20T10:13:43Z

docs/modules/ROOT/pages/node_types.adoc

@@ -170,6 +170,8 @@ The following fields are given when relevant to nodes in the source code:

 |numblock|Block that has numbered arguments (`_1`) referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is an `int` (the number of numeric arguments) and the third child is a body statement.|proc { _1 + _3 }|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/BlockNode[BlockNode]

+|itblock|Block that has the implicit it parameter (`it`) referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.|proc { it }|N/A


Suggested change

|itblock|Block that has the implicit it parameter (`it`) referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.|proc { it }|N/A

|itblock|Block that has the implicit `it` parameter (`it`) referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.|proc { it }|N/A

koic · 2025-03-20T10:37:23Z

lib/rubocop/ast/node/block_node.rb

        max_param = children[1]
        1.upto(max_param).map do |i|
          ArgNode.new(:arg, [:"_#{i}"])
        end.freeze
      end
+
+      IMPLICIT_IT_ARGUMENT = [ArgNode.new(:arg, [:it])].freeze
+      private_constant :IMPLICIT_IT_ARGUMENT


Can you move this constant definition above method definitions?

include MethodIdentifierPredicates + IMPLICIT_IT_ARGUMENT = [ArgNode.new(:arg, [:it])].freeze + private_constant :IMPLICIT_IT_ARGUMENT VOID_CONTEXT_METHODS = %i[each tap].freeze private_constant :VOID_CONTEXT_METHODS

And IT_BLOCK_ARGUMENT might be a clearer name.

I rather like this where it is now because it is locally close to the the numbered parameter method. Should I still move it?

Thank you for explaining the intention. However, constants are generally expected to be at the top, so please move it accordingly. This will also maintain consistency with how existing constants are handled in this file.

koic · 2025-03-20T11:41:53Z

spec/rubocop/ast/traversal_spec.rb

+
+        include RuboCop::AST::Traversal
+        def on_itblock(node)
+          (@calls ||= []) << node.children.first&.type


Safe navigation seems unnecessary for this test case.

Suggested change

(@calls ||= []) << node.children.first&.type

(@calls ||= []) << node.children.first.type

I removed it

koic · 2025-03-20T11:41:54Z

spec/rubocop/ast/traversal_spec.rb

+      foo { bar(it) }
+      xxx { _1 }
+      yyy { |zzz| }


This example might be slightly adjustable. e.g.,:

Suggested change

foo { bar(it) }

xxx { _1 }

yyy { |zzz| }

it_block { do_something(it) }

numbered_block { do_something(_1) }

normal_block { |arg| do_something(arg) }

koic · 2025-03-20T14:03:36Z

docs/modules/ROOT/pages/node_types.adoc

@@ -170,6 +170,8 @@ The following fields are given when relevant to nodes in the source code:

 |numblock|Block that has numbered arguments (`_1`) referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is an `int` (the number of numeric arguments) and the third child is a body statement.|proc { _1 + _3 }|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/BlockNode[BlockNode]

+|itblock|Block that has the implicit `it` parameter referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.|proc { it }|N/A


This is just a terminology adjustment for the PR overall, "implicit it parameter" doesn't seem to be a commonly used term. How about simply using "it block parameter"?

Suggested change

|itblock|Block that has the implicit `it` parameter referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.|proc { it }|N/A

|itblock|Block that has the `it` block parameter referenced inside it.|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.|proc { it }|N/A

The naming seems to be intended to contrast with the traditional explicit do |it|. However, when "it block parameter" is mentioned, it typically refers to the "it block parameter" introduced in Ruby 3.4.

Sure, updated.

In tests, `parser` will be capped to Ruby 3.3 and tests that don't pass with get marked as broken.

koic · 2025-03-20T15:32:17Z

Looks good to me. Thanks!

koic · 2025-03-20T15:40:07Z

This change affects existing cops that use any_block, for users who have set ParserEngine: parser_prism. For now, I've confirmed that the latest build passes based on the rubocop/rubocop repository. Let's proceed with the release and monitor the situation. If any feedback arises, it will likely be directed at rubocop/rubocop rather than rubocop/rubocop-ast.

I will proceed with the minor version release of this PR. @marcandre @bbatsov

bbatsov · 2025-03-20T15:45:30Z

Sounds good!

koic · 2025-03-20T15:50:18Z

RuboCop AST 1.41.0 has been released:
https://github.com/rubocop/rubocop-ast/releases/tag/v1.41.0

Earlopain · 2025-03-20T16:06:35Z

Great, thank you!

Follow-up to rubocop/rubocop-ast#365 (comment) "implicit `it` parameter" doesn't seem to be a commonly used term. Use "`it` block parameter" instead.

This was referenced Mar 19, 2025

+ add prism-specific node types whitequark/parser#1071

Merged

Require prism as a parser to analyse newly released ruby versions rubocop/rubocop#13617

Closed

davidrunger mentioned this pull request Mar 19, 2025

undefined method 'on_itblock' for an instance of RuboCop::Cop::Commissioner with prism 1.4.0 and rubocop-ast 1.40.0 #366

Closed

Earlopain force-pushed the itblock branch from bde56f2 to 29f0d67 Compare March 20, 2025 09:02

Earlopain marked this pull request as ready for review March 20, 2025 09:05

Earlopain force-pushed the itblock branch from 29f0d67 to 8bf4c0c Compare March 20, 2025 09:17

Earlopain mentioned this pull request Mar 20, 2025

Remove a bit of backwards compat code #368

Merged

koic reviewed Mar 20, 2025

View reviewed changes

Earlopain force-pushed the itblock branch from 8bf4c0c to 8de39cd Compare March 20, 2025 10:26

koic reviewed Mar 20, 2025

View reviewed changes

Earlopain force-pushed the itblock branch from 8de39cd to 32e67df Compare March 20, 2025 11:03

koic reviewed Mar 20, 2025

View reviewed changes

Earlopain force-pushed the itblock branch from 32e67df to ff6d583 Compare March 20, 2025 12:27

koic reviewed Mar 20, 2025

View reviewed changes

Add support for itblock node for Ruby 3.4

Loading
Loading status checks…

e6f23d8

In tests, `parser` will be capped to Ruby 3.3 and tests that don't pass with get marked as broken.

Earlopain force-pushed the itblock branch from ff6d583 to e6f23d8 Compare March 20, 2025 14:58

koic merged commit 7c71213 into rubocop:master Mar 20, 2025
22 checks passed

Earlopain deleted the itblock branch March 20, 2025 16:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Add support for `itblock` node for Ruby 3.4 #365

Add support for `itblock` node for Ruby 3.4 #365

Earlopain commented Mar 19, 2025 •

edited

Loading

koic commented Mar 20, 2025

Earlopain commented Mar 20, 2025

koic Mar 20, 2025

Earlopain Mar 20, 2025

koic Mar 20, 2025

Earlopain Mar 20, 2025

koic Mar 20, 2025

koic Mar 20, 2025 •

edited

Loading

koic Mar 20, 2025

Earlopain Mar 20, 2025

koic Mar 20, 2025

koic Mar 20, 2025 •

edited

Loading

Earlopain Mar 20, 2025

koic Mar 20, 2025

koic Mar 20, 2025

Earlopain Mar 20, 2025

koic commented Mar 20, 2025

koic commented Mar 20, 2025

bbatsov commented Mar 20, 2025

koic commented Mar 20, 2025

Earlopain commented Mar 20, 2025

		@@ -170,6 +170,8 @@ The following fields are given when relevant to nodes in the source code:

		\|numblock\|Block that has numbered arguments (`_1`) referenced inside it.\|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is an `int` (the number of numeric arguments) and the third child is a body statement.\|proc { _1 + _3 }\|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/BlockNode[BlockNode]

		\|itblock\|Block that has the implicit it parameter (`it`) referenced inside it.\|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.\|proc { it }\|N/A

	\|itblock\|Block that has the implicit it parameter (`it`) referenced inside it.\|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.\|proc { it }\|N/A
	\|itblock\|Block that has the implicit `it` parameter (`it`) referenced inside it.\|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.\|proc { it }\|N/A

	(@calls \|\|= []) << node.children.first&.type
	(@calls \|\|= []) << node.children.first.type

	\|itblock\|Block that has the implicit `it` parameter referenced inside it.\|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.\|proc { it }\|N/A
	\|itblock\|Block that has the `it` block parameter referenced inside it.\|Three children. First child is a `send`/`csend` node representing the way the block is created, second child is the symbol `it` and the third child is a body statement.\|proc { it }\|N/A

Add support for itblock node for Ruby 3.4 #365

Add support for itblock node for Ruby 3.4 #365

Conversation

Earlopain commented Mar 19, 2025 • edited Loading

koic commented Mar 20, 2025

Earlopain commented Mar 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koic Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koic Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koic commented Mar 20, 2025

koic commented Mar 20, 2025

bbatsov commented Mar 20, 2025

koic commented Mar 20, 2025

Earlopain commented Mar 20, 2025

Add support for `itblock` node for Ruby 3.4 #365

Add support for `itblock` node for Ruby 3.4 #365

Earlopain commented Mar 19, 2025 •

edited

Loading

koic Mar 20, 2025 •

edited

Loading

koic Mar 20, 2025 •

edited

Loading