Refactorings for build and check (use-ink#116)

* Make naming consistent

* Fix number of steps displayed in `check`

* Move base name function to utilities

* Shift logic to `BuildCommand` and `CheckCommand`

* Consistent naming: GenerationResult ➜ BuildResult

* Consistent naming: GenerateArtifacts ➜ BuildArtifacts

* Update comment

* Move comment to struct

* Remove Note section from comment

src/cmd/build.rs

@@ -15,6 +15,7 @@
 // along with cargo-contract.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::{
+    convert::TryFrom,
     fs::{metadata, File},
     io::{Read, Write},
     path::PathBuf,
@@ -24,15 +25,89 @@ use crate::{
     crate_metadata::CrateMetadata,
     util,
     workspace::{ManifestPath, Profile, Workspace},
-    GenerateArtifacts, GenerationResult, OptimizationResult, UnstableFlags, Verbosity,
+    BuildArtifacts, BuildResult, UnstableFlags, UnstableOptions, VerbosityFlags,
 };
+use crate::{OptimizationResult, Verbosity};
 use anyhow::{Context, Result};
 use colored::Colorize;
 use parity_wasm::elements::{External, MemoryType, Module, Section};
+use structopt::StructOpt;
 
 /// This is the maximum number of pages available for a contract to allocate.
 const MAX_MEMORY_PAGES: u32 = 16;
 
+/// Executes build of the smart-contract which produces a wasm binary that is ready for deploying.
+///
+/// It does so by invoking `cargo build` and then post processing the final binary.
+#[derive(Debug, StructOpt)]
+#[structopt(name = "build")]
+pub struct BuildCommand {
+    /// Path to the Cargo.toml of the contract to build
+    #[structopt(long, parse(from_os_str))]
+    manifest_path: Option<PathBuf>,
+    /// Which build artifacts to generate.
+    ///
+    /// - `all`: Generate the Wasm, the metadata and a bundled `<name>.contract` file.
+    ///
+    /// - `code-only`: Only the Wasm is created, generation of metadata and a bundled
+    ///   `<name>.contract` file is skipped.
+    #[structopt(
+        long = "generate",
+        default_value = "all",
+        value_name = "all | code-only",
+        verbatim_doc_comment
+    )]
+    build_artifact: BuildArtifacts,
+    #[structopt(flatten)]
+    verbosity: VerbosityFlags,
+    #[structopt(flatten)]
+    unstable_options: UnstableOptions,
+}
+
+impl BuildCommand {
+    pub fn exec(&self) -> Result<BuildResult> {
+        let manifest_path = ManifestPath::try_from(self.manifest_path.as_ref())?;
+        let unstable_flags: UnstableFlags =
+            TryFrom::<&UnstableOptions>::try_from(&self.unstable_options)?;
+        let verbosity: Option<Verbosity> = TryFrom::<&VerbosityFlags>::try_from(&self.verbosity)?;
+        execute(
+            &manifest_path,
+            verbosity,
+            true,
+            self.build_artifact,
+            unstable_flags,
+        )
+    }
+}
+
+#[derive(Debug, StructOpt)]
+#[structopt(name = "check")]
+pub struct CheckCommand {
+    /// Path to the Cargo.toml of the contract to build
+    #[structopt(long, parse(from_os_str))]
+    manifest_path: Option<PathBuf>,
+    #[structopt(flatten)]
+    verbosity: VerbosityFlags,
+    #[structopt(flatten)]
+    unstable_options: UnstableOptions,
+}
+
+impl CheckCommand {
+    pub fn exec(&self) -> Result<BuildResult> {
+        let manifest_path = ManifestPath::try_from(self.manifest_path.as_ref())?;
+        let unstable_flags: UnstableFlags =
+            TryFrom::<&UnstableOptions>::try_from(&self.unstable_options)?;
+        let verbosity: Option<Verbosity> = TryFrom::<&VerbosityFlags>::try_from(&self.verbosity)?;
+        execute(
+            &manifest_path,
+            verbosity,
+            false,
+            BuildArtifacts::CheckOnly,
+            unstable_flags,
+        )
+    }
+}
+
 /// Builds the project in the specified directory, defaults to the current directory.
 ///
 /// Uses the unstable cargo feature [`build-std`](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#build-std)
@@ -51,7 +126,7 @@ const MAX_MEMORY_PAGES: u32 = 16;
 fn build_cargo_project(
     crate_metadata: &CrateMetadata,
     verbosity: Option<Verbosity>,
-    unstable_options: UnstableFlags,
+    unstable_flags: UnstableFlags,
 ) -> Result<()> {
     util::assert_channel()?;
 
@@ -80,7 +155,7 @@ fn build_cargo_project(
         Ok(())
     };
 
-    if unstable_options.original_manifest {
+    if unstable_flags.original_manifest {
         println!(
             "{} {}",
             "warning:".yellow().bold(),
@@ -218,39 +293,34 @@ fn optimize_wasm(crate_metadata: &CrateMetadata) -> Result<OptimizationResult> {
 /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying.
 ///
 /// It does so by invoking `cargo build` and then post processing the final binary.
-///
-/// # Note
-///
-/// Collects the contract crate's metadata using the supplied manifest (`Cargo.toml`) path. Use
-/// [`execute_build_with_metadata`] if an instance is already available.
-pub(crate) fn execute(
+fn execute(
     manifest_path: &ManifestPath,
     verbosity: Option<Verbosity>,
     optimize_contract: bool,
-    build_artifact: GenerateArtifacts,
-    unstable_options: UnstableFlags,
-) -> Result<GenerationResult> {
+    build_artifact: BuildArtifacts,
+    unstable_flags: UnstableFlags,
+) -> Result<BuildResult> {
     let crate_metadata = CrateMetadata::collect(manifest_path)?;
-    if build_artifact == GenerateArtifacts::CodeOnly {
+    if build_artifact == BuildArtifacts::CodeOnly || build_artifact == BuildArtifacts::CheckOnly {
         let (maybe_dest_wasm, maybe_optimization_result) = execute_with_crate_metadata(
             &crate_metadata,
             verbosity,
             optimize_contract,
             build_artifact,
-            unstable_options,
+            unstable_flags,
         )?;
-        let res = GenerationResult {
+        let res = BuildResult {
             dest_wasm: maybe_dest_wasm,
             dest_metadata: None,
             dest_bundle: None,
             target_directory: crate_metadata.cargo_meta.target_directory,
             optimization_result: maybe_optimization_result,
+            build_artifact,
         };
         return Ok(res);
     }
 
-    let res =
-        super::metadata::execute(&manifest_path, verbosity, build_artifact, unstable_options)?;
+    let res = super::metadata::execute(&manifest_path, verbosity, build_artifact, unstable_flags)?;
     Ok(res)
 }
 
@@ -267,15 +337,15 @@ pub(crate) fn execute_with_crate_metadata(
     crate_metadata: &CrateMetadata,
     verbosity: Option<Verbosity>,
     optimize_contract: bool,
-    build_artifact: GenerateArtifacts,
-    unstable_options: UnstableFlags,
+    build_artifact: BuildArtifacts,
+    unstable_flags: UnstableFlags,
 ) -> Result<(Option<PathBuf>, Option<OptimizationResult>)> {
     println!(
         " {} {}",
         format!("[1/{}]", build_artifact.steps()).bold(),
         "Building cargo project".bright_green().bold()
     );
-    build_cargo_project(&crate_metadata, verbosity, unstable_options)?;
+    build_cargo_project(&crate_metadata, verbosity, unstable_flags)?;
     println!(
         " {} {}",
         format!("[2/{}]", build_artifact.steps()).bold(),
@@ -300,7 +370,7 @@ pub(crate) fn execute_with_crate_metadata(
 #[cfg(feature = "test-ci-only")]
 #[cfg(test)]
 mod tests {
-    use crate::{cmd, util::tests::with_tmp_dir, GenerateArtifacts, ManifestPath, UnstableFlags};
+    use crate::{cmd, util::tests::with_tmp_dir, BuildArtifacts, ManifestPath, UnstableFlags};
 
     #[test]
     fn build_template() {
@@ -312,7 +382,7 @@ mod tests {
                 &manifest_path,
                 None,
                 true,
-                GenerateArtifacts::All,
+                BuildArtifacts::All,
                 UnstableFlags::default(),
             )
             .expect("build failed");

src/cmd/metadata.rs

@@ -18,7 +18,7 @@ use crate::{
     crate_metadata::CrateMetadata,
     util,
     workspace::{ManifestPath, Workspace},
-    GenerateArtifacts, GenerationResult, OptimizationResult, UnstableFlags, Verbosity,
+    BuildArtifacts, BuildResult, OptimizationResult, UnstableFlags, Verbosity,
 };
 
 use anyhow::Result;
@@ -38,7 +38,7 @@ const METADATA_FILE: &str = "metadata.json";
 struct GenerateMetadataCommand {
     crate_metadata: CrateMetadata,
     verbosity: Option<Verbosity>,
-    build_artifact: GenerateArtifacts,
+    build_artifact: BuildArtifacts,
     unstable_options: UnstableFlags,
 }
 
@@ -52,7 +52,7 @@ struct ExtendedMetadataResult {
 }
 
 impl GenerateMetadataCommand {
-    pub fn exec(&self) -> Result<GenerationResult> {
+    pub fn exec(&self) -> Result<BuildResult> {
         util::assert_channel()?;
 
         let cargo_meta = &self.crate_metadata.cargo_meta;
@@ -104,7 +104,7 @@ impl GenerateMetadataCommand {
                 current_progress += 1;
             }
 
-            if self.build_artifact == GenerateArtifacts::All {
+            if self.build_artifact == BuildArtifacts::All {
                 println!(
                     " {} {}",
                     format!("[{}/{}]", current_progress, self.build_artifact.steps()).bold(),
@@ -131,17 +131,18 @@ impl GenerateMetadataCommand {
                 .using_temp(generate_metadata)?;
         }
 
-        let dest_bundle = if self.build_artifact == GenerateArtifacts::All {
+        let dest_bundle = if self.build_artifact == BuildArtifacts::All {
             Some(out_path_bundle)
         } else {
             None
         };
-        Ok(GenerationResult {
+        Ok(BuildResult {
             dest_metadata: Some(out_path_metadata),
             dest_wasm,
             dest_bundle,
             optimization_result,
             target_directory,
+            build_artifact: self.build_artifact,
         })
     }
 
@@ -167,7 +168,7 @@ impl GenerateMetadataCommand {
         let source = {
             let lang = SourceLanguage::new(Language::Ink, ink_version.clone());
             let compiler = SourceCompiler::new(Compiler::RustC, rust_version);
-            let maybe_wasm = if self.build_artifact == GenerateArtifacts::All {
+            let maybe_wasm = if self.build_artifact == BuildArtifacts::All {
                 let wasm = fs::read(&self.crate_metadata.dest_wasm)?;
                 // The Wasm which we read must have the same hash as `source.hash`
                 debug_assert!({
@@ -258,9 +259,9 @@ fn blake2_hash(code: &[u8]) -> CodeHash {
 pub(crate) fn execute(
     manifest_path: &ManifestPath,
     verbosity: Option<Verbosity>,
-    build_artifact: GenerateArtifacts,
+    build_artifact: BuildArtifacts,
     unstable_options: UnstableFlags,
-) -> Result<GenerationResult> {
+) -> Result<BuildResult> {
     let crate_metadata = CrateMetadata::collect(manifest_path)?;
     let res = GenerateMetadataCommand {
         crate_metadata,
@@ -277,7 +278,7 @@ pub(crate) fn execute(
 mod tests {
     use crate::cmd::metadata::blake2_hash;
     use crate::{
-        cmd, crate_metadata::CrateMetadata, util::tests::with_tmp_dir, GenerateArtifacts,
+        cmd, crate_metadata::CrateMetadata, util::tests::with_tmp_dir, BuildArtifacts,
         ManifestPath, UnstableFlags,
     };
     use contract_metadata::*;
@@ -375,7 +376,7 @@ mod tests {
             let dest_bundle = cmd::metadata::execute(
                 &test_manifest.manifest_path,
                 None,
-                GenerateArtifacts::All,
+                BuildArtifacts::All,
                 UnstableFlags::default(),
             )?
             .dest_bundle

src/cmd/mod.rs

@@ -22,5 +22,6 @@ mod instantiate;
 pub mod metadata;
 pub mod new;
 
+pub(crate) use self::build::{BuildCommand, CheckCommand};
 #[cfg(feature = "extrinsics")]
 pub(crate) use self::{deploy::execute_deploy, instantiate::execute_instantiate};