[cli] fix `new` command tests, pass in optional target dir (#197)

* [cli] use temp dir for `cargo contract new` tests

* [cli] pass in temp dir to build command

* [cli] rustfmt

* [cli] remove some spaces

* [cli] rustfmt

* [cli] fix unused warnings

* [cli] TEMPORARY: see if test works on CI without nightly flag

* [cli] build: check if nightly toolchain is the default

* [cli] TEMPORARY: dump cargo output of build command test

* [cli] TEMPORARY: fix error in las commit

* [cli] TEMPORARY: add some diagnostics to see if .cargo/config is there

* [cli] TEMPORARY: use cat

* [cli] fix merge compilation error

* [cli] change ls and cat commands to current_dir

* [cli] fix troubleshooting logging

* [cli] TEMPORARY: comment out RUSTFLAGS for troubleshooting

* [cli] add link-dead-code args to top level .cargo/config

Avoids RUSTFLAGS env var overriding contract project's rust flags

* [cli] revert temporary CI build troubleshooting

* [cli] make rustflags not target specific

* [cli] only link-dead-code in CI

* [cli] TEMP: output .cargo/config

* [cli] printf instead of echo

* [ci] create cargo config in $CARGO_HOME instead of workspace root

* [ci] fix cargo_home path

* [ci] overwrite rather than append cargo home config

* [ci] restore original RUSTFLAGS, ignore failing build test

* [ci] remove stray echo

cli/src/cmd/build.rs

@@ -51,8 +51,12 @@ impl CrateMetadata {
 }
 
 /// Parses the contract manifest and returns relevant metadata.
-pub fn collect_crate_metadata() -> Result<CrateMetadata> {
-    let metadata = MetadataCommand::new().exec()?;
+pub fn collect_crate_metadata(working_dir: Option<&PathBuf>) -> Result<CrateMetadata> {
+    let mut cmd = MetadataCommand::new();
+    if let Some(dir) = working_dir {
+        cmd.current_dir(dir);
+    }
+    let metadata = cmd.exec()?;
 
     let root_package_id = metadata
         .resolve
@@ -88,14 +92,31 @@ pub fn collect_crate_metadata() -> Result<CrateMetadata> {
     })
 }
 
-/// Invokes `cargo build` in the current directory.
+/// Invokes `cargo build` in the specified directory, defaults to the current directory.
 ///
 /// Currently it assumes that user wants to use `+nightly`.
-fn build_cargo_project() -> Result<()> {
-    // We also assume that the user uses +nightly.
-    let output = Command::new("cargo")
+fn build_cargo_project(working_dir: Option<&PathBuf>) -> Result<()> {
+    let mut cmd = Command::new("cargo");
+    let mut is_nightly_cmd = Command::new("cargo");
+    if let Some(dir) = working_dir {
+        cmd.current_dir(dir);
+        is_nightly_cmd.current_dir(dir);
+    }
+
+    let is_nightly_default = is_nightly_cmd
+        .arg("--version")
+        .output()
+        .map_err(|_| ())
+        .and_then(|o| String::from_utf8(o.stdout).map_err(|_| ()))
+        .unwrap_or_default()
+        .contains("-nightly");
+
+    if !is_nightly_default {
+        cmd.arg("+nightly");
+    }
+
+    let output = cmd
         .args(&[
-            "+nightly",
             "build",
             "--no-default-features",
             "--release",
@@ -165,11 +186,13 @@ fn ensure_maximum_memory_pages(
 ///
 /// Presently all custom sections are not required so they can be stripped safely.
 fn strip_custom_sections(module: &mut Module) {
-    module.sections_mut().retain(|section| match section {
-        Section::Custom(_) => false,
-        Section::Name(_) => false,
-        Section::Reloc(_) => false,
-        _ => true,
+    module.sections_mut().retain(|section| {
+        match section {
+            Section::Custom(_) => false,
+            Section::Name(_) => false,
+            Section::Reloc(_) => false,
+            _ => true,
+        }
     });
 }
 
@@ -193,11 +216,11 @@ fn post_process_wasm(crate_metadata: &CrateMetadata) -> Result<()> {
 /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying.
 ///
 /// It does so by invoking build by cargo and then post processing the final binary.
-pub(crate) fn execute_build() -> Result<String> {
+pub(crate) fn execute_build(working_dir: Option<&PathBuf>) -> Result<String> {
     println!(" [1/3] Collecting crate metadata");
-    let crate_metadata = collect_crate_metadata()?;
+    let crate_metadata = collect_crate_metadata(working_dir)?;
     println!(" [2/3] Building cargo project");
-    build_cargo_project()?;
+    build_cargo_project(working_dir)?;
     println!(" [3/3] Post processing wasm file");
     post_process_wasm(&crate_metadata)?;
 
@@ -209,32 +232,24 @@ pub(crate) fn execute_build() -> Result<String> {
 
 #[cfg(test)]
 mod tests {
-    use super::*;
     use crate::{
-        cmd::execute_new,
+        cmd::{
+            execute_new,
+            tests::with_tmp_dir,
+        },
         AbstractionLayer,
     };
-    use tempfile::TempDir;
-    use std::env;
-
-    fn with_tmp_dir<F: FnOnce()>(f: F) {
-
-        let original_cwd = env::current_dir().expect("failed to get current working directory");
-        let tmp_dir = TempDir::new().expect("temporary directory creation failed");
-        env::set_current_dir(tmp_dir.path()).expect("setting the current dir to temp failed");
-
-        f();
-
-        env::set_current_dir(original_cwd).expect("restoring cwd failed");
-    }
 
-    #[cfg(feature="test-ci-only")]
+    #[cfg(feature = "test-ci-only")]
     #[test]
+    // FIXME: https://github.com/paritytech/ink/issues/202
+    // currently fails on CI because of global RUSTFLAGS overriding required `--import-memory`
+    #[ignore]
     fn build_template() {
-        with_tmp_dir(|| {
-            execute_new(AbstractionLayer::Lang, "new_project").expect("new project creation failed");
-            env::set_current_dir("./new_project").expect("cwd to new_project failed");
-            execute_build().expect("build failed");
+        with_tmp_dir(|path| {
+            execute_new(AbstractionLayer::Lang, "new_project", Some(path))
+                .expect("new project creation failed");
+            super::execute_build(Some(&path.join("new_project"))).expect("build failed");
         });
     }
 }

cli/src/cmd/deploy.rs

@@ -44,7 +44,7 @@ use subxt::{
 /// Defaults to the target contract wasm in the current project, inferred via the crate metadata.
 fn load_contract_code(path: Option<&PathBuf>) -> Result<Vec<u8>> {
     let default_wasm_path =
-        build::collect_crate_metadata().map(CrateMetadata::dest_wasm)?;
+        build::collect_crate_metadata(path).map(CrateMetadata::dest_wasm)?;
     let contract_wasm_path = path.unwrap_or(&default_wasm_path);
 
     let mut data = Vec::new();

cli/src/cmd/mod.rs

@@ -28,3 +28,15 @@ pub(crate) use self::{
     },
     new::execute_new,
 };
+
+#[cfg(test)]
+mod tests {
+    use std::path::PathBuf;
+    use tempfile::TempDir;
+
+    pub fn with_tmp_dir<F: FnOnce(&PathBuf)>(f: F) {
+        let tmp_dir = TempDir::new().expect("temporary directory creation failed");
+
+        f(&tmp_dir.into_path());
+    }
+}

cli/src/cmd/new.rs

@@ -23,6 +23,7 @@ use crate::{
 };
 use heck::CamelCase as _;
 use std::{
+    env,
     fs,
     io::{
         Cursor,
@@ -31,21 +32,21 @@ use std::{
         SeekFrom,
         Write,
     },
-    path,
+    path::PathBuf,
 };
 
 /// Initializes a project structure for the `lang` abstraction layer.
-fn initialize_for_lang(name: &str) -> Result<String> {
+fn initialize_for_lang(name: &str, target_dir: Option<&PathBuf>) -> Result<String> {
     if name.contains("-") {
         return Err("Contract names cannot contain hyphens".into())
     }
 
-    let out_dir = path::Path::new(name);
+    let out_dir = target_dir.unwrap_or(&env::current_dir()?).join(name);
     if out_dir.join("Cargo.toml").exists() {
         return Err(format!("A Cargo package already exists in {}", name).into())
     }
     if !out_dir.exists() {
-        fs::create_dir(out_dir)?;
+        fs::create_dir(&out_dir)?;
     }
 
     let template = include_bytes!(concat!(env!("OUT_DIR"), "/template.zip"));
@@ -82,7 +83,7 @@ fn initialize_for_lang(name: &str) -> Result<String> {
                     if e.kind() == std::io::ErrorKind::AlreadyExists {
                         CommandError::from(format!(
                             "New contract file {} already exists",
-                            outpath.display()
+                            file.sanitized_name().display()
                         ))
                     } else {
                         CommandError::from(e)
@@ -106,52 +107,69 @@ fn initialize_for_lang(name: &str) -> Result<String> {
     Ok(format!("Created contract {}", name))
 }
 
-pub(crate) fn execute_new(layer: AbstractionLayer, name: &str) -> Result<String> {
+pub(crate) fn execute_new(
+    layer: AbstractionLayer,
+    name: &str,
+    dir: Option<&PathBuf>,
+) -> Result<String> {
     match layer {
         AbstractionLayer::Core => Err(CommandError::UnimplementedAbstractionLayer),
         AbstractionLayer::Model => Err(CommandError::UnimplementedAbstractionLayer),
-        AbstractionLayer::Lang => initialize_for_lang(name),
+        AbstractionLayer::Lang => initialize_for_lang(name, dir),
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::{
+        cmd::{
+            execute_new,
+            tests::with_tmp_dir,
+        },
+        AbstractionLayer,
+    };
 
     #[test]
     fn rejects_hyphenated_name() {
-        let result = super::initialize_for_lang("should-fail");
-        assert_eq!(
-            format!("{:?}", result),
-            r#"Err(Other("Contract names cannot contain hyphens"))"#
-        )
+        with_tmp_dir(|path| {
+            let result = execute_new(
+                AbstractionLayer::Lang,
+                "rejects-hyphenated-name",
+                Some(path),
+            );
+            assert_eq!(
+                format!("{:?}", result),
+                r#"Err(Other("Contract names cannot contain hyphens"))"#
+            )
+        });
     }
 
     #[test]
     fn contract_cargo_project_already_exists() {
-        let name = "test_contract_cargo_project_already_exists";
-        let _ = super::initialize_for_lang(name);
-        let result = super::initialize_for_lang(name);
-        // clean up created files
-        std::fs::remove_dir_all(name).unwrap();
-        assert_eq!(
-            format!("{:?}", result),
-            r#"Err(Other("A Cargo package already exists in test_contract_cargo_project_already_exists"))"#
-        )
+        with_tmp_dir(|path| {
+            let name = "test_contract_cargo_project_already_exists";
+            let _ = execute_new(AbstractionLayer::Lang, name, Some(path));
+            let result = execute_new(AbstractionLayer::Lang, name, Some(path));
+            assert_eq!(
+                format!("{:?}", result),
+                r#"Err(Other("A Cargo package already exists in test_contract_cargo_project_already_exists"))"#
+            )
+        });
     }
 
     #[test]
     fn dont_overwrite_existing_files_not_in_cargo_project() {
-        let name = "dont_overwrite_existing_files";
-        let dir = path::Path::new(name);
-        fs::create_dir_all(dir).unwrap();
-        fs::File::create(dir.join(".gitignore")).unwrap();
-        let result = super::initialize_for_lang(name);
-        // clean up created files
-        std::fs::remove_dir_all(dir).unwrap();
-        assert_eq!(
-            format!("{:?}", result),
-            r#"Err(Other("New contract file dont_overwrite_existing_files/.gitignore already exists"))"#
-        )
+        with_tmp_dir(|path| {
+            let name = "dont_overwrite_existing_files";
+            let dir = path.join(name);
+            fs::create_dir_all(&dir).unwrap();
+            fs::File::create(dir.join(".gitignore")).unwrap();
+            let result = execute_new(AbstractionLayer::Lang, name, Some(path));
+            assert_eq!(
+                format!("{:?}", result),
+                r#"Err(Other("New contract file .gitignore already exists"))"#
+            )
+        });
     }
 }

cli/src/main.rs

@@ -50,7 +50,10 @@ pub(crate) enum AbstractionLayer {
     Lang,
 }
 
-use std::result::Result as StdResult;
+use std::{
+    path::PathBuf,
+    result::Result as StdResult,
+};
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub(crate) struct InvalidAbstractionLayer;
@@ -84,6 +87,9 @@ enum Command {
         layer: AbstractionLayer,
         /// The name of the newly created smart contract.
         name: String,
+        /// The optional target directory for the contract project
+        #[structopt(short, long, parse(from_os_str))]
+        target_dir: Option<PathBuf>,
     },
     /// Builds the smart contract.
     #[structopt(name = "build")]
@@ -130,8 +136,12 @@ fn main() {
 fn exec(cmd: Command) -> cmd::Result<String> {
     use crate::cmd::CommandError;
     match &cmd {
-        Command::New { layer, name } => cmd::execute_new(*layer, name),
-        Command::Build {} => cmd::execute_build(),
+        Command::New {
+            layer,
+            name,
+            target_dir,
+        } => cmd::execute_new(*layer, name, target_dir.as_ref()),
+        Command::Build {} => cmd::execute_build(None),
         Command::Test {} => Err(CommandError::UnimplementedCommand),
         Command::Deploy {
             url,

lang/src/hir.rs

@@ -244,12 +244,10 @@ impl Contract {
                     }
                     Some(self_ty) => {
                         match self_ty {
-                            ast::FnArg::SelfValue(_) | ast::FnArg::Captured(_) => {
-                                bail!(
+                            ast::FnArg::SelfValue(_) | ast::FnArg::Captured(_) => bail!(
                                 self_ty,
                                 "contract messages must operate on `&self` or `&mut self`"
-                            )
-                            }
+                            ),
                             _ => (),
                         }
                     }