Upgrade clippy and fmt

This patch 1. removes redundant closures 2. removes an unreachable pattern 3. renames clippy::cognitive_complexity to clippy::cyclomatic_complexity 4. allows deprecated item(std::error::Error::cause) * Allow it until error-chain and quick-error macros are fixed. 5. removes a redundant import 6. makes clippy skip false alarms
CodeChain-io · Apr 12, 2019 · 03d6df9 · 03d6df9
1 parent 0f128d4
commit 03d6df9
Show file tree

Hide file tree

Showing 29 changed files with 148 additions and 108 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -58,17 +58,17 @@ jobs:
         - SKIP=`.travis/check-change '^test/'` && if [[ "$SKIP" = "noskip" ]]; then CHECK_RUST=1; fi
       install:
         - if [[ $CHECK_RUST ]]; then
-            rustup toolchain install nightly-2018-12-06 || exit 1;
-            rustup component add rustfmt-preview --toolchain nightly-2018-12-06 || exit 1;
-            rustup component add clippy-preview --toolchain nightly-2018-12-06 || exit 1;
+            rustup toolchain install nightly-2019-04-11 || exit 1;
+            rustup component add rustfmt-preview --toolchain nightly-2019-04-11 || exit 1;
+            rustup component add clippy-preview --toolchain nightly-2019-04-11 || exit 1;
           fi
         - nvm install 10
         - nvm use 10
         - npm install -g yarn
       script:
         - if [[ $CHECK_RUST ]]; then
-            cargo +nightly-2018-12-06 fmt -- --check || FAILED=1;
-            cargo +nightly-2018-12-06 clippy --all --all-targets -- -D warnings || FAILED=1;
+            cargo +nightly-2019-04-11 fmt -- --check || FAILED=1;
+            cargo +nightly-2019-04-11 clippy --all --all-targets -- -D warnings || FAILED=1;
           fi; test ! $FAILED
         - cd test && yarn && yarn lint
     - stage: deploy

diff --git a/codechain/config/mod.rs b/codechain/config/mod.rs
@@ -58,7 +58,7 @@ impl Config {
 
     pub fn miner_options(&self) -> Result<MinerOptions, String> {
         let (reseal_on_own_transaction, reseal_on_external_transaction) =
-            match self.mining.reseal_on_txs.as_ref().map(|s| s.as_str()) {
+            match self.mining.reseal_on_txs.as_ref().map(String::as_str) {
                 Some("all") => (true, true),
                 Some("own") => (true, false),
                 Some("ext") => (false, true),

diff --git a/codechain/run_node.rs b/codechain/run_node.rs
@@ -25,7 +25,7 @@ use ccore::{
     Miner, MinerService, Scheme, Stratum, StratumConfig, StratumError, NUM_COLUMNS,
 };
 use cdiscovery::{Config, Discovery};
-use ckey::{Address, NetworkId};
+use ckey::{Address, NetworkId, PlatformAddress};
 use ckeystore::accounts_dir::RootDiskDirectory;
 use ckeystore::KeyStore;
 use clap::ArgMatches;
@@ -81,7 +81,7 @@ fn discovery_start(
         bucket_size: cfg.discovery_bucket_size.unwrap(),
         t_refresh: cfg.discovery_refresh.unwrap(),
     };
-    let use_kademlia = match cfg.discovery_type.as_ref().map(|s| s.as_str()) {
+    let use_kademlia = match cfg.discovery_type.as_ref().map(String::as_str) {
         Some("unstructured") => false,
         Some("kademlia") => true,
         Some(discovery_type) => return Err(format!("Unknown discovery {}", discovery_type)),
@@ -154,7 +154,7 @@ fn new_miner(
                 }
             },
             EngineType::Solo => miner
-                .set_author(config.mining.author.map_or(Address::default(), |a| a.into_address()))
+                .set_author(config.mining.author.map_or(Address::default(), PlatformAddress::into_address))
                 .expect("set_author never fails when Solo is used"),
         }
     }
@@ -203,7 +203,7 @@ fn unlock_accounts(ap: &AccountProvider, pf: &PasswordFile) -> Result<(), String
 }
 
 pub fn open_db(cfg: &config::Operating, client_config: &ClientConfig) -> Result<Arc<KeyValueDB>, String> {
-    let db_path = cfg.db_path.as_ref().map(|s| s.as_str()).unwrap();
+    let db_path = cfg.db_path.as_ref().map(String::as_str).unwrap();
     let client_path = Path::new(db_path);
     let mut db_config = DatabaseConfig::with_columns(NUM_COLUMNS);
 

diff --git a/core/src/block.rs b/core/src/block.rs
@@ -244,7 +244,7 @@ impl<'x> OpenBlock<'x> {
         })?;
         self.block.header.set_transactions_root(skewed_merkle_root(
             parent_transactions_root,
-            self.block.transactions.iter().map(|e| e.rlp_bytes()),
+            self.block.transactions.iter().map(Encodable::rlp_bytes),
         ));
         self.block.header.set_state_root(state_root);
 

diff --git a/core/src/blockchain/blockchain.rs b/core/src/blockchain/blockchain.rs
@@ -391,6 +391,8 @@ impl BlockChain {
     #[allow(dead_code)]
     pub fn epoch_transition_for(&self, parent_hash: H256) -> Option<EpochTransition> {
         // slow path: loop back block by block
+        #[allow(clippy::identity_conversion)]
+        // This is a false alarm. https://github.com/rust-lang/rust-clippy/issues/3944
         for hash in self.ancestry_iter(parent_hash)? {
             let details = self.block_details(&hash)?;
 

diff --git a/core/src/client/client.rs b/core/src/client/client.rs
@@ -199,7 +199,7 @@ impl Client {
         self.queue_transactions.fetch_sub(transactions.len(), AtomicOrdering::SeqCst);
         let transactions: Vec<UnverifiedTransaction> =
             transactions.iter().filter_map(|bytes| UntrustedRlp::new(bytes).as_val().ok()).collect();
-        let hashes: Vec<_> = transactions.iter().map(|tx| tx.hash()).collect();
+        let hashes: Vec<_> = transactions.iter().map(UnverifiedTransaction::hash).collect();
         self.notify(|notify| {
             notify.transactions_received(hashes.clone(), peer_id);
         });

diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs
@@ -417,7 +417,7 @@ impl Worker {
 
         proposal.and_then(|proposal| {
             let block_hash = proposal.on.block_hash.expect("Proposal message always include block hash");
-            let bytes = self.client().block(&BlockId::Hash(block_hash)).map(|block| block.into_inner());
+            let bytes = self.client().block(&BlockId::Hash(block_hash)).map(encoded::Block::into_inner);
             bytes.map(|bytes| (proposal.signature, proposal.signer_index, bytes))
         })
     }
@@ -1093,6 +1093,8 @@ impl Worker {
         let precommit_hash = message_hash(step, *header.parent_hash());
         let mut counter = 0;
 
+        #[allow(clippy::identity_conversion)]
+        // This is a false alarm. https://github.com/rust-lang/rust-clippy/issues/3944
         for (bitset_index, signature) in seal_view.signatures()? {
             let public = self.validators.get(header.parent_hash(), bitset_index);
             if !verify_schnorr(&public, &signature, &precommit_hash)? {

diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs
@@ -31,7 +31,7 @@ use kvdb::KeyValueDB;
 use parking_lot::{Mutex, RwLock};
 use primitives::{Bytes, H256};
 
-use super::mem_pool::MemPool;
+use super::mem_pool::{Error as MemPoolError, MemPool};
 use super::mem_pool_types::{AccountDetails, MemPoolInput, TxOrigin, TxTimelock};
 use super::sealing_queue::SealingQueue;
 use super::work_notify::{NotifyWork, WorkPoster};
@@ -203,7 +203,7 @@ impl Miner {
 
     /// Get `Some` `clone()` of the current pending block or `None` if we're not sealing.
     pub fn pending_block(&self, latest_block_number: BlockNumber) -> Option<Block> {
-        self.map_pending_block(|b| b.to_base(), latest_block_number)
+        self.map_pending_block(IsBlock::to_base, latest_block_number)
     }
 
     /// Get `Some` `clone()` of the current pending block header or `None` if we're not sealing.
@@ -328,7 +328,7 @@ impl Miner {
                 Err(e) => Err(e),
                 Ok(()) => {
                     let idx = insertion_results_index;
-                    let result = insertion_results[idx].clone().map_err(|x| x.into_core_error())?;
+                    let result = insertion_results[idx].clone().map_err(MemPoolError::into_core_error)?;
                     inserted.push(tx_hashes[idx]);
                     insertion_results_index += 1;
                     Ok(result)

diff --git a/crypto/src/error.rs b/crypto/src/error.rs
@@ -14,9 +14,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use rcrypto;
-use ring;
-
 quick_error! {
     #[derive(Debug)]
     pub enum Error {
@@ -48,32 +45,39 @@ quick_error! {
     }
 }
 
-quick_error! {
-    #[derive(Debug)]
-    pub enum SymmError wraps PrivSymmErr {
-        RustCrypto(e: rcrypto::symmetriccipher::SymmetricCipherError) {
-            display("symmetric crypto error")
-            from()
-        }
-        Ring(e: ring::error::Unspecified) {
-            display("symmetric crypto error")
-            cause(e)
-            from()
-        }
-        Offset(x: usize) {
-            display("offset {} greater than slice length", x)
+#[allow(deprecated)]
+mod errors {
+    use rcrypto;
+    use ring;
+
+    quick_error! {
+        #[derive(Debug)]
+        pub enum SymmError wraps PrivSymmErr {
+            RustCrypto(e: rcrypto::symmetriccipher::SymmetricCipherError) {
+                display("symmetric crypto error")
+                    from()
+            }
+            Ring(e: ring::error::Unspecified) {
+                display("symmetric crypto error")
+                    cause(e)
+                    from()
+            }
+            Offset(x: usize) {
+                display("offset {} greater than slice length", x)
+            }
         }
     }
-}
 
-impl From<ring::error::Unspecified> for SymmError {
-    fn from(e: ring::error::Unspecified) -> SymmError {
-        SymmError(PrivSymmErr::Ring(e))
+    impl From<ring::error::Unspecified> for SymmError {
+        fn from(e: ring::error::Unspecified) -> SymmError {
+            SymmError(PrivSymmErr::Ring(e))
+        }
     }
-}
 
-impl From<rcrypto::symmetriccipher::SymmetricCipherError> for SymmError {
-    fn from(e: rcrypto::symmetriccipher::SymmetricCipherError) -> SymmError {
-        SymmError(PrivSymmErr::RustCrypto(e))
+    impl From<rcrypto::symmetriccipher::SymmetricCipherError> for SymmError {
+        fn from(e: rcrypto::symmetriccipher::SymmetricCipherError) -> SymmError {
+            SymmError(PrivSymmErr::RustCrypto(e))
+        }
     }
 }
+pub use self::errors::SymmError;
diff --git a/discovery/src/extension.rs b/discovery/src/extension.rs
@@ -104,7 +104,7 @@ impl NetworkExtension<Never> for Extension {
 
                     addresses
                         .into_iter()
-                        .map(|kademlia_id| kademlia_id.into())
+                        .map(From::from)
                         .take(::std::cmp::min(self.config.bucket_size, len) as usize)
                         .collect()
                 } else {

diff --git a/keystore/src/accounts_dir/disk.rs b/keystore/src/accounts_dir/disk.rs
@@ -15,6 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::collections::HashMap;
+use std::ffi::OsStr;
 use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::{fs, io};
@@ -29,11 +30,11 @@ const IGNORED_FILES: &[&str] = &["thumbs.db"];
 
 #[cfg(not(windows))]
 fn restrict_permissions_to_owner(file_path: &Path) -> Result<(), i32> {
-    use libc;
+    use libc::{chmod, S_IRUSR, S_IWUSR};
     use std::ffi;
 
     let cstr = ffi::CString::new(&*file_path.to_string_lossy()).map_err(|_| -1)?;
-    match unsafe { libc::chmod(cstr.as_ptr(), libc::S_IWUSR | libc::S_IRUSR) } {
+    match unsafe { chmod(cstr.as_ptr(), S_IWUSR | S_IRUSR) } {
         0 => Ok(()),
         x => Err(x),
     }
@@ -146,7 +147,7 @@ where
             .into_iter()
             .filter_map(|path| {
                 let filename = Some(
-                    path.file_name().and_then(|n| n.to_str()).expect("Keys have valid UTF8 names only.").to_owned(),
+                    path.file_name().and_then(OsStr::to_str).expect("Keys have valid UTF8 names only.").to_owned(),
                 );
                 fs::File::open(path.clone())
                     .map_err(Into::into)

diff --git a/keystore/src/import.rs b/keystore/src/import.rs
@@ -15,6 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::collections::HashSet;
+use std::ffi::OsStr;
 use std::fs;
 use std::path::Path;
 
@@ -27,7 +28,7 @@ use crate::Error;
 pub fn import_account(path: &Path, dst: &KeyDirectory) -> Result<Address, Error> {
     let key_manager = DiskKeyFileManager;
     let existing_accounts = dst.load()?.into_iter().map(|a| a.address).collect::<HashSet<_>>();
-    let filename = path.file_name().and_then(|n| n.to_str()).map(|f| f.to_owned());
+    let filename = path.file_name().and_then(OsStr::to_str).map(ToOwned::to_owned);
     let account = fs::File::open(&path).map_err(Into::into).and_then(|file| key_manager.read(filename, file))?;
 
     let address = account.address;

diff --git a/keystore/src/keystore.rs b/keystore/src/keystore.rs
@@ -224,6 +224,8 @@ impl KeyMultiStore {
         let mut cache = self.cache.write();
 
         let mut new_accounts = BTreeMap::new();
+        #[allow(clippy::identity_conversion)]
+        // This is a false alarm. https://github.com/rust-lang/rust-clippy/issues/3944
         for account in self.dir.load()? {
             let account_ref = account.address;
             new_accounts.entry(account_ref).or_insert_with(Vec::new).push(account);
@@ -369,6 +371,8 @@ impl SimpleSecretStore for KeyMultiStore {
         new_password: &Password,
     ) -> Result<(), Error> {
         let mut changed_any = false;
+        #[allow(clippy::identity_conversion)]
+        // This is a false alarm. https://github.com/rust-lang/rust-clippy/issues/3944
         for account in self.get_safe_accounts(account_ref)? {
             let new_account = match account.change_password(old_password, new_password, self.iterations) {
                 Ok(new_account) => new_account,

diff --git a/network/src/p2p/handler.rs b/network/src/p2p/handler.rs
@@ -402,7 +402,7 @@ impl IoHandler<Message> for Handler {
         Ok(())
     }
 
-    #[allow(clippy::cyclomatic_complexity)]
+    #[allow(clippy::cognitive_complexity)]
     fn message(&self, io: &IoContext<Message>, message: Message) -> IoHandlerResult<()> {
         match message {
             Message::RequestConnection(socket_address) => {
@@ -598,7 +598,7 @@ impl IoHandler<Message> for Handler {
         Ok(())
     }
 
-    #[allow(clippy::cyclomatic_complexity)]
+    #[allow(clippy::cognitive_complexity)]
     fn stream_readable(&self, io: &IoContext<Message>, stream_token: StreamToken) -> IoHandlerResult<()> {
         match stream_token {
             ACCEPT => {

diff --git a/rpc/src/v1/types/action.rs b/rpc/src/v1/types/action.rs
@@ -625,7 +625,7 @@ impl From<Action> for Result<ActionType, ConversionError> {
                     approver,
                     registrar,
                     allowed_script_hashes,
-                    inputs: inputs.into_iter().map(|input| input.into()).collect(),
+                    inputs: inputs.into_iter().map(From::from).collect(),
                     output: Box::new(output_content),
                     approvals,
                 }

diff --git a/rpc/src/v1/types/asset.rs b/rpc/src/v1/types/asset.rs
@@ -14,6 +14,8 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
+use std::ops::Deref;
+
 use cjson::uint::Uint;
 use cstate::{Asset as AssetType, OwnedAsset as OwnedAssetType};
 use primitives::{H160, H256};
@@ -54,7 +56,7 @@ impl From<OwnedAssetType> for OwnedAsset {
             },
             lock_script_hash: *asset.lock_script_hash(),
             order_hash: *asset.order_hash(),
-            parameters: asset.parameters().iter().map(|bytes| bytes.to_hex()).collect(),
+            parameters: asset.parameters().iter().map(Deref::deref).map(<[u8]>::to_hex).collect(),
         }
     }
 }
diff --git a/rpc/src/v1/types/asset_output.rs b/rpc/src/v1/types/asset_output.rs
@@ -16,6 +16,7 @@
 extern crate rustc_serialize;
 
 use std::iter::FromIterator;
+use std::ops::Deref;
 
 use cjson::uint::Uint;
 use ctypes::transaction::{AssetMintOutput as AssetMintOutputType, AssetTransferOutput as AssetTransferOutputType};
@@ -37,7 +38,7 @@ impl From<AssetTransferOutputType> for AssetTransferOutput {
     fn from(from: AssetTransferOutputType) -> Self {
         AssetTransferOutput {
             lock_script_hash: from.lock_script_hash,
-            parameters: from.parameters.iter().map(|bytes| bytes.to_hex()).collect(),
+            parameters: from.parameters.iter().map(Deref::deref).map(<[u8]>::to_hex).collect(),
             asset_type: from.asset_type,
             shard_id: from.shard_id,
             quantity: from.quantity.into(),
@@ -49,7 +50,7 @@ impl From<AssetTransferOutput> for Result<AssetTransferOutputType, FromHexError>
     fn from(from: AssetTransferOutput) -> Self {
         Ok(AssetTransferOutputType {
             lock_script_hash: from.lock_script_hash,
-            parameters: Result::from_iter(from.parameters.iter().map(|hexstr| hexstr.from_hex()))?,
+            parameters: Result::from_iter(from.parameters.iter().map(Deref::deref).map(FromHex::from_hex))?,
             asset_type: from.asset_type,
             shard_id: from.shard_id,
             quantity: from.quantity.into(),
@@ -69,7 +70,7 @@ impl From<AssetMintOutputType> for AssetMintOutput {
     fn from(from: AssetMintOutputType) -> Self {
         AssetMintOutput {
             lock_script_hash: from.lock_script_hash,
-            parameters: from.parameters.iter().map(|bytes| bytes.to_hex()).collect(),
+            parameters: from.parameters.iter().map(Deref::deref).map(<[u8]>::to_hex).collect(),
             supply: from.supply.into(),
         }
     }
@@ -79,7 +80,7 @@ impl From<AssetMintOutput> for Result<AssetMintOutputType, FromHexError> {
     fn from(from: AssetMintOutput) -> Self {
         Ok(AssetMintOutputType {
             lock_script_hash: from.lock_script_hash,
-            parameters: Result::from_iter(from.parameters.iter().map(|hexstr| hexstr.from_hex()))?,
+            parameters: Result::from_iter(from.parameters.iter().map(Deref::deref).map(FromHex::from_hex))?,
             supply: from.supply.into(),
         })
     }