Repository: ruby/prism
Pull request #2069 (2023-12-13 22:49:07)

Fix the implementation of the flag on keyword hash nodes
The previous implementation in #2046 was incorrect since it was just checking for all keys in assoc nodes to be static literals but the actual check is that all keys in assoc nodes must be symbol nodes (Ref: https://github.com/ruby/ruby/blob/2f1d6da8c45590bf3461ed4bf051a4e1009eaf85/compile.c#L4577-L4578).

This commit fixes that implementation, and, also, aliases the flag to `PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS` so that ruby/ruby can start using the new flag name.

I intend to later change the real flag name to `PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS` and remove the alias.


Patch:
diff --git a/include/prism/parser.h b/include/prism/parser.h
index 2c58131b..7f26054f 100644
--- a/include/prism/parser.h
+++ b/include/prism/parser.h
@@ -17,6 +17,12 @@
 
 #include <stdbool.h>
 
+// TODO: remove this by renaming the original flag
+/**
+ * Temporary alias for the PM_NODE_FLAG_STATIC_KEYS flag.
+ */
+#define PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS PM_KEYWORD_HASH_NODE_FLAGS_STATIC_KEYS
+
 /**
  * This enum provides various bits that represent different kinds of states that
  * the lexer can track. This is used to determine which kind of token to return
diff --git a/src/prism.c b/src/prism.c
index 4b15f658..74e92c89 100644
--- a/src/prism.c
+++ b/src/prism.c
@@ -3949,7 +3949,7 @@ pm_keyword_hash_node_create(pm_parser_t *parser) {
         .base = {
             .type = PM_KEYWORD_HASH_NODE,
             .location = PM_OPTIONAL_LOCATION_NOT_PROVIDED_VALUE,
-            .flags = PM_KEYWORD_HASH_NODE_FLAGS_STATIC_KEYS
+            .flags = PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS
         },
         .elements = { 0 }
     };
@@ -3962,10 +3962,11 @@ pm_keyword_hash_node_create(pm_parser_t *parser) {
  */
 static void
 pm_keyword_hash_node_elements_append(pm_keyword_hash_node_t *hash, pm_node_t *element) {
-    // If the element being added is not an AssocNode or does not have a static literal key, then
+    // If the element being added is not an AssocNode or does not have a symbol key, then
     // we want to turn the STATIC_KEYS flag off.
-    if (!PM_NODE_TYPE_P(element, PM_ASSOC_NODE) || !PM_NODE_FLAG_P(((pm_assoc_node_t *) element)->key, PM_NODE_FLAG_STATIC_LITERAL)) {
-        pm_node_flag_unset((pm_node_t *)hash, PM_KEYWORD_HASH_NODE_FLAGS_STATIC_KEYS);
+    // TODO: Rename the flag to SYMBOL_KEYS instead.
+    if (!PM_NODE_TYPE_P(element, PM_ASSOC_NODE) || !PM_NODE_TYPE_P(((pm_assoc_node_t *) element)->key, PM_SYMBOL_NODE)) {
+        pm_node_flag_unset((pm_node_t *)hash, PM_KEYWORD_HASH_NODE_FLAGS_SYMBOL_KEYS);
     }
 
     pm_node_list_append(&hash->elements, element);


Test patch:
diff --git a/test/prism/snapshots/seattlerb/aref_args_assocs.txt b/test/prism/snapshots/seattlerb/aref_args_assocs.txt
index 8286751c..398a0142 100644
--- a/test/prism/snapshots/seattlerb/aref_args_assocs.txt
+++ b/test/prism/snapshots/seattlerb/aref_args_assocs.txt
@@ -7,7 +7,7 @@
             ├── flags: ∅
             ├── elements: (length: 1)
             │   └── @ KeywordHashNode (location: (1,1)-(1,7))
-            │       ├── flags: static_keys
+            │       ├── flags: ∅
             │       └── elements: (length: 1)
             │           └── @ AssocNode (location: (1,1)-(1,7))
             │               ├── key:
diff --git a/test/prism/snapshots/seattlerb/aref_args_lit_assocs.txt b/test/prism/snapshots/seattlerb/aref_args_lit_assocs.txt
index c809f426..7a63842f 100644
--- a/test/prism/snapshots/seattlerb/aref_args_lit_assocs.txt
+++ b/test/prism/snapshots/seattlerb/aref_args_lit_assocs.txt
@@ -9,7 +9,7 @@
             │   ├── @ IntegerNode (location: (1,1)-(1,2))
             │   │   └── flags: decimal
             │   └── @ KeywordHashNode (location: (1,4)-(1,10))
-            │       ├── flags: static_keys
+            │       ├── flags: ∅
             │       └── elements: (length: 1)
             │           └── @ AssocNode (location: (1,4)-(1,10))
             │               ├── key:
diff --git a/test/prism/snapshots/seattlerb/call_arg_assoc.txt b/test/prism/snapshots/seattlerb/call_arg_assoc.txt
index 4b7666a3..b52ba539 100644
--- a/test/prism/snapshots/seattlerb/call_arg_assoc.txt
+++ b/test/prism/snapshots/seattlerb/call_arg_assoc.txt
@@ -17,7 +17,7 @@
             │       ├── @ IntegerNode (location: (1,2)-(1,3))
             │       │   └── flags: decimal
             │       └── @ KeywordHashNode (location: (1,5)-(1,9))
-            │           ├── flags: static_keys
+            │           ├── flags: ∅
             │           └── elements: (length: 1)
             │               └── @ AssocNode (location: (1,5)-(1,9))
             │                   ├── key:
diff --git a/test/prism/snapshots/seattlerb/call_args_assoc_trailing_comma.txt b/test/prism/snapshots/seattlerb/call_args_assoc_trailing_comma.txt
index a21be7ff..ae1e2be4 100644
--- a/test/prism/snapshots/seattlerb/call_args_assoc_trailing_comma.txt
+++ b/test/prism/snapshots/seattlerb/call_args_assoc_trailing_comma.txt
@@ -17,7 +17,7 @@
             │       ├── @ IntegerNode (location: (1,2)-(1,3))
             │       │   └── flags: decimal
             │       └── @ KeywordHashNode (location: (1,5)-(1,9))
-            │           ├── flags: static_keys
+            │           ├── flags: ∅
             │           └── elements: (length: 1)
             │               └── @ AssocNode (location: (1,5)-(1,9))
             │                   ├── key:
diff --git a/test/prism/snapshots/seattlerb/call_assoc.txt b/test/prism/snapshots/seattlerb/call_assoc.txt
index ba458373..ac3a0e1e 100644
--- a/test/prism/snapshots/seattlerb/call_assoc.txt
+++ b/test/prism/snapshots/seattlerb/call_assoc.txt
@@ -15,7 +15,7 @@
             │   ├── flags: ∅
             │   └── arguments: (length: 1)
             │       └── @ KeywordHashNode (location: (1,2)-(1,6))
-            │           ├── flags: static_keys
+            │           ├── flags: ∅
             │           └── elements: (length: 1)
             │               └── @ AssocNode (location: (1,2)-(1,6))
             │                   ├── key:
diff --git a/test/prism/snapshots/seattlerb/call_assoc_trailing_comma.txt b/test/prism/snapshots/seattlerb/call_assoc_trailing_comma.txt
index 20e402ff..18e937c5 100644
--- a/test/prism/snapshots/seattlerb/call_assoc_trailing_comma.txt
+++ b/test/prism/snapshots/seattlerb/call_assoc_trailing_comma.txt
@@ -15,7 +15,7 @@
             │   ├── flags: ∅
             │   └── arguments: (length: 1)
             │       └── @ KeywordHashNode (location: (1,2)-(1,6))
-            │           ├── flags: static_keys
+            │           ├── flags: ∅
             │           └── elements: (length: 1)
             │               └── @ AssocNode (location: (1,2)-(1,6))
             │                   ├── key:
diff --git a/test/prism/snapshots/seattlerb/method_call_assoc_trailing_comma.txt b/test/prism/snapshots/seattlerb/method_call_assoc_trailing_comma.txt
index 75ae0d99..e9eeb336 100644
--- a/test/prism/snapshots/seattlerb/method_call_assoc_trailing_comma.txt
+++ b/test/prism/snapshots/seattlerb/method_call_assoc_trailing_comma.txt
@@ -25,7 +25,7 @@
             │   ├── flags: ∅
             │   └── arguments: (length: 1)
             │       └── @ KeywordHashNode (location: (1,4)-(1,8))
-            │           ├── flags: static_keys
+            │           ├── flags: ∅
             │           └── elements: (length: 1)
             │               └── @ AssocNode (location: (1,4)-(1,8))
             │                   ├── key:
diff --git a/test/prism/snapshots/seattlerb/parse_opt_call_args_assocs_comma.txt b/test/prism/snapshots/seattlerb/parse_opt_call_args_assocs_comma.txt
index 3dc68795..310507d2 100644
--- a/test/prism/snapshots/seattlerb/parse_opt_call_args_assocs_comma.txt
+++ b/test/prism/snapshots/seattlerb/parse_opt_call_args_assocs_comma.txt
@@ -17,7 +17,7 @@
             │   ├── flags: ∅
             │   └── arguments: (length: 1)
             │       └── @ KeywordHashNode (location: (1,2)-(1,6))
-            │           ├── flags: static_keys
+            │           ├── flags: ∅
             │           └── elements: (length: 1)
             │               └── @ AssocNode (location: (1,2)-(1,6))
             │                   ├── key:
diff --git a/test/prism/snapshots/whitequark/array_assocs.txt b/test/prism/snapshots/whitequark/array_assocs.txt
index 2b374744..c66356b8 100644
--- a/test/prism/snapshots/whitequark/array_assocs.txt
+++ b/test/prism/snapshots/whitequark/array_assocs.txt
@@ -7,7 +7,7 @@
         │   ├── flags: ∅
         │   ├── elements: (length: 1)
         │   │   └── @ KeywordHashNode (location: (1,2)-(1,8))
-        │   │       ├── flags: static_keys
+        │   │       ├── flags: ∅
         │   │       └── elements: (length: 1)
         │   │           └── @ AssocNode (location: (1,2)-(1,8))
         │   │               ├── key:
@@ -25,7 +25,7 @@
             │   ├── @ IntegerNode (location: (3,2)-(3,3))
             │   │   └── flags: decimal
             │   └── @ KeywordHashNode (location: (3,5)-(3,11))
-            │       ├── flags: static_keys
+            │       ├── flags: ∅
             │       └── elements: (length: 1)
             │           └── @ AssocNode (location: (3,5)-(3,11))
             │               ├── key: