Use enum for source inclusion and compression (#997)

Author: jeffcharles

crates/cli/src/commands.rs

@@ -171,6 +171,16 @@ where
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum Source {
+    /// Omit the source code.
+    Omitted,
+    /// Compress the source code.
+    Compressed,
+    /// Include source code but don't compress it.
+    Uncompressed,
+}
+
 /// Code generation option group.
 /// This group gets configured from the [`CodegenOption`] enum.
 //
@@ -180,8 +190,7 @@ where
 pub struct CodegenOptionGroup {
     pub dynamic: bool,
     pub wit: WitOptions,
-    pub source: bool,
-    pub source_compression: bool,
+    pub source: Source,
     pub plugin: Option<PathBuf>,
 }
 
@@ -190,8 +199,7 @@ impl Default for CodegenOptionGroup {
         Self {
             dynamic: false,
             wit: WitOptions::default(),
-            source: true,
-            source_compression: true,
+            source: Source::Compressed,
             plugin: None,
         }
     }
@@ -209,14 +217,12 @@ option_group! {
         /// Optional WIT world name for WIT file. Must be specified if WIT is
         /// file path is specified.
         WitWorld(String),
-        /// Embed the JavaScript source in a custom section in the generated
-        /// WebAssembly module.
-        Source(bool),
-        /// Enable source code compression, which generates smaller WebAssembly
-        /// files at the cost of increased compile time. This option has no effect
-        /// if the `source` shouldn't be embedded in the generated WebAssembly
-        /// module.
-        SourceCompression(bool),
+        /// How to embed the JavaScript source in a custom section in the generated
+        /// WebAssembly module. Options are `omitted`, `compressed`, and
+        /// `uncompressed`. `compressed` enables source code compression which
+        /// generates smaller WebAssembly files at the cost of increased compile
+        /// time.
+        Source(Source),
         /// Optional path to Javy plugin Wasm module. Required for dynamically
         /// linked modules. JavaScript config options are also not supported when
         /// using this parameter.
@@ -236,7 +242,6 @@ impl TryFrom<Vec<GroupOption<CodegenOption>>> for CodegenOptionGroup {
         let mut wit_specified = false;
         let mut wit_world_specified = false;
         let mut source_specified = false;
-        let mut source_compression_specified = false;
         let mut plugin_specified = false;
 
         for option in value.iter().flat_map(|i| i.0.iter()) {
@@ -262,20 +267,13 @@ impl TryFrom<Vec<GroupOption<CodegenOption>>> for CodegenOptionGroup {
                     wit_world = Some(world);
                     wit_world_specified = true;
                 }
-                CodegenOption::Source(enabled) => {
+                CodegenOption::Source(source) => {
                     if source_specified {
                         bail!("source can only be specified once");
                     }
-                    options.source = *enabled;
+                    options.source = *source;
                     source_specified = true;
                 }
-                CodegenOption::SourceCompression(enabled) => {
-                    if source_compression_specified {
-                        bail!("source-compression can only be specified once");
-                    }
-                    options.source_compression = *enabled;
-                    source_compression_specified = true;
-                }
                 CodegenOption::Plugin(path) => {
                     if plugin_specified {
                         bail!("plugin can only be specified once");
@@ -422,7 +420,7 @@ mod tests {
     use std::path::PathBuf;
 
     use crate::{
-        commands::{JsGroupOption, JsGroupValue},
+        commands::{JsGroupOption, JsGroupValue, Source},
         js_config::JsConfig,
         plugin::PLUGIN_MODULE,
         CliPlugin, Plugin, PluginKind,
@@ -536,19 +534,21 @@ mod tests {
 
         assert_eq!(group, expected);
 
-        let raw = vec![GroupOption(vec![CodegenOption::Source(false)])];
+        let raw = vec![GroupOption(vec![CodegenOption::Source(Source::Omitted)])];
         let group: CodegenOptionGroup = raw.try_into()?;
         let expected = CodegenOptionGroup {
-            source: false,
+            source: Source::Omitted,
             ..Default::default()
         };
 
         assert_eq!(group, expected);
 
-        let raw = vec![GroupOption(vec![CodegenOption::SourceCompression(false)])];
+        let raw = vec![GroupOption(vec![CodegenOption::Source(
+            Source::Uncompressed,
+        )])];
         let group: CodegenOptionGroup = raw.try_into()?;
         let expected = CodegenOptionGroup {
-            source_compression: false,
+            source: Source::Uncompressed,
             ..Default::default()
         };
 
@@ -597,13 +597,13 @@ mod tests {
         );
 
         let raw = vec![GroupOption(vec![
-            CodegenOption::SourceCompression(true),
-            CodegenOption::SourceCompression(false),
+            CodegenOption::Source(Source::Compressed),
+            CodegenOption::Source(Source::Uncompressed),
         ])];
         let result: Result<CodegenOptionGroup, Error> = raw.try_into();
         assert_eq!(
             result.err().unwrap().to_string(),
-            "source-compression can only be specified once"
+            "source can only be specified once"
         );
 
         let raw = vec![GroupOption(vec![

crates/cli/src/main.rs

@@ -91,15 +91,12 @@ fn main() -> Result<()> {
                 .wit_opts(codegen_opts.wit)
                 .js_runtime_config(js_opts.to_json()?);
 
-            if codegen_opts.source {
-                if codegen_opts.source_compression {
-                    generator.source_embedding(SourceEmbedding::Compressed);
-                } else {
-                    generator.source_embedding(SourceEmbedding::Uncompressed);
-                }
-            } else {
-                generator.source_embedding(SourceEmbedding::Omitted);
-            }
+            let source_embedding = match codegen_opts.source {
+                commands::Source::Omitted => SourceEmbedding::Omitted,
+                commands::Source::Compressed => SourceEmbedding::Compressed,
+                commands::Source::Uncompressed => SourceEmbedding::Uncompressed,
+            };
+            generator.source_embedding(source_embedding);
 
             set_producer_version(&mut generator);
 

crates/cli/src/option.rs

@@ -1,6 +1,8 @@
 use anyhow::{bail, Result};
 use std::path::PathBuf;
 
+use crate::commands;
+
 /// An option group used for parsing strings to their group option representation.
 #[derive(Clone, Debug)]
 pub struct GroupOption<T>(pub Vec<T>);
@@ -173,3 +175,22 @@ impl OptionValue for PathBuf {
         }
     }
 }
+
+impl OptionValue for commands::Source {
+    fn help() -> &'static str {
+        "[=omitted|compressed|uncompressed]"
+    }
+
+    fn parse(val: Option<&str>) -> Result<Self>
+    where
+        Self: Sized,
+    {
+        match val {
+            Some("omitted") => Ok(Self::Omitted),
+            Some("compressed") => Ok(Self::Compressed),
+            Some("uncompressed") => Ok(Self::Uncompressed),
+            Some(_) => bail!("Unexpected flag. Valid options: omitted, compressed, uncompressed"),
+            None => bail!("Expected source argument"),
+        }
+    }
+}

crates/cli/tests/integration_test.rs

@@ -1,5 +1,5 @@
 use anyhow::{bail, Result};
-use javy_runner::{Builder, Plugin, Runner, RunnerError};
+use javy_runner::{Builder, Plugin, Runner, RunnerError, Source};
 use std::{fs, io::Read, path::PathBuf, process::Command, str};
 use wasmparser::Parser;
 use wasmtime::{AsContextMut, Engine, Linker, Module, Store};
@@ -287,8 +287,8 @@ fn test_source_code_default(builder: &mut Builder) -> Result<()> {
 }
 
 #[javy_cli_test]
-fn test_source_code_compression_enabled(builder: &mut Builder) -> Result<()> {
-    let runner = builder.compress_source_code(true).build()?;
+fn test_source_code_compressed(builder: &mut Builder) -> Result<()> {
+    let runner = builder.source_code(Source::Compressed).build()?;
     let javy_source = runner
         .javy_source_custom_section()
         .expect("Should have javy_source custom section");
@@ -300,8 +300,8 @@ fn test_source_code_compression_enabled(builder: &mut Builder) -> Result<()> {
 }
 
 #[javy_cli_test]
-fn test_source_code_compression_disabled(builder: &mut Builder) -> Result<()> {
-    let runner = builder.compress_source_code(false).build()?;
+fn test_source_code_uncompressed(builder: &mut Builder) -> Result<()> {
+    let runner = builder.source_code(Source::Uncompressed).build()?;
     let javy_source = runner
         .javy_source_custom_section()
         .expect("Should have javy_source custom section");
@@ -314,24 +314,14 @@ fn test_source_code_compression_disabled(builder: &mut Builder) -> Result<()> {
 
 #[javy_cli_test(commands(not(Compile)))]
 fn test_source_code_omitted(builder: &mut Builder) -> Result<()> {
-    let runner = builder.source_code(false).build()?;
+    let runner = builder.source_code(Source::Omitted).build()?;
     assert!(
         runner.javy_source_custom_section().is_none(),
         "Should not have a source code section"
     );
     Ok(())
 }
 
-#[javy_cli_test(commands(not(Compile)))]
-fn test_source_code_not_omitted(builder: &mut Builder) -> Result<()> {
-    let runner = builder.source_code(true).build()?;
-    assert!(
-        runner.javy_source_custom_section().is_some(),
-        "Should have a source code section"
-    );
-    Ok(())
-}
-
 #[test]
 fn test_init_plugin() -> Result<()> {
     let engine = Engine::default();

crates/runner/src/lib.rs

@@ -27,6 +27,13 @@ pub enum Plugin {
     InvalidUser,
 }
 
+#[derive(Debug, Clone)]
+pub enum Source {
+    Omitted,
+    Compressed,
+    Uncompressed,
+}
+
 impl Plugin {
     pub fn namespace(&self) -> &'static str {
         match self {
@@ -95,10 +102,8 @@ pub struct Builder {
     command: JavyCommand,
     /// The javy plugin.
     plugin: Plugin,
-    /// Whether to compress the source code.
-    compress_source_code: Option<bool>,
-    /// Whether to include the source code.
-    source_code: Option<bool>,
+    /// How to embed the source code.
+    source_code: Option<Source>,
 }
 
 impl Default for Builder {
@@ -117,7 +122,6 @@ impl Default for Builder {
             text_encoding: None,
             event_loop: None,
             plugin: Plugin::Default,
-            compress_source_code: None,
             source_code: None,
         }
     }
@@ -184,13 +188,8 @@ impl Builder {
         self
     }
 
-    pub fn compress_source_code(&mut self, enabled: bool) -> &mut Self {
-        self.compress_source_code = Some(enabled);
-        self
-    }
-
-    pub fn source_code(&mut self, enabled: bool) -> &mut Self {
-        self.source_code = Some(enabled);
+    pub fn source_code(&mut self, source: Source) -> &mut Self {
+        self.source_code = Some(source);
         self
     }
 
@@ -219,14 +218,20 @@ impl Builder {
             preload,
             command,
             plugin,
-            compress_source_code,
             source_code,
         } = std::mem::take(self);
 
         self.built = true;
 
         match command {
             JavyCommand::Compile => {
+                let compress_source = source_code
+                    .map(|s| match s {
+                        Source::Omitted => bail!("Unsupported for compile"),
+                        Source::Compressed => Ok(true),
+                        Source::Uncompressed => Ok(false),
+                    })
+                    .transpose()?;
                 if let Some(preload) = preload {
                     Runner::compile_dynamic(
                         bin_path,
@@ -235,10 +240,10 @@ impl Builder {
                         wit,
                         world,
                         preload,
-                        compress_source_code,
+                        compress_source,
                     )
                 } else {
-                    Runner::compile_static(bin_path, root, input, wit, world, compress_source_code)
+                    Runner::compile_static(bin_path, root, input, wit, world, compress_source)
                 }
             }
             JavyCommand::Build => Runner::build(
@@ -253,7 +258,6 @@ impl Builder {
                 event_loop,
                 preload,
                 plugin,
-                compress_source_code,
                 source_code,
             ),
         }
@@ -324,8 +328,7 @@ impl Runner {
         event_loop: Option<bool>,
         preload: Option<(String, PathBuf)>,
         plugin: Plugin,
-        compress_source_code: Option<bool>,
-        source_code: Option<bool>,
+        source_code: Option<Source>,
     ) -> Result<Self> {
         // This directory is unique and will automatically get deleted
         // when `tempdir` goes out of scope.
@@ -345,7 +348,6 @@ impl Runner {
             &text_encoding,
             &event_loop,
             &plugin,
-            &compress_source_code,
             &source_code,
         );
 
@@ -534,8 +536,7 @@ impl Runner {
         text_encoding: &Option<bool>,
         event_loop: &Option<bool>,
         plugin: &Plugin,
-        compress_source_code: &Option<bool>,
-        source_code: &Option<bool>,
+        source_code: &Option<Source>,
     ) -> Vec<String> {
         let mut args = vec![
             "build".to_string(),
@@ -587,19 +588,18 @@ impl Runner {
             args.push(format!("plugin={}", plugin.path().to_str().unwrap()));
         }
 
-        if let Some(enabled) = *compress_source_code {
+        if let Some(source) = source_code {
             args.push("-C".into());
             args.push(format!(
-                "source-compression={}",
-                if enabled { "y" } else { "n" }
+                "source={}",
+                match source {
+                    Source::Omitted => "omitted",
+                    Source::Compressed => "compressed",
+                    Source::Uncompressed => "uncompressed",
+                }
             ));
         }
 
-        if let Some(enabled) = *source_code {
-            args.push("-C".into());
-            args.push(format!("source={}", if enabled { "y" } else { "n" }));
-        }
-
         args
     }