Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ion-schema and ion-rs dependencies #151

Merged
merged 1 commit into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
558 changes: 264 additions & 294 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ sha2 = "0.9"
sha3 = "0.9"
flate2 = "1.0"
infer = "0.15.0"
ion-rs = { version = "1.0.0-rc.7", features = ["experimental", "experimental-ion-hash"] }
ion-rs = { version = "1.0.0-rc.8", features = ["experimental", "experimental-ion-hash"] }
tempfile = "3.2.0"
ion-schema = "0.10.0"
ion-schema = "0.14.0"
lowcharts = "0.5.8"
serde = { version = "1.0.163", features = ["derive"] }
serde_json = { version = "1.0.81", features = ["arbitrary_precision", "preserve_order"] }
Expand Down
6 changes: 3 additions & 3 deletions src/bin/ion/commands/generate/generator.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗺️ Clippy recommended fixes because there was a double reference going on when passing &isl_type_name.

Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ impl<'a, L: Language + 'static> CodeGenerator<'a, L> {
// Iterate through the ISL types, generate an abstract data type for each
for isl_type in schema.types() {
// unwrap here is safe because all the top-level type definition always has a name
let isl_type_name = isl_type.name().clone().unwrap();
self.generate_abstract_data_type(&isl_type_name, isl_type)?;
let isl_type_name = isl_type.name().unwrap();
self.generate_abstract_data_type(isl_type_name, isl_type)?;
}

Ok(())
Expand Down Expand Up @@ -266,7 +266,7 @@ impl<'a, L: Language + 'static> CodeGenerator<'a, L> {

fn generate_abstract_data_type(
&mut self,
isl_type_name: &String,
isl_type_name: &str,
isl_type: &IslType,
) -> CodeGenResult<()> {
let mut context = Context::new();
Expand Down
88 changes: 42 additions & 46 deletions src/bin/ion/commands/generate/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,31 +391,28 @@ mod model_tests {
name: vec!["org".to_string(), "example".to_string(), "Foo".to_string()],
doc_comment: "This is a structure".to_string(),
is_closed: false,
fields: HashMap::from_iter(
vec![
(
"foo".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["String".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
fields: HashMap::from_iter(vec![
(
"foo".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["String".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
(
"bar".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["int".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
),
(
"bar".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["int".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
]
.into_iter(),
),
),
]),
source: anonymous_type(vec![
type_constraint(named_type_ref("struct")),
fields(
Expand All @@ -436,6 +433,7 @@ mod model_tests {
),
]
.into_iter(),
true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗺️ Adding true is the only change in this file that's required by the version updates. The others are clippy/fmt fixes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the true here? Can you comment it or give it a name? Random booleans in interfaces grow like rust as codebases age, and inhibit readability and maintainability unless you stay on top of it, imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

),
]),
};
Expand All @@ -451,31 +449,28 @@ mod model_tests {
])
.doc_comment("This is a structure")
.is_closed(false)
.fields(HashMap::from_iter(
vec![
(
"foo".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["String".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
.fields(HashMap::from_iter(vec![
(
"foo".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["String".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
(
"bar".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["int".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
),
(
"bar".to_string(),
FieldReference(
FullyQualifiedTypeReference {
type_name: vec!["int".to_string()],
parameters: vec![],
},
FieldPresence::Required,
),
]
.into_iter(),
))
),
]))
.source(anonymous_type(vec![
type_constraint(named_type_ref("struct")),
fields(
Expand All @@ -496,6 +491,7 @@ mod model_tests {
),
]
.into_iter(),
true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing- what's the true?

Copy link
Contributor Author

@popematt popematt Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of the ion schema model. It is something to do with whether the fields are closed for a given type. I'll create an issue to replace it with an enum.

),
]));

Expand Down
12 changes: 7 additions & 5 deletions src/bin/ion/commands/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ impl<'a, 'b> IonInspector<'a, 'b> {
match invocation.kind() {
EExp(eexp_arg) => self.inspect_eexp(depth + 1, eexp_arg)?,
EExpArgGroup(_) => todo!("e-exp arg groups"),
TemplateMacro(_) => {
TemplateMacro(_) | TemplateArgGroup(_) => {
unreachable!("e-exp args by definition cannot be template invocations")
}
}
Expand Down Expand Up @@ -545,6 +545,9 @@ impl<'a, 'b> IonInspector<'a, 'b> {
Template(_, _element) => {
self.inspect_ephemeral_sequence(depth, "(", "", ")", delimiter, sexp, no_comment())
}
Constructed(_, _) => {
todo!()
}
}
}

Expand Down Expand Up @@ -662,10 +665,9 @@ impl<'a, 'b> IonInspector<'a, 'b> {
depth,
element.annotations().iter().map(|s| Ok(SymbolRef::from(s))),
),
ExpandedValueSource::Constructed(annotations, _) => self.inspect_ephemeral_annotations(
depth,
annotations.iter().copied().map(|s| Ok(s.into())),
),
ExpandedValueSource::Constructed(annotations, _) => {
self.inspect_ephemeral_annotations(depth, annotations.iter().copied().map(Ok))
}
Comment on lines -665 to +670
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗺️ This was a Clippy fix.

ExpandedValueSource::SingletonEExp(eexp) => self.inspect_ephemeral_annotations(
depth,
eexp.require_singleton_annotations().map(|s| Ok(s.into())),
Expand Down
48 changes: 11 additions & 37 deletions src/bin/ion/commands/schema/validate.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗺️ Now that the dependencies are updated, we can refactor the validate command to use the CommandIo trait like the other commands—but I'm saving that for another day.

Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
use crate::commands::IonCliCommand;
use anyhow::{Context, Result};
use clap::{Arg, ArgAction, ArgMatches, Command};
use ion_rs::{v1_0, Element, Sequence, SequenceWriter, StructWriter, TextFormat, Writer};
use ion_schema::authority::{DocumentAuthority, FileSystemDocumentAuthority};
use ion_schema::external::ion_rs::element::reader::ElementReader;
use ion_schema::external::ion_rs::element::writer::ElementWriter;
use ion_schema::external::ion_rs::element::writer::TextKind;
use ion_schema::external::ion_rs::element::Element;
use ion_schema::external::ion_rs::{IonResult, TextWriterBuilder};
use ion_schema::external::ion_rs::{IonType, IonWriter, ReaderBuilder};
use ion_schema::system::SchemaSystem;
use std::fs;
use std::path::Path;
Expand Down Expand Up @@ -87,9 +82,7 @@ impl IonCliCommand for ValidateCommand {
let input_file = args.get_one::<String>("input").unwrap();
let value =
fs::read(input_file).with_context(|| format!("Could not open '{}'", schema_id))?;
let owned_elements: Vec<Element> = ReaderBuilder::new()
.build(value.as_slice())?
.read_all_elements()
let elements: Sequence = Element::read_all(value)
.with_context(|| format!("Could not parse Ion file: '{}'", schema_id))?;

// Set up document authorities vector
Expand All @@ -113,46 +106,27 @@ impl IonCliCommand for ValidateCommand {
.with_context(|| format!("Schema {} does not have type {}", schema_id, schema_type))?;

// create a text writer to make the output
let mut output = vec![];
let mut writer = TextWriterBuilder::new(TextKind::Pretty).build(&mut output)?;
let mut writer = Writer::new(v1_0::Text.with_format(TextFormat::Pretty), vec![])?;

// validate owned_elements according to type_ref
for owned_element in owned_elements {
for owned_element in elements {
// create a validation report with validation result, value, schema and/or violation
writer.step_in(IonType::Struct)?;
let mut struct_writer = writer.struct_writer()?;
let validation_result = type_ref.validate(&owned_element);
writer.set_field_name("result");
match validation_result {
Ok(_) => {
writer.write_string("Valid")?;
writer.set_field_name("value");
writer.write_string(element_to_string(&owned_element)?)?;
writer.set_field_name("schema");
writer.write_string(schema_id)?;
struct_writer.write("result", "Valid")?;
struct_writer.write("value", format!("{}", &owned_element))?;
struct_writer.write("schema", schema_id)?;
Comment on lines -127 to +120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struct_writer really cleans this up, nice :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, @zslayton put a lot of effort into trying different approaches in order to find an ergonomic writer API.

}
Err(error) => {
writer.write_string("Invalid")?;
writer.set_field_name("violation");
writer.write_string(format!("{:#?}", error))?;
struct_writer.write("result", "Invalid")?;
struct_writer.write("violation", format!("{:#?}", error))?;
}
}
writer.step_out()?;
}
drop(writer);
println!("Validation report:");
println!("{}", from_utf8(&output).unwrap());
println!("{}", from_utf8(writer.output()).unwrap());
Ok(())
}
}

// TODO: this will be provided by Element's implementation of `Display` in a future
// release of ion-rs.
fn element_to_string(element: &Element) -> IonResult<String> {
let mut buffer = Vec::new();
let mut text_writer = TextWriterBuilder::new(TextKind::Pretty).build(&mut buffer)?;
text_writer.write_element(element)?;
text_writer.flush()?;
Ok(from_utf8(text_writer.output().as_slice())
.expect("Invalid UTF-8 output")
.to_string())
}
5 changes: 2 additions & 3 deletions src/bin/ion/commands/stats.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗺️ Also a clippy fix.

Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,12 @@ fn analyze_data_stream<Input: IonInput>(reader: &mut SystemReader<AnyEncoding, I
// Reduce the number of shared symbols.
let symbols_count = reader.symbol_table().symbols().iter().len() - 10;

let out = Output {
Output {
size_vec,
symtab_count,
symbols_count,
max_depth,
};
return out;
}
}

fn top_level_max_depth(value: LazyValue<AnyEncoding>) -> usize {
Expand Down
1 change: 0 additions & 1 deletion tests/code-gen-tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use anyhow::Result;
use assert_cmd::Command;
use rstest::rstest;
use std::fmt::format;
use std::fs::File;
use std::io::Write;
use std::path::PathBuf;
Expand Down
Loading