From 0aa01822102fbb23c0246927e2b1db20fb0a16a9 Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Wed, 13 Sep 2023 15:29:05 -0700 Subject: [PATCH 1/9] adds change for test path --- ion-schema-tests-runner/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ion-schema-tests-runner/src/lib.rs b/ion-schema-tests-runner/src/lib.rs index b0ac2f7..83eabe4 100644 --- a/ion-schema-tests-runner/src/lib.rs +++ b/ion-schema-tests-runner/src/lib.rs @@ -73,7 +73,7 @@ struct MacroArgs { /// /// ion_schema_tests!( /// // The root directory of a test suite. -/// root = "../ion-schema-tests/ion_schema_1_0/", +/// root = "ion-schema-tests/ion_schema_1_0/", /// // Optional, a list of tests to mark as `#[ignore]`. /// // Strings can be test names or regexes that match test names. /// ignored( From 45b2890e705938aa6f1e592b10a130ce843c229d Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Wed, 13 Sep 2023 15:29:32 -0700 Subject: [PATCH 2/9] adds changes for timestamp precision fixes --- ion-schema/src/isl/ranges.rs | 46 +++++++++++++++++++++++++++++++++++- ion-schema/src/isl/util.rs | 2 +- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/ion-schema/src/isl/ranges.rs b/ion-schema/src/isl/ranges.rs index 4338d3c..9c18b1d 100644 --- a/ion-schema/src/isl/ranges.rs +++ b/ion-schema/src/isl/ranges.rs @@ -132,7 +132,51 @@ pub type TimestampRange = base::Range; impl RangeValidation for TimestampRange {} pub type TimestampPrecisionRange = base::Range; -impl RangeValidation for TimestampPrecisionRange {} +impl RangeValidation for TimestampPrecisionRange { + fn is_empty(start: &Limit, end: &Limit) -> bool { + use crate::isl::util::TimestampPrecision::*; + + match (start, end) { + (Limit::Inclusive(lower), Limit::Inclusive(upper)) => lower > upper, + (Limit::Exclusive(lower), Limit::Inclusive(upper)) + | (Limit::Inclusive(lower), Limit::Exclusive(upper)) => lower >= upper, + (Limit::Exclusive(lower), Limit::Exclusive(upper)) => { + let start_value = match lower { + Year => -4, + Month => -3, + Day => -2, + Minute => -1, + Second => 0, + Millisecond => 3, + Microsecond => 6, + Nanosecond => 9, + OtherFractionalSeconds(scale) => *scale, + }; + + let end_value = match upper { + Year => -4, + Month => -3, + Day => -2, + Minute => -1, + Second => 0, + Millisecond => 3, + Microsecond => 6, + Nanosecond => 9, + OtherFractionalSeconds(scale) => *scale, + }; + + // Checking for e.g. range::[exclusive::1, exclusive::2] which is empty. + let adjusted_lower = start_value.checked_add(1); + // If the _lower_ bound wraps around when we add one, then we know it's empty. + if adjusted_lower.is_none() { + return true; + } + adjusted_lower.unwrap() >= end_value + } + _ => false, + } + } +} // usize does not implement Into // TODO: Remove after https://github.com/amazon-ion/ion-rust/issues/573 is released diff --git a/ion-schema/src/isl/util.rs b/ion-schema/src/isl/util.rs index 1f2fa36..4833ef8 100644 --- a/ion-schema/src/isl/util.rs +++ b/ion-schema/src/isl/util.rs @@ -129,7 +129,7 @@ impl TryFrom<&str> for TimestampPrecision { "year" => TimestampPrecision::Year, "month" => TimestampPrecision::Month, "day" => TimestampPrecision::Day, - "minute" | "hour" => TimestampPrecision::Minute, + "minute" => TimestampPrecision::Minute, "second" => TimestampPrecision::Second, "millisecond" => TimestampPrecision::Millisecond, "microsecond" => TimestampPrecision::Microsecond, From 67e3bb59ca0caaff7436970d0964b5a172e68106 Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Wed, 13 Sep 2023 15:29:56 -0700 Subject: [PATCH 3/9] adds chanegs for `contains` fixes --- ion-schema/src/constraint.rs | 14 ++++++++++---- ion-schema/src/isl/isl_constraint.rs | 6 ++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/ion-schema/src/constraint.rs b/ion-schema/src/constraint.rs index 4a0c77b..12dd6cf 100644 --- a/ion-schema/src/constraint.rs +++ b/ion-schema/src/constraint.rs @@ -1175,14 +1175,21 @@ impl ConstraintValidator for ContainsConstraint { // Create a peekable iterator for given sequence let values: Vec = match &value { IonSchemaElement::SingleElement(element) => { - match element.as_sequence() { - None => { + match element.value() { + Value::List(ion_sequence) | Value::SExp(ion_sequence) => { + ion_sequence.elements().map(|a| a.to_owned()).collect() + } + Value::Struct(ion_struct) => ion_struct + .fields() + .map(|(name, value)| value.to_owned()) + .collect(), + _ => { // return Violation if value is not an Ion sequence return Err(Violation::new( "contains", ViolationCode::TypeMismatched, &format!( - "expected list/sexp found {}", + "expected list/sexp/struct/document found {}", if element.is_null() { format!("{element}") } else { @@ -1192,7 +1199,6 @@ impl ConstraintValidator for ContainsConstraint { ion_path, )); } - Some(ion_sequence) => ion_sequence.elements().map(|a| a.to_owned()).collect(), } } IonSchemaElement::Document(document) => document.to_owned(), diff --git a/ion-schema/src/isl/isl_constraint.rs b/ion-schema/src/isl/isl_constraint.rs index 6eeefd1..584b69a 100644 --- a/ion-schema/src/isl/isl_constraint.rs +++ b/ion-schema/src/isl/isl_constraint.rs @@ -583,6 +583,12 @@ impl IslConstraintImpl { )); } + if value.annotations().contains("range") { + return invalid_schema_error(format!( + "contains constraint can not have ranges" + )); + } + let values: Vec = value .as_sequence() .unwrap() From 411ceb7d5e0ba6a66035babe6e5a9d669575b4d1 Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Wed, 13 Sep 2023 16:37:21 -0700 Subject: [PATCH 4/9] adds fix for import types --- ion-schema/src/system.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ion-schema/src/system.rs b/ion-schema/src/system.rs index f76e195..c51d58e 100644 --- a/ion-schema/src/system.rs +++ b/ion-schema/src/system.rs @@ -351,6 +351,11 @@ impl PendingTypes { if let Some(exists) = self.ids_by_name.get(name) { return exists.to_owned() + type_store.types_by_id.len(); } + + if let Some(exists) = type_store.imported_type_ids_by_name.get(name) { + return exists.to_owned(); + } + match type_store.update_deferred_type_def(type_def.to_owned(), name) { None => { let type_id = type_id - type_store.types_by_id.len(); From 5f77583e4c09c59c61dd5695692110552986ad5e Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Mon, 18 Sep 2023 11:33:43 -0700 Subject: [PATCH 5/9] adds fixes for type definition naming and annotations --- ion-schema/src/isl/isl_constraint.rs | 4 +- ion-schema/src/isl/isl_type.rs | 9 +- ion-schema/src/isl/isl_type_reference.rs | 4 +- ion-schema/src/system.rs | 107 ++++++++++------------- ion-schema/src/types.rs | 24 ++++- 5 files changed, 77 insertions(+), 71 deletions(-) diff --git a/ion-schema/src/isl/isl_constraint.rs b/ion-schema/src/isl/isl_constraint.rs index 584b69a..cdeb454 100644 --- a/ion-schema/src/isl/isl_constraint.rs +++ b/ion-schema/src/isl/isl_constraint.rs @@ -584,9 +584,7 @@ impl IslConstraintImpl { } if value.annotations().contains("range") { - return invalid_schema_error(format!( - "contains constraint can not have ranges" - )); + return invalid_schema_error("contains constraint can not have ranges"); } let values: Vec = value diff --git a/ion-schema/src/isl/isl_type.rs b/ion-schema/src/isl/isl_type.rs index 343dded..b7ffb73 100644 --- a/ion-schema/src/isl/isl_type.rs +++ b/ion-schema/src/isl/isl_type.rs @@ -165,7 +165,14 @@ impl IslTypeImpl { "type names must be a symbol with defined text", )) } - Some(name) => Some(name.to_owned()), + Some(name) => { + if !name_element.annotations().is_empty() { + return Err(invalid_schema_error_raw( + "type names must be a non null and unannotated symbol with defined text", + )); + } + Some(name.to_owned()) + } }, None => { return Err(invalid_schema_error_raw( diff --git a/ion-schema/src/isl/isl_type_reference.rs b/ion-schema/src/isl/isl_type_reference.rs index 80d3912..89dec2f 100644 --- a/ion-schema/src/isl/isl_type_reference.rs +++ b/ion-schema/src/isl/isl_type_reference.rs @@ -338,7 +338,9 @@ impl IslTypeRefImpl { } IslTypeRefImpl::TypeImport(isl_import_type, type_ref_modifier) => { // verify if the inline import type already exists in the type_store - match type_store.get_type_id_by_name(isl_import_type.type_name()) { + match type_store + .get_defined_type_id_or_imported_type_id_by_name(isl_import_type.type_name()) + { None => unresolvable_schema_error(format!( "inline import type: {} does not exists", isl_import_type.type_name() diff --git a/ion-schema/src/system.rs b/ion-schema/src/system.rs index c51d58e..4255bc4 100644 --- a/ion-schema/src/system.rs +++ b/ion-schema/src/system.rs @@ -32,9 +32,7 @@ use crate::schema::Schema; use crate::types::{BuiltInTypeDefinition, Nullability, TypeDefinitionImpl, TypeDefinitionKind}; use crate::{is_isl_version_marker, is_reserved_word, UserReservedFields}; use ion_rs::element::{Annotations, Element}; -use ion_rs::types::IonType::Struct; use ion_rs::IonType; -use regex::Regex; use std::collections::{HashMap, HashSet}; use std::io::ErrorKind; use std::sync::Arc; @@ -178,8 +176,8 @@ impl PendingTypes { return match self.ids_by_name.get(import_type_name) { Some(id) => self.types_by_id[*id] .to_owned() - .and_then(|type_def| match type_def { - TypeDefinitionKind::Named(named_type_def) => Some(Ok(named_type_def)), + .map(|type_def| match type_def { + TypeDefinitionKind::Named(named_type_def) => Ok(named_type_def), TypeDefinitionKind::Anonymous(_) => { unreachable!( "The TypeDefinition for the imported type '{}' was Anonymous.", @@ -193,24 +191,26 @@ impl PendingTypes { ) } }), - None => match type_store.get_type_id_by_name(import_type_name) { - Some(id) => match type_store.types_by_id[*id].to_owned() { - TypeDefinitionKind::Named(named_type_def) => Some(Ok(named_type_def)), - TypeDefinitionKind::Anonymous(_) => { - unreachable!( - "The TypeDefinition for the imported type '{}' was Anonymous.", - import_type_name - ) - } - TypeDefinitionKind::BuiltIn(_) => { - unreachable!( - "The TypeDefinition for the imported type '{}' was a builtin type.", - import_type_name - ) - } - }, - None => None, - }, + None => { + match type_store.get_defined_type_id_or_imported_type_id_by_name(import_type_name) { + Some(id) => match type_store.types_by_id[*id].to_owned() { + TypeDefinitionKind::Named(named_type_def) => Some(Ok(named_type_def)), + TypeDefinitionKind::Anonymous(_) => { + unreachable!( + "The TypeDefinition for the imported type '{}' was Anonymous.", + import_type_name + ) + } + TypeDefinitionKind::BuiltIn(_) => { + unreachable!( + "The TypeDefinition for the imported type '{}' was a builtin type.", + import_type_name + ) + } + }, + None => None, + } + } }; } @@ -335,7 +335,9 @@ impl PendingTypes { ) -> Option { match self.ids_by_name.get(name) { Some(id) => Some(*id + type_store.types_by_id.len()), - None => type_store.get_type_id_by_name(name).copied(), + None => type_store + .get_defined_type_id_or_imported_type_id_by_name(name) + .copied(), } } @@ -409,7 +411,8 @@ impl PendingTypes { if let Some(exists) = self.ids_by_name.get(&name) { return exists.to_owned() + type_store.types_by_id.len(); } - if let Some(exists) = type_store.get_type_id_by_name(&name) { + if let Some(exists) = type_store.get_defined_type_id_or_imported_type_id_by_name(&name) + { return exists.to_owned(); } } @@ -567,12 +570,23 @@ impl TypeStore { /// Provides the [`TypeId`] associated with given name if it exists in the [`TypeStore`] as a type /// defined within schema (This doesn't include built-in types); Otherwise returns None - pub(crate) fn get_type_id_by_name(&self, name: &str) -> Option<&TypeId> { + pub(crate) fn get_defined_type_id_or_imported_type_id_by_name( + &self, + name: &str, + ) -> Option<&TypeId> { self.ids_by_name .get(name) .or_else(|| self.imported_type_ids_by_name.get(name)) } + /// Provides the [`Type`] associated with given name if it exists in the [`TypeStore`] as a type + /// defined within schema (This doesn't include built-in types and imported types); Otherwise returns None + pub(crate) fn get_type_def_by_name(&self, name: &str) -> Option<&TypeDefinitionKind> { + self.ids_by_name + .get(name) + .and_then(|id| self.types_by_id.get(*id)) + } + /// Provides the [`TypeId`] associated with given type name if it exists in the [`TypeStore`] /// Otherwise returns None pub(crate) fn get_builtin_type_id(&self, type_name: &str) -> Option { @@ -823,6 +837,11 @@ impl Resolver { } // load types for schema else if annotations.contains("type") { + if annotations.len() > 1 { + return invalid_schema_error( + "Top level types definitions must not have any other annotations then `type`", + ); + } // if we didn't find an isl version marker before finding a type definition // then isl version will be defaulted to be ISL 1.0 if !found_isl_version_marker { @@ -866,8 +885,7 @@ impl Resolver { } } else { // open content - if isl_version == IslVersion::V2_0 - && value.ion_type() == IonType::Symbol + if value.ion_type() == IonType::Symbol && !value.is_null() && is_isl_version_marker(value.as_text().unwrap()) { @@ -1084,41 +1102,6 @@ impl Resolver { } unresolvable_schema_error("Unable to load ISL model: ".to_owned() + id) } - - // This is a helper method that returns the ISL version for given schema content - // It returns an error for an ISL version that matches the version marker but doesn't match the existing ISL versions. - fn find_isl_version( - &self, - schema_content: &Vec, - isl_version_marker: Regex, - ) -> IonSchemaResult { - for value in schema_content { - // if find a type definition or a schema header before finding any version marker then this is ISL 1.0 - if value.ion_type() == Struct - && (value.annotations().contains("type") - || value.annotations().contains("schema_header")) - { - // default ISL 1.0 version will be returned - break; - } - // verify if value is an ISL version marker and if it has valid format - if value.ion_type() == IonType::Symbol - && isl_version_marker.is_match(value.as_text().unwrap()) - { - // This implementation supports Ion Schema 1.0 and Ion Schema 2.0 - return match value.as_text().unwrap() { - "$ion_schema_1_0" => Ok(IslVersion::V1_0), - "$ion_schema_2_0" => Ok(IslVersion::V2_0), - _ => invalid_schema_error(format!( - "Unsupported Ion Schema Language version: {value}" - )), - }; - } - } - - // default ISL version 1.0 if no version marker is found - Ok(IslVersion::V1_0) - } } /// Provides functions for instantiating instances of [`Schema`]. diff --git a/ion-schema/src/types.rs b/ion-schema/src/types.rs index 0a57b64..540c830 100644 --- a/ion-schema/src/types.rs +++ b/ion-schema/src/types.rs @@ -3,7 +3,7 @@ use crate::ion_path::IonPath; use crate::isl::isl_constraint::IslConstraintImpl; use crate::isl::isl_type::IslTypeImpl; use crate::isl::IslVersion; -use crate::result::{IonSchemaResult, ValidationResult}; +use crate::result::{invalid_schema_error, IonSchemaResult, ValidationResult}; use crate::system::{PendingTypes, TypeId, TypeStore}; use crate::violation::{Violation, ViolationCode}; use crate::IonSchemaElement; @@ -262,6 +262,14 @@ impl TypeDefinitionKind { )) } + /// Provides a boolean that represents whether this type is deferred type or not + pub fn is_deferred_type_def(&self) -> bool { + match self { + TypeDefinitionKind::Named(type_def) => type_def.is_deferred_type_def, + _ => false, + } + } + /// Creates an anonymous [`TypeDefinitionKind`] using the [`Constraint`]s defined within it pub fn anonymous>>(constraints: A) -> TypeDefinitionKind { TypeDefinitionKind::Anonymous(TypeDefinitionImpl::new(None, constraints.into(), None)) @@ -449,9 +457,17 @@ impl TypeDefinitionImpl { // parses an isl_type to a TypeDefinition let type_name = isl_type.name(); - // add parent information for named type - if type_name.is_some() { - pending_types.add_parent(type_name.to_owned().unwrap(), type_store); + if let Some(type_name) = type_name { + if let Some(type_def) = type_store.get_type_def_by_name(type_name) { + if isl_version == IslVersion::V2_0 && !type_def.is_deferred_type_def() { + return invalid_schema_error(format!( + "The schema document can not have two type definitions with same name: {}", + type_name + )); + } + } + // add parent information for named type + pending_types.add_parent(type_name.to_owned(), type_store); } // add this unresolved type to context for type_id From 02b3d255815f27a6eb607f7bd21875e8921a95b8 Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Mon, 18 Sep 2023 22:46:07 -0700 Subject: [PATCH 6/9] adds checks for schema header/footer position and annotations for ISL 2.0 --- ion-schema/src/system.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/ion-schema/src/system.rs b/ion-schema/src/system.rs index 4255bc4..2684edf 100644 --- a/ion-schema/src/system.rs +++ b/ion-schema/src/system.rs @@ -776,6 +776,7 @@ impl Resolver { let mut found_header = false; let mut found_footer = false; + let mut found_type_definition = false; let mut found_isl_version_marker = false; for value in elements { @@ -798,6 +799,26 @@ impl Resolver { }; found_isl_version_marker = true; } else if annotations.contains("schema_header") { + if isl_version == IslVersion::V2_0 { + if found_type_definition { + return invalid_schema_error( + "The schema header must come before top level type definitions", + ); + } + + if found_header { + return invalid_schema_error( + "Schema must only contain a single schema header", + ); + } + + if annotations.len() > 1 { + return invalid_schema_error( + "schema header must not have any other annotations then `schema_header`", + ); + } + } + found_header = true; // if we didn't find an isl version marker before finding a schema header @@ -837,7 +858,9 @@ impl Resolver { } // load types for schema else if annotations.contains("type") { - if annotations.len() > 1 { + found_type_definition = true; + + if isl_version == IslVersion::V2_0 && annotations.len() > 1 { return invalid_schema_error( "Top level types definitions must not have any other annotations then `type`", ); @@ -880,6 +903,11 @@ impl Resolver { else if annotations.contains("schema_footer") { found_footer = true; if isl_version == IslVersion::V2_0 { + if annotations.len() > 1 { + return invalid_schema_error( + "schema footer must not have any other annotations then `schema_footer`", + ); + } let schema_footer = try_to!(value.as_struct()); isl_user_reserved_fields.validate_field_names_in_footer(schema_footer)?; } From 731c0555293e36a4170dfe64b53a9b62f4747eed Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Tue, 19 Sep 2023 11:56:45 -0700 Subject: [PATCH 7/9] removed tests from skip list --- ion-schema-tests-runner/tests/ion-schema-tests-2-0.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ion-schema-tests-runner/tests/ion-schema-tests-2-0.rs b/ion-schema-tests-runner/tests/ion-schema-tests-2-0.rs index ca26e99..bcfc17f 100644 --- a/ion-schema-tests-runner/tests/ion-schema-tests-2-0.rs +++ b/ion-schema-tests-runner/tests/ion-schema-tests-2-0.rs @@ -5,17 +5,8 @@ ion_schema_tests!( // Support for ISL 2.0 is not completely implemented yet, so some tests are ignored. ignored( "imports", - "schema::*", - "null_or::*", - "constraints::contains", "constraints::ordered_elements", "constraints::precision", "constraints::regex::value_should_be_invalid_for_type_regex_unescaped_newline__2_", // https://github.com/amazon-ion/ion-rust/issues/399 - "constraints::timestamp_precision", - // following tests are related to: https://github.com/amazon-ion/ion-rust/pull/553 - "constraints::valid_values_ranges::value_should_be_valid_for_type_valid_values_range_timestamp_known_offset__12_", - "constraints::valid_values_ranges::value_should_be_valid_for_type_valid_values_range_timestamp_known_offset__13_", - "constraints::valid_values_ranges::value_should_be_valid_for_type_valid_values_range_timestamp_unknown_offset__12_", - "constraints::valid_values_ranges::value_should_be_valid_for_type_valid_values_range_timestamp_unknown_offset__13_" ) ); From 1ac7224232ad3193a9efa24b4b82b395f639ce3b Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Thu, 21 Sep 2023 09:32:58 -0700 Subject: [PATCH 8/9] adds suggested changes --- ion-schema/src/isl/isl_constraint.rs | 2 +- ion-schema/src/isl/ranges.rs | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/ion-schema/src/isl/isl_constraint.rs b/ion-schema/src/isl/isl_constraint.rs index cdeb454..5171283 100644 --- a/ion-schema/src/isl/isl_constraint.rs +++ b/ion-schema/src/isl/isl_constraint.rs @@ -584,7 +584,7 @@ impl IslConstraintImpl { } if value.annotations().contains("range") { - return invalid_schema_error("contains constraint can not have ranges"); + return invalid_schema_error("contains list can not have any annotations"); } let values: Vec = value diff --git a/ion-schema/src/isl/ranges.rs b/ion-schema/src/isl/ranges.rs index 9c18b1d..b5ed230 100644 --- a/ion-schema/src/isl/ranges.rs +++ b/ion-schema/src/isl/ranges.rs @@ -166,12 +166,8 @@ impl RangeValidation for TimestampPrecisionRange { }; // Checking for e.g. range::[exclusive::1, exclusive::2] which is empty. - let adjusted_lower = start_value.checked_add(1); - // If the _lower_ bound wraps around when we add one, then we know it's empty. - if adjusted_lower.is_none() { - return true; - } - adjusted_lower.unwrap() >= end_value + let adjusted_lower = start_value + 1; + adjusted_lower >= end_value } _ => false, } From b1c58611f1416985d5abe4b960bd8a9664791a25 Mon Sep 17 00:00:00 2001 From: Khushboo Desai Date: Tue, 26 Sep 2023 14:58:59 -0700 Subject: [PATCH 9/9] adds changes for ranges int_value() --- ion-schema/src/isl/isl_constraint.rs | 2 +- ion-schema/src/isl/ranges.rs | 27 ++---------------- ion-schema/src/isl/util.rs | 41 ++++++++++++---------------- 3 files changed, 20 insertions(+), 50 deletions(-) diff --git a/ion-schema/src/isl/isl_constraint.rs b/ion-schema/src/isl/isl_constraint.rs index 5171283..010d4e2 100644 --- a/ion-schema/src/isl/isl_constraint.rs +++ b/ion-schema/src/isl/isl_constraint.rs @@ -583,7 +583,7 @@ impl IslConstraintImpl { )); } - if value.annotations().contains("range") { + if !value.annotations().is_empty() { return invalid_schema_error("contains list can not have any annotations"); } diff --git a/ion-schema/src/isl/ranges.rs b/ion-schema/src/isl/ranges.rs index b5ed230..9925133 100644 --- a/ion-schema/src/isl/ranges.rs +++ b/ion-schema/src/isl/ranges.rs @@ -134,36 +134,13 @@ impl RangeValidation for TimestampRange {} pub type TimestampPrecisionRange = base::Range; impl RangeValidation for TimestampPrecisionRange { fn is_empty(start: &Limit, end: &Limit) -> bool { - use crate::isl::util::TimestampPrecision::*; - match (start, end) { (Limit::Inclusive(lower), Limit::Inclusive(upper)) => lower > upper, (Limit::Exclusive(lower), Limit::Inclusive(upper)) | (Limit::Inclusive(lower), Limit::Exclusive(upper)) => lower >= upper, (Limit::Exclusive(lower), Limit::Exclusive(upper)) => { - let start_value = match lower { - Year => -4, - Month => -3, - Day => -2, - Minute => -1, - Second => 0, - Millisecond => 3, - Microsecond => 6, - Nanosecond => 9, - OtherFractionalSeconds(scale) => *scale, - }; - - let end_value = match upper { - Year => -4, - Month => -3, - Day => -2, - Minute => -1, - Second => 0, - Millisecond => 3, - Microsecond => 6, - Nanosecond => 9, - OtherFractionalSeconds(scale) => *scale, - }; + let start_value = lower.int_value(); + let end_value = upper.int_value(); // Checking for e.g. range::[exclusive::1, exclusive::2] which is empty. let adjusted_lower = start_value + 1; diff --git a/ion-schema/src/isl/util.rs b/ion-schema/src/isl/util.rs index 4833ef8..82bb15b 100644 --- a/ion-schema/src/isl/util.rs +++ b/ion-schema/src/isl/util.rs @@ -119,6 +119,21 @@ impl TimestampPrecision { TimestampPrecision::OtherFractionalSeconds(i) => format!("fractional second (10e{i})"), } } + + pub(crate) fn int_value(&self) -> i64 { + use TimestampPrecision::*; + match self { + Year => -4, + Month => -3, + Day => -2, + Minute => -1, + Second => 0, + Millisecond => 3, + Microsecond => 6, + Nanosecond => 9, + OtherFractionalSeconds(scale) => *scale, + } + } } impl TryFrom<&str> for TimestampPrecision { @@ -145,30 +160,8 @@ impl TryFrom<&str> for TimestampPrecision { impl PartialOrd for TimestampPrecision { fn partial_cmp(&self, other: &TimestampPrecision) -> Option { - use TimestampPrecision::*; - let self_value = match self { - Year => -4, - Month => -3, - Day => -2, - Minute => -1, - Second => 0, - Millisecond => 3, - Microsecond => 6, - Nanosecond => 9, - OtherFractionalSeconds(scale) => *scale, - }; - - let other_value = match other { - Year => -4, - Month => -3, - Day => -2, - Minute => -1, - Second => 0, - Millisecond => 3, - Microsecond => 6, - Nanosecond => 9, - OtherFractionalSeconds(scale) => *scale, - }; + let self_value = self.int_value(); + let other_value = other.int_value(); Some(self_value.cmp(&other_value)) }