12: make code perfect

.github/workflows/test.yml

@@ -43,7 +43,7 @@ jobs:
       - name: Clippy
         uses: clechasseur/rs-clippy-check@v3
         with:
-          args: --all-features --bins --examples --tests --benches -- -D warnings
+          args: --all-features --bins --examples --tests --benches -- -W clippy::all -W clippy::pedantic -D warnings
 
   tests:
     needs: pre_job

src/commands/clean.rs

@@ -9,7 +9,7 @@ pub fn cmd(data_dir: &Path) -> Result<()> {
             .context(format!("failed to remove directory {}", data_dir.display()))?;
         info!("Successfully removed : {}", data_dir.to_string_lossy());
     } else {
-        warn!("Data directory does not exist")
+        warn!("Data directory does not exist");
     }
 
     Ok(())

src/commands/config/apply.rs

@@ -13,12 +13,12 @@ fn apply_service_config(
     plugin_name: &str,
     plugin_version: &str,
     service_name: &str,
-    config: HashMap<String, Value>,
+    config: &HashMap<String, Value>,
     admin_socket: &Path,
 ) -> Result<()> {
     let mut queries: Vec<String> = Vec::new();
 
-    for (key, value) in &config {
+    for (key, value) in config {
         let value = serde_json::to_string(&value)
             .context(format!("failed to serialize the string with key {key}"))?;
         queries.push(format!(
@@ -83,12 +83,12 @@ pub fn cmd(config_path: &Path, data_dir: &Path) -> Result<()> {
             &cargo_manifest.package.name,
             &cargo_manifest.package.version,
             &service_name,
-            service_config,
+            &service_config,
             &admin_socket,
         )
         .context(format!(
             "failed to apply service config for service {service_name}"
-        ))?
+        ))?;
     }
 
     Ok(())

src/commands/lib.rs

@@ -6,6 +6,7 @@ pub enum BuildType {
     Debug,
 }
 
+#[allow(clippy::needless_pass_by_value)]
 pub fn cargo_build(build_type: BuildType) -> Result<()> {
     let output = match build_type {
         BuildType::Release => Command::new("cargo")

src/commands/plugin/new.rs

@@ -4,7 +4,7 @@ use std::{
     ffi::OsStr,
     fs::{self, File},
     io::Write,
-    path::{Path, PathBuf},
+    path::Path,
     process::Command,
 };
 
@@ -71,14 +71,14 @@ where
     Ok(())
 }
 
-fn workspace_init(root_path: &PathBuf, project_name: &str) -> Result<()> {
+fn workspace_init(root_path: &Path, project_name: &str) -> Result<()> {
     let cargo_toml_path = root_path.join("Cargo.toml");
 
     let mut cargo_toml =
         File::create(cargo_toml_path).context("failed to create Cargo.toml for workspace")?;
 
     cargo_toml
-        .write_all(format!("[workspace]\nmembers = [\n  \"{}\",\n]", project_name).as_bytes())?;
+        .write_all(format!("[workspace]\nmembers = [\n  \"{project_name}\",\n]").as_bytes())?;
 
     fs::copy(
         root_path.join(project_name).join("topology.toml"),
@@ -114,7 +114,7 @@ pub fn cmd(path: Option<&Path>, without_git: bool, init_workspace: bool) -> Resu
     let plugin_path = if init_workspace {
         path.join(project_name)
     } else {
-        path.to_path_buf()
+        path.clone()
     };
 
     std::fs::create_dir_all(&plugin_path)

src/commands/plugin/pack.rs

@@ -65,7 +65,7 @@ pub fn cmd(pack_debug: bool) -> Result<()> {
     )
     .context("failed to parse Cargo.toml")?;
 
-    let normalized_package_name = cargo_manifest.package.name.replace("-", "_");
+    let normalized_package_name = cargo_manifest.package.name.replace('-', "_");
 
     let compressed_file = File::create(format!(
         "target/{}-{}.tar.gz",
@@ -77,7 +77,7 @@ pub fn cmd(pack_debug: bool) -> Result<()> {
 
     let lib_name = format!("lib{normalized_package_name}.{LIB_EXT}");
     let mut lib_file =
-        File::open(build_dir.join(&lib_name)).context(format!("failed to open {}", lib_name))?;
+        File::open(build_dir.join(&lib_name)).context(format!("failed to open {lib_name}"))?;
 
     let mut manifest_file =
         File::open(build_dir.join("manifest.yaml")).context("failed to open file manifest.yaml")?;

src/commands/run.rs

@@ -58,20 +58,20 @@ fn enable_plugins(
             continue;
         };
         for service in services {
-            let plugin_dir = plugins_dir.join(service.plugin.clone());
+            let current_plugin_dir = plugins_dir.join(service.plugin.clone());
 
-            if !plugin_dir.exists() {
+            if !current_plugin_dir.exists() {
                 bail!(
                     "directory {} does not exist, run \"cargo build\" inside plugin directory",
-                    plugin_dir.display()
+                    current_plugin_dir.display()
                 );
             }
             plugins.entry(service.plugin.clone()).or_insert_with(|| {
-                let mut versions: Vec<_> = fs::read_dir(plugin_dir)
+                let mut versions: Vec<_> = fs::read_dir(current_plugin_dir)
                     .unwrap()
                     .map(|r| r.unwrap())
                     .collect();
-                versions.sort_by_key(|dir| dir.path());
+                versions.sort_by_key(std::fs::DirEntry::path);
                 versions
                     .last()
                     .unwrap()
@@ -85,8 +85,7 @@ fn enable_plugins(
 
     let mut queries: Vec<String> = Vec::new();
     // Queries to set migration variables, order of commands is not important
-    push_migration_envs_queries(&mut queries, topology, &plugins)
-        .context("failed to push migration variables")?;
+    push_migration_envs_queries(&mut queries, topology, &plugins);
 
     for (plugin, version) in &plugins {
         queries.push(format!(r#"CREATE PLUGIN "{plugin}" {version};"#));
@@ -134,10 +133,10 @@ fn push_migration_envs_queries(
     queries: &mut Vec<String>,
     topology: &Topology,
     plugins: &HashMap<String, String>,
-) -> Result<()> {
+) {
     info!("setting migration variables");
 
-    for (_, tier) in &topology.tiers {
+    for tier in topology.tiers.values() {
         let Some(migration_envs) = &tier.migration_envs else {
             continue;
         };
@@ -150,8 +149,6 @@ fn push_migration_envs_queries(
             }
         }
     }
-
-    Ok(())
 }
 
 fn kill_picodata_instances() -> Result<()> {
@@ -169,9 +166,9 @@ pub fn cmd(
     topology_path: &PathBuf,
     data_dir: &Path,
     disable_plugin_install: bool,
-    base_http_port: &i32,
+    base_http_port: i32,
     picodata_path: &PathBuf,
-    base_pg_port: &i32,
+    base_pg_port: i32,
     use_release: bool,
 ) -> Result<()> {
     fs::create_dir_all(data_dir).unwrap();
@@ -213,7 +210,7 @@ pub fn cmd(
             let bin_port = 3000 + instance_id;
             let http_port = base_http_port + instance_id;
             let pg_port = base_pg_port + instance_id;
-            let instance_data_dir = data_dir.join("cluster").join(format!("i_{}", instance_id));
+            let instance_data_dir = data_dir.join("cluster").join(format!("i_{instance_id}"));
 
             // TODO: make it as child processes with catch output and redirect it to main
             // output
@@ -227,34 +224,31 @@ pub fn cmd(
                     "--plugin-dir",
                     plugins_dir,
                     "--listen",
-                    &format!("127.0.0.1:{}", bin_port),
+                    &format!("127.0.0.1:{bin_port}"),
                     "--peer",
-                    &format!("127.0.0.1:{}", first_instance_bin_port),
+                    &format!("127.0.0.1:{first_instance_bin_port}"),
                     "--init-replication-factor",
                     &tier.replication_factor.to_string(),
                     "--http-listen",
-                    &format!("127.0.0.1:{}", http_port),
+                    &format!("127.0.0.1:{http_port}"),
                     "--pg-listen",
-                    &format!("127.0.0.1:{}", pg_port),
+                    &format!("127.0.0.1:{pg_port}"),
                     "--tier",
                     tier_name,
                 ])
                 .spawn()
-                .context(format!(
-                    "failed to start picodata instance: {}",
-                    instance_id
-                ))?;
+                .context(format!("failed to start picodata instance: {instance_id}"))?;
             thread::sleep(Duration::from_secs(5));
 
             // Save pid of picodata process to kill it after
             let pid = process.id();
             let pid_location = instance_data_dir.join("pid");
             let mut file = File::create(pid_location)?;
-            writeln!(file, "{}", pid)?;
+            writeln!(file, "{pid}")?;
 
             let processes_lock = Arc::clone(&get_picodata_processes());
             let mut processes = processes_lock.lock().unwrap();
-            processes.push(process)
+            processes.push(process);
         }
     }
 

src/main.rs

@@ -129,21 +129,21 @@ fn main() -> Result<()> {
             &topology,
             &data_dir,
             disable_plugin_install,
-            &base_http_port,
+            base_http_port,
             &picodata_path,
-            &base_pg_port,
+            base_pg_port,
             release,
         )
         .context("failed to execute Run command")?,
         Command::Stop { data_dir } => {
-            commands::stop::cmd(&data_dir).context("failed to execute \"stop\" command")?
+            commands::stop::cmd(&data_dir).context("failed to execute \"stop\" command")?;
         }
         Command::Clean { data_dir } => {
-            commands::clean::cmd(&data_dir).context("failed to execute \"clean\" command")?
+            commands::clean::cmd(&data_dir).context("failed to execute \"clean\" command")?;
         }
         Command::Plugin { command } => match command {
             Plugin::Pack { debug } => {
-                commands::plugin::pack::cmd(debug).context("failed to execute \"pack\" command")?
+                commands::plugin::pack::cmd(debug).context("failed to execute \"pack\" command")?;
             }
             Plugin::New {
                 path,