Warn on `unwrap`, remove last remaining instances

Signed-off-by: Alexis Sellier <alexis@radicle.xyz>

radicle/src/git.rs

@@ -45,8 +45,7 @@ pub enum ListRefsError {
 
 impl From<&RemoteId> for RefString {
     fn from(id: &RemoteId) -> Self {
-        // PublicKey strings contain only legal characters.
+        RefString::try_from(id.to_string()).expect("encoded public keys are valid ref strings")
-        RefString::try_from(id.to_string()).unwrap()
     }
 }
 
@@ -137,7 +136,7 @@ pub fn initial_commit<'a>(
     let tree_id = repo.index()?.write_tree()?;
     let tree = repo.find_tree(tree_id)?;
     let oid = repo.commit(None, sig, sig, "Initial commit", &tree, &[])?;
-    let commit = repo.find_commit(oid).unwrap();
+    let commit = repo.find_commit(oid)?;
 
     Ok(commit)
 }
@@ -154,20 +153,23 @@ pub fn commit<'a>(
     let tree_id = repo.index()?.write_tree()?;
     let tree = repo.find_tree(tree_id)?;
     let oid = repo.commit(Some(target.as_str()), &sig, &sig, message, &tree, &[parent])?;
-    let commit = repo.find_commit(oid).unwrap();
+    let commit = repo.find_commit(oid)?;
 
     Ok(commit)
 }
 
 /// Push the refs to the radicle remote.
 pub fn push(repo: &git2::Repository) -> Result<(), git2::Error> {
+    // NOTE: This function is going away soon.
+    #![allow(clippy::unwrap_used)]
+
     let mut remote = repo.find_remote("rad")?;
-    let refspecs = remote.push_refspecs().unwrap();
+    let refspecs = remote.push_refspecs()?;
     let refspec = refspecs.into_iter().next().unwrap().unwrap();
 
     // The `git2` crate doesn't seem to support push refspecs with '*' in them,
     // so we manually replace it with the current branch.
-    let head = repo.head().unwrap();
+    let head = repo.head()?;
     let branch = head.shorthand().unwrap();
     let refspec = refspec.replace('*', branch);
 

radicle/src/identity/project.rs

@@ -270,8 +270,8 @@ impl Doc<Unverified> {
         }
     }
 
-    pub fn from_json(bytes: &[u8]) -> Result<Self, serde_json::Error> {
+    pub fn from_json(bytes: &[u8]) -> Result<Self, DocError> {
-        serde_json::from_slice(bytes)
+        serde_json::from_slice(bytes).map_err(DocError::from)
     }
 
     pub fn verified(self) -> Result<Doc<Verified>, VerificationError> {
@@ -375,10 +375,16 @@ pub enum IdentityError {
     MismatchedRoot(Oid),
     #[error("commit signature for {0} is invalid: {1}")]
     InvalidSignature(PublicKey, crypto::Error),
+    #[error("commit message for {0} is invalid")]
+    InvalidCommitMessage(Oid),
+    #[error("commit trailers for {0} are invalid: {1}")]
+    InvalidCommitTrailers(Oid, trailers::Error),
     #[error("quorum not reached: {0} signatures for a threshold of {1}")]
     QuorumNotReached(usize, usize),
     #[error("identity document error: {0}")]
     Doc(#[from] DocError),
+    #[error("the document root is missing")]
+    MissingRoot,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -426,10 +432,10 @@ impl Identity<Untrusted> {
         let mut history = repo.revwalk(head)?.collect::<Vec<_>>();
 
         // Retrieve root document.
-        let root_oid = history.pop().unwrap()?.into();
+        let root_oid = history.pop().ok_or(IdentityError::MissingRoot)??.into();
         let root_blob = Doc::blob_at(root_oid, repo)?;
         let root: git::Oid = root_blob.id().into();
-        let trusted = Doc::from_json(root_blob.content()).unwrap();
+        let trusted = Doc::from_json(root_blob.content())?;
         let revision = history.len() as u32;
 
         let mut trusted = trusted.verified()?;
@@ -442,11 +448,14 @@ impl Identity<Untrusted> {
             let blob = Doc::blob_at(oid.into(), repo)?;
             let untrusted = Doc::from_json(blob.content()).map_err(DocError::from)?;
             let untrusted = untrusted.verified()?;
-            let commit = repo.commit(oid.into())?.unwrap();
+            let commit = repo.commit(oid.into())?;
-            let msg = commit.message_raw().unwrap();
+            let msg = commit
+                .message_raw()
+                .ok_or_else(|| IdentityError::InvalidCommitMessage(oid.into()))?;
 
             // Keys that signed the *current* document version.
-            signatures = trailers::parse_signatures(msg).unwrap();
+            signatures = trailers::parse_signatures(msg)
+                .map_err(|e| IdentityError::InvalidCommitTrailers(oid.into(), e))?;
             for (pk, sig) in &signatures {
                 if let Err(err) = pk.verify(blob.content(), sig) {
                     return Err(IdentityError::InvalidSignature(*pk, err));

radicle/src/keystore.rs

@@ -5,8 +5,21 @@ use std::os::unix::fs::DirBuilderExt;
 use std::os::unix::prelude::OpenOptionsExt;
 use std::path::{Path, PathBuf};
 
+use thiserror::Error;
+
+use crate::crypto;
 use crate::crypto::{PublicKey, SecretKey};
 
+#[derive(Debug, Error)]
+pub enum Error {
+    #[error(transparent)]
+    Io(#[from] io::Error),
+    #[error("invalid key file format: {0}")]
+    FromUtf8Error(#[from] std::string::FromUtf8Error),
+    #[error("invalid key format: {0}")]
+    Crypto(#[from] crypto::Error),
+}
+
 pub struct UnsafeKeystore {
     path: PathBuf,
 }
@@ -18,7 +31,7 @@ impl UnsafeKeystore {
         }
     }
 
-    pub fn put(&mut self, public: &PublicKey, secret: &SecretKey) -> Result<(), io::Error> {
+    pub fn put(&mut self, public: &PublicKey, secret: &SecretKey) -> Result<(), Error> {
         // TODO: Zeroize secret key.
         let public = public.to_pem();
         let secret = secret.to_pem();
@@ -45,14 +58,14 @@ impl UnsafeKeystore {
         Ok(())
     }
 
-    pub fn get(&self) -> Result<(PublicKey, SecretKey), io::Error> {
+    pub fn get(&self) -> Result<(PublicKey, SecretKey), Error> {
         let public = fs::read(self.path.join("radicle.pub"))?;
-        let public = String::from_utf8(public).unwrap();
+        let public = String::from_utf8(public)?;
-        let public = PublicKey::from_pem(&public).unwrap();
+        let public = PublicKey::from_pem(&public)?;
 
         let secret = fs::read(self.path.join("radicle"))?;
-        let secret = String::from_utf8(secret).unwrap();
+        let secret = String::from_utf8(secret)?;
-        let secret = SecretKey::from_pem(&secret).unwrap();
+        let secret = SecretKey::from_pem(&secret)?;
 
         Ok((public, secret))
     }

radicle/src/lib.rs

@@ -1,4 +1,5 @@
 #![allow(clippy::match_like_matches_macro)]
+#![cfg_attr(not(test), warn(clippy::unwrap_used))]
 pub mod collections;
 pub mod crypto;
 pub mod git;

radicle/src/profile.rs

@@ -14,7 +14,7 @@ use std::path::PathBuf;
 use std::{env, io};
 
 use crate::crypto::{KeyPair, PublicKey, SecretKey, Signature, Signer};
-use crate::keystore::UnsafeKeystore;
+use crate::keystore::{Error, UnsafeKeystore};
 use crate::node;
 use crate::storage::git::Storage;
 
@@ -42,7 +42,7 @@ pub struct Profile {
 }
 
 impl Profile {
-    pub fn init(keypair: KeyPair) -> Result<Self, io::Error> {
+    pub fn init(keypair: KeyPair) -> Result<Self, Error> {
         let home = self::home()?;
         let mut keystore = UnsafeKeystore::new(&home.join("keys"));
         let public = keypair.pk.into();
@@ -61,7 +61,7 @@ impl Profile {
         })
     }
 
-    pub fn load() -> Result<Self, io::Error> {
+    pub fn load() -> Result<Self, Error> {
         let home = self::home()?;
         let (public, secret) = UnsafeKeystore::new(&home.join("keys")).get()?;
         let signer = UnsafeSigner { public, secret };

radicle/src/ssh.rs

@@ -144,7 +144,7 @@ impl radicle_ssh::key::Private for SecretKey {
                 let public = r.read_string()?;
                 let pair = r.read_string()?;
                 let _comment = r.read_string()?;
-                let key = crypto::SecretKey::from_slice(pair).unwrap();
+                let key = crypto::SecretKey::from_slice(pair)?;
 
                 if public != key.public_key().as_ref() {
                     return Err(SecretKeyError::Mismatch);

radicle/src/storage.rs

@@ -247,7 +247,7 @@ pub trait ReadRepository {
         remote: &RemoteId,
         reference: &RefStr,
     ) -> Result<git2::Reference, git_ext::Error>;
-    fn commit(&self, oid: Oid) -> Result<Option<git2::Commit>, git2::Error>;
+    fn commit(&self, oid: Oid) -> Result<git2::Commit, git_ext::Error>;
     fn revwalk(&self, head: Oid) -> Result<git2::Revwalk, git2::Error>;
     fn reference_oid(&self, remote: &RemoteId, reference: &RefStr) -> Result<Oid, git_ext::Error>;
     fn references(&self, remote: &RemoteId) -> Result<Refs, Error>;

radicle/src/storage/git.rs

@@ -28,7 +28,6 @@ pub static REMOTES_GLOB: Lazy<refspec::PatternString> =
 pub static SIGNATURES_GLOB: Lazy<refspec::PatternString> =
     Lazy::new(|| refspec::pattern!("refs/remotes/*/radicle/signature"));
 
-// FIXME: Should this be here?
 #[derive(Error, Debug)]
 pub enum ProjectError {
     #[error("identity branches diverge from each other")]
@@ -424,14 +423,10 @@ impl ReadRepository for Repository {
         self.backend.find_reference(&name).map_err(git::Error::from)
     }
 
-    fn commit(&self, oid: Oid) -> Result<Option<git2::Commit>, git2::Error> {
+    fn commit(&self, oid: Oid) -> Result<git2::Commit, git::Error> {
-        self.backend.find_commit(oid.into()).map(Some).or_else(|e| {
+        self.backend
-            if git::ext::is_not_found_err(&e) {
+            .find_commit(oid.into())
-                Ok(None)
+            .map_err(git::Error::from)
-            } else {
-                Err(e)
-            }
-        })
     }
 
     fn revwalk(&self, head: Oid) -> Result<git2::Revwalk, git2::Error> {

radicle/src/storage/git/transport.rs

@@ -52,10 +52,12 @@ impl Smart {
     /// Take a stream from the map.
     /// This makes the stream unavailable until it is re-inserted.
     pub fn take(&self, id: &Id) -> Option<Stream> {
+        #[allow(clippy::unwrap_used)]
         self.streams.lock().unwrap().remove(id)
     }
 
     pub fn insert(&self, id: Id, stream: Stream) {
+        #[allow(clippy::unwrap_used)]
         self.streams.lock().unwrap().insert(id, stream);
     }
 }

radicle/src/test.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::unwrap_used)]
 pub mod arbitrary;
 pub mod assert;
 pub mod fixtures;

radicle/src/test/storage.rs

@@ -97,7 +97,7 @@ impl ReadRepository for MockRepository {
         todo!()
     }
 
-    fn commit(&self, _oid: Oid) -> Result<Option<git2::Commit>, git2::Error> {
+    fn commit(&self, _oid: Oid) -> Result<git2::Commit, git_ext::Error> {
         todo!()
     }