Pipe forward spans by formatting idents (#1665)

Apply a simple change to -sql-entity-graph and -macros:

Use `format_ident!` for every case we format... an identifier... and we
don't change the identifier, and reuse the existing span or worse, drop
the span on the floor by calling `Span::call_site`. This will, in
general, improve the error-reporting of spans so that errors refer back
to the original input that provoked the expansion.

It is possible we should by relocating synthesized spans, see
rust-lang/rust#124145 for more details. This
change is still preferred because it makes it easier to refactor these
again.

pgrx-macros/src/lib.rs

@@ -13,7 +13,7 @@ use proc_macro::TokenStream;
 use std::collections::HashSet;
 
 use proc_macro2::Ident;
-use quote::{quote, ToTokens};
+use quote::{format_ident, quote, ToTokens};
 use syn::spanned::Spanned;
 use syn::{parse_macro_input, Attribute, Data, DeriveInput, Item, ItemImpl};
 
@@ -103,7 +103,7 @@ pub fn pg_test(attr: TokenStream, item: TokenStream) -> TokenStream {
             };
 
             let sql_funcname = func.sig.ident.to_string();
-            let test_func_name = Ident::new(&format!("pg_{}", func.sig.ident), func.span());
+            let test_func_name = format_ident!("pg_{}", func.sig.ident);
 
             let attributes = func.attrs;
             let mut att_stream = proc_macro2::TokenStream::new();

pgrx-macros/src/rewriter.rs

@@ -9,8 +9,7 @@
 //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
 extern crate proc_macro;
 
-use proc_macro2::Ident;
-use quote::{quote, quote_spanned};
+use quote::{format_ident, quote, quote_spanned};
 use std::ops::Deref;
 use std::str::FromStr;
 use syn::punctuated::Punctuated;
@@ -58,7 +57,7 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result<proc_macro2::Tok
     // nor do we need a visibility beyond "private"
     func.vis = Visibility::Inherited;
 
-    func.sig.ident = Ident::new(&format!("{}_inner", func.sig.ident), func.sig.ident.span());
+    func.sig.ident = format_ident!("{}_inner", func.sig.ident);
 
     // the wrapper_inner function declaration may contain lifetimes that are not used, since our input type is `FunctionCallInfo` mainly and return type is `Datum`
     let unused_lifetimes = match generics.lifetimes().next() {
@@ -157,7 +156,7 @@ fn build_arg_list(sig: &Signature, suffix_arg_name: bool) -> syn::Result<proc_ma
             FnArg::Typed(ty) => {
                 if let Pat::Ident(ident) = ty.pat.deref() {
                     if suffix_arg_name && ident.ident.to_string() != "fcinfo" {
-                        let ident = Ident::new(&format!("{}_", ident.ident), ident.span());
+                        let ident = format_ident!("{}_", ident.ident);
                         arg_list.extend(quote! { #ident, });
                     } else {
                         arg_list.extend(quote! { #ident, });
@@ -189,7 +188,7 @@ fn rename_arg_list(sig: &Signature) -> syn::Result<proc_macro2::TokenStream> {
             FnArg::Typed(ty) => {
                 if let Pat::Ident(ident) = ty.pat.deref() {
                     // prefix argument name with "arg_""
-                    let name = Ident::new(&format!("arg_{}", ident.ident), ident.ident.span());
+                    let name = format_ident!("arg_{}", ident.ident);
                     arg_list.extend(quote! { #name, });
                 } else {
                     return Err(syn::Error::new(

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

@@ -21,8 +21,8 @@ pub mod entity;
 use crate::positioning_ref::PositioningRef;
 
 use crate::enrich::{CodeEnrichment, ToEntityGraphTokens, ToRustCodeTokens};
-use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
-use quote::{quote, ToTokens, TokenStreamExt};
+use proc_macro2::{Ident, TokenStream as TokenStream2};
+use quote::{format_ident, quote, ToTokens, TokenStreamExt};
 use syn::parse::{Parse, ParseStream};
 use syn::punctuated::Punctuated;
 use syn::{LitStr, Token};
@@ -94,8 +94,7 @@ impl ToEntityGraphTokens for ExtensionSqlFile {
         );
         let requires_iter = requires.iter();
         let creates_iter = creates.iter();
-        let sql_graph_entity_fn_name =
-            syn::Ident::new(&format!("__pgrx_internals_sql_{}", name.clone()), Span::call_site());
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_sql_{}", name.clone());
         quote! {
             #[no_mangle]
             #[doc(hidden)]
@@ -192,8 +191,7 @@ impl ToEntityGraphTokens for ExtensionSql {
         let creates_iter = creates.iter();
         let name = &self.name;
 
-        let sql_graph_entity_fn_name =
-            syn::Ident::new(&format!("__pgrx_internals_sql_{}", name.value()), Span::call_site());
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_sql_{}", name.value());
         quote! {
             #[no_mangle]
             #[doc(hidden)]

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

@@ -38,7 +38,7 @@ use crate::enrich::CodeEnrichment;
 use crate::enrich::ToEntityGraphTokens;
 use crate::enrich::ToRustCodeTokens;
 use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
-use quote::{quote, quote_spanned, ToTokens};
+use quote::{format_ident, quote, quote_spanned, ToTokens};
 use syn::parse::{Parse, ParseStream, Parser};
 use syn::punctuated::Punctuated;
 use syn::spanned::Spanned;
@@ -337,8 +337,7 @@ impl PgExtern {
             .collect::<Vec<_>>();
         let hrtb = if lifetimes.is_empty() { None } else { Some(quote! { for<#(#lifetimes),*> }) };
 
-        let sql_graph_entity_fn_name =
-            syn::Ident::new(&format!("__pgrx_internals_fn_{}", ident), Span::call_site());
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_fn_{}", ident);
         quote_spanned! { self.func.sig.span() =>
             #[no_mangle]
             #[doc(hidden)]
@@ -374,10 +373,7 @@ impl PgExtern {
     }
 
     fn finfo_tokens(&self) -> TokenStream2 {
-        let finfo_name = syn::Ident::new(
-            &format!("pg_finfo_{}_wrapper", self.func.sig.ident),
-            Span::call_site(),
-        );
+        let finfo_name = format_ident!("pg_finfo_{}_wrapper", self.func.sig.ident);
         quote_spanned! { self.func.sig.span() =>
             #[no_mangle]
             #[doc(hidden)]
@@ -390,10 +386,7 @@ impl PgExtern {
 
     pub fn wrapper_func(&self) -> TokenStream2 {
         let func_name = &self.func.sig.ident;
-        let func_name_wrapper = Ident::new(
-            &format!("{}_wrapper", &self.func.sig.ident.to_string()),
-            self.func.sig.ident.span(),
-        );
+        let func_name_wrapper = format_ident!("{}_wrapper", &self.func.sig.ident);
         let func_generics = &self.func.sig.generics;
         // the wrapper function declaration may contain lifetimes that are not used, since our input type is `FunctionCallInfo` mainly and return type is `Datum`
         let unused_lifetimes = match func_generics.lifetimes().next() {
@@ -407,10 +400,7 @@ impl PgExtern {
         let fcinfo_ident = syn::Ident::new("_fcinfo", self.func.sig.ident.span());
 
         let args = &self.inputs;
-        let arg_pats = args
-            .iter()
-            .map(|v| syn::Ident::new(&format!("{}_", &v.pat), self.func.sig.span()))
-            .collect::<Vec<_>>();
+        let arg_pats = args.iter().map(|v| format_ident!("{}_", &v.pat)).collect::<Vec<_>>();
         let arg_fetches = args.iter().enumerate().map(|(idx, arg)| {
             let pat = &arg_pats[idx];
             let resolved_ty = &arg.used_ty.resolved_ty;

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

@@ -22,7 +22,7 @@ use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
 use crate::{CodeEnrichment, ToSqlConfig};
 use attribute::PgTriggerAttribute;
 use proc_macro2::{Span, TokenStream as TokenStream2};
-use quote::quote;
+use quote::{format_ident, quote};
 use syn::{ItemFn, Token};
 
 #[derive(Debug, Clone)]
@@ -66,10 +66,7 @@ impl PgTrigger {
 
     pub fn wrapper_tokens(&self) -> Result<ItemFn, syn::Error> {
         let function_ident = &self.func.sig.ident;
-        let extern_func_ident = syn::Ident::new(
-            &format!("{}_wrapper", self.func.sig.ident),
-            self.func.sig.ident.span(),
-        );
+        let extern_func_ident = format_ident!("{}_wrapper", self.func.sig.ident);
         let tokens = quote! {
             #[no_mangle]
             #[::pgrx::pgrx_macros::pg_guard]
@@ -103,10 +100,7 @@ impl PgTrigger {
     }
 
     pub fn finfo_tokens(&self) -> Result<ItemFn, syn::Error> {
-        let finfo_name = syn::Ident::new(
-            &format!("pg_finfo_{}_wrapper", self.func.sig.ident),
-            proc_macro2::Span::call_site(),
-        );
+        let finfo_name = format_ident!("pg_finfo_{}_wrapper", self.func.sig.ident);
         let tokens = quote! {
             #[no_mangle]
             #[doc(hidden)]
@@ -121,18 +115,15 @@ impl PgTrigger {
 
 impl ToEntityGraphTokens for PgTrigger {
     fn to_entity_graph_tokens(&self) -> TokenStream2 {
-        let sql_graph_entity_fn_name = syn::Ident::new(
-            &format!("__pgrx_internals_trigger_{}", self.func.sig.ident),
-            self.func.sig.ident.span(),
-        );
         let func_sig_ident = &self.func.sig.ident;
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_trigger_{}", func_sig_ident);
         let function_name = func_sig_ident.to_string();
         let to_sql_config = &self.to_sql_config;
 
         quote! {
             #[no_mangle]
             #[doc(hidden)]
-            #[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
+            #[allow(unknown_lints, clippy::no_mangle_with_rust_abi, nonstandard_style)]
             pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
                 use core::any::TypeId;
                 extern crate alloc;

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

@@ -20,7 +20,7 @@ pub mod entity;
 use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
 use crate::{CodeEnrichment, ToSqlConfig};
 use proc_macro2::{Span, TokenStream as TokenStream2};
-use quote::quote;
+use quote::{format_ident, quote};
 use syn::parse::{Parse, ParseStream};
 use syn::punctuated::Punctuated;
 use syn::{DeriveInput, Generics, Ident, ItemEnum, Token};
@@ -122,8 +122,7 @@ impl ToEntityGraphTokens for PostgresEnum {
             static_generics.split_for_impl();
 
         let variants = self.variants.iter().map(|variant| variant.ident.clone());
-        let sql_graph_entity_fn_name =
-            syn::Ident::new(&format!("__pgrx_internals_enum_{}", name), Span::call_site());
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_enum_{}", name);
 
         let to_sql_config = &self.to_sql_config;
 
@@ -140,7 +139,7 @@ impl ToEntityGraphTokens for PostgresEnum {
 
             #[no_mangle]
             #[doc(hidden)]
-            #[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
+            #[allow(unknown_lints, clippy::no_mangle_with_rust_abi, nonstandard_style)]
             pub extern "Rust" fn  #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
                 extern crate alloc;
                 use alloc::vec::Vec;

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

@@ -18,8 +18,8 @@ to the `pgrx` framework and very subject to change between versions. While you m
 pub mod entity;
 
 use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
-use proc_macro2::{Span, TokenStream as TokenStream2};
-use quote::quote;
+use proc_macro2::TokenStream as TokenStream2;
+use quote::{format_ident, quote};
 use syn::parse::{Parse, ParseStream};
 use syn::{DeriveInput, Ident};
 
@@ -99,13 +99,12 @@ impl PostgresHash {
 impl ToEntityGraphTokens for PostgresHash {
     fn to_entity_graph_tokens(&self) -> TokenStream2 {
         let name = &self.name;
-        let sql_graph_entity_fn_name =
-            syn::Ident::new(&format!("__pgrx_internals_hash_{}", self.name), Span::call_site());
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_hash_{}", self.name);
         let to_sql_config = &self.to_sql_config;
         quote! {
             #[no_mangle]
             #[doc(hidden)]
-            #[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
+            #[allow(nonstandard_style, unknown_lints, clippy::no_mangle_with_rust_abi)]
             pub extern "Rust" fn  #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
                 use core::any::TypeId;
                 extern crate alloc;

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

@@ -18,8 +18,8 @@ to the `pgrx` framework and very subject to change between versions. While you m
 pub mod entity;
 
 use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
-use proc_macro2::{Span, TokenStream as TokenStream2};
-use quote::quote;
+use proc_macro2::TokenStream as TokenStream2;
+use quote::{format_ident, quote};
 use syn::parse::{Parse, ParseStream};
 use syn::{DeriveInput, Ident};
 
@@ -100,13 +100,12 @@ impl PostgresOrd {
 impl ToEntityGraphTokens for PostgresOrd {
     fn to_entity_graph_tokens(&self) -> TokenStream2 {
         let name = &self.name;
-        let sql_graph_entity_fn_name =
-            syn::Ident::new(&format!("__pgrx_internals_ord_{}", self.name), Span::call_site());
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_ord_{}", self.name);
         let to_sql_config = &self.to_sql_config;
         quote! {
             #[no_mangle]
             #[doc(hidden)]
-            #[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
+            #[allow(nonstandard_style, unknown_lints, clippy::no_mangle_with_rust_abi)]
             pub extern "Rust" fn  #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
                 use core::any::TypeId;
                 extern crate alloc;

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

@@ -18,8 +18,8 @@ to the `pgrx` framework and very subject to change between versions. While you m
 pub mod entity;
 
 use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
-use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
-use quote::quote;
+use proc_macro2::{Ident, TokenStream as TokenStream2};
+use quote::{format_ident, quote};
 use syn::parse::{Parse, ParseStream};
 use syn::{DeriveInput, Generics, ItemStruct, Lifetime, LifetimeParam};
 
@@ -125,8 +125,7 @@ impl ToEntityGraphTokens for PostgresTypeDerive {
         let in_fn = &self.in_fn;
         let out_fn = &self.out_fn;
 
-        let sql_graph_entity_fn_name =
-            syn::Ident::new(&format!("__pgrx_internals_type_{}", self.name), Span::call_site());
+        let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_type_{}", self.name);
 
         let to_sql_config = &self.to_sql_config;
 
@@ -144,7 +143,7 @@ impl ToEntityGraphTokens for PostgresTypeDerive {
 
             #[no_mangle]
             #[doc(hidden)]
-            #[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
+            #[allow(nonstandard_style, unknown_lints, clippy::no_mangle_with_rust_abi)]
             pub extern "Rust" fn  #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
                 extern crate alloc;
                 use alloc::vec::Vec;

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

@@ -18,7 +18,7 @@ to the `pgrx` framework and very subject to change between versions. While you m
 pub mod entity;
 
 use proc_macro2::TokenStream as TokenStream2;
-use quote::{quote, ToTokens, TokenStreamExt};
+use quote::{format_ident, quote, ToTokens, TokenStreamExt};
 use syn::parse::{Parse, ParseStream};
 use syn::ItemMod;
 
@@ -77,10 +77,8 @@ impl Schema {
             // End of hack
         };
 
-        let sql_graph_entity_fn_name = syn::Ident::new(
-            &format!("__pgrx_internals_schema_{}_{}", ident, postfix),
-            proc_macro2::Span::call_site(),
-        );
+        let sql_graph_entity_fn_name =
+            format_ident!("__pgrx_internals_schema_{}_{}", ident, postfix);
         quote! {
             #[no_mangle]
             #[doc(hidden)]

pgrx-tests/tests/todo/roundtrip-tests.stderr

@@ -9,15 +9,6 @@ error[E0261]: use of undeclared lifetime name `'a`
 69 |         Vec<Option<&'a str>>,
    |                     ^^ undeclared lifetime
 
-error[E0261]: use of undeclared lifetime name `'a`
-  --> tests/todo/roundtrip-tests.rs:69:21
-   |
-21 |             #[pg_extern]
-   |                         - lifetime `'a` is missing in item created through this procedural macro
-...
-69 |         Vec<Option<&'a str>>,
-   |                     ^^ undeclared lifetime
-
 error[E0261]: use of undeclared lifetime name `'a`
   --> tests/todo/roundtrip-tests.rs:69:21
    |