[ruby/prism] Duplicated when clauses

ruby/prism@865b0d5fbe

lib/prism/prism.gemspec

@@ -54,6 +54,7 @@ Gem::Specification.new do |spec|
     "include/prism/parser.h",
     "include/prism/prettyprint.h",
     "include/prism/regexp.h",
+    "include/prism/static_literals.h",
     "include/prism/util/pm_buffer.h",
     "include/prism/util/pm_char.h",
     "include/prism/util/pm_constant_pool.h",
@@ -104,6 +105,7 @@ Gem::Specification.new do |spec|
     "src/prettyprint.c",
     "src/regexp.c",
     "src/serialize.c",
+    "src/static_literals.c",
     "src/token_type.c",
     "src/util/pm_buffer.c",
     "src/util/pm_char.c",

prism/diagnostic.c

@@ -307,6 +307,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_LEN] = {
     [PM_WARN_AMBIGUOUS_PREFIX_STAR]             = { "ambiguous `*` has been interpreted as an argument prefix", PM_WARNING_LEVEL_VERBOSE },
     [PM_WARN_AMBIGUOUS_SLASH]                   = { "ambiguous `/`; wrap regexp in parentheses or add a space after `/` operator", PM_WARNING_LEVEL_VERBOSE },
     [PM_WARN_DUPLICATED_HASH_KEY]               = { "key %.*s is duplicated and overwritten on line %" PRIi32, PM_WARNING_LEVEL_DEFAULT },
+    [PM_WARN_DUPLICATED_WHEN_CLAUSE]            = { "duplicated 'when' clause with line %" PRIi32 " is ignored", PM_WARNING_LEVEL_VERBOSE },
     [PM_WARN_EQUAL_IN_CONDITIONAL]              = { "found `= literal' in conditional, should be ==", PM_WARNING_LEVEL_DEFAULT },
     [PM_WARN_END_IN_METHOD]                     = { "END in method; use at_exit", PM_WARNING_LEVEL_DEFAULT },
     [PM_WARN_FLOAT_OUT_OF_RANGE]                = { "Float %.*s%s out of range", PM_WARNING_LEVEL_VERBOSE }

prism/diagnostic.h

@@ -307,6 +307,7 @@ typedef enum {
     PM_WARN_EQUAL_IN_CONDITIONAL,
     PM_WARN_END_IN_METHOD,
     PM_WARN_DUPLICATED_HASH_KEY,
+    PM_WARN_DUPLICATED_WHEN_CLAUSE,
     PM_WARN_FLOAT_OUT_OF_RANGE,
 
     // This is the number of diagnostic codes.

prism/prism.c

@@ -11697,6 +11697,23 @@ pm_hash_key_static_literals_add(pm_parser_t *parser, pm_static_literals_t *liter
     }
 }
 
+/**
+ * Add a node to a set of static literals that holds a set of hash keys. If the
+ * node is a duplicate, then add an appropriate warning.
+ */
+static void
+pm_when_clause_static_literals_add(pm_parser_t *parser, pm_static_literals_t *literals, pm_node_t *node) {
+    if (pm_static_literals_add(parser, literals, node) != NULL) {
+        pm_diagnostic_list_append_format(
+            &parser->warning_list,
+            node->location.start,
+            node->location.end,
+            PM_WARN_DUPLICATED_WHEN_CLAUSE,
+            pm_newline_list_line_column(&parser->newline_list, node->location.start, parser->start_line).line
+        );
+    }
+}
+
 /**
  * Parse all of the elements of a hash. returns true if a double splat was found.
  */
@@ -15335,10 +15352,11 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
 
             if (match1(parser, PM_TOKEN_KEYWORD_WHEN)) {
                 pm_case_node_t *case_node = pm_case_node_create(parser, &case_keyword, predicate, &end_keyword);
+                pm_static_literals_t literals = { 0 };
 
                 // At this point we've seen a when keyword, so we know this is a
-                // case-when node. We will continue to parse the when nodes until we hit
-                // the end of the list.
+                // case-when node. We will continue to parse the when nodes
+                // until we hit the end of the list.
                 while (accept1(parser, PM_TOKEN_KEYWORD_WHEN)) {
                     pm_token_t when_keyword = parser->previous;
                     pm_when_node_t *when_node = pm_when_node_create(parser, &when_keyword);
@@ -15356,7 +15374,17 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
                             pm_node_t *condition = parse_value_expression(parser, PM_BINDING_POWER_DEFINED, false, PM_ERR_CASE_EXPRESSION_AFTER_WHEN);
                             pm_when_node_conditions_append(when_node, condition);
 
+                            // If we found a missing node, then this is a syntax
+                            // error and we should stop looping.
                             if (PM_NODE_TYPE_P(condition, PM_MISSING_NODE)) break;
+
+                            // If this is a string node, then we need to mark it
+                            // as frozen because when clause strings are frozen.
+                            if (PM_NODE_TYPE_P(condition, PM_STRING_NODE)) {
+                                pm_node_flag_set(condition, PM_STRING_FLAGS_FROZEN | PM_NODE_FLAG_STATIC_LITERAL);
+                            }
+
+                            pm_when_clause_static_literals_add(parser, &literals, condition);
                         }
                     } while (accept1(parser, PM_TOKEN_COMMA));
 
@@ -15382,6 +15410,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
                     pm_parser_err_token(parser, &case_keyword, PM_ERR_CASE_MISSING_CONDITIONS);
                 }
 
+                pm_static_literals_free(&literals);
                 node = (pm_node_t *) case_node;
             } else {
                 pm_case_match_node_t *case_node = pm_case_match_node_create(parser, &case_keyword, predicate, &end_keyword);

prism/util/pm_integer.c

@@ -172,7 +172,6 @@ pm_integer_compare(const pm_integer_t *left, const pm_integer_t *right) {
     }
 
     return 0;
-
 }
 
 /**

test/prism/snapshots/whitequark/case_expr.txt

@@ -20,7 +20,7 @@
             │       ├── keyword_loc: (1,10)-(1,14) = "when"
             │       ├── conditions: (length: 1)
             │       │   └── @ StringNode (location: (1,15)-(1,20))
-            │       │       ├── flags: ∅
+            │       │       ├── flags: frozen
             │       │       ├── opening_loc: (1,15)-(1,16) = "'"
             │       │       ├── content_loc: (1,16)-(1,19) = "bar"
             │       │       ├── closing_loc: (1,19)-(1,20) = "'"

test/prism/snapshots/whitequark/case_expr_else.txt

@@ -20,7 +20,7 @@
             │       ├── keyword_loc: (1,10)-(1,14) = "when"
             │       ├── conditions: (length: 1)
             │       │   └── @ StringNode (location: (1,15)-(1,20))
-            │       │       ├── flags: ∅
+            │       │       ├── flags: frozen
             │       │       ├── opening_loc: (1,15)-(1,16) = "'"
             │       │       ├── content_loc: (1,16)-(1,19) = "bar"
             │       │       ├── closing_loc: (1,19)-(1,20) = "'"

test/prism/snapshots/whitequark/when_multi.txt

@@ -20,13 +20,13 @@
             │       ├── keyword_loc: (1,10)-(1,14) = "when"
             │       ├── conditions: (length: 2)
             │       │   ├── @ StringNode (location: (1,15)-(1,20))
-            │       │   │   ├── flags: ∅
+            │       │   │   ├── flags: frozen
             │       │   │   ├── opening_loc: (1,15)-(1,16) = "'"
             │       │   │   ├── content_loc: (1,16)-(1,19) = "bar"
             │       │   │   ├── closing_loc: (1,19)-(1,20) = "'"
             │       │   │   └── unescaped: "bar"
             │       │   └── @ StringNode (location: (1,22)-(1,27))
-            │       │       ├── flags: ∅
+            │       │       ├── flags: frozen
             │       │       ├── opening_loc: (1,22)-(1,23) = "'"
             │       │       ├── content_loc: (1,23)-(1,26) = "baz"
             │       │       ├── closing_loc: (1,26)-(1,27) = "'"

test/prism/snapshots/whitequark/when_then.txt

@@ -20,7 +20,7 @@
             │       ├── keyword_loc: (1,10)-(1,14) = "when"
             │       ├── conditions: (length: 1)
             │       │   └── @ StringNode (location: (1,15)-(1,20))
-            │       │       ├── flags: ∅
+            │       │       ├── flags: frozen
             │       │       ├── opening_loc: (1,15)-(1,16) = "'"
             │       │       ├── content_loc: (1,16)-(1,19) = "bar"
             │       │       ├── closing_loc: (1,19)-(1,20) = "'"

test/prism/static_literals_test.rb

@@ -46,23 +46,36 @@ def test_static_literals
 
     private
 
-    def parse_warning(left, right)
-      source = <<~RUBY
+    def parse_warnings(left, right)
+      warnings = []
+
+      warnings << Prism.parse(<<~RUBY, filepath: __FILE__).warnings.first
         {
           #{left} => 1,
           #{right} => 2
         }
       RUBY
 
-      Prism.parse(source, filepath: __FILE__).warnings.first
+      warnings << Prism.parse(<<~RUBY, filepath: __FILE__).warnings.first
+        case foo
+        when #{left}
+        when #{right}
+        end
+      RUBY
+
+      warnings
     end
 
     def assert_warning(left, right = left)
-      assert_match %r{key #{Regexp.escape(left)} .+ line 3}, parse_warning(left, right)&.message
+      hash_keys, when_clauses = parse_warnings(left, right)
+
+      assert_include hash_keys.message, left
+      assert_include hash_keys.message, "line 3"
+      assert_include when_clauses.message, "line 3"
     end
 
     def refute_warning(left, right)
-      assert_nil parse_warning(left, right)
+      assert_empty parse_warnings(left, right).compact
     end
   end
 end