Rebased with master and followed a few clippy lints

Kixiron · Joshua Nelson · commit b7afc73eb728 · 2020-05-30T22:25:59.000-04:00
diff --git a/src/db/add_package.rs b/src/db/add_package.rs
@@ -185,13 +185,16 @@ fn initialize_package_in_database(conn: &Connection, pkg: &MetadataPackage) -> R
 
 /// Convert dependencies into Vec<(String, String, String)>
 fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> {
-    let mut dependencies: Vec<(String, String, String)> = Vec::new();
+    let mut dependencies: Vec<(String, String, String)> =
+        Vec::with_capacity(pkg.dependencies.len());
+
     for dependency in &pkg.dependencies {
         let name = dependency.name.clone();
         let version = dependency.req.clone();
         let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into());
         dependencies.push((name, version, kind.to_string()));
     }
+
     dependencies
 }
 
diff --git a/src/db/blacklist.rs b/src/db/blacklist.rs
@@ -83,11 +83,11 @@ mod tests {
         crate::test::wrapper(|env| {
             let db = env.db();
 
-            assert!(is_blacklisted(&db.conn(), "crate foo")? == false);
+            assert!(!is_blacklisted(&db.conn(), "crate foo")?);
             add_crate(&db.conn(), "crate foo")?;
-            assert!(is_blacklisted(&db.conn(), "crate foo")? == true);
+            assert!(is_blacklisted(&db.conn(), "crate foo")?);
             remove_crate(&db.conn(), "crate foo")?;
-            assert!(is_blacklisted(&db.conn(), "crate foo")? == false);
+            assert!(!is_blacklisted(&db.conn(), "crate foo")?);
             Ok(())
         });
     }
diff --git a/src/db/file.rs b/src/db/file.rs
@@ -37,17 +37,15 @@ pub fn add_path_into_database<P: AsRef<Path>>(
 }
 
 fn file_list_to_json(file_list: Vec<(PathBuf, String)>) -> Result<Value> {
-    let mut file_list_json: Vec<Value> = Vec::new();
-
-    for file in file_list {
-        let mut v = Vec::with_capacity(2);
-        v.push(Value::String(file.1));
-        v.push(Value::String(
-            file.0.into_os_string().into_string().unwrap(),
-        ));
-
-        file_list_json.push(Value::Array(v));
-    }
-
-    Ok(Value::Array(file_list_json))
+    let file_list: Vec<_> = file_list
+        .into_iter()
+        .map(|(path, name)| {
+            Value::Array(vec![
+                Value::String(name),
+                Value::String(path.into_os_string().into_string().unwrap()),
+            ])
+        })
+        .collect();
+
+    Ok(Value::Array(file_list))
 }
diff --git a/src/docbuilder/crates.rs b/src/docbuilder/crates.rs
@@ -12,7 +12,7 @@ where
     let reader = fs::File::open(path).map(BufReader::new)?;
 
     let mut name = String::new();
-    let mut versions = Vec::new();
+    let mut versions = Vec::new(); // TODO: Find a way to pre-allocate the versions vector
 
     for line in reader.lines() {
         // some crates have invalid UTF-8 (nanny-sys-0.0.7)
diff --git a/src/docbuilder/limits.rs b/src/docbuilder/limits.rs
@@ -119,6 +119,7 @@ fn scale(value: usize, interval: usize, labels: &[&str]) -> String {
 mod test {
     use super::*;
     use crate::test::*;
+
     #[test]
     fn retrieve_limits() {
         wrapper(|env| {
@@ -151,40 +152,41 @@ mod test {
                 targets: 1,
                 ..Limits::default()
             };
-            db.conn().query("INSERT INTO sandbox_overrides (crate_name, max_memory_bytes, timeout_seconds, max_targets)
-                                    VALUES ($1, $2, $3, $4)",
-                                    &[&krate, &(limits.memory as i64), &(limits.timeout.as_secs() as i32), &(limits.targets as i32)])?;
+            db.conn().query(
+                "INSERT INTO sandbox_overrides (crate_name, max_memory_bytes, timeout_seconds, max_targets)
+                 VALUES ($1, $2, $3, $4)",
+                &[&krate, &(limits.memory as i64), &(limits.timeout.as_secs() as i32), &(limits.targets as i32)]
+            )?;
             assert_eq!(limits, Limits::for_crate(&db.conn(), krate)?);
             Ok(())
         });
     }
+
     #[test]
     fn display_limits() {
         let limits = Limits {
-            memory: 102400,
+            memory: 102_400,
             timeout: Duration::from_secs(300),
             targets: 1,
             ..Limits::default()
         };
         let display = limits.for_website();
+        assert_eq!(display.get("Network access"), Some(&"blocked".into()));
         assert_eq!(
-            display.get("Network access".into()),
-            Some(&"blocked".into())
-        );
-        assert_eq!(
-            display.get("Maximum size of a build log".into()),
+            display.get("Maximum size of a build log"),
             Some(&"100 KB".into())
         );
         assert_eq!(
-            display.get("Maximum number of build targets".into()),
+            display.get("Maximum number of build targets"),
             Some(&limits.targets.to_string())
         );
         assert_eq!(
-            display.get("Maximum rustdoc execution time".into()),
+            display.get("Maximum rustdoc execution time"),
             Some(&"5 minutes".into())
         );
-        assert_eq!(display.get("Available RAM".into()), Some(&"100 KB".into()));
+        assert_eq!(display.get("Available RAM"), Some(&"100 KB".into()));
     }
+
     #[test]
     fn scale_limits() {
         // time
@@ -197,18 +199,18 @@ mod test {
         assert_eq!(SIZE_SCALE(100), "100 bytes");
         assert_eq!(SIZE_SCALE(1024), "1 KB");
         assert_eq!(SIZE_SCALE(10240), "10 KB");
-        assert_eq!(SIZE_SCALE(1048576), "1 MB");
-        assert_eq!(SIZE_SCALE(10485760), "10 MB");
-        assert_eq!(SIZE_SCALE(1073741824), "1 GB");
-        assert_eq!(SIZE_SCALE(10737418240), "10 GB");
+        assert_eq!(SIZE_SCALE(1_048_576), "1 MB");
+        assert_eq!(SIZE_SCALE(10_485_760), "10 MB");
+        assert_eq!(SIZE_SCALE(1_073_741_824), "1 GB");
+        assert_eq!(SIZE_SCALE(10_737_418_240), "10 GB");
         assert_eq!(SIZE_SCALE(std::u32::MAX as usize), "4 GB");
 
         // fractional sizes
         assert_eq!(TIME_SCALE(90), "1.5 minutes");
         assert_eq!(TIME_SCALE(5400), "1.5 hours");
 
-        assert_eq!(SIZE_SCALE(1288490189), "1.2 GB");
-        assert_eq!(SIZE_SCALE(3758096384), "3.5 GB");
-        assert_eq!(SIZE_SCALE(1048051712), "999.5 MB");
+        assert_eq!(SIZE_SCALE(1_288_490_189), "1.2 GB");
+        assert_eq!(SIZE_SCALE(3_758_096_384), "3.5 GB");
+        assert_eq!(SIZE_SCALE(1_048_051_712), "999.5 MB");
     }
 }
diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs
@@ -234,8 +234,8 @@ mod test {
         let metadata = Metadata::from_str(manifest);
 
         assert!(metadata.features.is_some());
-        assert!(metadata.all_features == true);
-        assert!(metadata.no_default_features == true);
+        assert!(metadata.all_features);
+        assert!(metadata.no_default_features);
         assert!(metadata.default_target.is_some());
         assert!(metadata.rustdoc_args.is_some());
 
diff --git a/src/utils/github_updater.rs b/src/utils/github_updater.rs
@@ -185,7 +185,7 @@ mod test {
         assert!(fields.is_ok());
 
         let fields = fields.unwrap();
-        assert!(fields.description != "".to_string());
+        assert!(fields.description != "");
         assert!(fields.stars >= 0);
         assert!(fields.forks >= 0);
         assert!(fields.issues >= 0);
diff --git a/src/utils/pubsubhubbub.rs b/src/utils/pubsubhubbub.rs
@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use reqwest::*;
 
 fn ping_hub(url: &str) -> Result<Response> {
-    let mut params = HashMap::new();
+    let mut params = HashMap::with_capacity(2);
     params.insert("hub.mode", "publish");
     params.insert("hub.url", "https://docs.rs/releases/feed");
     let client = Client::new();
diff --git a/src/web/builds.rs b/src/web/builds.rs
@@ -70,11 +70,7 @@ pub fn build_list_handler(req: &mut Request) -> IronResult<Response> {
     let conn = extension!(req, Pool).get()?;
     let limits = ctry!(Limits::for_crate(&conn, name));
 
-    let mut build_list: Vec<Build> = Vec::new();
-    let mut build_details = None;
-
-    // FIXME: getting builds.output may cause performance issues when release have tons of builds
-    for row in &ctry!(conn.query(
+    let query = ctry!(conn.query(
         "SELECT crates.name,
                 releases.version,
                 releases.description,
@@ -92,7 +88,13 @@ pub fn build_list_handler(req: &mut Request) -> IronResult<Response> {
          WHERE crates.name = $1 AND releases.version = $2
          ORDER BY id DESC",
         &[&name, &version]
-    )) {
+    ));
+
+    let mut build_list: Vec<Build> = Vec::with_capacity(query.len());
+    let mut build_details = None;
+
+    // FIXME: getting builds.output may cause performance issues when release have tons of builds
+    for row in query.iter() {
         let id: i32 = row.get(5);
 
         let build = Build {
diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs
@@ -157,25 +157,31 @@ impl CrateDetails {
 
         // sort versions with semver
         let releases = {
-            let mut versions: Vec<semver::Version> = Vec::new();
             let versions_from_db: Value = rows.get(0).get(17);
 
-            if let Some(vers) = versions_from_db.as_array() {
-                for version in vers {
-                    if let Some(version) = version.as_str() {
-                        if let Ok(sem_ver) = semver::Version::parse(&version) {
-                            versions.push(sem_ver);
+            if let Some(versions_from_db) = versions_from_db.as_array() {
+                let mut versions: Vec<semver::Version> = versions_from_db
+                    .iter()
+                    .filter_map(|version| {
+                        if let Some(version) = version.as_str() {
+                            if let Ok(sem_ver) = semver::Version::parse(&version) {
+                                return Some(sem_ver);
+                            }
                         }
-                    }
-                }
-            }
 
-            versions.sort();
-            versions.reverse();
-            versions
-                .iter()
-                .map(|version| map_to_release(&conn, crate_id, version.to_string()))
-                .collect()
+                        None
+                    })
+                    .collect();
+
+                versions.sort();
+                versions.reverse();
+                versions
+                    .iter()
+                    .map(|version| map_to_release(&conn, crate_id, version.to_string()))
+                    .collect()
+            } else {
+                Vec::new()
+            }
         };
 
         let metadata = MetaData {
@@ -237,30 +243,34 @@ impl CrateDetails {
         }
 
         // get authors
-        for row in &conn
+        let authors = conn
             .query(
                 "SELECT name, slug
                                 FROM authors
                                 INNER JOIN author_rels ON author_rels.aid = authors.id
                                 WHERE rid = $1",
                 &[&release_id],
             )
-            .unwrap()
-        {
+            .unwrap();
+        crate_details.authors.reserve(authors.len());
+
+        for row in authors.iter() {
             crate_details.authors.push((row.get(0), row.get(1)));
         }
 
         // get owners
-        for row in &conn
+        let owners = conn
             .query(
                 "SELECT login, avatar
-                                FROM owners
-                                INNER JOIN owner_rels ON owner_rels.oid = owners.id
-                                WHERE cid = $1",
+             FROM owners
+             INNER JOIN owner_rels ON owner_rels.oid = owners.id
+             WHERE cid = $1",
                 &[&crate_id],
             )
-            .unwrap()
-        {
+            .unwrap();
+        crate_details.owners.reserve(owners.len());
+
+        for row in owners.iter() {
             crate_details.owners.push((row.get(0), row.get(1)));
         }
 
@@ -394,7 +404,7 @@ mod tests {
         expected_last_successful_build: Option<&str>,
     ) -> Result<(), Error> {
         let details = CrateDetails::new(&db.conn(), package, version)
-            .ok_or(failure::err_msg("could not fetch crate details"))?;
+            .ok_or_else(|| failure::err_msg("could not fetch crate details"))?;
 
         assert_eq!(
             details.last_successful_build,
diff --git a/src/web/mod.rs b/src/web/mod.rs
@@ -289,7 +289,7 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option
 
     // we need to sort versions first
     let versions_sem = {
-        let mut versions_sem: Vec<(Version, i32)> = Vec::new();
+        let mut versions_sem: Vec<(Version, i32)> = Vec::with_capacity(versions.len());
 
         for version in versions.iter().filter(|(_, _, yanked)| !yanked) {
             // in theory a crate must always have a semver compatible version, but check result just in case
diff --git a/src/web/releases.rs b/src/web/releases.rs
@@ -140,9 +140,10 @@ pub(crate) fn get_releases(conn: &Connection, page: i64, limit: i64, order: Orde
              LIMIT $1 OFFSET $2"
         }
     };
+    let query = conn.query(&query, &[&limit, &offset]).unwrap();
 
-    let mut packages = Vec::new();
-    for row in &conn.query(&query, &[&limit, &offset]).unwrap() {
+    let mut packages = Vec::with_capacity(query.len());
+    for row in query.iter() {
         let package = Release {
             name: row.get(0),
             version: row.get(1),
@@ -182,10 +183,11 @@ fn get_releases_by_author(
                  WHERE authors.slug = $1
                  ORDER BY crates.github_stars DESC
                  LIMIT $2 OFFSET $3";
+    let query = conn.query(&query, &[&author, &limit, &offset]).unwrap();
 
     let mut author_name = String::new();
-    let mut packages = Vec::new();
-    for row in &conn.query(&query, &[&author, &limit, &offset]).unwrap() {
+    let mut packages = Vec::with_capacity(query.len());
+    for row in query.iter() {
         let package = Release {
             name: row.get(0),
             version: row.get(1),
@@ -227,10 +229,11 @@ fn get_releases_by_owner(
                  WHERE owners.login = $1
                  ORDER BY crates.github_stars DESC
                  LIMIT $2 OFFSET $3";
+    let query = conn.query(&query, &[&author, &limit, &offset]).unwrap();
 
     let mut author_name = String::new();
-    let mut packages = Vec::new();
-    for row in &conn.query(&query, &[&author, &limit, &offset]).unwrap() {
+    let mut packages = Vec::with_capacity(query.len());
+    for row in query.iter() {
         let package = Release {
             name: row.get(0),
             version: row.get(1),
@@ -637,17 +640,18 @@ pub fn activity_handler(req: &mut Request) -> IronResult<Response> {
 
 pub fn build_queue_handler(req: &mut Request) -> IronResult<Response> {
     let conn = extension!(req, Pool).get()?;
-    let mut crates: Vec<(String, String, i32)> = Vec::new();
-    for krate in &conn
+    let query = conn
         .query(
             "SELECT name, version, priority
-                              FROM queue
-                              WHERE attempt < 5
-                              ORDER BY priority ASC, attempt ASC, id ASC",
+             FROM queue
+             WHERE attempt < 5
+             ORDER BY priority ASC, attempt ASC, id ASC",
             &[],
         )
-        .unwrap()
-    {
+        .unwrap();
+
+    let mut crates: Vec<(String, String, i32)> = Vec::with_capacity(query.len());
+    for krate in query.iter() {
         crates.push((
             krate.get("name"),
             krate.get("version"),
diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs
@@ -579,6 +579,7 @@ mod test {
         let data = web.get(path).send()?.text()?;
         println!("{}", data);
         let dom = kuchiki::parse_html().one(data);
+
         if let Some(elem) = dom
             .select("form > ul > li > a.warn")
             .expect("invalid selector")
diff --git a/src/web/sitemap.rs b/src/web/sitemap.rs
diff --git a/src/web/source.rs b/src/web/source.rs
