From 35c02066fe09981aa429b8b81c9b73c9339189c9 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Thu, 2 Jul 2026 20:45:37 +0200 Subject: [PATCH] refactor(clippy): fix the never-loop error and clear all lint warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo clippy --all-targets` was failing on a deny-by-default `never_loop` in the analysis SSE handler (the `loop` always returned on the first iteration — the stream `unfold` already re-enters per event), plus ~28 warnings. All resolved with no behaviour change: - admin/analysis SSE: drop the dead `loop` wrapper. - app: match port literals directly instead of `if p == 80` guards. - repo/user: separate doc list from the following paragraphs. - repo/upload_history: `sort_by_key(Reverse(..))` over `sort_by`. - crawler/nav test: construct the error directly (no `unwrap_err` on a literal `Err`). - test helpers: build configs via struct-update syntax instead of `Default::default()` + field reassignment; add a type alias for a complex audit-row tuple. - plus the mechanical `deref`/etc. fixes from `cargo clippy --fix`. Note: not running `cargo fmt` — the backend uses a consistent hand-formatted style that differs from rustfmt-default across ~110 files, so a blanket reformat would be pure churn. Co-Authored-By: Claude Opus 4.8 --- backend/src/analysis/vision.rs | 18 ++++++----- backend/src/api/admin/analysis.rs | 26 +++++++-------- backend/src/api/mangas.rs | 10 +++--- backend/src/app.rs | 16 +++++---- backend/src/config.rs | 6 ++-- backend/src/crawler/nav.rs | 3 +- backend/src/repo/upload_history.rs | 2 +- backend/src/repo/user.rs | 2 ++ backend/src/settings.rs | 52 +++++++++++++++++++----------- backend/tests/api_admin_storage.rs | 9 +++--- backend/tests/api_admin_users.rs | 9 +++--- backend/tests/api_mangas.rs | 2 +- backend/tests/api_tags.rs | 6 ++-- 13 files changed, 91 insertions(+), 70 deletions(-) diff --git a/backend/src/analysis/vision.rs b/backend/src/analysis/vision.rs index 080a064..5313d75 100644 --- a/backend/src/analysis/vision.rs +++ b/backend/src/analysis/vision.rs @@ -1212,14 +1212,16 @@ mod tests { // VisionClient pointed at a closed local port. Reqwest fails the // POST immediately (connection refused), but the prep work runs // before the POST is even built. - let mut cfg = AnalysisConfig::default(); - cfg.endpoint = "http://127.0.0.1:1/v1/chat/completions".into(); - cfg.model = "stub".into(); - cfg.request_timeout = Duration::from_secs(1); - // Tune slice geometry to match the test image's shape. - cfg.max_pixels = 1_000_000; - cfg.min_slice_height = 100; - cfg.tall_aspect_threshold = 1.8; + let cfg = AnalysisConfig { + endpoint: "http://127.0.0.1:1/v1/chat/completions".into(), + model: "stub".into(), + request_timeout: Duration::from_secs(1), + // Tune slice geometry to match the test image's shape. + max_pixels: 1_000_000, + min_slice_height: 100, + tall_aspect_threshold: 1.8, + ..Default::default() + }; let http = reqwest::Client::builder() .timeout(cfg.request_timeout) .no_proxy() diff --git a/backend/src/api/admin/analysis.rs b/backend/src/api/admin/analysis.rs index 5776965..43b62b6 100644 --- a/backend/src/api/admin/analysis.rs +++ b/backend/src/api/admin/analysis.rs @@ -58,20 +58,20 @@ async fn stream_status( ) -> Sse>> { let rx = state.analysis_events.subscribe(); let stream = futures_util::stream::unfold(rx, |mut rx| async move { - loop { - match rx.recv().await { - Ok(ev) => { - let event = Event::default() - .event("analysis") - .json_data(&ev) - .unwrap_or_else(|_| Event::default().comment("serialize error")); - return Some((Ok(event), rx)); - } - Err(RecvError::Lagged(_)) => { - return Some((Ok(Event::default().event("lagged").data("")), rx)); - } - Err(RecvError::Closed) => return None, + // One recv per unfold step; the stream driver re-enters for the next + // event, so no explicit loop is needed here. + match rx.recv().await { + Ok(ev) => { + let event = Event::default() + .event("analysis") + .json_data(&ev) + .unwrap_or_else(|_| Event::default().comment("serialize error")); + Some((Ok(event), rx)) } + Err(RecvError::Lagged(_)) => { + Some((Ok(Event::default().event("lagged").data("")), rx)) + } + Err(RecvError::Closed) => None, } }); Sse::new(stream).keep_alive(KeepAlive::default()) diff --git a/backend/src/api/mangas.rs b/backend/src/api/mangas.rs index 2533773..a06b630 100644 --- a/backend/src/api/mangas.rs +++ b/backend/src/api/mangas.rs @@ -255,8 +255,8 @@ async fn create( ) .await?; - let author_refs = repo::author::set_for_manga(&mut *tx, manga.id, &authors).await?; - repo::genre::set_for_manga(&mut *tx, manga.id, &metadata.genre_ids).await?; + let author_refs = repo::author::set_for_manga(&mut tx, manga.id, &authors).await?; + repo::genre::set_for_manga(&mut tx, manga.id, &metadata.genre_ids).await?; if let Some(img) = cover { let key = format!("mangas/{}/cover.{}", manga.id, img.ext); @@ -321,7 +321,7 @@ async fn update( let mut tx = state.db.begin().await?; let _updated = repo::manga::update_basics( - &mut *tx, + &mut tx, id, patch.title.as_deref().map(str::trim), patch.status.as_deref().map(str::trim), @@ -331,10 +331,10 @@ async fn update( ) .await?; if let Some(ref names) = authors_owned { - repo::author::set_for_manga(&mut *tx, id, names).await?; + repo::author::set_for_manga(&mut tx, id, names).await?; } if let Some(ref ids) = patch.genre_ids { - repo::genre::set_for_manga(&mut *tx, id, ids).await?; + repo::genre::set_for_manga(&mut tx, id, ids).await?; } tx.commit().await?; diff --git a/backend/src/app.rs b/backend/src/app.rs index 1c1a3b3..86a3a52 100644 --- a/backend/src/app.rs +++ b/backend/src/app.rs @@ -1266,8 +1266,8 @@ fn parse_origin(raw: &str) -> Option { let host = url.host_str()?; let scheme = url.scheme(); let port_str = match (url.port(), scheme) { - (Some(p), "http") if p == 80 => String::new(), - (Some(p), "https") if p == 443 => String::new(), + (Some(80), "http") => String::new(), + (Some(443), "https") => String::new(), (Some(p), _) => format!(":{p}"), (None, _) => String::new(), }; @@ -1509,7 +1509,6 @@ mod tests { let storage_dir = tempfile::tempdir().unwrap(); let storage: Arc = Arc::new(LocalStorage::new(storage_dir.path())); - let mut cfg = crate::config::AnalysisConfig::default(); // The worker always runs OCR now (vision is dormant — see // `effective_backend`), and the `.rten` models aren't shipped to unit // CI. Point the engine at a path that can't exist so the *engine build* @@ -1518,10 +1517,13 @@ mod tests { // readiness, so the row must still be reclaimed even though spawn // returns Err. That's exactly the regression this test guards (an // analysis-only deploy must reclaim orphaned leases at startup). - cfg.ocr_detection_model = "/nonexistent/text-detection.rten".to_string(); - cfg.ocr_recognition_model = "/nonexistent/text-recognition.rten".to_string(); - cfg.workers = 1; - cfg.job_timeout = Duration::from_secs(1); + let cfg = crate::config::AnalysisConfig { + ocr_detection_model: "/nonexistent/text-detection.rten".to_string(), + ocr_recognition_model: "/nonexistent/text-recognition.rten".to_string(), + workers: 1, + job_timeout: Duration::from_secs(1), + ..Default::default() + }; let events = Arc::new(crate::analysis::events::AnalysisEvents::new()); let spawned = spawn_analysis_daemon(pool.clone(), storage, &cfg, events).await; diff --git a/backend/src/config.rs b/backend/src/config.rs index a084db0..f626b27 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -876,8 +876,10 @@ mod tests { // what `ANALYSIS_BACKEND` parsed to. `backend` still reflects the raw // request (so the override is visible/loggable), but `effective_backend` // is the value the daemon actually dispatches through. - let mut cfg = AnalysisConfig::default(); - cfg.backend = AnalysisBackend::Vision; + let mut cfg = AnalysisConfig { + backend: AnalysisBackend::Vision, + ..Default::default() + }; assert_eq!(cfg.effective_backend(), AnalysisBackend::Ocr); cfg.backend = AnalysisBackend::Ocr; assert_eq!(cfg.effective_backend(), AnalysisBackend::Ocr); diff --git a/backend/src/crawler/nav.rs b/backend/src/crawler/nav.rs index 516cda4..73e0a46 100644 --- a/backend/src/crawler/nav.rs +++ b/backend/src/crawler/nav.rs @@ -164,8 +164,7 @@ mod tests { #[test] fn anyhow_with_nav_timeout_in_chain_is_flagged() { - let inner: Result<(), NavError> = Err(NavError::Timeout(NAV_TIMEOUT)); - let outer = inner.unwrap_err(); + let outer = NavError::Timeout(NAV_TIMEOUT); let wrapped: anyhow::Error = anyhow::Error::new(outer).context("wait for chapter nav"); assert!(anyhow_looks_browser_dead(&wrapped)); diff --git a/backend/src/repo/upload_history.rs b/backend/src/repo/upload_history.rs index 4fe925b..b10dbd8 100644 --- a/backend/src/repo/upload_history.rs +++ b/backend/src/repo/upload_history.rs @@ -110,7 +110,7 @@ pub async fn list_for_user( }); } // Newest first; trim to limit after the merge. - entries.sort_by(|a, b| b.created_at().cmp(&a.created_at())); + entries.sort_by_key(|b| std::cmp::Reverse(b.created_at())); entries.truncate(limit as usize); let (manga_total, chapter_total): (i64, i64) = sqlx::query_as( diff --git a/backend/src/repo/user.rs b/backend/src/repo/user.rs index 904d856..d4acd25 100644 --- a/backend/src/repo/user.rs +++ b/backend/src/repo/user.rs @@ -157,8 +157,10 @@ pub async fn set_is_admin_unchecked(pool: &PgPool, id: Uuid, value: bool) -> App /// - If a row already exists: flip `is_admin` to true if needed; **never** /// touch the existing `password_hash`. Lets the operator rotate the /// admin password through the UI without env-var conflict. +/// /// Wrapped in a transaction so a concurrent `register` for the same /// username can't slip an INSERT between the SELECT and UPDATE/INSERT. +/// /// Set `is_admin` on a user with full safety checks: rejects self-demote, /// rejects demoting the only remaining admin (under `ADMIN_INVARIANT_LOCK_KEY` /// to close the parallel-demote race), and writes an `admin_audit` row diff --git a/backend/src/settings.rs b/backend/src/settings.rs index 29c13c4..322be38 100644 --- a/backend/src/settings.rs +++ b/backend/src/settings.rs @@ -567,11 +567,13 @@ mod tests { #[test] fn crawler_round_trips_through_dto() { - let mut base = CrawlerConfig::default(); - base.start_url = Some("https://example.com/".to_string()); - base.tz = Tz::Europe__Berlin; - base.chapter_workers = 3; - base.cookie_domain = Some("example.com".to_string()); + let base = CrawlerConfig { + start_url: Some("https://example.com/".to_string()), + tz: Tz::Europe__Berlin, + chapter_workers: 3, + cookie_domain: Some("example.com".to_string()), + ..Default::default() + }; let dto = CrawlerSettings::from_config(&base); let back = dto.to_config(&base).expect("valid"); assert_eq!(back.tz, Tz::Europe__Berlin); @@ -597,10 +599,12 @@ mod tests { #[test] fn crawler_overlay_preserves_env_only_fields() { - let mut base = CrawlerConfig::default(); - base.proxy = Some("socks5://127.0.0.1:9050".to_string()); - base.tor_control_password = Some("secret".to_string()); - base.phpsessid = Some("abc123".to_string()); + let base = CrawlerConfig { + proxy: Some("socks5://127.0.0.1:9050".to_string()), + tor_control_password: Some("secret".to_string()), + phpsessid: Some("abc123".to_string()), + ..Default::default() + }; // A DTO that knows nothing about the env-only fields. let dto = CrawlerSettings { rate_ms: 2000, @@ -704,8 +708,10 @@ mod tests { #[test] fn analysis_captures_env_prompt_override() { - let mut base = AnalysisConfig::default(); - base.system_prompt = "custom env prompt".to_string(); + let base = AnalysisConfig { + system_prompt: "custom env prompt".to_string(), + ..Default::default() + }; let dto = AnalysisSettings::from_config(&base); assert_eq!(dto.system_prompt.as_deref(), Some("custom env prompt")); } @@ -729,8 +735,10 @@ mod tests { #[test] fn analysis_overlay_preserves_api_key() { - let mut base = AnalysisConfig::default(); - base.api_key = Some("sk-secret".to_string()); + let base = AnalysisConfig { + api_key: Some("sk-secret".to_string()), + ..Default::default() + }; let dto = AnalysisSettings::from_config(&base); assert_eq!(dto.to_config(&base).unwrap().api_key.as_deref(), Some("sk-secret")); } @@ -872,8 +880,10 @@ mod tests { // a hostile/CSRF-able admin must NOT be able to point endpoint at // cloud metadata, loopback services, or RFC1918 hosts — when the // worker is enabled (toggling enabled=true later re-runs this gate). - let mut base = AnalysisConfig::default(); - base.backend = AnalysisBackend::Vision; + let base = AnalysisConfig { + backend: AnalysisBackend::Vision, + ..Default::default() + }; for url in [ "http://169.254.169.254/v1/chat/completions", "http://127.0.0.1:5432/", @@ -897,8 +907,10 @@ mod tests { // The documented default — docker DNS name resolving to a private IP // at runtime — must still validate, because the bearer recipient // identity is the operator-chosen hostname, not the underlying IP. - let mut base = AnalysisConfig::default(); - base.backend = AnalysisBackend::Vision; + let base = AnalysisConfig { + backend: AnalysisBackend::Vision, + ..Default::default() + }; for url in [ "http://mangalord-vision:8000/v1/chat/completions", "https://api.openai.com/v1/chat/completions", @@ -931,8 +943,10 @@ mod tests { fn analysis_requires_model_only_when_enabled() { // Vision base: the model id is required only when the vision worker // is actually live (see the OCR carve-out below). - let mut base = AnalysisConfig::default(); - base.backend = AnalysisBackend::Vision; + let base = AnalysisConfig { + backend: AnalysisBackend::Vision, + ..Default::default() + }; let disabled = AnalysisSettings { enabled: false, model: "".to_string(), diff --git a/backend/tests/api_admin_storage.rs b/backend/tests/api_admin_storage.rs index c69a81d..ff2c1ae 100644 --- a/backend/tests/api_admin_storage.rs +++ b/backend/tests/api_admin_storage.rs @@ -279,7 +279,7 @@ async fn backfill_fills_unmeasured_pages_and_covers_idempotently(pool: PgPool) { let body2 = common::body_json(resp2).await; assert_eq!(body2["pages"].as_i64().unwrap(), 0); assert_eq!(body2["covers"].as_i64().unwrap(), 0); - assert_eq!(body2["more_remaining"].as_bool().unwrap(), false); + assert!(!body2["more_remaining"].as_bool().unwrap()); } #[sqlx::test(migrations = "./migrations")] @@ -362,7 +362,7 @@ async fn backfill_caps_per_run_and_reports_more_remaining(pool: PgPool) { assert_eq!(resp.status(), StatusCode::OK); let body = common::body_json(resp).await; // Capped at 20_000 attempts this run; one row left → run again. - assert_eq!(body["more_remaining"].as_bool().unwrap(), true); + assert!(body["more_remaining"].as_bool().unwrap()); assert_eq!(body["missing"].as_i64().unwrap(), 20_000); assert_eq!(body["pages"].as_i64().unwrap(), 0); } @@ -403,9 +403,8 @@ async fn backfill_at_exact_cap_is_not_more_remaining(pool: PgPool) { .unwrap(); let body = common::body_json(resp).await; assert_eq!(body["missing"].as_i64().unwrap(), 20_000); - assert_eq!( - body["more_remaining"].as_bool().unwrap(), - false, + assert!( + !body["more_remaining"].as_bool().unwrap(), "exactly-cap backlog drains in one run; no follow-up needed" ); } diff --git a/backend/tests/api_admin_users.rs b/backend/tests/api_admin_users.rs index 2e92d9a..0f95d4a 100644 --- a/backend/tests/api_admin_users.rs +++ b/backend/tests/api_admin_users.rs @@ -381,10 +381,11 @@ async fn delete_writes_audit_row(pool: PgPool) { .unwrap(); assert_eq!(resp.status(), StatusCode::NO_CONTENT); - let rows: Vec<(Option, String, String, Option, serde_json::Value)> = - sqlx::query_as( - "SELECT actor_user_id, action, target_kind, target_id, payload FROM admin_audit", - ) + // (actor_user_id, action, target_kind, target_id, payload) + type AuditRow = (Option, String, String, Option, serde_json::Value); + let rows: Vec = sqlx::query_as( + "SELECT actor_user_id, action, target_kind, target_id, payload FROM admin_audit", + ) .fetch_all(&pool) .await .unwrap(); diff --git a/backend/tests/api_mangas.rs b/backend/tests/api_mangas.rs index 2892ba1..0aeefd5 100644 --- a/backend/tests/api_mangas.rs +++ b/backend/tests/api_mangas.rs @@ -446,7 +446,7 @@ async fn list_tie_break_orders_equal_keys_by_ascending_id(pool: PgPool) { // Paginating one row at a time reproduces that exact sequence — no overlap, // no gap — which only holds because the id tie-break makes the order total. - let paged: Vec = vec![page(1, 0).await, page(1, 1).await, page(1, 2).await] + let paged: Vec = [page(1, 0).await, page(1, 1).await, page(1, 2).await] .iter() .map(|b| b["items"][0]["title"].as_str().unwrap().to_string()) .collect(); diff --git a/backend/tests/api_tags.rs b/backend/tests/api_tags.rs index c959151..2026f7b 100644 --- a/backend/tests/api_tags.rs +++ b/backend/tests/api_tags.rs @@ -279,7 +279,7 @@ async fn tag_autocomplete_returns_matches_ordered_by_similarity(pool: PgPool) { .iter() .map(|t| t["name"].as_str().unwrap()) .collect(); - assert!(names.iter().any(|n| *n == "Mystery")); - assert!(names.iter().any(|n| *n == "Murder Mystery")); - assert!(!names.iter().any(|n| *n == "Comedy")); + assert!(names.contains(&"Mystery")); + assert!(names.contains(&"Murder Mystery")); + assert!(!names.contains(&"Comedy")); }