Skip to content

Commit 987051a

Browse files
workingjubileeworkingjubilee
authored
Pipe forward spans by formatting idents (pgcentralfoundation#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.
1 parent 23223b4 commit 987051a

File tree

11 files changed

+38
-75
lines changed

11 files changed

+38
-75
lines changed

‎pgrx-macros/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use proc_macro::TokenStream;
1313
use std::collections::HashSet;
1414

1515
use proc_macro2::Ident;
16-
use quote::{quote, ToTokens};
16+
use quote::{format_ident, quote, ToTokens};
1717
use syn::spanned::Spanned;
1818
use syn::{parse_macro_input, Attribute, Data, DeriveInput, Item, ItemImpl};
1919

@@ -103,7 +103,7 @@ pub fn pg_test(attr: TokenStream, item: TokenStream) -> TokenStream {
103103
};
104104

105105
let sql_funcname = func.sig.ident.to_string();
106-
let test_func_name = Ident::new(&format!("pg_{}", func.sig.ident), func.span());
106+
let test_func_name = format_ident!("pg_{}", func.sig.ident);
107107

108108
let attributes = func.attrs;
109109
let mut att_stream = proc_macro2::TokenStream::new();

‎pgrx-macros/src/rewriter.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99
//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
1010
extern crate proc_macro;
1111

12-
use proc_macro2::Ident;
13-
use quote::{quote, quote_spanned};
12+
use quote::{format_ident, quote, quote_spanned};
1413
use std::ops::Deref;
1514
use std::str::FromStr;
1615
use syn::punctuated::Punctuated;
@@ -58,7 +57,7 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result<proc_macro2::Tok
5857
// nor do we need a visibility beyond "private"
5958
func.vis = Visibility::Inherited;
6059

61-
func.sig.ident = Ident::new(&format!("{}_inner", func.sig.ident), func.sig.ident.span());
60+
func.sig.ident = format_ident!("{}_inner", func.sig.ident);
6261

6362
// the wrapper_inner function declaration may contain lifetimes that are not used, since our input type is `FunctionCallInfo` mainly and return type is `Datum`
6463
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
157156
FnArg::Typed(ty) => {
158157
if let Pat::Ident(ident) = ty.pat.deref() {
159158
if suffix_arg_name && ident.ident.to_string() != "fcinfo" {
160-
let ident = Ident::new(&format!("{}_", ident.ident), ident.span());
159+
let ident = format_ident!("{}_", ident.ident);
161160
arg_list.extend(quote! { #ident, });
162161
} else {
163162
arg_list.extend(quote! { #ident, });
@@ -189,7 +188,7 @@ fn rename_arg_list(sig: &Signature) -> syn::Result<proc_macro2::TokenStream> {
189188
FnArg::Typed(ty) => {
190189
if let Pat::Ident(ident) = ty.pat.deref() {
191190
// prefix argument name with "arg_""
192-
let name = Ident::new(&format!("arg_{}", ident.ident), ident.ident.span());
191+
let name = format_ident!("arg_{}", ident.ident);
193192
arg_list.extend(quote! { #name, });
194193
} else {
195194
return Err(syn::Error::new(

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

+4-6
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ pub mod entity;
2121
use crate::positioning_ref::PositioningRef;
2222

2323
use crate::enrich::{CodeEnrichment, ToEntityGraphTokens, ToRustCodeTokens};
24-
use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
25-
use quote::{quote, ToTokens, TokenStreamExt};
24+
use proc_macro2::{Ident, TokenStream as TokenStream2};
25+
use quote::{format_ident, quote, ToTokens, TokenStreamExt};
2626
use syn::parse::{Parse, ParseStream};
2727
use syn::punctuated::Punctuated;
2828
use syn::{LitStr, Token};
@@ -94,8 +94,7 @@ impl ToEntityGraphTokens for ExtensionSqlFile {
9494
);
9595
let requires_iter = requires.iter();
9696
let creates_iter = creates.iter();
97-
let sql_graph_entity_fn_name =
98-
syn::Ident::new(&format!("__pgrx_internals_sql_{}", name.clone()), Span::call_site());
97+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_sql_{}", name.clone());
9998
quote! {
10099
#[no_mangle]
101100
#[doc(hidden)]
@@ -192,8 +191,7 @@ impl ToEntityGraphTokens for ExtensionSql {
192191
let creates_iter = creates.iter();
193192
let name = &self.name;
194193

195-
let sql_graph_entity_fn_name =
196-
syn::Ident::new(&format!("__pgrx_internals_sql_{}", name.value()), Span::call_site());
194+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_sql_{}", name.value());
197195
quote! {
198196
#[no_mangle]
199197
#[doc(hidden)]

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

+5-15
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use crate::enrich::CodeEnrichment;
3838
use crate::enrich::ToEntityGraphTokens;
3939
use crate::enrich::ToRustCodeTokens;
4040
use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
41-
use quote::{quote, quote_spanned, ToTokens};
41+
use quote::{format_ident, quote, quote_spanned, ToTokens};
4242
use syn::parse::{Parse, ParseStream, Parser};
4343
use syn::punctuated::Punctuated;
4444
use syn::spanned::Spanned;
@@ -337,8 +337,7 @@ impl PgExtern {
337337
.collect::<Vec<_>>();
338338
let hrtb = if lifetimes.is_empty() { None } else { Some(quote! { for<#(#lifetimes),*> }) };
339339

340-
let sql_graph_entity_fn_name =
341-
syn::Ident::new(&format!("__pgrx_internals_fn_{}", ident), Span::call_site());
340+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_fn_{}", ident);
342341
quote_spanned! { self.func.sig.span() =>
343342
#[no_mangle]
344343
#[doc(hidden)]
@@ -374,10 +373,7 @@ impl PgExtern {
374373
}
375374

376375
fn finfo_tokens(&self) -> TokenStream2 {
377-
let finfo_name = syn::Ident::new(
378-
&format!("pg_finfo_{}_wrapper", self.func.sig.ident),
379-
Span::call_site(),
380-
);
376+
let finfo_name = format_ident!("pg_finfo_{}_wrapper", self.func.sig.ident);
381377
quote_spanned! { self.func.sig.span() =>
382378
#[no_mangle]
383379
#[doc(hidden)]
@@ -390,10 +386,7 @@ impl PgExtern {
390386

391387
pub fn wrapper_func(&self) -> TokenStream2 {
392388
let func_name = &self.func.sig.ident;
393-
let func_name_wrapper = Ident::new(
394-
&format!("{}_wrapper", &self.func.sig.ident.to_string()),
395-
self.func.sig.ident.span(),
396-
);
389+
let func_name_wrapper = format_ident!("{}_wrapper", &self.func.sig.ident);
397390
let func_generics = &self.func.sig.generics;
398391
// the wrapper function declaration may contain lifetimes that are not used, since our input type is `FunctionCallInfo` mainly and return type is `Datum`
399392
let unused_lifetimes = match func_generics.lifetimes().next() {
@@ -407,10 +400,7 @@ impl PgExtern {
407400
let fcinfo_ident = syn::Ident::new("_fcinfo", self.func.sig.ident.span());
408401

409402
let args = &self.inputs;
410-
let arg_pats = args
411-
.iter()
412-
.map(|v| syn::Ident::new(&format!("{}_", &v.pat), self.func.sig.span()))
413-
.collect::<Vec<_>>();
403+
let arg_pats = args.iter().map(|v| format_ident!("{}_", &v.pat)).collect::<Vec<_>>();
414404
let arg_fetches = args.iter().enumerate().map(|(idx, arg)| {
415405
let pat = &arg_pats[idx];
416406
let resolved_ty = &arg.used_ty.resolved_ty;

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

+5-14
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
2222
use crate::{CodeEnrichment, ToSqlConfig};
2323
use attribute::PgTriggerAttribute;
2424
use proc_macro2::{Span, TokenStream as TokenStream2};
25-
use quote::quote;
25+
use quote::{format_ident, quote};
2626
use syn::{ItemFn, Token};
2727

2828
#[derive(Debug, Clone)]
@@ -66,10 +66,7 @@ impl PgTrigger {
6666

6767
pub fn wrapper_tokens(&self) -> Result<ItemFn, syn::Error> {
6868
let function_ident = &self.func.sig.ident;
69-
let extern_func_ident = syn::Ident::new(
70-
&format!("{}_wrapper", self.func.sig.ident),
71-
self.func.sig.ident.span(),
72-
);
69+
let extern_func_ident = format_ident!("{}_wrapper", self.func.sig.ident);
7370
let tokens = quote! {
7471
#[no_mangle]
7572
#[::pgrx::pgrx_macros::pg_guard]
@@ -103,10 +100,7 @@ impl PgTrigger {
103100
}
104101

105102
pub fn finfo_tokens(&self) -> Result<ItemFn, syn::Error> {
106-
let finfo_name = syn::Ident::new(
107-
&format!("pg_finfo_{}_wrapper", self.func.sig.ident),
108-
proc_macro2::Span::call_site(),
109-
);
103+
let finfo_name = format_ident!("pg_finfo_{}_wrapper", self.func.sig.ident);
110104
let tokens = quote! {
111105
#[no_mangle]
112106
#[doc(hidden)]
@@ -121,18 +115,15 @@ impl PgTrigger {
121115

122116
impl ToEntityGraphTokens for PgTrigger {
123117
fn to_entity_graph_tokens(&self) -> TokenStream2 {
124-
let sql_graph_entity_fn_name = syn::Ident::new(
125-
&format!("__pgrx_internals_trigger_{}", self.func.sig.ident),
126-
self.func.sig.ident.span(),
127-
);
128118
let func_sig_ident = &self.func.sig.ident;
119+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_trigger_{}", func_sig_ident);
129120
let function_name = func_sig_ident.to_string();
130121
let to_sql_config = &self.to_sql_config;
131122

132123
quote! {
133124
#[no_mangle]
134125
#[doc(hidden)]
135-
#[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
126+
#[allow(unknown_lints, clippy::no_mangle_with_rust_abi, nonstandard_style)]
136127
pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
137128
use core::any::TypeId;
138129
extern crate alloc;

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub mod entity;
2020
use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
2121
use crate::{CodeEnrichment, ToSqlConfig};
2222
use proc_macro2::{Span, TokenStream as TokenStream2};
23-
use quote::quote;
23+
use quote::{format_ident, quote};
2424
use syn::parse::{Parse, ParseStream};
2525
use syn::punctuated::Punctuated;
2626
use syn::{DeriveInput, Generics, Ident, ItemEnum, Token};
@@ -122,8 +122,7 @@ impl ToEntityGraphTokens for PostgresEnum {
122122
static_generics.split_for_impl();
123123

124124
let variants = self.variants.iter().map(|variant| variant.ident.clone());
125-
let sql_graph_entity_fn_name =
126-
syn::Ident::new(&format!("__pgrx_internals_enum_{}", name), Span::call_site());
125+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_enum_{}", name);
127126

128127
let to_sql_config = &self.to_sql_config;
129128

@@ -140,7 +139,7 @@ impl ToEntityGraphTokens for PostgresEnum {
140139

141140
#[no_mangle]
142141
#[doc(hidden)]
143-
#[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
142+
#[allow(unknown_lints, clippy::no_mangle_with_rust_abi, nonstandard_style)]
144143
pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
145144
extern crate alloc;
146145
use alloc::vec::Vec;

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ to the `pgrx` framework and very subject to change between versions. While you m
1818
pub mod entity;
1919

2020
use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
21-
use proc_macro2::{Span, TokenStream as TokenStream2};
22-
use quote::quote;
21+
use proc_macro2::TokenStream as TokenStream2;
22+
use quote::{format_ident, quote};
2323
use syn::parse::{Parse, ParseStream};
2424
use syn::{DeriveInput, Ident};
2525

@@ -99,13 +99,12 @@ impl PostgresHash {
9999
impl ToEntityGraphTokens for PostgresHash {
100100
fn to_entity_graph_tokens(&self) -> TokenStream2 {
101101
let name = &self.name;
102-
let sql_graph_entity_fn_name =
103-
syn::Ident::new(&format!("__pgrx_internals_hash_{}", self.name), Span::call_site());
102+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_hash_{}", self.name);
104103
let to_sql_config = &self.to_sql_config;
105104
quote! {
106105
#[no_mangle]
107106
#[doc(hidden)]
108-
#[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
107+
#[allow(nonstandard_style, unknown_lints, clippy::no_mangle_with_rust_abi)]
109108
pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
110109
use core::any::TypeId;
111110
extern crate alloc;

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ to the `pgrx` framework and very subject to change between versions. While you m
1818
pub mod entity;
1919

2020
use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
21-
use proc_macro2::{Span, TokenStream as TokenStream2};
22-
use quote::quote;
21+
use proc_macro2::TokenStream as TokenStream2;
22+
use quote::{format_ident, quote};
2323
use syn::parse::{Parse, ParseStream};
2424
use syn::{DeriveInput, Ident};
2525

@@ -100,13 +100,12 @@ impl PostgresOrd {
100100
impl ToEntityGraphTokens for PostgresOrd {
101101
fn to_entity_graph_tokens(&self) -> TokenStream2 {
102102
let name = &self.name;
103-
let sql_graph_entity_fn_name =
104-
syn::Ident::new(&format!("__pgrx_internals_ord_{}", self.name), Span::call_site());
103+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_ord_{}", self.name);
105104
let to_sql_config = &self.to_sql_config;
106105
quote! {
107106
#[no_mangle]
108107
#[doc(hidden)]
109-
#[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
108+
#[allow(nonstandard_style, unknown_lints, clippy::no_mangle_with_rust_abi)]
110109
pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
111110
use core::any::TypeId;
112111
extern crate alloc;

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ to the `pgrx` framework and very subject to change between versions. While you m
1818
pub mod entity;
1919

2020
use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens};
21-
use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
22-
use quote::quote;
21+
use proc_macro2::{Ident, TokenStream as TokenStream2};
22+
use quote::{format_ident, quote};
2323
use syn::parse::{Parse, ParseStream};
2424
use syn::{DeriveInput, Generics, ItemStruct, Lifetime, LifetimeParam};
2525

@@ -125,8 +125,7 @@ impl ToEntityGraphTokens for PostgresTypeDerive {
125125
let in_fn = &self.in_fn;
126126
let out_fn = &self.out_fn;
127127

128-
let sql_graph_entity_fn_name =
129-
syn::Ident::new(&format!("__pgrx_internals_type_{}", self.name), Span::call_site());
128+
let sql_graph_entity_fn_name = format_ident!("__pgrx_internals_type_{}", self.name);
130129

131130
let to_sql_config = &self.to_sql_config;
132131

@@ -144,7 +143,7 @@ impl ToEntityGraphTokens for PostgresTypeDerive {
144143

145144
#[no_mangle]
146145
#[doc(hidden)]
147-
#[allow(unknown_lints, clippy::no_mangle_with_rust_abi)]
146+
#[allow(nonstandard_style, unknown_lints, clippy::no_mangle_with_rust_abi)]
148147
pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity {
149148
extern crate alloc;
150149
use alloc::vec::Vec;

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

+3-5
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ to the `pgrx` framework and very subject to change between versions. While you m
1818
pub mod entity;
1919

2020
use proc_macro2::TokenStream as TokenStream2;
21-
use quote::{quote, ToTokens, TokenStreamExt};
21+
use quote::{format_ident, quote, ToTokens, TokenStreamExt};
2222
use syn::parse::{Parse, ParseStream};
2323
use syn::ItemMod;
2424

@@ -77,10 +77,8 @@ impl Schema {
7777
// End of hack
7878
};
7979

80-
let sql_graph_entity_fn_name = syn::Ident::new(
81-
&format!("__pgrx_internals_schema_{}_{}", ident, postfix),
82-
proc_macro2::Span::call_site(),
83-
);
80+
let sql_graph_entity_fn_name =
81+
format_ident!("__pgrx_internals_schema_{}_{}", ident, postfix);
8482
quote! {
8583
#[no_mangle]
8684
#[doc(hidden)]

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

-9
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,6 @@ error[E0261]: use of undeclared lifetime name `'a`
99
69 | Vec<Option<&'a str>>,
1010
| ^^ undeclared lifetime
1111

12-
error[E0261]: use of undeclared lifetime name `'a`
13-
--> tests/todo/roundtrip-tests.rs:69:21
14-
|
15-
21 | #[pg_extern]
16-
| - lifetime `'a` is missing in item created through this procedural macro
17-
...
18-
69 | Vec<Option<&'a str>>,
19-
| ^^ undeclared lifetime
20-
2112
error[E0261]: use of undeclared lifetime name `'a`
2213
--> tests/todo/roundtrip-tests.rs:69:21
2314
|

0 commit comments

Comments
 (0)