Autoclippify cargo-pgrx and other build-support crates (#1382)

Last round of autoclippifying things for now...

...mostly because pgrx-sql-entity-graph induces such massive errors in
`cargo clippy --fix` that it repeatedly aborts and emits the most
fascinating errors, rather than try to complete the automatic fixes.

cargo-pgrx/src/command/connect.rs

@@ -73,7 +73,7 @@ impl CommandExecute for Connect {
             Some(dbname) => dbname,
             None => {
                 // We should infer from package
-                get_property(&package_manifest_path, "extname")
+                get_property(package_manifest_path, "extname")
                     .wrap_err("could not determine extension name")?
                     .ok_or(eyre!("extname not found in control file"))?
             }
@@ -97,5 +97,5 @@ pub(crate) fn connect_psql(pg_config: &PgConfig, dbname: &str, pgcli: bool) -> e
     }
 
     // run psql
-    exec_psql(&pg_config, dbname, pgcli)
+    exec_psql(pg_config, dbname, pgcli)
 }

cargo-pgrx/src/command/get.rs

@@ -40,7 +40,7 @@ impl CommandExecute for Get {
             crate::manifest::manifest_path(&metadata, self.package.as_ref())
                 .wrap_err("Couldn't get manifest path")?;
 
-        if let Some(value) = get_property(&package_manifest_path, &self.name)? {
+        if let Some(value) = get_property(package_manifest_path, &self.name)? {
             println!("{}", value);
         }
         Ok(())
@@ -72,7 +72,7 @@ pub fn get_property(manifest_path: impl AsRef<Path>, name: &str) -> eyre::Result
             continue;
         }
 
-        let (k, v) = (parts.get(0).unwrap().trim(), parts.get(1).unwrap().trim());
+        let (k, v) = (parts.first().unwrap().trim(), parts.get(1).unwrap().trim());
 
         if k == name {
             let v = v.trim_start_matches('\'');

cargo-pgrx/src/command/init.rs

@@ -26,7 +26,7 @@ use std::path::PathBuf;
 use std::process::{Command, Stdio};
 use std::sync::OnceLock;
 
-static PROCESS_ENV_DENYLIST: &'static [&'static str] = &[
+static PROCESS_ENV_DENYLIST: &[&str] = &[
     "DEBUG",
     "MAKEFLAGS",
     "MAKELEVEL",
@@ -131,14 +131,14 @@ impl CommandExecute for Init {
                     default_pgrx
                         .as_ref()
                         .unwrap() // We just set this
-                        .get(&pgver)
+                        .get(pgver)
                         .wrap_err_with(|| format!("{} is not a known Postgres version", pgver))?
                         .clone()
                 } else {
                     let config = PgConfig::new_with_defaults(pg_config_path.as_str().into());
                     let label = config.label().ok();
                     // We allow None in case it's configured via the environment or something.
-                    if label != None && label.as_deref() != Some(pgver) {
+                    if label.is_some() && label.as_deref() != Some(pgver) {
                         return Err(eyre!(
                             "wrong `pg_config` given to `--{pgver}` `{pg_config_path:?}` is for PostgreSQL {}",
                             config.major_version()?,
@@ -254,9 +254,9 @@ fn download_postgres(
     }
     let mut buf = Vec::new();
     let _count = http_response.into_reader().read_to_end(&mut buf)?;
-    let pgdir = untar(&buf, pgrx_home, pg_config, &init)?;
-    configure_postgres(pg_config, &pgdir, &init)?;
-    make_postgres(pg_config, &pgdir, &init)?;
+    let pgdir = untar(&buf, pgrx_home, pg_config, init)?;
+    configure_postgres(pg_config, &pgdir, init)?;
+    make_postgres(pg_config, &pgdir, init)?;
     make_install_postgres(pg_config, &pgdir, init) // returns a new PgConfig object
 }
 
@@ -431,7 +431,7 @@ fn configure_postgres(pg_config: &PgConfig, pgdir: &PathBuf, init: &Init) -> eyr
         .stderr(std::process::Stdio::piped())
         .stdin(std::process::Stdio::null())
         .env("PATH", prefix_path(pgdir))
-        .current_dir(&pgdir);
+        .current_dir(pgdir);
     for var in PROCESS_ENV_DENYLIST {
         command.env_remove(var);
     }
@@ -472,7 +472,7 @@ fn make_postgres(pg_config: &PgConfig, pgdir: &PathBuf, init: &Init) -> eyre::Re
         .stdout(std::process::Stdio::piped())
         .stderr(std::process::Stdio::piped())
         .stdin(std::process::Stdio::null())
-        .current_dir(&pgdir);
+        .current_dir(pgdir);
 
     for var in PROCESS_ENV_DENYLIST {
         command.env_remove(var);
@@ -514,7 +514,7 @@ fn make_install_postgres(
         .stdout(std::process::Stdio::piped())
         .stderr(std::process::Stdio::piped())
         .stdin(std::process::Stdio::null())
-        .current_dir(&pgdir);
+        .current_dir(pgdir);
     for var in PROCESS_ENV_DENYLIST {
         command.env_remove(var);
     }
@@ -606,7 +606,7 @@ pub(crate) fn initdb(bindir: &PathBuf, datadir: &PathBuf) -> eyre::Result<()> {
         .stderr(Stdio::piped())
         .args(get_c_locale_flags())
         .arg("-D")
-        .arg(&datadir);
+        .arg(datadir);
 
     let command_str = format!("{:?}", command);
     tracing::debug!(command = %command_str, "Running");

cargo-pgrx/src/command/install.rs

@@ -85,7 +85,7 @@ impl CommandExecute for Install {
         let pg_version = format!("pg{}", pg_config.major_version()?);
         let profile = CargoProfile::from_flags(
             self.profile.as_deref(),
-            self.release.then_some(CargoProfile::Release).unwrap_or(CargoProfile::Dev),
+            if self.release { CargoProfile::Release } else { CargoProfile::Dev },
         )?;
 
         crate::manifest::modify_features_for_version(
@@ -96,7 +96,7 @@ impl CommandExecute for Install {
             self.test,
         );
 
-        display_version_info(&pg_config, &PgVersionSource::PgConfig(pg_config.label()?.into()));
+        display_version_info(&pg_config, &PgVersionSource::PgConfig(pg_config.label()?));
         install_extension(
             self.manifest_path.as_ref(),
             self.package.as_ref(),
@@ -146,7 +146,7 @@ pub(crate) fn install_extension(
     let versioned_so = get_property(&package_manifest_path, "module_pathname")?.is_none();
 
     let build_command_output =
-        build_extension(user_manifest_path.as_ref(), user_package, &profile, &features)?;
+        build_extension(user_manifest_path.as_ref(), user_package, profile, features)?;
     let build_command_bytes = build_command_output.stdout;
     let build_command_reader = BufReader::new(build_command_bytes.as_slice());
     let build_command_stream = cargo_metadata::Message::parse_stream(build_command_reader);
@@ -162,7 +162,7 @@ pub(crate) fn install_extension(
         let mut dest = base_directory.clone();
         dest.push(&extdir);
         dest.push(
-            &control_file
+            control_file
                 .file_name()
                 .ok_or_else(|| eyre!("Could not get filename for `{}`", control_file.display()))?,
         );
@@ -260,18 +260,18 @@ fn copy_file(
         })?,
     };
 
-    println!("{} {} to {}", "     Copying".bold().green(), msg, format_display_path(&dest)?.cyan());
+    println!("{} {} to {}", "     Copying".bold().green(), msg, format_display_path(dest)?.cyan());
 
     if do_filter {
         // we want to filter the contents of the file we're to copy
-        let input = std::fs::read_to_string(&src)
+        let input = std::fs::read_to_string(src)
             .wrap_err_with(|| format!("failed to read `{}`", src.display()))?;
         let input = filter_contents(package_manifest_path, input)?;
-        std::fs::write(&dest, &input).wrap_err_with(|| {
+        std::fs::write(dest, input).wrap_err_with(|| {
             format!("failed writing `{}` to `{}`", src.display(), dest.display())
         })?;
     } else {
-        std::fs::copy(&src, &dest).wrap_err_with(|| {
+        std::fs::copy(src, dest).wrap_err_with(|| {
             format!("failed copying `{}` to `{}`", src.display(), dest.display())
         })?;
     }
@@ -461,10 +461,10 @@ pub(crate) fn get_version(manifest_path: impl AsRef<Path>) -> eyre::Result<Strin
                 crate::metadata::validate(Some(manifest_path), &metadata)?;
                 let manifest_path = crate::manifest::manifest_path(&metadata, None)
                     .wrap_err("Couldn't get manifest path")?;
-                let manifest = Manifest::from_path(&manifest_path)
+                let manifest = Manifest::from_path(manifest_path)
                     .wrap_err("Couldn't parse manifest")?;
-                let version = manifest.package_version()?;
-                version
+
+                manifest.package_version()?
             } else {
                 v
             }
@@ -482,7 +482,7 @@ fn get_git_hash(manifest_path: impl AsRef<Path>) -> eyre::Result<String> {
     let path_string = manifest_path.as_ref().to_owned();
 
     if let Some(hash) = GIT_HASH.lock().unwrap().get(&path_string) {
-        return Ok(hash.clone());
+        Ok(hash.clone())
     } else {
         let hash = match get_property(manifest_path, "git_hash")? {
             Some(hash) => hash,
@@ -504,7 +504,7 @@ fn make_relative(path: PathBuf) -> PathBuf {
     let mut relative = PathBuf::new();
     let mut components = path.components();
     components.next(); // skip the root
-    while let Some(part) = components.next() {
+    for part in components {
         relative.push(part)
     }
     relative
@@ -514,7 +514,7 @@ pub(crate) fn format_display_path(path: impl AsRef<Path>) -> eyre::Result<String
     let path = path.as_ref();
     let out = path
         .strip_prefix(get_target_dir()?.parent().unwrap())
-        .unwrap_or(&path)
+        .unwrap_or(path)
         .display()
         .to_string();
     Ok(out)

cargo-pgrx/src/command/mod.rs

@@ -30,7 +30,7 @@ pub(crate) mod version;
 
 // Build a ureq::Agent by the given url. Requests from this agent are proxied if we have
 // set the HTTPS_PROXY/HTTP_PROXY environment variables.
-pub(self) fn build_agent_for_url(url: &str) -> eyre::Result<Agent> {
+fn build_agent_for_url(url: &str) -> eyre::Result<Agent> {
     if let Some(proxy_url) = for_url_str(url).to_string() {
         Ok(AgentBuilder::new().proxy(Proxy::new(proxy_url)?).build())
     } else {

cargo-pgrx/src/command/new.rs

@@ -82,7 +82,7 @@ fn create_control_file(path: &PathBuf, name: &str) -> Result<(), std::io::Error>
     filename.push(format!("{}.control", name));
     let mut file = std::fs::File::create(filename)?;
 
-    file.write_all(&format!(include_str!("../templates/control"), name = name).as_bytes())?;
+    file.write_all(format!(include_str!("../templates/control"), name = name).as_bytes())?;
 
     Ok(())
 }
@@ -93,7 +93,7 @@ fn create_cargo_toml(path: &PathBuf, name: &str) -> Result<(), std::io::Error> {
     filename.push("Cargo.toml");
     let mut file = std::fs::File::create(filename)?;
 
-    file.write_all(&format!(include_str!("../templates/cargo_toml"), name = name).as_bytes())?;
+    file.write_all(format!(include_str!("../templates/cargo_toml"), name = name).as_bytes())?;
 
     Ok(())
 }
@@ -119,10 +119,10 @@ fn create_lib_rs(path: &PathBuf, name: &str, is_bgworker: bool) -> Result<(), st
 
     if is_bgworker {
         file.write_all(
-            &format!(include_str!("../templates/bgworker_lib_rs"), name = name).as_bytes(),
+            format!(include_str!("../templates/bgworker_lib_rs"), name = name).as_bytes(),
         )?;
     } else {
-        file.write_all(&format!(include_str!("../templates/lib_rs"), name = name).as_bytes())?;
+        file.write_all(format!(include_str!("../templates/lib_rs"), name = name).as_bytes())?;
     }
 
     Ok(())

cargo-pgrx/src/command/package.rs

@@ -60,7 +60,7 @@ impl Package {
 
         let pg_config = match self.pg_config {
             None => PgConfig::from_path(),
-            Some(config) => PgConfig::new_with_defaults(PathBuf::from(config)),
+            Some(config) => PgConfig::new_with_defaults(config),
         };
         let pg_version = format!("pg{}", pg_config.major_version()?);
 
@@ -74,7 +74,7 @@ impl Package {
         let profile = CargoProfile::from_flags(
             self.profile.as_deref(),
             // NB:  `cargo pgrx package` defaults to "--release" whereas all other commands default to "debug"
-            self.debug.then_some(CargoProfile::Dev).unwrap_or(CargoProfile::Release),
+            if self.debug { CargoProfile::Dev } else { CargoProfile::Release },
         )?;
         let out_dir = if let Some(out_dir) = self.out_dir {
             out_dir
@@ -127,7 +127,7 @@ pub(crate) fn package_extension(
         std::fs::create_dir_all(&out_dir)?;
     }
 
-    display_version_info(pg_config, &PgVersionSource::PgConfig(pg_config.label()?.into()));
+    display_version_info(pg_config, &PgVersionSource::PgConfig(pg_config.label()?));
     install_extension(
         user_manifest_path,
         user_package,

cargo-pgrx/src/command/run.rs

@@ -74,7 +74,7 @@ impl CommandExecute for Run {
         };
         let profile = CargoProfile::from_flags(
             self.profile.as_deref(),
-            self.release.then_some(CargoProfile::Release).unwrap_or(CargoProfile::Dev),
+            if self.release { CargoProfile::Release } else { CargoProfile::Dev },
         )?;
 
         run(

cargo-pgrx/src/command/schema.rs

@@ -106,7 +106,7 @@ impl CommandExecute for Schema {
 
         let profile = CargoProfile::from_flags(
             self.profile.as_deref(),
-            self.release.then_some(CargoProfile::Release).unwrap_or(CargoProfile::Dev),
+            if self.release { CargoProfile::Release } else { CargoProfile::Dev },
         )?;
 
         generate_schema(
@@ -306,7 +306,7 @@ pub(crate) fn generate_schema(
 
     let lib_so_data = std::fs::read(&lib_so).wrap_err("couldn't read extension shared object")?;
     let lib_so_obj_file =
-        parse_object(&*lib_so_data).wrap_err("couldn't parse extension shared object")?;
+        parse_object(&lib_so_data).wrap_err("couldn't parse extension shared object")?;
     let lib_so_exports =
         lib_so_obj_file.exports().wrap_err("couldn't get exports from extension shared object")?;
 
@@ -340,11 +340,8 @@ pub(crate) fn generate_schema(
     let mut num_aggregates = 0_usize;
     for func in &fns_to_call {
         if func.starts_with("__pgrx_internals_schema_") {
-            let schema = func
-                .split('_')
-                .skip(5)
-                .next()
-                .expect("Schema extern name was not of expected format");
+            let schema =
+                func.split('_').nth(5).expect("Schema extern name was not of expected format");
             seen_schemas.push(schema);
         } else if func.starts_with("__pgrx_internals_fn_") {
             num_funcs += 1;
@@ -369,8 +366,8 @@ pub(crate) fn generate_schema(
         "{} {} SQL entities: {} schemas ({} unique), {} functions, {} types, {} enums, {} sqls, {} ords, {} hashes, {} aggregates, {} triggers",
         "  Discovered".bold().green(),
         fns_to_call.len().to_string().bold().cyan(),
-        seen_schemas.iter().count().to_string().bold().cyan(),
-        seen_schemas.iter().collect::<std::collections::HashSet<_>>().iter().count().to_string().bold().cyan(),
+        seen_schemas.len().to_string().bold().cyan(),
+        seen_schemas.iter().collect::<std::collections::HashSet<_>>().len().to_string().bold().cyan(),
         num_funcs.to_string().bold().cyan(),
         num_types.to_string().bold().cyan(),
         num_enums.to_string().bold().cyan(),
@@ -502,7 +499,7 @@ fn create_stub(
     }
 
     let postmaster_obj_file =
-        parse_object(&*postmaster_bin_data).wrap_err("couldn't parse postmaster")?;
+        parse_object(&postmaster_bin_data).wrap_err("couldn't parse postmaster")?;
     let postmaster_exports = postmaster_obj_file
         .exports()
         .wrap_err("couldn't get exports from extension shared object")?;
@@ -528,7 +525,7 @@ fn create_stub(
     let mut so_rustc_invocation = crate::env::rustc();
     so_rustc_invocation.stderr(Stdio::inherit());
 
-    if let Some(rustc_flags_str) = std::env::var("RUSTFLAGS").ok() {
+    if let Ok(rustc_flags_str) = std::env::var("RUSTFLAGS") {
         let rustc_flags = rustc_flags_str.split(' ').collect::<Vec<_>>();
         so_rustc_invocation.args(rustc_flags);
     }
@@ -564,20 +561,20 @@ fn create_stub(
 }
 
 fn parse_object(data: &[u8]) -> object::Result<object::File> {
-    let kind = object::FileKind::parse(&*data)?;
+    let kind = object::FileKind::parse(data)?;
 
     match kind {
         FileKind::MachOFat32 => {
             let arch = env::consts::ARCH;
 
-            match slice_arch32(&*data, arch) {
-                Some(slice) => parse_object(&*slice),
+            match slice_arch32(data, arch) {
+                Some(slice) => parse_object(slice),
                 None => {
                     panic!("Failed to slice architecture '{arch}' from universal binary.")
                 }
             }
         }
-        _ => object::File::parse(&*data),
+        _ => object::File::parse(data),
     }
 }
 
@@ -593,10 +590,10 @@ fn slice_arch32<'a>(data: &'a [u8], arch: &str) -> Option<&'a [u8]> {
         _ => Architecture::Unknown,
     };
 
-    let candidates = FatHeader::parse_arch32(&*data).ok()?;
+    let candidates = FatHeader::parse_arch32(data).ok()?;
     let architecture = candidates.iter().find(|a| a.architecture() == target)?;
 
-    architecture.data(&*data).ok()
+    architecture.data(data).ok()
 }
 
 #[cfg(test)]
@@ -629,7 +626,7 @@ mod tests {
 
         let slice = slice_arch32(&bin, "aarch64")
             .expect("Failed to slice architecture 'aarch64' from universal binary.");
-        assert!(parse_object(&slice).is_ok());
+        assert!(parse_object(slice).is_ok());
     }
 
     #[test]