diff --git a/crates/stackable-versioned-macros/src/attrs/item/field.rs b/crates/stackable-versioned-macros/src/attrs/item/field.rs index f088bc438..359854744 100644 --- a/crates/stackable-versioned-macros/src/attrs/item/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/item/field.rs @@ -1,4 +1,4 @@ -use darling::{Error, FromField, Result, util::Flag}; +use darling::{Error, FromField, FromMeta, Result, util::Flag}; use syn::{Attribute, Ident}; use crate::{ @@ -44,6 +44,10 @@ pub struct FieldAttributes { /// is needed to let the macro know to generate conversion code with support /// for tracking across struct boundaries. pub nested: Flag, + + /// Provide a hint if a field is wrapped in either `Option` or `Vec` to + /// generate correct code in the `From` impl blocks. + pub hint: Option, } impl FieldAttributes { @@ -81,3 +85,10 @@ impl FieldAttributes { Ok(()) } } + +/// Supported field hints. +#[derive(Debug, FromMeta)] +pub enum Hint { + Option, + Vec, +} diff --git a/crates/stackable-versioned-macros/src/codegen/item/field.rs b/crates/stackable-versioned-macros/src/codegen/item/field.rs index 5579f6f3f..0134b1b6f 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/field.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/field.rs @@ -4,10 +4,10 @@ use darling::{FromField, Result, util::IdentString}; use k8s_version::Version; use proc_macro2::TokenStream; use quote::quote; -use syn::{Attribute, Field, Ident, Type}; +use syn::{Attribute, Field, Ident, Path, Type}; use crate::{ - attrs::item::FieldAttributes, + attrs::item::{FieldAttributes, Hint}, codegen::{ Direction, VersionDefinition, changes::{BTreeMapExt, ChangesetExt}, @@ -21,6 +21,7 @@ pub struct VersionedField { pub original_attributes: Vec, pub changes: Option>, pub idents: FieldIdents, + pub hint: Option, pub nested: bool, pub ty: Type, } @@ -47,6 +48,7 @@ impl VersionedField { Ok(Self { original_attributes: field_attributes.attrs, + hint: field_attributes.hint, ty: field.ty, changes, idents, @@ -60,19 +62,31 @@ impl VersionedField { } } + /// Generates field definitions for the use inside container (struct) definitions. + /// + /// This function needs to take into account multiple conditions: + /// + /// - Only emit the field if it exists for the currently generated version. + /// - Emit field with new name and type if there was a name and/or type change. + /// - Handle deprecated fields accordingly. + /// + /// ### Example + /// + /// ```ignore + /// struct Foo { + /// bar: usize, // This function generates one or more of these definitions + /// } + /// ``` pub fn generate_for_container(&self, version: &VersionDefinition) -> Option { let original_attributes = &self.original_attributes; match &self.changes { Some(changes) => { - // Check if the provided container version is present in the map - // of actions. If it is, some action occurred in exactly that - // version and thus code is generated for that field based on - // the type of action. - // If not, the provided version has no action attached to it. - // The code generation then depends on the relation to other - // versions (with actions). - + // Check if the provided container version is present in the map of actions. If it + // is, some action occurred in exactly that version and thus code is generated for + // that field based on the type of action. + // If not, the provided version has no action attached to it. The code generation + // then depends on the relation to other versions (with actions). let field_type = &self.ty; // NOTE (@Techassi): https://rust-lang.github.io/rust-clippy/master/index.html#/expect_fun_call @@ -97,14 +111,12 @@ impl VersionedField { note, .. } => { - // FIXME (@Techassi): Emitting the deprecated attribute - // should cary over even when the item status is - // 'NoChange'. - // TODO (@Techassi): Make the generation of deprecated - // items customizable. When a container is used as a K8s - // CRD, the item must continue to exist, even when - // deprecated. For other versioning use-cases, that - // might not be the case. + // FIXME (@Techassi): Emitting the deprecated attribute should cary over even + // when the item status is 'NoChange'. + // TODO (@Techassi): Make the generation of deprecated items customizable. + // When a container is used as a K8s CRD, the item must continue to exist, + // even when deprecated. For other versioning use-cases, that might not be + // the case. let deprecated_attr = if let Some(note) = note { quote! {#[deprecated = #note]} } else { @@ -124,8 +136,7 @@ impl VersionedField { ty, .. } => { - // TODO (@Techassi): Also carry along the deprecation - // note. + // TODO (@Techassi): Also carry along the deprecation note. let deprecated_attr = previously_deprecated.then(|| quote! {#[deprecated]}); Some(quote! { @@ -137,8 +148,8 @@ impl VersionedField { } } None => { - // If there is no chain of field actions, the field is not - // versioned and therefore included in all versions. + // If there is no chain of field actions, the field is not versioned and therefore + // included in all versions. let field_ident = &self.idents.original; let field_type = &self.ty; @@ -150,6 +161,27 @@ impl VersionedField { } } + /// Generates field definitions for the use inside `From` impl blocks. + /// + /// This function needs to take into account multiple conditions: + /// + /// - Only emit the field if it exists for the currently generated version. + /// - Emit fields which previously didn't exist with the correct initialization function. + /// - Emit field with new name and type if there was a name and/or type change. + /// - Handle tracking conversions without data-loss. + /// - Handle deprecated fields accordingly. + /// + /// ### Example + /// + /// ```ignore + /// impl From for v1alpha2::Foo { + /// fn from(value: v1alpha1::Foo) -> Self { + /// Self { + /// bar: value.bar, // This function generates one or more of these definitions + /// } + /// } + /// } + /// ``` pub fn generate_for_from_impl( &self, direction: Direction, @@ -163,9 +195,9 @@ impl VersionedField { let change = changes.get_expect(&version.inner); match (change, next_change) { - // If both this status and the next one is NotPresent, which means - // a field was introduced after a bunch of versions, we don't - // need to generate any code for the From impl. + // If both this status and the next one is NotPresent, which means a field was + // introduced after a bunch of versions, we don't need to generate any code for + // the From impl. (ItemStatus::NotPresent, ItemStatus::NotPresent) => None, ( _, @@ -186,73 +218,38 @@ impl VersionedField { .. }, ) => match direction { - Direction::Upgrade => match upgrade_with { - // The user specified a custom conversion function which - // will be used here instead of the default .into() call - // which utilizes From impls. - // FIXME (@Techassi): A custom conversion function needs - // to integrate with tracking as well. - Some(upgrade_fn) => Some(quote! { - #to_ident: #upgrade_fn(#from_struct_ident.#from_ident), - }), - // Default .into() call using From impls. - None => { - if self.nested { - let json_path_ident = to_ident.json_path_ident(); - - Some(quote! { - #to_ident: #from_struct_ident.#from_ident.tracking_into(status, &#json_path_ident), - }) - } else { - Some(quote! { - #to_ident: #from_struct_ident.#from_ident.into(), - }) - } - } - }, - Direction::Downgrade => match downgrade_with { - Some(downgrade_fn) => Some(quote! { - #from_ident: #downgrade_fn(#from_struct_ident.#to_ident), - }), - None => { - if self.nested { - let json_path_ident = from_ident.json_path_ident(); - - Some(quote! { - #from_ident: #from_struct_ident.#to_ident.tracking_into(status, &#json_path_ident), - }) - } else { - Some(quote! { - #from_ident: #from_struct_ident.#to_ident.into(), - }) - } - } - }, + Direction::Upgrade => Some(self.generate_from_impl_field( + to_ident, + from_struct_ident, + from_ident, + upgrade_with.as_ref(), + )), + Direction::Downgrade => Some(self.generate_from_impl_field( + from_ident, + from_struct_ident, + to_ident, + downgrade_with.as_ref(), + )), }, (old, next) => { let next_field_ident = next.get_ident(); let old_field_ident = old.get_ident(); - // NOTE (@Techassi): Do we really need .into() here. I'm - // currently not sure why it is there and if it is needed - // in some edge cases. + // NOTE (@Techassi): Do we really need .into() here. I'm currently not sure + // why it is there and if it is needed in some edge cases. match direction { - Direction::Upgrade => { - if self.nested { - let json_path_ident = next_field_ident.json_path_ident(); - - Some(quote! { - #next_field_ident: #from_struct_ident.#old_field_ident.tracking_into(status, &#json_path_ident), - }) - } else { - Some(quote! { - #next_field_ident: #from_struct_ident.#old_field_ident.into(), - }) - } - } - Direction::Downgrade => Some(quote! { - #old_field_ident: #from_struct_ident.#next_field_ident.into(), - }), + Direction::Upgrade => Some(self.generate_from_impl_field( + next_field_ident, + from_struct_ident, + old_field_ident, + None, + )), + Direction::Downgrade => Some(self.generate_from_impl_field( + old_field_ident, + from_struct_ident, + next_field_ident, + None, + )), } } } @@ -260,21 +257,18 @@ impl VersionedField { None => { let field_ident = &self.idents.original; - if self.nested { - let json_path_ident = field_ident.json_path_ident(); - - Some(quote! { - #field_ident: #from_struct_ident.#field_ident.tracking_into(status, &#json_path_ident), - }) - } else { - Some(quote! { - #field_ident: #from_struct_ident.#field_ident.into(), - }) - } + Some(self.generate_from_impl_field( + field_ident, + from_struct_ident, + field_ident, + None, + )) } } } + /// Generates code needed when a tracked conversion for this field needs to be inserted into the + /// status. pub fn generate_for_status_insertion( &self, direction: Direction, @@ -317,6 +311,8 @@ impl VersionedField { } } + /// Generates code needed when a tracked conversion for this field needs to be removed from the + /// status. pub fn generate_for_status_removal( &self, direction: Direction, @@ -391,6 +387,66 @@ impl VersionedField { } } } + + /// Generates field definitions to be used inside `From` impl blocks. + fn generate_from_impl_field( + &self, + lhs_field_ident: &IdentString, + rhs_struct_ident: &IdentString, + rhs_field_ident: &IdentString, + custom_conversion_function: Option<&Path>, + ) -> TokenStream { + match custom_conversion_function { + // The user specified a custom conversion function which will be used here instead of the + // default conversion call which utilizes From impls. + Some(convert_fn) => quote! { + #lhs_field_ident: #convert_fn(#rhs_struct_ident.#rhs_field_ident), + }, + // Default conversion call using From impls. + None => { + if self.nested { + let json_path_ident = lhs_field_ident.json_path_ident(); + let func = self.generate_tracking_conversion_function(json_path_ident); + + quote! { + #lhs_field_ident: #rhs_struct_ident.#rhs_field_ident.#func, + } + } else { + let func = self.generate_conversion_function(); + + quote! { + #lhs_field_ident: #rhs_struct_ident.#rhs_field_ident.#func, + } + } + } + } + } + + /// Generates tracking conversion functions used by field definitions in `From` impl blocks. + fn generate_tracking_conversion_function(&self, json_path_ident: IdentString) -> TokenStream { + match &self.hint { + Some(hint) => match hint { + Hint::Option => { + quote! { map(|v| v.tracking_into(status, &#json_path_ident)) } + } + Hint::Vec => { + quote! { into_iter().map(|v| v.tracking_into(status, &#json_path_ident)).collect() } + } + }, + None => quote! { tracking_into(status, &#json_path_ident) }, + } + } + + /// Generates conversion functions used by field definitions in `From` impl blocks. + fn generate_conversion_function(&self) -> TokenStream { + match &self.hint { + Some(hint) => match hint { + Hint::Option => quote! { map(Into::into) }, + Hint::Vec => quote! { into_iter().map(Into::into).collect() }, + }, + None => quote! { into() }, + } + } } /// A collection of field idents used for different purposes. diff --git a/crates/stackable-versioned-macros/tests/inputs/pass/conversion_hints.rs b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_hints.rs new file mode 100644 index 000000000..8cac67982 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_hints.rs @@ -0,0 +1,17 @@ +use stackable_versioned::versioned; +// --- +#[versioned(version(name = "v1alpha1"), version(name = "v1alpha2"))] +// --- +mod versioned { + struct Foo { + #[versioned(hint(option))] + bar: Option, + + #[versioned(hint(vec))] + baz: Vec, + + quux: bool, + } +} +// --- +fn main() {} diff --git a/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_hints.rs b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_hints.rs new file mode 100644 index 000000000..77d903fc0 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_hints.rs @@ -0,0 +1,28 @@ +use stackable_versioned::versioned; +// --- +#[versioned( + version(name = "v1alpha1"), + version(name = "v1alpha2"), + options(k8s(experimental_conversion_tracking)) +)] +// --- +mod versioned { + struct Foo { + #[versioned(nested, hint(option))] + bar: Option, + + #[versioned(hint(vec))] + baz: Vec, + + quux: bool, + } + + struct Bar { + bar_bar: String, + + #[versioned(added(since = "v1alpha2"))] + baz_baz: u8, + } +} +// --- +fn main() {} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_hints.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_hints.rs.snap new file mode 100644 index 000000000..3711aeb63 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_hints.rs.snap @@ -0,0 +1,43 @@ +--- +source: crates/stackable-versioned-macros/src/lib.rs +expression: formatted +input_file: crates/stackable-versioned-macros/tests/inputs/pass/conversion_hints.rs +--- +#[automatically_derived] +mod v1alpha1 { + use super::*; + pub struct Foo { + pub bar: Option, + pub baz: Vec, + pub quux: bool, + } +} +#[automatically_derived] +impl ::core::convert::From for v1alpha2::Foo { + fn from(__sv_foo: v1alpha1::Foo) -> Self { + Self { + bar: __sv_foo.bar.map(Into::into), + baz: __sv_foo.baz.into_iter().map(Into::into).collect(), + quux: __sv_foo.quux.into(), + } + } +} +#[automatically_derived] +impl ::core::convert::From for v1alpha1::Foo { + fn from(__sv_foo: v1alpha2::Foo) -> Self { + Self { + bar: __sv_foo.bar.map(Into::into), + baz: __sv_foo.baz.into_iter().map(Into::into).collect(), + quux: __sv_foo.quux.into(), + } + } +} +#[automatically_derived] +mod v1alpha2 { + use super::*; + pub struct Foo { + pub bar: Option, + pub baz: Vec, + pub quux: bool, + } +} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_hints.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_hints.rs.snap new file mode 100644 index 000000000..96a70d907 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_hints.rs.snap @@ -0,0 +1,112 @@ +--- +source: crates/stackable-versioned-macros/src/lib.rs +expression: formatted +input_file: crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_hints.rs +--- +#[automatically_derived] +mod v1alpha1 { + use super::*; + pub struct Foo { + pub bar: Option, + pub baz: Vec, + pub quux: bool, + } + pub struct Bar { + pub bar_bar: String, + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1alpha2::Foo +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_foo: v1alpha1::Foo, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_bar_path = ::stackable_versioned::jthong_path(parent, "bar"); + let mut spec = Self { + bar: __sv_foo.bar.map(|v| v.tracking_into(status, &__sv_bar_path)), + baz: __sv_foo.baz.into_iter().map(Into::into).collect(), + quux: __sv_foo.quux.into(), + }; + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1alpha1::Foo +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_foo: v1alpha2::Foo, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_bar_path = ::stackable_versioned::jthong_path(parent, "bar"); + let mut spec = Self { + bar: __sv_foo.bar.map(|v| v.tracking_into(status, &__sv_bar_path)), + baz: __sv_foo.baz.into_iter().map(Into::into).collect(), + quux: __sv_foo.quux.into(), + }; + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1alpha2::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1alpha1::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_baz_baz_path = ::stackable_versioned::jthong_path(parent, "baz_baz"); + let mut spec = Self { + bar_bar: __sv_bar.bar_bar.into(), + baz_baz: ::std::default::Default::default(), + }; + if let Some(upgrades) = status.changes().upgrades.remove(&"v1alpha2".to_owned()) + { + for ::stackable_versioned::ChangedValue { json_path, value } in upgrades { + match json_path { + json_path if json_path == __sv_baz_baz_path => { + spec.baz_baz = serde_yaml::from_value(value).unwrap(); + } + _ => unreachable!(), + } + } + } + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1alpha1::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1alpha2::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_baz_baz_path = ::stackable_versioned::jthong_path(parent, "baz_baz"); + let upgrades = status + .changes() + .upgrades + .entry("v1alpha2".to_owned()) + .or_default(); + upgrades + .push(::stackable_versioned::ChangedValue { + json_path: __sv_baz_baz_path, + value: ::serde_yaml::to_value(&__sv_bar.baz_baz).unwrap(), + }); + let mut spec = Self { + bar_bar: __sv_bar.bar_bar.into(), + }; + spec + } +} +#[automatically_derived] +mod v1alpha2 { + use super::*; + pub struct Foo { + pub bar: Option, + pub baz: Vec, + pub quux: bool, + } + pub struct Bar { + pub bar_bar: String, + pub baz_baz: u8, + } +} diff --git a/crates/stackable-versioned-macros/tests/trybuild.rs b/crates/stackable-versioned-macros/tests/trybuild.rs index 8d14fdb39..f2a215bd7 100644 --- a/crates/stackable-versioned-macros/tests/trybuild.rs +++ b/crates/stackable-versioned-macros/tests/trybuild.rs @@ -19,6 +19,8 @@ mod inputs { mod pass { // mod added; // mod basic; + // mod conversion_hints; + // mod conversion_tracking_hints; // mod conversion_tracking; // mod crate_overrides; // mod docs; diff --git a/crates/stackable-versioned/CHANGELOG.md b/crates/stackable-versioned/CHANGELOG.md index d24cee026..e0d67c92a 100644 --- a/crates/stackable-versioned/CHANGELOG.md +++ b/crates/stackable-versioned/CHANGELOG.md @@ -4,11 +4,18 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Add new `#[versioned(hint)]` argument to provide type hints for struct fields ([#1089]). + - `#[versioned(hint(option))]` for fields wrapped in an `Option`. Generates `.map(Into::into)`. + - `#[versioned(hint(vec))]` for fields wrapped in an `Vec`. Generates `.into_iter().map(Into::into).collect()`. + ### Fixed - Correctly emit enum variant fields in `From` impl blocks ([#1086]). [#1086]: https://github.com/stackabletech/operator-rs/pull/1086 +[#1089]: https://github.com/stackabletech/operator-rs/pull/1089 ## [0.8.1] - 2025-08-21