adds ion-schema-tests and related fixes for ISL 2.0

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(

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_"
     )
 );

ion-schema/src/constraint.rs

@@ -1175,14 +1175,21 @@ impl ConstraintValidator for ContainsConstraint {
         // Create a peekable iterator for given sequence
         let values: Vec<Element> = 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(),

ion-schema/src/isl/isl_constraint.rs

@@ -583,6 +583,10 @@ impl IslConstraintImpl {
                     ));
                 }
 
+                if value.annotations().contains("range") {
+                    return invalid_schema_error("contains constraint can not have ranges");
+                }
+
                 let values: Vec<Element> = value
                     .as_sequence()
                     .unwrap()

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(

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()

ion-schema/src/isl/ranges.rs

@@ -132,7 +132,51 @@ pub type TimestampRange = base::Range<Timestamp>;
 impl RangeValidation<Timestamp> for TimestampRange {}
 
 pub type TimestampPrecisionRange = base::Range<TimestampPrecision>;
-impl RangeValidation<TimestampPrecision> for TimestampPrecisionRange {}
+impl RangeValidation<TimestampPrecision> for TimestampPrecisionRange {
+    fn is_empty(start: &Limit<TimestampPrecision>, end: &Limit<TimestampPrecision>) -> 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<Element>
 // TODO: Remove after https://github.com/amazon-ion/ion-rust/issues/573 is released

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,

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<TypeId> {
         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(),
         }
     }
 
@@ -351,6 +353,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();
@@ -404,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();
             }
         }
@@ -562,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<TypeId> {
@@ -757,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 {
@@ -779,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
@@ -818,6 +858,13 @@ impl Resolver {
             }
             // load types for schema
             else if annotations.contains("type") {
+                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`",
+                    );
+                }
                 // 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 {
@@ -856,13 +903,17 @@ 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)?;
                 }
             } 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())
                 {
@@ -1079,41 +1130,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<Element>,
-        isl_version_marker: Regex,
-    ) -> IonSchemaResult<IslVersion> {
-        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`].