Cleanup pgrx sql entity graph more

pgrx-sql-entity-graph/src/extension_sql/entity.rs

@@ -192,9 +192,9 @@ impl SqlDeclaredEntity {
 
     pub fn has_sql_declared_entity(&self, identifier: &SqlDeclared) -> bool {
         match (&identifier, &self) {
-            (SqlDeclared::Type(identifier_name), &SqlDeclaredEntity::Type(data))
-            | (SqlDeclared::Enum(identifier_name), &SqlDeclaredEntity::Enum(data))
-            | (SqlDeclared::Function(identifier_name), &SqlDeclaredEntity::Function(data)) => {
+            (SqlDeclared::Type(ident_name), &SqlDeclaredEntity::Type(data))
+            | (SqlDeclared::Enum(ident_name), &SqlDeclaredEntity::Enum(data))
+            | (SqlDeclared::Function(ident_name), &SqlDeclaredEntity::Function(data)) => {
                 let matches = |identifier_name: &str| {
                     identifier_name == &data.name
                         || identifier_name == &data.option
@@ -206,21 +206,15 @@ impl SqlDeclaredEntity {
                         || identifier_name == &data.option_array
                         || identifier_name == &data.varlena
                 };
-                if matches(identifier_name) || data.pg_box.contains(identifier_name) {
+                if matches(ident_name) || data.pg_box.contains(ident_name) {
                     return true;
                 }
                 // there are cases where the identifier is
                 // `core::option::Option<Foo>` while the data stores
                 // `Option<Foo>` check again for this
-                let generics_start = match identifier_name.find('<') {
-                    None => return false,
-                    Some(idx) => idx,
-                };
-                let qualification_end = match identifier_name[..generics_start].rfind("::") {
-                    None => return false,
-                    Some(idx) => idx,
-                };
-                matches(&identifier_name[qualification_end + 2..])
+                let Some(generics_start) = ident_name.find('<') else { return false };
+                let Some(qual_end) = ident_name[..generics_start].rfind("::") else { return false };
+                matches(&ident_name[qual_end + 2..])
             }
             _ => false,
         }

pgrx-sql-entity-graph/src/lib.rs

@@ -110,7 +110,7 @@ pub enum SqlGraphEntity {
 impl SqlGraphEntity {
     pub fn sql_anchor_comment(&self) -> String {
         let maybe_file_and_line = if let (Some(file), Some(line)) = (self.file(), self.line()) {
-            format!("-- {file}:{line}\n", file = file, line = line)
+            format!("-- {file}:{line}\n")
         } else {
             String::default()
         };
@@ -119,7 +119,6 @@ impl SqlGraphEntity {
             {maybe_file_and_line}\
             -- {rust_identifier}\
         ",
-            maybe_file_and_line = maybe_file_and_line,
             rust_identifier = self.rust_identifier(),
         )
     }
@@ -132,7 +131,7 @@ impl SqlGraphIdentifier for SqlGraphEntity {
             SqlGraphEntity::CustomSql(item) => item.dot_identifier(),
             SqlGraphEntity::Function(item) => item.dot_identifier(),
             SqlGraphEntity::Type(item) => item.dot_identifier(),
-            SqlGraphEntity::BuiltinType(item) => format!("preexisting type {}", item),
+            SqlGraphEntity::BuiltinType(item) => format!("preexisting type {item}"),
             SqlGraphEntity::Enum(item) => item.dot_identifier(),
             SqlGraphEntity::Ord(item) => item.dot_identifier(),
             SqlGraphEntity::Hash(item) => item.dot_identifier(),
@@ -268,18 +267,16 @@ impl ToSql for SqlGraphEntity {
 pub fn ident_is_acceptable_to_postgres(ident: &syn::Ident) -> Result<(), syn::Error> {
     // Roughly `pgrx::pg_sys::NAMEDATALEN`
     //
-    // Technically it **should** be that exactly, however this is `pgrx-utils` and a this data is used at macro time.
+    // Technically it **should** be that, but we need to guess at build time
     const POSTGRES_IDENTIFIER_MAX_LEN: usize = 64;
 
-    let ident_string = ident.to_string();
-    if ident_string.len() >= POSTGRES_IDENTIFIER_MAX_LEN {
+    let len = ident.to_string().len();
+    if len >= POSTGRES_IDENTIFIER_MAX_LEN {
         return Err(syn::Error::new(
             ident.span(),
             format!(
-                "Identifier `{}` was {} characters long, PostgreSQL will truncate identifiers with less than \
-                {POSTGRES_IDENTIFIER_MAX_LEN} characters, opt for an identifier which Postgres won't truncate",
-                ident,
-                ident_string.len(),
+                "Identifier `{ident}` was {len} characters long, PostgreSQL will truncate identifiers with less than \
+                {POSTGRES_IDENTIFIER_MAX_LEN} characters, opt for an identifier which Postgres won't truncate"
             )
         ));
     }

pgrx-sql-entity-graph/src/lifetimes.rs

@@ -117,38 +117,35 @@ pub fn anonymize_lifetimes(value: &mut syn::Type) {
     match value {
         syn::Type::Path(type_path) => {
             for segment in &mut type_path.path.segments {
-                match &mut segment.arguments {
-                    syn::PathArguments::AngleBracketed(bracketed) => {
-                        for arg in &mut bracketed.args {
-                            match arg {
-                                // rename lifetimes to the anonymous lifetime
-                                syn::GenericArgument::Lifetime(lifetime) => {
-                                    lifetime.ident = syn::Ident::new("_", lifetime.ident.span());
-                                }
+                if let syn::PathArguments::AngleBracketed(bracketed) = &mut segment.arguments {
+                    for arg in &mut bracketed.args {
+                        match arg {
+                            // rename lifetimes to the anonymous lifetime
+                            syn::GenericArgument::Lifetime(lifetime) => {
+                                lifetime.ident = syn::Ident::new("_", lifetime.ident.span());
+                            }
 
-                                // recurse
-                                syn::GenericArgument::Type(ty) => anonymize_lifetimes(ty),
-                                syn::GenericArgument::Binding(binding) => {
-                                    anonymize_lifetimes(&mut binding.ty)
-                                }
-                                syn::GenericArgument::Constraint(constraint) => {
-                                    for bound in constraint.bounds.iter_mut() {
-                                        match bound {
-                                            syn::TypeParamBound::Lifetime(lifetime) => {
-                                                lifetime.ident =
-                                                    syn::Ident::new("_", lifetime.ident.span())
-                                            }
-                                            _ => {}
+                            // recurse
+                            syn::GenericArgument::Type(ty) => anonymize_lifetimes(ty),
+                            syn::GenericArgument::Binding(binding) => {
+                                anonymize_lifetimes(&mut binding.ty)
+                            }
+                            syn::GenericArgument::Constraint(constraint) => {
+                                for bound in constraint.bounds.iter_mut() {
+                                    match bound {
+                                        syn::TypeParamBound::Lifetime(lifetime) => {
+                                            lifetime.ident =
+                                                syn::Ident::new("_", lifetime.ident.span())
                                         }
+                                        _ => {}
                                     }
                                 }
-
-                                // nothing to do otherwise
-                                _ => {}
                             }
+
+                            // nothing to do otherwise
+                            _ => {}
                         }
                     }
-                    _ => {}
                 }
             }
         }

pgrx-sql-entity-graph/src/pg_extern/entity/mod.rs

@@ -273,11 +273,10 @@ impl ToSql for PgExternEntity {
             PgExternReturnEntity::Iterated { tys: table_items, optional: _, result: _ } => {
                 let mut items = String::new();
                 let metadata_retval = self.metadata.retval.clone().ok_or_else(|| eyre!("Macro expansion time and SQL resolution time had differing opinions about the return value existing"))?;
-                let metadata_retval_sqls = match metadata_retval.return_sql {
+                let metadata_retval_sqls: Vec<String> = match metadata_retval.return_sql {
                         Ok(Returns::Table(variants)) => {
-                            let mut retval_sqls = vec![];
-                            for (idx, variant) in variants.iter().enumerate() {
-                                let sql = match variant {
+                            variants.iter().enumerate().map(|(idx, variant)| {
+                                match variant {
                                     SqlMapping::As(sql) => sql.clone(),
                                     SqlMapping::Composite { array_brackets } => {
                                         let composite = table_items[idx].ty.composite_type.unwrap();
@@ -287,10 +286,8 @@ impl ToSql for PgExternEntity {
                                         fmt::with_array_brackets(context.source_only_to_sql_type(table_items[idx].ty.ty_source).unwrap(), *array_brackets)
                                     }
                                     SqlMapping::Skip => todo!(),
-                                };
-                                retval_sqls.push(sql)
-                            }
-                            retval_sqls
+                                }
+                            }).collect()
                         },
                         Ok(_other) => return Err(eyre!("Got non-table return variant SQL in what macro-expansion thought was a table")),
                         Err(err) => return Err(err).wrap_err("Error mapping return SQL"),
@@ -302,11 +299,11 @@ impl ToSql for PgExternEntity {
                     let graph_index =
                         context.graph.neighbors_undirected(self_index).find(|neighbor| {
                             match &context.graph[*neighbor] {
-                                SqlGraphEntity::Type(neightbor_ty) => {
-                                    neightbor_ty.id_matches(&ty.ty_id)
+                                SqlGraphEntity::Type(neighbor_ty) => {
+                                    neighbor_ty.id_matches(&ty.ty_id)
                                 }
-                                SqlGraphEntity::Enum(neightbor_en) => {
-                                    neightbor_en.id_matches(&ty.ty_id)
+                                SqlGraphEntity::Enum(neighbor_en) => {
+                                    neighbor_en.id_matches(&ty.ty_id)
                                 }
                                 SqlGraphEntity::BuiltinType(defined) => defined == ty.ty_source,
                                 _ => false,

pgrx-sql-entity-graph/src/pg_extern/mod.rs

@@ -190,12 +190,11 @@ impl PgExtern {
                         } else if in_commented_sql_block && inner.value().trim() == "```" {
                             in_commented_sql_block = false;
                         } else if in_commented_sql_block {
-                            let sql = retval.get_or_insert_with(String::default);
                             let line = inner.value().trim_start().replace(
                                 "@FUNCTION_NAME@",
                                 &(self.func.sig.ident.to_string() + "_wrapper"),
                             ) + "\n";
-                            sql.push_str(&line);
+                            retval.get_or_insert_with(String::default).push_str(&line);
                         }
                     }
                 }

pgrx-sql-entity-graph/src/pg_trigger/entity.rs

@@ -44,6 +44,7 @@ impl ToSql for PgTriggerEntity {
         let self_index = context.triggers[self];
         let schema = context.schema_prefix_for(&self_index);
 
+        let PgTriggerEntity { file, line, full_path, function_name, .. } = self;
         let sql = format!(
             "\n\
             -- {file}:{line}\n\
@@ -52,11 +53,6 @@ impl ToSql for PgTriggerEntity {
                 \tRETURNS TRIGGER\n\
                 \tLANGUAGE c\n\
                 \tAS 'MODULE_PATHNAME', '{wrapper_function_name}';",
-            schema = schema,
-            file = self.file,
-            line = self.line,
-            full_path = self.full_path,
-            function_name = self.function_name,
             wrapper_function_name = self.wrapper_function_name(),
         );
         Ok(sql)