From 73c3434169a598ea4af7497f700888aa41efc5cd Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Fri, 10 Jul 2026 15:52:24 -0500 Subject: [PATCH] ci: Fix all clippy warnings and force check in CI --- .github/workflows/main.yml | 2 +- credentialsd-common/src/memfd.rs | 4 +-- credentialsd-common/src/model.rs | 5 ++- credentialsd-ui/src/dbus.rs | 6 ++-- credentialsd-ui/src/gui/mod.rs | 8 ++--- credentialsd-ui/src/gui/view_model/gtk/mod.rs | 8 ++--- .../src/gui/view_model/gtk/window.rs | 15 ++++---- credentialsd/src/credential_service/nfc.rs | 2 +- credentialsd/src/credential_service/usb.rs | 2 +- credentialsd/src/dbus/flow_control.rs | 8 ++--- credentialsd/src/dbus/ui_control.rs | 6 +++- credentialsd/src/gateway/dbus.rs | 22 ++++-------- credentialsd/src/gateway/mod.rs | 6 ++-- credentialsd/src/webauthn.rs | 36 +++++++++---------- 14 files changed, 64 insertions(+), 66 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 301078e..84bb684 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -85,7 +85,7 @@ jobs: working-directory: build/ - name: Check clippy recommendations - run: cargo clippy --locked --workspace --all-features --target-dir build/target + run: cargo clippy --locked --workspace --all-features --target-dir build/target -- -Dwarnings - name: Check formatting run: cargo fmt --all --check diff --git a/credentialsd-common/src/memfd.rs b/credentialsd-common/src/memfd.rs index 04060d4..49e22ab 100644 --- a/credentialsd-common/src/memfd.rs +++ b/credentialsd-common/src/memfd.rs @@ -76,12 +76,12 @@ impl Mmap { .ok_or_else(|| io::Error::other("mmap returned NULL pointer"))? }; - return Ok(Self { + Ok(Self { inner: ptr, fd, size, pos: 0, - }); + }) } fn into_fd(self) -> OwnedFd { diff --git a/credentialsd-common/src/model.rs b/credentialsd-common/src/model.rs index 437697a..260aeec 100644 --- a/credentialsd-common/src/model.rs +++ b/credentialsd-common/src/model.rs @@ -201,9 +201,8 @@ impl TryFrom<&Structure<'_>> for BackgroundEvent { .iter() .map(|v| v.try_to_owned().unwrap()) .map(|v| { - let cred: Result = Value::from(v) - .downcast::() - .map(Credential::from); + let cred: Result = + Value::from(v).downcast::(); cred }) .collect(); diff --git a/credentialsd-ui/src/dbus.rs b/credentialsd-ui/src/dbus.rs index de36384..b7e783f 100644 --- a/credentialsd-ui/src/dbus.rs +++ b/credentialsd-ui/src/dbus.rs @@ -46,6 +46,8 @@ pub(crate) struct UiContext { /// These methods are called by the credential service to control the UI. #[interface(name = "org.freedesktop.impl.portal.experimental.Credential")] impl CredentialPortalBackend { + // D-Bus has long argument signatures. + #[expect(clippy::too_many_arguments)] async fn initialize( &self, #[zbus(connection)] connection: &Connection, @@ -277,7 +279,7 @@ impl CeremonyObject { pid: self.ui_context.app_pid, }, initial_devices: self.ui_context.devices.clone(), - window_handle: self.ui_context.parent_window.clone().into(), + window_handle: self.ui_context.parent_window.clone(), }, Arc::new(AsyncMutex::new(flow_control_client)), cancel_rx, @@ -302,7 +304,7 @@ impl CeremonyObject { } else { tracing::error!("Flow was not properly initialized before receiving events."); } - return Err(fdo::Error::Failed("Failed to handle event".to_string())); + Err(fdo::Error::Failed("Failed to handle event".to_string())) } #[zbus(signal)] diff --git a/credentialsd-ui/src/gui/mod.rs b/credentialsd-ui/src/gui/mod.rs index f716817..0659204 100644 --- a/credentialsd-ui/src/gui/mod.rs +++ b/credentialsd-ui/src/gui/mod.rs @@ -43,10 +43,10 @@ fn run_gui( let (tx_event, rx_event) = async_std::channel::unbounded::(); let tx_event2 = tx_event.clone(); let cancel_task = async_std::task::spawn(async move { - if let Ok(_) = cancel_rx.recv().await { - if tx_event2.send(ViewEvent::UserCancelled).await.is_err() { - tracing::error!("Failed to send cancellation to view model"); - } + if let Ok(_) = cancel_rx.recv().await + && tx_event2.send(ViewEvent::UserCancelled).await.is_err() + { + tracing::error!("Failed to send cancellation to view model"); } }); let event_loop = async_std::task::spawn(async move { diff --git a/credentialsd-ui/src/gui/view_model/gtk/mod.rs b/credentialsd-ui/src/gui/view_model/gtk/mod.rs index e8c9017..927010b 100644 --- a/credentialsd-ui/src/gui/view_model/gtk/mod.rs +++ b/credentialsd-ui/src/gui/view_model/gtk/mod.rs @@ -158,7 +158,7 @@ impl ViewModel { let localized = ngettext( "Enter your PIN. One attempt remaining.", "Enter your PIN. %d attempts remaining.", - left.into(), + left, ); localized.replace("%d", &format!("{}", left)) } else { @@ -173,7 +173,7 @@ impl ViewModel { let localized = ngettext( "Touch your device again. One attempt remaining.", "Touch your device again. %d attempts remaining.", - left.into(), + left, ); localized.replace("%d", &format!("{}", left)) } @@ -192,7 +192,7 @@ impl ViewModel { } ViewUpdate::HybridConnecting => { view_model.set_qr_code_visible(false); - _ = view_model.qr_code_paintable().take(); + _ = view_model.qr_code_paintable(); view_model.waiting_for_device(&Device { id: "x".to_string(), transport: Transport::HybridQr, @@ -204,7 +204,7 @@ impl ViewModel { } ViewUpdate::HybridConnected => { view_model.set_qr_code_visible(false); - _ = view_model.qr_code_paintable().take(); + _ = view_model.qr_code_paintable(); view_model.set_prompt(gettext( "Device connected. Follow the instructions on your device", )); diff --git a/credentialsd-ui/src/gui/view_model/gtk/window.rs b/credentialsd-ui/src/gui/view_model/gtk/window.rs index 6a33e29..2a8aca3 100644 --- a/credentialsd-ui/src/gui/view_model/gtk/window.rs +++ b/credentialsd-ui/src/gui/view_model/gtk/window.rs @@ -110,17 +110,16 @@ mod imp { impl WindowImpl for CredentialsUiWindow { // Save window state on delete event fn close_request(&self) -> glib::Propagation { - if let Some(vm) = self.view_model.borrow().as_ref() { - if vm + if let Some(vm) = self.view_model.borrow().as_ref() + && vm .get_sender() .send_blocking(ViewEvent::UserCancelled) .is_err() - { - tracing::warn!( - "Failed to notify the backend service that the user cancelled the request." - ); - }; - } + { + tracing::warn!( + "Failed to notify the backend service that the user cancelled the request." + ); + }; // Pass close request on to the parent self.parent_close_request() diff --git a/credentialsd/src/credential_service/nfc.rs b/credentialsd/src/credential_service/nfc.rs index 0d4d15d..219d572 100644 --- a/credentialsd/src/credential_service/nfc.rs +++ b/credentialsd/src/credential_service/nfc.rs @@ -450,7 +450,7 @@ impl From<&NfcState> for BackgroundEvent { } } NfcState::SelectingCredential { creds, .. } => BackgroundEvent::SelectingCredential { - creds: creds.to_owned().into_iter().map(|c| c.into()).collect(), + creds: creds.to_vec(), }, NfcState::Completed => BackgroundEvent::CeremonyCompleted, NfcState::Failed(Error::AuthenticatorError) => BackgroundEvent::ErrorAuthenticator, diff --git a/credentialsd/src/credential_service/usb.rs b/credentialsd/src/credential_service/usb.rs index 7d5de16..88069c7 100644 --- a/credentialsd/src/credential_service/usb.rs +++ b/credentialsd/src/credential_service/usb.rs @@ -555,7 +555,7 @@ impl From<&UsbState> for BackgroundEvent { } UsbState::NeedsUserPresence => BackgroundEvent::NeedsUserPresence, UsbState::SelectingCredential { creds, .. } => BackgroundEvent::SelectingCredential { - creds: creds.to_owned().into_iter().map(|c| c.into()).collect(), + creds: creds.to_vec(), }, UsbState::Completed => BackgroundEvent::CeremonyCompleted, UsbState::Failed(Error::AuthenticatorError) => BackgroundEvent::ErrorAuthenticator, diff --git a/credentialsd/src/dbus/flow_control.rs b/credentialsd/src/dbus/flow_control.rs index 2d64356..36e4f65 100644 --- a/credentialsd/src/dbus/flow_control.rs +++ b/credentialsd/src/dbus/flow_control.rs @@ -72,7 +72,7 @@ pub async fn start_flow_control_service { let pin_fd = OwnedFd::from(pin_fd); - let pin = match read_secret(pin_fd.into()) + let pin = match read_secret(pin_fd) .map_err(|err| format!("Could not read from file descriptor: {err}")) .and_then(|bytes| { String::from_utf8(bytes).map_err(|err| { @@ -227,8 +227,8 @@ async fn handle, @@ -153,7 +157,7 @@ async fn forward_ui_events( while let Some(signal) = ui_event_stream.next().await { tracing::trace!(?signal, "Received event from UI"); let event = signal.args()?.update; - if let Err(_) = tx.send(event).await { + if tx.send(event).await.is_err() { tracing::trace!("credential service event listener stopped listening for UI events. Ending event stream listener"); break; } diff --git a/credentialsd/src/gateway/dbus.rs b/credentialsd/src/gateway/dbus.rs index 10d9109..2dab562 100644 --- a/credentialsd/src/gateway/dbus.rs +++ b/credentialsd/src/gateway/dbus.rs @@ -55,6 +55,8 @@ struct CredentialPortalGateway { /// The D-Bus interface is responsible for authorizing the client and collecting /// the contextual information about the client to pass onto the GatewayService /// for evaluation. +// D-Bus has long argument signatures. +#[expect(clippy::too_many_arguments)] #[interface(name = "org.freedesktop.handler.portal.experimental.Credential")] impl CredentialPortalGateway { #[zbus(out_args("response", "results"))] @@ -87,7 +89,7 @@ impl CredentialPortalGateway { &header, claimed_app_id, origin.clone(), - top_origin.clone().into(), + top_origin.clone(), ) .await; let context = match app_validation_result { @@ -111,12 +113,7 @@ impl CredentialPortalGateway { .gateway_service .lock() .await - .handle_create_credential( - request, - context, - parent_window.into(), - activation_token.into(), - ) + .handle_create_credential(request, context, parent_window.into(), activation_token) .await .map_err(Error::from); @@ -143,7 +140,7 @@ impl CredentialPortalGateway { &header, claimed_app_id, origin.clone(), - top_origin.clone().into(), + top_origin.clone(), ) .await; @@ -172,12 +169,7 @@ impl CredentialPortalGateway { .gateway_service .lock() .await - .handle_get_credential( - request, - context, - parent_window.into(), - activation_token.into(), - ) + .handle_get_credential(request, context, parent_window.into(), activation_token) .await .map_err(Error::from); response.into() @@ -292,7 +284,7 @@ impl From for Error { #[derive(Serialize)] enum PortalResponse { Success = 0, - Cancelled = 1, + // Cancelled = 1, Other = 2, } diff --git a/credentialsd/src/gateway/mod.rs b/credentialsd/src/gateway/mod.rs index ef15e6f..00c2b6f 100644 --- a/credentialsd/src/gateway/mod.rs +++ b/credentialsd/src/gateway/mod.rs @@ -281,9 +281,9 @@ async fn should_trust_app_id(pid: u32) -> bool { ); if !trusted_callers.as_slice().contains(&exe_path) { tracing::warn!(?exe_path, "Request received from untrusted caller"); - return false; + false } else { - return true; + true } } @@ -426,6 +426,8 @@ impl From for CreateCredentialResponse { } } +// We want to keep these aligned with how the spec names them. +#[expect(clippy::enum_variant_names)] #[derive(Debug)] pub enum WebAuthnError { /// The ceremony was cancelled by an AbortController. See § 5.6 Abort diff --git a/credentialsd/src/webauthn.rs b/credentialsd/src/webauthn.rs index a59ffff..89e69c9 100644 --- a/credentialsd/src/webauthn.rs +++ b/credentialsd/src/webauthn.rs @@ -83,9 +83,9 @@ impl TryFrom<&Origin> for RelyingPartyId { fn try_from(origin: &Origin) -> Result { match origin { Origin::Https { host, .. } => { - RelyingPartyId::try_from(host.as_str()).map_err(|_| OriginParseError::InvalidHost) + RelyingPartyId::try_from(host.as_str()).map_err(|_| OriginParseError::Host) } - Origin::AppId(_) => Err(OriginParseError::InvalidScheme), + Origin::AppId(_) => Err(OriginParseError::Scheme), } } } @@ -103,33 +103,33 @@ impl FromStr for Origin { // begins with a letter match host_candidate.chars().nth(0) { Some(c) if c.is_ascii_alphabetic() => {} - _ => return Err(OriginParseError::InvalidHost), + _ => return Err(OriginParseError::Host), }; // alphanumeric with hyphens and labels separated by dots if !host_candidate .chars() .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '.') { - return Err(OriginParseError::InvalidHost); + return Err(OriginParseError::Host); } // ends with a valid label if host_candidate.ends_with('.') { - return Err(OriginParseError::InvalidHost); + return Err(OriginParseError::Host); } let host = host_candidate.to_ascii_lowercase(); let Ok(port) = port_candidate.map(|p| p.parse()).transpose() else { - return Err(OriginParseError::InvalidPort); + return Err(OriginParseError::Port); }; Ok(Origin::Https { host, port }) } else if let Some(app_id_candidate) = s.strip_prefix("app:") { let app_id = app_id_candidate .parse() - .map_err(|_| OriginParseError::InvalidHost)?; + .map_err(|_| OriginParseError::Host)?; Ok(Origin::AppId(app_id)) } else { - Err(OriginParseError::InvalidScheme) + Err(OriginParseError::Scheme) } } } @@ -147,9 +147,9 @@ pub(crate) enum NavigationContext { #[derive(Debug)] pub(crate) enum OriginParseError { - InvalidScheme, - InvalidHost, - InvalidPort, + Scheme, + Host, + Port, } impl std::error::Error for OriginParseError {} @@ -157,9 +157,9 @@ impl std::error::Error for OriginParseError {} impl Display for OriginParseError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::InvalidScheme => f.write_str("Invalid scheme"), - Self::InvalidHost => f.write_str("Invalid host"), - Self::InvalidPort => f.write_str("Invalid port"), + Self::Scheme => f.write_str("Invalid scheme"), + Self::Host => f.write_str("Invalid host"), + Self::Port => f.write_str("Invalid port"), } } } @@ -179,7 +179,7 @@ mod tests { #[test] fn test_origin_parse_when_http_fails() { let err = "http://example.com".parse::().unwrap_err(); - assert!(matches!(err, OriginParseError::InvalidScheme)); + assert!(matches!(err, OriginParseError::Scheme)); } #[test] @@ -195,19 +195,19 @@ mod tests { #[test] fn test_origin_parse_with_trailing_slash_fails() { let err = "https://example.org/".parse::().unwrap_err(); - assert!(matches!(err, OriginParseError::InvalidHost)); + assert!(matches!(err, OriginParseError::Host)); } #[test] fn test_origin_parse_with_port_and_path_fails() { let err = "https://example.org:8443/".parse::().unwrap_err(); - assert!(matches!(err, OriginParseError::InvalidPort)); + assert!(matches!(err, OriginParseError::Port)); } #[test] fn test_origin_parse_with_invalid_characters_fails() { let err = "https://😭.edu:1234".parse::().unwrap_err(); - assert!(matches!(err, OriginParseError::InvalidHost)); + assert!(matches!(err, OriginParseError::Host)); } #[test]