Address comments; document dfxerror

dfx/src/commands/build.rs

@@ -153,12 +153,8 @@ where
         .get_config()
         .ok_or(DfxError::CommandMustBeRunInAProject)?;
 
-    // get_path() returns the full path of the config file. We need to
-    // get the directory name.
-    let project_root = config
-        .get_path()
-        .parent()
-        .ok_or_else(|| DfxError::InvalidArgument("project root path".to_string()))?;
+    // We need to get the directory name.
+    let project_root = config.get_project_root();
 
     let build_root = project_root.join(
         config
@@ -176,10 +172,7 @@ where
                 let input_as_path = project_root.join(path);
                 let path = path.strip_prefix("src/").unwrap_or(&path);
                 let output_path = build_root.join(path).with_extension("wasm");
-                let parent_output_path = output_path.parent().ok_or_else(|| {
-                    DfxError::Unknown("unable to retrieve root path parent".to_string())
-                })?;
-                std::fs::create_dir_all(parent_output_path)?;
+                std::fs::create_dir_all(output_path.parent().unwrap())?;
 
                 build_file(
                     env,

dfx/src/commands/canister/call.rs

@@ -52,9 +52,9 @@ where
     // Read the config.
     let canister_id = args
         .value_of("deployment_id")
-        .ok_or_else(|| DfxError::InvalidArgument("deployment id".to_string()))?
+        .ok_or_else(|| DfxError::InvalidArgument("deployment_id".to_string()))?
         .parse::<CanisterId>()
-        .map_err(|e| DfxError::InvalidArgument(format!("Invalid deployment ID: {}", e)))?;
+        .map_err(|e| DfxError::InvalidArgument(format!("invalid deployment_id: {}", e)))?;
     let method_name = args.value_of("method name").ok_or_else(|| {
         DfxError::InvalidArgument("method name argument provided invalid".to_string())
     })?;

dfx/src/commands/canister/install.rs

@@ -41,18 +41,18 @@ where
         .get_config()
         .ok_or(DfxError::CommandMustBeRunInAProject)?;
 
-    let project_root = config
-        .get_path()
-        .parent()
-        .ok_or_else(|| DfxError::InvalidConfiguration("project root".to_string()))?;
+    let project_root = config.get_project_root();
+
     let canister_id = args
         .value_of("deployment_id")
-        .ok_or_else(|| DfxError::InvalidArgument("canister id".to_string()))?
+        .ok_or_else(|| {
+            DfxError::InvalidArgument(
+                "failed to retrieve deployment_id -- required argument".to_string(),
+            )
+        })?
         .parse::<CanisterId>()
         .map_err(|e| DfxError::InvalidArgument(format!("Invalid deployment ID: {}", e)))?;
-    let wasm_path = args
-        .value_of("wasm")
-        .ok_or_else(|| DfxError::InvalidConfiguration("wasm path".to_string()))?;
+    let wasm_path = args.value_of("wasm").unwrap();
     let wasm_path = PathBuf::from(project_root).join(wasm_path);
 
     let wasm = std::fs::read(wasm_path)?;

dfx/src/commands/canister/request_status.rs

@@ -26,9 +26,11 @@ where
     T: ClientEnv,
 {
     let request_id = RequestId::from_str(
-        &args
-            .value_of("request_id")
-            .ok_or_else(|| DfxError::InvalidArgument("request id".to_string()))?[2..],
+        &args.value_of("request_id").ok_or_else(|| {
+            DfxError::InvalidArgument(
+                "failed to retrieve request_id -- required argument".to_string(),
+            )
+        })?[2..],
     )
     .map_err(|e| DfxError::InvalidArgument(format!("invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError
     let request_status = request_status(env.get_client(), request_id);

dfx/src/commands/new.rs

@@ -116,7 +116,7 @@ where
 
         let mut s = String::new();
         file.read_to_string(&mut s)
-            .or_else(|e| Err(DfxError::IO(e)))?;
+            .or_else(|e| Err(DfxError::Io(e)))?;
 
         // Perform replacements.
         let s = s.replace("{project_name}", project_name_str);
@@ -143,27 +143,22 @@ where
             .arg("init")
             .current_dir(&project_name)
             .status();
-        let is_success = init_status
-            .or_else(|e| {
-                Err(DfxError::IO(std::io::Error::new(
-                    e.kind(),
-                    format!("Unable to execute git {}", e),
-                )))
-            })?
-            .success();
-        if is_success {
-            eprintln!("Creating git repository...");
-            std::process::Command::new("git")
-                .arg("add")
-                .current_dir(&project_name)
-                .arg(".")
-                .output()?;
-            std::process::Command::new("git")
-                .arg("commit")
-                .current_dir(&project_name)
-                .arg("-a")
-                .arg("--message=Initial commit.")
-                .output()?;
+
+        if let Ok(s) = init_status {
+            if s.success() {
+                eprintln!("Creating git repository...");
+                std::process::Command::new("git")
+                    .arg("add")
+                    .current_dir(&project_name)
+                    .arg(".")
+                    .output()?;
+                std::process::Command::new("git")
+                    .arg("commit")
+                    .current_dir(&project_name)
+                    .arg("-a")
+                    .arg("--message=Initial commit.")
+                    .output()?;
+            }
         }
     }
 

dfx/src/config/dfinity.rs

@@ -213,6 +213,15 @@ impl Config {
         &self.config
     }
 
+    pub fn get_project_root(&self) -> &Path {
+        // a configuration path contains a file name specifically. As
+        // such we should be returning at least root as parent. If
+        // this is invariance is broken, we must fail.
+        self.path.parent().expect(
+            "An incorrect configuration path was set with no parent, i.e. did not include root",
+        )
+    }
+
     pub fn save(&self) -> DfxResult {
         let json_pretty = serde_json::to_string_pretty(&self.json).or_else(|e| {
             Err(DfxError::InvalidData(format!(

dfx/src/lib/error.rs

@@ -53,36 +53,44 @@ impl fmt::Display for BuildErrorKind {
 
 // TODO: refactor this enum into a *Kind enum and a struct DfxError.
 #[derive(Debug)]
+/// Provides dfx user facing errors.
 pub enum DfxError {
     /// An error happened during build.
     BuildError(BuildErrorKind),
     Clap(clap::Error),
-    IO(std::io::Error),
+    Io(std::io::Error),
     Reqwest(reqwest::Error),
     Url(reqwest::UrlError),
 
     /// An unknown command was used. The argument is the command itself.
     UnknownCommand(String),
 
-    // Cannot create a new project because the directory already exists.
+    /// Cannot create a new project because the directory already exists.
     ProjectExists,
 
-    // Not in a project.
+    /// Not in a project.
     CommandMustBeRunInAProject,
 
-    // The client returned an error. It normally specifies the error as an
-    // HTTP status (so 400-599), and has a string as the error message.
-    // Once the client support errors from the public spec or as an enum,
-    // we should update this type.
-    // We don't use StatusCode here because the client might return some other
-    // number if they support public spec's errors (< 100).
+    /// The client returned an error. It normally specifies the error as an
+    /// HTTP status (so 400-599), and has a string as the error message.
+    /// Once the client support errors from the public spec or as an enum,
+    /// we should update this type.
+    /// We don't use StatusCode here because the client might return some other
+    /// number if they support public spec's errors (< 100).
     ClientError(u16, String),
+
+    /// This option is used when the source/cause of the error is
+    /// ambiguous. If the cause is known use or add a new option.
     Unknown(String),
 
-    // Configuration path does not exist in the config file.
+    /// Configuration path does not exist in the config file.
     ConfigPathDoesNotExist(String),
+    /// Argument provided is invalid.
     InvalidArgument(String),
+    #[allow(dead_code)]
+    /// Configuration provided is invalid.
     InvalidConfiguration(String),
+    /// Data provided is invalid.
     InvalidData(String),
 }
 
@@ -103,6 +111,6 @@ impl From<reqwest::Error> for DfxError {
 
 impl From<std::io::Error> for DfxError {
     fn from(err: std::io::Error) -> DfxError {
-        DfxError::IO(err)
+        DfxError::Io(err)
     }
 }