Little Refactors (#683)

* Refactors!
* Removed duplicate CRATESFYI_PREFIX variable

Co-Authored-By: Joshua Nelson <[email protected]>

* Split reponame into a separate variable
* Return query error
* Changed ok_or_else to ok_or
* Annotated &&str dereferences with reasoning

Author: Kixiron

build.rs

@@ -19,6 +19,7 @@ fn main() {
 fn write_git_version() {
     let maybe_hash = get_git_hash();
     let git_hash = maybe_hash.as_deref().unwrap_or("???????");
+
     let build_date = time::strftime("%Y-%m-%d", &time::now_utc()).unwrap();
     let dest_path = Path::new(&env::var("OUT_DIR").unwrap()).join("git_version");
 

src/db/add_package.rs

@@ -410,6 +410,7 @@ fn add_keywords_into_database(
                 .get(0)
             }
         };
+
         // add releationship
         let _ = conn.query(
             "INSERT INTO keyword_rels (rid, kid) VALUES ($1, $2)",

src/db/file.rs

@@ -127,13 +127,15 @@ pub(super) fn s3_client() -> Option<S3Client> {
     if std::env::var_os("AWS_ACCESS_KEY_ID").is_none() && std::env::var_os("FORCE_S3").is_none() {
         return None;
     }
+
     let creds = match DefaultCredentialsProvider::new() {
         Ok(creds) => creds,
         Err(err) => {
             warn!("failed to retrieve AWS credentials: {}", err);
             return None;
         }
     };
+
     Some(S3Client::new_with(
         rusoto_core::request::HttpClient::new().unwrap(),
         creds,

src/docbuilder/crates.rs

@@ -18,13 +18,16 @@ where
     for line in reader.lines() {
         // some crates have invalid UTF-8 (nanny-sys-0.0.7)
         // skip them
-        let line = match line {
-            Ok(l) => l,
-            Err(_) => continue,
+        let line = if let Ok(line) = line {
+            line
+        } else {
+            continue;
         };
-        let data = match Json::from_str(line.trim()) {
-            Ok(d) => d,
-            Err(_) => continue,
+
+        let data = if let Ok(data) = Json::from_str(line.trim()) {
+            data
+        } else {
+            continue;
         };
 
         let obj = data

src/docbuilder/mod.rs

@@ -38,6 +38,7 @@ impl DocBuilder {
     /// Loads build cache
     pub fn load_cache(&mut self) -> Result<()> {
         debug!("Loading cache");
+
         let path = PathBuf::from(&self.options.prefix).join("cache");
         let reader = fs::File::open(path).map(BufReader::new);
 
@@ -54,6 +55,7 @@ impl DocBuilder {
 
     fn load_database_cache(&mut self) -> Result<()> {
         debug!("Loading database cache");
+
         use crate::db::connect_db;
         let conn = connect_db()?;
 
@@ -64,6 +66,7 @@ impl DocBuilder {
         )? {
             let name: String = row.get(0);
             let version: String = row.get(1);
+
             self.db_cache.insert(format!("{}-{}", name, version));
         }
 
@@ -73,11 +76,14 @@ impl DocBuilder {
     /// Saves build cache
     pub fn save_cache(&self) -> Result<()> {
         debug!("Saving cache");
+
         let path = PathBuf::from(&self.options.prefix).join("cache");
         let mut file = fs::OpenOptions::new().write(true).create(true).open(path)?;
+
         for krate in &self.cache {
             writeln!(file, "{}", krate)?;
         }
+
         Ok(())
     }
 
@@ -91,6 +97,7 @@ impl DocBuilder {
         if !path.exists() {
             fs::OpenOptions::new().write(true).create(true).open(path)?;
         }
+
         Ok(())
     }
 
@@ -100,6 +107,7 @@ impl DocBuilder {
         if path.exists() {
             fs::remove_file(path)?;
         }
+
         Ok(())
     }
 
@@ -121,6 +129,7 @@ impl DocBuilder {
         let name = format!("{}-{}", name, version);
         let local = self.options.skip_if_log_exists && self.cache.contains(&name);
         let db = self.options.skip_if_exists && self.db_cache.contains(&name);
+
         !(local || db)
     }
 }

src/docbuilder/options.rs

@@ -55,6 +55,7 @@ impl DocBuilderOptions {
     /// Creates new DocBuilderOptions from prefix
     pub fn from_prefix(prefix: PathBuf) -> DocBuilderOptions {
         let (prefix, crates_io_index_path) = generate_paths(prefix);
+
         DocBuilderOptions {
             prefix,
             crates_io_index_path,
@@ -69,6 +70,7 @@ impl DocBuilderOptions {
                 self.crates_io_index_path.display()
             );
         }
+
         Ok(())
     }
 }

src/docbuilder/rustwide_builder.rs

@@ -122,8 +122,9 @@ impl RustwideBuilder {
 
         let mut targets_to_install = TARGETS
             .iter()
-            .map(|&t| t.to_string())
+            .map(|&t| t.to_string()) // &str has a specialized ToString impl, while &&str goes through Display
             .collect::<HashSet<_>>();
+
         let installed_targets = match self.toolchain.installed_targets(&self.workspace) {
             Ok(targets) => targets,
             Err(err) => {

src/utils/daemon.rs

@@ -19,16 +19,16 @@ use time;
 use ::{libc::fork, std::fs::File, std::io::Write, std::process::exit};
 
 pub fn start_daemon(background: bool) {
-    // first check required environment variables
-    for v in [
+    const CRATE_VARIABLES: [&str; 3] = [
         "CRATESFYI_PREFIX",
         "CRATESFYI_GITHUB_USERNAME",
         "CRATESFYI_GITHUB_ACCESSTOKEN",
-    ]
-    .iter()
-    {
+    ];
+
+    // first check required environment variables
+    for v in CRATE_VARIABLES.iter() {
         if env::var(v).is_err() {
-            panic!("Environment variable {} not found", v);
+            panic!("Environment variable {} not found", v)
         }
     }
 
@@ -42,6 +42,7 @@ pub fn start_daemon(background: bool) {
         {
             panic!("running in background not supported on windows");
         }
+
         #[cfg(not(target_os = "windows"))]
         {
             // fork the process
@@ -87,6 +88,7 @@ pub fn start_daemon(background: bool) {
         .unwrap();
 
     // build new crates every minute
+    // REFACTOR: Break this into smaller functions
     thread::Builder::new().name("build queue reader".to_string()).spawn(move || {
         let opts = opts();
         let mut doc_builder = DocBuilder::new(opts);
@@ -146,6 +148,7 @@ pub fn start_daemon(background: bool) {
                     error!("Failed to read the number of crates in the queue: {}", e);
                     continue;
                 }
+
                 Ok(0) => {
                     if status.count() > 0 {
                         // ping the hubs before continuing
@@ -162,6 +165,7 @@ pub fn start_daemon(background: bool) {
                     status = BuilderState::EmptyQueue;
                     continue;
                 }
+
                 Ok(queue_count) => {
                     info!("Starting build with {} crates in queue (currently on a {} crate streak)",
                           queue_count, status.count());
@@ -185,7 +189,6 @@ pub fn start_daemon(background: bool) {
                     Ok(crate_built) => if crate_built {
                         status.increment();
                     }
-
                 }
             }));
 
@@ -257,6 +260,7 @@ pub fn start_daemon(background: bool) {
 
     // at least start web server
     info!("Starting web server");
+
     crate::Server::start(None);
 }
 

src/utils/github_updater.rs

@@ -136,16 +136,16 @@ fn get_github_path(url: &str) -> Option<String> {
         Some(cap) => {
             let username = cap.get(1).unwrap().as_str();
             let reponame = cap.get(2).unwrap().as_str();
-            Some(format!(
-                "{}/{}",
-                username,
-                if reponame.ends_with(".git") {
-                    reponame.split(".git").next().unwrap()
-                } else {
-                    reponame
-                }
-            ))
+
+            let reponame = if reponame.ends_with(".git") {
+                reponame.split(".git").next().unwrap()
+            } else {
+                reponame
+            };
+
+            Some(format!("{}/{}", username, reponame))
         }
+
         None => None,
     }
 }

src/utils/html.rs

@@ -32,13 +32,15 @@ fn extract_from_rcdom(dom: &RcDom) -> Result<(Handle, Handle)> {
                         head = Some(handle.clone());
                     }
                 }
+
                 "body" => {
                     if body.is_some() {
                         return Err(err_msg("duplicate <body> tag"));
                     } else {
                         body = Some(handle.clone());
                     }
                 }
+
                 _ => {} // do nothing
             }
         }
@@ -68,6 +70,7 @@ fn extract_class(node: &Handle) -> String {
                 .find(|a| &a.name.local == "class")
                 .map_or(String::new(), |a| a.value.to_string())
         }
+
         _ => String::new(),
     }
 }
@@ -79,6 +82,7 @@ mod test {
         let (head, body, class) = super::extract_head_and_body(
             r#"<head><meta name="generator" content="rustdoc"></head><body class="rustdoc struct"><p>hello</p>"#
         ).unwrap();
+
         assert_eq!(head, r#"<meta name="generator" content="rustdoc">"#);
         assert_eq!(body, "<p>hello</p>");
         assert_eq!(class, "rustdoc struct");
@@ -91,6 +95,7 @@ mod test {
         let expected_head = std::fs::read_to_string("tests/regex/head.html").unwrap();
         let expected_body = std::fs::read_to_string("tests/regex/body.html").unwrap();
         let (head, body, class) = super::extract_head_and_body(&original).unwrap();
+
         assert_eq!(head, expected_head.trim());
         assert_eq!(&body, &expected_body.trim());
         assert_eq!(class, "rustdoc struct");