Cleans up a number of Option / Result patterns and warts (#226)

bin/oe/configuration.rs

@@ -518,10 +518,8 @@ impl Configuration {
     }
 
     fn ip_filter(&self) -> Result<IpFilter, String> {
-        match IpFilter::parse(self.args.arg_allow_ips.as_str()) {
+        IpFilter::parse(self.args.arg_allow_ips.as_str())
-            Ok(allow_ip) => Ok(allow_ip),
+            .map_err(|_| "Invalid IP filter value".to_owned())
-            Err(_) => Err("Invalid IP filter value".to_owned()),
-        }
     }
 
     fn min_peers(&self) -> u32 {

bin/oe/helpers.rs

@@ -105,10 +105,10 @@ pub fn to_block_id(s: &str) -> Result<BlockId, String> {
 pub fn to_u256(s: &str) -> Result<U256, String> {
     if let Ok(decimal) = U256::from_dec_str(s) {
         Ok(decimal)
-    } else if let Ok(hex) = clean_0x(s).parse() {
-        Ok(hex)
     } else {
-        Err(format!("Invalid numeric value: {}", s))
+        clean_0x(s)
+            .parse()
+            .map_err(|_| format!("Invalid numeric value: {}", s))
     }
 }
 
@@ -171,15 +171,12 @@ pub fn to_price(s: &str) -> Result<f32, String> {
 }
 
 pub fn join_set(set: Option<&HashSet<String>>) -> Option<String> {
-    match set {
+    set.map(|s| {
-        Some(s) => Some(
+        s.iter()
-            s.iter()
+            .map(|s| s.as_str())
-                .map(|s| s.as_str())
+            .collect::<Vec<&str>>()
-                .collect::<Vec<&str>>()
+            .join(",")
-                .join(","),
+    })
-        ),
-        None => None,
-    }
 }
 
 /// Flush output buffer.

bin/oe/logger/src/lib.rs

@@ -157,10 +157,11 @@ pub fn setup_log(config: &Config) -> Result<Arc<RotatingLogger>, String> {
             Ok(logs)
         })
         // couldn't create new logger - try to fall back on previous logger.
-        .or_else(|err| match ROTATING_LOGGER.lock().upgrade() {
+        .or_else(|err| {
-            Some(l) => Ok(l),
+            ROTATING_LOGGER
-            // no previous logger. fatal.
+                .lock()
-            None => Err(format!("{:?}", err)),
+                .upgrade()
+                .ok_or_else(|| format!("{:?}", err))
         })
 }
 

crates/accounts/ethkey/src/random.rs

@@ -25,10 +25,7 @@ impl Generator for Random {
 
     fn generate(&mut self) -> Result<KeyPair, Self::Error> {
         let mut rng = OsRng::new()?;
-        match rng.generate() {
+        rng.generate().or_else(|void| match void {})
-            Ok(pair) => Ok(pair),
-            Err(void) => match void {}, // LLVM unreachable
-        }
     }
 }
 

crates/accounts/ethstore/src/json/crypto.rs

@@ -159,10 +159,7 @@ impl<'a> Visitor<'a> for CryptoVisitor {
             (Some(_), None) => return Err(V::Error::missing_field("cipherparams")),
         };
 
-        let ciphertext = match ciphertext {
+        let ciphertext = ciphertext.ok_or_else(|| V::Error::missing_field("ciphertext"))?;
-            Some(ciphertext) => ciphertext,
-            None => return Err(V::Error::missing_field("ciphertext")),
-        };
 
         let kdf = match (kdf, kdfparams) {
             (Some(KdfSer::Pbkdf2), Some(KdfSerParams::Pbkdf2(params))) => Kdf::Pbkdf2(params),
@@ -172,10 +169,7 @@ impl<'a> Visitor<'a> for CryptoVisitor {
             (Some(_), None) => return Err(V::Error::missing_field("kdfparams")),
         };
 
-        let mac = match mac {
+        let mac = mac.ok_or_else(|| V::Error::missing_field("mac"))?;
-            Some(mac) => mac,
-            None => return Err(V::Error::missing_field("mac")),
-        };
 
         let result = Crypto {
             cipher: cipher,

crates/accounts/ethstore/src/json/key_file.rs

@@ -169,20 +169,11 @@ impl<'a> Visitor<'a> for KeyFileVisitor {
             }
         }
 
-        let id = match id {
+        let id = id.ok_or_else(|| V::Error::missing_field("id"))?;
-            Some(id) => id,
-            None => return Err(V::Error::missing_field("id")),
-        };
 
-        let version = match version {
+        let version = version.ok_or_else(|| V::Error::missing_field("version"))?;
-            Some(version) => version,
-            None => return Err(V::Error::missing_field("version")),
-        };
 
-        let crypto = match crypto {
+        let crypto = crypto.ok_or_else(|| V::Error::missing_field("crypto"))?;
-            Some(crypto) => crypto,
-            None => return Err(V::Error::missing_field("crypto")),
-        };
 
         let result = KeyFile {
             id: id,

crates/concensus/ethash/src/compute.rs

@@ -198,7 +198,7 @@ fn hash_compute(light: &Light, full_size: usize, header_hash: &H256, nonce: u64)
     struct MixBuf {
         half_mix: Node,
         compress_bytes: [u8; MIX_WORDS],
-    };
+    }
 
     if full_size % MIX_WORDS != 0 {
         panic!("Unaligned full size");

crates/ethcore/src/client/client.rs

@@ -1240,10 +1240,7 @@ impl Client {
             return Some(state);
         }
 
-        let block_number = match self.block_number(id) {
+        let block_number = self.block_number(id)?;
-            Some(num) => num,
-            None => return None,
-        };
 
         self.block_header(id).and_then(|header| {
             let db = self.state_db.read().boxed_clone();
@@ -1370,15 +1367,12 @@ impl Client {
                     None => best_block_number.saturating_sub(history),
                 };
 
-                match self.block_hash(BlockId::Number(start_num)) {
+                self.block_hash(BlockId::Number(start_num))
-                    Some(h) => h,
+                    .ok_or_else(|| snapshot::Error::InvalidStartingBlock(at))?
-                    None => return Err(snapshot::Error::InvalidStartingBlock(at).into()),
-                }
             }
-            _ => match self.block_hash(at) {
+            _ => self
-                Some(hash) => hash,
+                .block_hash(at)
-                None => return Err(snapshot::Error::InvalidStartingBlock(at).into()),
+                .ok_or_else(|| snapshot::Error::InvalidStartingBlock(at))?,
-            },
         };
 
         let processing_threads = self.config.snapshot.processing_threads;
@@ -2407,18 +2401,13 @@ impl BlockChainClient for Client {
                 .collect::<Vec<H256>>()
         } else {
             // Otherwise, we use a slower version that finds a link between from_block and to_block.
-            let from_hash = match Self::block_hash(&chain, filter.from_block) {
+            let from_hash = Self::block_hash(&chain, filter.from_block)
-                Some(val) => val,
+                .ok_or_else(|| filter.from_block.clone())?;
-                None => return Err(filter.from_block.clone()),
+            let from_number = chain
-            };
+                .block_number(&from_hash)
-            let from_number = match chain.block_number(&from_hash) {
+                .ok_or_else(|| BlockId::Hash(from_hash))?;
-                Some(val) => val,
+            let to_hash =
-                None => return Err(BlockId::Hash(from_hash)),
+                Self::block_hash(&chain, filter.to_block).ok_or_else(|| filter.to_block.clone())?;
-            };
-            let to_hash = match Self::block_hash(&chain, filter.to_block) {
-                Some(val) => val,
-                None => return Err(filter.to_block.clone()),
-            };
 
             let blooms = filter.bloom_possibilities();
             let bloom_match = |header: &encoded::Header| {
@@ -2432,10 +2421,9 @@ impl BlockChainClient for Client {
                 let mut current_hash = to_hash;
 
                 loop {
-                    let header = match chain.block_header_data(&current_hash) {
+                    let header = chain
-                        Some(val) => val,
+                        .block_header_data(&current_hash)
-                        None => return Err(BlockId::Hash(current_hash)),
+                        .ok_or_else(|| BlockId::Hash(current_hash))?;
-                    };
                     if bloom_match(&header) {
                         blocks.push(current_hash);
                     }

crates/ethcore/src/engines/validator_set/safe_contract.rs

@@ -292,10 +292,9 @@ impl ValidatorSafeContract {
             });
 
         // only last log is taken into account
-        match decoded_events.next() {
+        decoded_events
-            None => None,
+            .next()
-            Some(matched_event) => Some(SimpleList::new(matched_event.new_set)),
+            .map(|matched_event| SimpleList::new(matched_event.new_set))
-        }
     }
 }
 

crates/ethcore/src/executive.rs

@@ -623,10 +623,7 @@ impl<'a> CallCreateExecutive<'a> {
                         tracer,
                         vm_tracer,
                     );
-                    match exec.exec(&mut ext) {
+                    exec.exec(&mut ext).map(|val| val.finalize(ext))
-                        Ok(val) => Ok(val.finalize(ext)),
-                        Err(err) => Err(err),
-                    }
                 };
 
                 let res = match out {
@@ -694,10 +691,7 @@ impl<'a> CallCreateExecutive<'a> {
                         tracer,
                         vm_tracer,
                     );
-                    match exec.exec(&mut ext) {
+                    exec.exec(&mut ext).map(|val| val.finalize(ext))
-                        Ok(val) => Ok(val.finalize(ext)),
-                        Err(err) => Err(err),
-                    }
                 };
 
                 let res = match out {
@@ -763,10 +757,7 @@ impl<'a> CallCreateExecutive<'a> {
                         tracer,
                         vm_tracer,
                     );
-                    match exec.exec(&mut ext) {
+                    exec.exec(&mut ext).map(|val| val.finalize(ext))
-                        Ok(val) => Ok(val.finalize(ext)),
-                        Err(err) => Err(err),
-                    }
                 };
 
                 let res = match out {
@@ -840,10 +831,7 @@ impl<'a> CallCreateExecutive<'a> {
                         tracer,
                         vm_tracer,
                     );
-                    match exec.exec(&mut ext) {
+                    exec.exec(&mut ext).map(|val| val.finalize(ext))
-                        Ok(val) => Ok(val.finalize(ext)),
-                        Err(err) => Err(err),
-                    }
                 };
 
                 let res = match out {

crates/ethcore/src/state/account.rs

@@ -298,13 +298,10 @@ impl Account {
     /// Get cached original storage value after last state commitment. Returns `None` if the key is not in the cache.
     pub fn cached_original_storage_at(&self, key: &H256) -> Option<H256> {
         match &self.original_storage_cache {
-            Some((_, ref original_storage_cache)) => {
+            Some((_, ref original_storage_cache)) => original_storage_cache
-                if let Some(value) = original_storage_cache.borrow_mut().get_mut(key) {
+                .borrow_mut()
-                    Some(value.clone())
+                .get_mut(key)
-                } else {
+                .map(|value| value.clone()),
-                    None
-                }
-            }
             None => self.cached_moved_original_storage_at(key),
         }
     }
@@ -317,11 +314,10 @@ impl Account {
             return Some(H256::new());
         }
 
-        if let Some(value) = self.storage_cache.borrow_mut().get_mut(key) {
+        self.storage_cache
-            Some(value.clone())
+            .borrow_mut()
-        } else {
+            .get_mut(key)
-            None
+            .map(|value| value.clone())
-        }
     }
 
     /// return the balance associated with this account.

crates/ethcore/src/state/mod.rs

@@ -246,10 +246,7 @@ pub fn prove_transaction_virtual<H: AsHashDB<KeccakHasher, DBValue> + Send + Syn
         factories,
     );
 
-    let mut state = match res {
+    let mut state = res.ok()?;
-        Ok(state) => state,
-        Err(_) => return None,
-    };
 
     let options = TransactOptions::with_no_tracing()
         .dont_check_nonce()

crates/ethcore/sync/src/api.rs

@@ -294,10 +294,7 @@ impl SyncProvider for EthSync {
                     .into_iter()
                     .zip(peer_info)
                     .filter_map(|(peer_id, peer_info)| {
-                        let session_info = match ctx.session_info(peer_id) {
+                        let session_info = ctx.session_info(peer_id)?;
-                            None => return None,
-                            Some(info) => info,
-                        };
 
                         Some(PeerInfo {
                             id: session_info.id.map(|id| format!("{:x}", id)),
@@ -746,14 +743,14 @@ impl NetworkConfiguration {
         Ok(BasicNetworkConfiguration {
             config_path: self.config_path,
             net_config_path: self.net_config_path,
-            listen_address: match self.listen_address {
+            listen_address: self
-                None => None,
+                .listen_address
-                Some(addr) => Some(SocketAddr::from_str(&addr)?),
+                .map(|addr| SocketAddr::from_str(&addr))
-            },
+                .transpose()?,
-            public_address: match self.public_address {
+            public_address: self
-                None => None,
+                .public_address
-                Some(addr) => Some(SocketAddr::from_str(&addr)?),
+                .map(|addr| SocketAddr::from_str(&addr))
-            },
+                .transpose()?,
             udp_port: self.udp_port,
             nat_enabled: self.nat_enabled,
             discovery_enabled: self.discovery_enabled,

crates/net/network-devp2p/src/node_table.rs

@@ -503,10 +503,7 @@ impl Drop for NodeTable {
 
 /// Check if node url is valid
 pub fn validate_node_url(url: &str) -> Option<Error> {
-    match Node::from_str(url) {
+    Node::from_str(url).err()
-        Ok(_) => None,
-        Err(e) => Some(e),
-    }
 }
 
 mod json {

crates/rpc/src/v1/impls/eth.rs

@@ -683,10 +683,10 @@ where
         let num = num.unwrap_or_default();
 
         try_bf!(check_known(&*self.client, num.clone()));
-        let res = match self.client.balance(&address, self.get_state(num)) {
+        let res = self
-            Some(balance) => Ok(balance),
+            .client
-            None => Err(errors::state_pruned()),
+            .balance(&address, self.get_state(num))
-        };
+            .ok_or_else(|| errors::state_pruned());
 
         Box::new(future::done(res))
     }
@@ -779,17 +779,13 @@ where
                         self.client.nonce(&address, BlockId::Latest)
                     });
 
-                match nonce {
+                nonce.ok_or_else(|| errors::database("latest nonce missing"))
-                    Some(nonce) => Ok(nonce),
-                    None => Err(errors::database("latest nonce missing")),
-                }
             }
             number => {
                 try_bf!(check_known(&*self.client, number.clone()));
-                match self.client.nonce(&address, block_number_to_id(number)) {
+                self.client
-                    Some(nonce) => Ok(nonce),
+                    .nonce(&address, block_number_to_id(number))
-                    None => Err(errors::state_pruned()),
+                    .ok_or_else(|| errors::state_pruned())
-                }
             }
         };
 

crates/rpc/src/v1/impls/parity_set.rs

@@ -110,10 +110,9 @@ where
     F: Fetch + 'static,
 {
     fn set_min_gas_price(&self, gas_price: U256) -> Result<bool> {
-        match self.miner.set_minimal_gas_price(gas_price) {
+        self.miner
-            Ok(success) => Ok(success),
+            .set_minimal_gas_price(gas_price)
-            Err(e) => Err(errors::unsupported(e, None)),
+            .map_err(|e| errors::unsupported(e, None))
-        }
     }
 
     fn set_transactions_limit(&self, _limit: usize) -> Result<bool> {