Address clippy findings

.github/workflows/ci.yml

@@ -68,6 +68,10 @@ jobs:
     name: Backend (linting)
     runs-on: ubuntu-20.04
 
+    env:
+      CARGO_INCREMENTAL: 0
+      RUSTFLAGS: "-D warnings"
+
     steps:
       - uses: actions/checkout@v2
 
@@ -98,8 +102,9 @@ jobs:
       - name: Install Rust
         run: |
           rustup set profile minimal
-          rustup update stable
-          rustup default stable
+          # Pin to older version until a clippy regression is fixed
+          rustup update 1.52.1
+          rustup default 1.52.1
 
       - run: rustup component add rustfmt
       - run: rustup component add clippy

src/admin/migrate.rs

@@ -1,6 +1,6 @@
 use anyhow::Error;
 
-static CATEGORIES_TOML: &'static str = include_str!("../boot/categories.toml");
+static CATEGORIES_TOML: &str = include_str!("../boot/categories.toml");
 diesel_migrations::embed_migrations!("./migrations");
 
 #[derive(clap::Clap, Debug, Copy, Clone)]

src/app.rs

@@ -141,7 +141,7 @@ impl App {
                     .connection_customizer(Box::new(replica_db_connection_config))
                     .thread_pool(thread_pool);
 
-                Some(DieselPool::new(&url, replica_db_config).unwrap())
+                Some(DieselPool::new(url, replica_db_config).unwrap())
             }
         } else {
             None

src/bin/crates-admin.rs

@@ -29,7 +29,7 @@ enum SubCommand {
 fn main() -> anyhow::Result<()> {
     let opts: Opts = Opts::parse();
 
-    Ok(match opts.command {
+    match opts.command {
         SubCommand::DeleteCrate(opts) => delete_crate::run(opts),
         SubCommand::DeleteVersion(opts) => delete_version::run(opts),
         SubCommand::Populate(opts) => populate::run(opts),
@@ -38,5 +38,7 @@ fn main() -> anyhow::Result<()> {
         SubCommand::TransferCrates(opts) => transfer_crates::run(opts),
         SubCommand::VerifyToken(opts) => verify_token::run(opts).unwrap(),
         SubCommand::Migrate(opts) => migrate::run(opts)?,
-    })
+    }
+
+    Ok(())
 }

src/controllers/crate_owner_invitation.rs

@@ -61,7 +61,7 @@ pub fn list(req: &mut dyn RequestExt) -> EndpointResult {
                 .cloned()
                 .unwrap_or_else(|| String::from("(unknown crate name)"));
 
-            let expires_at = invitation.expires_at(&config);
+            let expires_at = invitation.expires_at(config);
             EncodableCrateOwnerInvitation::from(invitation, inviter_name, crate_name, expires_at)
         })
         .collect();
@@ -101,11 +101,11 @@ pub fn handle_invite(req: &mut dyn RequestExt) -> EndpointResult {
     let conn = &*req.db_conn()?;
     let config = &req.app().config;
 
-    let invitation = CrateOwnerInvitation::find_by_id(user_id, crate_invite.crate_id, &conn)?;
+    let invitation = CrateOwnerInvitation::find_by_id(user_id, crate_invite.crate_id, conn)?;
     if crate_invite.accepted {
-        invitation.accept(&conn, config)?;
+        invitation.accept(conn, config)?;
     } else {
-        invitation.decline(&conn)?;
+        invitation.decline(conn)?;
     }
 
     #[derive(Serialize)]

src/controllers/krate/search.rs

@@ -73,7 +73,7 @@ pub fn search(req: &mut dyn RequestExt) -> EndpointResult {
             query = query.filter(
                 q.clone()
                     .matches(crates::textsearchable_index_col)
-                    .or(Crate::loosly_matches_name(&q_string)),
+                    .or(Crate::loosly_matches_name(q_string)),
             );
 
             query = query.select((

src/controllers/user/session.rs

@@ -103,12 +103,7 @@ pub fn authorize(req: &mut dyn RequestExt) -> EndpointResult {
 
     // Fetch the user info from GitHub using the access token we just got and create a user record
     let ghuser = req.app().github.current_user(token)?;
-    let user = save_user_to_database(
-        &ghuser,
-        &token.secret(),
-        &req.app().emails,
-        &*req.db_conn()?,
-    )?;
+    let user = save_user_to_database(&ghuser, token.secret(), &req.app().emails, &*req.db_conn()?)?;
 
     // Log in by setting a cookie and the middleware authentication
     req.session_mut()

src/controllers/util.rs

@@ -127,7 +127,7 @@ impl<'a> UserAuthenticationExt for dyn RequestExt + 'a {
             };
             if still_locked {
                 return Err(account_locked(
-                    &reason,
+                    reason,
                     authenticated_user.user.account_lock_until,
                 ));
             }

src/controllers/version.rs

@@ -13,7 +13,7 @@ fn version_and_crate(
     semver: &str,
 ) -> AppResult<(Version, Crate)> {
     let krate: Crate = Crate::by_name(crate_name).first(conn)?;
-    let version = krate.find_version(&conn, semver)?;
+    let version = krate.find_version(conn, semver)?;
 
     Ok((version, krate))
 }

src/downloads_counter.rs

@@ -78,7 +78,7 @@ impl DownloadsCounter {
         let mut stats = PersistStats::default();
         for shard in self.inner.shards() {
             let shard = std::mem::take(&mut *shard.write());
-            stats = stats.merge(self.persist_shard(&conn, shard)?);
+            stats = stats.merge(self.persist_shard(conn, shard)?);
         }
 
         Ok(stats)
@@ -92,7 +92,7 @@ impl DownloadsCounter {
         let idx = self.shard_idx.fetch_add(1, Ordering::SeqCst) % shards.len();
         let shard = std::mem::take(&mut *shards[idx].write());
 
-        let mut stats = self.persist_shard(&conn, shard)?;
+        let mut stats = self.persist_shard(conn, shard)?;
         stats.shard = Some(idx);
         Ok(stats)
     }

src/email.rs

@@ -109,7 +109,7 @@ or go to https://{domain}/me/pending-invites to manage all of your crate ownersh
                 login,
                 password,
             } => {
-                SmtpTransport::relay(&server)?
+                SmtpTransport::relay(server)?
                     .credentials(Credentials::new(login.clone(), password.clone()))
                     .authentication(vec![Mechanism::Plain])
                     .build()

src/git.rs

@@ -203,7 +203,7 @@ impl Repository {
         let parent = self.repository.find_commit(head)?;
         let sig = self.repository.signature()?;
         self.repository
-            .commit(Some("HEAD"), &sig, &sig, &msg, &tree, &[&parent])?;
+            .commit(Some("HEAD"), &sig, &sig, msg, &tree, &[&parent])?;
 
         self.push()
     }

src/metrics/log_encoder.rs

@@ -27,6 +27,12 @@ const CHUNKS_MAX_SIZE_BYTES: usize = 5000;
 #[derive(Debug, Clone, Copy)]
 pub struct LogEncoder(());
 
+impl Default for LogEncoder {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl LogEncoder {
     pub fn new() -> Self {
         Self(())
@@ -132,6 +138,7 @@ fn serialize_and_split_list<'a, S: Serialize + 'a>(
         let mut serializer = Serializer::new(EncoderWriter::new(&mut writer, base64::STANDARD));
 
         let mut seq = serializer.serialize_seq(None)?;
+        #[allow(clippy::while_let_on_iterator)]
         while let Some(next) = items.next() {
             seq.serialize_element(next)?;
             if written_count.get() >= max_size_hint {
@@ -363,7 +370,7 @@ mod tests {
     fn test_encoding() -> Result<(), Error> {
         let gauge = IntGauge::with_opts(Opts::new("sample_gauge", "sample_gauge help message"))?;
         let registry = Registry::new();
-        registry.register(Box::new(gauge.clone()))?;
+        registry.register(Box::new(gauge))?;
 
         let mut output = Vec::new();
         LogEncoder::new().encode(&registry.gather(), &mut output)?;

src/models/krate.rs

@@ -336,7 +336,7 @@ impl Crate {
                 let config = &app.config;
                 match CrateOwnerInvitation::create(user.id, req_user.id, self.id, conn, config)? {
                     NewCrateOwnerInvitationOutcome::InviteCreated { plaintext_token } => {
-                        if let Ok(Some(email)) = user.verified_email(&conn) {
+                        if let Ok(Some(email)) = user.verified_email(conn) {
                             // Swallow any error. Whether or not the email is sent, the invitation
                             // entry will be created in the database and the user will see the
                             // invitation when they visit https://crates.io/me/pending-invites/.

src/models/version.rs

@@ -72,13 +72,13 @@ impl TopVersions {
             .collect();
 
         let newest = pairs.iter().max().map(|(_, v)| v.clone());
-        let highest = pairs.iter().map(|(_, v)| v).max().map(|v| v.clone());
+        let highest = pairs.iter().map(|(_, v)| v).max().cloned();
         let highest_stable = pairs
             .iter()
             .map(|(_, v)| v)
             .filter(|v| v.pre.is_empty())
             .max()
-            .map(|v| v.clone());
+            .cloned();
 
         Self {
             highest,

src/render.rs

@@ -276,7 +276,7 @@ pub fn render_and_upload_readme(
     let rendered = readme_to_html(&text, &file_name, base_url.as_deref());
 
     conn.transaction(|| {
-        Version::record_readme_rendering(version_id, &conn)?;
+        Version::record_readme_rendering(version_id, conn)?;
         let (crate_name, vers): (String, String) = versions::table
             .find(version_id)
             .inner_join(crates::table)

src/s3/lib.rs

@@ -46,7 +46,7 @@ impl Bucket {
         content_type: &str,
         extra_headers: header::HeaderMap,
     ) -> Result<Response, Error> {
-        let path = path.strip_prefix("/").unwrap_or(path);
+        let path = path.strip_prefix('/').unwrap_or(path);
         let date = Utc::now().to_rfc2822();
         let auth = self.auth("PUT", &date, path, "", content_type);
         let url = self.url(path);
@@ -66,7 +66,7 @@ impl Bucket {
     }
 
     pub fn delete(&self, client: &Client, path: &str) -> Result<Response, Error> {
-        let path = path.strip_prefix("/").unwrap_or(path);
+        let path = path.strip_prefix('/').unwrap_or(path);
         let date = Utc::now().to_rfc2822();
         let auth = self.auth("DELETE", &date, path, "", "");
         let url = self.url(path);

src/tasks/update_downloads.rs

@@ -8,7 +8,7 @@ use swirl::PerformError;
 
 #[swirl::background_job]
 pub fn update_downloads(conn: &PgConnection) -> Result<(), PerformError> {
-    update(&conn)?;
+    update(conn)?;
     Ok(())
 }
 

src/tests/category.rs

@@ -75,7 +75,7 @@ fn update_crate() {
         assert_ok!(
             new_category("Category 2", "category-2", "Category 2 crates").create_or_update(conn)
         );
-        let krate = CrateBuilder::new("foo_crate", user.id).expect_build(&conn);
+        let krate = CrateBuilder::new("foo_crate", user.id).expect_build(conn);
 
         // Updating with no categories has no effect
         Category::update_crate(conn, &krate, &[]).unwrap();
@@ -127,7 +127,7 @@ fn update_crate() {
 
         // Add a category and its subcategory
         assert_ok!(new_category("cat1::bar", "cat1::bar", "bar crates").create_or_update(conn));
-        Category::update_crate(&conn, &krate, &["cat1", "cat1::bar"]).unwrap();
+        Category::update_crate(conn, &krate, &["cat1", "cat1::bar"]).unwrap();
 
         assert_eq!(count(&anon, "cat1"), 1);
         assert_eq!(count(&anon, "cat1::bar"), 1);

src/tests/krate/publish.rs

@@ -640,7 +640,7 @@ fn publish_records_an_audit_action() {
 
     let (app, anon, _, token) = TestApp::full().with_token();
 
-    app.db(|conn| assert!(VersionOwnerAction::all(&conn).unwrap().is_empty()));
+    app.db(|conn| assert!(VersionOwnerAction::all(conn).unwrap().is_empty()));
 
     // Upload a new crate, putting it in the git index
     let crate_to_publish = PublishBuilder::new("fyk");

src/tests/krate/yanking.rs

@@ -40,6 +40,7 @@ impl crate::util::MockCookieUser {
 }
 
 #[test]
+#[allow(unknown_lints, clippy::bool_assert_comparison)] // for claim::assert_some_eq! with bool
 fn yank_works_as_intended() {
     let (app, anon, cookie, token) = TestApp::full().with_token();
 

src/tests/owners.rs

@@ -215,7 +215,7 @@ fn modify_multiple_owners() {
         response.json(),
         json!({ "errors": [{ "detail": "cannot remove all individual owners of a crate. Team member don't have permission to modify owners, so at least one individual owner is required." }] })
     );
-    assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 3);
+    assert_eq!(app.db(|conn| krate.owners(conn).unwrap()).len(), 3);
 
     // Deleting two owners at once is allowed.
     let response = token.remove_named_owners("owners_multiple", &["user2", "user3"]);
@@ -224,7 +224,7 @@ fn modify_multiple_owners() {
         response.json(),
         json!({ "msg": "owners successfully removed", "ok": true })
     );
-    assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 1);
+    assert_eq!(app.db(|conn| krate.owners(conn).unwrap()).len(), 1);
 
     // Adding multiple users fails if one of them already is an owner.
     let response = token.add_named_owners("owners_multiple", &["user2", username]);
@@ -233,7 +233,7 @@ fn modify_multiple_owners() {
         response.json(),
         json!({ "errors": [{ "detail": "`foo` is already an owner" }] })
     );
-    assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 1);
+    assert_eq!(app.db(|conn| krate.owners(conn).unwrap()).len(), 1);
 
     // Adding multiple users at once succeeds.
     let response = token.add_named_owners("owners_multiple", &["user2", "user3"]);
@@ -249,7 +249,7 @@ fn modify_multiple_owners() {
     user2.accept_ownership_invitation(&krate.name, krate.id);
     user3.accept_ownership_invitation(&krate.name, krate.id);
 
-    assert_eq!(app.db(|conn| krate.owners(&conn).unwrap()).len(), 3);
+    assert_eq!(app.db(|conn| krate.owners(conn).unwrap()).len(), 3);
 }
 
 #[test]
@@ -449,7 +449,7 @@ fn invitations_list() {
         InvitationListResponse {
             crate_owner_invitations: vec![EncodableCrateOwnerInvitation {
                 crate_id: krate.id,
-                crate_name: krate.name.clone(),
+                crate_name: krate.name,
                 invited_by_username: owner.gh_login.clone(),
                 invitee_id: user.as_model().id,
                 inviter_id: owner.id,
@@ -484,7 +484,7 @@ fn invitations_list_does_not_include_expired_invites() {
         InvitationListResponse {
             crate_owner_invitations: vec![EncodableCrateOwnerInvitation {
                 crate_id: krate2.id,
-                crate_name: krate2.name.clone(),
+                crate_name: krate2.name,
                 invited_by_username: owner.gh_login.clone(),
                 invitee_id: user.as_model().id,
                 inviter_id: owner.id,

src/tests/server_binary.rs

@@ -92,10 +92,7 @@ impl ServerBin {
         env.remove("DB_MIN_SIZE");
         // Other configuration variables needed for the application to boot.
         env.insert("WEB_ALLOWED_ORIGINS".into(), "http://localhost:8888".into());
-        env.insert(
-            "SESSION_KEY".into(),
-            std::iter::repeat('a').take(32).collect(),
-        );
+        env.insert("SESSION_KEY".into(), "a".repeat(32));
         env.insert("GH_CLIENT_ID".into(), String::new());
         env.insert("GH_CLIENT_SECRET".into(), String::new());
 

src/tests/token.rs

@@ -97,11 +97,10 @@ fn list_tokens_exclude_revoked() {
     // Check that we now have one less token being listed.
     let json: ListResponse = user.get(URL).good();
     assert_eq!(json.api_tokens.len(), tokens.len() - 1);
-    assert!(json
+    assert!(!json
         .api_tokens
         .iter()
-        .find(|token| token.name == tokens[0].model.name)
-        .is_none());
+        .any(|token| token.name == tokens[0].model.name));
 }
 
 #[test]
@@ -175,7 +174,7 @@ fn create_token_success() {
         app.db(|conn| assert_ok!(ApiToken::belonging_to(user.as_model()).load(conn)));
     assert_eq!(tokens.len(), 1);
     assert_eq!(tokens[0].name, "bar");
-    assert_eq!(tokens[0].revoked, false);
+    assert!(!tokens[0].revoked);
     assert_eq!(tokens[0].last_used_at, None);
 }