Merge pull request #2144 from tenderlove/dup-param-flag

Add a "repeated flag" to parameter nodes

Author: kddnewton

config.yml

@@ -378,6 +378,11 @@ flags:
       - name: BEGIN_MODIFIER
         comment: "a loop after a begin statement, so the body is executed first before the condition"
     comment: Flags for while and until loop nodes.
+  - name: ParameterFlags
+    values:
+      - name: REPEATED_PARAMETER
+        comment: "a parameter name that has been repeated in the method signature"
+    comment: Flags for parameter nodes.
   - name: RangeFlags
     values:
       - name: EXCLUDE_END
@@ -648,6 +653,9 @@ nodes:
           ^^^^^^^^^^
   - name: BlockLocalVariableNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant
     comment: |
@@ -676,6 +684,9 @@ nodes:
                      ^^^^^^^^^^^^^^
   - name: BlockParameterNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant?
       - name: name_loc
@@ -1923,6 +1934,9 @@ nodes:
               ^^^^
   - name: KeywordRestParameterNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant?
       - name: name_loc
@@ -2221,6 +2235,9 @@ nodes:
           ^^
   - name: OptionalKeywordParameterNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant
       - name: name_loc
@@ -2235,6 +2252,9 @@ nodes:
           end
   - name: OptionalParameterNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant
       - name: name_loc
@@ -2451,6 +2471,9 @@ nodes:
           ^^^^^^
   - name: RequiredKeywordParameterNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant
       - name: name_loc
@@ -2463,6 +2486,9 @@ nodes:
           end
   - name: RequiredParameterNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant
     comment: |
@@ -2514,6 +2540,9 @@ nodes:
       `ex` is in the `exception` field.
   - name: RestParameterNode
     fields:
+      - name: flags
+        type: flags
+        kind: ParameterFlags
       - name: name
         type: constant?
       - name: name_loc

lib/prism/debug.rb

@@ -138,28 +138,32 @@ def self.prism_locals(source)
               *params.keywords.grep(OptionalKeywordParameterNode).map(&:name),
             ]
 
+            sorted << AnonymousLocal if params.keywords.any?
+
             if params.keyword_rest.is_a?(ForwardingParameterNode)
               sorted.push(:*, :&, :"...")
+            elsif params.keyword_rest.is_a?(KeywordRestParameterNode)
+              sorted << params.keyword_rest.name if params.keyword_rest.name
             end
 
-            sorted << AnonymousLocal if params.keywords.any?
-
             # Recurse down the parameter tree to find any destructured
             # parameters and add them after the other parameters.
             param_stack = params.requireds.concat(params.posts).grep(MultiTargetNode).reverse
             while (param = param_stack.pop)
               case param
               when MultiTargetNode
                 param_stack.concat(param.rights.reverse)
-                param_stack << param.rest
+                param_stack << param.rest if param.rest&.expression && !sorted.include?(param.rest.expression.name)
                 param_stack.concat(param.lefts.reverse)
               when RequiredParameterNode
                 sorted << param.name
               when SplatNode
-                sorted << param.expression.name if param.expression
+                sorted << param.expression.name
               end
             end
 
+            sorted << params.block.name if params.block&.name
+
             names = sorted.concat(names - sorted)
           end
 

src/prism.c

@@ -887,6 +887,22 @@ pm_node_flag_unset(pm_node_t *node, pm_node_flags_t flag) {
     node->flags &= (pm_node_flags_t) ~flag;
 }
 
+/**
+ * Set the repeated parameter flag on the given node.
+ */
+static inline void
+pm_node_flag_set_repeated_parameter(pm_node_t *node) {
+    assert(PM_NODE_TYPE(node) == PM_BLOCK_LOCAL_VARIABLE_NODE ||
+            PM_NODE_TYPE(node) == PM_BLOCK_PARAMETER_NODE ||
+            PM_NODE_TYPE(node) == PM_KEYWORD_REST_PARAMETER_NODE ||
+            PM_NODE_TYPE(node) == PM_OPTIONAL_KEYWORD_PARAMETER_NODE ||
+            PM_NODE_TYPE(node) == PM_OPTIONAL_PARAMETER_NODE ||
+            PM_NODE_TYPE(node) == PM_REQUIRED_KEYWORD_PARAMETER_NODE ||
+            PM_NODE_TYPE(node) == PM_REQUIRED_PARAMETER_NODE ||
+            PM_NODE_TYPE(node) == PM_REST_PARAMETER_NODE);
+
+    pm_node_flag_set(node, PM_PARAMETER_FLAGS_REPEATED_PARAMETER);
+}
 
 /******************************************************************************/
 /* Node creation functions                                                    */
@@ -5995,23 +6011,28 @@ pm_parser_local_add_owned(pm_parser_t *parser, const uint8_t *start, size_t leng
 /**
  * Add a parameter name to the current scope and check whether the name of the
  * parameter is unique or not.
+ *
+ * Returns `true` if this is a duplicate parameter name, otherwise returns
+ * false.
  */
-static void
+static bool
 pm_parser_parameter_name_check(pm_parser_t *parser, const pm_token_t *name) {
     // We want to check whether the parameter name is a numbered parameter or
     // not.
     pm_refute_numbered_parameter(parser, name->start, name->end);
 
-    // We want to ignore any parameter name that starts with an underscore.
-    if ((name->start < name->end) && (*name->start == '_')) return;
-
     // Otherwise we'll fetch the constant id for the parameter name and check
     // whether it's already in the current scope.
     pm_constant_id_t constant_id = pm_parser_constant_id_token(parser, name);
 
     if (pm_constant_id_list_includes(&parser->current_scope->locals, constant_id)) {
-        pm_parser_err_token(parser, name, PM_ERR_PARAMETER_NAME_REPEAT);
+        // Add an error if the parameter doesn't start with _ and has been seen before
+        if ((name->start < name->end) && (*name->start != '_')) {
+            pm_parser_err_token(parser, name, PM_ERR_PARAMETER_NAME_REPEAT);
+        }
+        return true;
     }
+    return false;
 }
 
 /**
@@ -11466,7 +11487,9 @@ parse_required_destructured_parameter(pm_parser_t *parser) {
             if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
                 pm_token_t name = parser->previous;
                 value = (pm_node_t *) pm_required_parameter_node_create(parser, &name);
-                pm_parser_parameter_name_check(parser, &name);
+                if (pm_parser_parameter_name_check(parser, &name)) {
+                    pm_node_flag_set_repeated_parameter(value);
+                }
                 pm_parser_local_add_token(parser, &name);
             }
 
@@ -11476,7 +11499,9 @@ parse_required_destructured_parameter(pm_parser_t *parser) {
             pm_token_t name = parser->previous;
 
             param = (pm_node_t *) pm_required_parameter_node_create(parser, &name);
-            pm_parser_parameter_name_check(parser, &name);
+            if (pm_parser_parameter_name_check(parser, &name)) {
+                pm_node_flag_set_repeated_parameter(param);
+            }
             pm_parser_local_add_token(parser, &name);
         }
 
@@ -11593,9 +11618,10 @@ parse_parameters(
                 pm_token_t operator = parser->previous;
                 pm_token_t name;
 
+                bool repeated = false;
                 if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
                     name = parser->previous;
-                    pm_parser_parameter_name_check(parser, &name);
+                    repeated = pm_parser_parameter_name_check(parser, &name);
                     pm_parser_local_add_token(parser, &name);
                 } else {
                     name = not_provided(parser);
@@ -11606,6 +11632,9 @@ parse_parameters(
                 }
 
                 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);
+                }
                 if (params->block == NULL) {
                     pm_parameters_node_block_set(params, param);
                 } else {
@@ -11678,7 +11707,7 @@ parse_parameters(
                 }
 
                 pm_token_t name = parser->previous;
-                pm_parser_parameter_name_check(parser, &name);
+                bool repeated = pm_parser_parameter_name_check(parser, &name);
                 pm_parser_local_add_token(parser, &name);
 
                 if (accept1(parser, PM_TOKEN_EQUAL)) {
@@ -11689,6 +11718,9 @@ parse_parameters(
                     pm_node_t *value = parse_value_expression(parser, binding_power, false, PM_ERR_PARAMETER_NO_DEFAULT);
 
                     pm_optional_parameter_node_t *param = pm_optional_parameter_node_create(parser, &name, &operator, value);
+                    if (repeated) {
+                        pm_node_flag_set_repeated_parameter((pm_node_t *)param);
+                    }
                     pm_parameters_node_optionals_append(params, param);
 
                     parser->current_param_name = old_param_name;
@@ -11703,9 +11735,15 @@ parse_parameters(
                     }
                 } else if (order > PM_PARAMETERS_ORDER_AFTER_OPTIONAL) {
                     pm_required_parameter_node_t *param = pm_required_parameter_node_create(parser, &name);
+                    if (repeated) {
+                        pm_node_flag_set_repeated_parameter((pm_node_t *)param);
+                    }
                     pm_parameters_node_requireds_append(params, (pm_node_t *) param);
                 } else {
                     pm_required_parameter_node_t *param = pm_required_parameter_node_create(parser, &name);
+                    if (repeated) {
+                        pm_node_flag_set_repeated_parameter((pm_node_t *)param);
+                    }
                     pm_parameters_node_posts_append(params, (pm_node_t *) param);
                 }
 
@@ -11720,14 +11758,17 @@ parse_parameters(
                 pm_token_t local = name;
                 local.end -= 1;
 
-                pm_parser_parameter_name_check(parser, &local);
+                bool repeated = pm_parser_parameter_name_check(parser, &local);
                 pm_parser_local_add_token(parser, &local);
 
                 switch (parser->current.type) {
                     case PM_TOKEN_COMMA:
                     case PM_TOKEN_PARENTHESIS_RIGHT:
                     case PM_TOKEN_PIPE: {
                         pm_node_t *param = (pm_node_t *) pm_required_keyword_parameter_node_create(parser, &name);
+                        if (repeated) {
+                            pm_node_flag_set_repeated_parameter(param);
+                        }
                         pm_parameters_node_keywords_append(params, param);
                         break;
                     }
@@ -11739,6 +11780,9 @@ parse_parameters(
                         }
 
                         pm_node_t *param = (pm_node_t *) pm_required_keyword_parameter_node_create(parser, &name);
+                        if (repeated) {
+                            pm_node_flag_set_repeated_parameter(param);
+                        }
                         pm_parameters_node_keywords_append(params, param);
                         break;
                     }
@@ -11758,6 +11802,9 @@ parse_parameters(
                             param = (pm_node_t *) pm_required_keyword_parameter_node_create(parser, &name);
                         }
 
+                        if (repeated) {
+                            pm_node_flag_set_repeated_parameter(param);
+                        }
                         pm_parameters_node_keywords_append(params, param);
 
                         // If parsing the value of the parameter resulted in error recovery,
@@ -11780,10 +11827,10 @@ parse_parameters(
 
                 pm_token_t operator = parser->previous;
                 pm_token_t name;
-
+                bool repeated = false;
                 if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
                     name = parser->previous;
-                    pm_parser_parameter_name_check(parser, &name);
+                    repeated = pm_parser_parameter_name_check(parser, &name);
                     pm_parser_local_add_token(parser, &name);
                 } else {
                     name = not_provided(parser);
@@ -11794,6 +11841,9 @@ parse_parameters(
                 }
 
                 pm_node_t *param = (pm_node_t *) pm_rest_parameter_node_create(parser, &operator, &name);
+                if (repeated) {
+                    pm_node_flag_set_repeated_parameter(param);
+                }
                 if (params->rest == NULL) {
                     pm_parameters_node_rest_set(params, param);
                 } else {
@@ -11816,9 +11866,10 @@ parse_parameters(
                 } else {
                     pm_token_t name;
 
+                    bool repeated = false;
                     if (accept1(parser, PM_TOKEN_IDENTIFIER)) {
                         name = parser->previous;
-                        pm_parser_parameter_name_check(parser, &name);
+                        repeated = pm_parser_parameter_name_check(parser, &name);
                         pm_parser_local_add_token(parser, &name);
                     } else {
                         name = not_provided(parser);
@@ -11829,6 +11880,9 @@ parse_parameters(
                     }
 
                     param = (pm_node_t *) pm_keyword_rest_parameter_node_create(parser, &operator, &name);
+                    if (repeated) {
+                        pm_node_flag_set_repeated_parameter(param);
+                    }
                 }
 
                 if (params->keyword_rest == NULL) {
@@ -12064,10 +12118,13 @@ parse_block_parameters(
     if ((opening->type != PM_TOKEN_NOT_PROVIDED) && accept1(parser, PM_TOKEN_SEMICOLON)) {
         do {
             expect1(parser, PM_TOKEN_IDENTIFIER, PM_ERR_BLOCK_PARAM_LOCAL_VARIABLE);
-            pm_parser_parameter_name_check(parser, &parser->previous);
+            bool repeated = pm_parser_parameter_name_check(parser, &parser->previous);
             pm_parser_local_add_token(parser, &parser->previous);
 
             pm_block_local_variable_node_t *local = pm_block_local_variable_node_create(parser, &parser->previous);
+            if (repeated) {
+                pm_node_flag_set_repeated_parameter((pm_node_t *)local);
+            }
             pm_block_parameters_node_append_local(block_parameters, local);
         } while (accept1(parser, PM_TOKEN_COMMA));
     }