From 73781a2eb6a40f66a59f7da1e4bd14f78af089a9 Mon Sep 17 00:00:00 2001 From: Josh Clayton Date: Tue, 31 Mar 2026 10:46:25 -0400 Subject: [PATCH] chore: Add clippy lints and fix violations New lints: - unwrap_used: complements existing expect_used; prevents .unwrap() in library code - wildcard_enum_match_arm: complements exhaustive_enums; forces explicit variant handling instead of _ catch-alls - missing_errors_doc: enforces # Errors sections on public Result-returning fns - missing_panics_doc: enforces # Panics sections where applicable - redundant_else: removes else after blocks that already return/break Fixes: - Add unwrap_used to test module expect lists (alongside existing expect_used) - Expand wildcard match arms in jwks.rs to list variants explicitly - Replace wildcard match in pages.rs error formatting with if-let - Replace match/panic patterns in page tests with assert!(matches!(...)) Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 71 +++++++++++++++++++++++----------- Cargo.toml | 12 ++++-- examples/jwks_demo.rs | 1 + src/builder.rs | 1 + src/jwks.rs | 19 +++++++-- src/oidc/claims/email.rs | 4 ++ src/oidc/claims/issuer.rs | 4 ++ src/oidc/claims/mod.rs | 1 + src/oidc/claims/picture_url.rs | 4 ++ src/pages.rs | 47 +++++++++++----------- src/scope.rs | 4 ++ src/test_support.rs | 1 + src/token/mod.rs | 1 + tests/auth_flow.rs | 1 + tests/jwks_flow.rs | 1 + tests/oidc_discovery.rs | 1 + tests/oidc_flow.rs | 1 + 17 files changed, 121 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68c2d73..5d388f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -135,11 +135,11 @@ checksum = "843867be96c8daad0d758b57df9392b6d8d271134fce549de6ce169ff98a92af" [[package]] name = "block-buffer" -version = "0.10.4" +version = "0.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" +checksum = "cdd35008169921d80bc60d3d0ab416eecb028c4cd653352907921d95084790be" dependencies = [ - "generic-array", + "hybrid-array", ] [[package]] @@ -191,7 +191,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6f8d983286843e49675a4b7a2d174efe136dc93a18d69130dd18198a6c167601" dependencies = [ "cfg-if", - "cpufeatures 0.3.0", + "cpufeatures", "rand_core 0.10.0", ] @@ -220,6 +220,12 @@ version = "0.9.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2459377285ad874054d797f3ccebf984978aa39129f6eafde5cdc8315b612f8" +[[package]] +name = "const-oid" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c" + [[package]] name = "core-foundation" version = "0.9.4" @@ -246,15 +252,6 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" -[[package]] -name = "cpufeatures" -version = "0.2.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" -dependencies = [ - "libc", -] - [[package]] name = "cpufeatures" version = "0.3.0" @@ -274,13 +271,22 @@ dependencies = [ "typenum", ] +[[package]] +name = "crypto-common" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77727bb15fa921304124b128af125e7e3b968275d1b108b379190264f4423710" +dependencies = [ + "hybrid-array", +] + [[package]] name = "der" version = "0.7.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e7c1832837b905bbfb5101e07cc24c8deddf52f93225eee6ead5f4d63d53ddcb" dependencies = [ - "const-oid", + "const-oid 0.9.6", "pem-rfc7468", "zeroize", ] @@ -299,10 +305,20 @@ name = "digest" version = "0.10.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" +dependencies = [ + "const-oid 0.9.6", + "crypto-common 0.1.7", +] + +[[package]] +name = "digest" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4850db49bf08e663084f7fb5c87d202ef91a3907271aff24a94eb97ff039153c" dependencies = [ "block-buffer", - "const-oid", - "crypto-common", + "const-oid 0.10.2", + "crypto-common 0.2.1", ] [[package]] @@ -555,6 +571,15 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" +[[package]] +name = "hybrid-array" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a79f2aff40c18ab8615ddc5caa9eb5b96314aef18fe5823090f204ad988e813" +dependencies = [ + "typenum", +] + [[package]] name = "hyper" version = "1.8.1" @@ -1478,8 +1503,8 @@ version = "0.9.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8573f03f5883dcaebdfcf4725caa1ecb9c15b2ef50c43a07b816e06799bb12d" dependencies = [ - "const-oid", - "digest", + "const-oid 0.9.6", + "digest 0.10.7", "num-bigint-dig", "num-integer", "num-traits", @@ -1716,13 +1741,13 @@ dependencies = [ [[package]] name = "sha2" -version = "0.10.9" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" +checksum = "446ba717509524cb3f22f17ecc096f10f4822d76ab5c0b9822c5f9c284e825f4" dependencies = [ "cfg-if", - "cpufeatures 0.2.17", - "digest", + "cpufeatures", + "digest 0.11.2", ] [[package]] @@ -1756,7 +1781,7 @@ version = "2.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77549399552de45a898a580c1b41d445bf730df867cc44e6c0233bbc4b8329de" dependencies = [ - "digest", + "digest 0.10.7", "rand_core 0.6.4", ] diff --git a/Cargo.toml b/Cargo.toml index e01cb5c..a933d62 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ homepage = "https://github.com/joshuaclayton/loopauth" documentation = "https://docs.rs/loopauth" readme = "README.md" keywords = ["oauth2", "pkce", "cli", "authentication", "oidc"] -categories = ["authentication", "web-programming"] +categories = ["authentication", "command-line-interface"] [dependencies] async-trait = "0.1" @@ -24,7 +24,7 @@ rsa = { version = "0.9", features = ["pem"] } rustls = "0.23" serde = { version = "1", features = ["derive"] } serde_json = "1" -sha2 = "0.10" +sha2 = "0.11" subtle = "2.6.1" thiserror = "2" tokio = { version = "1", features = ["full"] } @@ -50,6 +50,7 @@ exhaustive_structs = "deny" panic = "deny" panic_in_result_fn = "deny" unwrap_in_result = "deny" +unwrap_used = "deny" get_unwrap = "deny" expect_used = "deny" todo = "deny" @@ -64,11 +65,16 @@ exit = "deny" # Error handling discipline map_err_ignore = "deny" indexing_slicing = "deny" -string_slice = "deny" # can panic on non-ASCII boundaries +string_slice = "deny" # can panic on non-ASCII boundaries +wildcard_enum_match_arm = "deny" # Meta: suppressions must explain themselves allow_attributes_without_reason = "deny" allow_attributes = "deny" +# Public API documentation completeness +missing_errors_doc = "deny" +missing_panics_doc = "deny" # Style consistency worth enforcing partial_pub_fields = "deny" unseparated_literal_suffix = "deny" missing_assert_message = "deny" +redundant_else = "deny" diff --git a/examples/jwks_demo.rs b/examples/jwks_demo.rs index 891ba89..69823b9 100644 --- a/examples/jwks_demo.rs +++ b/examples/jwks_demo.rs @@ -1,6 +1,7 @@ #![expect( clippy::string_slice, clippy::expect_used, + clippy::unwrap_used, reason = "CLI examples can be more lax" )] // # Usage diff --git a/src/builder.rs b/src/builder.rs index 3c57ff5..0d0ad53 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1665,6 +1665,7 @@ mod tests { #![expect( clippy::indexing_slicing, clippy::expect_used, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] diff --git a/src/jwks.rs b/src/jwks.rs index 72f4cc0..cac9f78 100644 --- a/src/jwks.rs +++ b/src/jwks.rs @@ -266,7 +266,11 @@ fn select_key_with_kid<'a>(keys: &'a [JwkKey], kid: &str) -> (Option<&'a JwkKey> found = Some(key); break; } - _ => { + JwkKey::RsaWithKid { .. } + | JwkKey::Rsa { .. } + | JwkKey::Ec { .. } + | JwkKey::EcWithKid { .. } + | JwkKey::Unsupported { .. } => { skipped.push(jwk_key_description(key)); } } @@ -284,7 +288,9 @@ fn build_decoding_key_and_validation( alg: jsonwebtoken::Algorithm, client_id: &str, ) -> Result<(jsonwebtoken::DecodingKey, jsonwebtoken::Validation), JwksValidationError> { - use jsonwebtoken::Algorithm::{ES256, ES384, PS256, PS384, PS512, RS256, RS384, RS512}; + use jsonwebtoken::Algorithm::{ + ES256, ES384, EdDSA, HS256, HS384, HS512, PS256, PS384, PS512, RS256, RS384, RS512, + }; let mut validation = jsonwebtoken::Validation::new(alg); validation.leeway = CLOCK_SKEW_LEEWAY_SECONDS; @@ -305,7 +311,10 @@ fn build_decoding_key_and_validation( let expected_crv = match alg { ES256 => "P-256", ES384 => "P-384", - other => { + // The outer match arm constrains `alg` to ES256|ES384, + // so this branch is unreachable; kept as a defensive guard. + other @ (HS256 | HS384 | HS512 | RS256 | RS384 | RS512 | PS256 | PS384 | PS512 + | EdDSA) => { return Err(JwksValidationError::new(format!( "unexpected EC algorithm: {other:?}" ))); @@ -416,6 +425,10 @@ impl JwksValidator for RemoteJwksValidator { #[cfg(test)] mod tests { + #![expect( + clippy::unwrap_used, + reason = "tests do not need to meet production lint standards" + )] use super::{JwksValidationError, RemoteJwksValidator}; use crate::oidc::OpenIdConfiguration; use url::Url; diff --git a/src/oidc/claims/email.rs b/src/oidc/claims/email.rs index c289a6d..c2cab0e 100644 --- a/src/oidc/claims/email.rs +++ b/src/oidc/claims/email.rs @@ -116,6 +116,10 @@ impl<'de> Deserialize<'de> for Email { #[cfg(test)] mod tests { + #![expect( + clippy::unwrap_used, + reason = "tests do not need to meet production lint standards" + )] use super::Email; #[test] diff --git a/src/oidc/claims/issuer.rs b/src/oidc/claims/issuer.rs index 50db6f5..bf2a5c4 100644 --- a/src/oidc/claims/issuer.rs +++ b/src/oidc/claims/issuer.rs @@ -98,6 +98,10 @@ impl<'de> Deserialize<'de> for Issuer { #[cfg(test)] mod tests { + #![expect( + clippy::unwrap_used, + reason = "tests do not need to meet production lint standards" + )] use super::Issuer; use url::Url; diff --git a/src/oidc/claims/mod.rs b/src/oidc/claims/mod.rs index 47a37da..e89f375 100644 --- a/src/oidc/claims/mod.rs +++ b/src/oidc/claims/mod.rs @@ -201,6 +201,7 @@ mod tests { #![expect( clippy::indexing_slicing, clippy::expect_used, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] use super::Claims; diff --git a/src/oidc/claims/picture_url.rs b/src/oidc/claims/picture_url.rs index b15fd44..4c7dd5f 100644 --- a/src/oidc/claims/picture_url.rs +++ b/src/oidc/claims/picture_url.rs @@ -87,6 +87,10 @@ impl<'de> Deserialize<'de> for PictureUrl { #[cfg(test)] mod tests { + #![expect( + clippy::unwrap_used, + reason = "tests do not need to meet production lint standards" + )] use super::PictureUrl; #[test] diff --git a/src/pages.rs b/src/pages.rs index 6984f54..7e6682f 100644 --- a/src/pages.rs +++ b/src/pages.rs @@ -296,25 +296,23 @@ impl ErrorPageRenderer for DefaultErrorPageRenderer { async fn render_error(&self, ctx: &ErrorPageContext<'_>) -> String { let mut content = String::new(); - match ctx.error() { - crate::error::AuthError::Callback(crate::error::CallbackError::ProviderError { - error, - description, - }) => { - let _ = write!( - content, - "

{}

{}

", - html_escape(description), - html_escape(error) - ); - } - other => { - let _ = write!( - content, - "

{}

", - html_escape(&other.to_string()) - ); - } + if let crate::error::AuthError::Callback(crate::error::CallbackError::ProviderError { + error, + description, + }) = ctx.error() + { + let _ = write!( + content, + "

{}

{}

", + html_escape(description), + html_escape(error) + ); + } else { + let _ = write!( + content, + "

{}

", + html_escape(&ctx.error().to_string()) + ); } if !ctx.scopes().is_empty() { @@ -370,7 +368,7 @@ fn html_escape(s: &str) -> String { #[cfg(test)] mod tests { #![expect( - clippy::panic, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] use super::{ @@ -451,10 +449,11 @@ mod tests { ctx.redirect_uri().as_str(), "http://127.0.0.1:8080/callback" ); - match ctx.error() { - AuthError::Timeout => {} - other => panic!("expected Timeout, got {other:?}"), - } + assert!( + matches!(ctx.error(), AuthError::Timeout), + "expected Timeout, got {:?}", + ctx.error() + ); } #[tokio::test] diff --git a/src/scope.rs b/src/scope.rs index af5f0bf..720bfac 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -149,6 +149,10 @@ impl From<&str> for RequestScope { #[cfg(test)] mod tests { + #![expect( + clippy::unwrap_used, + reason = "tests do not need to meet production lint standards" + )] use super::{OAuth2Scope, RequestScope}; #[test] diff --git a/src/test_support.rs b/src/test_support.rs index cdb7ab5..1955390 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -1,6 +1,7 @@ //! Test support utilities for doctests and unit tests. #![expect( clippy::pedantic, + clippy::unwrap_used, reason = "test support code does not need to meet production lint standards" )] diff --git a/src/token/mod.rs b/src/token/mod.rs index d12b9ea..6b4cf1f 100644 --- a/src/token/mod.rs +++ b/src/token/mod.rs @@ -264,6 +264,7 @@ mod tests { #![expect( clippy::indexing_slicing, clippy::expect_used, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] use super::{AccessToken, RefreshToken, TokenSet, Unvalidated, Validated}; diff --git a/tests/auth_flow.rs b/tests/auth_flow.rs index a356ba9..6257d8c 100644 --- a/tests/auth_flow.rs +++ b/tests/auth_flow.rs @@ -2,6 +2,7 @@ clippy::panic, clippy::indexing_slicing, clippy::expect_used, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] use loopauth::{ diff --git a/tests/jwks_flow.rs b/tests/jwks_flow.rs index 01649c6..de019d2 100644 --- a/tests/jwks_flow.rs +++ b/tests/jwks_flow.rs @@ -1,6 +1,7 @@ #![expect( clippy::panic, clippy::expect_used, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] use async_trait::async_trait; diff --git a/tests/oidc_discovery.rs b/tests/oidc_discovery.rs index 00f0d87..57646d4 100644 --- a/tests/oidc_discovery.rs +++ b/tests/oidc_discovery.rs @@ -1,5 +1,6 @@ #![expect( clippy::panic, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] use axum::{Json, Router, routing::get}; diff --git a/tests/oidc_flow.rs b/tests/oidc_flow.rs index 208ec7c..836d7e4 100644 --- a/tests/oidc_flow.rs +++ b/tests/oidc_flow.rs @@ -1,5 +1,6 @@ #![expect( clippy::expect_used, + clippy::unwrap_used, reason = "tests do not need to meet production lint standards" )] use loopauth::{