Skip to content

Commit f48ff1f

Browse files
weiznichweiznich
authored
Merge pull request diesel-rs#2972 from diesel-rs/derive-ff
Better error message span
2 parents 6aadc7f + df207e3 commit f48ff1f

11 files changed

Lines changed: 159 additions & 65 deletions

File tree

‎diesel_compile_tests/tests/fail/derive/bad_postgres_type.stderr‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,19 +47,19 @@ error: unexpected `oid` when `name` is present
4747

4848
= help: The correct format looks like either `#[diesel(postgres_type(name = "foo", schema = "public"))]` or `#[diesel(postgres_type(oid = 37, array_oid = 54))]`
4949

50-
--> $DIR/bad_postgres_type.rs:30:44
50+
--> $DIR/bad_postgres_type.rs:30:38
5151
|
5252
30 | #[diesel(postgres_type(name = "foo", oid = 2, array_oid = 3))]
53-
| ^
53+
| ^^^
5454

5555
error: unexpected `array_oid` when `name` is present
5656

5757
= help: The correct format looks like either `#[diesel(postgres_type(name = "foo", schema = "public"))]` or `#[diesel(postgres_type(oid = 37, array_oid = 54))]`
5858

59-
--> $DIR/bad_postgres_type.rs:34:50
59+
--> $DIR/bad_postgres_type.rs:34:38
6060
|
6161
34 | #[diesel(postgres_type(name = "foo", array_oid = 3))]
62-
| ^
62+
| ^^^^^^^^^
6363

6464
error: expected `oid` and `array_oid` attribute or `name` attribute
6565

@@ -86,10 +86,10 @@ error: expected `name` to be also present
8686

8787
= help: make sure `name` is present, `#[diesel(postgres_type(name = "...", schema = "foo"))]`
8888

89-
--> $DIR/bad_postgres_type.rs:50:33
89+
--> $DIR/bad_postgres_type.rs:50:24
9090
|
9191
50 | #[diesel(postgres_type(schema = "foo"))]
92-
| ^^^^^
92+
| ^^^^^^
9393

9494
error: unknown attribute, expected one of `oid`, `array_oid`, `name`, `schema`
9595
--> $DIR/bad_postgres_type.rs:54:24

‎diesel_compile_tests/tests/fail/derive/embed_and_serialize_as_cannot_be_mixed.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: `#[diesel(embed)]` cannot be combined with `#[diesel(serialize_as)]`
2-
--> $DIR/embed_and_serialize_as_cannot_be_mixed.rs:22:36
2+
--> $DIR/embed_and_serialize_as_cannot_be_mixed.rs:22:13
33
|
44
22 | #[diesel(embed, serialize_as = SomeType)]
5-
| ^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

‎diesel_compile_tests/tests/fail/derive_deprecated/deprecated_postgres_type.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,19 +47,19 @@ error: unexpected `oid` when `name` is present
4747

4848
= help: The correct format looks like either `#[diesel(postgres_type(name = "foo", schema = "public"))]` or `#[diesel(postgres_type(oid = 37, array_oid = 54))]`
4949

50-
--> $DIR/deprecated_postgres_type.rs:30:37
50+
--> $DIR/deprecated_postgres_type.rs:30:31
5151
|
5252
30 | #[postgres(type_name = "foo", oid = "2", array_oid = "3")]
53-
| ^^^
53+
| ^^^
5454

5555
error: unexpected `array_oid` when `name` is present
5656

5757
= help: The correct format looks like either `#[diesel(postgres_type(name = "foo", schema = "public"))]` or `#[diesel(postgres_type(oid = 37, array_oid = 54))]`
5858

59-
--> $DIR/deprecated_postgres_type.rs:34:43
59+
--> $DIR/deprecated_postgres_type.rs:34:31
6060
|
6161
34 | #[postgres(type_name = "foo", array_oid = "3")]
62-
| ^^^
62+
| ^^^^^^^^^
6363

6464
error: expected `oid` and `array_oid` attribute or `name` attribute
6565

‎diesel_derives/src/attrs.rs‎

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::fmt::{Display, Formatter};
22

33
use proc_macro2::{Span, TokenStream};
44
use proc_macro_error::ResultExt;
5+
use quote::spanned::Spanned;
56
use quote::ToTokens;
67
use syn::parse::discouraged::Speculative;
78
use syn::parse::{Parse, ParseStream, Parser, Result};
@@ -17,6 +18,12 @@ use util::{
1718
SQL_TYPE_NOTE, TABLE_NAME_NOTE, TREAT_NONE_AS_DEFAULT_VALUE_NOTE, TREAT_NONE_AS_NULL_NOTE,
1819
};
1920

21+
pub struct AttributeSpanWrapper<T> {
22+
pub item: T,
23+
pub attribute_span: Span,
24+
pub ident_span: Span,
25+
}
26+
2027
pub enum FieldAttr {
2128
Embed(Ident),
2229

@@ -118,6 +125,18 @@ impl Parse for FieldAttr {
118125
}
119126
}
120127

128+
impl Spanned for FieldAttr {
129+
fn __span(&self) -> Span {
130+
match self {
131+
FieldAttr::Embed(ident)
132+
| FieldAttr::ColumnName(ident, _)
133+
| FieldAttr::SqlType(ident, _)
134+
| FieldAttr::SerializeAs(ident, _)
135+
| FieldAttr::DeserializeAs(ident, _) => ident.span(),
136+
}
137+
}
138+
}
139+
121140
#[allow(clippy::large_enum_variant)]
122141
pub enum StructAttr {
123142
Aggregate(Ident),
@@ -203,15 +222,46 @@ impl Parse for StructAttr {
203222
}
204223
}
205224

206-
pub fn parse_attributes<T: Parse + ParseDeprecated>(attrs: &[Attribute]) -> Vec<T> {
225+
impl Spanned for StructAttr {
226+
fn __span(&self) -> Span {
227+
match self {
228+
StructAttr::Aggregate(ident)
229+
| StructAttr::NotSized(ident)
230+
| StructAttr::ForeignDerive(ident)
231+
| StructAttr::TableName(ident, _)
232+
| StructAttr::SqlType(ident, _)
233+
| StructAttr::TreatNoneAsDefaultValue(ident, _)
234+
| StructAttr::TreatNoneAsNull(ident, _)
235+
| StructAttr::BelongsTo(ident, _)
236+
| StructAttr::MysqlType(ident, _)
237+
| StructAttr::SqliteType(ident, _)
238+
| StructAttr::PostgresType(ident, _)
239+
| StructAttr::PrimaryKey(ident, _) => ident.span(),
240+
}
241+
}
242+
}
243+
244+
pub fn parse_attributes<T>(attrs: &[Attribute]) -> Vec<AttributeSpanWrapper<T>>
245+
where
246+
T: Parse + ParseDeprecated + Spanned,
247+
{
248+
use syn::spanned::Spanned;
249+
207250
attrs
208251
.iter()
209252
.flat_map(|attr| {
210253
if attr.path.is_ident("diesel") {
211254
attr.parse_args_with(Punctuated::<T, Comma>::parse_terminated)
212255
.unwrap_or_abort()
256+
.into_iter()
257+
.map(|a| AttributeSpanWrapper {
258+
ident_span: a.span(),
259+
item: a,
260+
attribute_span: attr.tokens.span(),
261+
})
262+
.collect::<Vec<_>>()
213263
} else {
214-
let mut p = Punctuated::new();
264+
let mut p = Vec::new();
215265
let Attribute { path, tokens, .. } = attr;
216266
let ident = path.get_ident().map(|f| f.to_string());
217267

@@ -223,10 +273,13 @@ pub fn parse_attributes<T: Parse + ParseDeprecated>(attrs: &[Attribute]) -> Vec<
223273
let value = Parser::parse(T::parse_deprecated, ts).unwrap_or_abort();
224274

225275
if let Some(value) = value {
226-
p.push_value(value);
276+
p.push(AttributeSpanWrapper {
277+
ident_span: value.span(),
278+
item: value,
279+
attribute_span: attr.tokens.span(),
280+
});
227281
}
228282
}
229-
230283
p
231284
}
232285
})

‎diesel_derives/src/deprecated/postgres_type.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ pub fn parse_postgres_type(name: Ident, input: ParseStream) -> Result<PostgresTy
5555

5656
for attr in Punctuated::<Attr, Comma>::parse_terminated(&content)? {
5757
match attr {
58-
Attr::Oid(_, value) => oid = Some(value),
59-
Attr::ArrayOid(_, value) => array_oid = Some(value),
60-
Attr::TypeName(_, value) => type_name = Some(value),
58+
Attr::Oid(ident, value) => oid = Some((ident, value)),
59+
Attr::ArrayOid(ident, value) => array_oid = Some((ident, value)),
60+
Attr::TypeName(ident, value) => type_name = Some((ident, value)),
6161
}
6262
}
6363

‎diesel_derives/src/field.rs‎

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,17 @@ use quote::ToTokens;
33
use syn::spanned::Spanned;
44
use syn::{Field as SynField, Ident, Index, Type};
55

6-
use attrs::{parse_attributes, FieldAttr, SqlIdentifier};
6+
use attrs::{parse_attributes, AttributeSpanWrapper, FieldAttr, SqlIdentifier};
77

88
pub struct Field {
99
pub ty: Type,
1010
pub span: Span,
1111
pub name: FieldName,
12-
column_name: Option<SqlIdentifier>,
13-
pub sql_type: Option<Type>,
14-
pub serialize_as: Option<Type>,
15-
pub deserialize_as: Option<Type>,
16-
pub embed: bool,
12+
column_name: Option<AttributeSpanWrapper<SqlIdentifier>>,
13+
pub sql_type: Option<AttributeSpanWrapper<Type>>,
14+
pub serialize_as: Option<AttributeSpanWrapper<Type>>,
15+
pub deserialize_as: Option<AttributeSpanWrapper<Type>>,
16+
pub embed: Option<AttributeSpanWrapper<bool>>,
1717
}
1818

1919
impl Field {
@@ -26,15 +26,47 @@ impl Field {
2626
let mut sql_type = None;
2727
let mut serialize_as = None;
2828
let mut deserialize_as = None;
29-
let mut embed = false;
29+
let mut embed = None;
3030

3131
for attr in parse_attributes(attrs) {
32-
match attr {
33-
FieldAttr::ColumnName(_, value) => column_name = Some(value),
34-
FieldAttr::SqlType(_, value) => sql_type = Some(Type::Path(value)),
35-
FieldAttr::SerializeAs(_, value) => serialize_as = Some(Type::Path(value)),
36-
FieldAttr::DeserializeAs(_, value) => deserialize_as = Some(Type::Path(value)),
37-
FieldAttr::Embed(_) => embed = true,
32+
let attribute_span = attr.attribute_span;
33+
let ident_span = attr.ident_span;
34+
match attr.item {
35+
FieldAttr::ColumnName(_, value) => {
36+
column_name = Some(AttributeSpanWrapper {
37+
item: value,
38+
attribute_span,
39+
ident_span,
40+
})
41+
}
42+
FieldAttr::SqlType(_, value) => {
43+
sql_type = Some(AttributeSpanWrapper {
44+
item: Type::Path(value),
45+
attribute_span,
46+
ident_span,
47+
})
48+
}
49+
FieldAttr::SerializeAs(_, value) => {
50+
serialize_as = Some(AttributeSpanWrapper {
51+
item: Type::Path(value),
52+
attribute_span,
53+
ident_span,
54+
})
55+
}
56+
FieldAttr::DeserializeAs(_, value) => {
57+
deserialize_as = Some(AttributeSpanWrapper {
58+
item: Type::Path(value),
59+
attribute_span,
60+
ident_span,
61+
})
62+
}
63+
FieldAttr::Embed(_) => {
64+
embed = Some(AttributeSpanWrapper {
65+
item: true,
66+
attribute_span,
67+
ident_span,
68+
})
69+
}
3870
}
3971
}
4072

@@ -61,24 +93,31 @@ impl Field {
6193
}
6294

6395
pub fn column_name(&self) -> SqlIdentifier {
64-
self.column_name.clone().unwrap_or_else(|| match self.name {
65-
FieldName::Named(ref x) => x.into(),
66-
FieldName::Unnamed(ref x) => {
67-
abort!(
96+
self.column_name
97+
.as_ref()
98+
.map(|a| a.item.clone())
99+
.unwrap_or_else(|| match self.name {
100+
FieldName::Named(ref x) => x.into(),
101+
FieldName::Unnamed(ref x) => {
102+
abort!(
68103
x,
69104
"All fields of tuple structs must be annotated with `#[diesel(column_name)]`"
70105
);
71-
}
72-
})
106+
}
107+
})
73108
}
74109

75110
pub fn ty_for_deserialize(&self) -> &Type {
76-
if let Some(value) = &self.deserialize_as {
111+
if let Some(AttributeSpanWrapper { item: value, .. }) = &self.deserialize_as {
77112
value
78113
} else {
79114
&self.ty
80115
}
81116
}
117+
118+
pub(crate) fn embed(&self) -> bool {
119+
self.embed.as_ref().map(|a| a.item).unwrap_or(false)
120+
}
82121
}
83122

84123
pub enum FieldName {

‎diesel_derives/src/insertable.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use proc_macro2::TokenStream;
22
use syn::{DeriveInput, Expr, Path, Type};
33

4+
use attrs::AttributeSpanWrapper;
45
use field::Field;
56
use model::Model;
67
use util::{inner_of_option_ty, is_option_ty, wrap_in_dummy_mod};
@@ -25,7 +26,7 @@ pub fn derive(item: DeriveInput) -> TokenStream {
2526
let mut ref_field_assign = Vec::with_capacity(model.fields().len());
2627

2728
for field in model.fields() {
28-
match (field.serialize_as.as_ref(), field.embed) {
29+
match (field.serialize_as.as_ref(), field.embed()) {
2930
(None, true) => {
3031
direct_field_ty.push(field_ty_embed(field, None));
3132
direct_field_assign.push(field_expr_embed(field, None));
@@ -58,7 +59,7 @@ pub fn derive(item: DeriveInput) -> TokenStream {
5859
treat_none_as_default_value,
5960
));
6061
}
61-
(Some(ty), false) => {
62+
(Some(AttributeSpanWrapper { item: ty, .. }), false) => {
6263
direct_field_ty.push(field_ty_serialize_as(
6364
field,
6465
table_name,
@@ -74,9 +75,9 @@ pub fn derive(item: DeriveInput) -> TokenStream {
7475

7576
generate_borrowed_insert = false; // as soon as we hit one field with #[diesel(serialize_as)] there is no point in generating the impl of Insertable for borrowed structs
7677
}
77-
(Some(ty), true) => {
78+
(Some(AttributeSpanWrapper { attribute_span, .. }), true) => {
7879
abort!(
79-
ty,
80+
attribute_span,
8081
"`#[diesel(embed)]` cannot be combined with `#[diesel(serialize_as)]`"
8182
)
8283
}

‎diesel_derives/src/model.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ impl Model {
6363
let mut postgres_type = None;
6464

6565
for attr in parse_attributes(attrs) {
66-
match attr {
66+
match attr.item {
6767
StructAttr::SqlType(_, value) => sql_types.push(Type::Path(value)),
6868
StructAttr::TableName(_, value) => table_name = Some(value),
6969
StructAttr::PrimaryKey(_, keys) => {

0 commit comments

Comments
 (0)