fixes #589 allow min and max to be within range #733
Conversation
|
Hey @ahl does this fix look ok to you? |
| && (min.map(|f| f.ge(imin)).unwrap_or(false)) | ||
| && (max.map(|f| f.le(imax)).unwrap_or(false)) |
There was a problem hiding this comment.
this doesn't seem quite right. Previously the logic said: "if it's None OR is the specified bound". Now this says "if it's Some AND is within the specified bound". So this will only apply if both the minimum and maximum values are set in the schema.
I'm not sure of the best path forward here. I tried adjusting this, but then the NonZeroUXX tests failed. One option might be to specify add formats for the NonZeroXX types in the table above and then here to filter rather than find.
| @@ -2115,6 +2116,35 @@ mod tests { | |||
| validate_builtin!(std::collections::BTreeSet<u32>); | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
I'd suggest deleting this test and adding some types to typify/tests/schemas/simple-types.json. These are typically easier to create, easier to update, and easier to understand.
| @@ -1011,14 +1011,14 @@ impl TypeSpace { | |||
| ]; | |||
|
|
|||
| if let Some(format) = format { | |||
| if let Some((_, ty, imin, imax)) = formats | |||
| if let Some((_fmt, ty, imin, imax)) = formats | |||
| .iter() | |||
| .find(|(int_format, _, _, _)| int_format == format) | |||
| { | |||
| // If the type matches with other constraints, we're done. | |||
There was a problem hiding this comment.
This comment no longer seems to accurately describe the code below it.
|
Thanks, I will take a look through them |
fixes #589
Allow min and max to be within range of the specified format