From c942d73c66967c013a6bc9338d6a2958e9e6f160 Mon Sep 17 00:00:00 2001 From: Jacob Hagstedt Date: Sun, 18 Oct 2026 22:33:31 +0200 Subject: [PATCH] Server overhaul: clear clippy::pedantic wins Taken from a -W clippy::pedantic scan, no allow added: map_or/map_or_else, method references instead of closures, clone_from, write! instead of push_str(format!), borrowed Option<&str>, push_of without a needless Option, the envelope's entity moved instead of cloned, the scheduler's plan() no longer a needless async fn (its doc now says what it does on failure), sort_unstable, digit separators and inline format args in tests. Not taken: missing # Errors/# Panics docs, must_use, doc backticks, assert!(x.is_empty()) in tests, too_many_lines. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TQd3pEP9UeN2uifbHknQ7B --- server/crates/kvitt-import/src/categories.rs | 2 +- server/crates/kvitt-import/src/plan.rs | 11 +++++------ server/crates/kvitt-import/tests/import.rs | 2 +- server/crates/kvitt-server/src/api/landing.rs | 7 +++++-- server/crates/kvitt-server/src/api/rows.rs | 6 +++--- server/crates/kvitt-server/src/auth.rs | 2 +- server/crates/kvitt-server/src/openapi.rs | 3 ++- server/crates/kvitt-server/src/push/fcm.rs | 2 +- server/crates/kvitt-server/src/router.rs | 6 ++---- server/crates/kvitt-server/src/sync/push.rs | 19 +++++++++++-------- .../kvitt-server/src/tasks/scheduler.rs | 10 +++++----- .../kvitt-server/tests/core_api/generation.rs | 14 ++++++++++++-- .../kvitt-server/tests/core_api/logging.rs | 2 +- .../kvitt-server/tests/core_api/rate_limit.rs | 2 +- .../kvitt-server/tests/openapi_contract.rs | 4 ++-- .../tests/skeleton/wire_examples.rs | 1 - .../kvitt-server/tests/sync_api/audit.rs | 2 +- .../kvitt-server/tests/sync_api/pull.rs | 4 ++-- 18 files changed, 56 insertions(+), 43 deletions(-) diff --git a/server/crates/kvitt-import/src/categories.rs b/server/crates/kvitt-import/src/categories.rs index 97c4397..864f611 100644 --- a/server/crates/kvitt-import/src/categories.rs +++ b/server/crates/kvitt-import/src/categories.rs @@ -75,7 +75,7 @@ impl CategoryTable { else { return Err(format!( "category table: line {} must have three fields", - record.position().map_or(0, |p| p.line()) + record.position().map_or(0, csv::Position::line) )); }; if name.trim().is_empty() || name.trim() != name { diff --git a/server/crates/kvitt-import/src/plan.rs b/server/crates/kvitt-import/src/plan.rs index 83b3ac5..f6658ae 100644 --- a/server/crates/kvitt-import/src/plan.rs +++ b/server/crates/kvitt-import/src/plan.rs @@ -249,12 +249,11 @@ fn plan_row( } let category = row.category.trim(); - let category_key = match ctx.categories.lookup(category) { - Some(key) => key.to_owned(), - None => { - *unmapped.entry(category.to_owned()).or_insert(0) += 1; - FALLBACK_KEY.to_owned() - } + let category_key = if let Some(key) = ctx.categories.lookup(category) { + key.to_owned() + } else { + *unmapped.entry(category.to_owned()).or_insert(0) += 1; + FALLBACK_KEY.to_owned() }; if description.is_empty() { return Err("empty description".into()); diff --git a/server/crates/kvitt-import/tests/import.rs b/server/crates/kvitt-import/tests/import.rs index f543815..c842aca 100644 --- a/server/crates/kvitt-import/tests/import.rs +++ b/server/crates/kvitt-import/tests/import.rs @@ -549,7 +549,7 @@ async fn header_failures_write_nothing() { for file in ["bad_header.csv", "wrong_member.csv"] { match db.import(&db.options(file, "Shared")).await { Err(e @ ImportError::Header(_)) => { - assert_eq!(e.exit_code(), kvitt_import::EXIT_INVALID) + assert_eq!(e.exit_code(), kvitt_import::EXIT_INVALID); } other => panic!("{file}: {other:?}"), } diff --git a/server/crates/kvitt-server/src/api/landing.rs b/server/crates/kvitt-server/src/api/landing.rs index 9fc79ae..a6105f6 100644 --- a/server/crates/kvitt-server/src/api/landing.rs +++ b/server/crates/kvitt-server/src/api/landing.rs @@ -6,6 +6,8 @@ //! `from` query parameter (the inviting admin's display name, set by E-12's link) is only passed on into the //! `kvitt://invite` link, never shown. Any code of 1 to 32 characters gets 200; anything else a 404 page. +use std::fmt::Write as _; + use axum::extract::{Path, RawQuery, State}; use axum::http::{HeaderMap, HeaderValue, StatusCode, header}; use axum::response::{IntoResponse, Response}; @@ -153,10 +155,11 @@ pub fn render( utf8_percent_encode(&code, NON_ALPHANUMERIC) ); if let Some(name) = from { - open_url.push_str(&format!( + let _ = write!( + open_url, "&{FROM_PARAM}={}", utf8_percent_encode(name, NON_ALPHANUMERIC) - )); + ); } let download = download_url .map(|url| { diff --git a/server/crates/kvitt-server/src/api/rows.rs b/server/crates/kvitt-server/src/api/rows.rs index f7ca378..6522534 100644 --- a/server/crates/kvitt-server/src/api/rows.rs +++ b/server/crates/kvitt-server/src/api/rows.rs @@ -22,8 +22,8 @@ pub fn uuid(text: &str) -> Result { Uuid::parse_str(text).map_err(|e| AppError::internal(format!("stored id is not a uuid: {e}"))) } -fn opt_uuid(text: Option) -> Result, AppError> { - text.as_deref().map(uuid).transpose() +fn opt_uuid(text: Option<&str>) -> Result, AppError> { + text.map(uuid).transpose() } /// Parses a stored snake_case enum token through its serde form. @@ -504,7 +504,7 @@ pub async fn expense(conn: &mut SqliteConnection, id: Uuid) -> Result>()?, - recurring_template_id: opt_uuid(row.recurring_template_id)?, + recurring_template_id: opt_uuid(row.recurring_template_id.as_deref())?, recurring_period: row.recurring_period, created_by_user_id: uuid(&row.created_by_user_id)?, updated_by_user_id: uuid(&row.updated_by_user_id)?, diff --git a/server/crates/kvitt-server/src/auth.rs b/server/crates/kvitt-server/src/auth.rs index 908d099..c2a4bd8 100644 --- a/server/crates/kvitt-server/src/auth.rs +++ b/server/crates/kvitt-server/src/auth.rs @@ -46,7 +46,7 @@ const TOUCH_INTERVAL: Duration = Duration::hours(1); pub fn sha256_hex(text: &str) -> String { let digest = Sha256::digest(text.as_bytes()); let mut hex = String::with_capacity(64); - for byte in digest.iter() { + for byte in &digest { let _ = write!(hex, "{byte:02x}"); } hex diff --git a/server/crates/kvitt-server/src/openapi.rs b/server/crates/kvitt-server/src/openapi.rs index 2bf193d..966ab4d 100644 --- a/server/crates/kvitt-server/src/openapi.rs +++ b/server/crates/kvitt-server/src/openapi.rs @@ -331,7 +331,8 @@ impl Modify for CommonRules { fn is_unauthenticated(operation: &Operation) -> bool { operation.security.as_ref().is_some_and(|requirements| { requirements.iter().all(|r| { - serde_json::to_value(r).is_ok_and(|v| v.as_object().is_some_and(|o| o.is_empty())) + serde_json::to_value(r) + .is_ok_and(|v| v.as_object().is_some_and(serde_json::Map::is_empty)) }) }) } diff --git a/server/crates/kvitt-server/src/push/fcm.rs b/server/crates/kvitt-server/src/push/fcm.rs index d804002..4fc9e04 100644 --- a/server/crates/kvitt-server/src/push/fcm.rs +++ b/server/crates/kvitt-server/src/push/fcm.rs @@ -214,7 +214,7 @@ impl FcmClient { fn assertion(&self, now: DateTime) -> Result { let mut header = Header::new(Algorithm::RS256); - header.kid = self.account.private_key_id.clone(); + header.kid.clone_from(&self.account.private_key_id); let claims = Claims { iss: &self.account.client_email, scope: SCOPE, diff --git a/server/crates/kvitt-server/src/router.rs b/server/crates/kvitt-server/src/router.rs index 984040f..f106363 100644 --- a/server/crates/kvitt-server/src/router.rs +++ b/server/crates/kvitt-server/src/router.rs @@ -136,8 +136,7 @@ async fn log_request(request: Request, next: Next) -> Response { let route = request .extensions() .get::() - .map(|p| p.as_str().to_owned()) - .unwrap_or_else(|| "".to_owned()); + .map_or_else(|| "".to_owned(), |p| p.as_str().to_owned()); let span = tracing::info_span!( "request", method = %method, @@ -151,8 +150,7 @@ async fn log_request(request: Request, next: Next) -> Response { let error_code = response .extensions() .get::() - .map(|c| c.0.as_str()) - .unwrap_or(""); + .map_or("", |c| c.0.as_str()); span.in_scope(|| { tracing::info!(status, latency_ms, error_code, "request"); }); diff --git a/server/crates/kvitt-server/src/sync/push.rs b/server/crates/kvitt-server/src/sync/push.rs index 8a5c0fd..1aa3efa 100644 --- a/server/crates/kvitt-server/src/sync/push.rs +++ b/server/crates/kvitt-server/src/sync/push.rs @@ -57,7 +57,7 @@ fn canonical_uuid(value: Option<&Value>) -> Option { .filter(|id| id.hyphenated().to_string() == text) } -fn envelope(index: usize, raw: Value) -> Result { +fn envelope(index: usize, mut raw: Value) -> Result { let bad = |what: &str| { AppError::invalid_request(format!("mutations[{index}]: {what}")).with_field("mutations") }; @@ -68,7 +68,10 @@ fn envelope(index: usize, raw: Value) -> Result { Some("settlement") => EntityType::Settlement, _ => return Err(bad("entity_type must be expense or settlement")), }; - let entity = raw.get("entity").cloned().unwrap_or(Value::Null); + let entity = raw + .get_mut("entity") + .map(Value::take) + .unwrap_or(Value::Null); let entity_id = canonical_uuid(entity.get("id")) .ok_or_else(|| bad("entity.id must be a lowercase hyphenated uuid"))?; Ok(Envelope { @@ -410,13 +413,13 @@ fn rejected(env: &Envelope, rejection: Rejection, current: Option) - }) } -fn push_of(push_type: PushType, env: &Envelope, ctx: &Ctx<'_>) -> Option { - Some(PushMessage { +fn push_of(push_type: PushType, env: &Envelope, ctx: &Ctx<'_>) -> PushMessage { + PushMessage { push_type, entity_id: Some(env.entity_id), actor_user_id: ctx.caller.user_id, count: 1, - }) + } } async fn reload(conn: &mut SqliteConnection, id: Uuid) -> Result { @@ -552,7 +555,7 @@ async fn process( .await?; return Ok(Processed { result: accepted(env, K::entity(after)), - push: push_of(K::DELETED, env, ctx), + push: Some(push_of(K::DELETED, env, ctx)), }); } // Created and deleted before it ever synced: store the tombstone, no audit, no push. It is validated like a @@ -595,7 +598,7 @@ async fn process( .await?; return Ok(Processed { result: accepted(env, K::entity(after)), - push: push_of(K::CREATED, env, ctx), + push: Some(push_of(K::CREATED, env, ctx)), }); }; @@ -638,7 +641,7 @@ async fn process( .await?; Ok(Processed { result: accepted(env, K::entity(after)), - push: push_of(K::UPDATED, env, ctx), + push: Some(push_of(K::UPDATED, env, ctx)), }) } diff --git a/server/crates/kvitt-server/src/tasks/scheduler.rs b/server/crates/kvitt-server/src/tasks/scheduler.rs index a9ddff3..cfad659 100644 --- a/server/crates/kvitt-server/src/tasks/scheduler.rs +++ b/server/crates/kvitt-server/src/tasks/scheduler.rs @@ -61,17 +61,17 @@ pub async fn run_daily( if run_at_start { job().await; } - let mut due = plan(clock.now(), at).await; + let mut due = plan(clock.now(), at); loop { let now = clock.now(); if now >= due { job().await; - due = plan(clock.now().max(due), at).await; + due = plan(clock.now().max(due), at); continue; } if due - now > Duration::days(1) + Duration::hours(2) { // The clock went backward: plan from the new now. - due = plan(now, at).await; + due = plan(now, at); continue; } let wait = (due - now).to_std().unwrap_or(StdDuration::ZERO).min(poll); @@ -79,8 +79,8 @@ pub async fn run_daily( } } -/// [`next_run`], retried after a poll interval if it fails (it cannot for valid times; logged, never panics). -async fn plan(after: DateTime, at: NaiveTime) -> DateTime { +/// [`next_run`], or one day after `after` if it fails (it cannot for valid times; logged, never panics). +fn plan(after: DateTime, at: NaiveTime) -> DateTime { match next_run(after, at) { Ok(instant) => instant, Err(e) => { diff --git a/server/crates/kvitt-server/tests/core_api/generation.rs b/server/crates/kvitt-server/tests/core_api/generation.rs index 6c90b2e..964fdcd 100644 --- a/server/crates/kvitt-server/tests/core_api/generation.rs +++ b/server/crates/kvitt-server/tests/core_api/generation.rs @@ -301,7 +301,14 @@ async fn an_existing_period_is_skipped_silently_by_the_unique_index() { let (id, _) = new_template(&app, json!({"day_of_month": 1, "start_period": "2026-11"})).await; // Another writer already created 2026-11 (as a racing scheduler run would). let other = app - .insert_expense(ACCOUNT_1, HOME, 1250000, ADMIN_ID, (750000, 500000), false) + .insert_expense( + ACCOUNT_1, + HOME, + 1_250_000, + ADMIN_ID, + (750_000, 500_000), + false, + ) .await; sqlx::query( "UPDATE expenses SET recurring_template_id = ?, recurring_period = '2026-11' WHERE id = ?", @@ -469,7 +476,10 @@ async fn archived_account_continues_and_edits_affect_only_later_expenses() { dates(&generated(&app, id).await), ["2026-10-01", "2026-11-01"] ); - assert_eq!(expense_of(&app, id, "2026-10").await["amount_ore"], 1250000); + assert_eq!( + expense_of(&app, id, "2026-10").await["amount_ore"], + 1_250_000 + ); let november = expense_of(&app, id, "2026-11").await; assert_eq!(november["amount_ore"], 2000); assert_eq!(november["account_id"], ACCOUNT_1); diff --git a/server/crates/kvitt-server/tests/core_api/logging.rs b/server/crates/kvitt-server/tests/core_api/logging.rs index 30d1ded..ca6ba29 100644 --- a/server/crates/kvitt-server/tests/core_api/logging.rs +++ b/server/crates/kvitt-server/tests/core_api/logging.rs @@ -81,7 +81,7 @@ async fn logs_hold_no_personal_content() { "/recurring-templates", &admin_token(), Some(template_body(json!({ - "description": "Hemligbeskrivning", "notes": "Hemliganteckning", "amount_ore": 987654, + "description": "Hemligbeskrivning", "notes": "Hemliganteckning", "amount_ore": 987_654, "day_of_month": 1, "start_period": "2026-09" }))), ) diff --git a/server/crates/kvitt-server/tests/core_api/rate_limit.rs b/server/crates/kvitt-server/tests/core_api/rate_limit.rs index c090245..3593fed 100644 --- a/server/crates/kvitt-server/tests/core_api/rate_limit.rs +++ b/server/crates/kvitt-server/tests/core_api/rate_limit.rs @@ -80,7 +80,7 @@ async fn unknown_tokens_count_against_the_ip_but_known_devices_keep_working() { 200 ); for i in 0..60 { - let tok = format!("kvt_{:0>43}", i); + let tok = format!("kvt_{i:0>43}"); let reply = app.send(from("192.0.2.1:1", None, "/me", Some(&tok))).await; assert_error(&reply, 401, "unauthorized"); } diff --git a/server/crates/kvitt-server/tests/openapi_contract.rs b/server/crates/kvitt-server/tests/openapi_contract.rs index e9ab4da..eb797db 100644 --- a/server/crates/kvitt-server/tests/openapi_contract.rs +++ b/server/crates/kvitt-server/tests/openapi_contract.rs @@ -206,7 +206,7 @@ fn canon(value: &Value, names: bool) -> Value { Some(Value::Array(inner)) if alternative.as_object().is_some_and(|o| o.len() == 1) => { - flat.extend(inner.iter().cloned()) + flat.extend(inner.iter().cloned()); } _ => flat.push(alternative), } @@ -383,7 +383,7 @@ fn generated_document_equals_contract() { paths .values() .filter_map(Value::as_object) - .map(|m| m.len()) + .map(serde_json::Map::len) .sum::() }) .unwrap_or(0); diff --git a/server/crates/kvitt-server/tests/skeleton/wire_examples.rs b/server/crates/kvitt-server/tests/skeleton/wire_examples.rs index 69b70f9..a4006f6 100644 --- a/server/crates/kvitt-server/tests/skeleton/wire_examples.rs +++ b/server/crates/kvitt-server/tests/skeleton/wire_examples.rs @@ -104,7 +104,6 @@ fn every_example_round_trips() { checked += 1; } else if dir_name == "push" { // FCM payloads (SPEC ยง14.6) are built by the push sender, not by a wire type of this crate. - continue; } else if check(&name, &example) { checked += 1; } else { diff --git a/server/crates/kvitt-server/tests/sync_api/audit.rs b/server/crates/kvitt-server/tests/sync_api/audit.rs index 7585b91..dc13468 100644 --- a/server/crates/kvitt-server/tests/sync_api/audit.rs +++ b/server/crates/kvitt-server/tests/sync_api/audit.rs @@ -44,7 +44,7 @@ async fn generation_writes_recurring_audit_with_server_now_and_no_device() { assert_eq!((a.client_at.as_str(), a.server_at.as_str()), (NOW, NOW)); assert_eq!( a.changes[3], - json!({"field": "amount_ore", "old": null, "new": 1250000}) + json!({"field": "amount_ore", "old": null, "new": 1_250_000}) ); // The entry reaches the phones through pull. let page = pull(&app, &admin_token(), 0, None).await.json; diff --git a/server/crates/kvitt-server/tests/sync_api/pull.rs b/server/crates/kvitt-server/tests/sync_api/pull.rs index b7960d1..1a1959c 100644 --- a/server/crates/kvitt-server/tests/sync_api/pull.rs +++ b/server/crates/kvitt-server/tests/sync_api/pull.rs @@ -28,7 +28,7 @@ fn versions(page: &Value) -> Vec { .map(|r| r["server_version"].as_i64().unwrap()) }) .collect(); - all.sort(); + all.sort_unstable(); all } @@ -99,7 +99,7 @@ async fn the_first_page_has_everything_in_version_order_with_the_epoch_and_serve .map(|r| r["server_version"].as_i64().unwrap()) .collect(); let mut sorted = v.clone(); - sorted.sort(); + sorted.sort_unstable(); assert_eq!(v, sorted, "{a} sorted by server_version"); } // Children come with the parent.