From cf3bbf9d2c21fca6c656e83883f9d37f683227b0 Mon Sep 17 00:00:00 2001 From: S-H-GAMELINKS Date: Mon, 21 Jul 2025 11:00:33 +0900 Subject: [PATCH 1/5] Make `it = it` assign `nil` to match parse.y behavior [Bug #21139] Currently Prism returns `42` for code like this: ```ruby 42.tap { it = it; p it } # => 42 ``` But parse.y returns `nil`: ```ruby 42.tap { it = it; p it } # => nil ``` In parse.y, it on the right-hand side is parsed as a local variable. In Prism, it was parsed as the implicit block parameter it, which caused this inconsistent behavior. This change makes the right-hand side it to be parsed as a local variable, aligning with parse.y's behavior. Bug ticket: https://bugs.ruby-lang.org/issues/21139 --- src/prism.c | 7 +++++++ test/prism/fixtures/it_assignment.txt | 1 + test/prism/ruby/parser_test.rb | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+) create mode 100644 test/prism/fixtures/it_assignment.txt diff --git a/src/prism.c b/src/prism.c index 6d4381565c..f9aee2b72f 100644 --- a/src/prism.c +++ b/src/prism.c @@ -16456,6 +16456,13 @@ parse_variable(pm_parser_t *parser) { return node; } else if ((parser->version != PM_OPTIONS_VERSION_CRUBY_3_3) && pm_token_is_it(parser->previous.start, parser->previous.end)) { + if (match1(parser, PM_TOKEN_EQUAL)) { + pm_constant_id_t name_id = pm_parser_local_add_location(parser, parser->previous.start, parser->previous.end, 0); + pm_node_t *node = (pm_node_t *) pm_local_variable_read_node_create_constant_id(parser, &parser->previous, name_id, 0, false); + + return node; + } + pm_node_t *node = (pm_node_t *) pm_it_local_variable_read_node_create(parser, &parser->previous); pm_node_list_append(¤t_scope->implicit_parameters, node); diff --git a/test/prism/fixtures/it_assignment.txt b/test/prism/fixtures/it_assignment.txt new file mode 100644 index 0000000000..523b0ffe1e --- /dev/null +++ b/test/prism/fixtures/it_assignment.txt @@ -0,0 +1 @@ +42.tap { it = it; p it } diff --git a/test/prism/ruby/parser_test.rb b/test/prism/ruby/parser_test.rb index 82b5ea54a8..add06dd8a6 100644 --- a/test/prism/ruby/parser_test.rb +++ b/test/prism/ruby/parser_test.rb @@ -179,6 +179,25 @@ def test_it_block_parameter_syntax assert_equal(it_block_parameter_sexp, actual_ast.to_sexp) end + def test_it_assignment_syntax + it_assignment_fixture_path = Pathname(__dir__).join('../../../test/prism/fixtures/it_assignment.txt') + + buffer = Parser::Source::Buffer.new(it_assignment_fixture_path) + buffer.source = it_assignment_fixture_path.read + actual_ast = Prism::Translation::Parser34.new.tokenize(buffer)[0] + + it_assignment_sexp = parse_sexp { + s(:block, + s(:send, s(:int, 42), :tap), + s(:args), + s(:begin, + s(:lvasgn, :it, s(:lvar, :it)), + s(:send, nil, :p, s(:lvar, :it)))) + } + + assert_equal(it_assignment_sexp, actual_ast.to_sexp) + end + private def assert_equal_parses(fixture, compare_asts: true, compare_tokens: true, compare_comments: true) From a6b448b10f7ef86a9baeff831ca56d132fb96437 Mon Sep 17 00:00:00 2001 From: S-H-GAMELINKS Date: Wed, 23 Jul 2025 21:20:50 +0900 Subject: [PATCH 2/5] Remove uneeded test --- test/prism/ruby/parser_test.rb | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/test/prism/ruby/parser_test.rb b/test/prism/ruby/parser_test.rb index add06dd8a6..82b5ea54a8 100644 --- a/test/prism/ruby/parser_test.rb +++ b/test/prism/ruby/parser_test.rb @@ -179,25 +179,6 @@ def test_it_block_parameter_syntax assert_equal(it_block_parameter_sexp, actual_ast.to_sexp) end - def test_it_assignment_syntax - it_assignment_fixture_path = Pathname(__dir__).join('../../../test/prism/fixtures/it_assignment.txt') - - buffer = Parser::Source::Buffer.new(it_assignment_fixture_path) - buffer.source = it_assignment_fixture_path.read - actual_ast = Prism::Translation::Parser34.new.tokenize(buffer)[0] - - it_assignment_sexp = parse_sexp { - s(:block, - s(:send, s(:int, 42), :tap), - s(:args), - s(:begin, - s(:lvasgn, :it, s(:lvar, :it)), - s(:send, nil, :p, s(:lvar, :it)))) - } - - assert_equal(it_assignment_sexp, actual_ast.to_sexp) - end - private def assert_equal_parses(fixture, compare_asts: true, compare_tokens: true, compare_comments: true) From 659d769621a461a889772aa37a5d9dbf0f8743fe Mon Sep 17 00:00:00 2001 From: S-H-GAMELINKS Date: Wed, 23 Jul 2025 21:23:47 +0900 Subject: [PATCH 3/5] Add it read and assignment test --- test/prism/fixtures/it_read_and_assignment.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 test/prism/fixtures/it_read_and_assignment.txt diff --git a/test/prism/fixtures/it_read_and_assignment.txt b/test/prism/fixtures/it_read_and_assignment.txt new file mode 100644 index 0000000000..2cceeb2a54 --- /dev/null +++ b/test/prism/fixtures/it_read_and_assignment.txt @@ -0,0 +1 @@ +42.tap { p it; it = it; p it } From 51e2b043a672d5ba75af42766cec72429060f622 Mon Sep 17 00:00:00 2001 From: S-H-GAMELINKS Date: Wed, 23 Jul 2025 21:53:35 +0900 Subject: [PATCH 4/5] Add snapshots --- snapshots/it_assignment.txt | 58 +++++++++++++++++++++ snapshots/it_read_and_assignment.txt | 75 ++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 snapshots/it_assignment.txt create mode 100644 snapshots/it_read_and_assignment.txt diff --git a/snapshots/it_assignment.txt b/snapshots/it_assignment.txt new file mode 100644 index 0000000000..83469791c0 --- /dev/null +++ b/snapshots/it_assignment.txt @@ -0,0 +1,58 @@ +@ ProgramNode (location: (1,0)-(1,24)) +├── flags: ∅ +├── locals: [] +└── statements: + @ StatementsNode (location: (1,0)-(1,24)) + ├── flags: ∅ + └── body: (length: 1) + └── @ CallNode (location: (1,0)-(1,24)) + ├── flags: newline + ├── receiver: + │ @ IntegerNode (location: (1,0)-(1,2)) + │ ├── flags: static_literal, decimal + │ └── value: 42 + ├── call_operator_loc: (1,2)-(1,3) = "." + ├── name: :tap + ├── message_loc: (1,3)-(1,6) = "tap" + ├── opening_loc: ∅ + ├── arguments: ∅ + ├── closing_loc: ∅ + └── block: + @ BlockNode (location: (1,7)-(1,24)) + ├── flags: ∅ + ├── locals: [:it] + ├── parameters: ∅ + ├── body: + │ @ StatementsNode (location: (1,9)-(1,22)) + │ ├── flags: ∅ + │ └── body: (length: 2) + │ ├── @ LocalVariableWriteNode (location: (1,9)-(1,16)) + │ │ ├── flags: newline + │ │ ├── name: :it + │ │ ├── depth: 0 + │ │ ├── name_loc: (1,9)-(1,11) = "it" + │ │ ├── value: + │ │ │ @ LocalVariableReadNode (location: (1,14)-(1,16)) + │ │ │ ├── flags: ∅ + │ │ │ ├── name: :it + │ │ │ └── depth: 0 + │ │ └── operator_loc: (1,12)-(1,13) = "=" + │ └── @ CallNode (location: (1,18)-(1,22)) + │ ├── flags: newline, ignore_visibility + │ ├── receiver: ∅ + │ ├── call_operator_loc: ∅ + │ ├── name: :p + │ ├── message_loc: (1,18)-(1,19) = "p" + │ ├── opening_loc: ∅ + │ ├── arguments: + │ │ @ ArgumentsNode (location: (1,20)-(1,22)) + │ │ ├── flags: ∅ + │ │ └── arguments: (length: 1) + │ │ └── @ LocalVariableReadNode (location: (1,20)-(1,22)) + │ │ ├── flags: ∅ + │ │ ├── name: :it + │ │ └── depth: 0 + │ ├── closing_loc: ∅ + │ └── block: ∅ + ├── opening_loc: (1,7)-(1,8) = "{" + └── closing_loc: (1,23)-(1,24) = "}" diff --git a/snapshots/it_read_and_assignment.txt b/snapshots/it_read_and_assignment.txt new file mode 100644 index 0000000000..73aa2b5909 --- /dev/null +++ b/snapshots/it_read_and_assignment.txt @@ -0,0 +1,75 @@ +@ ProgramNode (location: (1,0)-(1,30)) +├── flags: ∅ +├── locals: [] +└── statements: + @ StatementsNode (location: (1,0)-(1,30)) + ├── flags: ∅ + └── body: (length: 1) + └── @ CallNode (location: (1,0)-(1,30)) + ├── flags: newline + ├── receiver: + │ @ IntegerNode (location: (1,0)-(1,2)) + │ ├── flags: static_literal, decimal + │ └── value: 42 + ├── call_operator_loc: (1,2)-(1,3) = "." + ├── name: :tap + ├── message_loc: (1,3)-(1,6) = "tap" + ├── opening_loc: ∅ + ├── arguments: ∅ + ├── closing_loc: ∅ + └── block: + @ BlockNode (location: (1,7)-(1,30)) + ├── flags: ∅ + ├── locals: [:it] + ├── parameters: + │ @ ItParametersNode (location: (1,7)-(1,30)) + │ └── flags: ∅ + ├── body: + │ @ StatementsNode (location: (1,9)-(1,28)) + │ ├── flags: ∅ + │ └── body: (length: 3) + │ ├── @ CallNode (location: (1,9)-(1,13)) + │ │ ├── flags: newline, ignore_visibility + │ │ ├── receiver: ∅ + │ │ ├── call_operator_loc: ∅ + │ │ ├── name: :p + │ │ ├── message_loc: (1,9)-(1,10) = "p" + │ │ ├── opening_loc: ∅ + │ │ ├── arguments: + │ │ │ @ ArgumentsNode (location: (1,11)-(1,13)) + │ │ │ ├── flags: ∅ + │ │ │ └── arguments: (length: 1) + │ │ │ └── @ ItLocalVariableReadNode (location: (1,11)-(1,13)) + │ │ │ └── flags: ∅ + │ │ ├── closing_loc: ∅ + │ │ └── block: ∅ + │ ├── @ LocalVariableWriteNode (location: (1,15)-(1,22)) + │ │ ├── flags: newline + │ │ ├── name: :it + │ │ ├── depth: 0 + │ │ ├── name_loc: (1,15)-(1,17) = "it" + │ │ ├── value: + │ │ │ @ LocalVariableReadNode (location: (1,20)-(1,22)) + │ │ │ ├── flags: ∅ + │ │ │ ├── name: :it + │ │ │ └── depth: 0 + │ │ └── operator_loc: (1,18)-(1,19) = "=" + │ └── @ CallNode (location: (1,24)-(1,28)) + │ ├── flags: newline, ignore_visibility + │ ├── receiver: ∅ + │ ├── call_operator_loc: ∅ + │ ├── name: :p + │ ├── message_loc: (1,24)-(1,25) = "p" + │ ├── opening_loc: ∅ + │ ├── arguments: + │ │ @ ArgumentsNode (location: (1,26)-(1,28)) + │ │ ├── flags: ∅ + │ │ └── arguments: (length: 1) + │ │ └── @ LocalVariableReadNode (location: (1,26)-(1,28)) + │ │ ├── flags: ∅ + │ │ ├── name: :it + │ │ └── depth: 0 + │ ├── closing_loc: ∅ + │ └── block: ∅ + ├── opening_loc: (1,7)-(1,8) = "{" + └── closing_loc: (1,29)-(1,30) = "}" From fb136c6eb525747e88b46db3251201585cb5e0d2 Mon Sep 17 00:00:00 2001 From: S-H-GAMELINKS Date: Mon, 28 Jul 2025 23:25:49 +0900 Subject: [PATCH 5/5] Convert implicit parameter `it` to local variable in `parse_expression_infix` function --- src/prism.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/prism.c b/src/prism.c index f9aee2b72f..32930b900e 100644 --- a/src/prism.c +++ b/src/prism.c @@ -16456,13 +16456,6 @@ parse_variable(pm_parser_t *parser) { return node; } else if ((parser->version != PM_OPTIONS_VERSION_CRUBY_3_3) && pm_token_is_it(parser->previous.start, parser->previous.end)) { - if (match1(parser, PM_TOKEN_EQUAL)) { - pm_constant_id_t name_id = pm_parser_local_add_location(parser, parser->previous.start, parser->previous.end, 0); - pm_node_t *node = (pm_node_t *) pm_local_variable_read_node_create_constant_id(parser, &parser->previous, name_id, 0, false); - - return node; - } - pm_node_t *node = (pm_node_t *) pm_it_local_variable_read_node_create(parser, &parser->previous); pm_node_list_append(¤t_scope->implicit_parameters, node); @@ -21178,6 +21171,13 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t } PRISM_FALLTHROUGH case PM_CASE_WRITABLE: { + // When we have `it = value`, we need to add `it` as a local + // variable before parsing the value, in case the value + // references the variable. + if (PM_NODE_TYPE_P(node, PM_IT_LOCAL_VARIABLE_READ_NODE)) { + pm_parser_local_add_location(parser, node->location.start, node->location.end, 0); + } + parser_lex(parser); pm_node_t *value = parse_assignment_values(parser, previous_binding_power, PM_NODE_TYPE_P(node, PM_MULTI_TARGET_NODE) ? PM_BINDING_POWER_MULTI_ASSIGNMENT + 1 : binding_power, accepts_command_call, PM_ERR_EXPECT_EXPRESSION_AFTER_EQUAL, (uint16_t) (depth + 1));