Quoted values are not valid dictionary keys anymore (facebookresearch…

…#1599)
odelalleau · May 3, 2021 · 901fb39 · 901fb39
1 parent 69924a3
commit 901fb39
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 27 deletions.
diff --git a/hydra/grammar/OverrideParser.g4 b/hydra/grammar/OverrideParser.g4
@@ -67,13 +67,12 @@ primitive:
 
 // Same as `primitive` except that `COLON` and `INTERPOLATION` are not allowed.
 dictKey:
-      QUOTED_VALUE                               // 'hello world', "hello world"
-    | (   ID                                     // foo_10
-        | NULL                                   // null, NULL
-        | INT                                    // 0, 10, -20, 1_000_000
-        | FLOAT                                  // 3.14, -20.0, 1e-1, -10e3
-        | BOOL                                   // true, TrUe, false, False
-        | UNQUOTED_CHAR                          // /, -, \, +, ., $, %, *, @
-        | ESC                                    // \\, \(, \), \[, \], \{, \}, \:, \=, \ , \\t, \,
-        | WS                                     // whitespaces
+    (   ID                                     // foo_10
+      | NULL                                   // null, NULL
+      | INT                                    // 0, 10, -20, 1_000_000
+      | FLOAT                                  // 3.14, -20.0, 1e-1, -10e3
+      | BOOL                                   // true, TrUe, false, False
+      | UNQUOTED_CHAR                          // /, -, \, +, ., $, %, *, @
+      | ESC                                    // \\, \(, \), \[, \], \{, \}, \:, \=, \ , \\t, \,
+      | WS                                     // whitespaces
     )+;
diff --git a/tests/test_overrides_parser.py b/tests/test_overrides_parser.py
@@ -273,14 +273,6 @@ def test_shuffle_sequence(value: str, expected: Any) -> None:
         param("{a:10,b:20}", {"a": 10, "b": 20}, id="dict"),
         param("{a:10,b:{}}", {"a": 10, "b": {}}, id="dict"),
         param("{a:10,b:{c:[1,2]}}", {"a": 10, "b": {"c": [1, 2]}}, id="dict"),
-        param(
-            "{'0a': 0, \"1b\": 1}",
-            {
-                QuotedString(text="0a", quote=Quote.single): 0,
-                QuotedString(text="1b", quote=Quote.double): 1,
-            },
-            id="dict_quoted_key",
-        ),
         param("{null: 1}", {None: 1}, id="dict_null_key"),
         param("{123: 1, 0: 2, -1: 3}", {123: 1, 0: 2, -1: 3}, id="dict_int_key"),
         param("{3.14: 0, 1e3: 1}", {3.14: 0, 1000.0: 1}, id="dict_float_key"),
@@ -293,9 +285,9 @@ def test_shuffle_sequence(value: str, expected: Any) -> None:
         ),
         param("{white spaces: 1}", {"white spaces": 1}, id="dict_ws_key"),
         param(
-            "{'a:b': 1, ab 123.5 True: 2, null false: 3, 1: 4, null: 5}",
+            "{a_b: 1, ab 123.5 True: 2, null false: 3, 1: 4, null: 5}",
             {
-                QuotedString(text="a:b", quote=Quote.single): 1,
+                "a_b": 1,
                 "ab 123.5 True": 2,
                 "null false": 3,
                 1: 4,
@@ -437,6 +429,30 @@ def test_interval_sweep(value: str, expected: Any) -> None:
             raises(HydraException, match=re.escape("token recognition error at: '''")),
             id="error:left_overs",
         ),
+        param(
+            "dictContainer",
+            "{'0a': 0, \"1b\": 1}",
+            raises(HydraException, match=re.escape("mismatched input ''0a''")),
+            id="error:dict_quoted_key_dictContainer",
+        ),
+        param(
+            "override",
+            "key={' abc ': 0}",
+            raises(
+                HydraException,
+                match=re.escape("no viable alternative at input '{' abc ''"),
+            ),
+            id="error:dict_quoted_key_override_single",
+        ),
+        param(
+            "override",
+            'key={" abc ": 0}',
+            raises(
+                HydraException,
+                match=re.escape("""no viable alternative at input '{" abc "'"""),
+            ),
+            id="error:dict_quoted_key_override_double",
+        ),
     ],
 )
 def test_parse_errors(rule: str, value: str, expected: Any) -> None:
@@ -938,12 +954,6 @@ def test_get_key_element(override: str, expected: str) -> None:
         param("key={a:10,b:20}", "{a:10,b:20}", False, id="dict"),
         param("key={a:10,b:20}", "{a: 10, b: 20}", True, id="dict"),
         param("key={a:10,b:[1,2,3]}", "{a: 10, b: [1, 2, 3]}", True, id="dict"),
-        param(
-            "key={'null':1, \"a:b\": 0}",
-            "{'null': 1, \"a:b\": 0}",
-            True,
-            id="dict_quoted_key",
-        ),
         param(
             "key={/-\\+.$%*@: 1}",
             "{/-\\\\+.$%*@: 1}",  # note that \ gets escaped
@@ -991,8 +1001,6 @@ def test_override_get_value_element_method(
         param("key={a:10,b:20}", {"a": 10, "b": 20}, id="dict"),
         param("key={a:10,b:[1,2,3]}", {"a": 10, "b": [1, 2, 3]}, id="dict"),
         param("key={123id: 0}", {"123id": 0}, id="dict_key_int_plus_id"),
-        param("key={' abc ': 0}", {" abc ": 0}, id="dict_key_quoted_single"),
-        param('key={" abc ": 0}', {" abc ": 0}, id="dict_key_quoted_double"),
         param("key={a/-\\+.$%*@: 0}", {"a/-\\+.$%*@": 0}, id="dict_key_noquote"),
         param("key={w s: 0}", {"w s": 0}, id="dict_key_ws"),
         param(