From 150fa8f3144d1229efe4d2a5cd52c62597e696f7 Mon Sep 17 00:00:00 2001 From: Sam Limbu Date: Thu, 27 Aug 2026 22:24:54 +1000 Subject: [PATCH] SEC-01: make cargo fmt and clippy clean so the CI job can go green The CI workflow added in #5 fails on code that predates it: three files were unformatted and `cargo clippy --all-targets -- -D warnings` reported 26 errors. A red check cannot be made a required status check on the protect-main ruleset, which is what this unblocks. `cargo fmt --all` over three files, and 26 clippy errors resolved: 18 via `cargo clippy --all-targets --fix`, the rest by hand. Two fixes uncovered lints that had been masked (an `unnecessary_unwrap` in info.rs behind the needless borrow on the line above, and two `unnecessary_to_owned` at the call sites of a signature that moved from `&String` to `&str`), so 28 fixes for 26 warnings. Three `#[allow]`s where the only real fix would change a signature or a public API: `result_large_err` on `AppState::new` (the large variant is `supabase::Error`, owned by supabase-lib-rs) and `too_many_arguments` on `log_embed` and on the `embed` slash command. No behaviour change. src/ only; .github/, Dockerfile, .dockerignore, Cargo.toml and Cargo.lock are untouched. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/info.rs | 6 +++--- src/commands/member_info.rs | 21 ++++++++++----------- src/commands/mods_only.rs | 24 +++++++++++++++--------- src/commands/weather.rs | 18 +++++++++--------- src/events.rs | 4 ++-- src/events/interaction_create.rs | 32 ++++++++++++++++---------------- src/events/message.rs | 9 ++++----- src/main.rs | 9 +++++---- 8 files changed, 64 insertions(+), 59 deletions(-) diff --git a/src/commands/info.rs b/src/commands/info.rs index 287c18a..bfec8f3 100644 --- a/src/commands/info.rs +++ b/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)); diff --git a/src/commands/member_info.rs b/src/commands/member_info.rs index 57895c2..6d9ccc2 100644 --- a/src/commands/member_info.rs +++ b/src/commands/member_info.rs @@ -18,7 +18,7 @@ struct DiscordLinkRow { student_id: String, } -async fn member_data(database: &Database, user_id: &String) -> Result, Error> { +async fn member_data(database: &Database, user_id: &str) -> Result, Error> { let rows: Vec = 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 Result, Error> { +async fn recorded_member(database: &Database, user_id: &str) -> Result, Error> { let rows: Vec = 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 diff --git a/src/commands/mods_only.rs b/src/commands/mods_only.rs index 289d56e..015bf93 100644 --- a/src/commands/mods_only.rs +++ b/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, @@ -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, @@ -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 diff --git a/src/commands/weather.rs b/src/commands/weather.rs index 43622bf..8b7dd02 100644 --- a/src/commands/weather.rs +++ b/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) diff --git a/src/events.rs b/src/events.rs index 49002e6..720a303 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1,3 +1,3 @@ -pub mod ready; pub mod interaction_create; -pub mod message; \ No newline at end of file +pub mod message; +pub mod ready; diff --git a/src/events/interaction_create.rs b/src/events/interaction_create.rs index 20f7ca9..1c04f06 100644 --- a/src/events/interaction_create.rs +++ b/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 Result { +async fn member_recorded(data: &Data, user_id: &str) -> Result { let rows: Vec = 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, ) diff --git a/src/events/message.rs b/src/events/message.rs index 47a68a9..2f52edb 100644 --- a/src/events/message.rs +++ b/src/events/message.rs @@ -65,7 +65,7 @@ async fn create_leetcode_thread( fn create_title_from_message(message: impl Into) -> 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(()) } diff --git a/src/main.rs b/src/main.rs index a5b7e67..8091baa 100644 --- a/src/main.rs +++ b/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 { 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?;