Make pop new <contract | parachain> consistent (#100)

* formatting: align new contract msg with new parachain

* align new contract with new parachain

* make tests pass

* remove invalid test

* fixes: fix contract out_dir

* fixes: fix contract engine unit tests

* pr feedback

src/commands/new/contract.rs

@@ -15,20 +15,19 @@ pub struct NewContractCommand {
 }
 
 impl NewContractCommand {
-	pub(crate) fn execute(&self) -> anyhow::Result<()> {
+	pub(crate) fn execute(self) -> anyhow::Result<()> {
 		clear_screen()?;
 		intro(format!(
 			"{}: Generating new contract \"{}\"!",
 			style(" Pop CLI ").black().on_magenta(),
 			&self.name,
 		))?;
 		set_theme(Theme);
-		let contract_name = self.name.clone();
-		let contract_path = self
-			.path
-			.as_ref()
-			.unwrap_or(&current_dir().expect("current dir is inaccessible"))
-			.join(contract_name.clone());
+		let contract_path = if let Some(ref path) = self.path {
+			path.join(&self.name)
+		} else {
+			current_dir()?.join(&self.name)
+		};
 		if contract_path.exists() {
 			if !confirm(format!(
 				"\"{}\" directory already exists. Would you like to remove it?",
@@ -42,17 +41,14 @@ impl NewContractCommand {
 				))?;
 				return Ok(());
 			}
-			fs::remove_dir_all(contract_path)?;
+			fs::remove_dir_all(contract_path.as_path())?;
 		}
+		fs::create_dir_all(contract_path.as_path())?;
 		let mut spinner = cliclack::spinner();
 		spinner.start("Generating contract...");
-
-		create_smart_contract(self.name.clone(), &self.path)?;
-		spinner.stop(format!(
-			"Smart contract created! Located in the following directory {:?}",
-			self.path.clone().unwrap_or(PathBuf::from(format!("/{}", self.name))).display()
-		));
-		outro(format!("cd into \"{}\" and enjoy hacking! 🚀", &self.name))?;
+		create_smart_contract(self.name, contract_path.as_path())?;
+		spinner.stop("Smart contract created!");
+		outro(format!("cd into \"{}\" and enjoy hacking! 🚀", contract_path.display()))?;
 		Ok(())
 	}
 }
@@ -74,16 +70,4 @@ mod tests {
 
 		Ok(())
 	}
-
-	#[test]
-	fn test_new_contract_command_execute_fails_path_no_exist() -> Result<()> {
-		let temp_contract_dir = tempfile::tempdir().expect("Could not create temp dir");
-		let command = NewContractCommand {
-			name: "test_contract".to_string(),
-			path: Some(temp_contract_dir.path().join("new_contract")),
-		};
-		let result_error = command.execute();
-		assert!(result_error.is_err());
-		Ok(())
-	}
 }

src/commands/new/parachain.rs

@@ -4,7 +4,7 @@ use crate::{
 	style::{style, Theme},
 };
 use clap::{Args, Parser};
-use std::{fs, path::Path};
+use std::{fs, path::PathBuf};
 use strum_macros::{Display, EnumString};
 
 use cliclack::{clear_screen, confirm, intro, log, outro, outro_cancel, set_theme};
@@ -21,7 +21,7 @@ pub enum Template {
 
 #[derive(Args)]
 pub struct NewParachainCommand {
-	#[arg(help = "Name of the project. Also works as a directory path for your project")]
+	#[arg(help = "Name of the project")]
 	pub(crate) name: String,
 	#[arg(
 		help = "Template to use; Options are 'cpt', 'fpt'. Leave empty for default parachain template"
@@ -39,6 +39,12 @@ pub struct NewParachainCommand {
 		default_value = "1u64 << 60"
 	)]
 	pub(crate) initial_endowment: Option<String>,
+	#[arg(
+		short = 'p',
+		long,
+		help = "Path for the parachain project, [default: current directory]"
+	)]
+	pub(crate) path: Option<PathBuf>,
 }
 
 impl NewParachainCommand {
@@ -51,7 +57,11 @@ impl NewParachainCommand {
 			&self.template
 		))?;
 		set_theme(Theme);
-		let destination_path = Path::new(&self.name);
+		let destination_path = if let Some(ref path) = self.path {
+			path.join(&self.name)
+		} else {
+			PathBuf::from(&self.name)
+		};
 		if destination_path.exists() {
 			if !confirm(format!(
 				"\"{}\" directory already exists. Would you like to remove it?",
@@ -65,20 +75,20 @@ impl NewParachainCommand {
 				))?;
 				return Ok(());
 			}
-			fs::remove_dir_all(destination_path)?;
+			fs::remove_dir_all(destination_path.as_path())?;
 		}
 		let mut spinner = cliclack::spinner();
 		spinner.start("Generating parachain...");
 		let tag = instantiate_template_dir(
 			&self.template,
-			destination_path,
+			destination_path.as_path(),
 			Config {
 				symbol: self.symbol.clone().expect("default values"),
 				decimals: self.decimals.clone().expect("default values"),
 				initial_endowment: self.initial_endowment.clone().expect("default values"),
 			},
 		)?;
-		if let Err(err) = git_init(destination_path, "initialized parachain") {
+		if let Err(err) = git_init(destination_path.as_path(), "initialized parachain") {
 			if err.class() == git2::ErrorClass::Config && err.code() == git2::ErrorCode::NotFound {
 				outro_cancel("git signature could not be found. Please configure your git config with your name and email")?;
 			}
@@ -87,7 +97,7 @@ impl NewParachainCommand {
 		if let Some(tag) = tag {
 			log::info(format!("Version: {}", tag))?;
 		}
-		outro(format!("cd into \"{}\" and enjoy hacking! 🚀", &self.name))?;
+		outro(format!("cd into \"{}\" and enjoy hacking! 🚀", destination_path.display()))?;
 		Ok(())
 	}
 }
@@ -98,7 +108,7 @@ mod tests {
 	use git2::Repository;
 
 	use super::*;
-	use std::fs;
+	use std::{fs, path::Path};
 
 	#[test]
 	fn test_new_parachain_command_execute() -> anyhow::Result<()> {
@@ -108,6 +118,7 @@ mod tests {
 			symbol: Some("UNIT".to_string()),
 			decimals: Some(12),
 			initial_endowment: Some("1u64 << 60".to_string()),
+			path: None,
 		};
 		let result = command.execute();
 		assert!(result.is_ok());

src/engines/contract_engine.rs

@@ -1,7 +1,7 @@
 use anyhow::Context;
 use cliclack::log;
 use duct::cmd;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use contract_build::{
 	execute, new_contract_project, BuildArtifacts, BuildMode, ExecuteArgs, Features, ManifestPath,
@@ -14,10 +14,15 @@ use sp_weights::Weight;
 use subxt::PolkadotConfig as DefaultConfig;
 use subxt_signer::sr25519::Keypair;
 
-pub fn create_smart_contract(name: String, target: &Option<PathBuf>) -> anyhow::Result<()> {
-	new_contract_project(&name, target.as_ref())
+/// Create a new smart contract at `target`
+pub fn create_smart_contract(name: String, target: &Path) -> anyhow::Result<()> {
+	// In this code, out_dir will automatically join `name` to `target`,
+	// which is created prior to the call to this function
+	// So we must pass `target.parent()`
+	new_contract_project(&name, target.canonicalize()?.parent())
 }
 
+/// Build a smart contract
 pub fn build_smart_contract(path: &Option<PathBuf>) -> anyhow::Result<()> {
 	// If the user specifies a path (which is not the current directory), it will have to manually
 	// add a Cargo.toml file. If not provided, pop-cli will ask the user for a specific path. or ask
@@ -172,34 +177,33 @@ pub async fn dry_run_call(
 mod tests {
 	use super::*;
 	use anyhow::{Error, Result};
-	use std::{fs, path::PathBuf};
+	use std::fs;
 
 	fn setup_test_environment() -> Result<tempfile::TempDir, Error> {
-		let temp_contract_dir = tempfile::tempdir().expect("Could not create temp dir");
-		let result: anyhow::Result<()> = create_smart_contract(
-			"test_contract".to_string(),
-			&Some(PathBuf::from(temp_contract_dir.path())),
-		);
-
-		assert!(result.is_ok(), "Result should be Ok");
-
-		Ok(temp_contract_dir)
+		let temp_dir = tempfile::tempdir().expect("Could not create temp dir");
+		let temp_contract_dir = temp_dir.path().join("test_contract");
+		fs::create_dir(&temp_contract_dir)?;
+		let result =
+			create_smart_contract("test_contract".to_string(), temp_contract_dir.as_path());
+		assert!(result.is_ok(), "Contract test environment setup failed");
+
+		Ok(temp_dir)
 	}
 
 	#[test]
 	fn test_contract_create() -> Result<(), Error> {
-		let temp_contract_dir = setup_test_environment()?;
+		let temp_dir = setup_test_environment()?;
 
 		// Verify that the generated smart contract contains the expected content
 		let generated_file_content =
-			fs::read_to_string(temp_contract_dir.path().join("test_contract/lib.rs"))
+			fs::read_to_string(temp_dir.path().join("test_contract/lib.rs"))
 				.expect("Could not read file");
 
 		assert!(generated_file_content.contains("#[ink::contract]"));
 		assert!(generated_file_content.contains("mod test_contract {"));
 
 		// Verify that the generated Cargo.toml file contains the expected content
-		fs::read_to_string(temp_contract_dir.path().join("test_contract/Cargo.toml"))
+		fs::read_to_string(temp_dir.path().join("test_contract/Cargo.toml"))
 			.expect("Could not read file");
 		Ok(())
 	}

src/main.rs

@@ -51,8 +51,8 @@ enum Commands {
 #[tokio::main]
 async fn main() -> Result<()> {
 	let cli = Cli::parse();
-	match &cli.command {
-		Commands::New(args) => match &args.command {
+	match cli.command {
+		Commands::New(args) => match args.command {
 			#[cfg(feature = "parachain")]
 			commands::new::NewCommands::Parachain(cmd) => cmd.execute(),
 			#[cfg(feature = "parachain")]