diff --git a/bootstraptest/test_flow.rb b/bootstraptest/test_flow.rb index 15528a42130fc1..7a95def1e6fb15 100644 --- a/bootstraptest/test_flow.rb +++ b/bootstraptest/test_flow.rb @@ -376,7 +376,7 @@ def m1 *args; $a << 2 ; $a << 3 end; $a << 4 def m2; $a << 5 - m1(:a, :b, (return 1; :c)); $a << 6 + m1(:a, :b, (return 1 if true; :c)); $a << 6 end; $a << 7 m2; $a << 8 ; $a << 9 @@ -399,7 +399,7 @@ def m(); $a << 4 m2(begin; $a << 5 2; $a << 6 ensure; $a << 7 - return 3; $a << 8 + return 3 if true; $a << 8 end); $a << 9 4; $a << 10 end; $a << 11 diff --git a/bootstraptest/test_syntax.rb b/bootstraptest/test_syntax.rb index fbc9c6f62e377a..29bf93cb8f6957 100644 --- a/bootstraptest/test_syntax.rb +++ b/bootstraptest/test_syntax.rb @@ -571,7 +571,7 @@ class << (ary=[]); def []; 0; end; def []=(x); super(0,x);end;end; ary[]+=1 assert_equal 'ok', %q{ 1.times{ - p(1, (next; 2)) + p(1, (next if true; 2)) }; :ok } assert_equal '3', %q{ @@ -585,7 +585,7 @@ class << (ary=[]); def []; 0; end; def []=(x); super(0,x);end;end; ary[]+=1 i = 0 1 + (while true break 2 if (i+=1) > 1 - p(1, (next; 2)) + p(1, (next if true; 2)) end) } # redo @@ -594,7 +594,7 @@ class << (ary=[]); def []; 0; end; def []=(x); super(0,x);end;end; ary[]+=1 1.times{ break if i>1 i+=1 - p(1, (redo; 2)) + p(1, (redo if true; 2)) }; :ok } assert_equal '3', %q{ @@ -608,7 +608,7 @@ class << (ary=[]); def []; 0; end; def []=(x); super(0,x);end;end; ary[]+=1 i = 0 1 + (while true break 2 if (i+=1) > 1 - p(1, (redo; 2)) + p(1, (redo if true; 2)) end) } assert_equal '1', %q{ diff --git a/prism/extension.c b/prism/extension.c index cde10bf360df2a..363144970c8c97 100644 --- a/prism/extension.c +++ b/prism/extension.c @@ -201,12 +201,10 @@ build_options_i(VALUE key, VALUE value, VALUE argument) { const char *version = check_string(value); if (RSTRING_LEN(value) == 7 && strncmp(version, "current", 7) == 0) { - const char *ruby_version = RSTRING_PTR(rb_const_get(rb_cObject, rb_intern("RUBY_VERSION"))); if (!pm_options_version_set(options, ruby_version, 3)) { rb_exc_raise(rb_exc_new_cstr(rb_cPrismCurrentVersionError, ruby_version)); } } else if (RSTRING_LEN(value) == 7 && strncmp(version, "nearest", 7) == 0) { - const char *ruby_version = RSTRING_PTR(rb_const_get(rb_cObject, rb_intern("RUBY_VERSION"))); const char *nearest_version; if (ruby_version[0] < '3' || (ruby_version[0] == '3' && ruby_version[2] < '3')) { diff --git a/prism/extension.h b/prism/extension.h index 4ddc3a7b8617d0..a2aaa6388fdd66 100644 --- a/prism/extension.h +++ b/prism/extension.h @@ -5,6 +5,7 @@ #include #include +#include #include "prism.h" VALUE pm_source_new(const pm_parser_t *parser, rb_encoding *encoding, bool freeze); diff --git a/prism/prism.c b/prism/prism.c index 2c039612850e92..8f3617adce9b44 100644 --- a/prism/prism.c +++ b/prism/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,25 +1092,36 @@ 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; } - if (void_node == NULL) { - void_node = vn; - } } 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; } @@ -1118,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); @@ -1130,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 00000000000000..2954f7ea4895f2 --- /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 00000000000000..c03139bb052be4 --- /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 00000000000000..bc6cf9c602de4f --- /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 00000000000000..a27ffd763aa237 --- /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 6247b4f025523f..ae850fca29d885 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 aa8f1ded964c66..fb968a12e117f8 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 00000000000000..bfb8eff09c9577 --- /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 00000000000000..915112d62346a0 --- /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 00000000000000..388e9f2574bcf6 --- /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 7df97029d3aa52..3a704b83895054 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 4f8d6080e8075d..48449018041dcf 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 4643fb134fe759..27f1119b98333c 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 758505ac2ab96d..15f535f3d6a157 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 4b7e9c93eddedc..6e4ba8ce16f25f 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", diff --git a/test/ruby/test_syntax.rb b/test/ruby/test_syntax.rb index b355128a7344a6..5065a1db332a54 100644 --- a/test/ruby/test_syntax.rb +++ b/test/ruby/test_syntax.rb @@ -1539,10 +1539,10 @@ def test_return_toplevel begin raise; rescue; return; end return false; raise return 1; raise - "#{return}" - raise((return; "should not raise")) + "#{return if true}" + raise((return if true; "should not raise")) begin raise; ensure return; end; self - nil&defined?0--begin e=no_method_error(); return; 0;end + nil&defined?0--begin e=no_method_error(); return if true; 0;end return puts('ignored') #=> ignored BEGIN {return} END {return if false}