Promotes head, improves inspect (#118)

* `head` is no longer in the `beta` namespace * Bumps `ion_rs` dependency to v1.0.0-rc.6 * `inspect` confirms its input is a supported encoding * Fixes a bug in symbol ID assignments * Removes workaround for `body_span` now-fixed bug in `ion_rs` * Makes `--no-auto-decompress` a supported-by-default flag for all subcommands. * Hides the `dump` alias for `cat` from `--help`'s list of subcommands.
amazon-ion · Jun 6, 2024 · 52740fe · 52740fe
1 parent b23594c
commit 52740fe
Show file tree

Hide file tree

Showing 11 changed files with 280 additions and 266 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -17,7 +17,7 @@ clap = { version = "4.0.17", features = ["cargo"] }
 colored = "2.0.0"
 flate2 = "1.0"
 infer = "0.15.0"
-ion-rs = { version = "1.0.0-rc.5", features = ["experimental"] }
+ion-rs = { version = "1.0.0-rc.6", features = ["experimental"] }
 tempfile = "3.2.0"
 ion-schema = "0.10.0"
 serde = { version = "1.0.163", features = ["derive"] }

diff --git a/src/bin/ion/commands/beta/mod.rs b/src/bin/ion/commands/beta/mod.rs
@@ -3,7 +3,6 @@ pub mod from;
 
 #[cfg(feature = "experimental-code-gen")]
 pub mod generate;
-pub mod head;
 pub mod primitive;
 pub mod schema;
 pub mod symtab;
@@ -13,7 +12,6 @@ use crate::commands::beta::count::CountCommand;
 use crate::commands::beta::from::FromNamespace;
 #[cfg(feature = "experimental-code-gen")]
 use crate::commands::beta::generate::GenerateCommand;
-use crate::commands::beta::head::HeadCommand;
 use crate::commands::beta::primitive::PrimitiveCommand;
 use crate::commands::beta::schema::SchemaNamespace;
 use crate::commands::beta::symtab::SymtabNamespace;
@@ -36,7 +34,6 @@ impl IonCliCommand for BetaNamespace {
             Box::new(CountCommand),
             Box::new(PrimitiveCommand),
             Box::new(SchemaNamespace),
-            Box::new(HeadCommand),
             Box::new(FromNamespace),
             Box::new(ToNamespace),
             Box::new(SymtabNamespace),

diff --git a/src/bin/ion/commands/beta/symtab/filter.rs b/src/bin/ion/commands/beta/symtab/filter.rs
@@ -21,7 +21,6 @@ impl IonCliCommand for SymtabFilterCommand {
     fn configure_args(&self, command: Command) -> Command {
         command.with_input()
             .with_output()
-            .with_compression_control()
             .arg(Arg::new("lift")
                 .long("lift")
                 .short('l')

diff --git a/src/bin/ion/commands/beta/to/json.rs b/src/bin/ion/commands/beta/to/json.rs
@@ -24,10 +24,7 @@ impl IonCliCommand for ToJsonCommand {
     fn configure_args(&self, command: Command) -> Command {
         // NOTE: it may be necessary to add format-specific options. For example, a "pretty" option
         // would make sense for JSON, but not binary formats like CBOR.
-        command
-            .with_input()
-            .with_output()
-            .with_compression_control()
+        command.with_input().with_output()
     }
 
     fn run(&self, _command_path: &mut Vec<String>, args: &ArgMatches) -> Result<()> {

diff --git a/src/bin/ion/commands/cat.rs b/src/bin/ion/commands/cat.rs
@@ -17,11 +17,7 @@ impl IonCliCommand for CatCommand {
     }
 
     fn configure_args(&self, command: Command) -> Command {
-        command
-            .with_input()
-            .with_output()
-            .with_format()
-            .with_compression_control()
+        command.with_input().with_output().with_format()
     }
 
     fn run(&self, _command_path: &mut Vec<String>, args: &ArgMatches) -> Result<()> {

diff --git a/src/bin/ion/commands/beta/head.rs → src/bin/ion/commands/head.rs b/src/bin/ion/commands/beta/head.rs → src/bin/ion/commands/head.rs
diff --git a/src/bin/ion/commands/inspect.rs b/src/bin/ion/commands/inspect.rs
@@ -2,7 +2,7 @@ use std::fmt::Display;
 use std::io::Write;
 use std::str::FromStr;
 
-use anyhow::{Context, Result};
+use anyhow::{bail, Context, Result};
 use clap::{Arg, ArgMatches, Command};
 use ion_rs::v1_0::{LazyRawBinaryValue, RawValueRef};
 use ion_rs::*;
@@ -187,22 +187,39 @@ impl<'a, 'b> IonInspector<'a, 'b> {
         Ok(inspector)
     }
 
+    fn confirm_encoding_is_supported(&self, encoding: IonEncoding) -> Result<()> {
+        use IonEncoding::*;
+        match encoding {
+            Text_1_0 | Text_1_1 => {
+                bail!("`inspect` does not support text Ion streams.");
+            }
+            Binary_1_0 => Ok(()),
+            Binary_1_1 => bail!("`inspect` does not yet support binary Ion v1.1"),
+            // `IonEncoding` is #[non_exhaustive]
+            _ => bail!("`inspect does not yet support {}", encoding.name()),
+        }
+    }
+
     /// Iterates over the items in `reader`, printing a table section for each top level value.
     fn inspect_top_level<Input: IonInput>(
         &mut self,
         reader: &mut SystemReader<AnyEncoding, Input>,
     ) -> Result<()> {
         const TOP_LEVEL_DEPTH: usize = 0;
-        self.write_table_header()?;
+
         let mut is_first_item = true;
         let mut has_printed_skip_message = false;
         loop {
-            // TODO: This does not account for shared symbol table imports. However, the CLI does not
-            //       yet support specifying a catalog, so it's correct enough for the moment.
-            let mut next_symbol_id = reader.symbol_table().len();
             let item = reader.next_item()?;
             let is_last_item = matches!(item, SystemStreamItem::EndOfStream(_));
 
+            if is_first_item {
+                // The first item in a stream cannot be ephemeral, so we can safely unwrap this.
+                let encoding = item.raw_stream_item().unwrap().encoding();
+                self.confirm_encoding_is_supported(encoding)?;
+                self.write_table_header()?;
+            }
+
             match self.select_action(
                 TOP_LEVEL_DEPTH,
                 &mut has_printed_skip_message,
@@ -222,17 +239,13 @@ impl<'a, 'b> IonInspector<'a, 'b> {
 
             match item {
                 SystemStreamItem::SymbolTable(lazy_struct) => {
-                    let is_append = lazy_struct.get("imports")?
-                        == Some(ValueRef::Symbol(SymbolRef::with_text("$ion_symbol_table")));
-                    if !is_append {
-                        next_symbol_id = 10; // First available SID after system symbols in Ion 1.0
-                    }
-                    self.inspect_symbol_table(next_symbol_id, lazy_struct)?;
+                    self.inspect_symbol_table(lazy_struct)?;
                 }
                 SystemStreamItem::Value(lazy_value) => {
                     self.inspect_value(0, "", lazy_value, no_comment())?;
                 }
                 SystemStreamItem::VersionMarker(marker) => {
+                    self.confirm_encoding_is_supported(marker.encoding())?;
                     self.inspect_ivm(marker)?;
                 }
                 SystemStreamItem::EndOfStream(_) => {
@@ -462,11 +475,7 @@ impl<'a, 'b> IonInspector<'a, 'b> {
         }
     }
 
-    fn inspect_symbol_table(
-        &mut self,
-        next_symbol_id: usize,
-        struct_: LazyStruct<'_, AnyEncoding>,
-    ) -> Result<()> {
+    fn inspect_symbol_table(&mut self, struct_: LazyStruct<'_, AnyEncoding>) -> Result<()> {
         let value = struct_.as_value();
         if value.has_annotations() {
             self.newline()?;
@@ -479,9 +488,7 @@ impl<'a, 'b> IonInspector<'a, 'b> {
 
         use LazyRawValueKind::*;
         match raw_struct.as_value().kind() {
-            Binary_1_0(v) => {
-                self.inspect_binary_1_0_symbol_table(next_symbol_id, struct_, raw_struct, v)
-            }
+            Binary_1_0(v) => self.inspect_binary_1_0_symbol_table(struct_, raw_struct, v),
             Binary_1_1(_) => todo!("Binary Ion 1.1 symbol table"),
             Text_1_0(_) | Text_1_1(_) => unreachable!("text value"),
         }
@@ -742,7 +749,6 @@ impl<'a, 'b> IonInspector<'a, 'b> {
 
     fn inspect_binary_1_0_symbol_table(
         &mut self,
-        next_symbol_id: usize,
         struct_: LazyStruct<AnyEncoding>,
         raw_struct: LazyRawAnyStruct,
         raw_value: LazyRawBinaryValue,
@@ -771,7 +777,7 @@ impl<'a, 'b> IonInspector<'a, 'b> {
             )? {
                 InspectorAction::Skip => continue,
                 InspectorAction::Inspect if field.name()? == "symbols" => {
-                    self.inspect_lst_symbols_field(next_symbol_id, field, raw_field)?
+                    self.inspect_lst_symbols_field(struct_, field, raw_field)?
                 }
                 // TODO: if field.name()? == "imports" => {}
                 InspectorAction::Inspect => {
@@ -791,7 +797,7 @@ impl<'a, 'b> IonInspector<'a, 'b> {
 
     fn inspect_lst_symbols_field(
         &mut self,
-        mut next_symbol_id: usize,
+        symtab_struct: LazyStruct<AnyEncoding>,
         field: LazyField<AnyEncoding>,
         raw_field: LazyRawFieldExpr<AnyEncoding>,
     ) -> Result<()> {
@@ -825,6 +831,16 @@ impl<'a, 'b> IonInspector<'a, 'b> {
             Ok(())
         })?;
 
+        // TODO: This does not account for shared symbol table imports. However, the CLI does not
+        //       yet support specifying a catalog, so it's correct enough for the moment.
+        let symtab_value = symtab_struct.as_value();
+        let mut next_symbol_id = symtab_value.symbol_table().len();
+        let is_append = symtab_struct.get("imports")?
+            == Some(ValueRef::Symbol(SymbolRef::with_text("$ion_symbol_table")));
+        if !is_append {
+            next_symbol_id = 10; // First available SID after system symbols in Ion 1.0
+        }
+
         let mut has_printed_skip_message = false;
         for (raw_value_res, value_res) in nested_raw_values.zip(nested_values) {
             let (raw_nested_value, nested_value) = (raw_value_res?, value_res?);
@@ -925,15 +941,7 @@ impl<'a, 'b> IonInspector<'a, 'b> {
             BytesKind::TrailingLength,
             encoding.trailing_length_span().bytes(),
         );
-        // TODO: There is a bug in the `body_span()` method that causes it fail when the value is annotated.
-        //       When it's fixed, this can be:
-        //           let body_bytes = IonBytes::new(BytesKind::ValueBody, body_span);
-        let body_len = raw_value.encoded_data().body_range().len();
-        let total_len = raw_value.encoded_data().range().len();
-        let body_bytes = IonBytes::new(
-            BytesKind::ValueBody,
-            &encoding.span().bytes()[total_len - body_len..],
-        );
+        let body_bytes = IonBytes::new(BytesKind::ValueBody, encoding.body_span().bytes());
 
         let mut formatter =
             BytesFormatter::new(BYTES_PER_ROW, vec![opcode_bytes, length_bytes, body_bytes]);

diff --git a/src/bin/ion/commands/mod.rs b/src/bin/ion/commands/mod.rs
@@ -11,6 +11,7 @@ use termcolor::{ColorChoice, StandardStream, StandardStreamLock};
 pub mod beta;
 pub mod cat;
 pub mod dump;
+pub mod head;
 pub mod inspect;
 
 /// Behaviors common to all Ion CLI commands, including both namespaces (groups of commands)
@@ -51,7 +52,8 @@ pub trait IonCliCommand {
             .about(self.about())
             .version(crate_version!())
             .author(crate_authors!())
-            .hide(self.hide_from_help_message());
+            .hide(self.hide_from_help_message())
+            .with_compression_control();
 
         // If there are subcommands, add them to the configuration and set 'subcommand_required'.
         if !clap_subcommands.is_empty() {
@@ -175,8 +177,8 @@ impl<'a> CommandIo<'a> {
 
     /// Returns `true` if the user has not explicitly disabled auto decompression.
     fn auto_decompression_enabled(&self) -> bool {
-        if let Some(flag) = self.args.get_one::<bool>("no-auto-decompress") {
-            !*flag
+        if let Some(is_disabled) = self.args.get_one::<bool>("no-auto-decompress") {
+            !*is_disabled
         } else {
             true
         }

diff --git a/src/bin/ion/main.rs b/src/bin/ion/main.rs
@@ -13,6 +13,7 @@ use ion_rs::IonError;
 use std::io::ErrorKind;
 
 use crate::commands::dump::DumpCommand;
+use crate::commands::head::HeadCommand;
 use crate::commands::inspect::InspectCommand;
 
 fn main() -> Result<()> {
@@ -48,8 +49,9 @@ impl IonCliCommand for RootCommand {
     fn subcommands(&self) -> Vec<Box<dyn IonCliCommand>> {
         vec![
             Box::new(BetaNamespace),
-            Box::new(DumpCommand),
             Box::new(CatCommand),
+            Box::new(DumpCommand),
+            Box::new(HeadCommand),
             Box::new(InspectCommand),
         ]
     }

diff --git a/tests/cli.rs b/tests/cli.rs
@@ -201,7 +201,6 @@ fn test_write_all_values(#[case] number: i32, #[case] expected_output: &str) ->
     input_file.write_all(test_data.as_bytes())?;
     input_file.flush()?;
     cmd.args([
-        "beta",
         "head",
         "--values",
         &number.to_string(),