Skip to content

Commit 3937f57

Browse files
committed
feat: Basic support for type changes
The current implementation is very brittle and less than ideal. It currently only produces correct code in the struct definitions, but not in the From impls. For that reason, test files currently skip the From impl generation. I have a bunch of thoughts on how to improve the situation, but I would like to tackle these changes in follow-up commits.
1 parent 0bfa8da commit 3937f57

File tree

5 files changed

+95
-23
lines changed

5 files changed

+95
-23
lines changed

crates/stackable-versioned-macros/src/attrs/common/item.rs

+5
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ pub(crate) enum ItemType {
9797
Variant,
9898
}
9999

100+
// TODO (@Techassi): Shower thought: Track actions as a Vec of an ActionAttribute
101+
// enum and implement Ord on it. This would allow us to order the items by type
102+
// of action (added < changed < deprecated) as well as sort changed action to
103+
// each other by specified version (which already implements Ord)
104+
100105
/// These attributes are meant to be used in super structs, which add
101106
/// [`Field`](syn::Field) or [`Variant`](syn::Variant) specific attributes via
102107
/// darling's flatten feature. This struct only provides shared attributes.

crates/stackable-versioned-macros/src/codegen/common/item.rs

+53-11
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::{collections::BTreeMap, marker::PhantomData, ops::Deref};
22

33
use quote::format_ident;
4-
use syn::{spanned::Spanned, Attribute, Ident, Path};
4+
use syn::{spanned::Spanned, Attribute, Ident, Path, Type};
55

66
use crate::{
77
attrs::common::{ContainerAttributes, ItemAttributes, ValidateVersions},
@@ -21,7 +21,7 @@ use crate::{
2121
pub(crate) trait Item<I, A>: Sized
2222
where
2323
A: for<'i> TryFrom<&'i I> + Attributes,
24-
I: Named + Spanned,
24+
I: InnerItem,
2525
{
2626
/// Creates a new versioned item (struct field or enum variant) by consuming
2727
/// the parsed [Field](syn::Field) or [Variant](syn::Variant) and validating
@@ -43,6 +43,10 @@ where
4343
fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident>;
4444
}
4545

46+
pub(crate) trait InnerItem: Named + Spanned {
47+
fn ty(&self) -> Type;
48+
}
49+
4650
/// This trait enables access to the ident of named items, like fields and
4751
/// variants.
4852
///
@@ -87,7 +91,7 @@ pub(crate) trait Attributes {
8791
pub(crate) struct VersionedItem<I, A>
8892
where
8993
A: for<'i> TryFrom<&'i I> + Attributes,
90-
I: Named + Spanned,
94+
I: InnerItem,
9195
{
9296
pub(crate) original_attributes: Vec<Attribute>,
9397
pub(crate) chain: Option<VersionChain>,
@@ -99,7 +103,7 @@ impl<I, A> Item<I, A> for VersionedItem<I, A>
99103
where
100104
syn::Error: for<'i> From<<A as TryFrom<&'i I>>::Error>,
101105
A: for<'i> TryFrom<&'i I> + Attributes + ValidateVersions<I>,
102-
I: Named + Spanned,
106+
I: InnerItem,
103107
{
104108
fn new(item: I, container_attrs: &ContainerAttributes) -> syn::Result<Self> {
105109
// We use the TryFrom trait here, because the type parameter `A` can use
@@ -135,6 +139,8 @@ where
135139
// requires access to the item ident to infer the item ident for
136140
// the latest change.
137141
let mut ident = item.cleaned_ident();
142+
let mut ty = item.ty();
143+
138144
let mut actions = BTreeMap::new();
139145

140146
actions.insert(
@@ -147,20 +153,33 @@ where
147153
);
148154

149155
for change in common_attributes.changes.iter().rev() {
150-
let from = if let Some(from) = change.from_name.as_deref() {
156+
dbg!(&ty, &change.since);
157+
let from_ident = if let Some(from) = change.from_name.as_deref() {
151158
format_ident!("{from}")
152159
} else {
153160
ident.clone()
154161
};
155162

163+
// TODO (@Techassi): This is an awful lot of cloning, can we get
164+
// rid of it?
165+
let from_ty = change
166+
.from_type
167+
.as_ref()
168+
.map(|sv| sv.deref().clone())
169+
.unwrap_or(ty.clone());
170+
156171
actions.insert(
157172
*change.since,
158173
ItemStatus::Change {
159-
from_ident: from.clone(),
174+
from_ident: from_ident.clone(),
160175
to_ident: ident,
176+
from_type: from_ty.clone(),
177+
to_type: ty,
161178
},
162179
);
163-
ident = from;
180+
181+
ident = from_ident;
182+
ty = from_ty;
164183
}
165184

166185
// After the last iteration above (if any) we use the ident for the
@@ -171,6 +190,7 @@ where
171190
ItemStatus::Addition {
172191
default_fn: added.default_fn.deref().clone(),
173192
ident,
193+
ty,
174194
},
175195
);
176196
}
@@ -182,24 +202,39 @@ where
182202
inner: item,
183203
})
184204
} else if !common_attributes.changes.is_empty() {
185-
let mut actions = BTreeMap::new();
186205
let mut ident = item.ident().clone();
206+
let mut ty = item.ty();
207+
208+
let mut actions = BTreeMap::new();
187209

188210
for change in common_attributes.changes.iter().rev() {
189-
let from = if let Some(from) = change.from_name.as_deref() {
211+
dbg!(&ty, &change.since);
212+
let from_ident = if let Some(from) = change.from_name.as_deref() {
190213
format_ident!("{from}")
191214
} else {
192215
ident.clone()
193216
};
194217

218+
// TODO (@Techassi): This is an awful lot of cloning, can we get
219+
// rid of it?
220+
let from_ty = change
221+
.from_type
222+
.as_ref()
223+
.map(|sv| sv.deref().clone())
224+
.unwrap_or(ty.clone());
225+
195226
actions.insert(
196227
*change.since,
197228
ItemStatus::Change {
198-
from_ident: from.clone(),
229+
from_ident: from_ident.clone(),
199230
to_ident: ident,
231+
from_type: from_ty.clone(),
232+
to_type: ty,
200233
},
201234
);
202-
ident = from;
235+
236+
ident = from_ident;
237+
ty = from_ty;
203238
}
204239

205240
// After the last iteration above (if any) we use the ident for the
@@ -210,6 +245,7 @@ where
210245
ItemStatus::Addition {
211246
default_fn: added.default_fn.deref().clone(),
212247
ident,
248+
ty,
213249
},
214250
);
215251
}
@@ -229,6 +265,7 @@ where
229265
ItemStatus::Addition {
230266
default_fn: added.default_fn.deref().clone(),
231267
ident: item.ident().clone(),
268+
ty: item.ty(),
232269
},
233270
);
234271

@@ -314,10 +351,15 @@ pub(crate) enum ItemStatus {
314351
Addition {
315352
ident: Ident,
316353
default_fn: Path,
354+
// NOTE (@Techassi): We need to carry idents and type information in
355+
// nearly every status. Ideally, we would store this in separate maps.
356+
ty: Type,
317357
},
318358
Change {
319359
from_ident: Ident,
320360
to_ident: Ident,
361+
from_type: Type,
362+
to_type: Type,
321363
},
322364
Deprecation {
323365
previous_ident: Ident,

crates/stackable-versioned-macros/src/codegen/venum/variant.rs

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use std::ops::{Deref, DerefMut};
22

33
use darling::FromVariant;
4-
use proc_macro2::TokenStream;
4+
use proc_macro2::{Span, TokenStream};
55
use quote::quote;
6-
use syn::{Ident, Variant};
6+
use syn::{token::Not, Ident, Type, TypeNever, Variant};
77

88
use crate::{
99
attrs::{
@@ -13,8 +13,8 @@ use crate::{
1313
codegen::{
1414
chain::BTreeMapExt,
1515
common::{
16-
remove_deprecated_variant_prefix, Attributes, ContainerVersion, Item, ItemStatus,
17-
Named, VersionedItem,
16+
remove_deprecated_variant_prefix, Attributes, ContainerVersion, InnerItem, Item,
17+
ItemStatus, Named, VersionedItem,
1818
},
1919
},
2020
};
@@ -64,6 +64,17 @@ impl Attributes for VariantAttributes {
6464
}
6565
}
6666

67+
impl InnerItem for Variant {
68+
fn ty(&self) -> syn::Type {
69+
// FIXME (@Techassi): As we currently don't support enum variants with
70+
// data, we just return the Never type as the code generation code for
71+
// enum variants won't use this type information.
72+
Type::Never(TypeNever {
73+
bang_token: Not([Span::call_site()]),
74+
})
75+
}
76+
}
77+
6778
impl Named for Variant {
6879
fn cleaned_ident(&self) -> Ident {
6980
remove_deprecated_variant_prefix(self.ident())

crates/stackable-versioned-macros/src/codegen/vstruct/field.rs

+20-7
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use crate::{
1111
field::FieldAttributes,
1212
},
1313
codegen::common::{
14-
remove_deprecated_field_prefix, Attributes, ContainerVersion, Item, ItemStatus, Named,
15-
VersionedItem,
14+
remove_deprecated_field_prefix, Attributes, ContainerVersion, InnerItem, Item, ItemStatus,
15+
Named, VersionedItem,
1616
},
1717
};
1818

@@ -63,6 +63,12 @@ impl Attributes for FieldAttributes {
6363
}
6464
}
6565

66+
impl InnerItem for Field {
67+
fn ty(&self) -> syn::Type {
68+
self.ty.clone()
69+
}
70+
}
71+
6672
impl Named for Field {
6773
fn cleaned_ident(&self) -> Ident {
6874
let ident = self.ident();
@@ -115,13 +121,15 @@ impl VersionedField {
115121
container_version.inner
116122
)
117123
}) {
118-
ItemStatus::Addition { ident, .. } => Some(quote! {
124+
ItemStatus::Addition { ident, ty, .. } => Some(quote! {
119125
#(#original_attributes)*
120-
pub #ident: #field_type,
126+
pub #ident: #ty,
121127
}),
122-
ItemStatus::Change { to_ident, .. } => Some(quote! {
128+
ItemStatus::Change {
129+
to_ident, to_type, ..
130+
} => Some(quote! {
123131
#(#original_attributes)*
124-
pub #to_ident: #field_type,
132+
pub #to_ident: #to_type,
125133
}),
126134
ItemStatus::Deprecation {
127135
ident: field_ident,
@@ -170,7 +178,12 @@ impl VersionedField {
170178
.get(&next_version.inner)
171179
.expect("internal error: chain must contain container version"),
172180
) {
173-
(_, ItemStatus::Addition { ident, default_fn }) => quote! {
181+
(
182+
_,
183+
ItemStatus::Addition {
184+
ident, default_fn, ..
185+
},
186+
) => quote! {
174187
#ident: #default_fn(),
175188
},
176189
(old, next) => {

crates/stackable-versioned-macros/tests/good/basic.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ use stackable_versioned_macros::versioned;
99
version(name = "v1beta1"),
1010
version(name = "v1"),
1111
version(name = "v2"),
12-
version(name = "v3")
12+
version(name = "v3"),
13+
options(skip(from))
1314
)]
1415
pub(crate) struct Foo {
1516
#[versioned(

0 commit comments

Comments
 (0)