From c3c62912db8b3135f5ff03d4990e2bdbf8e30427 Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 26 Sep 2024 11:54:31 +0200 Subject: [PATCH] feat(stackable-versioned): Add conditional allow attr generation (#876) * feat(stackable-versioned): Add conditional allow attr generation * feat: Add allow attr to enum, carry over deprecated attr --- .../src/codegen/chain.rs | 31 ++++++++++---- .../src/codegen/common/item.rs | 42 ++++++++++++++----- .../src/codegen/venum/mod.rs | 42 +++++++++++++++++-- .../src/codegen/venum/variant.rs | 19 +++++++-- .../src/codegen/vstruct/field.rs | 20 +++++++-- .../src/codegen/vstruct/mod.rs | 41 ++++++++++++++++-- crates/stackable-versioned-macros/src/lib.rs | 2 +- .../tests/default/pass/deprecate_enum.rs | 16 +++++++ .../{deprecate.rs => deprecate_struct.rs} | 6 ++- .../tests/trybuild.rs | 13 +++--- 10 files changed, 188 insertions(+), 44 deletions(-) create mode 100644 crates/stackable-versioned-macros/tests/default/pass/deprecate_enum.rs rename crates/stackable-versioned-macros/tests/default/pass/{deprecate.rs => deprecate_struct.rs} (59%) diff --git a/crates/stackable-versioned-macros/src/codegen/chain.rs b/crates/stackable-versioned-macros/src/codegen/chain.rs index 7c6913dc5..2376e3ffd 100644 --- a/crates/stackable-versioned-macros/src/codegen/chain.rs +++ b/crates/stackable-versioned-macros/src/codegen/chain.rs @@ -4,8 +4,23 @@ pub(crate) trait Neighbors 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(&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)>; } @@ -14,15 +29,6 @@ impl Neighbors for BTreeMap 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 @@ -51,6 +57,13 @@ where } } + fn value_is(&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() } diff --git a/crates/stackable-versioned-macros/src/codegen/common/item.rs b/crates/stackable-versioned-macros/src/codegen/common/item.rs index f97a14311..d040e637f 100644 --- a/crates/stackable-versioned-macros/src/codegen/common/item.rs +++ b/crates/stackable-versioned-macros/src/codegen/common/item.rs @@ -303,6 +303,7 @@ where } => chain.insert( version.inner, ItemStatus::NoChange { + previously_deprecated: false, ident: from_ident.clone(), ty: from_type.clone(), }, @@ -310,13 +311,19 @@ where 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(), }, @@ -324,37 +331,51 @@ where 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(), }, @@ -398,6 +419,7 @@ pub(crate) enum ItemStatus { ident: Ident, }, NoChange { + previously_deprecated: bool, ident: Ident, ty: Type, }, diff --git a/crates/stackable-versioned-macros/src/codegen/venum/mod.rs b/crates/stackable-versioned-macros/src/codegen/venum/mod.rs index d6b691afb..14092e0b8 100644 --- a/crates/stackable-versioned-macros/src/codegen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/codegen/venum/mod.rs @@ -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, }, }; @@ -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 { @@ -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, + .. + } + ) + }) + }) + }) + } } diff --git a/crates/stackable-versioned-macros/src/codegen/venum/variant.rs b/crates/stackable-versioned-macros/src/codegen/venum/variant.rs index c6ec34486..1415d6bb1 100644 --- a/crates/stackable-versioned-macros/src/codegen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/codegen/venum/variant.rs @@ -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 => { diff --git a/crates/stackable-versioned-macros/src/codegen/vstruct/field.rs b/crates/stackable-versioned-macros/src/codegen/vstruct/field.rs index 81a9a9844..191303915 100644 --- a/crates/stackable-versioned-macros/src/codegen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/codegen/vstruct/field.rs @@ -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 => { diff --git a/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs index 4c9c08c62..f48a80c21 100644 --- a/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs @@ -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, }, @@ -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 { @@ -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 diff --git a/crates/stackable-versioned-macros/src/lib.rs b/crates/stackable-versioned-macros/src/lib.rs index 814b1c62b..d5ec7dea9 100644 --- a/crates/stackable-versioned-macros/src/lib.rs +++ b/crates/stackable-versioned-macros/src/lib.rs @@ -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()); # } ``` diff --git a/crates/stackable-versioned-macros/tests/default/pass/deprecate_enum.rs b/crates/stackable-versioned-macros/tests/default/pass/deprecate_enum.rs new file mode 100644 index 000000000..b19f36b88 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/default/pass/deprecate_enum.rs @@ -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, + } +} diff --git a/crates/stackable-versioned-macros/tests/default/pass/deprecate.rs b/crates/stackable-versioned-macros/tests/default/pass/deprecate_struct.rs similarity index 59% rename from crates/stackable-versioned-macros/tests/default/pass/deprecate.rs rename to crates/stackable-versioned-macros/tests/default/pass/deprecate_struct.rs index d525ee6b1..feeafd377 100644 --- a/crates/stackable-versioned-macros/tests/default/pass/deprecate.rs +++ b/crates/stackable-versioned-macros/tests/default/pass/deprecate_struct.rs @@ -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, } diff --git a/crates/stackable-versioned-macros/tests/trybuild.rs b/crates/stackable-versioned-macros/tests/trybuild.rs index dc0208c8c..fda62dee9 100644 --- a/crates/stackable-versioned-macros/tests/trybuild.rs +++ b/crates/stackable-versioned-macros/tests/trybuild.rs @@ -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 {