Fix a bunch of clippy warnings (#7)

Author: illicitonion

Cargo.toml

@@ -49,3 +49,9 @@ tower-sessions = "0.14.0"
 tracing = "0.1.41"
 tracing-subscriber = "0.3.19"
 uuid = { version = "1.17.0", features = ["serde", "v4"] }
+
+[lints.clippy]
+# foo.clone() feels more clear than *foo for copying.
+clone_on_copy = "allow"
+# Because only idents not expressions are allowed inline, sometimes switching between the two leaves reasonable code Clippy dislikes.
+uninlined_format_args = "allow"

src/auth.rs

@@ -110,7 +110,7 @@ pub async fn handle_google_oauth_callback(
     let mut client = Client::new(
         server_state.config.google_apis_client_id.clone(),
         (*server_state.config.google_apis_client_secret).clone(),
-        String::from(redirect_uri),
+        redirect_uri,
         String::new(),
         String::new(),
     );

src/bin/pr-metadata-validator.rs

@@ -72,13 +72,13 @@ async fn main() {
     }
 }
 
-const COULD_NOT_MATCH_COMMENT: &'static str = r#"Your PR couldn't be matched to an assignment in this module.
+const COULD_NOT_MATCH_COMMENT: &str = r#"Your PR couldn't be matched to an assignment in this module.
 
 Please check its title is in the correct format, and that you only have one PR per assignment.
 
 If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed)."#;
 
-const BAD_TITLE_COMMENT_PREFIX: &'static str = r#"Your PR's title isn't in the expected format.
+const BAD_TITLE_COMMENT_PREFIX: &str = r#"Your PR's title isn't in the expected format.
 
 Please check its title is in the correct format, and update it.
 
@@ -98,11 +98,11 @@ async fn validate_pr(
     pr_number: u64,
 ) -> Result<ValidationResult, Error> {
     let course = course_schedule
-        .with_assignments(&octocrab, github_org_name)
+        .with_assignments(octocrab, github_org_name)
         .await
         .map_err(|err| err.context("Failed to get assignments"))?;
 
-    let module_prs = get_prs(&octocrab, github_org_name, module_name, false)
+    let module_prs = get_prs(octocrab, github_org_name, module_name, false)
         .await
         .map_err(|err| err.context("Failed to get PRs"))?;
     let pr_in_question = module_prs

src/config.rs

@@ -21,8 +21,10 @@ pub struct Config {
     /// Courses being tracked. Keys are things like "itp" or "sdc".
     /// Ideally this would be less hard-coded.
     /// Possible sources of truth for this are:
+    ///
     ///  * GitHub team structure (except that lacks dates)
     ///  * Class Planner API (except that has fiddly auth)
+    ///
     /// We assume the following GitHub team structure:
     ///  ${course}-trainees contains groups of batches of trainees.
     ///  ${course}-mentors is a group of reviewers.
@@ -56,15 +58,13 @@ impl Config {
         batch: &str,
     ) -> Option<CourseScheduleWithRegisterSheetId> {
         if let Some(course_info) = self.courses.get(&course_name) {
-            if let Some(course_schedule) = course_info.batches.get(batch) {
-                Some(CourseScheduleWithRegisterSheetId {
+            course_info.batches.get(batch).map(|course_schedule| {
+                CourseScheduleWithRegisterSheetId {
                     name: course_name,
                     course_schedule: course_schedule.clone(),
                     register_sheet_id: course_info.register_sheet_id.clone(),
-                })
-            } else {
-                None
-            }
+                }
+            })
         } else {
             None
         }

src/course.rs

@@ -46,7 +46,7 @@ impl CourseScheduleWithRegisterSheetId {
                 module_name.clone(),
                 Module {
                     sprints: module_sprint_dates
-                        .into_iter()
+                        .iter()
                         .map(|class_dates| Sprint {
                             assignments: vec![Assignment::Attendance {
                                 class_dates: class_dates.clone(),
@@ -108,7 +108,7 @@ impl CourseScheduleWithRegisterSheetId {
         .await
         .map_err(|err| err.context("Failed to fetch module issues"))?;
 
-        issues.sort_by_cached_key(|&Issue { ref title, .. }| title.clone());
+        issues.sort_by_cached_key(|Issue { title, .. }| title.clone());
 
         for issue in issues {
             if let Some((sprint_number, assignment)) = parse_issue(&issue)? {
@@ -201,7 +201,7 @@ fn parse_issue(issue: &Issue) -> Result<Option<(NonZeroUsize, Option<Assignment>
     })?;
     // TODO
     // let assignment = assignment.ok_or_else(|| Error::UserFacing(format!("Failed to parse issue {} - no submit label", html_url)))?;
-    let assignment = assignment.or_else(|| Some(None)).unwrap();
+    let assignment = assignment.unwrap_or(None);
     Ok(Some((sprint, assignment)))
 }
 
@@ -303,7 +303,7 @@ impl Batch {
             let count = region_counts
                 .entry(trainee.trainee.region.clone())
                 .or_default();
-            *count = *count + 1;
+            *count += 1;
         }
         let mut region_counts = region_counts.into_iter().collect::<Vec<_>>();
         region_counts.sort_by_key(|(_region, count)| *count);
@@ -485,10 +485,10 @@ pub enum Attendance {
 impl Attendance {
     pub fn register_url(&self) -> &str {
         match self {
-            Attendance::Absent { register_url } => &register_url,
-            Attendance::OnTime { register_url } => &register_url,
-            Attendance::Late { register_url } => &register_url,
-            Attendance::WrongDay { register_url } => &register_url,
+            Attendance::Absent { register_url } => register_url,
+            Attendance::OnTime { register_url } => register_url,
+            Attendance::Late { register_url } => register_url,
+            Attendance::WrongDay { register_url } => register_url,
         }
     }
 }
@@ -506,7 +506,7 @@ pub(crate) async fn fetch_batch_metadata(
     let teams = all_pages("teams", octocrab, async || {
         octocrab
             .teams(github_org)
-            .list_children(&format!("{}-trainees", course_name))
+            .list_children(format!("{}-trainees", course_name))
             .send()
             .await
     })
@@ -642,11 +642,11 @@ pub async fn get_batch_with_submissions(
                 &register_info,
                 module_name,
                 trainee_email.clone(),
-                &course,
+                course,
                 &region,
             )?;
             let module_with_submissions = match_prs_to_assignments(
-                &module,
+                module,
                 module_to_prs[&module_name].clone(),
                 module_attendance,
                 &region,
@@ -793,6 +793,8 @@ pub fn match_prs_to_assignments(
         }
     }
 
+    let number_regex = Regex::new(r"(\d+)").unwrap();
+
     let mut unknown_prs = Vec::new();
     for pr in prs {
         let title_lower = pr.title.to_lowercase();
@@ -803,7 +805,6 @@ pub fn match_prs_to_assignments(
         let mut sprint_index = None;
         for title_part in title_parts {
             if title_part.starts_with("sprint") || title_part.starts_with("week") {
-                let number_regex = Regex::new(r"(\d+)").unwrap();
                 if let Some(number_match) = number_regex
                     .captures(title_part)
                     .and_then(|captures| captures.get(1))
@@ -837,8 +838,8 @@ pub fn match_prs_to_assignments(
 fn match_pr_to_assignment(
     pr: Pr,
     claimed_sprint_index: Option<usize>,
-    assignments: &Vec<Sprint>,
-    submissions: &mut Vec<SprintWithSubmissions>,
+    assignments: &[Sprint],
+    submissions: &mut [SprintWithSubmissions],
     unknown_prs: &mut Vec<Pr>,
 ) {
     #[derive(Clone, Copy)]
@@ -902,10 +903,8 @@ fn match_pr_to_assignment(
     {
         submissions[sprint_index].submissions[assignment_index] =
             SubmissionState::Some(Submission::PullRequest { pull_request: pr });
-    } else {
-        if !pr.is_closed {
-            unknown_prs.push(pr);
-        }
+    } else if !pr.is_closed {
+        unknown_prs.push(pr);
     }
 }
 

src/frontend.rs

@@ -38,7 +38,7 @@ pub async fn list_courses(
     let batch_metadata = join_all(
         courses
             .keys()
-            .map(|course_name| fetch_batch_metadata(&octocrab, github_org.clone(), &course_name)),
+            .map(|course_name| fetch_batch_metadata(&octocrab, github_org.clone(), course_name)),
     )
     .await
     .into_iter()

src/google_groups.rs

@@ -104,12 +104,12 @@ pub(crate) async fn get_groups(client: &Client) -> Result<GoogleGroups, Error> {
                 members: members
                     .into_iter()
                     .map(|Member { email, .. }| {
-                        Ok(EmailAddress::from_str(&email).with_context(|| {
+                        EmailAddress::from_str(&email).with_context(|| {
                             format!(
                                 "Failed to parse group member email address {} (member of {})",
                                 email, group.email
                             )
-                        })?)
+                        })
                     })
                     .collect::<Result<_, anyhow::Error>>()?,
             })

src/register.rs

@@ -126,7 +126,7 @@ fn read_module(
             let cells = row.values;
             // Some sheets have documentation or pivot table
             if row_number == 0
-                && cells.len() >= 1
+                && !cells.is_empty()
                 && cell_string(&cells[0]).unwrap_or_default() != "Name"
             {
                 continue 'sheet;
@@ -150,7 +150,7 @@ fn read_module(
                     })
                     .collect::<Result<Vec<_>, _>>()?;
                 if headings
-                    != &[
+                    != [
                         "Name",
                         "Email",
                         "Timestamp",

src/reviewer_staff_info.rs

@@ -75,7 +75,7 @@ fn reviewer_staff_detail_from_sheet(
             );
 
             let notes = match cells.get(6) {
-                Some(cell) => cell_string(&cell).context("Failed to read reviewer notes")?,
+                Some(cell) => cell_string(cell).context("Failed to read reviewer notes")?,
                 None => String::new(),
             };
 

src/sheets.rs

@@ -90,7 +90,7 @@ impl SheetsClient {
                     make_redirect_uri(
                         &self.server_state,
                         self.original_uri,
-                        &&redirect_endpoint(&self.server_state),
+                        &redirect_endpoint(&self.server_state),
                         GoogleScope::Sheets,
                     )
                     .await?,