Merge pull request #793 from cloudflare/avery/fix-clippy

Fix some clippy warnings

src/commands/build/wranglerjs/mod.rs

@@ -43,7 +43,7 @@ pub fn run_build(target: &Target) -> Result<(), failure::Error> {
     let status = command.status()?;
 
     if status.success() {
-        let output = fs::read_to_string(temp_file.clone()).expect("could not retrieve ouput");
+        let output = fs::read_to_string(temp_file).expect("could not retrieve output");
 
         let wranglerjs_output: WranglerjsOutput =
             serde_json::from_str(&output).expect("could not parse wranglerjs output");

src/commands/kv/bucket/mod.rs

@@ -435,7 +435,7 @@ mod tests {
         let actual_path_with_hash =
             generate_path_with_hash(&path, hashed_value.to_owned()).unwrap();
 
-        let expected_path_with_hash = format!("path/to/asset.{}", hashed_value);;
+        let expected_path_with_hash = format!("path/to/asset.{}", hashed_value);
 
         assert_eq!(actual_path_with_hash, expected_path_with_hash);
     }

src/commands/kv/bucket/upload.rs

@@ -165,7 +165,7 @@ mod tests {
         // Old values found on remote
         let mut exclude_keys = HashSet::new();
         exclude_keys.insert(key_a_old.clone());
-        exclude_keys.insert(key_b_old.clone());
+        exclude_keys.insert(key_b_old);
 
         // local files (with b updated) to upload
         let pairs_to_upload = vec![
@@ -206,7 +206,7 @@ mod tests {
             // todo(gabbi): Implement PartialEq for KeyValuePair in cloudflare-rs.
             assert!(pair.key == actual[idx].key);
             assert!(pair.value == actual[idx].value);
-            idx = idx + 1;
+            idx += 1;
         }
     }
 }

src/commands/kv/bulk/delete.rs

@@ -38,10 +38,9 @@ pub fn delete(
         Ok(file_type) if file_type.is_file() => {
             let data = fs::read_to_string(filename)?;
             let keys_vec = serde_json::from_str(&data);
-            if keys_vec.is_err() {
-                failure::bail!("Failed to decode JSON. Please make sure to follow the format, [{\"key\": \"test_key\", \"value\": \"test_value\"}, ...]")
-            } else {
-                Ok(keys_vec.unwrap())
+            match keys_vec {
+                Ok(keys_vec) => Ok(keys_vec),
+                Err(_) => failure::bail!("Failed to decode JSON. Please make sure to follow the format, [{\"key\": \"test_key\", \"value\": \"test_value\"}, ...]")
             }
         }
         Ok(_) => failure::bail!("{} should be a JSON file, but is not", filename.display()),

src/commands/kv/bulk/put.rs

@@ -27,11 +27,9 @@ pub fn put(
         Ok(file_type) if file_type.is_file() => {
             let data = fs::read_to_string(filename)?;
             let data_vec = serde_json::from_str(&data);
-            if data_vec.is_err() {
-                Err(failure::format_err!("Failed to decode JSON. Please make sure to follow the format, [{{\"key\": \"test_key\", \"value\": \"test_value\"}}, ...]"))
-            } else {
-                let data_vec: Vec<KeyValuePair> = data_vec.unwrap();
-                Ok(data_vec)
+            match data_vec {
+                Ok(data_vec) => Ok(data_vec),
+                Err(_) => Err(failure::format_err!("Failed to decode JSON. Please make sure to follow the format, [{{\"key\": \"test_key\", \"value\": \"test_value\"}}, ...]"))
             }
         }
         Ok(_) => Err(failure::format_err!(
@@ -62,7 +60,7 @@ pub fn call_api(
     client: &impl ApiClient,
     target: &Target,
     namespace_id: &str,
-    pairs: &Vec<KeyValuePair>,
+    pairs: &[KeyValuePair],
 ) -> Result<ApiSuccess<()>, ApiFailure> {
     client.request(&WriteBulk {
         account_identifier: &target.account_id,

src/commands/kv/key/mod.rs

@@ -8,4 +8,4 @@ pub use delete::delete;
 pub use get::get;
 pub use key_list::KeyList;
 pub use list::list;
-pub use put::put;
+pub use put::{put, KVMetaData};

src/commands/kv/key/put.rs

@@ -14,32 +14,32 @@ use crate::settings::global_user::GlobalUser;
 use crate::settings::target::Target;
 use crate::terminal::message;
 
-pub fn put(
-    target: &Target,
-    user: &GlobalUser,
-    id: &str,
-    key: &str,
-    value: &str,
-    is_file: bool,
-    expiration: Option<&str>,
-    expiration_ttl: Option<&str>,
-) -> Result<(), failure::Error> {
+pub struct KVMetaData {
+    pub namespace_id: String,
+    pub key: String,
+    pub value: String,
+    pub is_file: bool,
+    pub expiration: Option<String>,
+    pub expiration_ttl: Option<String>,
+}
+
+pub fn put(target: &Target, user: &GlobalUser, data: KVMetaData) -> Result<(), failure::Error> {
     kv::validate_target(target)?;
 
     let api_endpoint = format!(
         "https://api.cloudflare.com/client/v4/accounts/{}/storage/kv/namespaces/{}/values/{}",
         target.account_id,
-        id,
-        kv::url_encode_key(key)
+        &data.namespace_id,
+        kv::url_encode_key(&data.key)
     );
 
     // Add expiration and expiration_ttl query options as necessary.
     let mut query_params: Vec<(&str, &str)> = vec![];
 
-    if let Some(exp) = expiration {
+    if let Some(exp) = &data.expiration {
         query_params.push(("expiration", exp))
     };
-    if let Some(ttl) = expiration_ttl {
+    if let Some(ttl) = &data.expiration_ttl {
         query_params.push(("expiration_ttl", ttl))
     };
     let url = Url::parse_with_params(&api_endpoint, query_params);
@@ -50,20 +50,21 @@ pub fn put(
 
     // If is_file is true, overwrite value to be the contents of the given
     // filename in the 'value' arg.
-    let mut res = if is_file {
-        match &metadata(value) {
+    let mut res = if data.is_file {
+        match &metadata(&data.value) {
             Ok(file_type) if file_type.is_file() => {
-                let file = fs::File::open(value)?;
+                let file = fs::File::open(&data.value)?;
                 client.put(&url_into_str).body(file).send()?
             }
-            Ok(file_type) if file_type.is_dir() => {
-                failure::bail!("--path argument takes a file, {} is a directory", value)
-            }
-            Ok(_) => failure::bail!("--path argument takes a file, {} is a symlink", value), // last remaining value is symlink
+            Ok(file_type) if file_type.is_dir() => failure::bail!(
+                "--path argument takes a file, {} is a directory",
+                data.value
+            ),
+            Ok(_) => failure::bail!("--path argument takes a file, {} is a symlink", data.value), // last remaining value is symlink
             Err(e) => failure::bail!("{}", e),
         }
     } else {
-        client.put(&url_into_str).body(value.to_string()).send()?
+        client.put(&url_into_str).body(data.value).send()?
     };
 
     if res.status().is_success() {

src/commands/publish/upload_form/mod.rs

@@ -37,7 +37,7 @@ pub fn build_script_and_upload_form(
             build_generated_dir()?;
             concat_js(&name)?;
 
-            let path = format!("./pkg/{}_bg.wasm", name).to_string();
+            let path = format!("./pkg/{}_bg.wasm", name);
             let binding = "wasm".to_string();
             let wasm_module = WasmModule::new(path, binding)?;
 

src/main.rs

@@ -20,6 +20,7 @@ mod settings;
 mod terminal;
 mod util;
 
+use crate::commands::kv::key::KVMetaData;
 use crate::settings::target::TargetType;
 use exitfailure::ExitFailure;
 use terminal::emoji;
@@ -544,7 +545,7 @@ fn run() -> Result<(), failure::Error> {
                         .unwrap() // clap configs ensure that if "binding" isn't present,"namespace-id" must be.
                         .to_string(),
                 };
-                (target, namespace_id.to_string())
+                (target, namespace_id)
             }
             None => unreachable!(), // this is unreachable because all kv:key commands have required arguments.
         };
@@ -555,23 +556,27 @@ fn run() -> Result<(), failure::Error> {
                 commands::kv::key::get(&target, &user, &namespace_id, key)?
             }
             ("put", Some(put_key_matches)) => {
-                let key = put_key_matches.value_of("key").unwrap();
+                let key = put_key_matches.value_of("key").unwrap().to_string();
 
                 // If is_file is true, overwrite value to be the contents of the given
                 // filename in the 'value' arg.
-                let value = put_key_matches.value_of("value").unwrap();
-                let expiration = put_key_matches.value_of("expiration");
-                let ttl = put_key_matches.value_of("expiration-ttl");
-                commands::kv::key::put(
-                    &target,
-                    &user,
-                    &namespace_id,
+                let value = put_key_matches.value_of("value").unwrap().to_string();
+                let is_file = put_key_matches.is_present("path");
+                let expiration = put_key_matches
+                    .value_of("expiration")
+                    .map(|e| e.to_string());
+                let expiration_ttl = put_key_matches
+                    .value_of("expiration-ttl")
+                    .map(|t| t.to_string());
+                let kv_metadata = KVMetaData {
+                    namespace_id,
                     key,
-                    &value,
-                    put_key_matches.is_present("path"),
+                    value,
+                    is_file,
                     expiration,
-                    ttl,
-                )?
+                    expiration_ttl,
+                };
+                commands::kv::key::put(&target, &user, kv_metadata)?
             }
             ("delete", Some(delete_key_matches)) => {
                 let key = delete_key_matches.value_of("key").unwrap();
@@ -603,7 +608,7 @@ fn run() -> Result<(), failure::Error> {
                         .unwrap() // clap configs ensure that if "binding" isn't present,"namespace-id" must be.
                         .to_string(),
                 };
-                (target, namespace_id.to_string())
+                (target, namespace_id)
             }
             None => unreachable!(), // this is unreachable because all kv:key commands have required arguments.
         };

src/settings/target/mod.rs

@@ -151,9 +151,9 @@ impl Manifest {
             account_id: String::new(),
             env: None,
             kv_namespaces: None,
-            name: name.clone(),
+            name,
             private: None,
-            target_type: target_type.clone(),
+            target_type,
             route: Some(String::new()),
             routes: None,
             webpack_config: None,

tests/config.rs

@@ -34,7 +34,7 @@ fn generate_config_with(eol: &str) {
     let config_file = fake_home_dir.join("config").join("default.toml");
 
     let config = fs::read_to_string(&config_file)
-        .expect(&format!("could not read config at {:?}", &config_file));
+        .unwrap_or_else(|_| panic!("could not read config at {:?}", &config_file));
     assert_eq!(
         config,
         r#"email = "[email protected]"