Skip to content

Commit 4b27d7f

Browse files
popemattpopematt
authored
Makes IonSchemaElement use borrowed Elements (#222)
1 parent 8fe6994 commit 4b27d7f

File tree

13 files changed

+381
-353
lines changed

13 files changed

+381
-353
lines changed

‎ion-schema-tests-runner/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ proc-macro = true
99
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
1010

1111
[dependencies]
12-
ion-rs = "1.0.0-rc.7"
12+
ion-rs = "1.0.0-rc.8"
1313
ion-schema = { path = "../ion-schema" }
1414
quote = "1.0.21"
1515
syn = "1.0.102"

‎ion-schema-tests-runner/src/generator.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -321,18 +321,15 @@ fn generate_preamble(root_dir_path: &Path) -> TokenStream {
321321

322322
/// Asserts that a value is or is not valid for a given ISL type.
323323
fn __assert_value_validity_for_type(value_ion: &str, schema_id: &str, type_id: &str, expect_valid: bool) -> Result<(), String> {
324-
let schema = __new_schema_system().load_schema(schema_id).unwrap();
325-
let isl_type = schema.get_type(type_id).unwrap();
326-
let value: ion_rs::Element = ion_rs::Element::read_one(value_ion.as_bytes()).unwrap();
324+
let schema = __new_schema_system().load_schema(schema_id).expect(&format!("Expected to load schema: {}", schema_id));
325+
let isl_type = schema.get_type(type_id).expect(&format!("Expected to get type: {}", type_id));
326+
let value: ion_rs::Element = ion_rs::Element::read_one(value_ion.as_bytes()).expect(&format!("Expected to be able to read value: {}", value_ion));
327327
let prepared_value: ion_schema::IonSchemaElement = if value.annotations().contains("document") && value.ion_type() == ion_rs::IonType::SExp {
328-
let element_vec = value.as_sequence()
329-
.unwrap_or_else(|| unreachable!("We already confirmed that this is a s-expression."))
330-
.elements()
331-
.map(|it| it.to_owned())
332-
.collect::<Vec<_>>();
333-
ion_schema::IonSchemaElement::Document(element_vec)
328+
let values = value.as_sequence()
329+
.expect("We already confirmed that this is a s-expression.");
330+
ion_schema::IonSchemaElement::from(ion_schema::AsDocumentHint::as_document(values))
334331
} else {
335-
ion_schema::IonSchemaElement::SingleElement(value)
332+
ion_schema::IonSchemaElement::from(&value)
336333
};
337334
let validation_result = isl_type.validate(prepared_value);
338335
if validation_result.is_ok() == expect_valid {

‎ion-schema-tests-runner/tests/ion-schema-tests-2-0.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use ion_schema_tests_runner::ion_schema_tests;
22

33
ion_schema_tests!(
44
root = "ion-schema-tests/ion_schema_2_0/",
5-
// Support for ISL 2.0 is not completely implemented yet, so some tests are ignored.
65
ignored(
6+
// Not fully implemented yet.
77
"imports",
88
"constraints::ordered_elements",
9-
"constraints::precision",
10-
"constraints::regex::value_should_be_invalid_for_type_regex_unescaped_newline__2_", // https://github.com/amazon-ion/ion-rust/issues/399
9+
// Failing because of https://github.com/amazon-ion/ion-rust/issues/399
10+
"constraints::regex::value_should_be_invalid_for_type_regex_unescaped_newline__2_",
1111
)
1212
);

‎ion-schema/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ edition = "2021"
2222
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
2323

2424
[dependencies]
25-
ion-rs = { version = "1.0.0-rc.7", features = ["experimental-reader-writer"] }
25+
ion-rs = { version = "1.0.0-rc.8", features = ["experimental-reader-writer"] }
2626
thiserror = "1.0"
2727
num-bigint = "0.3"
2828
num-traits = "0.2"

‎ion-schema/src/constraint.rs

Lines changed: 80 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -899,27 +899,24 @@ impl ConstraintValidator for OrderedElementsConstraint {
899899
) -> ValidationResult {
900900
let violations: Vec<Violation> = vec![];
901901

902-
let values: Vec<Element> = match &value {
903-
IonSchemaElement::SingleElement(element) => match element.as_sequence() {
904-
None => {
905-
return Err(Violation::with_violations(
906-
"ordered_elements",
907-
ViolationCode::TypeMismatched,
908-
format!(
909-
"expected list/sexp ion found {}",
910-
if element.is_null() {
911-
format!("{element}")
912-
} else {
913-
format!("{}", element.ion_type())
914-
}
915-
),
916-
ion_path,
917-
violations,
918-
));
919-
}
920-
Some(sequence) => sequence.elements().map(|a| a.to_owned()).collect(),
921-
},
922-
IonSchemaElement::Document(document) => document.to_owned(),
902+
let mut element_iter = match value.as_sequence_iter() {
903+
Some(iter) => iter.peekable(),
904+
None => {
905+
return Err(Violation::with_violations(
906+
"ordered_elements",
907+
ViolationCode::TypeMismatched,
908+
format!(
909+
"expected list, sexp, or document; found {}",
910+
if value.is_null() {
911+
format!("{value}")
912+
} else {
913+
format!("{}", value.ion_schema_type())
914+
}
915+
),
916+
ion_path,
917+
violations,
918+
));
919+
}
923920
};
924921

925922
// build nfa for validation
@@ -928,7 +925,7 @@ impl ConstraintValidator for OrderedElementsConstraint {
928925
type_store,
929926
);
930927

931-
if !values.is_empty() && nfa_evaluation.nfa.get_final_states().is_empty() {
928+
if element_iter.peek().is_some() && nfa_evaluation.nfa.get_final_states().is_empty() {
932929
return Err(Violation::with_violations(
933930
"ordered_elements",
934931
ViolationCode::TypeMismatched,
@@ -939,7 +936,7 @@ impl ConstraintValidator for OrderedElementsConstraint {
939936
}
940937

941938
// use nfa_evaluation for validation
942-
nfa_evaluation.validate_ordered_elements(&values, type_store);
939+
nfa_evaluation.validate_ordered_elements(element_iter, type_store);
943940

944941
if !nfa_evaluation.has_final_state(type_store) {
945942
return Err(Violation::with_violations(
@@ -1173,36 +1170,24 @@ impl ConstraintValidator for ContainsConstraint {
11731170
type_store: &TypeStore,
11741171
ion_path: &mut IonPath,
11751172
) -> ValidationResult {
1176-
// Create a peekable iterator for given sequence
1177-
let values: Vec<Element> = match &value {
1178-
IonSchemaElement::SingleElement(element) => {
1179-
match element.value() {
1180-
Value::List(ion_sequence) | Value::SExp(ion_sequence) => {
1181-
ion_sequence.elements().map(|a| a.to_owned()).collect()
1182-
}
1183-
Value::Struct(ion_struct) => ion_struct
1184-
.fields()
1185-
.map(|(name, value)| value.to_owned())
1186-
.collect(),
1187-
_ => {
1188-
// return Violation if value is not an Ion sequence
1189-
return Err(Violation::new(
1190-
"contains",
1191-
ViolationCode::TypeMismatched,
1192-
format!(
1193-
"expected list/sexp/struct/document found {}",
1194-
if element.is_null() {
1195-
format!("{element}")
1196-
} else {
1197-
format!("{}", element.ion_type())
1198-
}
1199-
),
1200-
ion_path,
1201-
));
1173+
let values: Vec<IonData<&Element>> = if let Some(element_iter) = value.as_sequence_iter() {
1174+
element_iter.map(IonData::from).collect()
1175+
} else if let Some(strukt) = value.as_struct() {
1176+
strukt.fields().map(|(k, v)| v).map(IonData::from).collect()
1177+
} else {
1178+
return Err(Violation::new(
1179+
"contains",
1180+
ViolationCode::TypeMismatched,
1181+
format!(
1182+
"expected list/sexp/struct/document found {}",
1183+
if value.is_null() {
1184+
format!("{value}")
1185+
} else {
1186+
format!("{}", value.ion_schema_type())
12021187
}
1203-
}
1204-
}
1205-
IonSchemaElement::Document(document) => document.to_owned(),
1188+
),
1189+
ion_path,
1190+
));
12061191
};
12071192

12081193
// add all the missing values found during validation
@@ -1256,36 +1241,24 @@ impl ConstraintValidator for ContainerLengthConstraint {
12561241
ion_path: &mut IonPath,
12571242
) -> ValidationResult {
12581243
// get the size of given value container
1259-
let size = match value {
1260-
IonSchemaElement::SingleElement(element) => {
1261-
// Check for null container
1262-
if element.is_null() {
1263-
return Err(Violation::new(
1264-
"container_length",
1265-
ViolationCode::TypeMismatched,
1266-
format!("expected a container found {element}"),
1267-
ion_path,
1268-
));
1269-
}
1270-
1271-
match element.ion_type() {
1272-
IonType::List | IonType::SExp => element.as_sequence().unwrap().len(),
1273-
IonType::Struct => element.as_struct().unwrap().iter().count(),
1274-
_ => {
1275-
// return Violation if value is not an Ion container
1276-
return Err(Violation::new(
1277-
"container_length",
1278-
ViolationCode::TypeMismatched,
1279-
format!(
1280-
"expected a container (a list/sexp/struct) but found {}",
1281-
element.ion_type()
1282-
),
1283-
ion_path,
1284-
));
1285-
}
1286-
}
1287-
}
1288-
IonSchemaElement::Document(document) => document.len(),
1244+
let size = if let Some(element_iter) = value.as_sequence_iter() {
1245+
element_iter.count()
1246+
} else if let Some(strukt) = value.as_struct() {
1247+
strukt.fields().map(|(k, v)| v).count()
1248+
} else {
1249+
return Err(Violation::new(
1250+
"container_length",
1251+
ViolationCode::TypeMismatched,
1252+
if value.is_null() {
1253+
format!("expected a container found {value}")
1254+
} else {
1255+
format!(
1256+
"expected a container (a list/sexp/struct) but found {}",
1257+
value.ion_schema_type()
1258+
)
1259+
},
1260+
ion_path,
1261+
));
12891262
};
12901263

12911264
// get isl length as a range
@@ -1438,58 +1411,38 @@ impl ConstraintValidator for ElementConstraint {
14381411
let mut element_set = vec![];
14391412

14401413
// get elements for given container in the form (ion_path_element, element_value)
1441-
let elements: Vec<(IonPathElement, &Element)> = match value {
1442-
IonSchemaElement::SingleElement(element) => {
1414+
let elements: Vec<(IonPathElement, &Element)> =
1415+
if let Some(element_iter) = value.as_sequence_iter() {
1416+
element_iter
1417+
.enumerate()
1418+
.map(|(index, val)| (IonPathElement::Index(index), val))
1419+
.collect()
1420+
} else if let Some(strukt) = value.as_struct() {
1421+
strukt
1422+
.fields()
1423+
.map(|(name, val)| (IonPathElement::Field(name.to_string()), val))
1424+
.collect()
1425+
} else {
14431426
// Check for null container
1444-
if element.is_null() {
1427+
if value.is_null() {
14451428
return Err(Violation::new(
14461429
"element",
14471430
ViolationCode::TypeMismatched,
1448-
format!("expected a container but found {element}"),
1431+
format!("expected a container but found {value}"),
14491432
ion_path,
14501433
));
14511434
}
1452-
1453-
// validate each element of the given value container
1454-
match element.ion_type() {
1455-
IonType::List | IonType::SExp => element
1456-
.as_sequence()
1457-
.unwrap()
1458-
.elements()
1459-
.enumerate()
1460-
.map(|(index, val)| (IonPathElement::Index(index), val))
1461-
.collect(),
1462-
IonType::Struct => element
1463-
.as_struct()
1464-
.unwrap()
1465-
.iter()
1466-
.map(|(field_name, val)| {
1467-
(
1468-
IonPathElement::Field(field_name.text().unwrap().to_owned()),
1469-
val,
1470-
)
1471-
})
1472-
.collect(),
1473-
_ => {
1474-
// return Violation if value is not an Ion container
1475-
return Err(Violation::new(
1476-
"element",
1477-
ViolationCode::TypeMismatched,
1478-
format!(
1479-
"expected a container (a list/sexp/struct) but found {}",
1480-
element.ion_type()
1481-
),
1482-
ion_path,
1483-
));
1484-
}
1485-
}
1486-
}
1487-
IonSchemaElement::Document(document) => document
1488-
.iter()
1489-
.enumerate()
1490-
.map(|(index, val)| (IonPathElement::Index(index), val))
1491-
.collect(),
1492-
};
1435+
// return Violation if value is not an Ion container
1436+
return Err(Violation::new(
1437+
"element",
1438+
ViolationCode::TypeMismatched,
1439+
format!(
1440+
"expected a container (a list/sexp/struct) but found {}",
1441+
value.ion_schema_type()
1442+
),
1443+
ion_path,
1444+
));
1445+
};
14931446

14941447
// validate element constraint
14951448
for (ion_path_element, val) in elements {

0 commit comments

Comments
 (0)