Reject def f a, (b) = 1

Fixes [#Bug 21660], followup to #3674

Co-Authored-By: tomoya ishida <[email protected]>

Author: Earlopain

src/prism.c

@@ -14621,18 +14621,6 @@ update_parameter_state(pm_parser_t *parser, pm_token_t *token, pm_parameters_ord
     return true;
 }
 
-/**
- * Ensures that after parsing a parameter, the next token is not `=`.
- * Some parameters like `def(* = 1)` cannot become optional. When no parens
- * are present like in `def * = 1`, this creates ambiguity with endless method definitions.
- */
-static inline void
-refute_optional_parameter(pm_parser_t *parser) {
-    if (match1(parser, PM_TOKEN_EQUAL)) {
-        pm_parser_err_previous(parser, PM_ERR_DEF_ENDLESS_PARAMETERS);
-    }
-}
-
 /**
  * Parse a list of parameters on a method definition.
  */
@@ -14685,10 +14673,6 @@ parse_parameters(
                     parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_FORWARDING_BLOCK;
                 }
 
-                if (!uses_parentheses) {
-                    refute_optional_parameter(parser);
-                }
-
                 pm_block_parameter_node_t *param = pm_block_parameter_node_create(parser, &name, &operator);
                 if (repeated) {
                     pm_node_flag_set_repeated_parameter((pm_node_t *)param);
@@ -14710,10 +14694,6 @@ parse_parameters(
                 bool succeeded = update_parameter_state(parser, &parser->current, &order);
                 parser_lex(parser);
 
-                if (!uses_parentheses) {
-                    refute_optional_parameter(parser);
-                }
-
                 parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_FORWARDING_ALL;
                 pm_forwarding_parameter_node_t *param = pm_forwarding_parameter_node_create(parser, &parser->previous);
 
@@ -14895,10 +14875,6 @@ parse_parameters(
                         context_pop(parser);
                         pm_parameters_node_keywords_append(params, param);
 
-                        if (!uses_parentheses) {
-                            refute_optional_parameter(parser);
-                        }
-
                         // If parsing the value of the parameter resulted in error recovery,
                         // then we can put a missing node in its place and stop parsing the
                         // parameters entirely now.
@@ -14930,10 +14906,6 @@ parse_parameters(
                     parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_FORWARDING_POSITIONALS;
                 }
 
-                if (!uses_parentheses) {
-                    refute_optional_parameter(parser);
-                }
-
                 pm_node_t *param = (pm_node_t *) pm_rest_parameter_node_create(parser, &operator, &name);
                 if (repeated) {
                     pm_node_flag_set_repeated_parameter(param);
@@ -14982,10 +14954,6 @@ parse_parameters(
                     }
                 }
 
-                if (!uses_parentheses) {
-                    refute_optional_parameter(parser);
-                }
-
                 if (params->keyword_rest == NULL) {
                     pm_parameters_node_keyword_rest_set(params, param);
                 } else {
@@ -19586,6 +19554,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
             pm_token_t rparen;
             pm_parameters_node_t *params;
 
+            bool accept_endless_def = true;
             switch (parser->current.type) {
                 case PM_TOKEN_PARENTHESIS_LEFT: {
                     parser_lex(parser);
@@ -19621,6 +19590,10 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
                     rparen = not_provided(parser);
                     params = parse_parameters(parser, PM_BINDING_POWER_DEFINED, false, false, true, true, false, (uint16_t) (depth + 1));
 
+                    // Reject `def * = 1` and similar. We have to specifically check
+                    // for them because they create ambiguity with optional arguments.
+                    accept_endless_def = false;
+
                     context_pop(parser);
                     break;
                 }
@@ -19642,6 +19615,9 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
                 if (token_is_setter_name(&name)) {
                     pm_parser_err_token(parser, &name, PM_ERR_DEF_ENDLESS_SETTER);
                 }
+                if (!accept_endless_def) {
+                    pm_parser_err_previous(parser, PM_ERR_DEF_ENDLESS_PARAMETERS);
+                }
                 equal = parser->previous;
 
                 context_push(parser, PM_CONTEXT_DEF);

test/prism/errors/endless_method_command_call_parameters.txt

@@ -1,24 +1,27 @@
 def f x: = 1
-      ^~ could not parse the endless method parameters
+         ^ could not parse the endless method parameters
 
 def f ... = 1
-      ^~~ could not parse the endless method parameters
+          ^ could not parse the endless method parameters
 
 def f * = 1
-      ^ could not parse the endless method parameters
+        ^ could not parse the endless method parameters
 
 def f ** = 1
-      ^~ could not parse the endless method parameters
+         ^ could not parse the endless method parameters
 
 def f & = 1
-      ^ could not parse the endless method parameters
+        ^ could not parse the endless method parameters
 
 def f *a = 1
-       ^ could not parse the endless method parameters
+         ^ could not parse the endless method parameters
 
 def f **a = 1
-        ^ could not parse the endless method parameters
+          ^ could not parse the endless method parameters
 
 def f &a = 1
-       ^ could not parse the endless method parameters
+         ^ could not parse the endless method parameters
+
+def f a, (b) = 1
+             ^ could not parse the endless method parameters