Repository: oxidecomputer/typify
Pull request #412 (2023-09-28 15:37:43)

fix array merging and mutual exclusivity checks
In some cases, arrays that should fail to merge were returning a schema like this: `{ type: array, items: [false, false] }` ...which isn't that useful. This change takes a more deliberate approach to finding the intersection of array schemas including lowering the `maxItems` value up to the point where pairwise `items` array schema merging fails.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 64c31f7..d463d13 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1195,6 +1195,7 @@ dependencies = [
 name = "typify-impl"
 version = "0.0.14"
 dependencies = [
+ "env_logger",
  "expectorate",
  "heck",
  "log",
diff --git a/typify-impl/Cargo.toml b/typify-impl/Cargo.toml
index 125c259..8c80874 100644
--- a/typify-impl/Cargo.toml
+++ b/typify-impl/Cargo.toml
@@ -20,6 +20,7 @@ thiserror = "1.0"
 unicode-ident = "1.0.12"
 
 [dev-dependencies]
+env_logger = "0.10.0"
 expectorate = "1.0"
 paste = "1.0"
 rustfmt-wrapper = "0.2"
diff --git a/typify-impl/src/merge.rs b/typify-impl/src/merge.rs
index 42be322..dfccef4 100644
--- a/typify-impl/src/merge.rs
+++ b/typify-impl/src/merge.rs
@@ -21,39 +21,71 @@ pub(crate) fn merge_all(schemas: &[Schema], defs: &BTreeMap<RefKey, Schema>) ->
         panic!("merge_all requires at least two schemas")
     };
 
-    let start = merge_schema(a, b, defs);
-
-    ss.fold(start, |schema, other| merge_schema(&schema, other, defs))
+    try_merge_schema(a, b, defs)
+        .and_then(|start| {
+            ss.try_fold(start, |schema, other| {
+                try_merge_schema(&schema, other, defs)
+            })
+        })
+        .unwrap_or_else(|_| Schema::Bool(false))
 }
 
-/// Merge two schemas returning the resulting schema. If the two schemas are
-/// incompatible (i.e. if there is no data that can satisfy them both
-/// simultaneously) then this returns `Schema::Bool(false)`.
-fn merge_schema(a: &Schema, b: &Schema, defs: &BTreeMap<RefKey, Schema>) -> Schema {
-    match try_merge_schema(a, b, defs) {
-        Ok(schema) => schema,
-        // An error indicates that there is no value that satisfies both
-        // schemas.
-        Err(()) => Schema::Bool(false),
+/// Given two additionalItems schemas that might be None--which is equivalent
+/// to Schema::Bool(true)--this returns the appropriate value. This is only
+/// called in a situation where additionalItems are relevant, so we prefer
+/// `true` to the (equivalent) absence of the schema. In other words, this will
+/// never return None.
+fn merge_additional_items(
+    a: Option<&Schema>,
+    b: Option<&Schema>,
+    defs: &BTreeMap<RefKey, Schema>,
+) -> Option<Schema> {
+    match (a, b) {
+        (None, None) => Some(Schema::Bool(true)),
+        _ => merge_additional_properties(a, b, defs),
     }
 }
 
-fn merge_maybe_schema(
+/// Given two additionalProperties schemas that might be None--which is
+/// equivalent to Schema::Bool(true)--this returns the appropriate value. We
+/// prefer None to `true` for objects since the named properties are th main
+/// event.
+fn merge_additional_properties(
     a: Option<&Schema>,
     b: Option<&Schema>,
     defs: &BTreeMap<RefKey, Schema>,
 ) -> Option<Schema> {
     match (a, b) {
         (None, other) | (other, None) => other.cloned(),
-        (Some(aa), Some(bb)) => Some(merge_schema(aa, bb, defs)),
+        (Some(aa), Some(bb)) => {
+            Some(try_merge_schema(aa, bb, defs).unwrap_or_else(|_| Schema::Bool(false)))
+        }
     }
 }
 
+/// Merge two schemas returning the resulting schema. If the two schemas are
+/// incompatible (i.e. if there is no data that can satisfy them both
+/// simultaneously) then this returns Err.
 fn try_merge_schema(a: &Schema, b: &Schema, defs: &BTreeMap<RefKey, Schema>) -> Result<Schema, ()> {
     match (a, b) {
         (Schema::Bool(false), _) | (_, Schema::Bool(false)) => Ok(Schema::Bool(false)),
         (Schema::Bool(true), other) | (other, Schema::Bool(true)) => Ok(other.clone()),
 
+        // If we have two references to the same schema, that's easy!
+        (
+            Schema::Object(SchemaObject {
+                reference: Some(a_ref_name),
+                ..
+            }),
+            Schema::Object(SchemaObject {
+                reference: Some(b_ref_name),
+                ..
+            }),
+        ) if a_ref_name == b_ref_name => Ok(Schema::Object(SchemaObject {
+            reference: Some(a_ref_name.clone()),
+            ..Default::default()
+        })),
+
         // Resolve references here before we start to merge the objects.
         //
         // TODO: need to mitigate circular references so we don't go into a
@@ -129,6 +161,9 @@ fn merge_schema_object(
         extensions: Default::default(),
     };
 
+    // TODO if the merged schema is Default::default() then we should probably
+    // take some shortcut here...
+
     // If we have subschemas for either schema then we merge the body of the
     // two schemas and then do the appropriate merge with subschemas (i.e.
     // potentially twice). This is effectively an `allOf` between the merged
@@ -278,6 +313,8 @@ fn try_merge_with_subschemas(
                         // Skip if the merged schema is unsatisfiable.
                         let _ =
                             try_merge_schema(&schema_object.clone().into(), other, defs).ok()?;
+                        // TODO if the merged result is equal to either whole,
+                        // we should just use that.
                         Some(join_schema(&schema_object, other))
                     })
                     .collect(),
@@ -615,7 +652,7 @@ fn merge_so_array(
             }
 
             // The items and additional_items fields need to be considered
-            // together.
+            // together, and the results of merging can affect the max.
             //
             // - If items is a singleton, additional_items is ignored and all
             //   items in the array must obey the items schema.
@@ -632,18 +669,18 @@ fn merge_so_array(
             // items schema array is at least that long, additional_items is
             // irrelevant so we omit it. This case appears several times below.
 
-            let (items, additional_items) = match (
+            let (items, additional_items, max_items) = match (
                 (&aa.items, &aa.additional_items),
                 (&bb.items, &bb.additional_items),
             ) {
                 // Both items are none; items and additional_items are None.
-                ((None, _), (None, _)) => (None, None),
+                ((None, _), (None, _)) => (None, None, max_items),
 
                 // A None and a single-item; we can use the single item and
                 // additional_items are irrelevant.
                 ((None, _), (Some(SingleOrVec::Single(item)), _))
                 | ((Some(SingleOrVec::Single(item)), _), (None, _)) => {
-                    (Some(SingleOrVec::Single(item.clone())), None)
+                    (Some(SingleOrVec::Single(item.clone())), None, max_items)
                 }
 
                 // A None and a array of schemas; we can take the array,
@@ -657,10 +694,12 @@ fn merge_so_array(
                                 items.iter().take(max as usize).cloned().collect(),
                             )),
                             None,
+                            max_items,
                         ),
                         _ => (
                             Some(SingleOrVec::Vec(items.clone())),
                             additional_items.clone(),
+                            max_items,
                         ),
                     }
                 }
@@ -671,10 +710,11 @@ fn merge_so_array(
                     (Some(SingleOrVec::Single(aa_single)), _),
                     (Some(SingleOrVec::Single(bb_single)), _),
                 ) => (
-                    Some(SingleOrVec::Single(Box::new(merge_schema(
+                    Some(SingleOrVec::Single(Box::new(try_merge_schema(
                         aa_single, bb_single, defs,
-                    )))),
+                    )?))),
                     None,
+                    max_items,
                 ),
 
                 // A single item and an array of schemas. We merge the
@@ -686,32 +726,29 @@ fn merge_so_array(
                 | (
                     (Some(SingleOrVec::Vec(items)), additional_items),
                     (Some(SingleOrVec::Single(single)), _),
-                ) => match (max_items, items.len()) {
-                    (Some(max), len) if len >= max as usize => (
-                        Some(SingleOrVec::Vec(
-                            items
-                                .iter()
-                                .take(max as usize)
-                                .map(|item_schema| merge_schema(item_schema, single, defs))
-                                .collect(),
-                        )),
-                        None,
-                    ),
-                    _ => {
-                        let items = items
-                            .iter()
-                            .map(|item_schema| merge_schema(item_schema, single, defs))
-                            .collect();
+                ) => {
+                    let (items, allow_additional_items) = merge_items_array(
+                        items.iter().zip(repeat(single.as_ref())),
+                        min_items,
+                        max_items,
+                        defs,
+                    )?;
+
+                    if allow_additional_items {
                         let additional_items = additional_items.as_deref().map_or_else(
-                            || single.as_ref().clone(),
-                            |additional_schema| merge_schema(additional_schema, single, defs),
-                        );
+                            || Ok(single.as_ref().clone()),
+                            |additional_schema| try_merge_schema(additional_schema, single, defs),
+                        )?;
                         (
                             Some(SingleOrVec::Vec(items)),
                             Some(Box::new(additional_items)),
+                            max_items,
                         )
+                    } else {
+                        let len = items.len() as u32;
+                        (Some(SingleOrVec::Vec(items)), None, Some(len))
                     }
-                },
+                }
 
                 // We need to pairwise merge schemas--as many as the longer
                 // of the two items arrays, limited by the max size of the
@@ -726,46 +763,36 @@ fn merge_so_array(
                     let items_len = aa_items.len().max(bb_items.len());
 
                     // Note that one of these .chain(repeat(..)) statements is
-                    // always irrelevant because we will always .take(..) a
-                    // quantity less than or equal to the longest of the two
-                    // schema arrays; we just do them both and don't sweat it.
-                    let aa_items_iter = aa_items
-                        .iter()
-                        .map(Some)
-                        .chain(repeat(aa_additional_items.as_deref()));
-                    let bb_items_iter = bb_items
-                        .iter()
-                        .map(Some)
-                        .chain(repeat(bb_additional_items.as_deref()));
-                    let items_iter =
-                        aa_items_iter
-                            .zip(bb_items_iter)
-                            .map(|schemas| match schemas {
-                                (None, None) => unreachable!(),
-                                (None, Some(item)) => item.clone(),
-                                (Some(item), None) => item.clone(),
-                                (Some(aa_item), Some(bb_item)) => {
-                                    merge_schema(aa_item, bb_item, defs)
-                                }
-                            });
-
-                    match max_items {
-                        Some(max) if items_len <= max as usize => {
-                            let items = items_iter.take(max as usize).collect();
-                            (Some(SingleOrVec::Vec(items)), None)
-                        }
-
-                        _ => {
-                            let items = items_iter.take(items_len).collect();
-                            let additional_items = merge_maybe_schema(
-                                aa_additional_items.as_deref(),
-                                bb_additional_items.as_deref(),
-                                defs,
-                            )
-                            .map(Box::new);
-
-                            (Some(SingleOrVec::Vec(items)), additional_items)
-                        }
+                    // always irrelevant because we will always .take(..) the
+                    // shorter of the two (and may consume even fewer). We just
+                    // chain them both for simplicity and don't sweat it.
+                    let aa_items_iter = aa_items.iter().chain(repeat(
+                        aa_additional_items
+                            .as_deref()
+                            .unwrap_or_else(|| &Schema::Bool(true)),
+                    ));
+                    let bb_items_iter = bb_items.iter().chain(repeat(
+                        bb_additional_items
+                            .as_deref()
+                            .unwrap_or_else(|| &Schema::Bool(true)),
+                    ));
+                    let items_iter = aa_items_iter.zip(bb_items_iter).take(items_len);
+                    let (items, allow_additional_items) =
+                        merge_items_array(items_iter, min_items, max_items, defs)?;
+                    if allow_additional_items {
+                        let additional_items = merge_additional_items(
+                            aa_additional_items.as_deref(),
+                            bb_additional_items.as_deref(),
+                            defs,
+                        );
+                        (
+                            Some(SingleOrVec::Vec(items)),
+                            additional_items.map(Box::new),
+                            max_items,
+                        )
+                    } else {
+                        let len = items.len() as u32;
+                        (Some(SingleOrVec::Vec(items)), None, Some(len))
                     }
                 }
             };
@@ -782,6 +809,36 @@ fn merge_so_array(
     }
 }
 
+fn merge_items_array<'a>(
+    items_iter: impl Iterator<Item = (&'a Schema, &'a Schema)>,
+    min_items: Option<u32>,
+    max_items: Option<u32>,
+    defs: &BTreeMap<RefKey, Schema>,
+) -> Result<(Vec<Schema>, bool), ()> {
+    let mut items = Vec::new();
+    for (a, b) in items_iter {
+        match try_merge_schema(a, b, defs) {
+            Ok(schema) => {
+                items.push(schema);
+                if let Some(max) = max_items {
+                    if items.len() == max as usize {
+                        return Ok((items, false));
+                    }
+                }
+            }
+            Err(_) => {
+                let len = items.len() as u32;
+                if len < min_items.unwrap_or(1) {
+                    return Err(());
+                }
+                return Ok((items, false));
+            }
+        }
+    }
+
+    Ok((items, true))
+}
+
 /// Prefer Some over None and the result of `prefer` if both are Some.
 fn choose_value<T, F>(a: Option<T>, b: Option<T>, prefer: F) -> Option<T>
 where
@@ -808,8 +865,6 @@ fn merge_so_object(
                     filter_prop(name, a_schema, bb)
                 };
 
-                // TODO I'm going to copy/paste this so move it to a
-                // subroutine.
                 match resolved_schema {
                     // If a required field is incompatible with the
                     // other schema, this object is unsatisfiable.
@@ -828,8 +883,6 @@ fn merge_so_object(
                     // We handled the intersection above.
                     None
                 } else {
-                    // TODO I'm going to copy/paste this so move it to a
-                    // subroutine.
                     match filter_prop(name, b_schema, aa) {
                         // If a required field is incompatible with the
                         // other schema, this object is unsatisfiable.
@@ -850,47 +903,24 @@ fn merge_so_object(
             let additional_properties = merge_additional_properties(
                 aa.additional_properties.as_deref(),
                 bb.additional_properties.as_deref(),
-            );
+                defs,
+            )
+            .map(Box::new);
 
             let object_validation = ObjectValidation {
                 required,
                 properties,
                 additional_properties,
-                ..Default::default()
+                max_properties: Default::default(),     // TODO
+                min_properties: Default::default(),     // TODO
+                pattern_properties: Default::default(), // TODO
+                property_names: Default::default(),     // TODO
             };
             Ok(Some(object_validation.into()))
         }
     }
 }
 
-// TODO this is starting to feel redundant...
-fn merge_additional_properties(a: Option<&Schema>, b: Option<&Schema>) -> Option<Box<Schema>> {
-    match (a, b) {
-        (Some(Schema::Bool(true)), other)
-        | (None, other)
-        | (other, Some(Schema::Bool(true)))
-        | (other, None) => other.cloned().map(Box::new),
-
-        (Some(Schema::Bool(false)), _) | (_, Some(Schema::Bool(false))) => None,
-
-        (Some(aa @ Schema::Object(_)), Some(bb @ Schema::Object(_))) => Some(Box::new(
-            SchemaObject {
-                subschemas: Some(Box::new(SubschemaValidation {
-                    // TODO it would be a good idea to merge these now rather than
-                    // deferring that since the schemas might be unresolvable i.e.
-                    // they might have no intersection. However, a non-true/false/
-                    // absent additionalProperties within an allOf is an uncommon
-                    // pattern so this is likely good enough for the moment.
-                    all_of: Some(vec![aa.clone(), bb.clone()]),
-                    ..Default::default()
-                })),
-                ..Default::default()
-            }
-            .into(),
-        )),
-    }
-}
-
 fn filter_prop(
     name: &str,
     prop_schema: &Schema,
@@ -947,6 +977,7 @@ mod tests {
     use crate::merge::merge_so_instance_type;
 
     use super::try_merge_schema;
+
     #[test]
     fn test_simple_merge() {
         let a = json!({
@@ -988,8 +1019,6 @@ mod tests {
 
         let merged = try_merge_schema(&a, &b, &BTreeMap::default()).unwrap();
 
-        println!("{}", serde_json::to_string_pretty(&merged).unwrap());
-
         assert_eq!(merged, ab);
     }
 
@@ -1110,8 +1139,6 @@ mod tests {
 
         let merged = try_merge_schema(&a, &b, &BTreeMap::default()).unwrap();
 
-        println!("{}", serde_json::to_string_pretty(&merged).unwrap());
-
         assert_eq!(merged, a);
     }
 
@@ -1196,4 +1223,216 @@ mod tests {
             Err(()),
         );
     }
+
+    #[test]
+    fn test_array_fail() {
+        let a = json!({
+            "type": "array",
+            "items": { "type": "integer" }
+        });
+        let b = json!({
+            "type": "array",
+            "items": { "type": "string" }
+        });
+        let a = serde_json::from_value(a).unwrap();
+        let b = serde_json::from_value(b).unwrap();
+
+        let ab = try_merge_schema(&a, &b, &Default::default());
+
+        assert!(ab.is_err());
+
+        let a = json!({
+            "type": "array",
+            "items": [{ "type": "integer" }, {"type": "object"}]
+        });
+        let b = json!({
+            "type": "array",
+            "items": { "type": "string" }
+        });
+        let a = serde_json::from_value(a).unwrap();
+        let b = serde_json::from_value(b).unwrap();
+
+        let ab = try_merge_schema(&a, &b, &Default::default());
+
+        assert!(ab.is_err());
+
+        let a = json!({
+            "type": "array",
+            "items": [{ "type": "integer" }, {"type": "object"}]
+        });
+        let b = json!({
+            "type": "array",
+            "items": [{ "type": "string" }, { "type": "object" }]
+        });
+        let a = serde_json::from_value(a).unwrap();
+        let b = serde_json::from_value(b).unwrap();
+
+        let ab = try_merge_schema(&a, &b, &Default::default());
+
+        assert!(
+            ab.is_err(),
+            "{}",
+            serde_json::to_string_pretty(&ab).unwrap(),
+        );
+
+        let a = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "minItems": 3,
+            "maxItems": 4
+        });
+        let b = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "additionalItems": { "type": "string" },
+            "maxItems": 100
+        });
+        let a = serde_json::from_value(a).unwrap();
+        let b = serde_json::from_value(b).unwrap();
+
+        let ab = try_merge_schema(&a, &b, &Default::default());
+
+        assert!(
+            ab.is_err(),
+            "{}",
+            serde_json::to_string_pretty(&ab).unwrap(),
+        );
+    }
+
+    #[test]
+    fn test_array_good1() {
+        let a = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "maxItems": 4
+        });
+        let b = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "additionalItems": { "type": "integer" },
+            "maxItems": 3
+        });
+        let ab = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "maxItems": 3
+        });
+
+        let a = serde_json::from_value(a).unwrap();
+        let b = serde_json::from_value(b).unwrap();
+        let ab = serde_json::from_value(ab).unwrap();
+
+        let merged = try_merge_schema(&a, &b, &BTreeMap::default()).unwrap();
+        assert_eq!(
+            merged,
+            ab,
+            "{}",
+            serde_json::to_string_pretty(&merged).unwrap(),
+        )
+    }
+
+    #[test]
+    fn test_array_good2() {
+        let a = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "maxItems": 4
+        });
+        let b = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "additionalItems": true,
+        });
+        let ab = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "additionalItems": true,
+            "maxItems": 4
+        });
+
+        let a = serde_json::from_value(a).unwrap();
+        let b = serde_json::from_value(b).unwrap();
+        let ab = serde_json::from_value(ab).unwrap();
+
+        let merged = try_merge_schema(&a, &b, &BTreeMap::default()).unwrap();
+        assert_eq!(
+            merged,
+            ab,
+            "{}",
+            serde_json::to_string_pretty(&merged).unwrap(),
+        )
+    }
+
+    #[test]
+    fn test_array_good3() {
+        let a = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "maxItems": 4
+        });
+        let b = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" },
+                { "type": "string" }
+            ],
+            "additionalItems": true,
+        });
+        let ab = json!({
+            "type": "array",
+            "items": [
+                { "type": "integer" },
+                { "type": "integer" }
+            ],
+            "maxItems": 2
+        });
+
+        let a = serde_json::from_value(a).unwrap();
+        let b = serde_json::from_value(b).unwrap();
+        let ab = serde_json::from_value(ab).unwrap();
+
+        let merged = try_merge_schema(&a, &b, &BTreeMap::default()).unwrap();
+        assert_eq!(
+            merged,
+            ab,
+            "{}",
+            serde_json::to_string_pretty(&merged).unwrap(),
+        )
+    }
 }
diff --git a/typify-impl/src/util.rs b/typify-impl/src/util.rs
index b7d0f7e..2195515 100644
--- a/typify-impl/src/util.rs
+++ b/typify-impl/src/util.rs
@@ -2,6 +2,7 @@
 
 use std::collections::{BTreeSet, HashSet};
 
+use log::debug;
 use schemars::schema::{
     ArrayValidation, InstanceType, Metadata, ObjectValidation, Schema, SchemaObject, SingleOrVec,
     StringValidation, SubschemaValidation,
@@ -33,6 +34,14 @@ pub(crate) fn metadata_title_and_description(metadata: &Option<Box<Metadata>>) -
         })
 }
 
+/// Check if all schemas are mutually exclusive.
+///
+/// TODO This is used to turn an `anyOf` into a `oneOf` (i.e. if true). However
+/// a better approach is probably to use the (newish) merge logic to try to
+/// explode an `anyOf` into its 2^N possibilities. Some of those may be invalid
+/// in which case we'll end up looking like a `oneOf`. The logic of merging is
+/// conceptually identical to the logic below that validates **if** the schemas
+/// **could** be merged (i.e. if they're compatible).
 pub(crate) fn all_mutually_exclusive(
     subschemas: &[Schema],
     definitions: &schemars::Map<RefKey, Schema>,
@@ -412,53 +421,34 @@ fn array_schemas_mutually_exclusive(
             .iter()
             .any(|schema| schemas_mutually_exclusive(schema, single)),
 
-        // Tuples with different numbers of elements are incompatible.
-        (
-            ArrayValidation {
-                items: Some(SingleOrVec::Vec(a_items)),
-                additional_items: None,
-                max_items: Some(a_max_items),
-                min_items: Some(a_min_items),
-                unique_items: None,
-                contains: None,
-            },
-            ArrayValidation {
-                items: Some(SingleOrVec::Vec(b_items)),
-                additional_items: None,
-                max_items: Some(b_max_items),
-                min_items: Some(b_min_items),
-                unique_items: None,
-                contains: None,
-            },
-        ) if a_max_items == a_min_items
-            && *a_max_items as usize == a_items.len()
-            && b_max_items == b_min_items
-            && *b_max_items as usize == b_items.len() =>
-        {
-            a_max_items != b_max_items
-        }
+        (aa, bb) => {
+            // If min > max then these schemas are incompatible.
+            match (&aa.max_items, &bb.min_items) {
+                (Some(max), Some(min)) if min > max => return true,
+                _ => (),
+            }
+            match (&bb.max_items, &aa.min_items) {
+                (Some(max), Some(min)) if min > max => return true,
+                _ => (),
+            }
 
-        // Plain, vanilla arrays.
-        (
-            ArrayValidation {
-                items: Some(SingleOrVec::Single(a_items)),
-                additional_items: None,
-                max_items: None,
-                min_items: None,
-                unique_items: None,
-                contains: None,
-            },
-            ArrayValidation {
-                items: Some(SingleOrVec::Single(b_items)),
-                additional_items: None,
-                max_items: None,
-                min_items: None,
-                unique_items: None,
-                contains: None,
-            },
-        ) => schemas_mutually_exclusive(a_items, b_items),
+            match (&aa.items, &aa.max_items, &bb.items, &bb.max_items) {
+                //
+                (Some(SingleOrVec::Single(a_items)), _, Some(SingleOrVec::Single(b_items)), _)
+                    if schemas_mutually_exclusive(a_items, b_items) =>
+                {
+                    return true;
+                }
 
-        (aa, bb) => todo!("{:#?} {:#?}", aa, bb),
+                _ => (),
+            }
+            debug!(
+                "givin up on mututal exclusivity check {} {}",
+                serde_json::to_string_pretty(aa).unwrap(),
+                serde_json::to_string_pretty(bb).unwrap(),
+            );
+            false
+        }
     }
 }
 


Test patch:
diff --git a/typify-impl/tests/github.out b/typify-impl/tests/github.out
index cab7c19..b037d05 100644
--- a/typify-impl/tests/github.out
+++ b/typify-impl/tests/github.out
@@ -8478,6 +8478,7 @@ impl From<&DiscussionAnsweredDiscussion> for DiscussionAnsweredDiscussion {
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
 pub struct DiscussionAnsweredDiscussionAnswerChosenBy {
     pub avatar_url: String,
     #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -16163,6 +16164,7 @@ impl std::convert::TryFrom<String> for InstallationSuspendInstallationRepository
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
 pub struct InstallationSuspendInstallationSuspendedBy {
     pub avatar_url: String,
     #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -19163,7 +19165,7 @@ pub struct IssueCommentCreatedIssue {
     pub events_url: String,
     pub html_url: String,
     pub id: i64,
-    pub labels: Vec<IssueCommentCreatedIssueLabelsItem>,
+    pub labels: Vec<Label>,
     pub labels_url: String,
     pub locked: bool,
     pub milestone: Option<Milestone>,
@@ -19244,7 +19246,7 @@ impl std::convert::TryFrom<String> for IssueCommentCreatedIssueActiveLockReason
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
-#[serde(untagged)]
+#[serde(untagged, deny_unknown_fields)]
 pub enum IssueCommentCreatedIssueAssignee {
     Variant0 {
         avatar_url: String,
@@ -19330,21 +19332,7 @@ impl std::convert::TryFrom<String> for IssueCommentCreatedIssueAssigneeVariant0T
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
-pub struct IssueCommentCreatedIssueLabelsItem {
-    pub color: String,
-    pub default: bool,
-    pub description: Option<String>,
-    pub id: i64,
-    pub name: String,
-    pub node_id: String,
-    pub url: String,
-}
-impl From<&IssueCommentCreatedIssueLabelsItem> for IssueCommentCreatedIssueLabelsItem {
-    fn from(value: &IssueCommentCreatedIssueLabelsItem) -> Self {
-        value.clone()
-    }
-}
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
 pub struct IssueCommentCreatedIssuePullRequest {
     pub diff_url: String,
     pub html_url: String,
@@ -19483,7 +19471,7 @@ pub struct IssueCommentDeletedIssue {
     pub events_url: String,
     pub html_url: String,
     pub id: i64,
-    pub labels: Vec<IssueCommentDeletedIssueLabelsItem>,
+    pub labels: Vec<Label>,
     pub labels_url: String,
     pub locked: bool,
     pub milestone: Option<Milestone>,
@@ -19564,7 +19552,7 @@ impl std::convert::TryFrom<String> for IssueCommentDeletedIssueActiveLockReason
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
-#[serde(untagged)]
+#[serde(untagged, deny_unknown_fields)]
 pub enum IssueCommentDeletedIssueAssignee {
     Variant0 {
         avatar_url: String,
@@ -19650,21 +19638,7 @@ impl std::convert::TryFrom<String> for IssueCommentDeletedIssueAssigneeVariant0T
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
-pub struct IssueCommentDeletedIssueLabelsItem {
-    pub color: String,
-    pub default: bool,
-    pub description: Option<String>,
-    pub id: i64,
-    pub name: String,
-    pub node_id: String,
-    pub url: String,
-}
-impl From<&IssueCommentDeletedIssueLabelsItem> for IssueCommentDeletedIssueLabelsItem {
-    fn from(value: &IssueCommentDeletedIssueLabelsItem) -> Self {
-        value.clone()
-    }
-}
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
 pub struct IssueCommentDeletedIssuePullRequest {
     pub diff_url: String,
     pub html_url: String,
@@ -19827,7 +19801,7 @@ pub struct IssueCommentEditedIssue {
     pub events_url: String,
     pub html_url: String,
     pub id: i64,
-    pub labels: Vec<IssueCommentEditedIssueLabelsItem>,
+    pub labels: Vec<Label>,
     pub labels_url: String,
     pub locked: bool,
     pub milestone: Option<Milestone>,
@@ -19908,7 +19882,7 @@ impl std::convert::TryFrom<String> for IssueCommentEditedIssueActiveLockReason {
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
-#[serde(untagged)]
+#[serde(untagged, deny_unknown_fields)]
 pub enum IssueCommentEditedIssueAssignee {
     Variant0 {
         avatar_url: String,
@@ -19994,21 +19968,7 @@ impl std::convert::TryFrom<String> for IssueCommentEditedIssueAssigneeVariant0Ty
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
-pub struct IssueCommentEditedIssueLabelsItem {
-    pub color: String,
-    pub default: bool,
-    pub description: Option<String>,
-    pub id: i64,
-    pub name: String,
-    pub node_id: String,
-    pub url: String,
-}
-impl From<&IssueCommentEditedIssueLabelsItem> for IssueCommentEditedIssueLabelsItem {
-    fn from(value: &IssueCommentEditedIssueLabelsItem) -> Self {
-        value.clone()
-    }
-}
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
 pub struct IssueCommentEditedIssuePullRequest {
     pub diff_url: String,
     pub html_url: String,
@@ -21402,11 +21362,12 @@ impl std::convert::TryFrom<String> for IssuesMilestonedIssueActiveLockReason {
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
 pub struct IssuesMilestonedIssueMilestone {
     pub closed_at: Option<chrono::DateTime<chrono::offset::Utc>>,
     pub closed_issues: i64,
     pub created_at: chrono::DateTime<chrono::offset::Utc>,
-    pub creator: IssuesMilestonedIssueMilestoneCreator,
+    pub creator: User,
     pub description: Option<String>,
     pub due_on: Option<chrono::DateTime<chrono::offset::Utc>>,
     pub html_url: String,
@@ -21425,88 +21386,6 @@ impl From<&IssuesMilestonedIssueMilestone> for IssuesMilestonedIssueMilestone {
         value.clone()
     }
 }
-#[derive(Clone, Debug, Deserialize, Serialize)]
-pub struct IssuesMilestonedIssueMilestoneCreator {
-    pub avatar_url: String,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub email: Option<String>,
-    pub events_url: String,
-    pub followers_url: String,
-    pub following_url: String,
-    pub gists_url: String,
-    pub gravatar_id: String,
-    pub html_url: String,
-    pub id: i64,
-    pub login: String,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub name: Option<String>,
-    pub node_id: String,
-    pub organizations_url: String,
-    pub received_events_url: String,
-    pub repos_url: String,
-    pub site_admin: bool,
-    pub starred_url: String,
-    pub subscriptions_url: String,
-    #[serde(rename = "type")]
-    pub type_: IssuesMilestonedIssueMilestoneCreatorType,
-    pub url: String,
-}
-impl From<&IssuesMilestonedIssueMilestoneCreator> for IssuesMilestonedIssueMilestoneCreator {
-    fn from(value: &IssuesMilestonedIssueMilestoneCreator) -> Self {
-        value.clone()
-    }
-}
-#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
-pub enum IssuesMilestonedIssueMilestoneCreatorType {
-    Bot,
-    User,
-    Organization,
-}
-impl From<&IssuesMilestonedIssueMilestoneCreatorType>
-    for IssuesMilestonedIssueMilestoneCreatorType
-{
-    fn from(value: &IssuesMilestonedIssueMilestoneCreatorType) -> Self {
-        value.clone()
-    }
-}
-impl ToString for IssuesMilestonedIssueMilestoneCreatorType {
-    fn to_string(&self) -> String {
-        match *self {
-            Self::Bot => "Bot".to_string(),
-            Self::User => "User".to_string(),
-            Self::Organization => "Organization".to_string(),
-        }
-    }
-}
-impl std::str::FromStr for IssuesMilestonedIssueMilestoneCreatorType {
-    type Err = &'static str;
-    fn from_str(value: &str) -> Result<Self, &'static str> {
-        match value {
-            "Bot" => Ok(Self::Bot),
-            "User" => Ok(Self::User),
-            "Organization" => Ok(Self::Organization),
-            _ => Err("invalid value"),
-        }
-    }
-}
-impl std::convert::TryFrom<&str> for IssuesMilestonedIssueMilestoneCreatorType {
-    type Error = &'static str;
-    fn try_from(value: &str) -> Result<Self, &'static str> {
-        value.parse()
-    }
-}
-impl std::convert::TryFrom<&String> for IssuesMilestonedIssueMilestoneCreatorType {
-    type Error = &'static str;
-    fn try_from(value: &String) -> Result<Self, &'static str> {
-        value.parse()
-    }
-}
-impl std::convert::TryFrom<String> for IssuesMilestonedIssueMilestoneCreatorType {
-    type Error = &'static str;
-    fn try_from(value: String) -> Result<Self, &'static str> {
-        value.parse()
-    }
-}
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
 pub enum IssuesMilestonedIssueMilestoneState {
     #[serde(rename = "open")]
@@ -38901,6 +38780,7 @@ impl std::convert::TryFrom<String> for WorkflowJobStartedWorkflowJobStatus {
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
 pub struct WorkflowJobStartedWorkflowJobStepsItem {
     pub completed_at: (),
     pub conclusion: (),
diff --git a/typify-impl/tests/vega.out b/typify-impl/tests/vega.out
index 711ffb9..a4ce11c 100644
--- a/typify-impl/tests/vega.out
+++ b/typify-impl/tests/vega.out
@@ -33140,8 +33140,8 @@ pub struct MarkGroup {
     pub data: Vec<Data>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub description: Option<String>,
-    #[serde(default, skip_serializing_if = "serde_json::Map::is_empty")]
-    pub encode: serde_json::Map<String, serde_json::Value>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub encode: Option<Encode>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub from: Option<MarkGroupFrom>,
     #[serde(default, skip_serializing_if = "Option::is_none")]