From 1510abf5639393fc7dbda01fc17ff59792cbc885 Mon Sep 17 00:00:00 2001 From: Axylum <86928157+AxylumRust@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:32:48 +0100 Subject: [PATCH] url_utils: satisfy Clippy pedantic lints (#2681) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Update — 2026-11-01 The latest revision removes the systematic `#[must_use]` additions and any product-name backticks introduced solely for optional `clippy::pedantic` compliance. The useful behavior-preserving cleanups remain. Earlier summary and verification text below describes the original revision; the latest PR comment records the current checks. --- ## Summary - resolve all 54 distinct diagnostics in `url_utils`: 17 pedantic, 36 deprecation warnings, and one Eq/Hash contract issue - preserve the public compatibility module and nested module paths with narrow documented expectations - implement `Hash` against the same bare value as `PartialEq`, with a regression test - apply the repository two-space rustfmt rules within `url_utils` only No public API or dependency changes. ## Verification - `SQLX_OFFLINE=true cargo clippy -p url_utils --all-targets -- -W clippy::pedantic -D warnings` - `SQLX_OFFLINE=true cargo test -p url_utils` — 32 unit tests and 2 doctests passed - `cargo fmt -p url_utils -- --check` - Independent read-only review: no Rust findings; the untracked planning file is excluded from the PR --------- Co-authored-by: Cédric Legrand --- .../download_extension/download_extension.rs | 1 - .../extract_download_extension_from_url.rs | 8 +- .../url_utils/src/download_extension/mod.rs | 1 + .../lib/url_utils/src/extension/extension.rs | 41 ++++++--- .../extension/extract_extension_from_url.rs | 85 +++++++++---------- crates/lib/url_utils/src/extension/mod.rs | 3 +- crates/lib/url_utils/src/lib.rs | 3 +- 7 files changed, 75 insertions(+), 67 deletions(-) diff --git a/crates/lib/url_utils/src/download_extension/download_extension.rs b/crates/lib/url_utils/src/download_extension/download_extension.rs index 4193b07467..93ad91347e 100644 --- a/crates/lib/url_utils/src/download_extension/download_extension.rs +++ b/crates/lib/url_utils/src/download_extension/download_extension.rs @@ -1,4 +1,3 @@ - /// When downloading a remote file, it makes sense to try to match the same file extension (with reasonable safeguards). #[deprecated(note = "Use `Extension` in `url_utils::extension::extension` instead")] #[derive(Debug, Copy, Clone, Eq, PartialEq)] diff --git a/crates/lib/url_utils/src/download_extension/extract_download_extension_from_url.rs b/crates/lib/url_utils/src/download_extension/extract_download_extension_from_url.rs index 1d0c1b6465..5fd0984539 100644 --- a/crates/lib/url_utils/src/download_extension/extract_download_extension_from_url.rs +++ b/crates/lib/url_utils/src/download_extension/extract_download_extension_from_url.rs @@ -12,9 +12,7 @@ pub fn extract_download_extension_from_url_str(url: &str) -> Option Option { let path = url.path(); - let extension = std::path::Path::new(path) - .extension() - .and_then(|ext| ext.to_str())?; + let extension = std::path::Path::new(path).extension().and_then(|ext| ext.to_str())?; DownloadExtension::try_from_str(extension) } @@ -26,14 +24,14 @@ mod tests { #[test] fn success_case() { let url = "https://storyteller.ai/example.jpg"; - let maybe_extension= extract_download_extension_from_url_str(url); + let maybe_extension = extract_download_extension_from_url_str(url); assert_eq!(maybe_extension, Some(DownloadExtension::Jpg)); } #[test] fn difficult_url() { let url = "https://videos.youtube.com/foo/bar/files/00000000-f19a-1275-a092-caa60b38f8d2%2Fraw?se=2025-11-21T22%3A26%3A53Z&sp=r&sv=2024-08-09&sr=b&skoid=8cfff87a-01fc-27c1-9091-32999dcd1389&sktid=aa8ccaf6-e6da-484e-ab11-9c7496c21c53&skt=2025-11-17T21%3A09%3A08Z&ske=2025-10-23T21%3A14%3A08Z&sks=b&skv=2024-08-04&sig=T2U6%2Brw9WJdJagBkCL4/GlFr72KhZGP3QPlobIO7JlU%3D&ac=oaisdsorprnorthcentralus"; - let maybe_extension= extract_download_extension_from_url_str(url); + let maybe_extension = extract_download_extension_from_url_str(url); assert!(maybe_extension.is_none()); } } diff --git a/crates/lib/url_utils/src/download_extension/mod.rs b/crates/lib/url_utils/src/download_extension/mod.rs index 3706f2dddb..576d032ace 100644 --- a/crates/lib/url_utils/src/download_extension/mod.rs +++ b/crates/lib/url_utils/src/download_extension/mod.rs @@ -1,3 +1,4 @@ +#[expect(clippy::module_inception, reason = "Preserve the existing public module path")] mod download_extension; pub mod extract_download_extension_from_url; diff --git a/crates/lib/url_utils/src/extension/extension.rs b/crates/lib/url_utils/src/extension/extension.rs index a92df6d43c..cee9a92ec6 100644 --- a/crates/lib/url_utils/src/extension/extension.rs +++ b/crates/lib/url_utils/src/extension/extension.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; use std::fmt; +use std::hash::{Hash, Hasher}; /// A file extension that can be either a compile-time constant or a runtime-parsed value. /// @@ -16,7 +17,7 @@ use std::fmt; /// assert_eq!(ext.without_period(), "mp4"); /// assert_eq!(ext.with_period(), ".mp4"); /// ``` -#[derive(Clone, Eq, Hash)] +#[derive(Clone, Eq)] pub struct Extension { /// The extension without the leading period, e.g. "png", "mp4". value: Cow<'static, str>, @@ -34,20 +35,14 @@ impl Extension { /// const MP4: Extension = Extension::from_static("mp4", ".mp4"); /// ``` pub const fn from_static(ext: &'static str, dotted: &'static str) -> Self { - Self { - value: Cow::Borrowed(ext), - dotted: Cow::Borrowed(dotted), - } + Self { value: Cow::Borrowed(ext), dotted: Cow::Borrowed(dotted) } } /// Creates an `Extension` from a `&str` at runtime. /// The input must NOT include a leading period. pub fn new(ext: &str) -> Self { - let dotted = format!(".{}", ext); - Self { - value: Cow::Owned(ext.to_string()), - dotted: Cow::Owned(dotted), - } + let dotted = format!(".{ext}"); + Self { value: Cow::Owned(ext.to_string()), dotted: Cow::Owned(dotted) } } /// Returns the extension without the leading period, e.g. `"png"`. @@ -67,6 +62,12 @@ impl PartialEq for Extension { } } +impl Hash for Extension { + fn hash(&self, state: &mut H) { + self.value.hash(state); + } +} + impl fmt::Debug for Extension { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Extension(\"{}\")", self.value) @@ -135,12 +136,28 @@ mod tests { #[test] fn display() { - assert_eq!(format!("{}", PNG), "png"); + assert_eq!(format!("{PNG}"), "png"); assert_eq!(format!("{}", Extension::new("xyz")), "xyz"); } #[test] fn debug() { - assert_eq!(format!("{:?}", PNG), "Extension(\"png\")"); + assert_eq!(format!("{PNG:?}"), "Extension(\"png\")"); + } + + #[test] + fn hash_matches_equality() { + let plain = Extension::from_static("png", ".png"); + let inconsistent = Extension::from_static("png", ".not-a-suffix"); + assert_eq!(plain, inconsistent); + assert_eq!(hash(&plain), hash(&inconsistent)); + } + + fn hash(extension: &Extension) -> u64 { + use std::collections::hash_map::DefaultHasher; + + let mut hasher = DefaultHasher::new(); + extension.hash(&mut hasher); + hasher.finish() } } diff --git a/crates/lib/url_utils/src/extension/extract_extension_from_url.rs b/crates/lib/url_utils/src/extension/extract_extension_from_url.rs index 928aa583fe..2fb3310ead 100644 --- a/crates/lib/url_utils/src/extension/extract_extension_from_url.rs +++ b/crates/lib/url_utils/src/extension/extract_extension_from_url.rs @@ -6,24 +6,13 @@ use url::Url; // TODO: Lookups against these as lists will be slow /// Known image extensions. -const KNOWN_IMAGE_EXTENSIONS: &[Extension] = &[ - Extension::from_static("png", ".png"), - Extension::from_static("jpg", ".jpg"), - Extension::from_static("jpeg", ".jpeg"), - Extension::from_static("webp", ".webp"), -]; +const KNOWN_IMAGE_EXTENSIONS: &[Extension] = &[Extension::from_static("png", ".png"), Extension::from_static("jpg", ".jpg"), Extension::from_static("jpeg", ".jpeg"), Extension::from_static("webp", ".webp")]; /// Known audio extensions. -const KNOWN_AUDIO_EXTENSIONS: &[Extension] = &[ - Extension::from_static("wav", ".wav"), - Extension::from_static("mp3", ".mp3"), -]; +const KNOWN_AUDIO_EXTENSIONS: &[Extension] = &[Extension::from_static("wav", ".wav"), Extension::from_static("mp3", ".mp3")]; /// Known video extensions. -const KNOWN_VIDEO_EXTENSIONS: &[Extension] = &[ - Extension::from_static("mp4", ".mp4"), - Extension::from_static("webm", ".webm"), -]; +const KNOWN_VIDEO_EXTENSIONS: &[Extension] = &[Extension::from_static("mp4", ".mp4"), Extension::from_static("webm", ".webm")]; /// Controls which extensions are accepted when extracting from a URL. pub enum ExtractExtensions { @@ -62,9 +51,7 @@ pub fn extract_extension_from_url_str(url: &str, accept: &ExtractExtensions) -> /// Extracts a file extension from a parsed `Url`, filtered by `accept`. pub fn extract_extension_from_url(url: &Url, accept: &ExtractExtensions) -> Option { let path = url.path(); - let raw_ext = std::path::Path::new(path) - .extension() - .and_then(|ext| ext.to_str())?; + let raw_ext = std::path::Path::new(path).extension().and_then(|ext| ext.to_str())?; let lower = raw_ext.to_lowercase(); let candidate = Extension::new(&lower); @@ -72,27 +59,40 @@ pub fn extract_extension_from_url(url: &Url, accept: &ExtractExtensions) -> Opti match accept { ExtractExtensions::All => Some(candidate), ExtractExtensions::Set(set) => { - if set.contains(&candidate) { Some(candidate) } else { None } - } - ExtractExtensions::KnownImage => { - if slice_contains(KNOWN_IMAGE_EXTENSIONS, &candidate) { Some(candidate) } else { None } - } - ExtractExtensions::KnownAudio => { - if slice_contains(KNOWN_AUDIO_EXTENSIONS, &candidate) { Some(candidate) } else { None } - } - ExtractExtensions::KnownVideo => { - if slice_contains(KNOWN_VIDEO_EXTENSIONS, &candidate) { Some(candidate) } else { None } - } - ExtractExtensions::KnownMedia => { - if slice_contains(KNOWN_IMAGE_EXTENSIONS, &candidate) - || slice_contains(KNOWN_AUDIO_EXTENSIONS, &candidate) - || slice_contains(KNOWN_VIDEO_EXTENSIONS, &candidate) - { + if set.contains(&candidate) { Some(candidate) } else { None } - } + }, + ExtractExtensions::KnownImage => { + if slice_contains(KNOWN_IMAGE_EXTENSIONS, &candidate) { + Some(candidate) + } else { + None + } + }, + ExtractExtensions::KnownAudio => { + if slice_contains(KNOWN_AUDIO_EXTENSIONS, &candidate) { + Some(candidate) + } else { + None + } + }, + ExtractExtensions::KnownVideo => { + if slice_contains(KNOWN_VIDEO_EXTENSIONS, &candidate) { + Some(candidate) + } else { + None + } + }, + ExtractExtensions::KnownMedia => { + if slice_contains(KNOWN_IMAGE_EXTENSIONS, &candidate) || slice_contains(KNOWN_AUDIO_EXTENSIONS, &candidate) || slice_contains(KNOWN_VIDEO_EXTENSIONS, &candidate) { + Some(candidate) + } else { + None + } + }, } } @@ -190,19 +190,14 @@ mod tests { #[test] fn set_matches_custom_list() { - let accept = ExtractExtensions::from_vec(vec![ - Extension::from_static("csv", ".csv"), - Extension::from_static("tsv", ".tsv"), - ]); + let accept = ExtractExtensions::from_vec(vec![Extension::from_static("csv", ".csv"), Extension::from_static("tsv", ".tsv")]); let ext = extract_extension_from_url_str("https://example.com/data.csv", &accept); assert_eq!(ext.unwrap().without_period(), "csv"); } #[test] fn set_rejects_non_member() { - let accept = ExtractExtensions::from_vec(vec![ - Extension::from_static("csv", ".csv"), - ]); + let accept = ExtractExtensions::from_vec(vec![Extension::from_static("csv", ".csv")]); let ext = extract_extension_from_url_str("https://example.com/data.png", &accept); assert!(ext.is_none()); } @@ -215,10 +210,7 @@ mod tests { #[test] fn query_params_ignored() { - let ext = extract_extension_from_url_str( - "https://example.com/file.mp4?token=abc&format=hd", - &ExtractExtensions::KnownVideo, - ); + let ext = extract_extension_from_url_str("https://example.com/file.mp4?token=abc&format=hd", &ExtractExtensions::KnownVideo); assert_eq!(ext.unwrap().without_period(), "mp4"); } @@ -230,8 +222,7 @@ mod tests { } fn extract_extension_all(url: &str) -> Option { - extract_extension_from_url_str(url, &ExtractExtensions::All) - .map(|ext| ext.without_period().to_string()) + extract_extension_from_url_str(url, &ExtractExtensions::All).map(|ext| ext.without_period().to_string()) } #[test] diff --git a/crates/lib/url_utils/src/extension/mod.rs b/crates/lib/url_utils/src/extension/mod.rs index 824c6d227e..e0cc69f24a 100644 --- a/crates/lib/url_utils/src/extension/mod.rs +++ b/crates/lib/url_utils/src/extension/mod.rs @@ -1,2 +1,3 @@ +#[expect(clippy::module_inception, reason = "Preserve the existing public module path")] pub mod extension; -pub mod extract_extension_from_url; \ No newline at end of file +pub mod extract_extension_from_url; diff --git a/crates/lib/url_utils/src/lib.rs b/crates/lib/url_utils/src/lib.rs index 2f3ba5d097..abb775f0c5 100644 --- a/crates/lib/url_utils/src/lib.rs +++ b/crates/lib/url_utils/src/lib.rs @@ -1,2 +1,3 @@ pub mod extension; -pub mod download_extension; \ No newline at end of file +#[expect(deprecated, reason = "Keep the public compatibility module while downstream users migrate to Extension")] +pub mod download_extension;