Merge into missing

news/269.bugfix

@@ -0,0 +1 @@
+Merging into a MISSING Structured config node expands the node first to ensure the result is legal

news/271.bugfix

@@ -0,0 +1 @@
+Fix merging into a config with a read only node if merge is not mutating that node

omegaconf/basecontainer.py

@@ -12,8 +12,11 @@
     _convert_to_omegaconf_container,
     _get_value,
     _is_interpolation,
+    _resolve_optional,
+    get_ref_type,
     get_value_kind,
     get_yaml_loader,
+    is_dict_annotation,
     is_primitive_container,
     is_primitive_dict,
     is_structured_config,
@@ -229,8 +232,6 @@ def pretty(self, resolve: bool = False, sort_keys: bool = False) -> str:
     @staticmethod
     def _map_merge(dest: "BaseContainer", src: "BaseContainer") -> None:
         """merge src into dest and return a new copy, does not modified input"""
-        from omegaconf import OmegaConf
-
         from .dictconfig import DictConfig
         from .nodes import ValueNode
 
@@ -239,12 +240,17 @@ def _map_merge(dest: "BaseContainer", src: "BaseContainer") -> None:
         src_type = src._metadata.object_type
 
         dest._validate_set_merge_impl(key=None, value=src, is_assign=False)
-        for key, src_value in src.items_ex(resolve=False):
-            if OmegaConf.is_missing(dest, key):
-                if isinstance(src_value, DictConfig):
-                    if OmegaConf.is_missing(dest, key):
-                        dest[key] = src_value
 
+        if isinstance(dest, DictConfig) and dest._is_missing():
+            type_ = get_ref_type(dest)
+            if type_ is not None:
+                _is_optional, type_ = _resolve_optional(type_)
+                if is_dict_annotation(type_):
+                    dest._set_value({})
+                else:
+                    dest._set_value(type_)
+
+        for key, src_value in src.items_ex(resolve=False):
             dest_node = dest._get_node(key, validate_access=False)
             if dest_node is not None:
                 if dest_node._is_interpolation():

tests/__init__.py

@@ -104,3 +104,13 @@ class MissingDict:
 class DictEnum:
     color_key: Dict[Color, str] = field(default_factory=lambda: {})
     color_val: Dict[str, Color] = field(default_factory=lambda: {})
+
+
+@dataclass
+class A:
+    a: int = 10
+
+
+@dataclass
+class B:
+    x: A = MISSING

tests/test_merge.py

@@ -14,6 +14,7 @@
 from omegaconf._utils import is_structured_config
 
 from . import (
+    B,
     ConcretePlugin,
     ConfWithMissingDict,
     Group,
@@ -28,7 +29,7 @@
 @pytest.mark.parametrize(  # type: ignore
     "inputs, expected",
     [
-        # # dictionaries
+        # dictionaries
         ([{}, {"a": 1}], {"a": 1}),
         ([{"a": None}, {"b": None}], {"a": None, "b": None}),
         ([{"a": 1}, {"b": 2}], {"a": 1, "b": 2}),
@@ -41,10 +42,26 @@
         (({"a": 1}, {"a": nodes.IntegerNode(10)}), {"a": nodes.IntegerNode(10)}),
         (({"a": nodes.IntegerNode(10)}, {"a": 1}), {"a": 1}),
         (({"a": nodes.IntegerNode(10)}, {"a": 1}), {"a": nodes.IntegerNode(1)}),
+        pytest.param(
+            ({"a": "???"}, {"a": {}}), {"a": {}}, id="dict_merge_into_missing"
+        ),
+        pytest.param(
+            ({"a": "???"}, {"a": {"b": 10}}),
+            {"a": {"b": 10}},
+            id="dict_merge_into_missing",
+        ),
         # lists
         (([1, 2, 3], [4, 5, 6]), [4, 5, 6]),
         (([[1, 2, 3]], [[4, 5, 6]]), [[4, 5, 6]]),
         (([1, 2, {"a": 10}], [4, 5, {"b": 20}]), [4, 5, {"b": 20}]),
+        pytest.param(
+            ({"a": "???"}, {"a": []}), {"a": []}, id="list_merge_into_missing"
+        ),
+        pytest.param(
+            ({"a": "???"}, {"a": [1, 2, 3]}),
+            {"a": [1, 2, 3]},
+            id="list_merge_into_missing",
+        ),
         # Interpolations
         # value interpolation
         pytest.param(
@@ -151,7 +168,7 @@
             {"user": Group},
             id="merge_into_missing_node",
         ),
-        # Mising DictConfig
+        # missing DictConfig
         pytest.param(
             [{"dict": DictConfig(content="???")}, {"dict": {"foo": "bar"}}],
             {"dict": {"foo": "bar"}},
@@ -175,6 +192,14 @@
             {"list": ["a", "b", "c"]},
             id="merge_into_missing_List[str]",
         ),
+        # merging compatible dict into MISSING structured config expands it
+        # to ensure the resulting node follows the protocol set by the underlying type
+        pytest.param(
+            [B, {"x": {}}], {"x": {"a": 10}}, id="structured_merge_into_missing",
+        ),
+        pytest.param(
+            [B, {"x": {"a": 20}}], {"x": {"a": 20}}, id="structured_merge_into_missing",
+        ),
     ],
 )
 def test_merge(inputs: Any, expected: Any) -> None: