From 78933acb3c45b6ad47aa58fc33b7acbf63c3b079 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Mon, 18 Aug 2025 10:29:30 +0200 Subject: [PATCH] Collapse a few nested `if` now that we have `if let` chains --- crates/cli/build.rs | 14 ++--- crates/cli/src/app_state.rs | 8 +-- crates/cli/src/commands/manage.rs | 13 ++--- crates/cli/src/sync.rs | 10 ++-- crates/config/src/sections/telemetry.rs | 48 ++++++++-------- crates/context/src/fmt.rs | 48 ++++++++-------- crates/data-model/src/user_agent.rs | 55 +++++++++---------- crates/data-model/src/users.rs | 16 +++--- crates/handlers/src/admin/call_context.rs | 8 +-- crates/handlers/src/lib.rs | 14 ++--- crates/handlers/src/oauth2/introspection.rs | 8 +-- crates/handlers/src/oauth2/registration.rs | 40 +++++++------- .../handlers/src/upstream_oauth2/authorize.rs | 20 +++---- crates/handlers/src/views/logout.rs | 14 ++--- crates/i18n-scan/src/minijinja.rs | 52 +++++++++--------- crates/listener/src/server.rs | 10 ++-- crates/oauth2-types/src/oidc.rs | 8 +-- .../syn2mas/src/synapse_reader/config/mod.rs | 16 +++--- 18 files changed, 199 insertions(+), 203 deletions(-) diff --git a/crates/cli/build.rs b/crates/cli/build.rs index 7bad5d337..fd111273b 100644 --- a/crates/cli/build.rs +++ b/crates/cli/build.rs @@ -12,13 +12,13 @@ fn main() -> anyhow::Result<()> { // At build time, we override the version through the environment variable // VERGEN_GIT_DESCRIBE. In some contexts, it means this variable is set but // empty, so we unset it here. - if let Ok(ver) = std::env::var("VERGEN_GIT_DESCRIBE") { - if ver.is_empty() { - #[allow(unsafe_code)] - // SAFETY: This is safe because the build script is running a single thread - unsafe { - std::env::remove_var("VERGEN_GIT_DESCRIBE"); - } + if let Ok(ver) = std::env::var("VERGEN_GIT_DESCRIBE") + && ver.is_empty() + { + #[allow(unsafe_code)] + // SAFETY: This is safe because the build script is running a single thread + unsafe { + std::env::remove_var("VERGEN_GIT_DESCRIBE"); } } diff --git a/crates/cli/src/app_state.rs b/crates/cli/src/app_state.rs index dd7dd6c99..40ae94806 100644 --- a/crates/cli/src/app_state.rs +++ b/crates/cli/src/app_state.rs @@ -275,10 +275,10 @@ fn infer_client_ip( let peer = if let Some(info) = connection_info { // We can always trust the proxy protocol to give us the correct IP address - if let Some(proxy) = info.get_proxy_ref() { - if let Some(source) = proxy.source() { - return Some(source.ip()); - } + if let Some(proxy) = info.get_proxy_ref() + && let Some(source) = proxy.source() + { + return Some(source.ip()); } info.get_peer_addr().map(|addr| addr.ip()) diff --git a/crates/cli/src/commands/manage.rs b/crates/cli/src/commands/manage.rs index e991c1716..cedfcee2b 100644 --- a/crates/cli/src/commands/manage.rs +++ b/crates/cli/src/commands/manage.rs @@ -619,13 +619,12 @@ impl Options { let txn = conn.begin().await?; let mut repo = PgRepository::from_conn(txn); - if let Some(password) = &password { - if !ignore_password_complexity - && !password_manager.is_password_complex_enough(password)? - { - error!("That password is too weak."); - return Ok(ExitCode::from(1)); - } + if let Some(password) = &password + && !ignore_password_complexity + && !password_manager.is_password_complex_enough(password)? + { + error!("That password is too weak."); + return Ok(ExitCode::from(1)); } // If the username is provided, check if it's available and normalize it. diff --git a/crates/cli/src/sync.rs b/crates/cli/src/sync.rs index 700b8eea8..965a4fc95 100644 --- a/crates/cli/src/sync.rs +++ b/crates/cli/src/sync.rs @@ -208,11 +208,11 @@ pub async fn config_sync( // private key to hold the content of the private key file. // private key (raw) takes precedence so both can be defined // without issues - if siwa.private_key.is_none() { - if let Some(private_key_file) = siwa.private_key_file.take() { - let key = tokio::fs::read_to_string(private_key_file).await?; - siwa.private_key = Some(key); - } + if siwa.private_key.is_none() + && let Some(private_key_file) = siwa.private_key_file.take() + { + let key = tokio::fs::read_to_string(private_key_file).await?; + siwa.private_key = Some(key); } let encoded = serde_json::to_vec(&siwa)?; Some(encrypter.encrypt_to_string(&encoded)?) diff --git a/crates/config/src/sections/telemetry.rs b/crates/config/src/sections/telemetry.rs index 8e2d995e9..44e75a354 100644 --- a/crates/config/src/sections/telemetry.rs +++ b/crates/config/src/sections/telemetry.rs @@ -198,34 +198,34 @@ impl ConfigurationSection for TelemetryConfig { &self, _figment: &figment::Figment, ) -> Result<(), Box> { - if let Some(sample_rate) = self.sentry.sample_rate { - if !(0.0..=1.0).contains(&sample_rate) { - return Err(figment::error::Error::custom( - "Sentry sample rate must be between 0.0 and 1.0", - ) - .with_path("sentry.sample_rate") - .into()); - } + if let Some(sample_rate) = self.sentry.sample_rate + && !(0.0..=1.0).contains(&sample_rate) + { + return Err(figment::error::Error::custom( + "Sentry sample rate must be between 0.0 and 1.0", + ) + .with_path("sentry.sample_rate") + .into()); } - if let Some(sample_rate) = self.sentry.traces_sample_rate { - if !(0.0..=1.0).contains(&sample_rate) { - return Err(figment::error::Error::custom( - "Sentry sample rate must be between 0.0 and 1.0", - ) - .with_path("sentry.traces_sample_rate") - .into()); - } + if let Some(sample_rate) = self.sentry.traces_sample_rate + && !(0.0..=1.0).contains(&sample_rate) + { + return Err(figment::error::Error::custom( + "Sentry sample rate must be between 0.0 and 1.0", + ) + .with_path("sentry.traces_sample_rate") + .into()); } - if let Some(sample_rate) = self.tracing.sample_rate { - if !(0.0..=1.0).contains(&sample_rate) { - return Err(figment::error::Error::custom( - "Tracing sample rate must be between 0.0 and 1.0", - ) - .with_path("tracing.sample_rate") - .into()); - } + if let Some(sample_rate) = self.tracing.sample_rate + && !(0.0..=1.0).contains(&sample_rate) + { + return Err(figment::error::Error::custom( + "Tracing sample rate must be between 0.0 and 1.0", + ) + .with_path("tracing.sample_rate") + .into()); } Ok(()) diff --git a/crates/context/src/fmt.rs b/crates/context/src/fmt.rs index 579ea63a9..25908a2ca 100644 --- a/crates/context/src/fmt.rs +++ b/crates/context/src/fmt.rs @@ -129,31 +129,31 @@ where field_fromatter.format_fields(writer.by_ref(), event)?; // If we have a OTEL span, we can add the trace ID to the end of the log line - if let Some(span) = ctx.lookup_current() { - if let Some(otel) = span.extensions().get::() { - let parent_cx_span = otel.parent_cx.span(); - let sc = parent_cx_span.span_context(); + if let Some(span) = ctx.lookup_current() + && let Some(otel) = span.extensions().get::() + { + let parent_cx_span = otel.parent_cx.span(); + let sc = parent_cx_span.span_context(); - // Check if the span is sampled, first from the span builder, - // then from the parent context if nothing is set there - if otel - .builder - .sampling_result - .as_ref() - .map_or(sc.is_sampled(), |r| { - r.decision == SamplingDecision::RecordAndSample - }) - { - // If it is the root span, the trace ID will be in the span builder. Else, it - // will be in the parent OTEL context - let trace_id = otel.builder.trace_id.unwrap_or(sc.trace_id()); - if trace_id != TraceId::INVALID { - let label = Style::new() - .italic() - .force_styling(ansi) - .apply_to("trace.id"); - write!(&mut writer, " {label}={trace_id}")?; - } + // Check if the span is sampled, first from the span builder, + // then from the parent context if nothing is set there + if otel + .builder + .sampling_result + .as_ref() + .map_or(sc.is_sampled(), |r| { + r.decision == SamplingDecision::RecordAndSample + }) + { + // If it is the root span, the trace ID will be in the span builder. Else, it + // will be in the parent OTEL context + let trace_id = otel.builder.trace_id.unwrap_or(sc.trace_id()); + if trace_id != TraceId::INVALID { + let label = Style::new() + .italic() + .force_styling(ansi) + .apply_to("trace.id"); + write!(&mut writer, " {label}={trace_id}")?; } } } diff --git a/crates/data-model/src/user_agent.rs b/crates/data-model/src/user_agent.rs index 4efb4ff9f..d0e930586 100644 --- a/crates/data-model/src/user_agent.rs +++ b/crates/data-model/src/user_agent.rs @@ -88,32 +88,31 @@ impl UserAgent { #[must_use] pub fn parse(user_agent: String) -> Self { - if !user_agent.contains("Mozilla/") { - if let Some((name, version, model, os, os_version)) = + if !user_agent.contains("Mozilla/") + && let Some((name, version, model, os, os_version)) = UserAgent::parse_custom(&user_agent) - { - let mut device_type = DeviceType::Unknown; + { + let mut device_type = DeviceType::Unknown; - // Handle mobile simple mobile devices - if os == "Android" || os == "iOS" { - device_type = DeviceType::Mobile; - } - - // Handle iPads - if model.contains("iPad") { - device_type = DeviceType::Tablet; - } - - return Self { - name: Some(name.to_owned()), - version: Some(version.to_owned()), - os: Some(os.to_owned()), - os_version: os_version.map(std::borrow::ToOwned::to_owned), - model: Some(model.to_owned()), - device_type, - raw: user_agent, - }; + // Handle mobile simple mobile devices + if os == "Android" || os == "iOS" { + device_type = DeviceType::Mobile; } + + // Handle iPads + if model.contains("iPad") { + device_type = DeviceType::Tablet; + } + + return Self { + name: Some(name.to_owned()), + version: Some(version.to_owned()), + os: Some(os.to_owned()), + os_version: os_version.map(std::borrow::ToOwned::to_owned), + model: Some(model.to_owned()), + device_type, + raw: user_agent, + }; } let mut model = None; @@ -205,11 +204,11 @@ impl UserAgent { } // Special handling for Electron applications e.g. Element Desktop - if user_agent.contains("Electron/") { - if let Some(app) = UserAgent::parse_electron(&user_agent) { - result.name = app.0; - result.version = app.1; - } + if user_agent.contains("Electron/") + && let Some(app) = UserAgent::parse_electron(&user_agent) + { + result.name = app.0; + result.version = app.1; } Self { diff --git a/crates/data-model/src/users.rs b/crates/data-model/src/users.rs index c2addf8a3..920726ef8 100644 --- a/crates/data-model/src/users.rs +++ b/crates/data-model/src/users.rs @@ -223,17 +223,17 @@ impl UserRegistrationToken { } // Check if expired - if let Some(expires_at) = self.expires_at { - if now >= expires_at { - return false; - } + if let Some(expires_at) = self.expires_at + && now >= expires_at + { + return false; } // Check if usage limit exceeded - if let Some(usage_limit) = self.usage_limit { - if self.times_used >= usage_limit { - return false; - } + if let Some(usage_limit) = self.usage_limit + && self.times_used >= usage_limit + { + return false; } true diff --git a/crates/handlers/src/admin/call_context.rs b/crates/handlers/src/admin/call_context.rs index d5a5f4b94..a0470f851 100644 --- a/crates/handlers/src/admin/call_context.rs +++ b/crates/handlers/src/admin/call_context.rs @@ -187,10 +187,10 @@ where }; // If there is a user for this session, check that it is not locked - if let Some(user) = &user { - if !user.is_valid() { - return Err(Rejection::UserLocked); - } + if let Some(user) = &user + && !user.is_valid() + { + return Err(Rejection::UserLocked); } if !session.is_valid() { diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index 08367bbf5..8de6707be 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -475,13 +475,13 @@ fn recover_error( ) -> axum::response::Response { // Error responses should have an ErrorContext attached to them let ext = response.extensions().get::(); - if let Some(ctx) = ext { - if let Ok(res) = templates.render_error(ctx) { - let (mut parts, _original_body) = response.into_parts(); - parts.headers.remove(CONTENT_TYPE); - parts.headers.remove(CONTENT_LENGTH); - return (parts, Html(res)).into_response(); - } + if let Some(ctx) = ext + && let Ok(res) = templates.render_error(ctx) + { + let (mut parts, _original_body) = response.into_parts(); + parts.headers.remove(CONTENT_TYPE); + parts.headers.remove(CONTENT_LENGTH); + return (parts, Html(res)).into_response(); } response diff --git a/crates/handlers/src/oauth2/introspection.rs b/crates/handlers/src/oauth2/introspection.rs index 678c7fa1c..e52e3c303 100644 --- a/crates/handlers/src/oauth2/introspection.rs +++ b/crates/handlers/src/oauth2/introspection.rs @@ -288,10 +288,10 @@ pub(crate) async fn post( let token = &form.token; let token_type = TokenType::check(token)?; - if let Some(hint) = form.token_type_hint { - if token_type != hint { - return Err(RouteError::UnexpectedTokenType); - } + if let Some(hint) = form.token_type_hint + && token_type != hint + { + return Err(RouteError::UnexpectedTokenType); } // Not all device IDs can be encoded as scope. On OAuth 2.0 sessions, we diff --git a/crates/handlers/src/oauth2/registration.rs b/crates/handlers/src/oauth2/registration.rs index 09ace7351..0cdfb32ea 100644 --- a/crates/handlers/src/oauth2/registration.rs +++ b/crates/handlers/src/oauth2/registration.rs @@ -241,34 +241,34 @@ pub(crate) async fn post( // Some extra validation that is hard to do in OPA and not done by the // `validate` method either - if let Some(client_uri) = &metadata.client_uri { - if localised_url_has_public_suffix(client_uri) { - return Err(RouteError::UrlIsPublicSuffix("client_uri")); - } + if let Some(client_uri) = &metadata.client_uri + && localised_url_has_public_suffix(client_uri) + { + return Err(RouteError::UrlIsPublicSuffix("client_uri")); } - if let Some(logo_uri) = &metadata.logo_uri { - if localised_url_has_public_suffix(logo_uri) { - return Err(RouteError::UrlIsPublicSuffix("logo_uri")); - } + if let Some(logo_uri) = &metadata.logo_uri + && localised_url_has_public_suffix(logo_uri) + { + return Err(RouteError::UrlIsPublicSuffix("logo_uri")); } - if let Some(policy_uri) = &metadata.policy_uri { - if localised_url_has_public_suffix(policy_uri) { - return Err(RouteError::UrlIsPublicSuffix("policy_uri")); - } + if let Some(policy_uri) = &metadata.policy_uri + && localised_url_has_public_suffix(policy_uri) + { + return Err(RouteError::UrlIsPublicSuffix("policy_uri")); } - if let Some(tos_uri) = &metadata.tos_uri { - if localised_url_has_public_suffix(tos_uri) { - return Err(RouteError::UrlIsPublicSuffix("tos_uri")); - } + if let Some(tos_uri) = &metadata.tos_uri + && localised_url_has_public_suffix(tos_uri) + { + return Err(RouteError::UrlIsPublicSuffix("tos_uri")); } - if let Some(initiate_login_uri) = &metadata.initiate_login_uri { - if host_is_public_suffix(initiate_login_uri) { - return Err(RouteError::UrlIsPublicSuffix("initiate_login_uri")); - } + if let Some(initiate_login_uri) = &metadata.initiate_login_uri + && host_is_public_suffix(initiate_login_uri) + { + return Err(RouteError::UrlIsPublicSuffix("initiate_login_uri")); } for redirect_uri in metadata.redirect_uris() { diff --git a/crates/handlers/src/upstream_oauth2/authorize.rs b/crates/handlers/src/upstream_oauth2/authorize.rs index 016dd36d3..c9e96f936 100644 --- a/crates/handlers/src/upstream_oauth2/authorize.rs +++ b/crates/handlers/src/upstream_oauth2/authorize.rs @@ -93,17 +93,15 @@ pub(crate) async fn get( // Forward the raw login hint upstream for the provider to handle however it // sees fit - if provider.forward_login_hint { - if let Some(PostAuthAction::ContinueAuthorizationGrant { id }) = &query.post_auth_action { - if let Some(login_hint) = repo - .oauth2_authorization_grant() - .lookup(*id) - .await? - .and_then(|grant| grant.login_hint) - { - data = data.with_login_hint(login_hint); - } - } + if provider.forward_login_hint + && let Some(PostAuthAction::ContinueAuthorizationGrant { id }) = &query.post_auth_action + && let Some(login_hint) = repo + .oauth2_authorization_grant() + .lookup(*id) + .await? + .and_then(|grant| grant.login_hint) + { + data = data.with_login_hint(login_hint); } let data = if let Some(methods) = lazy_metadata.pkce_methods().await? { diff --git a/crates/handlers/src/views/logout.rs b/crates/handlers/src/views/logout.rs index 731071c8b..9d0c3ec5a 100644 --- a/crates/handlers/src/views/logout.rs +++ b/crates/handlers/src/views/logout.rs @@ -33,14 +33,14 @@ pub(crate) async fn post( if let Some(session_id) = session_info.current_session_id() { let maybe_session = repo.browser_session().lookup(session_id).await?; - if let Some(session) = maybe_session { - if session.finished_at.is_none() { - activity_tracker - .record_browser_session(&clock, &session) - .await; + if let Some(session) = maybe_session + && session.finished_at.is_none() + { + activity_tracker + .record_browser_session(&clock, &session) + .await; - repo.browser_session().finish(&clock, session).await?; - } + repo.browser_session().finish(&clock, session).await?; } } diff --git a/crates/i18n-scan/src/minijinja.rs b/crates/i18n-scan/src/minijinja.rs index b6e9dda51..63aa63aea 100644 --- a/crates/i18n-scan/src/minijinja.rs +++ b/crates/i18n-scan/src/minijinja.rs @@ -110,36 +110,36 @@ fn find_in_call<'a>( call: &'a Spanned>, ) -> Result<(), minijinja::Error> { let span = call.span(); - if let Expr::Var(var_) = &call.expr { - if var_.id == context.func() { - let key = call - .args - .first() - .and_then(as_const) - .and_then(|const_| const_.value.as_str()) - .ok_or(minijinja::Error::new( - ErrorKind::UndefinedError, - "t() first argument must be a string literal", - ))?; + if let Expr::Var(var_) = &call.expr + && var_.id == context.func() + { + let key = call + .args + .first() + .and_then(as_const) + .and_then(|const_| const_.value.as_str()) + .ok_or(minijinja::Error::new( + ErrorKind::UndefinedError, + "t() first argument must be a string literal", + ))?; - let has_count = call - .args - .iter() - .any(|arg| matches!(arg, CallArg::Kwarg("count", _))); + let has_count = call + .args + .iter() + .any(|arg| matches!(arg, CallArg::Kwarg("count", _))); - let key = Key::new( - if has_count { - crate::key::Kind::Plural - } else { - crate::key::Kind::Message - }, - key.to_owned(), - ); + let key = Key::new( + if has_count { + crate::key::Kind::Plural + } else { + crate::key::Kind::Message + }, + key.to_owned(), + ); - let key = context.set_key_location(key, span); + let key = context.set_key_location(key, span); - context.record(key); - } + context.record(key); } find_in_expr(context, &call.expr)?; diff --git a/crates/listener/src/server.rs b/crates/listener/src/server.rs index 026706a45..2a0b6ccda 100644 --- a/crates/listener/src/server.rs +++ b/crates/listener/src/server.rs @@ -279,11 +279,11 @@ where fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let mut this = self.project(); - if let Poll::Ready(()) = this.cancellation_future.poll(cx) { - if !*this.did_start_shutdown { - *this.did_start_shutdown = true; - this.connection.as_mut().graceful_shutdown(); - } + if let Poll::Ready(()) = this.cancellation_future.poll(cx) + && !*this.did_start_shutdown + { + *this.did_start_shutdown = true; + this.connection.as_mut().graceful_shutdown(); } this.connection.poll(cx) diff --git a/crates/oauth2-types/src/oidc.rs b/crates/oauth2-types/src/oidc.rs index a9befbea1..7ff1270e6 100644 --- a/crates/oauth2-types/src/oidc.rs +++ b/crates/oauth2-types/src/oidc.rs @@ -680,10 +680,10 @@ impl ProviderMetadata { validate_url("registration_endpoint", url, ExtraUrlRestrictions::None)?; } - if let Some(scopes) = &metadata.scopes_supported { - if !scopes.iter().any(|s| s == "openid") { - return Err(ProviderMetadataVerificationError::ScopesMissingOpenid); - } + if let Some(scopes) = &metadata.scopes_supported + && !scopes.iter().any(|s| s == "openid") + { + return Err(ProviderMetadataVerificationError::ScopesMissingOpenid); } validate_signing_alg_values_supported( diff --git a/crates/syn2mas/src/synapse_reader/config/mod.rs b/crates/syn2mas/src/synapse_reader/config/mod.rs index 4bd89921b..3c9454ba6 100644 --- a/crates/syn2mas/src/synapse_reader/config/mod.rs +++ b/crates/syn2mas/src/synapse_reader/config/mod.rs @@ -120,14 +120,14 @@ impl Config { pub fn all_oidc_providers(&self) -> BTreeMap { let mut out = BTreeMap::new(); - if let Some(provider) = &self.oidc_config { - if provider.has_required_fields() { - let mut provider = provider.clone(); - // The legacy configuration has an implied IdP ID of `oidc`. - let idp_id = provider.idp_id.take().unwrap_or("oidc".to_owned()); - provider.idp_id = Some(idp_id.clone()); - out.insert(idp_id, provider); - } + if let Some(provider) = &self.oidc_config + && provider.has_required_fields() + { + let mut provider = provider.clone(); + // The legacy configuration has an implied IdP ID of `oidc`. + let idp_id = provider.idp_id.take().unwrap_or("oidc".to_owned()); + provider.idp_id = Some(idp_id.clone()); + out.insert(idp_id, provider); } for provider in &self.oidc_providers {