From a39659b513124c8942b46abf4e92c72bbdd358a2 Mon Sep 17 00:00:00 2001 From: Juha Kukkonen Date: Fri, 6 Sep 2024 20:46:19 +0300 Subject: [PATCH] Fix negative value parsing on schema attributes This commit fixes issue where negative numbers caused parsing error by `rust_analyzer` however the code compiled correctly and created valid code. The issue is fixed for validation attributes and `AnyValue` which is used in various places. The original issue only reported the issue in validation attributes. Fixes #994 --- utoipa-gen/src/component/features.rs | 43 ++---- .../src/component/features/validation.rs | 132 ++++++++++++++---- .../src/component/features/validators.rs | 23 ++- utoipa-gen/src/lib.rs | 5 +- utoipa-gen/tests/schema_derive_test.rs | 36 +++++ utoipa/src/lib.rs | 61 ++++++++ utoipa/src/openapi/schema.rs | 58 +++++--- 7 files changed, 266 insertions(+), 92 deletions(-) diff --git a/utoipa-gen/src/component/features.rs b/utoipa-gen/src/component/features.rs index 67297a73..8b0c5280 100644 --- a/utoipa-gen/src/component/features.rs +++ b/utoipa-gen/src/component/features.rs @@ -1,12 +1,11 @@ -use std::{fmt::Display, mem, str::FromStr}; +use std::{fmt::Display, mem}; use proc_macro2::{Ident, TokenStream}; use quote::{quote, ToTokens}; -use syn::{parse::ParseStream, LitFloat, LitInt}; +use syn::parse::ParseStream; use crate::{ - as_tokens_or_diagnostics, parse_utils, schema_type::SchemaType, Diagnostics, OptionExt, - ToTokensDiagnostics, + as_tokens_or_diagnostics, schema_type::SchemaType, Diagnostics, OptionExt, ToTokensDiagnostics, }; use self::validators::{AboveZeroF64, AboveZeroUsize, IsNumber, IsString, IsVec, ValidatorChain}; @@ -17,32 +16,6 @@ pub mod attributes; pub mod validation; pub mod validators; -/// Parse `LitInt` from parse stream -fn parse_integer(input: ParseStream) -> syn::Result -where - ::Err: Display, -{ - parse_utils::parse_next(input, || input.parse::()?.base10_parse()) -} - -/// Parse any `number`. Tries to parse `LitInt` or `LitFloat` from parse stream. -fn parse_number(input: ParseStream) -> syn::Result -where - T: FromStr, - ::Err: Display, -{ - parse_utils::parse_next(input, || { - let lookup = input.lookahead1(); - if lookup.peek(LitInt) { - input.parse::()?.base10_parse() - } else if lookup.peek(LitFloat) { - input.parse::()?.base10_parse() - } else { - Err(lookup.error()) - } - }) -} - pub trait FeatureLike: Parse { fn get_name() -> std::borrow::Cow<'static, str> where @@ -145,7 +118,7 @@ impl Feature { pub fn validate(&self, schema_type: &SchemaType, type_tree: &TypeTree) -> Option { match self { Feature::MultipleOf(multiple_of) => multiple_of.validate( - ValidatorChain::new(&IsNumber(schema_type)).next(&AboveZeroF64(multiple_of.0)), + ValidatorChain::new(&IsNumber(schema_type)).next(&AboveZeroF64(&multiple_of.0)), ), Feature::Maximum(maximum) => maximum.validate(IsNumber(schema_type)), Feature::Minimum(minimum) => minimum.validate(IsNumber(schema_type)), @@ -156,17 +129,17 @@ impl Feature { exclusive_minimum.validate(IsNumber(schema_type)) } Feature::MaxLength(max_length) => max_length.validate( - ValidatorChain::new(&IsString(schema_type)).next(&AboveZeroUsize(max_length.0)), + ValidatorChain::new(&IsString(schema_type)).next(&AboveZeroUsize(&max_length.0)), ), Feature::MinLength(min_length) => min_length.validate( - ValidatorChain::new(&IsString(schema_type)).next(&AboveZeroUsize(min_length.0)), + ValidatorChain::new(&IsString(schema_type)).next(&AboveZeroUsize(&min_length.0)), ), Feature::Pattern(pattern) => pattern.validate(IsString(schema_type)), Feature::MaxItems(max_items) => max_items.validate( - ValidatorChain::new(&AboveZeroUsize(max_items.0)).next(&IsVec(type_tree)), + ValidatorChain::new(&AboveZeroUsize(&max_items.0)).next(&IsVec(type_tree)), ), Feature::MinItems(min_items) => min_items.validate( - ValidatorChain::new(&AboveZeroUsize(min_items.0)).next(&IsVec(type_tree)), + ValidatorChain::new(&AboveZeroUsize(&min_items.0)).next(&IsVec(type_tree)), ), unsupported => { const SUPPORTED_VARIANTS: [&str; 10] = [ diff --git a/utoipa-gen/src/component/features/validation.rs b/utoipa-gen/src/component/features/validation.rs index a2e58626..27ae7fec 100644 --- a/utoipa-gen/src/component/features/validation.rs +++ b/utoipa-gen/src/component/features/validation.rs @@ -1,17 +1,91 @@ -use proc_macro2::{Ident, Span, TokenStream}; -use quote::ToTokens; +use std::str::FromStr; + +use proc_macro2::{Ident, Literal, Span, TokenStream, TokenTree}; +use quote::{quote, ToTokens}; use syn::parse::ParseStream; use syn::LitStr; use crate::{parse_utils, Diagnostics}; use super::validators::Validator; -use super::{impl_feature, parse_integer, parse_number, Feature, Parse, Validate}; +use super::{impl_feature, Feature, Parse, Validate}; + +#[inline] +fn from_str(number: &str, span: Span) -> syn::Result +where + ::Err: std::fmt::Display, +{ + T::from_str(number).map_err(|error| syn::Error::new(span, error)) +} + +#[derive(Clone)] +#[cfg_attr(feature = "debug", derive(Debug))] +pub struct NumberValue { + minus: bool, + pub lit: Literal, +} + +impl NumberValue { + pub fn try_from_str(&self) -> syn::Result + where + T: FromStr, + ::Err: std::fmt::Display, + { + let number = if self.minus { + format!("-{}", &self.lit) + } else { + self.lit.to_string() + }; + + let parsed = from_str::(&number, self.lit.span())?; + Ok(parsed) + } +} + +impl syn::parse::Parse for NumberValue { + fn parse(input: ParseStream) -> syn::Result { + let mut minus = false; + let result = input.step(|cursor| { + let mut rest = *cursor; + + while let Some((tt, next)) = rest.token_tree() { + match &tt { + TokenTree::Punct(punct) if punct.as_char() == '-' => { + minus = true; + } + TokenTree::Literal(lit) => return Ok((lit.clone(), next)), + _ => (), + } + rest = next; + } + Err(cursor.error("no `literal` value found after this point")) + })?; + + Ok(Self { minus, lit: result }) + } +} + +impl ToTokens for NumberValue { + fn to_tokens(&self, tokens: &mut TokenStream) { + let punct = if self.minus { Some(quote! {-}) } else { None }; + let lit = &self.lit; + + tokens.extend(quote! { + #punct #lit + }) + } +} + +#[inline] +fn parse_next_number_value(input: ParseStream) -> syn::Result { + use syn::parse::Parse; + parse_utils::parse_next(input, || NumberValue::parse(input)) +} impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct MultipleOf(pub(super) f64, Ident); + pub struct MultipleOf(pub(super) NumberValue, Ident); } impl Validate for MultipleOf { @@ -26,7 +100,7 @@ impl Validate for MultipleOf { impl Parse for MultipleOf { fn parse(input: ParseStream, ident: Ident) -> syn::Result { - parse_number(input).map(|multiple_of| Self(multiple_of, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -45,7 +119,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct Maximum(pub(super) f64, Ident); + pub struct Maximum(pub(super) NumberValue, Ident); } impl Validate for Maximum { @@ -63,7 +137,7 @@ impl Parse for Maximum { where Self: Sized, { - parse_number(input).map(|maximum| Self(maximum, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -82,12 +156,18 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct Minimum(f64, Ident); + pub struct Minimum(NumberValue, Ident); } impl Minimum { pub fn new(value: f64, span: Span) -> Self { - Self(value, Ident::new("empty", span)) + Self( + NumberValue { + minus: value < 0.0, + lit: Literal::f64_suffixed(value), + }, + Ident::new("empty", span), + ) } } @@ -108,7 +188,7 @@ impl Parse for Minimum { where Self: Sized, { - parse_number(input).map(|maximum| Self(maximum, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -127,7 +207,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct ExclusiveMaximum(f64, Ident); + pub struct ExclusiveMaximum(NumberValue, Ident); } impl Validate for ExclusiveMaximum { @@ -145,7 +225,7 @@ impl Parse for ExclusiveMaximum { where Self: Sized, { - parse_number(input).map(|max| Self(max, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -164,7 +244,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct ExclusiveMinimum(f64, Ident); + pub struct ExclusiveMinimum(NumberValue, Ident); } impl Validate for ExclusiveMinimum { @@ -182,7 +262,7 @@ impl Parse for ExclusiveMinimum { where Self: Sized, { - parse_number(input).map(|min| Self(min, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -201,7 +281,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct MaxLength(pub(super) usize, Ident); + pub struct MaxLength(pub(super) NumberValue, Ident); } impl Validate for MaxLength { @@ -219,7 +299,7 @@ impl Parse for MaxLength { where Self: Sized, { - parse_integer(input).map(|max_length| Self(max_length, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -238,7 +318,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct MinLength(pub(super) usize, Ident); + pub struct MinLength(pub(super) NumberValue, Ident); } impl Validate for MinLength { @@ -256,7 +336,7 @@ impl Parse for MinLength { where Self: Sized, { - parse_integer(input).map(|max_length| Self(max_length, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -314,7 +394,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct MaxItems(pub(super) usize, Ident); + pub struct MaxItems(pub(super) NumberValue, Ident); } impl Validate for MaxItems { @@ -332,7 +412,7 @@ impl Parse for MaxItems { where Self: Sized, { - parse_number(input).map(|max_items| Self(max_items, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -351,7 +431,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct MinItems(pub(super) usize, Ident); + pub struct MinItems(pub(super) NumberValue, Ident); } impl Validate for MinItems { @@ -369,7 +449,7 @@ impl Parse for MinItems { where Self: Sized, { - parse_number(input).map(|max_items| Self(max_items, ident)) + parse_next_number_value(input).map(|number| Self(number, ident)) } } @@ -388,7 +468,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct MaxProperties(usize, ()); + pub struct MaxProperties(NumberValue, ()); } impl Parse for MaxProperties { @@ -396,7 +476,7 @@ impl Parse for MaxProperties { where Self: Sized, { - parse_integer(input).map(|max_properties| Self(max_properties, ())) + parse_next_number_value(input).map(|number| Self(number, ())) } } @@ -415,7 +495,7 @@ impl From for Feature { impl_feature! { #[cfg_attr(feature = "debug", derive(Debug))] #[derive(Clone)] - pub struct MinProperties(usize, ()); + pub struct MinProperties(NumberValue, ()); } impl Parse for MinProperties { @@ -423,7 +503,7 @@ impl Parse for MinProperties { where Self: Sized, { - parse_integer(input).map(|min_properties| Self(min_properties, ())) + parse_next_number_value(input).map(|number| Self(number, ())) } } diff --git a/utoipa-gen/src/component/features/validators.rs b/utoipa-gen/src/component/features/validators.rs index 01c21784..f0ba2fe1 100644 --- a/utoipa-gen/src/component/features/validators.rs +++ b/utoipa-gen/src/component/features/validators.rs @@ -1,6 +1,8 @@ use crate::component::{GenericType, TypeTree}; use crate::schema_type::SchemaType; +use super::validation::NumberValue; + pub trait Validator { fn is_valid(&self) -> Result<(), &'static str>; } @@ -53,11 +55,16 @@ impl Validator for IsVec<'_> { } } -pub struct AboveZeroUsize(pub(super) usize); +pub struct AboveZeroUsize<'a>(pub(super) &'a NumberValue); -impl Validator for AboveZeroUsize { +impl Validator for AboveZeroUsize<'_> { fn is_valid(&self) -> Result<(), &'static str> { - if self.0 != 0 { + let usize: usize = self + .0 + .try_from_str() + .map_err(|_| "invalid type, expected `usize`")?; + + if usize != 0 { Ok(()) } else { Err("can only be above zero value") @@ -65,11 +72,15 @@ impl Validator for AboveZeroUsize { } } -pub struct AboveZeroF64(pub(super) f64); +pub struct AboveZeroF64<'a>(pub(super) &'a NumberValue); -impl Validator for AboveZeroF64 { +impl Validator for AboveZeroF64<'_> { fn is_valid(&self) -> Result<(), &'static str> { - if self.0 > 0.0 { + let float: f64 = self + .0 + .try_from_str() + .map_err(|_| "invalid type, expected `f64`")?; + if float > 0.0 { Ok(()) } else { Err("can only be above zero value") diff --git a/utoipa-gen/src/lib.rs b/utoipa-gen/src/lib.rs index cd1e732a..cbbb2cde 100644 --- a/utoipa-gen/src/lib.rs +++ b/utoipa-gen/src/lib.rs @@ -2794,9 +2794,10 @@ impl AnyValue { fn parse_any(input: ParseStream) -> syn::Result { if input.peek(Lit) { - let lit = input.parse::().unwrap().to_token_stream(); + let punct = input.parse::>()?; + let lit = input.parse::().unwrap(); - Ok(AnyValue::Json(lit)) + Ok(AnyValue::Json(quote! { #punct #lit})) } else { let fork = input.fork(); let is_json = if fork.peek(syn::Ident) && fork.peek2(Token![!]) { diff --git a/utoipa-gen/tests/schema_derive_test.rs b/utoipa-gen/tests/schema_derive_test.rs index 3ca2d0d5..7250eda0 100644 --- a/utoipa-gen/tests/schema_derive_test.rs +++ b/utoipa-gen/tests/schema_derive_test.rs @@ -5638,3 +5638,39 @@ fn derive_schema_required_custom_type_required() { }) ); } + +#[test] +fn derive_negative_numbers() { + let value = api_doc! { + #[schema(default)] + #[derive(Default)] + struct Negative { + #[schema(default = -1, minimum = -2.1)] + number: f64, + #[schema(default = -2, maximum = -3)] + solid_number: i64, + } + }; + + assert_json_eq! { + value, + json!({ + "properties": { + "number": { + "type": "number", + "format": "double", + "default": -1, + "minimum": -2.1 + }, + "solid_number": { + "format": "int64", + "type": "integer", + "default": -2, + "maximum": -3, + } + }, + "required": [ "number", "solid_number" ], + "type": "object" + }) + } +} diff --git a/utoipa/src/lib.rs b/utoipa/src/lib.rs index 94394dd9..f1cf7b53 100644 --- a/utoipa/src/lib.rs +++ b/utoipa/src/lib.rs @@ -986,6 +986,67 @@ pub trait ToResponse<'__r> { fn response() -> (&'__r str, openapi::RefOr); } +/// Flexible number wrapper used by validation schema attributes to seamlessly support different +/// number syntaxes. +/// +/// # Examples +/// +/// _**Define object with two different number fields with minimum validation attribute.**_ +/// +/// ```rust +/// # use utoipa::Number; +/// # use utoipa::openapi::schema::{ObjectBuilder, SchemaType, Type}; +/// let _ = ObjectBuilder::new() +/// .property("int_value", ObjectBuilder::new() +/// .schema_type(Type::Integer).minimum(Some(1)) +/// ) +/// .property("float_value", ObjectBuilder::new() +/// .schema_type(Type::Number).minimum(Some(-2.5)) +/// ) +/// .build(); +/// ``` +#[derive(Clone, serde::Deserialize, serde::Serialize)] +#[cfg_attr(feature = "debug", derive(Debug))] +#[serde(untagged)] +pub enum Number { + Int(isize), + UInt(usize), + Float(f64), +} + +impl Eq for Number {} + +impl PartialEq for Number { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::Int(l0), Self::Int(r0)) => l0 == r0, + (Self::UInt(l0), Self::UInt(r0)) => l0 == r0, + (Self::Float(l0), Self::Float(r0)) => l0 == r0, + _ => false, + } + } +} + +macro_rules! impl_from_for_number { + ( $( $ty:ident => $pat:ident $( as $as:ident )? ),* ) => { + $( + impl From<$ty> for Number { + fn from(value: $ty) -> Self { + Self::$pat(value $( as $as )?) + } + } + )* + }; +} + +#[rustfmt::skip] +impl_from_for_number!( + f32 => Float as f64, f64 => Float, + i8 => Int as isize, i16 => Int as isize, i32 => Int as isize, i64 => Int as isize, + u8 => UInt as usize, u16 => UInt as usize, u32 => UInt as usize, u64 => UInt as usize, + isize => Int, usize => UInt +); + /// Internal dev module used internally by utoipa-gen #[doc(hidden)] #[cfg(feature = "macros")] diff --git a/utoipa/src/openapi/schema.rs b/utoipa/src/openapi/schema.rs index 910c78a1..a36e7bfd 100644 --- a/utoipa/src/openapi/schema.rs +++ b/utoipa/src/openapi/schema.rs @@ -905,27 +905,27 @@ builder! { /// Must be a number strictly greater than `0`. Numeric value is considered valid if value /// divided by the _`multiple_of`_ value results an integer. #[serde(skip_serializing_if = "Option::is_none", serialize_with = "omit_decimal_zero")] - pub multiple_of: Option, + pub multiple_of: Option, /// Specify inclusive upper limit for the [`Object`]'s value. Number is considered valid if /// it is equal or less than the _`maximum`_. #[serde(skip_serializing_if = "Option::is_none", serialize_with = "omit_decimal_zero")] - pub maximum: Option, + pub maximum: Option, /// Specify inclusive lower limit for the [`Object`]'s value. Number value is considered /// valid if it is equal or greater than the _`minimum`_. #[serde(skip_serializing_if = "Option::is_none", serialize_with = "omit_decimal_zero")] - pub minimum: Option, + pub minimum: Option, /// Specify exclusive upper limit for the [`Object`]'s value. Number value is considered /// valid if it is strictly less than _`exclusive_maximum`_. #[serde(skip_serializing_if = "Option::is_none", serialize_with = "omit_decimal_zero")] - pub exclusive_maximum: Option, + pub exclusive_maximum: Option, /// Specify exclusive lower limit for the [`Object`]'s value. Number value is considered /// valid if it is strictly above the _`exclusive_minimum`_. #[serde(skip_serializing_if = "Option::is_none", serialize_with = "omit_decimal_zero")] - pub exclusive_minimum: Option, + pub exclusive_minimum: Option, /// Specify maximum length for `string` values. _`max_length`_ cannot be a negative integer /// value. Value is considered valid if content length is equal or less than the _`max_length`_. @@ -1110,28 +1110,34 @@ impl ObjectBuilder { } /// Set or change _`multiple_of`_ validation flag for `number` and `integer` type values. - pub fn multiple_of(mut self, multiple_of: Option) -> Self { - set_value!(self multiple_of multiple_of) + pub fn multiple_of>(mut self, multiple_of: Option) -> Self { + set_value!(self multiple_of multiple_of.map(|multiple_of| multiple_of.into())) } /// Set or change inclusive maximum value for `number` and `integer` values. - pub fn maximum(mut self, maximum: Option) -> Self { - set_value!(self maximum maximum) + pub fn maximum>(mut self, maximum: Option) -> Self { + set_value!(self maximum maximum.map(|max| max.into())) } /// Set or change inclusive minimum value for `number` and `integer` values. - pub fn minimum(mut self, minimum: Option) -> Self { - set_value!(self minimum minimum) + pub fn minimum>(mut self, minimum: Option) -> Self { + set_value!(self minimum minimum.map(|min| min.into())) } /// Set or change exclusive maximum value for `number` and `integer` values. - pub fn exclusive_maximum(mut self, exclusive_maximum: Option) -> Self { - set_value!(self exclusive_maximum exclusive_maximum) + pub fn exclusive_maximum>( + mut self, + exclusive_maximum: Option, + ) -> Self { + set_value!(self exclusive_maximum exclusive_maximum.map(|exclusive_maximum| exclusive_maximum.into())) } /// Set or change exclusive minimum value for `number` and `integer` values. - pub fn exclusive_minimum(mut self, exclusive_minimum: Option) -> Self { - set_value!(self exclusive_minimum exclusive_minimum) + pub fn exclusive_minimum>( + mut self, + exclusive_minimum: Option, + ) -> Self { + set_value!(self exclusive_minimum exclusive_minimum.map(|exclusive_minimum| exclusive_minimum.into())) } /// Set or change maximum length for `string` values. @@ -1355,18 +1361,24 @@ impl From for RefOr { } } -fn omit_decimal_zero(maybe_value: &Option, s: S) -> Result +fn omit_decimal_zero( + maybe_value: &Option, + serializer: S, +) -> Result where S: serde::Serializer, { - if let Some(v) = maybe_value { - if v.fract() == 0.0 && *v >= i64::MIN as f64 && *v <= i64::MAX as f64 { - s.serialize_i64(v.trunc() as i64) - } else { - s.serialize_f64(*v) + match maybe_value { + Some(crate::utoipa::Number::Float(float)) => { + if float.fract() == 0.0 && *float >= i64::MIN as f64 && *float <= i64::MAX as f64 { + serializer.serialize_i64(float.trunc() as i64) + } else { + serializer.serialize_f64(*float) + } } - } else { - s.serialize_none() + Some(crate::utoipa::Number::Int(int)) => serializer.serialize_i64(*int as i64), + Some(crate::utoipa::Number::UInt(uint)) => serializer.serialize_u64(*uint as u64), + None => serializer.serialize_none(), } }