Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(stackable-versioned): Add conditional allow attr generation #876

Merged
merged 2 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions crates/stackable-versioned-macros/src/codegen/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,23 @@ pub(crate) trait Neighbors<K, V>
where
K: Ord + Eq,
{
/// Returns the values of keys which are neighbors of `key`.
///
/// Given a map which contains the following keys: 1, 3, 5. Calling this
/// function with these keys, results in the following return values:
///
/// - Key **0**: `(None, Some(1))`
/// - Key **2**: `(Some(1), Some(3))`
/// - Key **4**: `(Some(3), Some(5))`
/// - Key **6**: `(Some(5), None)`
fn get_neighbors(&self, key: &K) -> (Option<&V>, Option<&V>);

/// Returns whether the function `f` returns true if applied to the value
/// identified by `key`.
fn value_is<F>(&self, key: &K, f: F) -> bool
where
F: Fn(&V) -> bool;

fn lo_bound(&self, bound: Bound<&K>) -> Option<(&K, &V)>;
fn up_bound(&self, bound: Bound<&K>) -> Option<(&K, &V)>;
}
Expand All @@ -14,15 +29,6 @@ impl<K, V> Neighbors<K, V> for BTreeMap<K, V>
where
K: Ord + Eq,
{
/// Returns the values of keys which are neighbors of `key`.
///
/// Given a map which contains the following keys: 1, 3, 5. Calling this
/// function with these keys, results in the following return values:
///
/// - Key **0**: `(None, Some(1))`
/// - Key **2**: `(Some(1), Some(3))`
/// - Key **4**: `(Some(3), Some(5))`
/// - Key **6**: `(Some(5), None)`
fn get_neighbors(&self, key: &K) -> (Option<&V>, Option<&V>) {
// NOTE (@Techassi): These functions might get added to the standard
// library at some point. If that's the case, we can use the ones
Expand Down Expand Up @@ -51,6 +57,13 @@ where
}
}

fn value_is<F>(&self, key: &K, f: F) -> bool
where
F: Fn(&V) -> bool,
{
self.get(key).map_or(false, f)
}

fn lo_bound(&self, bound: Bound<&K>) -> Option<(&K, &V)> {
self.range((Bound::Unbounded, bound)).next_back()
}
Expand Down
42 changes: 32 additions & 10 deletions crates/stackable-versioned-macros/src/codegen/common/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,58 +303,79 @@ where
} => chain.insert(
version.inner,
ItemStatus::NoChange {
previously_deprecated: false,
ident: from_ident.clone(),
ty: from_type.clone(),
},
),
ItemStatus::Deprecation { previous_ident, .. } => chain.insert(
version.inner,
ItemStatus::NoChange {
previously_deprecated: false,
ident: previous_ident.clone(),
ty: self.inner.ty(),
},
),
ItemStatus::NoChange { ident, ty } => chain.insert(
ItemStatus::NoChange {
previously_deprecated,
ident,
ty,
} => chain.insert(
version.inner,
ItemStatus::NoChange {
previously_deprecated: *previously_deprecated,
ident: ident.clone(),
ty: ty.clone(),
},
),
ItemStatus::NotPresent => unreachable!(),
},
(Some(status), None) => {
let (ident, ty) = match status {
ItemStatus::Addition { ident, ty, .. } => (ident, ty),
let (ident, ty, previously_deprecated) = match status {
ItemStatus::Addition { ident, ty, .. } => (ident, ty, false),
ItemStatus::Change {
to_ident, to_type, ..
} => (to_ident, to_type),
ItemStatus::Deprecation { ident, .. } => (ident, &self.inner.ty()),
ItemStatus::NoChange { ident, ty } => (ident, ty),
} => (to_ident, to_type, false),
ItemStatus::Deprecation { ident, .. } => {
(ident, &self.inner.ty(), true)
}
ItemStatus::NoChange {
previously_deprecated,
ident,
ty,
..
} => (ident, ty, *previously_deprecated),
ItemStatus::NotPresent => unreachable!(),
};

chain.insert(
version.inner,
ItemStatus::NoChange {
previously_deprecated,
ident: ident.clone(),
ty: ty.clone(),
},
)
}
(Some(status), Some(_)) => {
let (ident, ty) = match status {
ItemStatus::Addition { ident, ty, .. } => (ident, ty),
let (ident, ty, previously_deprecated) = match status {
ItemStatus::Addition { ident, ty, .. } => (ident, ty, false),
ItemStatus::Change {
to_ident, to_type, ..
} => (to_ident, to_type),
ItemStatus::NoChange { ident, ty, .. } => (ident, ty),
} => (to_ident, to_type, false),
ItemStatus::NoChange {
previously_deprecated,
ident,
ty,
..
} => (ident, ty, *previously_deprecated),
_ => unreachable!(),
};

chain.insert(
version.inner,
ItemStatus::NoChange {
previously_deprecated,
ident: ident.clone(),
ty: ty.clone(),
},
Expand Down Expand Up @@ -398,6 +419,7 @@ pub(crate) enum ItemStatus {
ident: Ident,
},
NoChange {
previously_deprecated: bool,
ident: Ident,
ty: Type,
},
Expand Down
42 changes: 38 additions & 4 deletions crates/stackable-versioned-macros/src/codegen/venum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ use syn::{DataEnum, Error};
use crate::{
attrs::common::ContainerAttributes,
codegen::{
common::{Container, ContainerInput, ContainerVersion, Item, VersionedContainer},
chain::Neighbors,
common::{
Container, ContainerInput, ContainerVersion, Item, ItemStatus, VersionedContainer,
},
venum::variant::VersionedVariant,
},
};
Expand Down Expand Up @@ -193,11 +196,18 @@ impl VersionedEnum {
))
}

// TODO (@Techassi): Be a little bit more clever about when to include
// the #[allow(deprecated)] attribute.
// Include allow(deprecated) only when this or the next version is
// deprecated. Also include it, when a variant in this or the next
// version is deprecated.
let allow_attribute = (version.deprecated
|| next_version.deprecated
|| self.is_any_variant_deprecated(version)
|| self.is_any_variant_deprecated(next_version))
.then_some(quote! { #[allow(deprecated)] });

return quote! {
#[automatically_derived]
#[allow(deprecated)]
#allow_attribute
impl From<#module_name::#enum_ident> for #next_module_name::#enum_ident {
fn from(#from_ident: #module_name::#enum_ident) -> Self {
match #from_ident {
Expand All @@ -210,4 +220,28 @@ impl VersionedEnum {

quote! {}
}

/// Returns whether any field is deprecated in the provided
/// [`ContainerVersion`].
fn is_any_variant_deprecated(&self, version: &ContainerVersion) -> bool {
// First, iterate over all fields. Any will return true if any of the
// function invocations return true. If a field doesn't have a chain,
// we can safely default to false (unversioned fields cannot be
// deprecated). Then we retrieve the status of the field and ensure it
// is deprecated.
self.items.iter().any(|f| {
f.chain.as_ref().map_or(false, |c| {
c.value_is(&version.inner, |a| {
matches!(
a,
ItemStatus::Deprecation { .. }
| ItemStatus::NoChange {
previously_deprecated: true,
..
}
)
})
})
})
}
}
19 changes: 15 additions & 4 deletions crates/stackable-versioned-macros/src/codegen/venum/variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,21 @@ impl VersionedVariant {
#ident,
})
}
ItemStatus::NoChange { ident, .. } => Some(quote! {
#(#original_attributes)*
#ident,
}),
ItemStatus::NoChange {
previously_deprecated,
ident,
..
} => {
// TODO (@Techassi): Also carry along the deprecation
// note.
let deprecated_attr = previously_deprecated.then(|| quote! {#[deprecated]});

Some(quote! {
#(#original_attributes)*
#deprecated_attr
#ident,
})
}
ItemStatus::NotPresent => None,
},
None => {
Expand Down
20 changes: 16 additions & 4 deletions crates/stackable-versioned-macros/src/codegen/vstruct/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,22 @@ impl VersionedField {
})
}
ItemStatus::NotPresent => None,
ItemStatus::NoChange { ident, ty } => Some(quote! {
#(#original_attributes)*
pub #ident: #ty,
}),
ItemStatus::NoChange {
previously_deprecated,
ident,
ty,
..
} => {
// TODO (@Techassi): Also carry along the deprecation
// note.
let deprecated_attr = previously_deprecated.then(|| quote! {#[deprecated]});

Some(quote! {
#(#original_attributes)*
#deprecated_attr
pub #ident: #ty,
})
}
}
}
None => {
Expand Down
41 changes: 37 additions & 4 deletions crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ use syn::{parse_quote, DataStruct, Error, Ident};
use crate::{
attrs::common::ContainerAttributes,
codegen::{
chain::Neighbors,
common::{
Container, ContainerInput, ContainerVersion, Item, VersionExt, VersionedContainer,
Container, ContainerInput, ContainerVersion, Item, ItemStatus, VersionExt,
VersionedContainer,
},
vstruct::field::VersionedField,
},
Expand Down Expand Up @@ -241,11 +243,18 @@ impl VersionedStruct {

let fields = self.generate_from_fields(version, next_version, from_ident);

// TODO (@Techassi): Be a little bit more clever about when to include
// the #[allow(deprecated)] attribute.
// Include allow(deprecated) only when this or the next version is
// deprecated. Also include it, when a field in this or the next
// version is deprecated.
let allow_attribute = (version.deprecated
|| next_version.deprecated
|| self.is_any_field_deprecated(version)
|| self.is_any_field_deprecated(next_version))
.then_some(quote! { #[allow(deprecated)] });

return Some(quote! {
#[automatically_derived]
#[allow(deprecated)]
#allow_attribute
impl From<#module_name::#struct_ident> for #next_module_name::#struct_ident {
fn from(#from_ident: #module_name::#struct_ident) -> Self {
Self {
Expand Down Expand Up @@ -275,6 +284,30 @@ impl VersionedStruct {

token_stream
}

/// Returns whether any field is deprecated in the provided
/// [`ContainerVersion`].
fn is_any_field_deprecated(&self, version: &ContainerVersion) -> bool {
// First, iterate over all fields. Any will return true if any of the
// function invocations return true. If a field doesn't have a chain,
// we can safely default to false (unversioned fields cannot be
// deprecated). Then we retrieve the status of the field and ensure it
// is deprecated.
self.items.iter().any(|f| {
f.chain.as_ref().map_or(false, |c| {
c.value_is(&version.inner, |a| {
matches!(
a,
ItemStatus::Deprecation { .. }
| ItemStatus::NoChange {
previously_deprecated: true,
..
}
)
})
})
})
}
}

// Kubernetes specific code generation
Expand Down
2 changes: 1 addition & 1 deletion crates/stackable-versioned-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ pub struct FooSpec {
}

# fn main() {
let merged_crd = Foo::merged_crd("v1").unwrap();
let merged_crd = Foo::merged_crd(Foo::V1).unwrap();
println!("{}", serde_yaml::to_string(&merged_crd).unwrap());
# }
```
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
use stackable_versioned_macros::versioned;

fn main() {
#[versioned(
version(name = "v1alpha1"),
version(name = "v1beta1"),
version(name = "v1"),
version(name = "v2"),
version(name = "v3")
)]
enum Foo {
#[versioned(deprecated(since = "v1"))]
DeprecatedBar,
Baz,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ fn main() {
#[versioned(
version(name = "v1alpha1"),
version(name = "v1beta1"),
version(name = "v1")
version(name = "v1"),
version(name = "v2"),
version(name = "v3")
)]
struct Foo {
#[versioned(deprecated(since = "v1beta1", note = "gone"))]
#[versioned(deprecated(since = "v1", note = "gone"))]
deprecated_bar: usize,
baz: bool,
}
Expand Down
13 changes: 7 additions & 6 deletions crates/stackable-versioned-macros/tests/trybuild.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
#[allow(dead_code)]
mod default {
// mod pass {
// mod attributes_enum;
// mod attributes_struct;
// mod basic;
// // mod attributes_enum;
// // mod attributes_struct;
// // mod basic;

// mod deprecate;
// mod rename;
// mod skip_from_version;
// // mod deprecate_enum;
// // mod deprecate_struct;
// // mod rename;
// // mod skip_from_version;
// }

// mod fail {
Expand Down