From 499786de4658de5d9c1ebe68ec99fbbfdf63f8e1 Mon Sep 17 00:00:00 2001 From: TSUYUSATO Kitsune Date: Tue, 14 Nov 2023 16:20:41 +0900 Subject: [ruby/prism] Check value expressions on parsing arguments and assignments They are corresponding to `arg_value` in `parse.y`. https://github.com/ruby/prism/commit/a4a4834e0d --- prism/prism.c | 34 +++++++++++----------- test/prism/errors_test.rb | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/prism/prism.c b/prism/prism.c index dab43b0b10..2d03307bf8 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -10232,11 +10232,11 @@ static pm_node_t * parse_starred_expression(pm_parser_t *parser, pm_binding_power_t binding_power, pm_diagnostic_id_t diag_id) { if (accept1(parser, PM_TOKEN_USTAR)) { pm_token_t operator = parser->previous; - pm_node_t *expression = parse_expression(parser, binding_power, PM_ERR_EXPECT_EXPRESSION_AFTER_STAR); + pm_node_t *expression = parse_value_expression(parser, binding_power, PM_ERR_EXPECT_EXPRESSION_AFTER_STAR); return (pm_node_t *) pm_splat_node_create(parser, &operator, expression); } - return parse_expression(parser, binding_power, diag_id); + return parse_value_expression(parser, binding_power, diag_id); } /** @@ -10727,7 +10727,7 @@ parse_assocs(pm_parser_t *parser, pm_node_t *node) { pm_node_t *value = NULL; if (token_begins_expression_p(parser->current.type)) { - value = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_EXPRESSION_AFTER_SPLAT_HASH); + value = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_EXPRESSION_AFTER_SPLAT_HASH); } else if (pm_parser_local_depth(parser, &operator) == -1) { pm_parser_err_token(parser, &operator, PM_ERR_EXPECT_EXPRESSION_AFTER_SPLAT_HASH); } @@ -10745,7 +10745,7 @@ parse_assocs(pm_parser_t *parser, pm_node_t *node) { pm_node_t *value = NULL; if (token_begins_expression_p(parser->current.type)) { - value = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_EXPRESSION_AFTER_LABEL); + value = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_EXPRESSION_AFTER_LABEL); } else { if (parser->encoding.isupper_char(label.start, (label.end - 1) - label.start)) { pm_token_t constant = { .type = PM_TOKEN_CONSTANT, .start = label.start, .end = label.end - 1 }; @@ -10769,7 +10769,7 @@ parse_assocs(pm_parser_t *parser, pm_node_t *node) { break; } default: { - pm_node_t *key = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_KEY); + pm_node_t *key = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_KEY); pm_token_t operator; if (pm_symbol_node_label_p(key)) { @@ -10779,7 +10779,7 @@ parse_assocs(pm_parser_t *parser, pm_node_t *node) { operator = parser->previous; } - pm_node_t *value = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_VALUE); + pm_node_t *value = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_VALUE); element = (pm_node_t *) pm_assoc_node_create(parser, key, &operator, value); break; } @@ -10875,7 +10875,7 @@ parse_arguments(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_for pm_node_t *expression = NULL; if (token_begins_expression_p(parser->current.type)) { - expression = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_ARGUMENT); + expression = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_ARGUMENT); } else if (pm_parser_local_depth(parser, &operator) == -1) { pm_parser_err_token(parser, &operator, PM_ERR_ARGUMENT_NO_FORWARDING_AMP); } @@ -10901,7 +10901,7 @@ parse_arguments(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_for argument = (pm_node_t *) pm_splat_node_create(parser, &operator, NULL); } else { - pm_node_t *expression = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_EXPRESSION_AFTER_SPLAT); + pm_node_t *expression = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_EXPRESSION_AFTER_SPLAT); if (parsed_bare_hash) { pm_parser_err(parser, operator.start, expression->location.end, PM_ERR_ARGUMENT_SPLAT_AFTER_ASSOC_SPLAT); @@ -10937,7 +10937,7 @@ parse_arguments(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_for /* fallthrough */ default: { if (argument == NULL) { - argument = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_ARGUMENT); + argument = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_ARGUMENT); } bool contains_keyword_splat = false; @@ -10956,7 +10956,7 @@ parse_arguments(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_for pm_keyword_hash_node_t *bare_hash = pm_keyword_hash_node_create(parser); // Finish parsing the one we are part way through - pm_node_t *value = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_VALUE); + pm_node_t *value = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_HASH_VALUE); argument = (pm_node_t *) pm_assoc_node_create(parser, argument, &operator, value); pm_keyword_hash_node_elements_append(bare_hash, argument); @@ -11260,7 +11260,7 @@ parse_parameters( if (accept1(parser, PM_TOKEN_EQUAL)) { pm_token_t operator = parser->previous; context_push(parser, PM_CONTEXT_DEFAULT_PARAMS); - pm_node_t *value = parse_expression(parser, binding_power, PM_ERR_PARAMETER_NO_DEFAULT); + pm_node_t *value = parse_value_expression(parser, binding_power, PM_ERR_PARAMETER_NO_DEFAULT); pm_optional_parameter_node_t *param = pm_optional_parameter_node_create(parser, &name, &operator, value); pm_parameters_node_optionals_append(params, param); @@ -11319,7 +11319,7 @@ parse_parameters( if (token_begins_expression_p(parser->current.type)) { context_push(parser, PM_CONTEXT_DEFAULT_PARAMS); - pm_node_t *value = parse_expression(parser, binding_power, PM_ERR_PARAMETER_NO_DEFAULT_KW); + pm_node_t *value = parse_value_expression(parser, binding_power, PM_ERR_PARAMETER_NO_DEFAULT_KW); context_pop(parser); param = (pm_node_t *) pm_optional_keyword_parameter_node_create(parser, &name, value); } @@ -13954,14 +13954,14 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power) { do { if (accept1(parser, PM_TOKEN_USTAR)) { pm_token_t operator = parser->previous; - pm_node_t *expression = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_EXPRESSION_AFTER_STAR); + pm_node_t *expression = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_EXPECT_EXPRESSION_AFTER_STAR); pm_splat_node_t *splat_node = pm_splat_node_create(parser, &operator, expression); pm_when_node_conditions_append(when_node, (pm_node_t *) splat_node); if (PM_NODE_TYPE_P(expression, PM_MISSING_NODE)) break; } else { - pm_node_t *condition = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_CASE_EXPRESSION_AFTER_WHEN); + pm_node_t *condition = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_CASE_EXPRESSION_AFTER_WHEN); pm_when_node_conditions_append(when_node, condition); if (PM_NODE_TYPE_P(condition, PM_MISSING_NODE)) break; @@ -14002,11 +14002,11 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power) { // for guard clauses in the form of `if` or `unless` statements. if (accept1(parser, PM_TOKEN_KEYWORD_IF_MODIFIER)) { pm_token_t keyword = parser->previous; - pm_node_t *predicate = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_CONDITIONAL_IF_PREDICATE); + pm_node_t *predicate = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_CONDITIONAL_IF_PREDICATE); pattern = (pm_node_t *) pm_if_node_modifier_create(parser, pattern, &keyword, predicate); } else if (accept1(parser, PM_TOKEN_KEYWORD_UNLESS_MODIFIER)) { pm_token_t keyword = parser->previous; - pm_node_t *predicate = parse_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_CONDITIONAL_UNLESS_PREDICATE); + pm_node_t *predicate = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, PM_ERR_CONDITIONAL_UNLESS_PREDICATE); pattern = (pm_node_t *) pm_unless_node_modifier_create(parser, pattern, &keyword, predicate); } @@ -14363,7 +14363,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power) { case PM_TOKEN_PARENTHESIS_LEFT: { parser_lex(parser); pm_token_t lparen = parser->previous; - pm_node_t *expression = parse_expression(parser, PM_BINDING_POWER_STATEMENT, PM_ERR_DEF_RECEIVER); + pm_node_t *expression = parse_value_expression(parser, PM_BINDING_POWER_STATEMENT, PM_ERR_DEF_RECEIVER); expect1(parser, PM_TOKEN_PARENTHESIS_RIGHT, PM_ERR_EXPECT_RPAREN); pm_token_t rparen = parser->previous; diff --git a/test/prism/errors_test.rb b/test/prism/errors_test.rb index 0eb0f5bb58..9a63b32e30 100644 --- a/test/prism/errors_test.rb +++ b/test/prism/errors_test.rb @@ -1511,6 +1511,39 @@ module Prism ], compare_ripper: false # Ripper does not check 'void value expression'. end + def test_void_value_expression_in_def + source = <<~RUBY + def (return).x + end + def x(a = return) + end + def x(a: return) + end + RUBY + message = 'Unexpected void value expression' + assert_errors expression(source), source, [ + [message, 5..11], + [message, 29..35], + [message, 50..56], + ], compare_ripper: false # Ripper does not check 'void value expression'. + end + + def test_void_value_expression_in_assignment + source = <<~RUBY + a = return + a = 1, return + a, b = return, 1 + a, b = 1, *return + RUBY + message = 'Unexpected void value expression' + assert_errors expression(source), source, [ + [message, 4..10], + [message, 18..24], + [message, 32..38], + [message, 53..59], + ], compare_ripper: false # Ripper does not check 'void value expression'. + end + def test_void_value_expression_in_modifier source = <<~RUBY 1 if (return) @@ -1527,6 +1560,46 @@ module Prism ], compare_ripper: false # Ripper does not check 'void value expression'. end + def test_void_value_expression_in_arguments + source = <<~RUBY + foo(return) + foo(1, return) + foo(*return) + foo(**return) + foo(&return) + foo(return => 1) + foo(:a => return) + foo(a: return) + RUBY + message = 'Unexpected void value expression' + assert_errors expression(source), source, [ + [message, 4..10], + [message, 19..25], + [message, 32..38], + [message, 46..52], + [message, 59..65], + [message, 71..77], + [message, 94..100], + [message, 109..115], + ], compare_ripper: false # Ripper does not check 'void value expression'. + end + + def test_void_value_expression_in_hash + source = <<~RUBY + { return => 1 } + { 1 => return } + { a: return } + { **return } + RUBY + message = 'Unexpected void value expression' + assert_errors expression(source), source, [ + [message, 2..8], + [message, 23..29], + [message, 37..43], + [message, 50..56], + ], compare_ripper: false # Ripper does not check 'void value expression'. + end + private def assert_errors(expected, source, errors, compare_ripper: RUBY_ENGINE == "ruby") -- cgit v1.2.3