From 89877d6a2fc366858736b13c29fbc7dbc7dbc822 Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Wed, 14 Aug 2024 14:24:32 -0700
Subject: [PATCH 01/19] fix: better support for merging properties with allOf

---
 .../parser/properties/merge_properties.py     | 167 ++++++++++++++
 .../parser/properties/model_property.py       |  75 +------
 tests/conftest.py                             | 119 ++++------
 .../test_properties/test_merge_properties.py  | 206 ++++++++++++++++++
 .../test_properties/test_model_property.py    |   4 +-
 5 files changed, 435 insertions(+), 136 deletions(-)
 create mode 100644 openapi_python_client/parser/properties/merge_properties.py
 create mode 100644 tests/test_parser/test_properties/test_merge_properties.py

diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
new file mode 100644
index 000000000..a8e6f0e5f
--- /dev/null
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -0,0 +1,167 @@
+
+from collections.abc import Callable
+from typing import Any
+
+from attr import evolve
+
+from openapi_python_client.parser.properties.any import AnyProperty
+from openapi_python_client.parser.properties.enum_property import EnumProperty, ValueType
+from openapi_python_client.parser.properties.float import FloatProperty
+from openapi_python_client.parser.properties.int import IntProperty
+from openapi_python_client.parser.properties.list_property import ListProperty
+from openapi_python_client.parser.properties.protocol import PropertyProtocol
+from openapi_python_client.parser.properties.string import StringProperty
+
+
+def merge_properties(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
+    """Attempt to create a new property that incorporates the behavior of both.
+    
+    This is used when merging schemas with allOf, when two schemas define a property with the same name.
+    
+    OpenAPI defines allOf in terms of validation behavior: the input must pass the validation rules
+    defined in all of the listed schemas. Our task here is slightly more difficult, since we must end
+    up with a single Property object that will be used to generate a single class property in the
+    generated code. Due to limitations of our internal model, this may not be possible for some
+    combinations of property attributes that OpenAPI supports (for instance, we have no way to represent
+    a string property that must match two different regexes).
+
+    Properties can also have attributes that do not represent validation rules, such as "description"
+    and "example". OpenAPI does not define any overriding/aggregation rules for these in allOf. The
+    implementation here is, assuming prop1 and prop2 are in the same order that the schemas were in the
+    allOf, any such attributes that prop2 specifies will override the ones from prop1.
+
+    Any failure is thrown as a ValueError.
+    """
+    if isinstance(prop1, EnumProperty) or isinstance(prop2, EnumProperty):
+        return _merge_with_enum(prop1, prop2)
+
+    if prop1.__class__ == prop2.__class__:
+        return _merge_same_type(prop1, prop2)
+    
+    if isinstance(prop1, AnyProperty) or isinstance(prop2, AnyProperty):
+        return _merge_with_any(prop1, prop2)
+    
+    if _is_numeric(prop1) and _is_numeric(prop2):
+        return _merge_numeric(prop1, prop2)
+
+    raise ValueError("defined with two incompatible types")
+
+
+def _is_numeric(prop: PropertyProtocol) -> bool:
+    return isinstance(prop, IntProperty) or isinstance(prop, FloatProperty)
+
+
+def _merge_same_type(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
+    if prop1 == prop2:
+        # It's always OK to redefine a property with everything exactly the same
+        return prop1
+
+    if isinstance(prop1, StringProperty):
+        return _merge_string(prop1, prop2)
+
+    if isinstance(prop1, ListProperty):
+        # There's no clear way to represent the intersection of two different list types. Fail in this case.
+        if prop1.inner_property != prop2.inner_property:
+            raise ValueError("can't redefine a list property with a different element type")
+
+    # For all other property types, there aren't any special attributes that affect validation, so just
+    # apply the rules for common attributes like "description".
+    return _merge_common_attributes(prop1, prop2)
+
+
+def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty:
+    # If max_length was specified for both, the smallest value takes precedence. If only one of them
+    # specifies it, _combine_values will pick that one.
+    max_length: int | None = _combine_values(prop1.max_length, prop2.max_length, lambda a, b: min([a, b]))
+
+    # If pattern was specified for both, we have a problem. OpenAPI has no logical objection to this;
+    # it would just mean the value must match both of the patterns to be valid. But we have no way to
+    # represent this in our internal model currently.
+    pattern: str | None | ValueError = _combine_values(
+        prop1.pattern,
+        prop2.pattern,
+        lambda a, b: ValueError("specified two different regex patterns")
+    )
+    if isinstance(pattern, ValueError):
+        raise pattern
+    
+    return _merge_common_attributes(evolve(prop1, max_length=max_length, pattern=pattern), prop2)
+
+
+def _merge_numeric(prop1: PropertyProtocol, prop2: PropertyProtocol) -> IntProperty:
+    # Here, one of the properties was defined as "int" and the other was just a general number (which
+    # we call FloatProperty). "Must be integer" is the stricter validation rule, so the result must be
+    # an IntProperty.
+    int_prop = prop1 if isinstance(prop1, IntProperty) else prop2
+    result = _merge_common_attributes(int_prop, prop1, prop2)
+    if result.default is not None:
+        if isinstance(result.default, float) and not result.default.is_integer():
+            raise ValueError(f"default value {result.default} is not valid for an integer property")
+    return result
+    
+
+def _merge_with_any(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
+    # AnyProperty implies no validation rules for a value, so merging it with any other type just means
+    # we should use the validation rules for the other type and the result should not be an AnyProperty.
+    non_any_prop = prop2 if isinstance(prop1, AnyProperty) else prop1
+    return _merge_common_attributes(non_any_prop, prop1, prop2)
+
+
+def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumProperty:
+    if isinstance(prop1, EnumProperty) and isinstance(prop2, EnumProperty):
+        # We want the narrowest validation rules that fit both, so use whichever values list is a
+        # subset of the other.
+        values: dict[str, ValueType]
+        if _values_are_subset(prop1, prop2):
+            values = prop1.values
+        elif _values_are_subset(prop2, prop1):
+            values = prop2.values
+        else:
+            raise ValueError("can't redefine an enum property with incompatible lists of values")
+        return _merge_common_attributes(evolve(prop1, values=values), prop2)
+
+    # If enum values were specified for just one of the properties, use those.
+    enum_prop = prop1 if isinstance(prop1, EnumProperty) else prop2
+    non_enum_prop = prop2 if isinstance(prop1, EnumProperty) else prop1
+    if (
+        (isinstance(non_enum_prop, IntProperty) and enum_prop.value_type is int) or
+        (isinstance(non_enum_prop, StringProperty) and enum_prop.value_type is str)
+    ):
+        return _merge_common_attributes(enum_prop, prop1, prop2)
+    raise ValueError("defined with two incompatible types")
+
+
+def _merge_common_attributes(base: PropertyProtocol, *extend_with: PropertyProtocol) -> PropertyProtocol:
+    """Create a new instance based on base, overriding basic attributes with values from extend_with, in order.
+    
+    For "default", "description", and "example", a non-None value overrides any value from a previously
+    specified property. The behavior is similar to using the spread operator with dictionaries, except
+    that None means "not specified".
+
+    For "required", any True value overrides all other values (a property that was previously required
+    cannot become optional).
+    """
+    current = base
+    for override in extend_with:
+        current = evolve(
+            current,
+            required=current.required or override.required,
+            default = override.default or current.default,
+            description = override.description or current.description,
+            example = override.example or current.example,
+    )
+    return current
+
+
+def _values_are_subset(prop1: EnumProperty, prop2: EnumProperty) -> bool:
+    return set(prop1.values.items()) <= set(prop2.values.items())
+
+
+def _combine_values(value1: Any, value2: Any, combinator: Callable) -> Any:
+    if value1 == value2:
+        return value1
+    if value1 is None:
+        return value2
+    if value2 is None:
+        return value1
+    return combinator(value1, value2)
diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py
index e0c06641f..f380538da 100644
--- a/openapi_python_client/parser/properties/model_property.py
+++ b/openapi_python_client/parser/properties/model_property.py
@@ -5,12 +5,13 @@
 
 from attrs import define, evolve
 
+from openapi_python_client.parser.properties.merge_properties import merge_properties
+
 from ... import Config, utils
 from ... import schema as oai
 from ...utils import PythonIdentifier
 from ..errors import ParseError, PropertyError
 from .any import AnyProperty
-from .enum_property import EnumProperty
 from .protocol import PropertyProtocol, Value
 from .schemas import Class, ReferencePath, Schemas, parse_reference_path
 
@@ -216,59 +217,6 @@ def get_type_string(
 from .property import Property  # noqa: E402
 
 
-def _values_are_subset(first: EnumProperty, second: EnumProperty) -> bool:
-    return set(first.values.items()) <= set(second.values.items())
-
-
-def _types_are_subset(first: EnumProperty, second: Property) -> bool:
-    from . import IntProperty, StringProperty
-
-    if first.value_type is int and isinstance(second, IntProperty):
-        return True
-    if first.value_type is str and isinstance(second, StringProperty):
-        return True
-    return False
-
-
-def _enum_subset(first: Property, second: Property) -> EnumProperty | None:
-    """Return the EnumProperty that is the subset of the other, if possible."""
-
-    if isinstance(first, EnumProperty):
-        if isinstance(second, EnumProperty):
-            if _values_are_subset(first, second):
-                return first
-            if _values_are_subset(second, first):
-                return second
-            return None
-        return first if _types_are_subset(first, second) else None
-
-    if isinstance(second, EnumProperty) and _types_are_subset(second, first):
-        return second
-    return None
-
-
-def _merge_properties(first: Property, second: Property) -> Property | PropertyError:
-    required = first.required or second.required
-
-    err = None
-
-    if first.__class__ == second.__class__:
-        first = evolve(first, required=required)
-        second = evolve(second, required=required)
-        if first == second:
-            return first
-        err = PropertyError(header="Cannot merge properties", detail="Properties has conflicting values")
-
-    enum_subset = _enum_subset(first, second)
-    if enum_subset is not None:
-        return evolve(enum_subset, required=required)
-
-    return err or PropertyError(
-        header="Cannot merge properties",
-        detail=f"{first.__class__}, {second.__class__}Properties have incompatible types",
-    )
-
-
 def _resolve_naming_conflict(first: Property, second: Property, config: Config) -> PropertyError | None:
     first.set_python_name(first.name, config=config, skip_snake_case=True)
     second.set_python_name(second.name, config=config, skip_snake_case=True)
@@ -307,23 +255,24 @@ def _add_if_no_conflict(new_prop: Property) -> PropertyError | None:
         nonlocal properties
 
         name_conflict = properties.get(new_prop.name)
-        merged_prop_or_error = _merge_properties(name_conflict, new_prop) if name_conflict else new_prop
-        if isinstance(merged_prop_or_error, PropertyError):
-            merged_prop_or_error.header = (
-                f"Found conflicting properties named {new_prop.name} when creating {class_name}"
+        try:
+            merged_prop = merge_properties(name_conflict, new_prop) if name_conflict else new_prop
+        except ValueError as e:
+            return PropertyError(
+                header=f"Found conflicting properties named {new_prop.name} when creating {class_name}",
+                detail=str(e),
             )
-            return merged_prop_or_error
 
         for other_prop in properties.values():
-            if other_prop.name == merged_prop_or_error.name:
+            if other_prop.name == merged_prop.name:
                 continue  # Same property, probably just got merged
-            if other_prop.python_name != merged_prop_or_error.python_name:
+            if other_prop.python_name != merged_prop.python_name:
                 continue
-            naming_error = _resolve_naming_conflict(merged_prop_or_error, other_prop, config)
+            naming_error = _resolve_naming_conflict(merged_prop, other_prop, config)
             if naming_error is not None:
                 return naming_error
 
-        properties[merged_prop_or_error.name] = merged_prop_or_error
+        properties[merged_prop.name] = merged_prop
         return None
 
     unprocessed_props = data.properties or {}
diff --git a/tests/conftest.py b/tests/conftest.py
index 91b67752c..0bc8a0279 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,5 +1,5 @@
 from pathlib import Path
-from typing import Any, Callable, Dict
+from typing import Any, Callable, Dict, Union
 
 import pytest
 
@@ -20,6 +20,7 @@
     StringProperty,
     UnionProperty,
 )
+from openapi_python_client.parser.properties.float import FloatProperty
 from openapi_python_client.schema.openapi_schema_pydantic import Parameter
 from openapi_python_client.schema.parameter_location import ParameterLocation
 
@@ -67,6 +68,19 @@ def _factory(**kwargs):
     return _factory
 
 
+def _simple_factory(cls: type, default_kwargs: Union[dict, Callable[[dict], dict], None]=None):
+    def _factory(**kwargs):
+        kwargs = _common_kwargs(kwargs)
+        defaults = default_kwargs
+        if defaults:
+            if callable(defaults):
+                defaults = defaults(kwargs)
+            kwargs = {**defaults, **kwargs}
+        return cls(**kwargs)
+
+    return _factory
+
+
 @pytest.fixture
 def enum_property_factory() -> Callable[..., EnumProperty]:
     """
@@ -76,17 +90,11 @@ def enum_property_factory() -> Callable[..., EnumProperty]:
     """
     from openapi_python_client.parser.properties import Class
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        kwargs = {
-            "class_info": Class(name=kwargs["name"], module_name=kwargs["name"]),
-            "values": {},
-            "value_type": str,
-            **kwargs,
-        }
-        return EnumProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(EnumProperty, lambda kwargs: {
+        "class_info": Class(name=kwargs["name"], module_name=kwargs["name"]),
+        "values": {},
+        "value_type": str,
+    })
 
 
 @pytest.fixture
@@ -97,11 +105,7 @@ def any_property_factory() -> Callable[..., AnyProperty]:
     You can pass the same params into this as the AnyProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return AnyProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(AnyProperty)
 
 
 @pytest.fixture
@@ -112,56 +116,51 @@ def string_property_factory() -> Callable[..., StringProperty]:
     You can pass the same params into this as the StringProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return StringProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(StringProperty)
 
 
 @pytest.fixture
 def int_property_factory() -> Callable[..., IntProperty]:
     """
-    This fixture surfaces in the test as a function which manufactures StringProperties with defaults.
+    This fixture surfaces in the test as a function which manufactures IntProperties with defaults.
 
-    You can pass the same params into this as the StringProperty constructor to override defaults.
+    You can pass the same params into this as the IntProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return IntProperty(**kwargs)
+    return _simple_factory(IntProperty)
 
-    return _factory
+
+@pytest.fixture
+def float_property_factory() -> Callable[..., FloatProperty]:
+    """
+    This fixture surfaces in the test as a function which manufactures FloatProperties with defaults.
+
+    You can pass the same params into this as the FloatProperty constructor to override defaults.
+    """
+
+    return _simple_factory(FloatProperty)
 
 
 @pytest.fixture
 def none_property_factory() -> Callable[..., NoneProperty]:
     """
-    This fixture surfaces in the test as a function which manufactures StringProperties with defaults.
+    This fixture surfaces in the test as a function which manufactures NoneProperties with defaults.
 
-    You can pass the same params into this as the StringProperty constructor to override defaults.
+    You can pass the same params into this as the NoneProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return NoneProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(NoneProperty)
 
 
 @pytest.fixture
 def boolean_property_factory() -> Callable[..., BooleanProperty]:
     """
-    This fixture surfaces in the test as a function which manufactures StringProperties with defaults.
+    This fixture surfaces in the test as a function which manufactures BooleanProperties with defaults.
 
-    You can pass the same params into this as the StringProperty constructor to override defaults.
+    You can pass the same params into this as the BooleanProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return BooleanProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(BooleanProperty)
 
 
 @pytest.fixture
@@ -172,11 +171,7 @@ def date_time_property_factory() -> Callable[..., DateTimeProperty]:
     You can pass the same params into this as the DateTimeProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return DateTimeProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(DateTimeProperty)
 
 
 @pytest.fixture
@@ -187,11 +182,7 @@ def date_property_factory() -> Callable[..., DateProperty]:
     You can pass the same params into this as the DateProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return DateProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(DateProperty)
 
 
 @pytest.fixture
@@ -202,11 +193,7 @@ def file_property_factory() -> Callable[..., FileProperty]:
     You can pass the same params into this as the FileProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        return FileProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(FileProperty)
 
 
 @pytest.fixture
@@ -217,13 +204,7 @@ def list_property_factory(string_property_factory) -> Callable[..., ListProperty
     You can pass the same params into this as the ListProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        if "inner_property" not in kwargs:
-            kwargs["inner_property"] = string_property_factory()
-        return ListProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(ListProperty, {"inner_property": string_property_factory()})
 
 
 @pytest.fixture
@@ -234,13 +215,9 @@ def union_property_factory(date_time_property_factory, string_property_factory)
     You can pass the same params into this as the UnionProperty constructor to override defaults.
     """
 
-    def _factory(**kwargs):
-        kwargs = _common_kwargs(kwargs)
-        if "inner_properties" not in kwargs:
-            kwargs["inner_properties"] = [date_time_property_factory(), string_property_factory()]
-        return UnionProperty(**kwargs)
-
-    return _factory
+    return _simple_factory(UnionProperty, {
+        "inner_properties": [date_time_property_factory(), string_property_factory()]
+    })
 
 
 @pytest.fixture
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
new file mode 100644
index 000000000..5ac9e27e2
--- /dev/null
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -0,0 +1,206 @@
+
+import pytest
+from attr import evolve
+
+from openapi_python_client.parser.properties import StringProperty
+from openapi_python_client.parser.properties.float import FloatProperty
+from openapi_python_client.parser.properties.int import IntProperty
+from openapi_python_client.parser.properties.merge_properties import merge_properties
+
+MODULE_NAME = "openapi_python_client.parser.properties.merge_properties"
+
+
+def test_merge_basic_attributes_same_type(
+    boolean_property_factory,
+    int_property_factory,
+    float_property_factory,
+    string_property_factory,
+    list_property_factory,
+    model_property_factory,
+):
+    basic_props = [
+        boolean_property_factory(default=True),
+        int_property_factory(default=1),
+        float_property_factory(default=1.5),
+        string_property_factory(default="x"),
+        list_property_factory(),
+        model_property_factory(),
+    ]
+    for basic_prop in basic_props:
+        with_required = evolve(basic_prop, required=True)
+        assert merge_properties(basic_prop, with_required) == with_required
+        assert merge_properties(with_required, basic_prop) == with_required
+        without_default = evolve(basic_prop, default=None)
+        assert merge_properties(basic_prop, without_default) == basic_prop
+        assert merge_properties(without_default, basic_prop) == basic_prop
+        with_desc1 = evolve(basic_prop, description="desc1")
+        with_desc2 = evolve(basic_prop, description="desc2")
+        assert merge_properties(basic_prop, with_desc1) == with_desc1
+        assert merge_properties(with_desc1, basic_prop) == with_desc1
+        assert merge_properties(with_desc1, with_desc2) == with_desc2
+
+
+def test_incompatible_types(
+    boolean_property_factory,
+    int_property_factory,
+    float_property_factory,
+    string_property_factory,
+    list_property_factory,
+    model_property_factory,
+):
+    props = [
+        boolean_property_factory(default=True),
+        int_property_factory(default=1),
+        float_property_factory(default=1.5),
+        string_property_factory(default="x"),
+        list_property_factory(),
+        model_property_factory(),
+    ]
+    for i, prop1 in enumerate(props):
+        for j, prop2 in enumerate(props):
+            if i != j:
+                if {prop1.__class__, prop2.__class__} == {IntProperty, FloatProperty}:
+                    continue  # the int+float case is covered in another test
+                with pytest.raises(ValueError) as excinfo:
+                    merge_properties(prop1, prop2)
+                assert "incompatible types" in excinfo.value.args[0]
+
+
+def test_merge_int_with_float(int_property_factory, float_property_factory):
+    int_prop = int_property_factory(description="desc1")
+    float_prop = float_property_factory(default=2, description="desc2")
+
+    assert merge_properties(int_prop, float_prop) == (
+        evolve(int_prop, default=float_prop.default, description=float_prop.description)
+    )
+    assert merge_properties(float_prop, int_prop) == (
+        evolve(int_prop, default=float_prop.default)
+    )
+
+    float_prop_with_non_int_default = evolve(float_prop, default=2.5)
+    with pytest.raises(ValueError) as excinfo:
+        merge_properties(int_prop, float_prop_with_non_int_default)
+    assert "default value" in excinfo.value.args[0]
+
+
+def test_merge_with_any(
+    any_property_factory,
+    boolean_property_factory,
+    int_property_factory,
+    float_property_factory,
+    string_property_factory,
+    model_property_factory,
+):
+    original_desc = "description"
+    props = [
+        boolean_property_factory(default=True, description=original_desc),
+        int_property_factory(default=1, description=original_desc),
+        float_property_factory(default=1.5, description=original_desc),
+        string_property_factory(default="x", description=original_desc),
+        model_property_factory(description=original_desc),
+    ]
+    any_prop = any_property_factory()
+    for prop in props:
+        expected_result = evolve(prop, description=original_desc, default=prop.default)
+        assert merge_properties(any_prop, prop) == expected_result
+        assert merge_properties(prop, any_prop) == expected_result
+
+
+def test_merge_string_max_length(string_property_factory):
+    prop_with_no_max = string_property_factory()
+    prop_with_max_1 = evolve(prop_with_no_max, max_length=1)
+    prop_with_max_5 = evolve(prop_with_no_max, max_length=5)
+
+    assert merge_properties(prop_with_no_max, prop_with_max_1) == prop_with_max_1
+    assert merge_properties(prop_with_max_1, prop_with_no_max) == prop_with_max_1
+
+    assert merge_properties(prop_with_max_1, prop_with_max_5) == prop_with_max_1
+    assert merge_properties(prop_with_max_5, prop_with_max_1) == prop_with_max_1
+
+
+def test_merge_string_pattern(string_property_factory):
+    prop_with_no_pattern = string_property_factory()
+    prop_with_pattern1a = evolve(prop_with_no_pattern, pattern="pattern1")
+    prop_with_pattern1b = evolve(prop_with_no_pattern, pattern="pattern1")  # deliberately identical pattern
+    prop_with_pattern2 = evolve(prop_with_no_pattern, pattern="pattern2")
+
+    assert merge_properties(prop_with_no_pattern, prop_with_pattern1a) == prop_with_pattern1a
+    assert merge_properties(prop_with_pattern1a, prop_with_no_pattern) == prop_with_pattern1a
+
+    assert merge_properties(prop_with_pattern1a, prop_with_pattern1b) == prop_with_pattern1a
+
+    with pytest.raises(ValueError) as excinfo:
+        merge_properties(prop_with_pattern1a, prop_with_pattern2)
+    assert "regex patterns" in excinfo.value.args[0]
+
+
+def test_merge_string_with_string_enum(string_property_factory, enum_property_factory):
+    values = {"A": "A", "B": "B"}
+    string_prop = string_property_factory(default="default1", description="desc1", example="example1")
+    enum_prop = enum_property_factory(
+        default="default2",
+        description="desc2",
+        example="example2",
+        values=values,
+        value_type=str,
+    )
+
+    assert merge_properties(string_prop, enum_prop) == evolve(enum_prop, required=True)
+    assert merge_properties(enum_prop, string_prop) == evolve(
+        enum_prop,
+        required=True,
+        default=string_prop.default,
+        description=string_prop.description,
+        example=string_prop.example
+    )
+
+
+def test_merge_int_with_int_enum(int_property_factory, enum_property_factory):
+    values = {"VALUE_1": 1, "VALUE_2": 2}
+    int_prop = int_property_factory(default=100, description="desc1", example="example1")
+    enum_prop = enum_property_factory(
+        default=200,
+        description="desc2",
+        example="example2",
+        values=values,
+        value_type=int,
+    )
+
+    assert merge_properties(int_prop, enum_prop) == evolve(enum_prop, required=True)
+    assert merge_properties(enum_prop, int_prop) == evolve(
+        enum_prop,
+        required=True,
+        default=int_prop.default,
+        description=int_prop.description,
+        example=int_prop.example
+    )
+
+
+def test_merge_with_incompatible_enum(
+    boolean_property_factory,
+    int_property_factory,
+    float_property_factory,
+    string_property_factory,
+    enum_property_factory,
+    model_property_factory,
+):
+    props = [
+        boolean_property_factory(),
+        int_property_factory(),
+        float_property_factory(),
+        string_property_factory(),
+        model_property_factory(),
+    ]
+    string_enum_prop = enum_property_factory(value_type=str)
+    int_enum_prop = enum_property_factory(value_type=int)
+    for prop in props:
+        if not isinstance(prop, StringProperty):
+            with pytest.raises(ValueError):
+                merge_properties(prop, string_enum_prop)
+            with pytest.raises(ValueError):
+                merge_properties(string_enum_prop, prop)
+        if not isinstance(prop, IntProperty):
+            with pytest.raises(ValueError):
+                merge_properties(prop, int_enum_prop)
+            with pytest.raises(ValueError):
+                merge_properties(int_enum_prop, prop)
diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py
index a7d7c1f23..242e39f29 100644
--- a/tests/test_parser/test_properties/test_model_property.py
+++ b/tests/test_parser/test_properties/test_model_property.py
@@ -410,10 +410,10 @@ def test_conflicting_properties_same_types(self, model_property_factory, string_
         schemas = Schemas(
             classes_by_reference={
                 "/First": model_property_factory(
-                    required_properties=[], optional_properties=[string_property_factory(default="abc")]
+                    required_properties=[], optional_properties=[string_property_factory(pattern="abc")]
                 ),
                 "/Second": model_property_factory(
-                    required_properties=[], optional_properties=[string_property_factory()]
+                    required_properties=[], optional_properties=[string_property_factory(pattern="def")]
                 ),
             }
         )

From d9becd0acf065763dec96f57bce8931b04911e0b Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Wed, 14 Aug 2024 14:56:58 -0700
Subject: [PATCH 02/19] fix "ommitted" typo

---
 openapi_python_client/parser/openapi.py | 4 ++--
 tests/test_parser/test_openapi.py       | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py
index 0d6a7424b..9d62e1df5 100644
--- a/openapi_python_client/parser/openapi.py
+++ b/openapi_python_client/parser/openapi.py
@@ -155,7 +155,7 @@ def _add_responses(
                     ParseError(
                         detail=(
                             f"Invalid response status code {code} (not a valid HTTP "
-                            f"status code), response will be ommitted from generated "
+                            f"status code), response will be omitted from generated "
                             f"client"
                         )
                     )
@@ -175,7 +175,7 @@ def _add_responses(
                     ParseError(
                         detail=(
                             f"Cannot parse response for status code {status_code}{detail_suffix}, "
-                            f"response will be ommitted from generated client"
+                            f"response will be omitted from generated client"
                         ),
                         data=response.data,
                     )
diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py
index 75eea1b47..c43c12da0 100644
--- a/tests/test_parser/test_openapi.py
+++ b/tests/test_parser/test_openapi.py
@@ -90,7 +90,7 @@ def test__add_responses_status_code_error(self, response_status_code, mocker):
         assert response.errors == [
             ParseError(
                 detail=f"Invalid response status code {response_status_code} (not a valid HTTP status code), "
-                "response will be ommitted from generated client"
+                "response will be omitted from generated client"
             )
         ]
         response_from_data.assert_not_called()
@@ -121,12 +121,12 @@ def test__add_responses_error(self, mocker):
         assert response.errors == [
             ParseError(
                 detail="Cannot parse response for status code 200 (some problem), "
-                "response will be ommitted from generated client",
+                "response will be omitted from generated client",
                 data=parse_error.data,
             ),
             ParseError(
                 detail="Cannot parse response for status code 404 (some problem), "
-                "response will be ommitted from generated client",
+                "response will be omitted from generated client",
                 data=parse_error.data,
             ),
         ]

From 8c4ef9728d309c8c2c2f3419baf4733a89fc5c61 Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Wed, 21 Aug 2024 15:01:05 -0700
Subject: [PATCH 03/19] lint

---
 .../parser/properties/merge_properties.py     | 32 ++++++++-----------
 tests/conftest.py                             | 21 ++++++------
 .../test_properties/test_merge_properties.py  | 13 ++------
 3 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index a8e6f0e5f..05d24cfb4 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -1,4 +1,3 @@
-
 from collections.abc import Callable
 from typing import Any
 
@@ -15,9 +14,9 @@
 
 def merge_properties(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
     """Attempt to create a new property that incorporates the behavior of both.
-    
+
     This is used when merging schemas with allOf, when two schemas define a property with the same name.
-    
+
     OpenAPI defines allOf in terms of validation behavior: the input must pass the validation rules
     defined in all of the listed schemas. Our task here is slightly more difficult, since we must end
     up with a single Property object that will be used to generate a single class property in the
@@ -37,10 +36,10 @@ def merge_properties(prop1: PropertyProtocol, prop2: PropertyProtocol) -> Proper
 
     if prop1.__class__ == prop2.__class__:
         return _merge_same_type(prop1, prop2)
-    
+
     if isinstance(prop1, AnyProperty) or isinstance(prop2, AnyProperty):
         return _merge_with_any(prop1, prop2)
-    
+
     if _is_numeric(prop1) and _is_numeric(prop2):
         return _merge_numeric(prop1, prop2)
 
@@ -78,13 +77,11 @@ def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringPropert
     # it would just mean the value must match both of the patterns to be valid. But we have no way to
     # represent this in our internal model currently.
     pattern: str | None | ValueError = _combine_values(
-        prop1.pattern,
-        prop2.pattern,
-        lambda a, b: ValueError("specified two different regex patterns")
+        prop1.pattern, prop2.pattern, lambda a, b: ValueError("specified two different regex patterns")
     )
     if isinstance(pattern, ValueError):
         raise pattern
-    
+
     return _merge_common_attributes(evolve(prop1, max_length=max_length, pattern=pattern), prop2)
 
 
@@ -98,7 +95,7 @@ def _merge_numeric(prop1: PropertyProtocol, prop2: PropertyProtocol) -> IntPrope
         if isinstance(result.default, float) and not result.default.is_integer():
             raise ValueError(f"default value {result.default} is not valid for an integer property")
     return result
-    
+
 
 def _merge_with_any(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
     # AnyProperty implies no validation rules for a value, so merging it with any other type just means
@@ -123,9 +120,8 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
     # If enum values were specified for just one of the properties, use those.
     enum_prop = prop1 if isinstance(prop1, EnumProperty) else prop2
     non_enum_prop = prop2 if isinstance(prop1, EnumProperty) else prop1
-    if (
-        (isinstance(non_enum_prop, IntProperty) and enum_prop.value_type is int) or
-        (isinstance(non_enum_prop, StringProperty) and enum_prop.value_type is str)
+    if (isinstance(non_enum_prop, IntProperty) and enum_prop.value_type is int) or (
+        isinstance(non_enum_prop, StringProperty) and enum_prop.value_type is str
     ):
         return _merge_common_attributes(enum_prop, prop1, prop2)
     raise ValueError("defined with two incompatible types")
@@ -133,7 +129,7 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
 
 def _merge_common_attributes(base: PropertyProtocol, *extend_with: PropertyProtocol) -> PropertyProtocol:
     """Create a new instance based on base, overriding basic attributes with values from extend_with, in order.
-    
+
     For "default", "description", and "example", a non-None value overrides any value from a previously
     specified property. The behavior is similar to using the spread operator with dictionaries, except
     that None means "not specified".
@@ -146,10 +142,10 @@ def _merge_common_attributes(base: PropertyProtocol, *extend_with: PropertyProto
         current = evolve(
             current,
             required=current.required or override.required,
-            default = override.default or current.default,
-            description = override.description or current.description,
-            example = override.example or current.example,
-    )
+            default=override.default or current.default,
+            description=override.description or current.description,
+            example=override.example or current.example,
+        )
     return current
 
 
diff --git a/tests/conftest.py b/tests/conftest.py
index 0bc8a0279..9b5e1490f 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -68,7 +68,7 @@ def _factory(**kwargs):
     return _factory
 
 
-def _simple_factory(cls: type, default_kwargs: Union[dict, Callable[[dict], dict], None]=None):
+def _simple_factory(cls: type, default_kwargs: Union[dict, Callable[[dict], dict], None] = None):
     def _factory(**kwargs):
         kwargs = _common_kwargs(kwargs)
         defaults = default_kwargs
@@ -90,11 +90,14 @@ def enum_property_factory() -> Callable[..., EnumProperty]:
     """
     from openapi_python_client.parser.properties import Class
 
-    return _simple_factory(EnumProperty, lambda kwargs: {
-        "class_info": Class(name=kwargs["name"], module_name=kwargs["name"]),
-        "values": {},
-        "value_type": str,
-    })
+    return _simple_factory(
+        EnumProperty,
+        lambda kwargs: {
+            "class_info": Class(name=kwargs["name"], module_name=kwargs["name"]),
+            "values": {},
+            "value_type": str,
+        },
+    )
 
 
 @pytest.fixture
@@ -215,9 +218,9 @@ def union_property_factory(date_time_property_factory, string_property_factory)
     You can pass the same params into this as the UnionProperty constructor to override defaults.
     """
 
-    return _simple_factory(UnionProperty, {
-        "inner_properties": [date_time_property_factory(), string_property_factory()]
-    })
+    return _simple_factory(
+        UnionProperty, {"inner_properties": [date_time_property_factory(), string_property_factory()]}
+    )
 
 
 @pytest.fixture
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index 5ac9e27e2..8ca054e82 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -1,4 +1,3 @@
-
 import pytest
 from attr import evolve
 
@@ -73,9 +72,7 @@ def test_merge_int_with_float(int_property_factory, float_property_factory):
     assert merge_properties(int_prop, float_prop) == (
         evolve(int_prop, default=float_prop.default, description=float_prop.description)
     )
-    assert merge_properties(float_prop, int_prop) == (
-        evolve(int_prop, default=float_prop.default)
-    )
+    assert merge_properties(float_prop, int_prop) == (evolve(int_prop, default=float_prop.default))
 
     float_prop_with_non_int_default = evolve(float_prop, default=2.5)
     with pytest.raises(ValueError) as excinfo:
@@ -151,7 +148,7 @@ def test_merge_string_with_string_enum(string_property_factory, enum_property_fa
         required=True,
         default=string_prop.default,
         description=string_prop.description,
-        example=string_prop.example
+        example=string_prop.example,
     )
 
 
@@ -168,11 +165,7 @@ def test_merge_int_with_int_enum(int_property_factory, enum_property_factory):
 
     assert merge_properties(int_prop, enum_prop) == evolve(enum_prop, required=True)
     assert merge_properties(enum_prop, int_prop) == evolve(
-        enum_prop,
-        required=True,
-        default=int_prop.default,
-        description=int_prop.description,
-        example=int_prop.example
+        enum_prop, required=True, default=int_prop.default, description=int_prop.description, example=int_prop.example
     )
 
 

From b6d9d0ac9cbe6cae832b308345205e5d29563768 Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Wed, 21 Aug 2024 15:29:57 -0700
Subject: [PATCH 04/19] fix type hinting problems

---
 .../parser/properties/merge_properties.py     | 26 +++++++++++--------
 .../parser/properties/model_property.py       |  4 +--
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index 05d24cfb4..9edcbdb3f 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -1,5 +1,5 @@
 from collections.abc import Callable
-from typing import Any
+from typing import Any, Dict, TypeVar, Union, cast
 
 from attr import evolve
 
@@ -11,6 +11,10 @@
 from openapi_python_client.parser.properties.protocol import PropertyProtocol
 from openapi_python_client.parser.properties.string import StringProperty
 
+# Note that in this file we have to use PropertyProtocol instead of the union type Property,
+# to avoid circular imports.
+PropertyT = TypeVar("PropertyT", bound=PropertyProtocol)
+
 
 def merge_properties(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
     """Attempt to create a new property that incorporates the behavior of both.
@@ -56,11 +60,11 @@ def _merge_same_type(prop1: PropertyProtocol, prop2: PropertyProtocol) -> Proper
         return prop1
 
     if isinstance(prop1, StringProperty):
-        return _merge_string(prop1, prop2)
+        return _merge_string(prop1, cast(StringProperty, prop2))
 
     if isinstance(prop1, ListProperty):
         # There's no clear way to represent the intersection of two different list types. Fail in this case.
-        if prop1.inner_property != prop2.inner_property:
+        if prop1.inner_property != cast(ListProperty, prop2).inner_property:
             raise ValueError("can't redefine a list property with a different element type")
 
     # For all other property types, there aren't any special attributes that affect validation, so just
@@ -71,12 +75,12 @@ def _merge_same_type(prop1: PropertyProtocol, prop2: PropertyProtocol) -> Proper
 def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty:
     # If max_length was specified for both, the smallest value takes precedence. If only one of them
     # specifies it, _combine_values will pick that one.
-    max_length: int | None = _combine_values(prop1.max_length, prop2.max_length, lambda a, b: min([a, b]))
+    max_length: Union[int, None] = _combine_values(prop1.max_length, prop2.max_length, lambda a, b: min([a, b]))
 
     # If pattern was specified for both, we have a problem. OpenAPI has no logical objection to this;
     # it would just mean the value must match both of the patterns to be valid. But we have no way to
     # represent this in our internal model currently.
-    pattern: str | None | ValueError = _combine_values(
+    pattern: Union[str, None, ValueError] = _combine_values(
         prop1.pattern, prop2.pattern, lambda a, b: ValueError("specified two different regex patterns")
     )
     if isinstance(pattern, ValueError):
@@ -89,7 +93,7 @@ def _merge_numeric(prop1: PropertyProtocol, prop2: PropertyProtocol) -> IntPrope
     # Here, one of the properties was defined as "int" and the other was just a general number (which
     # we call FloatProperty). "Must be integer" is the stricter validation rule, so the result must be
     # an IntProperty.
-    int_prop = prop1 if isinstance(prop1, IntProperty) else prop2
+    int_prop = prop1 if isinstance(prop1, IntProperty) else cast(IntProperty, prop2)
     result = _merge_common_attributes(int_prop, prop1, prop2)
     if result.default is not None:
         if isinstance(result.default, float) and not result.default.is_integer():
@@ -108,7 +112,7 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
     if isinstance(prop1, EnumProperty) and isinstance(prop2, EnumProperty):
         # We want the narrowest validation rules that fit both, so use whichever values list is a
         # subset of the other.
-        values: dict[str, ValueType]
+        values: Dict[str, ValueType]
         if _values_are_subset(prop1, prop2):
             values = prop1.values
         elif _values_are_subset(prop2, prop1):
@@ -118,7 +122,7 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
         return _merge_common_attributes(evolve(prop1, values=values), prop2)
 
     # If enum values were specified for just one of the properties, use those.
-    enum_prop = prop1 if isinstance(prop1, EnumProperty) else prop2
+    enum_prop = prop1 if isinstance(prop1, EnumProperty) else cast(EnumProperty, prop2)
     non_enum_prop = prop2 if isinstance(prop1, EnumProperty) else prop1
     if (isinstance(non_enum_prop, IntProperty) and enum_prop.value_type is int) or (
         isinstance(non_enum_prop, StringProperty) and enum_prop.value_type is str
@@ -127,7 +131,7 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
     raise ValueError("defined with two incompatible types")
 
 
-def _merge_common_attributes(base: PropertyProtocol, *extend_with: PropertyProtocol) -> PropertyProtocol:
+def _merge_common_attributes(base: PropertyT, *extend_with: PropertyProtocol) -> PropertyT:
     """Create a new instance based on base, overriding basic attributes with values from extend_with, in order.
 
     For "default", "description", and "example", a non-None value overrides any value from a previously
@@ -140,7 +144,7 @@ def _merge_common_attributes(base: PropertyProtocol, *extend_with: PropertyProto
     current = base
     for override in extend_with:
         current = evolve(
-            current,
+            current,  # type: ignore # can't prove that every property type is an attrs class, but it is
             required=current.required or override.required,
             default=override.default or current.default,
             description=override.description or current.description,
@@ -153,7 +157,7 @@ def _values_are_subset(prop1: EnumProperty, prop2: EnumProperty) -> bool:
     return set(prop1.values.items()) <= set(prop2.values.items())
 
 
-def _combine_values(value1: Any, value2: Any, combinator: Callable) -> Any:
+def _combine_values(value1: Any, value2: Any, combinator: Callable[[Any, Any], Any]) -> Any:
     if value1 == value2:
         return value1
     if value1 is None:
diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py
index f380538da..3fe9836f4 100644
--- a/openapi_python_client/parser/properties/model_property.py
+++ b/openapi_python_client/parser/properties/model_property.py
@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from itertools import chain
-from typing import Any, ClassVar, NamedTuple
+from typing import Any, ClassVar, NamedTuple, cast
 
 from attrs import define, evolve
 
@@ -256,7 +256,7 @@ def _add_if_no_conflict(new_prop: Property) -> PropertyError | None:
 
         name_conflict = properties.get(new_prop.name)
         try:
-            merged_prop = merge_properties(name_conflict, new_prop) if name_conflict else new_prop
+            merged_prop = cast(Property, merge_properties(name_conflict, new_prop)) if name_conflict else new_prop
         except ValueError as e:
             return PropertyError(
                 header=f"Found conflicting properties named {new_prop.name} when creating {class_name}",

From 61e015b8f9b3242c79e237231af0a58280d6b110 Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Wed, 21 Aug 2024 15:32:27 -0700
Subject: [PATCH 05/19] another type hinting fix

---
 openapi_python_client/parser/properties/merge_properties.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index 9edcbdb3f..e740669e8 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -1,5 +1,4 @@
-from collections.abc import Callable
-from typing import Any, Dict, TypeVar, Union, cast
+from typing import Any, Callable, Dict, TypeVar, Union, cast
 
 from attr import evolve
 

From 8722001eff0224f5ba4d2ef938d2e275df2d6a2f Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Wed, 21 Aug 2024 15:40:34 -0700
Subject: [PATCH 06/19] add change description

---
 .changeset/improved_property_merging.md | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 .changeset/improved_property_merging.md

diff --git a/.changeset/improved_property_merging.md b/.changeset/improved_property_merging.md
new file mode 100644
index 000000000..e295ca692
--- /dev/null
+++ b/.changeset/improved_property_merging.md
@@ -0,0 +1,12 @@
+---
+default: minor
+---
+
+# Improved property-merging behavior with `allOf`
+
+When using `allOf` to extend a base object type, `openapi-python-client` is now able to handle some kinds of modifications to an existing property that would have previously caused an error:
+
+- Overriding attributes that do not affect validation, such as `description`.
+- Specifying stricter validation for the same type, such as setting a `maxLength` for a string that previously had no max length (or previously had a larger max length), or setting a `pattern` regex for a string that previously had none.
+- Changing a nonspecific numeric type to `int`.
+- Changing a property from `any` to a specific type.

From ad7730244ef972564a72e81a35a74ba1e9aacbba Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Thu, 22 Aug 2024 15:46:47 -0700
Subject: [PATCH 07/19] fix enum property merge logic

---
 .../parser/properties/merge_properties.py     |  6 +++-
 .../test_properties/test_merge_properties.py  | 29 +++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index e740669e8..d1a609630 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -8,6 +8,7 @@
 from openapi_python_client.parser.properties.int import IntProperty
 from openapi_python_client.parser.properties.list_property import ListProperty
 from openapi_python_client.parser.properties.protocol import PropertyProtocol
+from openapi_python_client.parser.properties.schemas import Class
 from openapi_python_client.parser.properties.string import StringProperty
 
 # Note that in this file we have to use PropertyProtocol instead of the union type Property,
@@ -112,13 +113,16 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
         # We want the narrowest validation rules that fit both, so use whichever values list is a
         # subset of the other.
         values: Dict[str, ValueType]
+        class_info: Class
         if _values_are_subset(prop1, prop2):
             values = prop1.values
+            class_info = prop1.class_info
         elif _values_are_subset(prop2, prop1):
             values = prop2.values
+            class_info = prop2.class_info
         else:
             raise ValueError("can't redefine an enum property with incompatible lists of values")
-        return _merge_common_attributes(evolve(prop1, values=values), prop2)
+        return _merge_common_attributes(evolve(prop1, values=values, class_info=class_info), prop2)
 
     # If enum values were specified for just one of the properties, use those.
     enum_prop = prop1 if isinstance(prop1, EnumProperty) else cast(EnumProperty, prop2)
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index 8ca054e82..1d325d7d4 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -5,6 +5,7 @@
 from openapi_python_client.parser.properties.float import FloatProperty
 from openapi_python_client.parser.properties.int import IntProperty
 from openapi_python_client.parser.properties.merge_properties import merge_properties
+from openapi_python_client.parser.properties.schemas import Class
 
 MODULE_NAME = "openapi_python_client.parser.properties.merge_properties"
 
@@ -131,6 +132,34 @@ def test_merge_string_pattern(string_property_factory):
     assert "regex patterns" in excinfo.value.args[0]
 
 
+def test_merge_enums(enum_property_factory, config):
+    enum_with_fewer_values = enum_property_factory(
+        description="desc1",
+        values={"A": "A", "B": "B"},
+        value_type=str,
+    )
+    enum_with_more_values = enum_property_factory(
+        example="example2",
+        values={"A": "A", "B": "B", "C": "C"},
+        value_type=str,
+    )
+    # Setting class_info separately because it doesn't get initialized by the constructor - we want
+    # to make sure the right enum class name gets used in the merged property
+    enum_with_fewer_values.class_info = Class.from_string(string="FewerValuesEnum", config=config)
+    enum_with_more_values.class_info = Class.from_string(string="MoreValuesEnum", config=config)
+
+    assert merge_properties(enum_with_fewer_values, enum_with_more_values) == evolve(
+        enum_with_more_values,
+        values=enum_with_fewer_values.values,
+        class_info=enum_with_fewer_values.class_info,
+        description=enum_with_fewer_values.description,
+    )
+    assert merge_properties(enum_with_more_values, enum_with_fewer_values) == evolve(
+        enum_with_fewer_values,
+        example=enum_with_more_values.example,
+    )
+
+
 def test_merge_string_with_string_enum(string_property_factory, enum_property_factory):
     values = {"A": "A", "B": "B"}
     string_prop = string_property_factory(default="default1", description="desc1", example="example1")

From 99a0c96a9d2fbc82f9f67a040c64ae2fc4dec09b Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Sat, 24 Aug 2024 22:30:38 -0600
Subject: [PATCH 08/19] Try to improve type hints

---
 .../parser/properties/enum_property.py        |  2 +-
 .../parser/properties/merge_properties.py     | 96 ++++++++++---------
 .../parser/properties/model_property.py       |  3 +-
 3 files changed, 53 insertions(+), 48 deletions(-)

diff --git a/openapi_python_client/parser/properties/enum_property.py b/openapi_python_client/parser/properties/enum_property.py
index 0f0db0d61..c5fd51772 100644
--- a/openapi_python_client/parser/properties/enum_property.py
+++ b/openapi_python_client/parser/properties/enum_property.py
@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-__all__ = ["EnumProperty"]
+__all__ = ["EnumProperty", "ValueType"]
 
 from typing import Any, ClassVar, List, Union, cast
 
diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index d1a609630..5d6da6bbc 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -1,28 +1,35 @@
-from typing import Any, Callable, Dict, TypeVar, Union, cast
+from __future__ import annotations
+
+__all__ = ["merge_properties"]
+
+from typing import TYPE_CHECKING, Any, Callable, TypeVar, cast
 
 from attr import evolve
 
-from openapi_python_client.parser.properties.any import AnyProperty
-from openapi_python_client.parser.properties.enum_property import EnumProperty, ValueType
-from openapi_python_client.parser.properties.float import FloatProperty
-from openapi_python_client.parser.properties.int import IntProperty
-from openapi_python_client.parser.properties.list_property import ListProperty
-from openapi_python_client.parser.properties.protocol import PropertyProtocol
-from openapi_python_client.parser.properties.schemas import Class
-from openapi_python_client.parser.properties.string import StringProperty
-
-# Note that in this file we have to use PropertyProtocol instead of the union type Property,
-# to avoid circular imports.
+from . import FloatProperty
+from .any import AnyProperty
+from .enum_property import EnumProperty, ValueType
+from .int import IntProperty
+from .list_property import ListProperty
+from .protocol import PropertyProtocol
+from .schemas import Class
+from .string import StringProperty
+
+if TYPE_CHECKING:
+    from .property import Property
+else:
+    Property = "Property"
+
 PropertyT = TypeVar("PropertyT", bound=PropertyProtocol)
 
 
-def merge_properties(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
+def merge_properties(prop1: Property, prop2: Property) -> Property:
     """Attempt to create a new property that incorporates the behavior of both.
 
     This is used when merging schemas with allOf, when two schemas define a property with the same name.
 
     OpenAPI defines allOf in terms of validation behavior: the input must pass the validation rules
-    defined in all of the listed schemas. Our task here is slightly more difficult, since we must end
+    defined in all the listed schemas. Our task here is slightly more difficult, since we must end
     up with a single Property object that will be used to generate a single class property in the
     generated code. Due to limitations of our internal model, this may not be possible for some
     combinations of property attributes that OpenAPI supports (for instance, we have no way to represent
@@ -35,36 +42,39 @@ def merge_properties(prop1: PropertyProtocol, prop2: PropertyProtocol) -> Proper
 
     Any failure is thrown as a ValueError.
     """
+    if isinstance(prop2, AnyProperty):
+        return _merge_common_attributes(prop1, prop2)
+
+    if isinstance(prop1, AnyProperty):
+        # Use the base type of `prop2`, but keep the override order
+        return _merge_common_attributes(prop2, prop1, prop2)
+
     if isinstance(prop1, EnumProperty) or isinstance(prop2, EnumProperty):
         return _merge_with_enum(prop1, prop2)
 
-    if prop1.__class__ == prop2.__class__:
-        return _merge_same_type(prop1, prop2)
-
-    if isinstance(prop1, AnyProperty) or isinstance(prop2, AnyProperty):
-        return _merge_with_any(prop1, prop2)
+    if (merged := _merge_same_type(prop1, prop2)) is not None:
+        return merged
 
-    if _is_numeric(prop1) and _is_numeric(prop2):
-        return _merge_numeric(prop1, prop2)
+    if (merged := _merge_numeric(prop1, prop2)) is not None:
+        return merged
 
     raise ValueError("defined with two incompatible types")
 
 
-def _is_numeric(prop: PropertyProtocol) -> bool:
-    return isinstance(prop, IntProperty) or isinstance(prop, FloatProperty)
-
+def _merge_same_type(prop1: Property, prop2: Property) -> Property | None:
+    if type(prop1) is not type(prop2):
+        return None
 
-def _merge_same_type(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
     if prop1 == prop2:
         # It's always OK to redefine a property with everything exactly the same
         return prop1
 
-    if isinstance(prop1, StringProperty):
-        return _merge_string(prop1, cast(StringProperty, prop2))
+    if isinstance(prop1, StringProperty) and isinstance(prop2, StringProperty):
+        return _merge_string(prop1, prop2)
 
-    if isinstance(prop1, ListProperty):
+    if isinstance(prop1, ListProperty) and isinstance(prop2, ListProperty):
         # There's no clear way to represent the intersection of two different list types. Fail in this case.
-        if prop1.inner_property != cast(ListProperty, prop2).inner_property:
+        if prop1.inner_property != prop2.inner_property:
             raise ValueError("can't redefine a list property with a different element type")
 
     # For all other property types, there aren't any special attributes that affect validation, so just
@@ -75,12 +85,12 @@ def _merge_same_type(prop1: PropertyProtocol, prop2: PropertyProtocol) -> Proper
 def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty:
     # If max_length was specified for both, the smallest value takes precedence. If only one of them
     # specifies it, _combine_values will pick that one.
-    max_length: Union[int, None] = _combine_values(prop1.max_length, prop2.max_length, lambda a, b: min([a, b]))
+    max_length: int | None = _combine_values(prop1.max_length, prop2.max_length, lambda a, b: min([a, b]))
 
     # If pattern was specified for both, we have a problem. OpenAPI has no logical objection to this;
     # it would just mean the value must match both of the patterns to be valid. But we have no way to
     # represent this in our internal model currently.
-    pattern: Union[str, None, ValueError] = _combine_values(
+    pattern: str | None | ValueError = _combine_values(
         prop1.pattern, prop2.pattern, lambda a, b: ValueError("specified two different regex patterns")
     )
     if isinstance(pattern, ValueError):
@@ -89,30 +99,26 @@ def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringPropert
     return _merge_common_attributes(evolve(prop1, max_length=max_length, pattern=pattern), prop2)
 
 
-def _merge_numeric(prop1: PropertyProtocol, prop2: PropertyProtocol) -> IntProperty:
-    # Here, one of the properties was defined as "int" and the other was just a general number (which
-    # we call FloatProperty). "Must be integer" is the stricter validation rule, so the result must be
-    # an IntProperty.
-    int_prop = prop1 if isinstance(prop1, IntProperty) else cast(IntProperty, prop2)
-    result = _merge_common_attributes(int_prop, prop1, prop2)
+def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None:
+    """Merge IntProperty with FloatProperty"""
+    if isinstance(prop1, IntProperty) and isinstance(prop2, (IntProperty, FloatProperty)):
+        result = _merge_common_attributes(prop1, prop2)
+    elif isinstance(prop2, IntProperty) and isinstance(prop1, (IntProperty, FloatProperty)):
+        # Use the IntProperty as a base since it's more restrictive, but keep the correct override order
+        result = _merge_common_attributes(prop2, prop1, prop2)
+    else:
+        return None
     if result.default is not None:
         if isinstance(result.default, float) and not result.default.is_integer():
             raise ValueError(f"default value {result.default} is not valid for an integer property")
     return result
 
 
-def _merge_with_any(prop1: PropertyProtocol, prop2: PropertyProtocol) -> PropertyProtocol:
-    # AnyProperty implies no validation rules for a value, so merging it with any other type just means
-    # we should use the validation rules for the other type and the result should not be an AnyProperty.
-    non_any_prop = prop2 if isinstance(prop1, AnyProperty) else prop1
-    return _merge_common_attributes(non_any_prop, prop1, prop2)
-
-
 def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumProperty:
     if isinstance(prop1, EnumProperty) and isinstance(prop2, EnumProperty):
         # We want the narrowest validation rules that fit both, so use whichever values list is a
         # subset of the other.
-        values: Dict[str, ValueType]
+        values: dict[str, ValueType]
         class_info: Class
         if _values_are_subset(prop1, prop2):
             values = prop1.values
diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py
index 3fe9836f4..79df36e01 100644
--- a/openapi_python_client/parser/properties/model_property.py
+++ b/openapi_python_client/parser/properties/model_property.py
@@ -5,13 +5,12 @@
 
 from attrs import define, evolve
 
-from openapi_python_client.parser.properties.merge_properties import merge_properties
-
 from ... import Config, utils
 from ... import schema as oai
 from ...utils import PythonIdentifier
 from ..errors import ParseError, PropertyError
 from .any import AnyProperty
+from .merge_properties import merge_properties
 from .protocol import PropertyProtocol, Value
 from .schemas import Class, ReferencePath, Schemas, parse_reference_path
 

From 90ad18b453849dd6d94ccd32486a54e43497b714 Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Sat, 24 Aug 2024 22:30:48 -0600
Subject: [PATCH 09/19] Use itertools permutations for merge tests

---
 .../test_properties/test_merge_properties.py    | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index 1d325d7d4..f08da2bcc 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -1,3 +1,5 @@
+from itertools import permutations
+
 import pytest
 from attr import evolve
 
@@ -56,14 +58,13 @@ def test_incompatible_types(
         list_property_factory(),
         model_property_factory(),
     ]
-    for i, prop1 in enumerate(props):
-        for j, prop2 in enumerate(props):
-            if i != j:
-                if {prop1.__class__, prop2.__class__} == {IntProperty, FloatProperty}:
-                    continue  # the int+float case is covered in another test
-                with pytest.raises(ValueError) as excinfo:
-                    merge_properties(prop1, prop2)
-                assert "incompatible types" in excinfo.value.args[0]
+
+    for prop1, prop2 in permutations(props, 2):
+        if {prop1.__class__, prop2.__class__} == {IntProperty, FloatProperty}:
+            continue  # the int+float case is covered in another test
+        with pytest.raises(ValueError) as excinfo:
+            merge_properties(prop1, prop2)
+        assert "incompatible types" in excinfo.value.args[0]
 
 
 def test_merge_int_with_float(int_property_factory, float_property_factory):

From d81fd5d86abc5f331cf57f3fe8b3ba705aa663af Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Sat, 24 Aug 2024 22:41:08 -0600
Subject: [PATCH 10/19] Stop checking string attributes that aren't actually
 used in generated code (`max_length` and `pattern`)

---
 .../parser/properties/__init__.py             |  1 -
 .../parser/properties/merge_properties.py     | 27 ++---------------
 .../parser/properties/string.py               |  4 ---
 .../test_parser/test_properties/test_init.py  |  4 +--
 .../test_properties/test_merge_properties.py  | 30 +------------------
 .../test_properties/test_model_property.py    |  4 +--
 6 files changed, 7 insertions(+), 63 deletions(-)

diff --git a/openapi_python_client/parser/properties/__init__.py b/openapi_python_client/parser/properties/__init__.py
index e692ce5bb..ddd0da316 100644
--- a/openapi_python_client/parser/properties/__init__.py
+++ b/openapi_python_client/parser/properties/__init__.py
@@ -83,7 +83,6 @@ def _string_based_property(
         name=name,
         default=data.default,
         required=required,
-        pattern=data.pattern,
         python_name=python_name,
         description=data.description,
         example=data.example,
diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index 5d6da6bbc..1e674dd04 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -2,7 +2,7 @@
 
 __all__ = ["merge_properties"]
 
-from typing import TYPE_CHECKING, Any, Callable, TypeVar, cast
+from typing import TYPE_CHECKING, TypeVar, cast
 
 from attr import evolve
 
@@ -83,20 +83,7 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None:
 
 
 def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty:
-    # If max_length was specified for both, the smallest value takes precedence. If only one of them
-    # specifies it, _combine_values will pick that one.
-    max_length: int | None = _combine_values(prop1.max_length, prop2.max_length, lambda a, b: min([a, b]))
-
-    # If pattern was specified for both, we have a problem. OpenAPI has no logical objection to this;
-    # it would just mean the value must match both of the patterns to be valid. But we have no way to
-    # represent this in our internal model currently.
-    pattern: str | None | ValueError = _combine_values(
-        prop1.pattern, prop2.pattern, lambda a, b: ValueError("specified two different regex patterns")
-    )
-    if isinstance(pattern, ValueError):
-        raise pattern
-
-    return _merge_common_attributes(evolve(prop1, max_length=max_length, pattern=pattern), prop2)
+    return _merge_common_attributes(prop1, prop2)
 
 
 def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None:
@@ -164,13 +151,3 @@ def _merge_common_attributes(base: PropertyT, *extend_with: PropertyProtocol) ->
 
 def _values_are_subset(prop1: EnumProperty, prop2: EnumProperty) -> bool:
     return set(prop1.values.items()) <= set(prop2.values.items())
-
-
-def _combine_values(value1: Any, value2: Any, combinator: Callable[[Any, Any], Any]) -> Any:
-    if value1 == value2:
-        return value1
-    if value1 is None:
-        return value2
-    if value2 is None:
-        return value1
-    return combinator(value1, value2)
diff --git a/openapi_python_client/parser/properties/string.py b/openapi_python_client/parser/properties/string.py
index 1afe02c62..f9adf04ae 100644
--- a/openapi_python_client/parser/properties/string.py
+++ b/openapi_python_client/parser/properties/string.py
@@ -21,8 +21,6 @@ class StringProperty(PropertyProtocol):
     python_name: PythonIdentifier
     description: str | None
     example: str | None
-    max_length: int | None = None
-    pattern: str | None = None
     _type_string: ClassVar[str] = "str"
     _json_type_string: ClassVar[str] = "str"
     _allowed_locations: ClassVar[set[oai.ParameterLocation]] = {
@@ -41,7 +39,6 @@ def build(
         python_name: PythonIdentifier,
         description: str | None,
         example: str | None,
-        pattern: str | None = None,
     ) -> StringProperty | PropertyError:
         checked_default = cls.convert_value(default)
         return cls(
@@ -51,7 +48,6 @@ def build(
             python_name=python_name,
             description=description,
             example=example,
-            pattern=pattern,
         )
 
     @classmethod
diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py
index 3290dcd39..5461624ca 100644
--- a/tests/test_parser/test_properties/test_init.py
+++ b/tests/test_parser/test_properties/test_init.py
@@ -746,14 +746,14 @@ def test_no_format(self, string_property_factory, required, config):
         from openapi_python_client.parser.properties import property_from_data
 
         name = "some_prop"
-        data = oai.Schema.model_construct(type="string", default='"hello world"', pattern="abcdef")
+        data = oai.Schema.model_construct(type="string", default='"hello world"')
 
         p, _ = property_from_data(
             name=name, required=required, data=data, parent_name=None, config=config, schemas=Schemas()
         )
 
         assert p == string_property_factory(
-            name=name, required=required, default="'\\\\\"hello world\\\\\"'", pattern=data.pattern
+            name=name, required=required, default="'\\\\\"hello world\\\\\"'"
         )
 
     def test_datetime_format(self, date_time_property_factory, config):
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index f08da2bcc..a8486ab30 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -3,7 +3,7 @@
 import pytest
 from attr import evolve
 
-from openapi_python_client.parser.properties import StringProperty
+from openapi_python_client.parser.properties.string import StringProperty
 from openapi_python_client.parser.properties.float import FloatProperty
 from openapi_python_client.parser.properties.int import IntProperty
 from openapi_python_client.parser.properties.merge_properties import merge_properties
@@ -105,34 +105,6 @@ def test_merge_with_any(
         assert merge_properties(prop, any_prop) == expected_result
 
 
-def test_merge_string_max_length(string_property_factory):
-    prop_with_no_max = string_property_factory()
-    prop_with_max_1 = evolve(prop_with_no_max, max_length=1)
-    prop_with_max_5 = evolve(prop_with_no_max, max_length=5)
-
-    assert merge_properties(prop_with_no_max, prop_with_max_1) == prop_with_max_1
-    assert merge_properties(prop_with_max_1, prop_with_no_max) == prop_with_max_1
-
-    assert merge_properties(prop_with_max_1, prop_with_max_5) == prop_with_max_1
-    assert merge_properties(prop_with_max_5, prop_with_max_1) == prop_with_max_1
-
-
-def test_merge_string_pattern(string_property_factory):
-    prop_with_no_pattern = string_property_factory()
-    prop_with_pattern1a = evolve(prop_with_no_pattern, pattern="pattern1")
-    prop_with_pattern1b = evolve(prop_with_no_pattern, pattern="pattern1")  # deliberately identical pattern
-    prop_with_pattern2 = evolve(prop_with_no_pattern, pattern="pattern2")
-
-    assert merge_properties(prop_with_no_pattern, prop_with_pattern1a) == prop_with_pattern1a
-    assert merge_properties(prop_with_pattern1a, prop_with_no_pattern) == prop_with_pattern1a
-
-    assert merge_properties(prop_with_pattern1a, prop_with_pattern1b) == prop_with_pattern1a
-
-    with pytest.raises(ValueError) as excinfo:
-        merge_properties(prop_with_pattern1a, prop_with_pattern2)
-    assert "regex patterns" in excinfo.value.args[0]
-
-
 def test_merge_enums(enum_property_factory, config):
     enum_with_fewer_values = enum_property_factory(
         description="desc1",
diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py
index 242e39f29..427966e7d 100644
--- a/tests/test_parser/test_properties/test_model_property.py
+++ b/tests/test_parser/test_properties/test_model_property.py
@@ -410,10 +410,10 @@ def test_conflicting_properties_same_types(self, model_property_factory, string_
         schemas = Schemas(
             classes_by_reference={
                 "/First": model_property_factory(
-                    required_properties=[], optional_properties=[string_property_factory(pattern="abc")]
+                    required_properties=[], optional_properties=[string_property_factory(default="abc")]
                 ),
                 "/Second": model_property_factory(
-                    required_properties=[], optional_properties=[string_property_factory(pattern="def")]
+                    required_properties=[], optional_properties=[string_property_factory(default="def")]
                 ),
             }
         )

From 4a7f603cd5f7d7f769b3d02af7ec02d82afb1176 Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Sat, 24 Aug 2024 23:03:45 -0600
Subject: [PATCH 11/19] Remove exceptions, support comparing internal props of
 lists.

---
 .../parser/properties/merge_properties.py     | 37 ++++++++++---------
 .../parser/properties/model_property.py       | 15 +++-----
 .../test_parser/test_properties/test_init.py  |  4 +-
 .../test_properties/test_merge_properties.py  | 27 ++++++--------
 4 files changed, 37 insertions(+), 46 deletions(-)

diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index 1e674dd04..f3056cc14 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -2,28 +2,25 @@
 
 __all__ = ["merge_properties"]
 
-from typing import TYPE_CHECKING, TypeVar, cast
+from typing import TypeVar, cast
 
 from attr import evolve
 
+from ..errors import PropertyError
 from . import FloatProperty
 from .any import AnyProperty
 from .enum_property import EnumProperty, ValueType
 from .int import IntProperty
 from .list_property import ListProperty
+from .property import Property
 from .protocol import PropertyProtocol
 from .schemas import Class
 from .string import StringProperty
 
-if TYPE_CHECKING:
-    from .property import Property
-else:
-    Property = "Property"
-
 PropertyT = TypeVar("PropertyT", bound=PropertyProtocol)
 
 
-def merge_properties(prop1: Property, prop2: Property) -> Property:
+def merge_properties(prop1: Property, prop2: Property) -> Property | PropertyError:
     """Attempt to create a new property that incorporates the behavior of both.
 
     This is used when merging schemas with allOf, when two schemas define a property with the same name.
@@ -39,8 +36,6 @@ def merge_properties(prop1: Property, prop2: Property) -> Property:
     and "example". OpenAPI does not define any overriding/aggregation rules for these in allOf. The
     implementation here is, assuming prop1 and prop2 are in the same order that the schemas were in the
     allOf, any such attributes that prop2 specifies will override the ones from prop1.
-
-    Any failure is thrown as a ValueError.
     """
     if isinstance(prop2, AnyProperty):
         return _merge_common_attributes(prop1, prop2)
@@ -58,10 +53,12 @@ def merge_properties(prop1: Property, prop2: Property) -> Property:
     if (merged := _merge_numeric(prop1, prop2)) is not None:
         return merged
 
-    raise ValueError("defined with two incompatible types")
+    return PropertyError(
+        detail=f"{prop1.get_type_string(no_optional=True)} can't be merged with {prop2.get_type_string(no_optional=True)}"
+    )
 
 
-def _merge_same_type(prop1: Property, prop2: Property) -> Property | None:
+def _merge_same_type(prop1: Property, prop2: Property) -> Property | None | PropertyError:
     if type(prop1) is not type(prop2):
         return None
 
@@ -74,8 +71,10 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None:
 
     if isinstance(prop1, ListProperty) and isinstance(prop2, ListProperty):
         # There's no clear way to represent the intersection of two different list types. Fail in this case.
-        if prop1.inner_property != prop2.inner_property:
-            raise ValueError("can't redefine a list property with a different element type")
+        inner_property = merge_properties(prop1.inner_property, prop2.inner_property)  # type: ignore
+        if isinstance(inner_property, PropertyError):
+            return PropertyError(detail=f"can't merge list properties: {inner_property.detail}")
+        prop1.inner_property = inner_property
 
     # For all other property types, there aren't any special attributes that affect validation, so just
     # apply the rules for common attributes like "description".
@@ -86,7 +85,7 @@ def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringPropert
     return _merge_common_attributes(prop1, prop2)
 
 
-def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None:
+def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None | PropertyError:
     """Merge IntProperty with FloatProperty"""
     if isinstance(prop1, IntProperty) and isinstance(prop2, (IntProperty, FloatProperty)):
         result = _merge_common_attributes(prop1, prop2)
@@ -97,11 +96,11 @@ def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None:
         return None
     if result.default is not None:
         if isinstance(result.default, float) and not result.default.is_integer():
-            raise ValueError(f"default value {result.default} is not valid for an integer property")
+            return PropertyError(detail=f"default value {result.default} is not valid for an integer property")
     return result
 
 
-def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumProperty:
+def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumProperty | PropertyError:
     if isinstance(prop1, EnumProperty) and isinstance(prop2, EnumProperty):
         # We want the narrowest validation rules that fit both, so use whichever values list is a
         # subset of the other.
@@ -114,7 +113,7 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
             values = prop2.values
             class_info = prop2.class_info
         else:
-            raise ValueError("can't redefine an enum property with incompatible lists of values")
+            return PropertyError(detail="can't redefine an enum property with incompatible lists of values")
         return _merge_common_attributes(evolve(prop1, values=values, class_info=class_info), prop2)
 
     # If enum values were specified for just one of the properties, use those.
@@ -124,7 +123,9 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
         isinstance(non_enum_prop, StringProperty) and enum_prop.value_type is str
     ):
         return _merge_common_attributes(enum_prop, prop1, prop2)
-    raise ValueError("defined with two incompatible types")
+    return PropertyError(
+        detail=f"can't combine enum of type {enum_prop.value_type} with {non_enum_prop.get_type_string(no_optional=True)}"
+    )
 
 
 def _merge_common_attributes(base: PropertyT, *extend_with: PropertyProtocol) -> PropertyT:
diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py
index 79df36e01..73d3b7c74 100644
--- a/openapi_python_client/parser/properties/model_property.py
+++ b/openapi_python_client/parser/properties/model_property.py
@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from itertools import chain
-from typing import Any, ClassVar, NamedTuple, cast
+from typing import Any, ClassVar, NamedTuple
 
 from attrs import define, evolve
 
@@ -10,7 +10,6 @@
 from ...utils import PythonIdentifier
 from ..errors import ParseError, PropertyError
 from .any import AnyProperty
-from .merge_properties import merge_properties
 from .protocol import PropertyProtocol, Value
 from .schemas import Class, ReferencePath, Schemas, parse_reference_path
 
@@ -244,6 +243,7 @@ def _process_properties(  # noqa: PLR0912, PLR0911
     roots: set[ReferencePath | utils.ClassName],
 ) -> _PropertyData | PropertyError:
     from . import property_from_data
+    from .merge_properties import merge_properties
 
     properties: dict[str, Property] = {}
     relative_imports: set[str] = set()
@@ -254,13 +254,10 @@ def _add_if_no_conflict(new_prop: Property) -> PropertyError | None:
         nonlocal properties
 
         name_conflict = properties.get(new_prop.name)
-        try:
-            merged_prop = cast(Property, merge_properties(name_conflict, new_prop)) if name_conflict else new_prop
-        except ValueError as e:
-            return PropertyError(
-                header=f"Found conflicting properties named {new_prop.name} when creating {class_name}",
-                detail=str(e),
-            )
+        merged_prop = merge_properties(name_conflict, new_prop) if name_conflict else new_prop
+        if isinstance(merged_prop, PropertyError):
+            merged_prop.header = f"Found conflicting properties named {new_prop.name} when creating {class_name}"
+            return merged_prop
 
         for other_prop in properties.values():
             if other_prop.name == merged_prop.name:
diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py
index 5461624ca..63fa07e64 100644
--- a/tests/test_parser/test_properties/test_init.py
+++ b/tests/test_parser/test_properties/test_init.py
@@ -752,9 +752,7 @@ def test_no_format(self, string_property_factory, required, config):
             name=name, required=required, data=data, parent_name=None, config=config, schemas=Schemas()
         )
 
-        assert p == string_property_factory(
-            name=name, required=required, default="'\\\\\"hello world\\\\\"'"
-        )
+        assert p == string_property_factory(name=name, required=required, default="'\\\\\"hello world\\\\\"'")
 
     def test_datetime_format(self, date_time_property_factory, config):
         from openapi_python_client.parser.properties import property_from_data
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index a8486ab30..36a4fb905 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -1,13 +1,13 @@
 from itertools import permutations
 
-import pytest
 from attr import evolve
 
-from openapi_python_client.parser.properties.string import StringProperty
+from openapi_python_client.parser.errors import PropertyError
 from openapi_python_client.parser.properties.float import FloatProperty
 from openapi_python_client.parser.properties.int import IntProperty
 from openapi_python_client.parser.properties.merge_properties import merge_properties
 from openapi_python_client.parser.properties.schemas import Class
+from openapi_python_client.parser.properties.string import StringProperty
 
 MODULE_NAME = "openapi_python_client.parser.properties.merge_properties"
 
@@ -62,9 +62,8 @@ def test_incompatible_types(
     for prop1, prop2 in permutations(props, 2):
         if {prop1.__class__, prop2.__class__} == {IntProperty, FloatProperty}:
             continue  # the int+float case is covered in another test
-        with pytest.raises(ValueError) as excinfo:
-            merge_properties(prop1, prop2)
-        assert "incompatible types" in excinfo.value.args[0]
+        error = merge_properties(prop1, prop2)
+        assert isinstance(error, PropertyError), f"Expected {type(prop1)} and {type(prop2)} to be incompatible"
 
 
 def test_merge_int_with_float(int_property_factory, float_property_factory):
@@ -77,9 +76,9 @@ def test_merge_int_with_float(int_property_factory, float_property_factory):
     assert merge_properties(float_prop, int_prop) == (evolve(int_prop, default=float_prop.default))
 
     float_prop_with_non_int_default = evolve(float_prop, default=2.5)
-    with pytest.raises(ValueError) as excinfo:
-        merge_properties(int_prop, float_prop_with_non_int_default)
-    assert "default value" in excinfo.value.args[0]
+    error = merge_properties(int_prop, float_prop_with_non_int_default)
+    assert isinstance(error, PropertyError), "Expected invalid default to error"
+    assert "default value" in error.detail
 
 
 def test_merge_with_any(
@@ -190,12 +189,8 @@ def test_merge_with_incompatible_enum(
     int_enum_prop = enum_property_factory(value_type=int)
     for prop in props:
         if not isinstance(prop, StringProperty):
-            with pytest.raises(ValueError):
-                merge_properties(prop, string_enum_prop)
-            with pytest.raises(ValueError):
-                merge_properties(string_enum_prop, prop)
+            assert isinstance(merge_properties(prop, string_enum_prop), PropertyError)
+            assert isinstance(merge_properties(string_enum_prop, prop), PropertyError)
         if not isinstance(prop, IntProperty):
-            with pytest.raises(ValueError):
-                merge_properties(prop, int_enum_prop)
-            with pytest.raises(ValueError):
-                merge_properties(int_enum_prop, prop)
+            assert isinstance(merge_properties(prop, int_enum_prop), PropertyError)
+            assert isinstance(merge_properties(int_enum_prop, prop), PropertyError)

From 770ebc21d024c3c85cd99b748ce024b88d5aaddd Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Sat, 24 Aug 2024 23:44:34 -0600
Subject: [PATCH 12/19] Validate all default types

---
 .../parser/properties/enum_property.py        |  6 +++
 .../parser/properties/merge_properties.py     | 25 ++++++------
 .../test_properties/test_merge_properties.py  | 39 +++++++++----------
 .../test_properties/test_model_property.py    | 19 ---------
 4 files changed, 37 insertions(+), 52 deletions(-)

diff --git a/openapi_python_client/parser/properties/enum_property.py b/openapi_python_client/parser/properties/enum_property.py
index c5fd51772..0e5c80588 100644
--- a/openapi_python_client/parser/properties/enum_property.py
+++ b/openapi_python_client/parser/properties/enum_property.py
@@ -164,6 +164,12 @@ def convert_value(self, value: Any) -> Value | PropertyError | None:
                 return PropertyError(detail=f"Value {value} is not valid for enum {self.name}")
         return PropertyError(detail=f"Cannot convert {value} to enum {self.name} of type {self.value_type}")
 
+    def default_to_raw(self) -> ValueType | None:
+        if self.default is None:
+            return None
+        key = self.default.split(".")[1]
+        return self.values[key]
+
     def get_base_type_string(self, *, quoted: bool = False) -> str:
         return self.class_info.name
 
diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index f3056cc14..22a175d0c 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -9,12 +9,11 @@
 from ..errors import PropertyError
 from . import FloatProperty
 from .any import AnyProperty
-from .enum_property import EnumProperty, ValueType
+from .enum_property import EnumProperty
 from .int import IntProperty
 from .list_property import ListProperty
 from .property import Property
 from .protocol import PropertyProtocol
-from .schemas import Class
 from .string import StringProperty
 
 PropertyT = TypeVar("PropertyT", bound=PropertyProtocol)
@@ -81,31 +80,25 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None | Prop
     return _merge_common_attributes(prop1, prop2)
 
 
-def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty:
+def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty | PropertyError:
     return _merge_common_attributes(prop1, prop2)
 
 
 def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None | PropertyError:
     """Merge IntProperty with FloatProperty"""
     if isinstance(prop1, IntProperty) and isinstance(prop2, (IntProperty, FloatProperty)):
-        result = _merge_common_attributes(prop1, prop2)
+        return _merge_common_attributes(prop1, prop2)
     elif isinstance(prop2, IntProperty) and isinstance(prop1, (IntProperty, FloatProperty)):
         # Use the IntProperty as a base since it's more restrictive, but keep the correct override order
-        result = _merge_common_attributes(prop2, prop1, prop2)
+        return _merge_common_attributes(prop2, prop1, prop2)
     else:
         return None
-    if result.default is not None:
-        if isinstance(result.default, float) and not result.default.is_integer():
-            return PropertyError(detail=f"default value {result.default} is not valid for an integer property")
-    return result
 
 
 def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumProperty | PropertyError:
     if isinstance(prop1, EnumProperty) and isinstance(prop2, EnumProperty):
         # We want the narrowest validation rules that fit both, so use whichever values list is a
         # subset of the other.
-        values: dict[str, ValueType]
-        class_info: Class
         if _values_are_subset(prop1, prop2):
             values = prop1.values
             class_info = prop1.class_info
@@ -128,7 +121,7 @@ def _merge_with_enum(prop1: PropertyProtocol, prop2: PropertyProtocol) -> EnumPr
     )
 
 
-def _merge_common_attributes(base: PropertyT, *extend_with: PropertyProtocol) -> PropertyT:
+def _merge_common_attributes(base: PropertyT, *extend_with: PropertyProtocol) -> PropertyT | PropertyError:
     """Create a new instance based on base, overriding basic attributes with values from extend_with, in order.
 
     For "default", "description", and "example", a non-None value overrides any value from a previously
@@ -140,10 +133,16 @@ def _merge_common_attributes(base: PropertyT, *extend_with: PropertyProtocol) ->
     """
     current = base
     for override in extend_with:
+        if isinstance(override, EnumProperty):
+            override_default = current.convert_value(override.default_to_raw())
+        else:
+            override_default = current.convert_value(override.default)
+        if isinstance(override_default, PropertyError):
+            return override_default
         current = evolve(
             current,  # type: ignore # can't prove that every property type is an attrs class, but it is
             required=current.required or override.required,
-            default=override.default or current.default,
+            default=override_default or current.default,
             description=override.description or current.description,
             example=override.example or current.example,
         )
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index 36a4fb905..c3cd95a4b 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -21,10 +21,10 @@ def test_merge_basic_attributes_same_type(
     model_property_factory,
 ):
     basic_props = [
-        boolean_property_factory(default=True),
-        int_property_factory(default=1),
-        float_property_factory(default=1.5),
-        string_property_factory(default="x"),
+        boolean_property_factory(default="True"),
+        int_property_factory(default="1"),
+        float_property_factory(default="1.5"),
+        string_property_factory(default=StringProperty.convert_value("x")),
         list_property_factory(),
         model_property_factory(),
     ]
@@ -68,17 +68,17 @@ def test_incompatible_types(
 
 def test_merge_int_with_float(int_property_factory, float_property_factory):
     int_prop = int_property_factory(description="desc1")
-    float_prop = float_property_factory(default=2, description="desc2")
+    float_prop = float_property_factory(default="2", description="desc2")
 
     assert merge_properties(int_prop, float_prop) == (
         evolve(int_prop, default=float_prop.default, description=float_prop.description)
     )
     assert merge_properties(float_prop, int_prop) == (evolve(int_prop, default=float_prop.default))
 
-    float_prop_with_non_int_default = evolve(float_prop, default=2.5)
+    float_prop_with_non_int_default = evolve(float_prop, default="2.5")
     error = merge_properties(int_prop, float_prop_with_non_int_default)
     assert isinstance(error, PropertyError), "Expected invalid default to error"
-    assert "default value" in error.detail
+    assert error.detail == "Invalid int value: 2.5"
 
 
 def test_merge_with_any(
@@ -91,17 +91,16 @@ def test_merge_with_any(
 ):
     original_desc = "description"
     props = [
-        boolean_property_factory(default=True, description=original_desc),
-        int_property_factory(default=1, description=original_desc),
-        float_property_factory(default=1.5, description=original_desc),
-        string_property_factory(default="x", description=original_desc),
+        boolean_property_factory(default="True", description=original_desc),
+        int_property_factory(default="1", description=original_desc),
+        float_property_factory(default="1.5", description=original_desc),
+        string_property_factory(default=StringProperty.convert_value("x"), description=original_desc),
         model_property_factory(description=original_desc),
     ]
     any_prop = any_property_factory()
     for prop in props:
-        expected_result = evolve(prop, description=original_desc, default=prop.default)
-        assert merge_properties(any_prop, prop) == expected_result
-        assert merge_properties(prop, any_prop) == expected_result
+        assert merge_properties(any_prop, prop) == prop
+        assert merge_properties(prop, any_prop) == prop
 
 
 def test_merge_enums(enum_property_factory, config):
@@ -134,9 +133,9 @@ def test_merge_enums(enum_property_factory, config):
 
 def test_merge_string_with_string_enum(string_property_factory, enum_property_factory):
     values = {"A": "A", "B": "B"}
-    string_prop = string_property_factory(default="default1", description="desc1", example="example1")
+    string_prop = string_property_factory(default="A", description="desc1", example="example1")
     enum_prop = enum_property_factory(
-        default="default2",
+        default="test.B",
         description="desc2",
         example="example2",
         values=values,
@@ -147,7 +146,7 @@ def test_merge_string_with_string_enum(string_property_factory, enum_property_fa
     assert merge_properties(enum_prop, string_prop) == evolve(
         enum_prop,
         required=True,
-        default=string_prop.default,
+        default="test.A",
         description=string_prop.description,
         example=string_prop.example,
     )
@@ -155,9 +154,9 @@ def test_merge_string_with_string_enum(string_property_factory, enum_property_fa
 
 def test_merge_int_with_int_enum(int_property_factory, enum_property_factory):
     values = {"VALUE_1": 1, "VALUE_2": 2}
-    int_prop = int_property_factory(default=100, description="desc1", example="example1")
+    int_prop = int_property_factory(default=1, description="desc1", example="example1")
     enum_prop = enum_property_factory(
-        default=200,
+        default="test.VALUE_1",
         description="desc2",
         example="example2",
         values=values,
@@ -166,7 +165,7 @@ def test_merge_int_with_int_enum(int_property_factory, enum_property_factory):
 
     assert merge_properties(int_prop, enum_prop) == evolve(enum_prop, required=True)
     assert merge_properties(enum_prop, int_prop) == evolve(
-        enum_prop, required=True, default=int_prop.default, description=int_prop.description, example=int_prop.example
+        enum_prop, required=True, description=int_prop.description, example=int_prop.example
     )
 
 
diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py
index 427966e7d..59bfc500c 100644
--- a/tests/test_parser/test_properties/test_model_property.py
+++ b/tests/test_parser/test_properties/test_model_property.py
@@ -403,25 +403,6 @@ def test_reference_not_processed(self, model_property_factory, config):
 
         assert isinstance(result, PropertyError)
 
-    def test_conflicting_properties_same_types(self, model_property_factory, string_property_factory, config):
-        data = oai.Schema.model_construct(
-            allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")]
-        )
-        schemas = Schemas(
-            classes_by_reference={
-                "/First": model_property_factory(
-                    required_properties=[], optional_properties=[string_property_factory(default="abc")]
-                ),
-                "/Second": model_property_factory(
-                    required_properties=[], optional_properties=[string_property_factory(default="def")]
-                ),
-            }
-        )
-
-        result = _process_properties(data=data, schemas=schemas, class_name="", config=config, roots={"root"})
-
-        assert isinstance(result, PropertyError)
-
     def test_allof_string_and_string_enum(
         self, model_property_factory, enum_property_factory, string_property_factory, config
     ):

From 00670a0b0949b8b2baa1a55f498e8bcc6b2fbd5c Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Sat, 24 Aug 2024 23:44:44 -0600
Subject: [PATCH 13/19] Update changeset to new expected end-state

---
 .changeset/improved_property_merging.md | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/.changeset/improved_property_merging.md b/.changeset/improved_property_merging.md
index e295ca692..2f4e763f6 100644
--- a/.changeset/improved_property_merging.md
+++ b/.changeset/improved_property_merging.md
@@ -7,6 +7,10 @@ default: minor
 When using `allOf` to extend a base object type, `openapi-python-client` is now able to handle some kinds of modifications to an existing property that would have previously caused an error:
 
 - Overriding attributes that do not affect validation, such as `description`.
-- Specifying stricter validation for the same type, such as setting a `maxLength` for a string that previously had no max length (or previously had a larger max length), or setting a `pattern` regex for a string that previously had none.
-- Changing a nonspecific numeric type to `int`.
-- Changing a property from `any` to a specific type.
+- Combining properties that this generator ignores, like `maxLength` or `pattern`.
+- Combining a generic numeric type with `int` (resulting in `int`).
+- Adding a `format` to a string.
+- Combining `any` with a specific type (resulting in that specific type).
+
+> [!NOTE]
+> `pattern` and `max_length` are no longer fields on `StringProperty`, which may impact custom templates.

From db90641b4f167c7353ad1d408ac87762b02994be Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Mon, 26 Aug 2024 12:40:25 -0700
Subject: [PATCH 14/19] allow merge of str with date/file/etc., add end-to-end
 tests

---
 end_to_end_tests/baseline_openapi_3.0.json    |  50 ++++++++
 end_to_end_tests/baseline_openapi_3.1.yaml    |  50 ++++++++
 .../my_test_api_client/models/__init__.py     |   4 +
 .../models/model_with_merged_properties.py    | 111 ++++++++++++++++++
 ...l_with_merged_properties_string_to_enum.py |   9 ++
 .../parser/properties/merge_properties.py     |  28 ++++-
 .../test_properties/test_merge_properties.py  |  26 ++++
 .../test_properties/test_model_property.py    |   6 +-
 8 files changed, 274 insertions(+), 10 deletions(-)
 create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
 create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties_string_to_enum.py

diff --git a/end_to_end_tests/baseline_openapi_3.0.json b/end_to_end_tests/baseline_openapi_3.0.json
index 6f9d711f9..85a1e08b7 100644
--- a/end_to_end_tests/baseline_openapi_3.0.json
+++ b/end_to_end_tests/baseline_openapi_3.0.json
@@ -2152,6 +2152,56 @@
           "additionalProperties": {}
         }
       },
+      "ModelWithMergedProperties": {
+        "title": "ModelWithMergedProperties",
+        "allOf": [
+          {
+            "type": "object",
+            "properties": {
+              "simpleString": {
+                "type": "string",
+                "description": "base simpleString description"
+              },
+              "stringToEnum": {
+                "type": "string",
+                "default": "a"
+              },
+              "stringToDate": {
+                "type": "string"
+              },
+              "numberToInt": {
+                "type": "number"
+              },
+              "anyToString": {}
+            }
+          },
+          {
+            "type": "object",
+            "properties": {
+              "simpleString": {
+                "type": "string",
+                "description": "extended simpleString description",
+                "default": "new default"
+              },
+              "stringToEnum": {
+                "type": "string",
+                "enum": ["a", "b"]
+              },
+              "stringToDate": {
+                "type": "string",
+                "format": "date"
+              },
+              "numberToInt": {
+                "type": "integer"
+              },
+              "anyToString": {
+                "type": "string",
+                "default": "x"
+              }
+            }
+          }
+        ]
+      },
       "ModelWithPrimitiveAdditionalProperties": {
         "title": "ModelWithPrimitiveAdditionalProperties",
         "type": "object",
diff --git a/end_to_end_tests/baseline_openapi_3.1.yaml b/end_to_end_tests/baseline_openapi_3.1.yaml
index 6bea1ec32..a3338fd91 100644
--- a/end_to_end_tests/baseline_openapi_3.1.yaml
+++ b/end_to_end_tests/baseline_openapi_3.1.yaml
@@ -2146,6 +2146,56 @@ info:
         "additionalProperties": { }
       }
     },
+    "ModelWithMergedProperties": {
+      "title": "ModelWithMergedProperties",
+      "allOf": [
+        {
+          "type": "object",
+          "properties": {
+            "simpleString": {
+              "type": "string",
+              "description": "base simpleString description"
+            },
+            "stringToEnum": {
+              "type": "string",
+              "default": "a"
+            },
+            "stringToDate": {
+              "type": "string"
+            },
+            "numberToInt": {
+              "type": "number"
+            },
+            "anyToString": {}
+          }
+        },
+        {
+          "type": "object",
+          "properties": {
+            "simpleString": {
+              "type": "string",
+              "description": "extended simpleString description",
+              "default": "new default"
+            },
+            "stringToEnum": {
+              "type": "string",
+              "enum": ["a", "b"]
+            },
+            "stringToDate": {
+              "type": "string",
+              "format": "date"
+            },
+            "numberToInt": {
+              "type": "integer"
+            },
+            "anyToString": {
+              "type": "string",
+              "default": "x"
+            }
+          }
+        }
+      ]
+    },
     "ModelWithPrimitiveAdditionalProperties": {
       "title": "ModelWithPrimitiveAdditionalProperties",
       "type": "object",
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py
index cd0ea68da..5210638e5 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py
@@ -60,6 +60,8 @@
 from .model_with_circular_ref_in_additional_properties_b import ModelWithCircularRefInAdditionalPropertiesB
 from .model_with_date_time_property import ModelWithDateTimeProperty
 from .model_with_discriminated_union import ModelWithDiscriminatedUnion
+from .model_with_merged_properties import ModelWithMergedProperties
+from .model_with_merged_properties_string_to_enum import ModelWithMergedPropertiesStringToEnum
 from .model_with_primitive_additional_properties import ModelWithPrimitiveAdditionalProperties
 from .model_with_primitive_additional_properties_a_date_holder import ModelWithPrimitiveAdditionalPropertiesADateHolder
 from .model_with_property_ref import ModelWithPropertyRef
@@ -137,6 +139,8 @@
     "ModelWithCircularRefInAdditionalPropertiesB",
     "ModelWithDateTimeProperty",
     "ModelWithDiscriminatedUnion",
+    "ModelWithMergedProperties",
+    "ModelWithMergedPropertiesStringToEnum",
     "ModelWithPrimitiveAdditionalProperties",
     "ModelWithPrimitiveAdditionalPropertiesADateHolder",
     "ModelWithPropertyRef",
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
new file mode 100644
index 000000000..6bc41a739
--- /dev/null
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
@@ -0,0 +1,111 @@
+import datetime
+from typing import Any, Dict, List, Type, TypeVar, Union
+
+from attrs import define as _attrs_define
+from attrs import field as _attrs_field
+from dateutil.parser import isoparse
+
+from ..models.model_with_merged_properties_string_to_enum import ModelWithMergedPropertiesStringToEnum
+from ..types import UNSET, Unset
+
+T = TypeVar("T", bound="ModelWithMergedProperties")
+
+
+@_attrs_define
+class ModelWithMergedProperties:
+    """
+    Attributes:
+        simple_string (Union[Unset, str]): extended simpleString description Default: 'new default'.
+        string_to_enum (Union[Unset, ModelWithMergedPropertiesStringToEnum]):
+        string_to_date (Union[Unset, datetime.date]):
+        number_to_int (Union[Unset, int]):
+        any_to_string (Union[Unset, str]):  Default: 'x'.
+    """
+
+    simple_string: Union[Unset, str] = "new default"
+    string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum] = UNSET
+    string_to_date: Union[Unset, datetime.date] = UNSET
+    number_to_int: Union[Unset, int] = UNSET
+    any_to_string: Union[Unset, str] = "x"
+    additional_properties: Dict[str, Any] = _attrs_field(init=False, factory=dict)
+
+    def to_dict(self) -> Dict[str, Any]:
+        simple_string = self.simple_string
+
+        string_to_enum: Union[Unset, str] = UNSET
+        if not isinstance(self.string_to_enum, Unset):
+            string_to_enum = self.string_to_enum.value
+
+        string_to_date: Union[Unset, str] = UNSET
+        if not isinstance(self.string_to_date, Unset):
+            string_to_date = self.string_to_date.isoformat()
+
+        number_to_int = self.number_to_int
+
+        any_to_string = self.any_to_string
+
+        field_dict: Dict[str, Any] = {}
+        field_dict.update(self.additional_properties)
+        field_dict.update({})
+        if simple_string is not UNSET:
+            field_dict["simpleString"] = simple_string
+        if string_to_enum is not UNSET:
+            field_dict["stringToEnum"] = string_to_enum
+        if string_to_date is not UNSET:
+            field_dict["stringToDate"] = string_to_date
+        if number_to_int is not UNSET:
+            field_dict["numberToInt"] = number_to_int
+        if any_to_string is not UNSET:
+            field_dict["anyToString"] = any_to_string
+
+        return field_dict
+
+    @classmethod
+    def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T:
+        d = src_dict.copy()
+        simple_string = d.pop("simpleString", UNSET)
+
+        _string_to_enum = d.pop("stringToEnum", UNSET)
+        string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum]
+        if isinstance(_string_to_enum, Unset):
+            string_to_enum = UNSET
+        else:
+            string_to_enum = ModelWithMergedPropertiesStringToEnum(_string_to_enum)
+
+        _string_to_date = d.pop("stringToDate", UNSET)
+        string_to_date: Union[Unset, datetime.date]
+        if isinstance(_string_to_date, Unset):
+            string_to_date = UNSET
+        else:
+            string_to_date = isoparse(_string_to_date).date()
+
+        number_to_int = d.pop("numberToInt", UNSET)
+
+        any_to_string = d.pop("anyToString", UNSET)
+
+        model_with_merged_properties = cls(
+            simple_string=simple_string,
+            string_to_enum=string_to_enum,
+            string_to_date=string_to_date,
+            number_to_int=number_to_int,
+            any_to_string=any_to_string,
+        )
+
+        model_with_merged_properties.additional_properties = d
+        return model_with_merged_properties
+
+    @property
+    def additional_keys(self) -> List[str]:
+        return list(self.additional_properties.keys())
+
+    def __getitem__(self, key: str) -> Any:
+        return self.additional_properties[key]
+
+    def __setitem__(self, key: str, value: Any) -> None:
+        self.additional_properties[key] = value
+
+    def __delitem__(self, key: str) -> None:
+        del self.additional_properties[key]
+
+    def __contains__(self, key: str) -> bool:
+        return key in self.additional_properties
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties_string_to_enum.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties_string_to_enum.py
new file mode 100644
index 000000000..5e146c5eb
--- /dev/null
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties_string_to_enum.py
@@ -0,0 +1,9 @@
+from enum import Enum
+
+
+class ModelWithMergedPropertiesStringToEnum(str, Enum):
+    A = "a"
+    B = "b"
+
+    def __str__(self) -> str:
+        return str(self.value)
diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index 22a175d0c..49825f667 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -1,5 +1,9 @@
 from __future__ import annotations
 
+from openapi_python_client.parser.properties.date import DateProperty
+from openapi_python_client.parser.properties.datetime import DateTimeProperty
+from openapi_python_client.parser.properties.file import FileProperty
+
 __all__ = ["merge_properties"]
 
 from typing import TypeVar, cast
@@ -19,7 +23,10 @@
 PropertyT = TypeVar("PropertyT", bound=PropertyProtocol)
 
 
-def merge_properties(prop1: Property, prop2: Property) -> Property | PropertyError:
+STRING_WITH_FORMAT_TYPES = (DateProperty, DateTimeProperty, FileProperty)
+
+
+def merge_properties(prop1: Property, prop2: Property) -> Property | PropertyError:  # noqa: PLR0911
     """Attempt to create a new property that incorporates the behavior of both.
 
     This is used when merging schemas with allOf, when two schemas define a property with the same name.
@@ -52,6 +59,9 @@ def merge_properties(prop1: Property, prop2: Property) -> Property | PropertyErr
     if (merged := _merge_numeric(prop1, prop2)) is not None:
         return merged
 
+    if (merged := _merge_string_with_format(prop1, prop2)) is not None:
+        return merged
+
     return PropertyError(
         detail=f"{prop1.get_type_string(no_optional=True)} can't be merged with {prop2.get_type_string(no_optional=True)}"
     )
@@ -65,9 +75,6 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None | Prop
         # It's always OK to redefine a property with everything exactly the same
         return prop1
 
-    if isinstance(prop1, StringProperty) and isinstance(prop2, StringProperty):
-        return _merge_string(prop1, prop2)
-
     if isinstance(prop1, ListProperty) and isinstance(prop2, ListProperty):
         # There's no clear way to represent the intersection of two different list types. Fail in this case.
         inner_property = merge_properties(prop1.inner_property, prop2.inner_property)  # type: ignore
@@ -80,8 +87,17 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None | Prop
     return _merge_common_attributes(prop1, prop2)
 
 
-def _merge_string(prop1: StringProperty, prop2: StringProperty) -> StringProperty | PropertyError:
-    return _merge_common_attributes(prop1, prop2)
+def _merge_string_with_format(prop1: Property, prop2: Property) -> Property | None | PropertyError:
+    """Merge a string that has no format with a string that has a format"""
+    # Here we need to use the DateProperty/DateTimeProperty/FileProperty as the base so that we preserve
+    # its class, but keep the correct override order for merging the attributes.
+    if isinstance(prop1, StringProperty) and isinstance(prop2, STRING_WITH_FORMAT_TYPES):
+        # Use the more specific class as a base, but keep the correct override order
+        return _merge_common_attributes(prop2, prop1, prop2)
+    elif isinstance(prop2, StringProperty) and isinstance(prop1, STRING_WITH_FORMAT_TYPES):
+        return _merge_common_attributes(prop1, prop2)
+    else:
+        return None
 
 
 def _merge_numeric(prop1: Property, prop2: Property) -> IntProperty | None | PropertyError:
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index c3cd95a4b..267cebce4 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -193,3 +193,29 @@ def test_merge_with_incompatible_enum(
         if not isinstance(prop, IntProperty):
             assert isinstance(merge_properties(prop, int_enum_prop), PropertyError)
             assert isinstance(merge_properties(int_enum_prop, prop), PropertyError)
+
+
+def test_merge_string_with_formatted_string(
+    date_property_factory,
+    date_time_property_factory,
+    file_property_factory,
+    string_property_factory,
+):
+    string_prop = string_property_factory(description="a plain string")
+    string_prop_with_invalid_default = string_property_factory(default="plain string value")
+    formatted_props = [
+        date_property_factory(description="a date"),
+        date_time_property_factory(description="a datetime"),
+        file_property_factory(description="a file"),
+    ]
+    for formatted_prop in formatted_props:
+        merged1 = merge_properties(string_prop, formatted_prop)
+        assert isinstance(merged1, formatted_prop.__class__)
+        assert merged1.description == formatted_prop.description
+
+        merged2 = merge_properties(formatted_prop, string_prop)
+        assert isinstance(merged2, formatted_prop.__class__)
+        assert merged2.description == string_prop.description
+
+        assert isinstance(merge_properties(string_prop_with_invalid_default, formatted_prop), PropertyError)
+        assert isinstance(merge_properties(formatted_prop, string_prop_with_invalid_default), PropertyError)
diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py
index 59bfc500c..19717cf9a 100644
--- a/tests/test_parser/test_properties/test_model_property.py
+++ b/tests/test_parser/test_properties/test_model_property.py
@@ -325,7 +325,7 @@ def test_process_properties_false(self, model_property_factory, config):
 
 class TestProcessProperties:
     def test_conflicting_properties_different_types(
-        self, model_property_factory, string_property_factory, date_time_property_factory, config
+        self, model_property_factory, string_property_factory, int_property_factory, config
     ):
         data = oai.Schema.model_construct(
             allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")]
@@ -335,9 +335,7 @@ def test_conflicting_properties_different_types(
                 "/First": model_property_factory(
                     required_properties=[], optional_properties=[string_property_factory()]
                 ),
-                "/Second": model_property_factory(
-                    required_properties=[], optional_properties=[date_time_property_factory()]
-                ),
+                "/Second": model_property_factory(required_properties=[], optional_properties=[int_property_factory()]),
             }
         )
 

From 22361feca3843045b101cd4d2d962838dae411c6 Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Mon, 26 Aug 2024 12:51:39 -0700
Subject: [PATCH 15/19] add test for merging lists

---
 .../parser/properties/merge_properties.py         |  1 -
 .../test_properties/test_merge_properties.py      | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index 49825f667..8c57cf8fa 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -76,7 +76,6 @@ def _merge_same_type(prop1: Property, prop2: Property) -> Property | None | Prop
         return prop1
 
     if isinstance(prop1, ListProperty) and isinstance(prop2, ListProperty):
-        # There's no clear way to represent the intersection of two different list types. Fail in this case.
         inner_property = merge_properties(prop1.inner_property, prop2.inner_property)  # type: ignore
         if isinstance(inner_property, PropertyError):
             return PropertyError(detail=f"can't merge list properties: {inner_property.detail}")
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index 267cebce4..d57176eb8 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -219,3 +219,18 @@ def test_merge_string_with_formatted_string(
 
         assert isinstance(merge_properties(string_prop_with_invalid_default, formatted_prop), PropertyError)
         assert isinstance(merge_properties(formatted_prop, string_prop_with_invalid_default), PropertyError)
+
+
+def test_merge_lists(int_property_factory, list_property_factory, string_property_factory):
+    string_prop_1 = string_property_factory(description="desc1")
+    string_prop_2 = string_property_factory(example="desc2")
+    int_prop = int_property_factory()
+    list_prop_1 = list_property_factory(inner_property=string_prop_1)
+    list_prop_2 = list_property_factory(inner_property=string_prop_2)
+    list_prop_3 = list_property_factory(inner_property=int_prop)
+
+    assert merge_properties(list_prop_1, list_prop_2) == evolve(
+        list_prop_1, inner_property=merge_properties(string_prop_1, string_prop_2)
+    )
+
+    assert isinstance(merge_properties(list_prop_1, list_prop_3), PropertyError)

From 3bfaef9de780e1ffc1351a95c6bf36020e311ade Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Mon, 2 Sep 2024 13:43:10 -0600
Subject: [PATCH 16/19] Mention default in changeset

---
 .changeset/improved_property_merging.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.changeset/improved_property_merging.md b/.changeset/improved_property_merging.md
index 2f4e763f6..ab44d6d37 100644
--- a/.changeset/improved_property_merging.md
+++ b/.changeset/improved_property_merging.md
@@ -11,6 +11,7 @@ When using `allOf` to extend a base object type, `openapi-python-client` is now
 - Combining a generic numeric type with `int` (resulting in `int`).
 - Adding a `format` to a string.
 - Combining `any` with a specific type (resulting in that specific type).
+- Adding or overriding a `default`
 
 > [!NOTE]
 > `pattern` and `max_length` are no longer fields on `StringProperty`, which may impact custom templates.

From 4209fb63b23c99835712afb803ed942610402680 Mon Sep 17 00:00:00 2001
From: Dylan Anthony <dbanty@users.noreply.github.com>
Date: Mon, 2 Sep 2024 14:40:52 -0600
Subject: [PATCH 17/19] Fix merging inline objects

---
 .../models/model_with_merged_properties.py                | 4 ++--
 openapi_python_client/parser/properties/model_property.py | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
index 6bc41a739..ad9feff8d 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
@@ -16,14 +16,14 @@ class ModelWithMergedProperties:
     """
     Attributes:
         simple_string (Union[Unset, str]): extended simpleString description Default: 'new default'.
-        string_to_enum (Union[Unset, ModelWithMergedPropertiesStringToEnum]):
+        string_to_enum (Union[Unset, ModelWithMergedPropertiesStringToEnum]):  Default: 'a'.
         string_to_date (Union[Unset, datetime.date]):
         number_to_int (Union[Unset, int]):
         any_to_string (Union[Unset, str]):  Default: 'x'.
     """
 
     simple_string: Union[Unset, str] = "new default"
-    string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum] = UNSET
+    string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum] = "a"
     string_to_date: Union[Unset, datetime.date] = UNSET
     number_to_int: Union[Unset, int] = UNSET
     any_to_string: Union[Unset, str] = "x"
diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py
index f7736459e..0dc13be54 100644
--- a/openapi_python_client/parser/properties/model_property.py
+++ b/openapi_python_client/parser/properties/model_property.py
@@ -275,7 +275,9 @@ def _add_if_no_conflict(new_prop: Property) -> PropertyError | None:
         properties[merged_prop.name] = merged_prop
         return None
 
-    unprocessed_props = data.properties or {}
+    unprocessed_props: list[tuple[str, oai.Reference | oai.Schema]] = (
+        list(data.properties.items()) if data.properties else []
+    )
     for sub_prop in data.allOf:
         if isinstance(sub_prop, oai.Reference):
             ref_path = parse_reference_path(sub_prop.ref)
@@ -297,10 +299,10 @@ def _add_if_no_conflict(new_prop: Property) -> PropertyError | None:
                     return err
             schemas.add_dependencies(ref_path=ref_path, roots=roots)
         else:
-            unprocessed_props.update(sub_prop.properties or {})
+            unprocessed_props.extend(sub_prop.properties.items() if sub_prop.properties else [])
             required_set.update(sub_prop.required or [])
 
-    for key, value in unprocessed_props.items():
+    for key, value in unprocessed_props:
         prop_required = key in required_set
         prop_or_error: Property | (PropertyError | None)
         prop_or_error, schemas = property_from_data(

From 1e995b77bff71fe8e0601edb3189cd828742ca6a Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Tue, 3 Sep 2024 11:22:07 -0700
Subject: [PATCH 18/19] fix default computation for merged properties

---
 .changeset/improved_property_merging.md       |  3 ++
 .../models/model_with_merged_properties.py    |  5 +--
 .../parser/properties/merge_properties.py     |  5 +--
 .../parser/properties/protocol.py             | 14 ++++++++
 .../test_parser/test_properties/test_init.py  | 32 +++++++++++++++----
 .../test_properties/test_model_property.py    | 27 ++++++++++++++++
 6 files changed, 74 insertions(+), 12 deletions(-)

diff --git a/.changeset/improved_property_merging.md b/.changeset/improved_property_merging.md
index ab44d6d37..68f3b9ed4 100644
--- a/.changeset/improved_property_merging.md
+++ b/.changeset/improved_property_merging.md
@@ -15,3 +15,6 @@ When using `allOf` to extend a base object type, `openapi-python-client` is now
 
 > [!NOTE]
 > `pattern` and `max_length` are no longer fields on `StringProperty`, which may impact custom templates.
+
+This also fixes a bug where properties of inline objects (as opposed to references) were not using the
+merge logic, but were simply overwriting previous definitions of the same property.
diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
index ad9feff8d..bcf1efa88 100644
--- a/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
+++ b/end_to_end_tests/golden-record/my_test_api_client/models/model_with_merged_properties.py
@@ -16,14 +16,15 @@ class ModelWithMergedProperties:
     """
     Attributes:
         simple_string (Union[Unset, str]): extended simpleString description Default: 'new default'.
-        string_to_enum (Union[Unset, ModelWithMergedPropertiesStringToEnum]):  Default: 'a'.
+        string_to_enum (Union[Unset, ModelWithMergedPropertiesStringToEnum]):  Default:
+            ModelWithMergedPropertiesStringToEnum.A.
         string_to_date (Union[Unset, datetime.date]):
         number_to_int (Union[Unset, int]):
         any_to_string (Union[Unset, str]):  Default: 'x'.
     """
 
     simple_string: Union[Unset, str] = "new default"
-    string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum] = "a"
+    string_to_enum: Union[Unset, ModelWithMergedPropertiesStringToEnum] = ModelWithMergedPropertiesStringToEnum.A
     string_to_date: Union[Unset, datetime.date] = UNSET
     number_to_int: Union[Unset, int] = UNSET
     any_to_string: Union[Unset, str] = "x"
diff --git a/openapi_python_client/parser/properties/merge_properties.py b/openapi_python_client/parser/properties/merge_properties.py
index 8c57cf8fa..f4e72849b 100644
--- a/openapi_python_client/parser/properties/merge_properties.py
+++ b/openapi_python_client/parser/properties/merge_properties.py
@@ -148,10 +148,7 @@ def _merge_common_attributes(base: PropertyT, *extend_with: PropertyProtocol) ->
     """
     current = base
     for override in extend_with:
-        if isinstance(override, EnumProperty):
-            override_default = current.convert_value(override.default_to_raw())
-        else:
-            override_default = current.convert_value(override.default)
+        override_default = current.convert_value(override.default_to_raw())
         if isinstance(override_default, PropertyError):
             return override_default
         current = evolve(
diff --git a/openapi_python_client/parser/properties/protocol.py b/openapi_python_client/parser/properties/protocol.py
index e3f2ceaac..c1a57b066 100644
--- a/openapi_python_client/parser/properties/protocol.py
+++ b/openapi_python_client/parser/properties/protocol.py
@@ -177,3 +177,17 @@ def is_base_type(self) -> bool:
             ListProperty.__name__,
             UnionProperty.__name__,
         }
+
+    def default_to_raw(self) -> Any | None:
+        d = self.default
+        if not isinstance(d, Value):
+            return d
+        if d.startswith('"') or d.startswith("'"):
+            return d[1:-1]
+        if d == "true":
+            return True
+        if d == "false":
+            return False
+        if "." in d:
+            return float(d)
+        return int(d)
diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py
index 20c164015..9eb478ed2 100644
--- a/tests/test_parser/test_properties/test_init.py
+++ b/tests/test_parser/test_properties/test_init.py
@@ -11,7 +11,7 @@
     StringProperty,
     UnionProperty,
 )
-from openapi_python_client.parser.properties.protocol import ModelProperty
+from openapi_python_client.parser.properties.protocol import ModelProperty, Value
 from openapi_python_client.schema import DataType
 from openapi_python_client.utils import ClassName, PythonIdentifier
 
@@ -23,11 +23,11 @@ def test_is_base_type(self, string_property_factory):
         assert string_property_factory().is_base_type is True
 
     @pytest.mark.parametrize(
-        "required, expected",
-        (
+        "required,expected",
+        [
             (True, "str"),
             (False, "Union[Unset, str]"),
-        ),
+        ],
     )
     def test_get_type_string(self, string_property_factory, required, expected):
         p = string_property_factory(required=required)
@@ -374,6 +374,24 @@ def test_values_from_list_duplicate(self):
 
 
 class TestPropertyFromData:
+    @pytest.mark.parametrize(
+        "default_value, default_repr",
+        ["a", "'a'"],
+        ["'a'", "\\'a\\'"],
+        ['"a"', "'a'"],
+    )
+    def test_property_from_data_str_defaults(self, default_value, default_repr, config):
+        from openapi_python_client.parser.properties import Schemas, property_from_data
+        from openapi_python_client.schema import Schema
+
+        name = "my_string"
+        data = Schema(type="string", default=default_value)
+        prop, _ = property_from_data(name=name, data=data, schemas=Schemas(), config=config)
+        assert isinstance(prop, StringProperty)
+        assert isinstance(prop, Value)
+        assert str(prop) == default_repr
+        assert prop.default_to_raw == default_value
+
     def test_property_from_data_str_enum(self, enum_property_factory, config):
         from openapi_python_client.parser.properties import Class, Schemas, property_from_data
         from openapi_python_client.schema import Schema
@@ -747,13 +765,15 @@ def test_no_format(self, string_property_factory, required, config):
         from openapi_python_client.parser.properties import property_from_data
 
         name = "some_prop"
-        data = oai.Schema.model_construct(type="string", default='"hello world"')
+        data = oai.Schema.model_construct(type="string", default="hello world")
 
         p, _ = property_from_data(
             name=name, required=required, data=data, parent_name=None, config=config, schemas=Schemas()
         )
 
-        assert p == string_property_factory(name=name, required=required, default="'\\\\\"hello world\\\\\"'")
+        assert p == string_property_factory(
+            name=name, required=required, default=StringProperty.convert_value("hello world")
+        )
 
     def test_datetime_format(self, date_time_property_factory, config):
         from openapi_python_client.parser.properties import property_from_data
diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py
index 19717cf9a..8adc88e39 100644
--- a/tests/test_parser/test_properties/test_model_property.py
+++ b/tests/test_parser/test_properties/test_model_property.py
@@ -642,6 +642,33 @@ def test_conflicting_property_names(self, config):
         result = _process_properties(data=data, schemas=schemas, class_name="", config=config, roots={"root"})
         assert isinstance(result, PropertyError)
 
+    def test_merge_inline_objects(self, model_property_factory, enum_property_factory, config):
+        data = oai.Schema.model_construct(
+            allOf=[
+                oai.Schema.model_construct(
+                    type="object",
+                    properties={
+                        "prop1": oai.Schema.model_construct(type="string", default="a"),
+                    },
+                ),
+                oai.Schema.model_construct(
+                    type="object",
+                    properties={
+                        "prop1": oai.Schema.model_construct(type="string", description="desc"),
+                    },
+                ),
+            ]
+        )
+        schemas = Schemas()
+
+        result = _process_properties(data=data, schemas=schemas, class_name="", config=config, roots={"root"})
+        assert not isinstance(result, PropertyError)
+        assert len(result.optional_props) == 1
+        prop1 = result.optional_props[0]
+        assert isinstance(prop1, StringProperty)
+        assert prop1.description == "desc"
+        assert prop1.default == StringProperty.convert_value("a")
+
 
 class TestProcessModel:
     def test_process_model_error(self, mocker, model_property_factory, config):

From 4046d031128db20ba8d82a85b0a2d8f8368017a5 Mon Sep 17 00:00:00 2001
From: Eli Bishop <eli.bishop@benchling.com>
Date: Tue, 3 Sep 2024 15:39:05 -0700
Subject: [PATCH 19/19] workarounds for value/Value conversion inconsistencies

---
 .../parser/properties/int.py                  |  7 ++--
 .../parser/properties/protocol.py             | 10 +-----
 tests/conftest.py                             | 16 ++++++++-
 .../test_parser/test_properties/test_init.py  | 36 ++++++-------------
 .../test_properties/test_merge_properties.py  |  9 ++---
 .../test_parser/test_properties/test_union.py |  3 +-
 6 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/openapi_python_client/parser/properties/int.py b/openapi_python_client/parser/properties/int.py
index ab7173d3d..b34663a34 100644
--- a/openapi_python_client/parser/properties/int.py
+++ b/openapi_python_client/parser/properties/int.py
@@ -60,10 +60,13 @@ def convert_value(cls, value: Any) -> Value | None | PropertyError:
             return value
         if isinstance(value, str):
             try:
-                int(value)
+                value = float(value)
             except ValueError:
                 return PropertyError(f"Invalid int value: {value}")
-            return Value(value)
+        if isinstance(value, float):
+            as_int = int(value)
+            if value == as_int:
+                value = as_int
         if isinstance(value, int) and not isinstance(value, bool):
             return Value(str(value))
         return PropertyError(f"Invalid int value: {value}")
diff --git a/openapi_python_client/parser/properties/protocol.py b/openapi_python_client/parser/properties/protocol.py
index c1a57b066..14af0c976 100644
--- a/openapi_python_client/parser/properties/protocol.py
+++ b/openapi_python_client/parser/properties/protocol.py
@@ -182,12 +182,4 @@ def default_to_raw(self) -> Any | None:
         d = self.default
         if not isinstance(d, Value):
             return d
-        if d.startswith('"') or d.startswith("'"):
-            return d[1:-1]
-        if d == "true":
-            return True
-        if d == "false":
-            return False
-        if "." in d:
-            return float(d)
-        return int(d)
+        return eval(str(d))  # This should be safe because we've already escaped string values
diff --git a/tests/conftest.py b/tests/conftest.py
index 9b5e1490f..a15a15741 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,3 +1,4 @@
+import inspect
 from pathlib import Path
 from typing import Any, Callable, Dict, Union
 
@@ -21,6 +22,7 @@
     UnionProperty,
 )
 from openapi_python_client.parser.properties.float import FloatProperty
+from openapi_python_client.parser.properties.protocol import Value
 from openapi_python_client.schema.openapi_schema_pydantic import Parameter
 from openapi_python_client.schema.parameter_location import ParameterLocation
 
@@ -76,7 +78,19 @@ def _factory(**kwargs):
             if callable(defaults):
                 defaults = defaults(kwargs)
             kwargs = {**defaults, **kwargs}
-        return cls(**kwargs)
+        # It's very easy to accidentally set "default" to a raw value rather than a Value in our test
+        # code, which is never valid but mypy can't catch it for us. So we'll transform it here.
+        default_value = kwargs.get("default")
+        if default_value is not None and not isinstance(default_value, Value):
+            # Some of our property classes have convert_value as a class method; others have it as
+            # an instance method (because the logic requires knowing the state of the property). We
+            # can only call it here if it's a class method.
+            if inspect.ismethod(cls.convert_value) and cls.convert_value.__self__ is cls:
+                kwargs["default"] = cls.convert_value(default_value)
+            else:
+                kwargs["default"] = Value(str(default_value))
+        rv = cls(**kwargs)
+        return rv
 
     return _factory
 
diff --git a/tests/test_parser/test_properties/test_init.py b/tests/test_parser/test_properties/test_init.py
index 9eb478ed2..2efa27744 100644
--- a/tests/test_parser/test_properties/test_init.py
+++ b/tests/test_parser/test_properties/test_init.py
@@ -23,11 +23,11 @@ def test_is_base_type(self, string_property_factory):
         assert string_property_factory().is_base_type is True
 
     @pytest.mark.parametrize(
-        "required,expected",
-        [
+        "required, expected",
+        (
             (True, "str"),
             (False, "Union[Unset, str]"),
-        ],
+        ),
     )
     def test_get_type_string(self, string_property_factory, required, expected):
         p = string_property_factory(required=required)
@@ -374,24 +374,6 @@ def test_values_from_list_duplicate(self):
 
 
 class TestPropertyFromData:
-    @pytest.mark.parametrize(
-        "default_value, default_repr",
-        ["a", "'a'"],
-        ["'a'", "\\'a\\'"],
-        ['"a"', "'a'"],
-    )
-    def test_property_from_data_str_defaults(self, default_value, default_repr, config):
-        from openapi_python_client.parser.properties import Schemas, property_from_data
-        from openapi_python_client.schema import Schema
-
-        name = "my_string"
-        data = Schema(type="string", default=default_value)
-        prop, _ = property_from_data(name=name, data=data, schemas=Schemas(), config=config)
-        assert isinstance(prop, StringProperty)
-        assert isinstance(prop, Value)
-        assert str(prop) == default_repr
-        assert prop.default_to_raw == default_value
-
     def test_property_from_data_str_enum(self, enum_property_factory, config):
         from openapi_python_client.parser.properties import Class, Schemas, property_from_data
         from openapi_python_client.schema import Schema
@@ -771,9 +753,7 @@ def test_no_format(self, string_property_factory, required, config):
             name=name, required=required, data=data, parent_name=None, config=config, schemas=Schemas()
         )
 
-        assert p == string_property_factory(
-            name=name, required=required, default=StringProperty.convert_value("hello world")
-        )
+        assert p == string_property_factory(name=name, required=required, default="hello world")
 
     def test_datetime_format(self, date_time_property_factory, config):
         from openapi_python_client.parser.properties import property_from_data
@@ -786,7 +766,9 @@ def test_datetime_format(self, date_time_property_factory, config):
             name=name, required=required, data=data, schemas=Schemas(), config=config, parent_name=""
         )
 
-        assert p == date_time_property_factory(name=name, required=required, default=f"isoparse('{data.default}')")
+        assert p == date_time_property_factory(
+            name=name, required=required, default=Value(f"isoparse('{data.default}')")
+        )
 
     def test_datetime_bad_default(self, config):
         from openapi_python_client.parser.properties import property_from_data
@@ -814,7 +796,9 @@ def test_date_format(self, date_property_factory, config):
             name=name, required=required, data=data, schemas=Schemas(), config=config, parent_name=""
         )
 
-        assert p == date_property_factory(name=name, required=required, default=f"isoparse('{data.default}').date()")
+        assert p == date_property_factory(
+            name=name, required=required, default=Value(f"isoparse('{data.default}').date()")
+        )
 
     def test_date_format_bad_default(self, config):
         from openapi_python_client.parser.properties import property_from_data
diff --git a/tests/test_parser/test_properties/test_merge_properties.py b/tests/test_parser/test_properties/test_merge_properties.py
index d57176eb8..78f6bf89f 100644
--- a/tests/test_parser/test_properties/test_merge_properties.py
+++ b/tests/test_parser/test_properties/test_merge_properties.py
@@ -6,6 +6,7 @@
 from openapi_python_client.parser.properties.float import FloatProperty
 from openapi_python_client.parser.properties.int import IntProperty
 from openapi_python_client.parser.properties.merge_properties import merge_properties
+from openapi_python_client.parser.properties.protocol import Value
 from openapi_python_client.parser.properties.schemas import Class
 from openapi_python_client.parser.properties.string import StringProperty
 
@@ -68,14 +69,14 @@ def test_incompatible_types(
 
 def test_merge_int_with_float(int_property_factory, float_property_factory):
     int_prop = int_property_factory(description="desc1")
-    float_prop = float_property_factory(default="2", description="desc2")
+    float_prop = float_property_factory(default=2, description="desc2")
 
     assert merge_properties(int_prop, float_prop) == (
-        evolve(int_prop, default=float_prop.default, description=float_prop.description)
+        evolve(int_prop, default=Value("2"), description=float_prop.description)
     )
-    assert merge_properties(float_prop, int_prop) == (evolve(int_prop, default=float_prop.default))
+    assert merge_properties(float_prop, int_prop) == evolve(int_prop, default=Value("2"))
 
-    float_prop_with_non_int_default = evolve(float_prop, default="2.5")
+    float_prop_with_non_int_default = evolve(float_prop, default=Value("2.5"))
     error = merge_properties(int_prop, float_prop_with_non_int_default)
     assert isinstance(error, PropertyError), "Expected invalid default to error"
     assert error.detail == "Invalid int value: 2.5"
diff --git a/tests/test_parser/test_properties/test_union.py b/tests/test_parser/test_properties/test_union.py
index d8a5d762c..fa8d954d0 100644
--- a/tests/test_parser/test_properties/test_union.py
+++ b/tests/test_parser/test_properties/test_union.py
@@ -1,6 +1,7 @@
 import openapi_python_client.schema as oai
 from openapi_python_client.parser.errors import ParseError, PropertyError
 from openapi_python_client.parser.properties import Schemas, UnionProperty
+from openapi_python_client.parser.properties.protocol import Value
 from openapi_python_client.schema import DataType, ParameterLocation
 
 
@@ -19,7 +20,7 @@ def test_property_from_data_union(union_property_factory, date_time_property_fac
         name=name,
         required=required,
         inner_properties=[
-            string_property_factory(name=f"{name}_type_0", default="'a'"),
+            string_property_factory(name=f"{name}_type_0", default=Value("'a'")),
             date_time_property_factory(name=f"{name}_type_1"),
         ],
     )