Support and deprecate empty args

news/572.api_change

@@ -0,0 +1,2 @@
+Implicitly empty resolver arguments (e.g., `${foo:a,}`) are deprecated in favor of explicit quoted strings (e.g., `${foo:a,""}`)
+

omegaconf/grammar/OmegaConfGrammarParser.g4

@@ -42,7 +42,7 @@ element:
 listContainer: BRACKET_OPEN sequence? BRACKET_CLOSE;                         // [], [1,2,3], [a,b,[1,2]]
 dictContainer: BRACE_OPEN (dictKeyValuePair (COMMA dictKeyValuePair)*)? BRACE_CLOSE;  // {}, {a:10,b:20}
 dictKeyValuePair: dictKey COLON element;
-sequence: element (COMMA element)*;
+sequence: (element (COMMA element?)*) | (COMMA element?)+;
 
 // Interpolations.
 

omegaconf/grammar_visitor.py

@@ -1,4 +1,5 @@
 import sys
+import warnings
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -246,20 +247,42 @@ def visitSequence(
     ) -> Generator[Any, None, None]:
         from ._utils import _get_value
 
-        assert ctx.getChildCount() >= 1  # element (COMMA element)*
-        for i, child in enumerate(ctx.getChildren()):
-            if i % 2 == 0:
-                assert isinstance(child, OmegaConfGrammarParser.ElementContext)
+        # (element (COMMA element?)*) | (COMMA element?)+
+        assert ctx.getChildCount() >= 1
+
+        # DEPRECATED: remove in 2.2 (revert #571)
+        def empty_str_warning() -> None:
+            txt = ctx.getText()
+            warnings.warn(
+                f"In the sequence `{txt}` some elements are missing: please replace "
+                f"them with empty quoted strings. "
+                f"See https://github.com/omry/omegaconf/issues/572 for details.",
+                category=UserWarning,
+            )
+
+        is_previous_comma = True  # whether previous child was a comma (init to True)
+        for child in ctx.getChildren():
+            if isinstance(child, OmegaConfGrammarParser.ElementContext):
                 # Also preserve the original text representation of `child` so
                 # as to allow backward compatibility with old resolvers (registered
                 # with `legacy_register_resolver()`). Note that we cannot just cast
                 # the value to string later as for instance `null` would become "None".
                 yield _get_value(self.visitElement(child)), child.getText()
+                is_previous_comma = False
             else:
                 assert (
                     isinstance(child, TerminalNode)
                     and child.symbol.type == OmegaConfGrammarLexer.COMMA
                 )
+                if is_previous_comma:
+                    empty_str_warning()
+                    yield "", ""
+                else:
+                    is_previous_comma = True
+        if is_previous_comma:
+            # Trailing comma.
+            empty_str_warning()
+            yield "", ""
 
     def visitSingleElement(
         self, ctx: OmegaConfGrammarParser.SingleElementContext

tests/test_grammar.py

@@ -1,8 +1,9 @@
 import math
+import re
 from typing import Any, Callable, List, Optional, Tuple
 
 import antlr4
-from pytest import mark, param, raises
+from pytest import mark, param, raises, warns
 
 from omegaconf import (
     DictConfig,
@@ -369,6 +370,30 @@ def test_config_value(
         parse_tree, expected_visit = self._parse("configValue", definition, expected)
         self._visit_with_config(parse_tree, expected_visit)
 
+    @parametrize_from(
+        [
+            ("trailing_comma", "${test:a,b,}", ["a", "b", ""]),
+            ("empty_middle", "${test:a,,b}", ["a", "", "b"]),
+            ("empty_first", "${test:,a,b}", ["", "a", "b"]),
+            ("single_comma", "${test:,}", ["", ""]),
+            (
+                "mixed_with_ws",
+                "${test:  ,a,b,\t,,c,  \t   \t ,d,, \t}",
+                ["", "a", "b", "", "", "c", "", "d", "", ""],
+            ),
+        ]
+    )
+    def test_deprecated_empty_args(
+        self, restore_resolvers: Any, definition: str, expected: Any
+    ) -> None:
+        OmegaConf.register_new_resolver("test", self._resolver_test)
+
+        parse_tree, expected_visit = self._parse("singleElement", definition, expected)
+        with warns(
+            UserWarning, match=re.escape("https://github.com/omry/omegaconf/issues/572")
+        ):
+            self._visit_with_config(parse_tree, expected_visit)
+
     def _check_is_same_type(self, value: Any, expected: Any) -> None:
         """
         Helper function to validate that types of `value` and `expected are the same.