fix: Remove unwraps (for commands)

dfx/src/commands/canister/call.rs

@@ -94,11 +94,13 @@ where
 
     let canister_name = args.value_of("canister_name").unwrap();
     let canister_info = CanisterInfo::load(config, canister_name)?;
+    // Read the config.
     let canister_id = canister_info.get_canister_id().ok_or_else(|| {
         DfxError::CannotFindBuildOutputForCanister(canister_info.get_name().to_owned())
     })?;
-
-    let method_name = args.value_of("method_name").unwrap();
+    let method_name = args
+        .value_of("method_name")
+        .ok_or_else(|| DfxError::InvalidArgument("method_name".to_string()))?;
     let arguments: Option<&str> = args.value_of("argument");
     let arg_type: Option<&str> = args.value_of("type");
 

dfx/src/commands/canister/request_status.rs

@@ -23,7 +23,11 @@ pub fn exec<T>(env: &T, args: &ArgMatches<'_>) -> DfxResult
 where
     T: ClientEnv,
 {
-    let request_id = RequestId::from_str(&args.value_of("request_id").unwrap()[2..])
-        .map_err(|e| DfxError::InvalidArgument(format!("Invalid request ID: {:?}", e)))?; // FIXME Default formatter for RequestIdFromStringError
+    let request_id = RequestId::from_str(
+        &args
+            .value_of("request_id")
+            .ok_or_else(|| DfxError::InvalidArgument("request_id".to_string()))?[2..],
+    )
+    .map_err(|_| DfxError::InvalidArgument("request_id".to_owned()))?;
     wait_on_request_status(&env.get_client(), request_id)
 }

dfx/src/commands/config.rs

@@ -22,7 +22,9 @@ pub fn exec<T: ProjectConfigEnv>(env: &T, args: &ArgMatches<'_>) -> DfxResult {
         .ok_or(DfxError::CommandMustBeRunInAProject)?
         .clone();
 
-    let config_path = args.value_of("config_path").unwrap();
+    let config_path = args
+        .value_of("config_path")
+        .ok_or_else(|| DfxError::InvalidArgument("config_path".to_string()))?;
 
     // We replace `.` with `/` so the user can use `path.value.field` instead of forcing him
     // to use `path/value/field`. Since none of our keys have slashes or tildes in them it

dfx/src/commands/new.rs

@@ -113,7 +113,10 @@ where
     T: BinaryCacheEnv + PlatformEnv + VersionEnv,
 {
     let dry_run = args.is_present(DRY_RUN);
-    let project_name = Path::new(args.value_of(PROJECT_NAME).unwrap());
+    let project_name_path = args
+        .value_of(PROJECT_NAME)
+        .ok_or_else(|| DfxError::InvalidArgument("project_path".to_string()))?;
+    let project_name = Path::new(project_name_path);
 
     if project_name.exists() {
         return Err(DfxError::ProjectExists);
@@ -127,6 +130,10 @@ where
     }
 
     let mut new_project_files = assets::new_project_files()?;
+    let project_name_str = project_name
+        .to_str()
+        .ok_or_else(|| DfxError::InvalidArgument("project_name".to_string()))?;
+
     for file in new_project_files.entries()? {
         let mut file = file?;
 
@@ -135,18 +142,19 @@ where
         }
 
         let mut s = String::new();
-        file.read_to_string(&mut s).unwrap();
+        file.read_to_string(&mut s)
+            .or_else(|e| Err(DfxError::Io(e)))?;
 
         // Perform replacements.
-        let s = s.replace("{project_name}", project_name.to_str().unwrap());
+        let s = s.replace("{project_name}", project_name_str);
         let s = s.replace("{dfx_version}", dfx_version);
 
         // Perform path replacements.
         let p = PathBuf::from(
             project_name
                 .join(file.header().path()?)
                 .to_str()
-                .unwrap()
+                .expect("Non unicode project name path.")
                 .replace("__dot__", ".")
                 .as_str(),
         );
@@ -192,7 +200,7 @@ where
         include_str!("../../assets/welcome.txt"),
         dfx_version,
         assets::dfinity_logo(),
-        project_name.to_str().unwrap()
+        project_name_str
     );
 
     Ok(())

dfx/src/config/dfinity.rs

@@ -218,11 +218,23 @@ 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 {
-        std::fs::write(
-            &self.path,
-            serde_json::to_string_pretty(&self.json).unwrap(),
-        )?;
+        let json_pretty = serde_json::to_string_pretty(&self.json).or_else(|e| {
+            Err(DfxError::InvalidData(format!(
+                "Failed to serialize dfx.json: {}",
+                e
+            )))
+        })?;
+        std::fs::write(&self.path, json_pretty)?;
         Ok(())
     }
 }

dfx/src/lib/error/mod.rs

@@ -6,6 +6,7 @@ pub use cache::CacheErrorKind;
 
 // 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),
@@ -30,28 +31,45 @@ pub enum DfxError {
     /// 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.
+    #[allow(dead_code)]
+    /// An error originating from the Client. The enclosed type should be
+    /// a descriptive error.
+    // TODO(eftychis): Consider to how to better represent this without a massive change.
+    ClientContainerError(String),
+
+    /// 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),
+    /// Method called invalid.
     InvalidMethodCall(String),
+
+    /// Data provided is invalid.
     InvalidData(String),
 
-    // The ide server shouldn't be started from a terminal
+    /// The ide server shouldn't be started from a terminal.
     LanguageServerFromATerminal,
 }
 

lib/serde_idl/src/de.rs

@@ -487,11 +487,14 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
         let value = visitor.visit_enum(Compound::new(&mut self, Style::Enum { len, fs }))?;
         Ok(value)
     }
-
+    /// Deserialize identifier.
+    /// # Panics
+    /// *Will Panic* when identifier name is None.
     fn deserialize_identifier<V>(self, visitor: V) -> Result<V::Value>
     where
         V: Visitor<'de>,
     {
+        // N.B. Here we want to panic as it indicates a logical error.
         let label = self.field_name.as_ref().unwrap();
         let v = match label {
             FieldLabel::Named(name) => visitor.visit_str(name),