Repository: omry/omegaconf

Issue:
Add a clear error for Union in structured configs not being supported for now
cc @jrapin


Discussion:
deepcode-ci-bot[bot]: ## DeepCode's analysis on [#08bced](https://www.deepcode.ai/app/gh/omry/omegaconf/2f42a758dbf0b515576d59bfcd13a6beda93001b/omry/omegaconf/08bced2b859407d3fb19b68ffa570248a308219c/_/%2F/code/?utm_source=gh_review) found:

:warning: **1** warning  :heavy_check_mark: **0** issues were fixed.

##### 👉 View analysis in [**DeepCode’s Dashboard**](https://www.deepcode.ai/app/gh/omry/omegaconf/2f42a758dbf0b515576d59bfcd13a6beda93001b/omry/omegaconf/08bced2b859407d3fb19b68ffa570248a308219c/_/%2F/code/?utm_source=gh_review)
---
:relaxed: If you want to provide feedback on our bot, here is how to [contact us](https://www.deepcode.ai/feedback?select=4&utm_source=gh_review).


Patch:
diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py
index bcd4be3..bec1fe1 100644
--- a/omegaconf/_utils.py
+++ b/omegaconf/_utils.py
@@ -9,6 +9,7 @@ import yaml
 from .errors import (
     ConfigIndexError,
     ConfigTypeError,
+    ConfigValueError,
     KeyValidationError,
     OmegaConfBaseException,
     ValidationError,
@@ -76,9 +77,11 @@ def _get_class(path: str) -> type:
     return klass
 
 
-def _resolve_optional(type_: Any) -> Tuple[bool, Any]:
-    from typing import Union
+def _is_union(type_: Any) -> bool:
+    return getattr(type_, "__origin__", None) is Union
+
 
+def _resolve_optional(type_: Any) -> Tuple[bool, Any]:
     if getattr(type_, "__origin__", None) is Union:
         args = type_.__args__
         if len(args) == 2 and args[1] == type(None):  # noqa E721
@@ -107,8 +110,8 @@ def get_attr_data(obj: Any) -> Dict[str, Any]:
     obj_type = obj if is_type else type(obj)
     for name, attrib in attr.fields_dict(obj_type).items():
         is_optional, type_ = _resolve_optional(attrib.type)
-        type_ = _resolve_forward(type_, obj.__module__)
         is_nested = is_attr_class(type_)
+        type_ = _resolve_forward(type_, obj.__module__)
         if not is_type:
             value = getattr(obj, name)
         else:
@@ -118,9 +121,15 @@ def get_attr_data(obj: Any) -> Dict[str, Any]:
                     value = type_
                 else:
                     raise ValueError(
-                        "Missing default value for {}, to indicate "
-                        "default must be populated later use '???'".format(name)
+                        f"Missing default value for {name}, to indicate "
+                        "default must be populated later use OmegaConf.MISSING"
                     )
+        if _is_union(type_):
+            e = ConfigValueError(
+                f"Union types are not supported:\n{name}: {type_str(type_)}"
+            )
+            format_and_raise(node=None, key=None, value=value, cause=e, msg=str(e))
+
         d[name] = _maybe_wrap(
             ref_type=type_, is_optional=is_optional, key=name, value=value, parent=None,
         )
@@ -146,10 +155,15 @@ def get_dataclass_data(obj: Any) -> Dict[str, Any]:
                     value = type_
                 else:
                     raise ValueError(
-                        "Missing default value for {}, to indicate "
-                        "default must be populated later use '???'".format(name)
+                        f"Missing default value for {name}, to indicate "
+                        "default must be populated later use OmegaConf.MISSING"
                     )
 
+        if _is_union(type_):
+            e = ConfigValueError(
+                f"Union types are not supported:\n{name}: {type_str(type_)}"
+            )
+            format_and_raise(node=None, key=None, value=value, cause=e, msg=str(e))
         d[name] = _maybe_wrap(
             ref_type=type_, is_optional=is_optional, key=name, value=value, parent=None,
         )
@@ -429,15 +443,15 @@ def format_and_raise(
     ref_type_str: Optional[str]
 
     child_node: Optional[Node] = None
-    if key is not None:
-        child_node = node._get_node(key, validate_access=False)
-
     if node is None:
         full_key = ""
         object_type = None
         ref_type = None
         ref_type_str = None
     else:
+        if key is not None:
+            child_node = node._get_node(key, validate_access=False)
+
         full_key = node._get_full_key(key=key)
 
         ref_type = OmegaConf.get_ref_type(node)
@@ -497,10 +511,27 @@ def format_and_raise(
 
 
 def type_str(t: Any) -> str:
-    if hasattr(t, "__name__"):
-        name = str(t.__name__)
-    else:
-        name = str(t._name)  # pragma: no cover
+    if sys.version_info < (3, 7, 0):  # pragma: no cover
+        # Python 3.6
+        if hasattr(t, "__name__"):
+            name = str(t.__name__)
+        else:
+            if t.__origin__ is not None:
+                name = type_str(t.__origin__)
+            else:
+                name = str(t)
+                if name.startswith("typing."):
+                    name = name[len("typing.") :]
+    else:  # pragma: no cover
+        # Python >= 3.7
+        if hasattr(t, "__name__"):
+            name = str(t.__name__)
+        else:
+            if t._name is None:
+                if t.__origin__ is not None:
+                    name = type_str(t.__origin__)
+            else:
+                name = str(t._name)
 
     args = getattr(t, "__args__", None)
     if args is not None:
diff --git a/omegaconf/omegaconf.py b/omegaconf/omegaconf.py
index c40cf22..6652a1e 100644
--- a/omegaconf/omegaconf.py
+++ b/omegaconf/omegaconf.py
@@ -40,6 +40,7 @@ from ._utils import (
     is_primitive_type,
     is_structured_config,
     isint,
+    type_str,
     valid_value_annotation_type,
 )
 from .base import Container, Node
@@ -586,7 +587,7 @@ def _node_wrap(
 ) -> ValueNode:
     if not valid_value_annotation_type(type_):
         raise ValidationError(
-            f"Annotated class '{type_.__name__}' is not a structured config. "
+            f"Annotated class '{type_str(type_)}' is not a structured config. "
             "did you forget to decorate it as a dataclass?"
         )
     node: ValueNode


Test patch:
diff --git a/tests/__init__.py b/tests/__init__.py
index 41a64ed..b831bca 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -1,7 +1,7 @@
 from contextlib import contextmanager
 from dataclasses import dataclass, field
 from enum import Enum
-from typing import Any, Dict, Iterator, List, Optional
+from typing import Any, Dict, Iterator, List, Optional, Union
 
 from omegaconf import II, MISSING
 
@@ -83,3 +83,8 @@ class StructuredWithMissing:
     inter_num: int = II("num")
     inter_user: User = II("user")
     inter_opt_user: Optional[User] = II("opt_user")
+
+
+@dataclass
+class UnionError:
+    x: Union[int, str] = 10
diff --git a/tests/structured_conf/data/attr_classes.py b/tests/structured_conf/data/attr_classes.py
index 8e088c4..c9767b1 100644
--- a/tests/structured_conf/data/attr_classes.py
+++ b/tests/structured_conf/data/attr_classes.py
@@ -1,4 +1,4 @@
-from typing import Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, List, Optional, Tuple, Union
 
 import attr  # noqaE402
 import pytest
@@ -387,3 +387,8 @@ class MissingTest:
 @attr.s(auto_attribs=True)
 class NestedWithNone:
     plugin: Optional[Plugin] = None
+
+
+@attr.s(auto_attribs=True)
+class UnionError:
+    x: Union[int, str] = 10
diff --git a/tests/structured_conf/data/dataclasses.py b/tests/structured_conf/data/dataclasses.py
index 282903b..6851d0c 100644
--- a/tests/structured_conf/data/dataclasses.py
+++ b/tests/structured_conf/data/dataclasses.py
@@ -1,5 +1,5 @@
 from dataclasses import dataclass, field  # noqaE402
-from typing import Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, List, Optional, Tuple, Union
 
 import pytest
 
@@ -394,3 +394,8 @@ class MissingTest:
 @dataclass
 class NestedWithNone:
     plugin: Optional[Plugin] = None
+
+
+@dataclass
+class UnionError:
+    x: Union[int, str] = 10
diff --git a/tests/structured_conf/test_structured_config.py b/tests/structured_conf/test_structured_config.py
index d812232..98a4977 100644
--- a/tests/structured_conf/test_structured_config.py
+++ b/tests/structured_conf/test_structured_config.py
@@ -1,3 +1,4 @@
+import re
 from importlib import import_module
 from typing import Any, Dict
 
@@ -142,9 +143,26 @@ class TestConfigs:
         conf1 = OmegaConf.structured(module.NestedConfig(default_value=module.Nested()))
         validate(conf1)
 
-        with pytest.raises(ValueError):
+    def test_value_without_a_default(self, class_type: str) -> None:
+        module: Any = import_module(class_type)
+        with pytest.raises(
+            ValueError,
+            match=re.escape(
+                "Missing default value for no_default, "
+                "to indicate default must be populated later use OmegaConf.MISSING"
+            ),
+        ):
             OmegaConf.structured(module.NoDefaultErrors)
 
+        OmegaConf.structured(module.NoDefaultErrors(no_default=10)) == {
+            "no_default": 10
+        }
+
+    def test_union_errors(self, class_type: str) -> None:
+        module: Any = import_module(class_type)
+        with pytest.raises(ValueError):
+            OmegaConf.structured(module.UnionError)
+
     def test_config_with_list(self, class_type: str) -> None:
         module: Any = import_module(class_type)
 
diff --git a/tests/test_errors.py b/tests/test_errors.py
index 7b972e5..2fca652 100644
--- a/tests/test_errors.py
+++ b/tests/test_errors.py
@@ -24,7 +24,14 @@ from omegaconf.errors import (
     MissingMandatoryValue,
 )
 
-from . import Color, ConcretePlugin, IllegalType, Plugin, StructuredWithMissing
+from . import (
+    Color,
+    ConcretePlugin,
+    IllegalType,
+    Plugin,
+    StructuredWithMissing,
+    UnionError,
+)
 
 
 @dataclass
@@ -405,6 +412,17 @@ params = [
         ),
         id="structured:create_from_unsupported_object",
     ),
+    pytest.param(
+        Expected(
+            create=lambda: None,
+            op=lambda cfg: OmegaConf.structured(UnionError),
+            exception_type=ValueError,
+            msg="Union types are not supported:\nx: Union[int, str]",
+            object_type_str=None,
+            ref_type_str=None,
+        ),
+        id="structured:create_with_union_error",
+    ),
     # assign
     pytest.param(
         Expected(
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 709ac63..74cb297 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -1,6 +1,6 @@
 from dataclasses import dataclass, field
 from enum import Enum
-from typing import Any, Dict, List
+from typing import Any, Dict, List, Union
 
 import attr
 import pytest
@@ -308,6 +308,7 @@ def test_is_primitive_type(type_: Any, is_primitive: bool) -> None:
         (List[str], "List[str]"),
         (List[Color], "List[Color]"),
         (List[Dict[str, Color]], "List[Dict[str, Color]]"),
+        (Union[str, int, Color], "Union[str, int, Color]"),
     ],
 )
 def test_type_str(type_: Any, expected: str) -> None: