From ed1148841573dac92e16b347f19fbaca0a609b33 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Fri, 14 Nov 2025 11:36:48 +0100 Subject: [PATCH 1/2] Remove dead code See https://bugs.ruby-lang.org/issues/21669#note-4 > At line 1067, void_node = vn is executed (and vn is guaranteed to be non-NULL on the line above), so it's impossible for void_node to be NULL at line 1075. Thus, it is indeed dead code. --- src/prism.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/prism.c b/src/prism.c index 2c03961285..6f9a19ef81 100644 --- a/src/prism.c +++ b/src/prism.c @@ -1102,9 +1102,6 @@ pm_check_value_expression(pm_parser_t *parser, pm_node_t *node) { void_node = NULL; break; } - if (void_node == NULL) { - void_node = vn; - } } if (cast->else_clause != NULL) { From ac656013d8102681cd3e0705ed1d5fe7bc3d43d2 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Thu, 29 Jan 2026 20:27:20 +0100 Subject: [PATCH 2/2] [Bug #21669] Thoroughly implement void value expression check --- snapshots/3.3-4.0/void_value.txt | 247 +++++++++++++++ snapshots/4.1/void_value.txt | 59 ++++ snapshots/non_void_value.txt | 288 ++++++++++++++++++ src/prism.c | 96 +++++- .../singleton_method_with_void_value.txt | 3 + test/prism/errors/3.4-4.0/void_value.txt | 18 ++ .../4.1/singleton_method_with_void_value.txt | 4 + test/prism/errors/4.1/void_value.txt | 44 +++ .../errors/singleton_method_for_literals.txt | 2 - ...id_value_expression_in_begin_statement.txt | 2 - test/prism/fixtures/3.3-4.0/void_value.txt | 29 ++ test/prism/fixtures/4.1/void_value.txt | 7 + test/prism/fixtures/non_void_value.txt | 31 ++ test/prism/fixtures_test.rb | 3 + test/prism/locals_test.rb | 3 + test/prism/result/warnings_test.rb | 19 ++ test/prism/ruby/ripper_test.rb | 3 + test/prism/ruby/ruby_parser_test.rb | 2 + 18 files changed, 846 insertions(+), 14 deletions(-) create mode 100644 snapshots/3.3-4.0/void_value.txt create mode 100644 snapshots/4.1/void_value.txt create mode 100644 snapshots/non_void_value.txt create mode 100644 test/prism/errors/3.3-4.0/singleton_method_with_void_value.txt create mode 100644 test/prism/errors/3.4-4.0/void_value.txt create mode 100644 test/prism/errors/4.1/singleton_method_with_void_value.txt create mode 100644 test/prism/errors/4.1/void_value.txt create mode 100644 test/prism/fixtures/3.3-4.0/void_value.txt create mode 100644 test/prism/fixtures/4.1/void_value.txt create mode 100644 test/prism/fixtures/non_void_value.txt diff --git a/snapshots/3.3-4.0/void_value.txt b/snapshots/3.3-4.0/void_value.txt new file mode 100644 index 0000000000..b5f501cd40 --- /dev/null +++ b/snapshots/3.3-4.0/void_value.txt @@ -0,0 +1,247 @@ +@ ProgramNode (location: (1,0)-(29,3)) +├── flags: ∅ +├── locals: [:x] +└── statements: + @ StatementsNode (location: (1,0)-(29,3)) + ├── flags: ∅ + └── body: (length: 5) + ├── @ LocalVariableWriteNode (location: (1,0)-(7,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (1,0)-(1,1) = "x" + │ ├── value: + │ │ @ BeginNode (location: (1,4)-(7,3)) + │ │ ├── flags: ∅ + │ │ ├── begin_keyword_loc: (1,4)-(1,9) = "begin" + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (2,2)-(2,5)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 1) + │ │ │ └── @ CallNode (location: (2,2)-(2,5)) + │ │ │ ├── flags: newline, variable_call, ignore_visibility + │ │ │ ├── receiver: ∅ + │ │ │ ├── call_operator_loc: ∅ + │ │ │ ├── name: :foo + │ │ │ ├── message_loc: (2,2)-(2,5) = "foo" + │ │ │ ├── opening_loc: ∅ + │ │ │ ├── arguments: ∅ + │ │ │ ├── closing_loc: ∅ + │ │ │ ├── equal_loc: ∅ + │ │ │ └── block: ∅ + │ │ ├── rescue_clause: + │ │ │ @ RescueNode (location: (3,0)-(4,8)) + │ │ │ ├── flags: ∅ + │ │ │ ├── keyword_loc: (3,0)-(3,6) = "rescue" + │ │ │ ├── exceptions: (length: 0) + │ │ │ ├── operator_loc: ∅ + │ │ │ ├── reference: ∅ + │ │ │ ├── then_keyword_loc: ∅ + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (4,2)-(4,8)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (4,2)-(4,8)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (4,2)-(4,8) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ └── subsequent: ∅ + │ │ ├── else_clause: + │ │ │ @ ElseNode (location: (5,0)-(7,3)) + │ │ │ ├── flags: ∅ + │ │ │ ├── else_keyword_loc: (5,0)-(5,4) = "else" + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (6,2)-(6,8)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (6,2)-(6,8)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (6,2)-(6,8) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ └── end_keyword_loc: (7,0)-(7,3) = "end" + │ │ ├── ensure_clause: ∅ + │ │ └── end_keyword_loc: (7,0)-(7,3) = "end" + │ └── operator_loc: (1,2)-(1,3) = "=" + ├── @ LocalVariableWriteNode (location: (9,0)-(12,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (9,0)-(9,1) = "x" + │ ├── value: + │ │ @ CaseNode (location: (9,4)-(12,3)) + │ │ ├── flags: ∅ + │ │ ├── predicate: ∅ + │ │ ├── conditions: (length: 1) + │ │ │ └── @ WhenNode (location: (10,2)-(10,20)) + │ │ │ ├── flags: ∅ + │ │ │ ├── keyword_loc: (10,2)-(10,6) = "when" + │ │ │ ├── conditions: (length: 1) + │ │ │ │ └── @ IntegerNode (location: (10,7)-(10,8)) + │ │ │ │ ├── flags: static_literal, decimal + │ │ │ │ └── value: 1 + │ │ │ ├── then_keyword_loc: (10,9)-(10,13) = "then" + │ │ │ └── statements: + │ │ │ @ StatementsNode (location: (10,14)-(10,20)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 1) + │ │ │ └── @ ReturnNode (location: (10,14)-(10,20)) + │ │ │ ├── flags: newline + │ │ │ ├── keyword_loc: (10,14)-(10,20) = "return" + │ │ │ └── arguments: ∅ + │ │ ├── else_clause: + │ │ │ @ ElseNode (location: (11,2)-(12,3)) + │ │ │ ├── flags: ∅ + │ │ │ ├── else_keyword_loc: (11,2)-(11,6) = "else" + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (11,14)-(11,20)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (11,14)-(11,20)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (11,14)-(11,20) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ └── end_keyword_loc: (12,0)-(12,3) = "end" + │ │ ├── case_keyword_loc: (9,4)-(9,8) = "case" + │ │ └── end_keyword_loc: (12,0)-(12,3) = "end" + │ └── operator_loc: (9,2)-(9,3) = "=" + ├── @ LocalVariableWriteNode (location: (14,0)-(17,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (14,0)-(14,1) = "x" + │ ├── value: + │ │ @ CaseMatchNode (location: (14,4)-(17,3)) + │ │ ├── flags: ∅ + │ │ ├── predicate: + │ │ │ @ IntegerNode (location: (14,9)-(14,10)) + │ │ │ ├── flags: static_literal, decimal + │ │ │ └── value: 1 + │ │ ├── conditions: (length: 1) + │ │ │ └── @ InNode (location: (15,2)-(15,18)) + │ │ │ ├── flags: ∅ + │ │ │ ├── pattern: + │ │ │ │ @ IntegerNode (location: (15,5)-(15,6)) + │ │ │ │ ├── flags: static_literal, decimal + │ │ │ │ └── value: 2 + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (15,12)-(15,18)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (15,12)-(15,18)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (15,12)-(15,18) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ ├── in_loc: (15,2)-(15,4) = "in" + │ │ │ └── then_loc: (15,7)-(15,11) = "then" + │ │ ├── else_clause: + │ │ │ @ ElseNode (location: (16,2)-(17,3)) + │ │ │ ├── flags: ∅ + │ │ │ ├── else_keyword_loc: (16,2)-(16,6) = "else" + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (16,12)-(16,18)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (16,12)-(16,18)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (16,12)-(16,18) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ └── end_keyword_loc: (17,0)-(17,3) = "end" + │ │ ├── case_keyword_loc: (14,4)-(14,8) = "case" + │ │ └── end_keyword_loc: (17,0)-(17,3) = "end" + │ └── operator_loc: (14,2)-(14,3) = "=" + ├── @ LocalVariableWriteNode (location: (19,0)-(22,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (19,0)-(19,1) = "x" + │ ├── value: + │ │ @ BeginNode (location: (19,4)-(22,3)) + │ │ ├── flags: ∅ + │ │ ├── begin_keyword_loc: (19,4)-(19,9) = "begin" + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (20,2)-(21,6)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 2) + │ │ │ ├── @ ReturnNode (location: (20,2)-(20,8)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (20,2)-(20,8) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ └── @ StringNode (location: (21,2)-(21,6)) + │ │ │ ├── flags: newline + │ │ │ ├── opening_loc: (21,2)-(21,3) = "\"" + │ │ │ ├── content_loc: (21,3)-(21,5) = "NG" + │ │ │ ├── closing_loc: (21,5)-(21,6) = "\"" + │ │ │ └── unescaped: "NG" + │ │ ├── rescue_clause: ∅ + │ │ ├── else_clause: ∅ + │ │ ├── ensure_clause: ∅ + │ │ └── end_keyword_loc: (22,0)-(22,3) = "end" + │ └── operator_loc: (19,2)-(19,3) = "=" + └── @ LocalVariableWriteNode (location: (24,0)-(29,3)) + ├── flags: newline + ├── name: :x + ├── depth: 0 + ├── name_loc: (24,0)-(24,1) = "x" + ├── value: + │ @ IfNode (location: (24,4)-(29,3)) + │ ├── flags: newline + │ ├── if_keyword_loc: (24,4)-(24,6) = "if" + │ ├── predicate: + │ │ @ CallNode (location: (24,7)-(24,17)) + │ │ ├── flags: ∅ + │ │ ├── receiver: + │ │ │ @ CallNode (location: (24,7)-(24,11)) + │ │ │ ├── flags: variable_call, ignore_visibility + │ │ │ ├── receiver: ∅ + │ │ │ ├── call_operator_loc: ∅ + │ │ │ ├── name: :rand + │ │ │ ├── message_loc: (24,7)-(24,11) = "rand" + │ │ │ ├── opening_loc: ∅ + │ │ │ ├── arguments: ∅ + │ │ │ ├── closing_loc: ∅ + │ │ │ ├── equal_loc: ∅ + │ │ │ └── block: ∅ + │ │ ├── call_operator_loc: ∅ + │ │ ├── name: :< + │ │ ├── message_loc: (24,12)-(24,13) = "<" + │ │ ├── opening_loc: ∅ + │ │ ├── arguments: + │ │ │ @ ArgumentsNode (location: (24,14)-(24,17)) + │ │ │ ├── flags: ∅ + │ │ │ └── arguments: (length: 1) + │ │ │ └── @ FloatNode (location: (24,14)-(24,17)) + │ │ │ ├── flags: static_literal + │ │ │ └── value: 0.5 + │ │ ├── closing_loc: ∅ + │ │ ├── equal_loc: ∅ + │ │ └── block: ∅ + │ ├── then_keyword_loc: ∅ + │ ├── statements: + │ │ @ StatementsNode (location: (25,2)-(26,6)) + │ │ ├── flags: ∅ + │ │ └── body: (length: 2) + │ │ ├── @ ReturnNode (location: (25,2)-(25,8)) + │ │ │ ├── flags: newline + │ │ │ ├── keyword_loc: (25,2)-(25,8) = "return" + │ │ │ └── arguments: ∅ + │ │ └── @ StringNode (location: (26,2)-(26,6)) + │ │ ├── flags: newline + │ │ ├── opening_loc: (26,2)-(26,3) = "\"" + │ │ ├── content_loc: (26,3)-(26,5) = "NG" + │ │ ├── closing_loc: (26,5)-(26,6) = "\"" + │ │ └── unescaped: "NG" + │ ├── subsequent: + │ │ @ ElseNode (location: (27,0)-(29,3)) + │ │ ├── flags: ∅ + │ │ ├── else_keyword_loc: (27,0)-(27,4) = "else" + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (28,2)-(28,8)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 1) + │ │ │ └── @ ReturnNode (location: (28,2)-(28,8)) + │ │ │ ├── flags: newline + │ │ │ ├── keyword_loc: (28,2)-(28,8) = "return" + │ │ │ └── arguments: ∅ + │ │ └── end_keyword_loc: (29,0)-(29,3) = "end" + │ └── end_keyword_loc: (29,0)-(29,3) = "end" + └── operator_loc: (24,2)-(24,3) = "=" diff --git a/snapshots/4.1/void_value.txt b/snapshots/4.1/void_value.txt new file mode 100644 index 0000000000..b6ca0253aa --- /dev/null +++ b/snapshots/4.1/void_value.txt @@ -0,0 +1,59 @@ +@ ProgramNode (location: (1,0)-(7,3)) +├── flags: ∅ +├── locals: [:x] +└── statements: + @ StatementsNode (location: (1,0)-(7,3)) + ├── flags: ∅ + └── body: (length: 1) + └── @ LocalVariableWriteNode (location: (1,0)-(7,3)) + ├── flags: newline + ├── name: :x + ├── depth: 0 + ├── name_loc: (1,0)-(1,1) = "x" + ├── value: + │ @ BeginNode (location: (1,4)-(7,3)) + │ ├── flags: ∅ + │ ├── begin_keyword_loc: (1,4)-(1,9) = "begin" + │ ├── statements: + │ │ @ StatementsNode (location: (2,2)-(2,8)) + │ │ ├── flags: ∅ + │ │ └── body: (length: 1) + │ │ └── @ ReturnNode (location: (2,2)-(2,8)) + │ │ ├── flags: newline + │ │ ├── keyword_loc: (2,2)-(2,8) = "return" + │ │ └── arguments: ∅ + │ ├── rescue_clause: + │ │ @ RescueNode (location: (3,0)-(4,6)) + │ │ ├── flags: ∅ + │ │ ├── keyword_loc: (3,0)-(3,6) = "rescue" + │ │ ├── exceptions: (length: 0) + │ │ ├── operator_loc: ∅ + │ │ ├── reference: ∅ + │ │ ├── then_keyword_loc: ∅ + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (4,2)-(4,6)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 1) + │ │ │ └── @ StringNode (location: (4,2)-(4,6)) + │ │ │ ├── flags: newline + │ │ │ ├── opening_loc: (4,2)-(4,3) = "\"" + │ │ │ ├── content_loc: (4,3)-(4,5) = "OK" + │ │ │ ├── closing_loc: (4,5)-(4,6) = "\"" + │ │ │ └── unescaped: "OK" + │ │ └── subsequent: ∅ + │ ├── else_clause: + │ │ @ ElseNode (location: (5,0)-(7,3)) + │ │ ├── flags: ∅ + │ │ ├── else_keyword_loc: (5,0)-(5,4) = "else" + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (6,2)-(6,8)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 1) + │ │ │ └── @ ReturnNode (location: (6,2)-(6,8)) + │ │ │ ├── flags: newline + │ │ │ ├── keyword_loc: (6,2)-(6,8) = "return" + │ │ │ └── arguments: ∅ + │ │ └── end_keyword_loc: (7,0)-(7,3) = "end" + │ ├── ensure_clause: ∅ + │ └── end_keyword_loc: (7,0)-(7,3) = "end" + └── operator_loc: (1,2)-(1,3) = "=" diff --git a/snapshots/non_void_value.txt b/snapshots/non_void_value.txt new file mode 100644 index 0000000000..8e7ab3e75f --- /dev/null +++ b/snapshots/non_void_value.txt @@ -0,0 +1,288 @@ +@ ProgramNode (location: (1,0)-(31,3)) +├── flags: ∅ +├── locals: [:x] +└── statements: + @ StatementsNode (location: (1,0)-(31,3)) + ├── flags: ∅ + └── body: (length: 5) + ├── @ LocalVariableWriteNode (location: (1,0)-(4,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (1,0)-(1,1) = "x" + │ ├── value: + │ │ @ BeginNode (location: (1,4)-(4,3)) + │ │ ├── flags: ∅ + │ │ ├── begin_keyword_loc: (1,4)-(1,9) = "begin" + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (2,2)-(3,15)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 2) + │ │ │ ├── @ IfNode (location: (2,2)-(2,16)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── if_keyword_loc: (2,9)-(2,11) = "if" + │ │ │ │ ├── predicate: + │ │ │ │ │ @ TrueNode (location: (2,12)-(2,16)) + │ │ │ │ │ └── flags: static_literal + │ │ │ │ ├── then_keyword_loc: ∅ + │ │ │ │ ├── statements: + │ │ │ │ │ @ StatementsNode (location: (2,2)-(2,8)) + │ │ │ │ │ ├── flags: ∅ + │ │ │ │ │ └── body: (length: 1) + │ │ │ │ │ └── @ ReturnNode (location: (2,2)-(2,8)) + │ │ │ │ │ ├── flags: newline + │ │ │ │ │ ├── keyword_loc: (2,2)-(2,8) = "return" + │ │ │ │ │ └── arguments: ∅ + │ │ │ │ ├── subsequent: ∅ + │ │ │ │ └── end_keyword_loc: ∅ + │ │ │ └── @ StringNode (location: (3,2)-(3,15)) + │ │ │ ├── flags: newline + │ │ │ ├── opening_loc: (3,2)-(3,3) = "\"" + │ │ │ ├── content_loc: (3,3)-(3,14) = "conditional" + │ │ │ ├── closing_loc: (3,14)-(3,15) = "\"" + │ │ │ └── unescaped: "conditional" + │ │ ├── rescue_clause: ∅ + │ │ ├── else_clause: ∅ + │ │ ├── ensure_clause: ∅ + │ │ └── end_keyword_loc: (4,0)-(4,3) = "end" + │ └── operator_loc: (1,2)-(1,3) = "=" + ├── @ LocalVariableWriteNode (location: (6,0)-(9,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (6,0)-(6,1) = "x" + │ ├── value: + │ │ @ IfNode (location: (6,4)-(9,3)) + │ │ ├── flags: newline + │ │ ├── if_keyword_loc: (6,4)-(6,6) = "if" + │ │ ├── predicate: + │ │ │ @ CallNode (location: (6,7)-(6,17)) + │ │ │ ├── flags: ∅ + │ │ │ ├── receiver: + │ │ │ │ @ CallNode (location: (6,7)-(6,11)) + │ │ │ │ ├── flags: variable_call, ignore_visibility + │ │ │ │ ├── receiver: ∅ + │ │ │ │ ├── call_operator_loc: ∅ + │ │ │ │ ├── name: :rand + │ │ │ │ ├── message_loc: (6,7)-(6,11) = "rand" + │ │ │ │ ├── opening_loc: ∅ + │ │ │ │ ├── arguments: ∅ + │ │ │ │ ├── closing_loc: ∅ + │ │ │ │ ├── equal_loc: ∅ + │ │ │ │ └── block: ∅ + │ │ │ ├── call_operator_loc: ∅ + │ │ │ ├── name: :< + │ │ │ ├── message_loc: (6,12)-(6,13) = "<" + │ │ │ ├── opening_loc: ∅ + │ │ │ ├── arguments: + │ │ │ │ @ ArgumentsNode (location: (6,14)-(6,17)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── arguments: (length: 1) + │ │ │ │ └── @ FloatNode (location: (6,14)-(6,17)) + │ │ │ │ ├── flags: static_literal + │ │ │ │ └── value: 0.5 + │ │ │ ├── closing_loc: ∅ + │ │ │ ├── equal_loc: ∅ + │ │ │ └── block: ∅ + │ │ ├── then_keyword_loc: ∅ + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (7,2)-(8,15)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 2) + │ │ │ ├── @ ReturnNode (location: (7,2)-(7,8)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (7,2)-(7,8) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ └── @ StringNode (location: (8,2)-(8,15)) + │ │ │ ├── flags: newline + │ │ │ ├── opening_loc: (8,2)-(8,3) = "\"" + │ │ │ ├── content_loc: (8,3)-(8,14) = "else is nil" + │ │ │ ├── closing_loc: (8,14)-(8,15) = "\"" + │ │ │ └── unescaped: "else is nil" + │ │ ├── subsequent: ∅ + │ │ └── end_keyword_loc: (9,0)-(9,3) = "end" + │ └── operator_loc: (6,2)-(6,3) = "=" + ├── @ LocalVariableWriteNode (location: (11,0)-(16,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (11,0)-(11,1) = "x" + │ ├── value: + │ │ @ IfNode (location: (11,4)-(16,3)) + │ │ ├── flags: newline + │ │ ├── if_keyword_loc: (11,4)-(11,6) = "if" + │ │ ├── predicate: + │ │ │ @ TrueNode (location: (11,7)-(11,11)) + │ │ │ └── flags: static_literal + │ │ ├── then_keyword_loc: ∅ + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (12,2)-(13,15)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 2) + │ │ │ ├── @ IfNode (location: (12,2)-(12,16)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── if_keyword_loc: (12,9)-(12,11) = "if" + │ │ │ │ ├── predicate: + │ │ │ │ │ @ TrueNode (location: (12,12)-(12,16)) + │ │ │ │ │ └── flags: static_literal + │ │ │ │ ├── then_keyword_loc: ∅ + │ │ │ │ ├── statements: + │ │ │ │ │ @ StatementsNode (location: (12,2)-(12,8)) + │ │ │ │ │ ├── flags: ∅ + │ │ │ │ │ └── body: (length: 1) + │ │ │ │ │ └── @ ReturnNode (location: (12,2)-(12,8)) + │ │ │ │ │ ├── flags: newline + │ │ │ │ │ ├── keyword_loc: (12,2)-(12,8) = "return" + │ │ │ │ │ └── arguments: ∅ + │ │ │ │ ├── subsequent: ∅ + │ │ │ │ └── end_keyword_loc: ∅ + │ │ │ └── @ StringNode (location: (13,2)-(13,15)) + │ │ │ ├── flags: newline + │ │ │ ├── opening_loc: (13,2)-(13,3) = "\"" + │ │ │ ├── content_loc: (13,3)-(13,14) = "conditional" + │ │ │ ├── closing_loc: (13,14)-(13,15) = "\"" + │ │ │ └── unescaped: "conditional" + │ │ ├── subsequent: + │ │ │ @ ElseNode (location: (14,0)-(16,3)) + │ │ │ ├── flags: ∅ + │ │ │ ├── else_keyword_loc: (14,0)-(14,4) = "else" + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (15,2)-(15,8)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (15,2)-(15,8)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (15,2)-(15,8) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ └── end_keyword_loc: (16,0)-(16,3) = "end" + │ │ └── end_keyword_loc: (16,0)-(16,3) = "end" + │ └── operator_loc: (11,2)-(11,3) = "=" + ├── @ LocalVariableWriteNode (location: (18,0)-(23,3)) + │ ├── flags: newline + │ ├── name: :x + │ ├── depth: 0 + │ ├── name_loc: (18,0)-(18,1) = "x" + │ ├── value: + │ │ @ IfNode (location: (18,4)-(23,3)) + │ │ ├── flags: newline + │ │ ├── if_keyword_loc: (18,4)-(18,6) = "if" + │ │ ├── predicate: + │ │ │ @ TrueNode (location: (18,7)-(18,11)) + │ │ │ └── flags: static_literal + │ │ ├── then_keyword_loc: ∅ + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (19,2)-(19,16)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 1) + │ │ │ └── @ IfNode (location: (19,2)-(19,16)) + │ │ │ ├── flags: newline + │ │ │ ├── if_keyword_loc: (19,9)-(19,11) = "if" + │ │ │ ├── predicate: + │ │ │ │ @ TrueNode (location: (19,12)-(19,16)) + │ │ │ │ └── flags: static_literal + │ │ │ ├── then_keyword_loc: ∅ + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (19,2)-(19,8)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (19,2)-(19,8)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (19,2)-(19,8) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ ├── subsequent: ∅ + │ │ │ └── end_keyword_loc: ∅ + │ │ ├── subsequent: + │ │ │ @ ElseNode (location: (20,0)-(23,3)) + │ │ │ ├── flags: ∅ + │ │ │ ├── else_keyword_loc: (20,0)-(20,4) = "else" + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (21,2)-(22,15)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 2) + │ │ │ │ ├── @ IfNode (location: (21,2)-(21,16)) + │ │ │ │ │ ├── flags: newline + │ │ │ │ │ ├── if_keyword_loc: (21,9)-(21,11) = "if" + │ │ │ │ │ ├── predicate: + │ │ │ │ │ │ @ TrueNode (location: (21,12)-(21,16)) + │ │ │ │ │ │ └── flags: static_literal + │ │ │ │ │ ├── then_keyword_loc: ∅ + │ │ │ │ │ ├── statements: + │ │ │ │ │ │ @ StatementsNode (location: (21,2)-(21,8)) + │ │ │ │ │ │ ├── flags: ∅ + │ │ │ │ │ │ └── body: (length: 1) + │ │ │ │ │ │ └── @ ReturnNode (location: (21,2)-(21,8)) + │ │ │ │ │ │ ├── flags: newline + │ │ │ │ │ │ ├── keyword_loc: (21,2)-(21,8) = "return" + │ │ │ │ │ │ └── arguments: ∅ + │ │ │ │ │ ├── subsequent: ∅ + │ │ │ │ │ └── end_keyword_loc: ∅ + │ │ │ │ └── @ StringNode (location: (22,2)-(22,15)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── opening_loc: (22,2)-(22,3) = "\"" + │ │ │ │ ├── content_loc: (22,3)-(22,14) = "conditional" + │ │ │ │ ├── closing_loc: (22,14)-(22,15) = "\"" + │ │ │ │ └── unescaped: "conditional" + │ │ │ └── end_keyword_loc: (23,0)-(23,3) = "end" + │ │ └── end_keyword_loc: (23,0)-(23,3) = "end" + │ └── operator_loc: (18,2)-(18,3) = "=" + └── @ LocalVariableWriteNode (location: (25,0)-(31,3)) + ├── flags: newline + ├── name: :x + ├── depth: 0 + ├── name_loc: (25,0)-(25,1) = "x" + ├── value: + │ @ CaseNode (location: (25,4)-(31,3)) + │ ├── flags: ∅ + │ ├── predicate: ∅ + │ ├── conditions: (length: 1) + │ │ └── @ WhenNode (location: (26,2)-(28,17)) + │ │ ├── flags: ∅ + │ │ ├── keyword_loc: (26,2)-(26,6) = "when" + │ │ ├── conditions: (length: 1) + │ │ │ └── @ IntegerNode (location: (26,7)-(26,8)) + │ │ │ ├── flags: static_literal, decimal + │ │ │ └── value: 1 + │ │ ├── then_keyword_loc: ∅ + │ │ └── statements: + │ │ @ StatementsNode (location: (27,4)-(28,17)) + │ │ ├── flags: ∅ + │ │ └── body: (length: 2) + │ │ ├── @ IfNode (location: (27,4)-(27,18)) + │ │ │ ├── flags: newline + │ │ │ ├── if_keyword_loc: (27,11)-(27,13) = "if" + │ │ │ ├── predicate: + │ │ │ │ @ TrueNode (location: (27,14)-(27,18)) + │ │ │ │ └── flags: static_literal + │ │ │ ├── then_keyword_loc: ∅ + │ │ │ ├── statements: + │ │ │ │ @ StatementsNode (location: (27,4)-(27,10)) + │ │ │ │ ├── flags: ∅ + │ │ │ │ └── body: (length: 1) + │ │ │ │ └── @ ReturnNode (location: (27,4)-(27,10)) + │ │ │ │ ├── flags: newline + │ │ │ │ ├── keyword_loc: (27,4)-(27,10) = "return" + │ │ │ │ └── arguments: ∅ + │ │ │ ├── subsequent: ∅ + │ │ │ └── end_keyword_loc: ∅ + │ │ └── @ StringNode (location: (28,4)-(28,17)) + │ │ ├── flags: newline + │ │ ├── opening_loc: (28,4)-(28,5) = "\"" + │ │ ├── content_loc: (28,5)-(28,16) = "conditional" + │ │ ├── closing_loc: (28,16)-(28,17) = "\"" + │ │ └── unescaped: "conditional" + │ ├── else_clause: + │ │ @ ElseNode (location: (29,2)-(31,3)) + │ │ ├── flags: ∅ + │ │ ├── else_keyword_loc: (29,2)-(29,6) = "else" + │ │ ├── statements: + │ │ │ @ StatementsNode (location: (30,4)-(30,10)) + │ │ │ ├── flags: ∅ + │ │ │ └── body: (length: 1) + │ │ │ └── @ ReturnNode (location: (30,4)-(30,10)) + │ │ │ ├── flags: newline + │ │ │ ├── keyword_loc: (30,4)-(30,10) = "return" + │ │ │ └── arguments: ∅ + │ │ └── end_keyword_loc: (31,0)-(31,3) = "end" + │ ├── case_keyword_loc: (25,4)-(25,8) = "case" + │ └── end_keyword_loc: (31,0)-(31,3) = "end" + └── operator_loc: (25,2)-(25,3) = "=" diff --git a/src/prism.c b/src/prism.c index 6f9a19ef81..8f3617adce 100644 --- a/src/prism.c +++ b/src/prism.c @@ -1054,6 +1054,13 @@ pm_parser_constant_id_token(pm_parser_t *parser, const pm_token_t *token) { return pm_parser_constant_id_raw(parser, token->start, token->end); } +/** + * This macro allows you to define a case statement for all of the nodes that + * may result in a void value. + */ +#define PM_CASE_VOID_VALUE PM_RETURN_NODE: case PM_BREAK_NODE: case PM_NEXT_NODE: \ + case PM_REDO_NODE: case PM_RETRY_NODE: case PM_MATCH_REQUIRED_NODE + /** * Check whether or not the given node is value expression. * If the node is value node, it returns NULL. @@ -1065,12 +1072,7 @@ pm_check_value_expression(pm_parser_t *parser, pm_node_t *node) { while (node != NULL) { switch (PM_NODE_TYPE(node)) { - case PM_RETURN_NODE: - case PM_BREAK_NODE: - case PM_NEXT_NODE: - case PM_REDO_NODE: - case PM_RETRY_NODE: - case PM_MATCH_REQUIRED_NODE: + case PM_CASE_VOID_VALUE: return void_node != NULL ? void_node : node; case PM_MATCH_PREDICATE_NODE: return NULL; @@ -1090,15 +1092,23 @@ pm_check_value_expression(pm_parser_t *parser, pm_node_t *node) { node = UP(cast->ensure_clause); } else if (cast->rescue_clause != NULL) { - if (cast->statements == NULL) return NULL; + // https://bugs.ruby-lang.org/issues/21669 + if (cast->else_clause == NULL || parser->version < PM_OPTIONS_VERSION_CRUBY_4_1) { + if (cast->statements == NULL) return NULL; - pm_node_t *vn = pm_check_value_expression(parser, UP(cast->statements)); - if (vn == NULL) return NULL; - if (void_node == NULL) void_node = vn; + pm_node_t *vn = pm_check_value_expression(parser, UP(cast->statements)); + if (vn == NULL) return NULL; + if (void_node == NULL) void_node = vn; + } for (pm_rescue_node_t *rescue_clause = cast->rescue_clause; rescue_clause != NULL; rescue_clause = rescue_clause->subsequent) { pm_node_t *vn = pm_check_value_expression(parser, UP(rescue_clause->statements)); + if (vn == NULL) { + // https://bugs.ruby-lang.org/issues/21669 + if (parser->version >= PM_OPTIONS_VERSION_CRUBY_4_1) { + return NULL; + } void_node = NULL; break; } @@ -1106,6 +1116,12 @@ pm_check_value_expression(pm_parser_t *parser, pm_node_t *node) { if (cast->else_clause != NULL) { node = UP(cast->else_clause); + + // https://bugs.ruby-lang.org/issues/21669 + if (parser->version >= PM_OPTIONS_VERSION_CRUBY_4_1) { + pm_node_t *vn = pm_check_value_expression(parser, node); + if (vn != NULL) return vn; + } } else { return void_node; } @@ -1115,6 +1131,50 @@ pm_check_value_expression(pm_parser_t *parser, pm_node_t *node) { break; } + case PM_CASE_NODE: { + // https://bugs.ruby-lang.org/issues/21669 + if (parser->version < PM_OPTIONS_VERSION_CRUBY_4_1) { + return NULL; + } + + pm_case_node_t *cast = (pm_case_node_t *) node; + if (cast->else_clause == NULL) return NULL; + + pm_node_t *condition; + PM_NODE_LIST_FOREACH(&cast->conditions, index, condition) { + assert(PM_NODE_TYPE_P(condition, PM_WHEN_NODE)); + + pm_when_node_t *cast = (pm_when_node_t *) condition; + pm_node_t *vn = pm_check_value_expression(parser, UP(cast->statements)); + if (vn == NULL) return NULL; + if (void_node == NULL) void_node = vn; + } + + node = UP(cast->else_clause); + break; + } + case PM_CASE_MATCH_NODE: { + // https://bugs.ruby-lang.org/issues/21669 + if (parser->version < PM_OPTIONS_VERSION_CRUBY_4_1) { + return NULL; + } + + pm_case_match_node_t *cast = (pm_case_match_node_t *) node; + if (cast->else_clause == NULL) return NULL; + + pm_node_t *condition; + PM_NODE_LIST_FOREACH(&cast->conditions, index, condition) { + assert(PM_NODE_TYPE_P(condition, PM_IN_NODE)); + + pm_in_node_t *cast = (pm_in_node_t *) condition; + pm_node_t *vn = pm_check_value_expression(parser, UP(cast->statements)); + if (vn == NULL) return NULL; + if (void_node == NULL) void_node = vn; + } + + node = UP(cast->else_clause); + break; + } case PM_ENSURE_NODE: { pm_ensure_node_t *cast = (pm_ensure_node_t *) node; node = UP(cast->statements); @@ -1127,6 +1187,22 @@ pm_check_value_expression(pm_parser_t *parser, pm_node_t *node) { } case PM_STATEMENTS_NODE: { pm_statements_node_t *cast = (pm_statements_node_t *) node; + + // https://bugs.ruby-lang.org/issues/21669 + if (parser->version >= PM_OPTIONS_VERSION_CRUBY_4_1) { + pm_node_t *body_part; + PM_NODE_LIST_FOREACH(&cast->body, index, body_part) { + switch (PM_NODE_TYPE(body_part)) { + case PM_CASE_VOID_VALUE: + if (void_node == NULL) { + void_node = body_part; + } + return void_node; + default: break; + } + } + } + node = cast->body.nodes[cast->body.size - 1]; break; } diff --git a/test/prism/errors/3.3-4.0/singleton_method_with_void_value.txt b/test/prism/errors/3.3-4.0/singleton_method_with_void_value.txt new file mode 100644 index 0000000000..2954f7ea48 --- /dev/null +++ b/test/prism/errors/3.3-4.0/singleton_method_with_void_value.txt @@ -0,0 +1,3 @@ +def ((return; 1)).bar; end + ^ cannot define singleton method for literals + diff --git a/test/prism/errors/3.4-4.0/void_value.txt b/test/prism/errors/3.4-4.0/void_value.txt new file mode 100644 index 0000000000..c03139bb05 --- /dev/null +++ b/test/prism/errors/3.4-4.0/void_value.txt @@ -0,0 +1,18 @@ +x = begin + return + ^~~~~~ unexpected void value expression +rescue + return +else + return +end + +x = begin + return +rescue + "OK" +else + return + ^~~~~~ unexpected void value expression +end + diff --git a/test/prism/errors/4.1/singleton_method_with_void_value.txt b/test/prism/errors/4.1/singleton_method_with_void_value.txt new file mode 100644 index 0000000000..bc6cf9c602 --- /dev/null +++ b/test/prism/errors/4.1/singleton_method_with_void_value.txt @@ -0,0 +1,4 @@ +def ((return; 1)).bar; end + ^~~~~~ unexpected void value expression + ^ cannot define singleton method for literals + diff --git a/test/prism/errors/4.1/void_value.txt b/test/prism/errors/4.1/void_value.txt new file mode 100644 index 0000000000..a27ffd763a --- /dev/null +++ b/test/prism/errors/4.1/void_value.txt @@ -0,0 +1,44 @@ +x = begin + return +rescue + return +else + return + ^~~~~~ unexpected void value expression +end + +x = begin + ignored_because_else_branch +rescue + return +else + return + ^~~~~~ unexpected void value expression +end + +x = case + when 1 then return + ^~~~~~ unexpected void value expression + else return +end + +x = case 1 + in 2 then return + ^~~~~~ unexpected void value expression + else return +end + +x = begin + return + ^~~~~~ unexpected void value expression + "NG" +end + +x = if rand < 0.5 + return + ^~~~~~ unexpected void value expression + "NG" +else + return +end + diff --git a/test/prism/errors/singleton_method_for_literals.txt b/test/prism/errors/singleton_method_for_literals.txt index 6247b4f025..ae850fca29 100644 --- a/test/prism/errors/singleton_method_for_literals.txt +++ b/test/prism/errors/singleton_method_for_literals.txt @@ -2,8 +2,6 @@ def (1).g; end ^ cannot define singleton method for literals def ((a; 1)).foo; end ^ cannot define singleton method for literals -def ((return; 1)).bar; end - ^ cannot define singleton method for literals def (((1))).foo; end ^ cannot define singleton method for literals def (__FILE__).foo; end diff --git a/test/prism/errors/void_value_expression_in_begin_statement.txt b/test/prism/errors/void_value_expression_in_begin_statement.txt index aa8f1ded96..fb968a12e1 100644 --- a/test/prism/errors/void_value_expression_in_begin_statement.txt +++ b/test/prism/errors/void_value_expression_in_begin_statement.txt @@ -14,8 +14,6 @@ x = begin return ensure return end ^~~~~~ unexpected void value expression x = begin return; rescue; return end ^~~~~~ unexpected void value expression -x = begin return; rescue; return; else return end - ^~~~~~ unexpected void value expression x = begin; return; rescue; retry; end ^~~~~~ unexpected void value expression diff --git a/test/prism/fixtures/3.3-4.0/void_value.txt b/test/prism/fixtures/3.3-4.0/void_value.txt new file mode 100644 index 0000000000..bfb8eff09c --- /dev/null +++ b/test/prism/fixtures/3.3-4.0/void_value.txt @@ -0,0 +1,29 @@ +x = begin + foo +rescue + return +else + return +end + +x = case + when 1 then return + else return +end + +x = case 1 + in 2 then return + else return +end + +x = begin + return + "NG" +end + +x = if rand < 0.5 + return + "NG" +else + return +end diff --git a/test/prism/fixtures/4.1/void_value.txt b/test/prism/fixtures/4.1/void_value.txt new file mode 100644 index 0000000000..915112d623 --- /dev/null +++ b/test/prism/fixtures/4.1/void_value.txt @@ -0,0 +1,7 @@ +x = begin + return +rescue + "OK" +else + return +end diff --git a/test/prism/fixtures/non_void_value.txt b/test/prism/fixtures/non_void_value.txt new file mode 100644 index 0000000000..388e9f2574 --- /dev/null +++ b/test/prism/fixtures/non_void_value.txt @@ -0,0 +1,31 @@ +x = begin + return if true + "conditional" +end + +x = if rand < 0.5 + return + "else is nil" +end + +x = if true + return if true + "conditional" +else + return +end + +x = if true + return if true +else + return if true + "conditional" +end + +x = case + when 1 + return if true + "conditional" + else + return +end diff --git a/test/prism/fixtures_test.rb b/test/prism/fixtures_test.rb index 7df97029d3..3a704b8389 100644 --- a/test/prism/fixtures_test.rb +++ b/test/prism/fixtures_test.rb @@ -24,7 +24,10 @@ class FixturesTest < TestCase except << "whitequark/ruby_bug_19281.txt" end + # https://bugs.ruby-lang.org/issues/21168#note-5 except << "command_method_call_2.txt" + # https://bugs.ruby-lang.org/issues/21669 + except << "4.1/void_value.txt" Fixture.each_for_current_ruby(except: except) do |fixture| define_method(fixture.test_name) { assert_valid_syntax(fixture.read) } diff --git a/test/prism/locals_test.rb b/test/prism/locals_test.rb index 4f8d6080e8..4844901804 100644 --- a/test/prism/locals_test.rb +++ b/test/prism/locals_test.rb @@ -28,6 +28,9 @@ class LocalsTest < TestCase # https://bugs.ruby-lang.org/issues/21168#note-5 "command_method_call_2.txt", + + # https://bugs.ruby-lang.org/issues/21669 + "4.1/void_value.txt" ] Fixture.each_for_current_ruby(except: except) do |fixture| diff --git a/test/prism/result/warnings_test.rb b/test/prism/result/warnings_test.rb index 4643fb134f..27f1119b98 100644 --- a/test/prism/result/warnings_test.rb +++ b/test/prism/result/warnings_test.rb @@ -230,6 +230,8 @@ def test_unused_local_variables refute_warning("foo = 1", compare: false, command_line: "e") refute_warning("foo = 1", compare: false, scopes: [[]]) + refute_warning("foo(bar = 1)") + assert_warning("def foo; bar = 1; end", "unused") assert_warning("def foo; bar, = 1; end", "unused") @@ -263,6 +265,23 @@ def test_unused_local_variables refute_warning("def foo; bar = 1; end", line: -2, compare: false) end + def test_unused_local_variable_or_assign_with_begin_node + assert_warning(<<~RUBY, "assigned but unused variable - foo", compare: false) + var ||= begin + foo = bar + baz + end + RUBY + + assert_warning(<<~RUBY, "assigned but unused variable - foo", compare: false) + foo = false + var ||= begin + foo = true + bar + end + RUBY + end + def test_void_statements assert_warning("foo = 1; foo", "a variable in void") assert_warning("@foo", "a variable in void") diff --git a/test/prism/ruby/ripper_test.rb b/test/prism/ruby/ripper_test.rb index a89a9503b9..157a68ba31 100644 --- a/test/prism/ruby/ripper_test.rb +++ b/test/prism/ruby/ripper_test.rb @@ -37,6 +37,9 @@ class RipperTest < TestCase ] end + # https://bugs.ruby-lang.org/issues/21669 + incorrect << "4.1/void_value.txt" + # Skip these tests that we haven't implemented yet. omitted_sexp_raw = [ "bom_leading_space.txt", diff --git a/test/prism/ruby/ruby_parser_test.rb b/test/prism/ruby/ruby_parser_test.rb index 4b7e9c93ed..6e4ba8ce16 100644 --- a/test/prism/ruby/ruby_parser_test.rb +++ b/test/prism/ruby/ruby_parser_test.rb @@ -83,6 +83,8 @@ class RubyParserTest < TestCase "3.3-3.3/keyword_args_in_array_assignment.txt", "3.3-3.3/return_in_sclass.txt", + "3.3-4.0/void_value.txt", + "3.4/circular_parameters.txt", "4.0/endless_methods_command_call.txt",