Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,17 +339,10 @@ types. Examples from users are very helpful in this regard.

### Bounded numbers

Bounded numbers aren't very well handled. Consider, for example, the schema:

```json
{
"type": "integer",
"minimum": 1,
"maximum": 6
}
```

The resulting types won't enforce those value constraints.
Named integer types with sub-range bounds (e.g., a `uint8` with `maximum: 63`)
generate constrained newtypes with `TryFrom` validation. Anonymous integer
properties with bounds that don't match a standard Rust type are not yet
constrained.
Comment on lines +342 to +345
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Truly anonymous types are rare (and arguably bugs). Note that this is under the WIP heading so we may want to rephrase with that in mind.


### Configurable dependencies

Expand Down
355 changes: 342 additions & 13 deletions typify-impl/src/convert.rs

Large diffs are not rendered by default.

105 changes: 102 additions & 3 deletions typify-impl/src/type_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ pub(crate) enum TypeEntryNewtypeConstraints {
min_length: Option<u32>,
pattern: Option<String>,
},
Integer {
min: Option<i128>,
max: Option<i128>,
},
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
Expand Down Expand Up @@ -540,6 +544,39 @@ impl TypeEntryNewtype {
extra_attrs: type_patch.attrs,
}
}

pub(crate) fn from_metadata_with_integer_validation(
type_space: &TypeSpace,
type_name: Name,
metadata: &Option<Box<Metadata>>,
type_id: TypeId,
min: Option<i128>,
max: Option<i128>,
schema: Schema,
) -> TypeEntry {
let name = get_type_name(&type_name, metadata)
.expect("type name required for constrained integer newtype");
let rename = None;
let description = metadata_description(metadata);

let type_patch = TypePatch::new(type_space, name);

let details = TypeEntryDetails::Newtype(Self {
name: type_patch.name,
rename,
description,
default: None,
type_id,
constraints: TypeEntryNewtypeConstraints::Integer { min, max },
schema: SchemaWrapper(schema),
});

TypeEntry {
details,
extra_derives: type_patch.derives,
extra_attrs: type_patch.attrs,
}
}
}

impl From<TypeEntryDetails> for TypeEntry {
Expand Down Expand Up @@ -1378,12 +1415,16 @@ impl TypeEntry {
let inner_type_name = inner_type.type_ident(type_space, &None);

let is_str = matches!(inner_type.details, TypeEntryDetails::String);
let is_int = matches!(inner_type.details, TypeEntryDetails::Integer(_));

// If this is just a wrapper around a string, we can derive some more
// useful traits.
if is_str {
// If this is just a wrapper around a string or integer, we can derive
// some more useful traits.
if is_str || is_int {
derive_set.extend(["PartialOrd", "Ord", "PartialEq", "Eq", "Hash"]);
}
if is_int {
derive_set.insert("Copy");
}
Comment on lines +1420 to +1427
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest

Suggested change
// If this is just a wrapper around a string or integer, we can derive
// some more useful traits.
if is_str || is_int {
derive_set.extend(["PartialOrd", "Ord", "PartialEq", "Eq", "Hash"]);
}
if is_int {
derive_set.insert("Copy");
}
// If this is just a wrapper around a string, we can derive
// some more useful traits.
if is_str {
derive_set.extend(["PartialOrd", "Ord", "PartialEq", "Eq", "Hash"]);
}
if is_int {
derive_set.extend(["PartialOrd", "Ord", "PartialEq", "Eq", "Hash", "Copy"]);
}


let constraint_impl = match constraints {
// In the unconstrained case we proxy impls through the inner type.
Expand Down Expand Up @@ -1629,6 +1670,64 @@ impl TypeEntry {
}
}
}

TypeEntryNewtypeConstraints::Integer { min, max } => {
let min_check = min.map(|v| {
let lit = proc_macro2::Literal::i128_unsuffixed(v);
let err = format!("value must be at least {}", v);
quote! {
if value < #lit {
return Err(#err.into());
}
}
});
let max_check = max.map(|v| {
let lit = proc_macro2::Literal::i128_unsuffixed(v);
let err = format!("value must be at most {}", v);
quote! {
if value > #lit {
return Err(#err.into());
}
}
});

// We're going to impl Deserialize so we can remove it
// from the set of derived impls.
derive_set.remove("::serde::Deserialize");

quote! {
impl ::std::convert::TryFrom<#inner_type_name> for #type_name {
type Error = self::error::ConversionError;

fn try_from(
value: #inner_type_name,
) -> ::std::result::Result<Self, self::error::ConversionError>
{
#min_check
#max_check
Ok(Self(value))
}
}

impl<'de> ::serde::Deserialize<'de> for #type_name {
fn deserialize<D>(
deserializer: D,
) -> ::std::result::Result<Self, D::Error>
where
D: ::serde::Deserializer<'de>,
{
Self::try_from(
<#inner_type_name>::deserialize(deserializer)?,
)
.map_err(|e| {
<D::Error as ::serde::de::Error>::custom(
e.to_string(),
)
})
}
}
}
}
};

// If there are no constraints, let consumers directly access the value.
Expand Down
2 changes: 1 addition & 1 deletion typify-test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = "2021"

[dependencies]
regress = { workspace = true }
serde = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }

[build-dependencies]
Expand Down
68 changes: 68 additions & 0 deletions typify-test/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,71 @@ impl JsonSchema for NonAsciiChars {
}
}

/// A uint8 bounded to [1, 63].
struct BoundedUint;
impl JsonSchema for BoundedUint {
fn schema_name() -> String {
"BoundedUint".to_string()
}

fn json_schema(_: &mut schemars::gen::SchemaGenerator) -> Schema {
schemars::schema::SchemaObject {
instance_type: Some(schemars::schema::InstanceType::Integer.into()),
format: Some("uint8".to_string()),
number: Some(Box::new(schemars::schema::NumberValidation {
minimum: Some(1.0),
maximum: Some(63.0),
..Default::default()
})),
..Default::default()
}
.into()
}
}

/// An int16 bounded to [-50, -10].
struct BoundedNegative;
impl JsonSchema for BoundedNegative {
fn schema_name() -> String {
"BoundedNegative".to_string()
}

fn json_schema(_: &mut schemars::gen::SchemaGenerator) -> Schema {
schemars::schema::SchemaObject {
instance_type: Some(schemars::schema::InstanceType::Integer.into()),
format: Some("int16".to_string()),
number: Some(Box::new(schemars::schema::NumberValidation {
minimum: Some(-50.0),
maximum: Some(-10.0),
..Default::default()
})),
..Default::default()
}
.into()
}
}

/// No format hint, bounds [0, 63] -- should infer u8.
struct InferredUint;
impl JsonSchema for InferredUint {
fn schema_name() -> String {
"InferredUint".to_string()
}

fn json_schema(_: &mut schemars::gen::SchemaGenerator) -> Schema {
schemars::schema::SchemaObject {
instance_type: Some(schemars::schema::InstanceType::Integer.into()),
number: Some(Box::new(schemars::schema::NumberValidation {
minimum: Some(0.0),
maximum: Some(63.0),
..Default::default()
})),
..Default::default()
}
.into()
}
}

struct Pancakes;
impl JsonSchema for Pancakes {
fn schema_name() -> String {
Expand Down Expand Up @@ -132,6 +197,9 @@ fn main() {
LoginName::add(&mut type_space);
NonAsciiChars::add(&mut type_space);
UnknownFormat::add(&mut type_space);
BoundedUint::add(&mut type_space);
BoundedNegative::add(&mut type_space);
InferredUint::add(&mut type_space);
ipnetwork::IpNetwork::add(&mut type_space);

let contents =
Expand Down
61 changes: 61 additions & 0 deletions typify-test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,67 @@ fn test_string_constraints_for_non_ascii_chars() {
assert!(NonAsciiChars::try_from("🍔").is_err());
}

#[test]
fn test_integer_bounds_try_from() {
// BoundedUint: u8 in [1, 63].
assert!(BoundedUint::try_from(0u8).is_err());
assert!(BoundedUint::try_from(1u8).is_ok());
assert!(BoundedUint::try_from(63u8).is_ok());
assert!(BoundedUint::try_from(64u8).is_err());
assert!(BoundedUint::try_from(255u8).is_err());

// Verify the inner value round-trips.
let v = BoundedUint::try_from(42u8).unwrap();
assert_eq!(*v, 42u8);
assert_eq!(u8::from(v), 42u8);
}

#[test]
fn test_integer_bounds_negative() {
// BoundedNegative: i16 in [-50, -10].
assert!(BoundedNegative::try_from(-51i16).is_err());
assert!(BoundedNegative::try_from(-50i16).is_ok());
assert!(BoundedNegative::try_from(-10i16).is_ok());
assert!(BoundedNegative::try_from(-9i16).is_err());
assert!(BoundedNegative::try_from(0i16).is_err());
}

#[test]
fn test_integer_bounds_inferred() {
// InferredUint: no format hint, [0, 63] -> inferred u8.
assert!(InferredUint::try_from(0u8).is_ok());
assert!(InferredUint::try_from(63u8).is_ok());
assert!(InferredUint::try_from(64u8).is_err());
}

#[test]
fn test_integer_bounds_serde() {
// Deserialization should enforce bounds.
let ok: Result<BoundedUint, _> = serde_json::from_str("42");
assert!(ok.is_ok());
assert_eq!(*ok.unwrap(), 42u8);

let too_high: Result<BoundedUint, _> = serde_json::from_str("64");
assert!(too_high.is_err());

let too_low: Result<BoundedUint, _> = serde_json::from_str("0");
assert!(too_low.is_err());

// Serialization should produce the bare integer.
let v = BoundedUint::try_from(42u8).unwrap();
assert_eq!(serde_json::to_string(&v).unwrap(), "42");

// Negative bounds.
let ok: Result<BoundedNegative, _> = serde_json::from_str("-25");
assert!(ok.is_ok());

let too_low: Result<BoundedNegative, _> = serde_json::from_str("-51");
assert!(too_low.is_err());

let too_high: Result<BoundedNegative, _> = serde_json::from_str("-9");
assert!(too_high.is_err());
}

#[test]
fn test_unknown_format() {
// An unknown format string should just render as a string.
Expand Down
Loading