Merge pull request #6 from dsec-hub/sec-01-fmt-clippy-clean

SEC-01: make cargo fmt and clippy clean so the CI job can go green

src/commands/info.rs

@@ -151,10 +151,10 @@ pub async fn serverinfo(ctx: Context<'_>) -> Result<(), Error> {
     let server_description = server_description_option.as_deref().unwrap_or("N/A");
 
     // rules channel, if empty N/A
-    let rules_channel = if (&partial_guild.rules_channel_id).is_none() {
-        "N/A"
+    let rules_channel = if let Some(rules_channel_id) = partial_guild.rules_channel_id {
+        &format!("<#{}>", rules_channel_id)
     } else {
-        &format!("<#{}>", &partial_guild.rules_channel_id.unwrap())
+        "N/A"
     };
 
     let embed_footer = CreateEmbedFooter::new(format!("ID: {}", server_id));

src/commands/member_info.rs

@@ -18,7 +18,7 @@ struct DiscordLinkRow {
     student_id: String,
 }
 
-async fn member_data(database: &Database, user_id: &String) -> Result<Option<MemberRow>, Error> {
+async fn member_data(database: &Database, user_id: &str) -> Result<Option<MemberRow>, Error> {
     let rows: Vec<MemberRow> = database
         .from("active_members")
         .select("student_id, full_name, campus, membership_status, end_date")
@@ -30,7 +30,7 @@ async fn member_data(database: &Database, user_id: &String) -> Result<Option<Mem
 }
 
 /// Look up the student id linked to a given discord user, if any.
-async fn recorded_member(database: &Database, user_id: &String) -> Result<Option<String>, Error> {
+async fn recorded_member(database: &Database, user_id: &str) -> Result<Option<String>, Error> {
     let rows: Vec<DiscordLinkRow> = database
         .from("dsec_discord_members")
         .select("student_id")
@@ -66,15 +66,7 @@ pub async fn member_info(ctx: ApplicationContext<'_>) -> Result<(), Error> {
 
     let user_data_option = member_data(&database, &student_id).await?;
 
-    if user_data_option.is_none() {
-        ctx.send(CreateReply::default().embed(
-            CreateEmbed::new().title("Couldn't find info").description(
-                "Your membership may have expired. Contact a club executive to be sure.",
-            ),
-        ))
-        .await?;
-    } else {
-        let user_data = user_data_option.expect("Member not found");
+    if let Some(user_data) = user_data_option {
         let member_name = user_data.full_name;
         let member_campus = user_data.campus;
         let membership_status = user_data.membership_status;
@@ -97,6 +89,13 @@ pub async fn member_info(ctx: ApplicationContext<'_>) -> Result<(), Error> {
                 .ephemeral(true),
         )
         .await?;
+    } else {
+        ctx.send(CreateReply::default().embed(
+            CreateEmbed::new().title("Couldn't find info").description(
+                "Your membership may have expired. Contact a club executive to be sure.",
+            ),
+        ))
+        .await?;
     }
 
     // let student_data = state

src/commands/mods_only.rs

@@ -1,8 +1,11 @@
 use crate::{Context, Error};
 use dotenv::dotenv;
-use poise::{serenity_prelude as serenity, CreateReply};
+use poise::{CreateReply, serenity_prelude as serenity};
 
 /// Send message to logs channel
+// Each argument is one optional embed field; collapsing them into a struct would
+// change this function's signature and every call site, which this PR does not do.
+#[allow(clippy::too_many_arguments)]
 pub async fn log_embed(
     ctx: &serenity::Context,
     title: Option<String>,
@@ -36,10 +39,10 @@ pub async fn log_embed(
     }
 
     // Set color (parse hex color)
-    if let Some(color_str) = colour {
-        if let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16) {
-            embed = embed.color(color_value);
-        }
+    if let Some(color_str) = colour
+        && let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16)
+    {
+        embed = embed.color(color_value);
     }
 
     // Set thumbnail
@@ -77,6 +80,9 @@ pub async fn log_embed(
     slash_command,
     required_permissions = "MANAGE_MESSAGES | MANAGE_THREADS"
 )]
+// These arguments are the slash command's options as Discord presents them;
+// bundling them into a struct would change the command's public interface.
+#[allow(clippy::too_many_arguments)]
 pub async fn embed(
     ctx: Context<'_>,
     #[description = "Title of embed"] title: Option<String>,
@@ -109,10 +115,10 @@ pub async fn embed(
     }
 
     // Set color (parse hex color)
-    if let Some(color_str) = colour {
-        if let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16) {
-            embed = embed.color(color_value);
-        }
+    if let Some(color_str) = colour
+        && let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16)
+    {
+        embed = embed.color(color_value);
     }
 
     // Set thumbnail

src/commands/weather.rs

@@ -26,24 +26,24 @@ pub async fn weather(
     let weather_response = get_weather(location).await?;
     let value: Value = serde_json::from_str(&weather_response)?;
 
-    let location_name = (&value["location"]["name"]).as_str().unwrap();
-    let location_region = (&value["location"]["region"]).as_str().unwrap();
-    let location_country = (&value["location"]["country"]).as_str().unwrap();
+    let location_name = value["location"]["name"].as_str().unwrap();
+    let location_region = value["location"]["region"].as_str().unwrap();
+    let location_country = value["location"]["country"].as_str().unwrap();
 
-    let weather_condition = (&value["current"]["condition"]["text"]).as_str().unwrap();
+    let weather_condition = value["current"]["condition"]["text"].as_str().unwrap();
     let weather_temp = &value["current"]["temp_c"];
     let weather_feels_like = &value["current"]["feelslike_c"];
     let weather_wind_kph = &value["current"]["wind_kph"];
     let weather_humidity = &value["current"]["humidity"];
     let weather_cloud = &value["current"]["cloud"];
 
-    let weather_icon = (&value["current"]["condition"]["icon"]).as_str().unwrap();
+    let weather_icon = value["current"]["condition"]["icon"].as_str().unwrap();
 
     let embed = CreateEmbed::new()
-        .field("Name", format!("{}", location_name), true)
-        .field("Region", format!("{}", location_region), true)
-        .field("Country", format!("{}", location_country), true)
-        .field("Condition", format!("{}", weather_condition), true)
+        .field("Name", location_name.to_string(), true)
+        .field("Region", location_region.to_string(), true)
+        .field("Country", location_country.to_string(), true)
+        .field("Condition", weather_condition.to_string(), true)
         .field("Temperature", format!("{} °C", weather_temp), true)
         .field("Feels like", format!("{} °C", weather_feels_like), true)
         .field("Wind", format!("{} kph", weather_wind_kph), true)

src/events.rs

@@ -1,3 +1,3 @@
-pub mod ready;
 pub mod interaction_create;
 pub mod message;
+pub mod ready;

src/events/interaction_create.rs

@@ -115,7 +115,7 @@ async fn grant_verified_role(
     data: &Data,
     modal_submit: &ModalInteraction,
     discord_member: &Member,
-    student_id: &String,
+    student_id: &str,
     verified_role_id: RoleId,
     via_cache: bool,
 ) -> Result<(), Error> {
@@ -170,7 +170,7 @@ async fn fetch_student(data: &Data, student_id: &str) -> Result<Option<StudentRo
     Ok(student_data.into_iter().next())
 }
 
-async fn member_recorded(data: &Data, user_id: &String) -> Result<bool, Error> {
+async fn member_recorded(data: &Data, user_id: &str) -> Result<bool, Error> {
     let rows: Vec<serde_json::Value> = data
         .state
         .supabase
@@ -210,18 +210,18 @@ async fn handle_verify(
     // already attached to the button interaction. Anything slower than this
     // (a DB query, a member fetch) must NOT run before the modal is shown, or
     // Discord's ~3s acknowledgement window elapses and the click fails.
-    if let Some(member) = &component_interaction.member {
-        if member.roles.contains(&verified_role_id) {
-            component_interaction
-                .create_response(
-                    ctx,
-                    ephemeral_embed(CreateEmbed::new().title("Already Verified ✅").description(
-                        format!("You already have the <@&{}> role!", verified_role_id),
-                    )),
-                )
-                .await?;
-            return Ok(());
-        }
+    if let Some(member) = &component_interaction.member
+        && member.roles.contains(&verified_role_id)
+    {
+        component_interaction
+            .create_response(
+                ctx,
+                ephemeral_embed(CreateEmbed::new().title("Already Verified ✅").description(
+                    format!("You already have the <@&{}> role!", verified_role_id),
+                )),
+            )
+            .await?;
+        return Ok(());
     }
 
     // Respond to the click with the modal immediately.
@@ -247,7 +247,7 @@ async fn handle_verify(
             data,
             &modal_submit,
             &discord_member,
-            &student_id.to_string(),
+            student_id,
             verified_role_id,
             true,
         )
@@ -277,7 +277,7 @@ async fn handle_verify(
             data,
             &modal_submit,
             &discord_member,
-            &student_id.to_string(),
+            student_id,
             verified_role_id,
             false,
         )

src/events/message.rs

@@ -65,7 +65,7 @@ async fn create_leetcode_thread(
         fn create_title_from_message(message: impl Into<String>) -> String {
             let message_string: String = message.into();
 
-            let title = message_string
+            message_string
                 .lines()
                 .next()
                 .map(|line| {
@@ -77,8 +77,7 @@ async fn create_leetcode_thread(
                     &line[start..]
                 })
                 .unwrap_or("")
-                .to_string();
-            title
+                .to_string()
         }
 
         let new_thread =
@@ -95,7 +94,7 @@ pub async fn on_message(
     ctx: &serenity::Context,
     new_message: &serenity::Message,
 ) -> Result<(), Error> {
-    let _ = honeypot(ctx, new_message).await?;
-    let _ = create_leetcode_thread(ctx, new_message).await?;
+    honeypot(ctx, new_message).await?;
+    create_leetcode_thread(ctx, new_message).await?;
     Ok(())
 }

src/main.rs

@@ -22,6 +22,9 @@ pub struct AppState {
 }
 
 impl AppState {
+    // The large `Err` variant is `supabase::Error` from the `supabase-lib-rs` crate;
+    // boxing it would change this public signature rather than shrink their type.
+    #[allow(clippy::result_large_err)]
     pub async fn new() -> supabase::Result<Self> {
         dotenv().ok();
         let supabase_url = std::env::var("SUPABASE_URL").expect("missing SUPABASE_URL");
@@ -42,9 +45,7 @@ impl AppState {
                 None => println!("User not found"),
             },
             Err(err) => {
-                eprintln!(
-                    "Failed to connect/sign in to Supabase, continuing setup anyways: {err}"
-                );
+                eprintln!("Failed to connect/sign in to Supabase, continuing setup anyways: {err}");
             }
         }
 
@@ -66,7 +67,7 @@ async fn event_handler(
             events::ready::on_ready(ctx, data_about_bot).await?;
         }
         serenity::FullEvent::InteractionCreate { interaction } => {
-            events::interaction_create::on_interaction_create(ctx, interaction, &data).await?;
+            events::interaction_create::on_interaction_create(ctx, interaction, data).await?;
         }
         serenity::FullEvent::Message { new_message } => {
             events::message::on_message(ctx, new_message).await?;