From 1535a0ac9ca4c6f50da3adbbef82e51e142e9452 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Wed, 29 Nov 2023 11:58:39 -0800 Subject: [PATCH] Clippy a bunch of code again (#1412) This runs `cargo clippy --fix` again and then finally just-about finishes the cleanup in pgrx-pg-sys manually so that it doesn't omit a bunch of pointless warnings, modulo a few cases left on purpose. --- cargo-pgrx/src/command/install.rs | 6 +-- cargo-pgrx/src/command/schema.rs | 2 +- pgrx-pg-sys/build.rs | 52 ++++++++----------- pgrx-pg-sys/build/clang.rs | 12 ++--- pgrx-pg-sys/src/lib.rs | 7 ++- pgrx-pg-sys/src/submodules/mod.rs | 2 +- pgrx-pg-sys/src/submodules/panic.rs | 2 +- pgrx-pg-sys/src/submodules/polyfill.rs | 2 +- .../src/extension_sql/entity.rs | 4 +- pgrx-sql-entity-graph/src/pgrx_sql.rs | 8 +-- pgrx-tests/src/framework.rs | 2 +- pgrx-version-updater/src/main.rs | 2 +- pgrx/src/hooks.rs | 2 +- pgrx/src/list/old_list.rs | 6 +-- pgrx/src/trigger_support/pg_trigger.rs | 6 +-- 15 files changed, 54 insertions(+), 61 deletions(-) diff --git a/cargo-pgrx/src/command/install.rs b/cargo-pgrx/src/command/install.rs index f08b436b7..d808d7320 100644 --- a/cargo-pgrx/src/command/install.rs +++ b/cargo-pgrx/src/command/install.rs @@ -173,7 +173,7 @@ pub(crate) fn install_extension( true, &package_manifest_path, &mut output_tracking, - &pg_config, + pg_config, )?; } @@ -218,7 +218,7 @@ pub(crate) fn install_extension( false, &package_manifest_path, &mut output_tracking, - &pg_config, + pg_config, )?; } @@ -409,7 +409,7 @@ fn copy_sql_files( true, &package_manifest_path, output_tracking, - &pg_config, + pg_config, )?; } } diff --git a/cargo-pgrx/src/command/schema.rs b/cargo-pgrx/src/command/schema.rs index 27aa82807..0245fa6ed 100644 --- a/cargo-pgrx/src/command/schema.rs +++ b/cargo-pgrx/src/command/schema.rs @@ -297,7 +297,7 @@ pub(crate) fn generate_schema( // The next action may take a few seconds, we'd like the user to know we're thinking. eprintln!("{} SQL entities", " Discovering".bold().green(),); - let postmaster_stub_built = create_stub(&postmaster_path, &postmaster_stub_dir)?; + let postmaster_stub_built = create_stub(postmaster_path, &postmaster_stub_dir)?; // Inspect the symbol table for a list of `__pgrx_internals` we should have the generator call let mut lib_so = target_dir_with_profile.clone(); diff --git a/pgrx-pg-sys/build.rs b/pgrx-pg-sys/build.rs index a42de56ef..9255a9e29 100644 --- a/pgrx-pg-sys/build.rs +++ b/pgrx-pg-sys/build.rs @@ -377,19 +377,15 @@ fn rewrite_items( fn extract_oids(code: &syn::File) -> BTreeMap> { let mut oids = BTreeMap::new(); // we would like to have a nice sorted set for item in &code.items { - match item { - Item::Const(ItemConst { ident, ty, expr, .. }) => { - // Retype as strings for easy comparison - let name = ident.to_string(); - let ty_str = ty.to_token_stream().to_string(); - - // This heuristic identifies "OIDs" - // We're going to warp the const declarations to be our newtype Oid - if ty_str == "u32" && is_builtin_oid(&name) { - oids.insert(ident.clone(), expr.clone()); - } - } - _ => {} + let Item::Const(ItemConst { ident, ty, expr, .. }) = item else { continue }; + // Retype as strings for easy comparison + let name = ident.to_string(); + let ty_str = ty.to_token_stream().to_string(); + + // This heuristic identifies "OIDs" + // We're going to warp the const declarations to be our newtype Oid + if ty_str == "u32" && is_builtin_oid(&name) { + oids.insert(ident.clone(), expr.clone()); } } oids @@ -402,7 +398,7 @@ fn is_builtin_oid(name: &str) -> bool { } fn rewrite_oid_consts( - items: &Vec, + items: &[syn::Item], oids: &BTreeMap>, ) -> Vec { items @@ -418,9 +414,7 @@ fn rewrite_oid_consts( .collect() } -fn format_builtin_oid_impl<'a>( - oids: BTreeMap>, -) -> proc_macro2::TokenStream { +fn format_builtin_oid_impl(oids: BTreeMap>) -> proc_macro2::TokenStream { let enum_variants: proc_macro2::TokenStream; let from_impl: proc_macro2::TokenStream; (enum_variants, from_impl) = oids @@ -451,12 +445,12 @@ fn format_builtin_oid_impl<'a>( } /// Implement our `PgNode` marker trait for `pg_sys::Node` and its "subclasses" -fn impl_pg_node(items: &Vec) -> eyre::Result { +fn impl_pg_node(items: &[syn::Item]) -> eyre::Result { let mut pgnode_impls = proc_macro2::TokenStream::new(); // we scope must of the computation so we can borrow `items` and then // extend it at the very end. - let struct_graph: StructGraph = StructGraph::from(&items[..]); + let struct_graph: StructGraph = StructGraph::from(items); // collect all the structs with `NodeTag` as their first member, // these will serve as roots in our forest of `Node`s @@ -586,7 +580,7 @@ impl<'a> From<&'a [syn::Item]> for StructGraph<'a> { for item in items.iter() { // grab the first field if it is struct let (id, first_field) = match &item { - &syn::Item::Struct(syn::ItemStruct { + syn::Item::Struct(syn::ItemStruct { ident: id, fields: syn::Fields::Named(fields), .. @@ -689,7 +683,7 @@ impl Ord for StructDescriptor<'_> { fn get_bindings( major_version: u16, pg_config: &PgConfig, - include_h: &PathBuf, + include_h: &path::Path, ) -> eyre::Result { let bindings = if let Some(info_dir) = target_env_tracked(&format!("PGRX_TARGET_INFO_PATH_PG{major_version}")) @@ -712,7 +706,7 @@ fn get_bindings( fn run_bindgen( major_version: u16, pg_config: &PgConfig, - include_h: &PathBuf, + include_h: &path::Path, ) -> eyre::Result { eprintln!("Generating bindings for pg{major_version}"); let configure = pg_config.configure()?; @@ -834,13 +828,13 @@ fn pg_target_include_flags(pg_version: u16, pg_config: &PgConfig) -> eyre::Resul } } -fn build_shim(shim_src: &PathBuf, shim_dst: &PathBuf, pg_config: &PgConfig) -> eyre::Result<()> { +fn build_shim( + shim_src: &path::Path, + shim_dst: &path::Path, + pg_config: &PgConfig, +) -> eyre::Result<()> { let major_version = pg_config.major_version()?; - let mut libpgrx_cshim: PathBuf = shim_dst.clone(); - - libpgrx_cshim.push(format!("libpgrx-cshim-{}.a", major_version)); - eprintln!("libpgrx_cshim={}", libpgrx_cshim.display()); // then build the shim for the version feature currently being built build_shim_for_version(shim_src, shim_dst, pg_config)?; @@ -855,8 +849,8 @@ fn build_shim(shim_src: &PathBuf, shim_dst: &PathBuf, pg_config: &PgConfig) -> e } fn build_shim_for_version( - shim_src: &PathBuf, - shim_dst: &PathBuf, + shim_src: &path::Path, + shim_dst: &path::Path, pg_config: &PgConfig, ) -> eyre::Result<()> { let path_env = prefix_path(pg_config.parent_path()); diff --git a/pgrx-pg-sys/build/clang.rs b/pgrx-pg-sys/build/clang.rs index ec191518e..f208d5cb4 100644 --- a/pgrx-pg-sys/build/clang.rs +++ b/pgrx-pg-sys/build/clang.rs @@ -76,7 +76,7 @@ pub(crate) fn detect_include_paths_for( !is_hidden(entry) && { entry_contains(entry, "clang") || entry_contains(entry, "include") - || entry_contains(entry, &*clang_major_fmt) + || entry_contains(entry, &clang_major_fmt) // we always want to descend from a lib dir, but only one step // as we don't really want to search all of /usr/lib's subdirs || os_str_contains(entry.file_name(), "lib") @@ -85,7 +85,7 @@ pub(crate) fn detect_include_paths_for( .filter_map(|e| e.ok()) // be discreet // We now need something that looks like it actually satisfies all our constraints .filter(|entry| { - entry_contains(entry, &*clang_major_fmt) + entry_contains(entry, &clang_major_fmt) && entry_contains(entry, "clang") && entry_contains(entry, "include") }) @@ -96,7 +96,7 @@ pub(crate) fn detect_include_paths_for( }) .filter_map(|entry| { let mut pbuf = entry.into_path(); - if pbuf.pop() && pbuf.is_dir() && os_str_contains(&*pbuf.file_name()?, "include") { + if pbuf.pop() && pbuf.is_dir() && os_str_contains(pbuf.file_name()?, "include") { Some(pbuf) } else { None @@ -104,20 +104,20 @@ pub(crate) fn detect_include_paths_for( }) .collect::>(); - if paths.len() > 0 { + if !paths.is_empty() { paths.sort(); paths.dedup(); break; } } // If we have anything better to recommend, don't autodetect! - let autodetect = paths.len() == 0; + let autodetect = paths.is_empty(); eprintln!("Found include dirs {:?}", paths); (autodetect, paths) } fn is_hidden(entry: &DirEntry) -> bool { - entry.file_name().to_str().map(|s| s.starts_with(".")).unwrap_or(false) + entry.file_name().to_str().map(|s| s.starts_with('.')).unwrap_or(false) } fn entry_contains(entry: &DirEntry, needle: &str) -> bool { diff --git a/pgrx-pg-sys/src/lib.rs b/pgrx-pg-sys/src/lib.rs index abaa745d1..a6e0a7fb8 100644 --- a/pgrx-pg-sys/src/lib.rs +++ b/pgrx-pg-sys/src/lib.rs @@ -20,10 +20,9 @@ #![cfg_attr(nightly, feature(strict_provenance))] #[cfg( - any( - // no features at all will cause problems - not(any(feature = "pg12", feature = "pg13", feature = "pg14", feature = "pg15", feature = "pg16")), - ))] + // no features at all will cause problems + not(any(feature = "pg12", feature = "pg13", feature = "pg14", feature = "pg15", feature = "pg16")), +)] std::compile_error!("exactly one feature must be provided (pg12, pg13, pg14, pg15, pg16)"); mod cshim; diff --git a/pgrx-pg-sys/src/submodules/mod.rs b/pgrx-pg-sys/src/submodules/mod.rs index 5c5fbb696..e2cfb03b2 100644 --- a/pgrx-pg-sys/src/submodules/mod.rs +++ b/pgrx-pg-sys/src/submodules/mod.rs @@ -31,7 +31,7 @@ pub use datum::Datum; pub use htup::*; pub use oids::*; pub use pg_try::*; -#[cfg(any(feature = "pg12"))] +#[cfg(feature = "pg12")] pub use polyfill::*; pub use utils::*; diff --git a/pgrx-pg-sys/src/submodules/panic.rs b/pgrx-pg-sys/src/submodules/panic.rs index 4406233d9..9203e4705 100644 --- a/pgrx-pg-sys/src/submodules/panic.rs +++ b/pgrx-pg-sys/src/submodules/panic.rs @@ -575,7 +575,7 @@ fn do_ereport(ereport: ErrorReportWithLevel) { /// to be freed in case level < ERROR #[inline(always)] #[rustfmt::skip] // my opinion wins - #[cfg(any(feature = "pg12"))] + #[cfg(feature = "pg12")] fn do_ereport_impl(ereport: ErrorReportWithLevel) { extern "C" { diff --git a/pgrx-pg-sys/src/submodules/polyfill.rs b/pgrx-pg-sys/src/submodules/polyfill.rs index 1eead0017..8f543c93a 100644 --- a/pgrx-pg-sys/src/submodules/polyfill.rs +++ b/pgrx-pg-sys/src/submodules/polyfill.rs @@ -15,5 +15,5 @@ mod typalign { pub const TYPALIGN_DOUBLE: u8 = b'd'; } -#[cfg(any(feature = "pg12"))] +#[cfg(feature = "pg12")] pub use typalign::*; diff --git a/pgrx-sql-entity-graph/src/extension_sql/entity.rs b/pgrx-sql-entity-graph/src/extension_sql/entity.rs index a4528a38f..c80acf8c6 100644 --- a/pgrx-sql-entity-graph/src/extension_sql/entity.rs +++ b/pgrx-sql-entity-graph/src/extension_sql/entity.rs @@ -73,7 +73,7 @@ impl ToSql for ExtensionSqlEntity { let ExtensionSqlEntity { file, line, sql, creates, requires, .. } = self; let creates = if !creates.is_empty() { let joined = - creates.into_iter().map(|i| format!("-- {}", i)).collect::>().join("\n"); + creates.iter().map(|i| format!("-- {}", i)).collect::>().join("\n"); format!( "\ -- creates:\n\ @@ -84,7 +84,7 @@ impl ToSql for ExtensionSqlEntity { }; let requires = if !requires.is_empty() { let joined = - requires.into_iter().map(|i| format!("-- {}", i)).collect::>().join("\n"); + requires.iter().map(|i| format!("-- {}", i)).collect::>().join("\n"); format!( "\ -- requires:\n\ diff --git a/pgrx-sql-entity-graph/src/pgrx_sql.rs b/pgrx-sql-entity-graph/src/pgrx_sql.rs index d1198ea61..079a95498 100644 --- a/pgrx-sql-entity-graph/src/pgrx_sql.rs +++ b/pgrx-sql-entity-graph/src/pgrx_sql.rs @@ -1108,7 +1108,7 @@ fn connect_hashes( enums, ); - if let Some((_, extern_index)) = externs.into_iter().find(|(extern_item, _)| { + if let Some((_, extern_index)) = externs.iter().find(|(extern_item, _)| { item.module_path == extern_item.module_path && extern_item.name == item.fn_name() }) { graph.add_edge(*extern_index, index, SqlGraphRelationship::RequiredBy); @@ -1414,7 +1414,7 @@ fn make_extern_connection( full_path: &str, externs: &HashMap, ) -> eyre::Result<()> { - match externs.into_iter().find(|(extern_item, _)| full_path == extern_item.full_path) { + match externs.iter().find(|(extern_item, _)| full_path == extern_item.full_path) { Some((_, extern_index)) => { graph.add_edge(*extern_index, index, SqlGraphRelationship::RequiredBy); Ok(()) @@ -1436,11 +1436,11 @@ fn make_type_or_enum_connection( types: &HashMap, enums: &HashMap, ) -> bool { - let found_ty = types.into_iter().find(|(ty_item, _)| ty_item.id_matches(ty_id)); + let found_ty = types.iter().find(|(ty_item, _)| ty_item.id_matches(ty_id)); if let Some((_, ty_index)) = found_ty { graph.add_edge(*ty_index, index, SqlGraphRelationship::RequiredBy); } - let found_enum = enums.into_iter().find(|(ty_item, _)| ty_item.id_matches(ty_id)); + let found_enum = enums.iter().find(|(ty_item, _)| ty_item.id_matches(ty_id)); if let Some((_, ty_index)) = found_enum { graph.add_edge(*ty_index, index, SqlGraphRelationship::RequiredBy); } diff --git a/pgrx-tests/src/framework.rs b/pgrx-tests/src/framework.rs index 911af3c39..7eeccefeb 100644 --- a/pgrx-tests/src/framework.rs +++ b/pgrx-tests/src/framework.rs @@ -258,7 +258,7 @@ pub fn client() -> eyre::Result<(postgres::Client, String)> { ) .wrap_err("There was an issue attempting to get the session ID from Postgres")?; - let session_id = match sid_query_result.get(0) { + let session_id = match sid_query_result.first() { Some(row) => row.get::<&str, &str>("sid").to_string(), None => Err(eyre!("Failed to obtain a client Session ID from Postgres"))?, }; diff --git a/pgrx-version-updater/src/main.rs b/pgrx-version-updater/src/main.rs index 6519e19d6..f05332b13 100644 --- a/pgrx-version-updater/src/main.rs +++ b/pgrx-version-updater/src/main.rs @@ -155,7 +155,7 @@ fn update_files(args: &UpdateFilesArgs) { } // Recursively walk down all directories to extract out any existing Cargo.toml files - for entry in WalkDir::new(¤t_dir) + for entry in WalkDir::new(current_dir) .into_iter() .filter_entry(|e| is_not_excluded_dir(e)) .filter_map(|v| v.ok()) diff --git a/pgrx/src/hooks.rs b/pgrx/src/hooks.rs index ae2a2162c..ffdd667ab 100644 --- a/pgrx/src/hooks.rs +++ b/pgrx/src/hooks.rs @@ -472,7 +472,7 @@ unsafe extern "C" fn pgrx_process_utility( .inner } -#[cfg(any(feature = "pg12"))] +#[cfg(feature = "pg12")] #[pg_guard] unsafe extern "C" fn pgrx_planner( parse: *mut pg_sys::Query, diff --git a/pgrx/src/list/old_list.rs b/pgrx/src/list/old_list.rs index 34fe2ad97..71896d51a 100644 --- a/pgrx/src/list/old_list.rs +++ b/pgrx/src/list/old_list.rs @@ -110,7 +110,7 @@ impl PgList { } } - #[cfg(any(feature = "pg12"))] + #[cfg(feature = "pg12")] #[inline] pub unsafe fn replace_ptr(&mut self, i: usize, with: *mut T) { let cell = pg_sys::pgrx_list_nth_cell(self.list, i as i32); @@ -124,7 +124,7 @@ impl PgList { cell.as_mut().expect("cell is null").ptr_value = with as void_mut_ptr; } - #[cfg(any(feature = "pg12"))] + #[cfg(feature = "pg12")] #[inline] pub fn replace_int(&mut self, i: usize, with: i32) { unsafe { @@ -142,7 +142,7 @@ impl PgList { } } - #[cfg(any(feature = "pg12"))] + #[cfg(feature = "pg12")] #[inline] pub fn replace_oid(&mut self, i: usize, with: pg_sys::Oid) { unsafe { diff --git a/pgrx/src/trigger_support/pg_trigger.rs b/pgrx/src/trigger_support/pg_trigger.rs index 7d3c80901..e203e0176 100644 --- a/pgrx/src/trigger_support/pg_trigger.rs +++ b/pgrx/src/trigger_support/pg_trigger.rs @@ -83,7 +83,7 @@ impl<'a> PgTrigger<'a> { // containing a known good `TriggerData` which also contains a known good `Trigger`... and the user agreed to // our `unsafe` constructor safety rules, we choose to trust this is indeed a valid pointer offered to us by // PostgreSQL, and that it trusts it. - unsafe { PgHeapTuple::from_trigger_data(&*self.trigger_data, TriggerTuple::New) } + unsafe { PgHeapTuple::from_trigger_data(self.trigger_data, TriggerTuple::New) } } /// Returns the old database row for UPDATE/DELETE operations in row-level triggers. @@ -95,7 +95,7 @@ impl<'a> PgTrigger<'a> { // containing a known good `TriggerData` which also contains a known good `Trigger`... and the user agreed to // our `unsafe` constructor safety rules, we choose to trust this is indeed a valid pointer offered to us by // PostgreSQL, and that it trusts it. - unsafe { PgHeapTuple::from_trigger_data(&*self.trigger_data, TriggerTuple::Old) } + unsafe { PgHeapTuple::from_trigger_data(self.trigger_data, TriggerTuple::Old) } } /// Variable that contains the name of the trigger actually fired @@ -206,7 +206,7 @@ impl<'a> PgTrigger<'a> { let slice: &[*mut c_char] = unsafe { core::slice::from_raw_parts(tgargs, tgnargs.try_into()?) }; let args = slice - .into_iter() + .iter() .map(|v| { // Safety: Given that we have a known good `FunctionCallInfo`, which PostgreSQL has checked is indeed a trigger, // containing a known good `TriggerData` which also contains a known good `Trigger`... and the user agreed to