From 74358f0957051606e476d32a845455f11802bf9b Mon Sep 17 00:00:00 2001 From: Tom van der Woerdt Date: Mon, 9 Nov 2026 16:43:40 -0500 Subject: [PATCH] Handle allOf(integer, number) correctly A generated jsonschema I tried to use with typify contained an `allOf:[integer,number]`, which typify thinks makes for an impossible intersection. However, the actual jsonschema says that `integer` is a subset of `number`, therefore `allOf(integer, number)` should be equal to `integer`. --- typify-impl/src/merge.rs | 92 ++++++++++++------- .../tests/schemas/numeric-intersection.json | 10 ++ typify/tests/schemas/numeric-intersection.rs | 77 ++++++++++++++++ 3 files changed, 148 insertions(+), 31 deletions(-) create mode 100644 typify/tests/schemas/numeric-intersection.json create mode 100644 typify/tests/schemas/numeric-intersection.rs diff --git a/typify-impl/src/merge.rs b/typify-impl/src/merge.rs index b7649019..c7e58c57 100644 --- a/typify-impl/src/merge.rs +++ b/typify-impl/src/merge.rs @@ -632,9 +632,9 @@ fn try_merge_schema_object_not( Ok(schema_object) } -/// Merge instance types which could be None (meaning type is valid), a +/// Merge instance types which could be None (meaning any type is valid), a /// singleton type, or an array of types. An error result indicates that the -/// types were non-overlappin and therefore incompatible. +/// types were non-overlapping and therefore incompatible. fn merge_so_instance_type( a: Option<&SingleOrVec>, b: Option<&SingleOrVec>, @@ -643,39 +643,31 @@ fn merge_so_instance_type( (None, None) => Ok(None), (None, other @ Some(_)) | (other @ Some(_), None) => Ok(other.cloned()), - // If each has a single type, it must match. - (Some(SingleOrVec::Single(aa)), Some(SingleOrVec::Single(bb))) => { - if aa == bb { - Ok(Some(SingleOrVec::Single(aa.clone()))) - } else { - Err(()) - } - } + (Some(aa), Some(bb)) => { + let expand = |types: &SingleOrVec| { + let mut types = match types { + SingleOrVec::Single(it) => BTreeSet::from([**it]), + SingleOrVec::Vec(types) => types.iter().copied().collect(), + }; + // JSON Schema numbers include integers. + if types.contains(&InstanceType::Number) { + types.insert(InstanceType::Integer); + } + types + }; - // If one has a single type and the other is an array, the type must - // appear in the array (and that's the resulting type). - (Some(SingleOrVec::Vec(types)), Some(SingleOrVec::Single(it))) - | (Some(SingleOrVec::Single(it)), Some(SingleOrVec::Vec(types))) => { - if types.contains(it) { - Ok(Some(SingleOrVec::Single(it.clone()))) - } else { - Err(()) - } - } + let mut types = expand(aa) + .intersection(&expand(bb)) + .copied() + .collect::>(); - // If both are arrays, we take the intersection; if the intersection is - // empty, we return an error. - (Some(SingleOrVec::Vec(aa)), Some(SingleOrVec::Vec(bb))) => { - let types = aa - .iter() - .collect::>() - .intersection(&bb.iter().collect::>()) - .cloned() - .cloned() - .collect::>(); + // Integer is redundant when Number is also in the union. + if types.contains(&InstanceType::Number) { + types.remove(&InstanceType::Integer); + } + let types = types.into_iter().collect::>(); match types.len() { - // No intersection 0 => Err(()), 1 => Ok(Some(types.into_iter().next().unwrap().into())), _ => Ok(Some(types.into())), @@ -1501,6 +1493,8 @@ mod tests { #[test] fn test_merge_instance_types() { + use InstanceType::{Integer, Null, Number, String}; + // Simple cases assert_eq!(merge_so_instance_type(None, None), Ok(None)); @@ -1579,6 +1573,42 @@ mod tests { ), Err(()), ); + + let cases = [ + (Number.into(), Integer.into(), Ok(Integer.into())), + (Integer.into(), vec![Number].into(), Ok(Integer.into())), + ( + Number.into(), + vec![Integer, Null].into(), + Ok(Integer.into()), + ), + ( + vec![Number, Null].into(), + vec![Integer, Null].into(), + Ok(vec![Null, Integer].into()), + ), + ( + Number.into(), + vec![Number, Integer].into(), + Ok(Number.into()), + ), + ( + vec![Number, Integer].into(), + vec![Number, Integer].into(), + Ok(Number.into()), + ), + (vec![Number].into(), vec![String, Null].into(), Err(())), + ]; + + for (a, b, expected) in cases { + for (a, b) in [(&a, &b), (&b, &a)] { + assert_eq!( + merge_so_instance_type(Some(a), Some(b)), + expected.clone().map(Some), + "{a:?} intersect {b:?}", + ); + } + } } #[test] diff --git a/typify/tests/schemas/numeric-intersection.json b/typify/tests/schemas/numeric-intersection.json new file mode 100644 index 00000000..dd8d95af --- /dev/null +++ b/typify/tests/schemas/numeric-intersection.json @@ -0,0 +1,10 @@ +{ + "title": "NumericIntersection", + "type": "object", + "required": ["n"], + "properties": { + "n": { + "allOf": [{"type": "number"}, {"type": "integer"}] + } + } +} diff --git a/typify/tests/schemas/numeric-intersection.rs b/typify/tests/schemas/numeric-intersection.rs new file mode 100644 index 00000000..10c5a5b5 --- /dev/null +++ b/typify/tests/schemas/numeric-intersection.rs @@ -0,0 +1,77 @@ +#![deny(warnings)] +#[doc = "`NumericIntersection`"] +#[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)] +pub struct NumericIntersection { + pub n: i64, +} +impl NumericIntersection { + pub fn builder() -> builder::NumericIntersection { + Default::default() + } +} +#[doc = " Types for composing complex structures."] +pub mod builder { + #[derive(Clone, Debug)] + pub struct NumericIntersection { + n: ::std::result::Result, + } + impl ::std::default::Default for NumericIntersection { + fn default() -> Self { + Self { + n: Err("no value supplied for n".to_string()), + } + } + } + impl NumericIntersection { + pub fn n(mut self, value: T) -> Self + where + T: ::std::convert::TryInto, + T::Error: ::std::fmt::Display, + { + self.n = value + .try_into() + .map_err(|e| format!("error converting supplied value for n: {e}")); + self + } + } + impl ::std::convert::TryFrom for super::NumericIntersection { + type Error = super::error::ConversionError; + fn try_from( + value: NumericIntersection, + ) -> ::std::result::Result { + Ok(Self { n: value.n? }) + } + } + impl ::std::convert::From for NumericIntersection { + fn from(value: super::NumericIntersection) -> Self { + Self { n: Ok(value.n) } + } + } +} +#[doc = " Error types."] +pub mod error { + #[doc = r" Error from a `TryFrom` or `FromStr` implementation."] + pub struct ConversionError(::std::borrow::Cow<'static, str>); + impl ::std::error::Error for ConversionError {} + impl ::std::fmt::Display for ConversionError { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> Result<(), ::std::fmt::Error> { + ::std::fmt::Display::fmt(&self.0, f) + } + } + impl ::std::fmt::Debug for ConversionError { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> Result<(), ::std::fmt::Error> { + ::std::fmt::Debug::fmt(&self.0, f) + } + } + impl From<&'static str> for ConversionError { + fn from(value: &'static str) -> Self { + Self(value.into()) + } + } + impl From for ConversionError { + fn from(value: String) -> Self { + Self(value.into()) + } + } +} +fn main() {}