Adapt the code to Clippy suggestions (#962)

- Fix several issues detected by Clippy.
- Rename agama-dbus-server "locale" to "l10n" (to avoid
`locale/locale`).
- Define some 'types' for D-Bus proxies to make easier to read the code.
- Run Clippy in the continuous integration.

.github/workflows/ci-rust.yml

@@ -78,6 +78,12 @@ jobs:
     - name: Install Rust toolchains
       run: rustup toolchain install stable
 
+    - name: Run clippy linter
+      run: cargo clippy
+
+    - name: Run rustfmt
+      run: cargo fmt --all -- --check
+
     - name: Install cargo-binstall
       uses: taiki-e/install-action@v2
       with:
@@ -89,9 +95,6 @@ jobs:
     - name: Run the tests
       run: cargo tarpaulin --out xml
 
-    - name: Lint formatting
-      run: cargo fmt --all -- --check
-
     # send the code coverage for the Rust part to the coveralls.io
     - name: Coveralls GitHub Action
       uses: coverallsapp/github-action@v2

rust/agama-cli/src/logs.rs

@@ -279,7 +279,7 @@ impl LogItem for LogCmd {
 
 /// Collect existing / requested paths which should already exist in the system.
 /// Turns them into list of log sources
-fn paths_to_log_sources(paths: &Vec<String>, tmp_dir: &TempDir) -> Vec<Box<dyn LogItem>> {
+fn paths_to_log_sources(paths: &[String], tmp_dir: &TempDir) -> Vec<Box<dyn LogItem>> {
     let mut log_sources: Vec<Box<dyn LogItem>> = Vec::new();
 
     for path in paths.iter() {
@@ -293,10 +293,7 @@ fn paths_to_log_sources(paths: &Vec<String>, tmp_dir: &TempDir) -> Vec<Box<dyn L
 }
 
 /// Some info can be collected via particular commands only, turn it into log sources
-fn cmds_to_log_sources(
-    commands: &Vec<(String, String)>,
-    tmp_dir: &TempDir,
-) -> Vec<Box<dyn LogItem>> {
+fn cmds_to_log_sources(commands: &[(String, String)], tmp_dir: &TempDir) -> Vec<Box<dyn LogItem>> {
     let mut log_sources: Vec<Box<dyn LogItem>> = Vec::new();
 
     for cmd in commands.iter() {

rust/agama-dbus-server/src/locale/timezone.rs → rust/agama-dbus-server/src/l10n/timezone.rs

@@ -131,11 +131,7 @@ mod tests {
     fn test_read_timezone_without_country() {
         let mut db = TimezonesDatabase::new();
         db.read("es").unwrap();
-        let timezone = db
-            .entries()
-            .into_iter()
-            .find(|tz| tz.code == "UTC")
-            .unwrap();
+        let timezone = db.entries().iter().find(|tz| tz.code == "UTC").unwrap();
         assert_eq!(timezone.country, None);
     }
 
@@ -145,7 +141,7 @@ mod tests {
         db.read("en").unwrap();
         let timezone = db
             .entries()
-            .into_iter()
+            .iter()
             .find(|tz| tz.code == "Europe/Kiev")
             .unwrap();
         assert_eq!(timezone.country, Some("Ukraine".to_string()));

rust/agama-dbus-server/src/lib.rs

@@ -1,4 +1,4 @@
 pub mod error;
-pub mod locale;
+pub mod l10n;
 pub mod network;
 pub mod questions;

rust/agama-dbus-server/src/main.rs

@@ -1,4 +1,7 @@
-use agama_dbus_server::{locale, locale::helpers, network, questions};
+use agama_dbus_server::{
+    l10n::{self, helpers},
+    network, questions,
+};
 
 use agama_lib::connection_to;
 use anyhow::Context;
@@ -36,7 +39,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     // When adding more services here, the order might be important.
     questions::export_dbus_objects(&connection).await?;
     log::info!("Started questions interface");
-    locale::export_dbus_objects(&connection, &locale).await?;
+    l10n::export_dbus_objects(&connection, &locale).await?;
     log::info!("Started locale interface");
     network::export_dbus_objects(&connection).await?;
     log::info!("Started network interface");

rust/agama-dbus-server/src/network/dbus/service.rs

@@ -17,7 +17,6 @@ impl NetworkService {
         connection: &Connection,
         adapter: T,
     ) -> Result<(), Box<dyn Error>> {
-        let connection = connection.clone();
         let mut network = NetworkSystem::new(connection.clone(), adapter);
 
         tokio::spawn(async move {

rust/agama-dbus-server/src/network/nm/dbus.rs

@@ -781,10 +781,7 @@ mod test {
         )]));
 
         let dbus_conn = HashMap::from([
-            (
-                "connection".to_string(),
-                connection_section.try_into().unwrap(),
-            ),
+            ("connection".to_string(), connection_section),
             (BOND_KEY.to_string(), bond_options.try_into().unwrap()),
         ]);
 

rust/agama-dbus-server/src/network/nm/proxies.rs

@@ -12,6 +12,7 @@
 //! …consequently `zbus-xmlgen` did not generate code for the above interfaces.
 //! Also some proxies can be used against multiple services when they share interface.
 
+use agama_lib::dbus::OwnedNestedHash;
 use zbus::dbus_proxy;
 
 #[dbus_proxy(
@@ -267,16 +268,7 @@ trait Device {
     fn disconnect(&self) -> zbus::Result<()>;
 
     /// GetAppliedConnection method
-    fn get_applied_connection(
-        &self,
-        flags: u32,
-    ) -> zbus::Result<(
-        std::collections::HashMap<
-            String,
-            std::collections::HashMap<String, zbus::zvariant::OwnedValue>,
-        >,
-        u64,
-    )>;
+    fn get_applied_connection(&self, flags: u32) -> zbus::Result<(OwnedNestedHash, u64)>;
 
     /// Reapply method
     fn reapply(

rust/agama-dbus-server/tests/common/mod.rs

@@ -109,7 +109,7 @@ impl NameOwnerChangedStream {
         loop {
             let signal = self.0.next().await.unwrap().unwrap();
             let (sname, _, _): (String, String, String) = signal.body().unwrap();
-            if &sname == name {
+            if sname == name {
                 return;
             }
         }

rust/agama-dbus-server/tests/network.rs

@@ -102,7 +102,7 @@ async fn test_add_bond_connection() -> Result<(), Box<dyn Error>> {
 
     let adapter = NetworkTestAdapter(NetworkState::default());
 
-    let _service = NetworkService::start(&server.connection(), adapter).await?;
+    NetworkService::start(&server.connection(), adapter).await?;
     server.request_name().await?;
 
     let client = NetworkClient::new(server.connection().clone()).await?;
@@ -128,7 +128,7 @@ async fn test_add_bond_connection() -> Result<(), Box<dyn Error>> {
     let conns = async_retry(|| client.connections()).await?;
     assert_eq!(conns.len(), 2);
 
-    let conn = conns.iter().find(|c| c.id == "bond0".to_string()).unwrap();
+    let conn = conns.iter().find(|c| &c.id == "bond0").unwrap();
     assert_eq!(conn.id, "bond0");
     assert_eq!(conn.device_type(), DeviceType::Bond);
     let bond = conn.bond.clone().unwrap();

rust/agama-lib/src/software/proxies.rs

@@ -3,6 +3,17 @@
 //! This code was generated by `zbus-xmlgen` `3.1.1` from DBus introspection data.
 use zbus::dbus_proxy;
 
+/// Software patterns map.
+///
+/// It uses the pattern name as key and a tuple containing the following information as value:
+///
+/// * Category.
+/// * Description.
+/// * Icon.
+/// * Summary.
+/// * Order.
+pub type PatternsMap = std::collections::HashMap<String, (String, String, String, String, String)>;
+
 #[dbus_proxy(
     interface = "org.opensuse.Agama.Software1",
     default_service = "org.opensuse.Agama.Software1",
@@ -22,10 +33,7 @@ trait Software1 {
     fn is_package_installed(&self, name: &str) -> zbus::Result<bool>;
 
     /// ListPatterns method
-    fn list_patterns(
-        &self,
-        filtered: bool,
-    ) -> zbus::Result<std::collections::HashMap<String, (String, String, String, String, String)>>;
+    fn list_patterns(&self, filtered: bool) -> zbus::Result<PatternsMap>;
 
     /// Probe method
     fn probe(&self) -> zbus::Result<()>;
@@ -50,6 +58,19 @@ trait Software1 {
     fn selected_patterns(&self) -> zbus::Result<std::collections::HashMap<String, u8>>;
 }
 
+/// Product definition.
+///
+/// It is composed of the following elements:
+///
+/// * Product ID.
+/// * Display name.
+/// * Some additional data which includes a "description" key.
+pub type Product = (
+    String,
+    String,
+    std::collections::HashMap<String, zbus::zvariant::OwnedValue>,
+);
+
 #[dbus_proxy(
     interface = "org.opensuse.Agama.Software1.Product",
     default_service = "org.opensuse.Agama.Software1",
@@ -61,15 +82,7 @@ trait SoftwareProduct {
 
     /// AvailableProducts property
     #[dbus_proxy(property)]
-    fn available_products(
-        &self,
-    ) -> zbus::Result<
-        Vec<(
-            String,
-            String,
-            std::collections::HashMap<String, zbus::zvariant::OwnedValue>,
-        )>,
-    >;
+    fn available_products(&self) -> zbus::Result<Vec<Product>>;
 
     /// SelectedProduct property
     #[dbus_proxy(property)]

rust/agama-lib/src/storage/store.rs

@@ -2,7 +2,6 @@
 
 use super::{StorageClient, StorageSettings};
 use crate::error::ServiceError;
-use std::default::Default;
 use zbus::Connection;
 
 /// Loads and stores the storage settings from/to the D-Bus service.
@@ -26,7 +25,6 @@ impl<'a> StorageStore<'a> {
             boot_device,
             lvm,
             encryption_password,
-            ..Default::default()
         })
     }
 

rust/agama-lib/src/users/client.rs

@@ -1,6 +1,6 @@
 //! Implements a client to access Agama's users service.
 
-use super::proxies::Users1Proxy;
+use super::proxies::{FirstUser as FirstUserFromDBus, Users1Proxy};
 use crate::error::ServiceError;
 use agama_settings::{settings::Settings, SettingValue, SettingsError};
 use serde::Serialize;
@@ -22,15 +22,7 @@ pub struct FirstUser {
 }
 
 impl FirstUser {
-    pub fn from_dbus(
-        dbus_data: zbus::Result<(
-            String,
-            String,
-            String,
-            bool,
-            std::collections::HashMap<String, zbus::zvariant::OwnedValue>,
-        )>,
-    ) -> zbus::Result<Self> {
+    pub fn from_dbus(dbus_data: zbus::Result<FirstUserFromDBus>) -> zbus::Result<Self> {
         let data = dbus_data?;
         Ok(Self {
             full_name: data.0,

rust/agama-lib/src/users/proxies.rs

@@ -3,6 +3,23 @@
 //! This code was generated by `zbus-xmlgen` `3.1.0` from DBus introspection data.`.
 use zbus::dbus_proxy;
 
+/// First user as it comes from D-Bus.
+///
+/// It is composed of:
+///
+/// * full name
+/// * user name
+/// * password
+/// * auto-login (enabled or not)
+/// * some optional and additional data
+pub type FirstUser = (
+    String,
+    String,
+    String,
+    bool,
+    std::collections::HashMap<String, zbus::zvariant::OwnedValue>,
+);
+
 #[dbus_proxy(
     interface = "org.opensuse.Agama.Users1",
     default_service = "org.opensuse.Agama.Manager1",
@@ -37,15 +54,7 @@ trait Users1 {
 
     /// FirstUser property
     #[dbus_proxy(property)]
-    fn first_user(
-        &self,
-    ) -> zbus::Result<(
-        String,
-        String,
-        String,
-        bool,
-        std::collections::HashMap<String, zbus::zvariant::OwnedValue>,
-    )>;
+    fn first_user(&self) -> zbus::Result<FirstUser>;
 
     /// RootPasswordSet property
     #[dbus_proxy(property)]

rust/agama-locale-data/src/lib.rs

@@ -162,9 +162,9 @@ mod tests {
         let first = result.first().expect("no keyboards");
         assert_eq!(first, "Africa/Abidjan");
         // test that we filter out deprecates Asmera ( there is already recent Asmara)
-        let asmera = result.iter().find(|&t| *t == "Africa/Asmera".to_string());
+        let asmera = result.iter().find(|&t| t == "Africa/Asmera");
         assert_eq!(asmera, None);
-        let asmara = result.iter().find(|&t| *t == "Africa/Asmara".to_string());
+        let asmara = result.iter().find(|&t| t == "Africa/Asmara");
         assert_eq!(asmara, Some(&"Africa/Asmara".to_string()));
         // here test that timezones from timezones matches ones in langtable ( as timezones can contain deprecated ones)
         // so this test catch if there is new zone that is not translated or if a zone is become deprecated