From cb8fbc99ec1646919499d30ff8a4b4ac19e623c0 Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 17 Jun 2024 11:39:54 +0200 Subject: [PATCH 01/18] Update changelog --- crates/stackable-versioned/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/stackable-versioned/CHANGELOG.md b/crates/stackable-versioned/CHANGELOG.md index 869ae6ece..3bc185a7b 100644 --- a/crates/stackable-versioned/CHANGELOG.md +++ b/crates/stackable-versioned/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Changed +- Add support for versioned enums ([#CHANGEME]). - Add auto-generated `From for NEW` implementations ([#790]). - Change from derive macro to attribute macro to be able to generate code _in place_ instead of _appending_ new code ([#793]). @@ -14,6 +15,7 @@ All notable changes to this project will be documented in this file. [#784]: https://github.com/stackabletech/operator-rs/pull/784 [#790]: https://github.com/stackabletech/operator-rs/pull/790 [#793]: https://github.com/stackabletech/operator-rs/pull/793 +[#CHANGEME]: https://github.com/stackabletech/operator-rs/pull/CHANGEME ## [0.1.0] - 2024-05-08 From d2641d451e555fb3b8f8512b8e41af41b7ee8869 Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 17 Jun 2024 11:44:45 +0200 Subject: [PATCH 02/18] Update PR link in the changelog --- crates/stackable-versioned/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stackable-versioned/CHANGELOG.md b/crates/stackable-versioned/CHANGELOG.md index 3bc185a7b..12591d6b1 100644 --- a/crates/stackable-versioned/CHANGELOG.md +++ b/crates/stackable-versioned/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file. ### Changed -- Add support for versioned enums ([#CHANGEME]). +- Add support for versioned enums ([#813]). - Add auto-generated `From for NEW` implementations ([#790]). - Change from derive macro to attribute macro to be able to generate code _in place_ instead of _appending_ new code ([#793]). @@ -15,7 +15,7 @@ All notable changes to this project will be documented in this file. [#784]: https://github.com/stackabletech/operator-rs/pull/784 [#790]: https://github.com/stackabletech/operator-rs/pull/790 [#793]: https://github.com/stackabletech/operator-rs/pull/793 -[#CHANGEME]: https://github.com/stackabletech/operator-rs/pull/CHANGEME +[#813]: https://github.com/stackabletech/operator-rs/pull/813 ## [0.1.0] - 2024-05-08 From 6721b0c1ab02939059aef728630ee7c291be3050 Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 19 Jun 2024 15:19:55 +0200 Subject: [PATCH 03/18] Start to move code, add enum impls --- .../src/attrs/common.rs | 31 +++++++++ .../src/attrs/field.rs | 41 +++--------- .../src/attrs/mod.rs | 2 + .../src/attrs/variant.rs | 40 +++++++++++ .../src/gen/common.rs | 51 ++++++++++++++ .../stackable-versioned-macros/src/gen/mod.rs | 18 +++-- .../src/gen/venum/mod.rs | 66 +++++++++++++++++++ .../src/gen/venum/variant.rs | 27 ++++++++ .../src/gen/version.rs | 10 --- .../src/gen/{ => vstruct}/field.rs | 4 +- .../src/gen/{vstruct.rs => vstruct/mod.rs} | 13 ++-- 11 files changed, 245 insertions(+), 58 deletions(-) create mode 100644 crates/stackable-versioned-macros/src/attrs/common.rs create mode 100644 crates/stackable-versioned-macros/src/attrs/variant.rs create mode 100644 crates/stackable-versioned-macros/src/gen/common.rs create mode 100644 crates/stackable-versioned-macros/src/gen/venum/mod.rs create mode 100644 crates/stackable-versioned-macros/src/gen/venum/variant.rs delete mode 100644 crates/stackable-versioned-macros/src/gen/version.rs rename crates/stackable-versioned-macros/src/gen/{ => vstruct}/field.rs (99%) rename crates/stackable-versioned-macros/src/gen/{vstruct.rs => vstruct/mod.rs} (96%) diff --git a/crates/stackable-versioned-macros/src/attrs/common.rs b/crates/stackable-versioned-macros/src/attrs/common.rs new file mode 100644 index 000000000..1b003674a --- /dev/null +++ b/crates/stackable-versioned-macros/src/attrs/common.rs @@ -0,0 +1,31 @@ +use darling::{util::SpannedValue, FromMeta}; +use k8s_version::Version; +use proc_macro2::Span; +use syn::Path; + +#[derive(Clone, Debug, FromMeta)] +pub(crate) struct AddedAttributes { + pub(crate) since: SpannedValue, + + #[darling(rename = "default", default = "default_default_fn")] + pub(crate) default_fn: SpannedValue, +} + +fn default_default_fn() -> SpannedValue { + SpannedValue::new( + syn::parse_str("std::default::Default::default").expect("internal error: path must parse"), + Span::call_site(), + ) +} + +#[derive(Clone, Debug, FromMeta)] +pub(crate) struct RenamedAttributes { + pub(crate) since: SpannedValue, + pub(crate) from: SpannedValue, +} + +#[derive(Clone, Debug, FromMeta)] +pub(crate) struct DeprecatedAttributes { + pub(crate) since: SpannedValue, + pub(crate) note: SpannedValue, +} diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index 31453f2ce..30199abb2 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -1,9 +1,13 @@ -use darling::{util::SpannedValue, Error, FromField, FromMeta}; -use k8s_version::Version; -use proc_macro2::Span; -use syn::{Field, Ident, Path}; +use darling::{Error, FromField}; +use syn::{Field, Ident}; -use crate::{attrs::container::ContainerAttributes, consts::DEPRECATED_PREFIX}; +use crate::{ + attrs::{ + common::{AddedAttributes, DeprecatedAttributes, RenamedAttributes}, + container::ContainerAttributes, + }, + consts::DEPRECATED_PREFIX, +}; /// This struct describes all available field attributes, as well as the field /// name to display better diagnostics. @@ -38,33 +42,6 @@ pub(crate) struct FieldAttributes { pub(crate) deprecated: Option, } -#[derive(Clone, Debug, FromMeta)] -pub(crate) struct AddedAttributes { - pub(crate) since: SpannedValue, - - #[darling(rename = "default", default = "default_default_fn")] - pub(crate) default_fn: SpannedValue, -} - -fn default_default_fn() -> SpannedValue { - SpannedValue::new( - syn::parse_str("std::default::Default::default").expect("internal error: path must parse"), - Span::call_site(), - ) -} - -#[derive(Clone, Debug, FromMeta)] -pub(crate) struct RenamedAttributes { - pub(crate) since: SpannedValue, - pub(crate) from: SpannedValue, -} - -#[derive(Clone, Debug, FromMeta)] -pub(crate) struct DeprecatedAttributes { - pub(crate) since: SpannedValue, - pub(crate) note: SpannedValue, -} - impl FieldAttributes { /// This associated function is called by darling (see and_then attribute) /// after it successfully parsed the attribute. This allows custom diff --git a/crates/stackable-versioned-macros/src/attrs/mod.rs b/crates/stackable-versioned-macros/src/attrs/mod.rs index 1231db5d3..e320be086 100644 --- a/crates/stackable-versioned-macros/src/attrs/mod.rs +++ b/crates/stackable-versioned-macros/src/attrs/mod.rs @@ -1,2 +1,4 @@ +pub(crate) mod common; pub(crate) mod container; pub(crate) mod field; +pub(crate) mod variant; diff --git a/crates/stackable-versioned-macros/src/attrs/variant.rs b/crates/stackable-versioned-macros/src/attrs/variant.rs new file mode 100644 index 000000000..1f7614836 --- /dev/null +++ b/crates/stackable-versioned-macros/src/attrs/variant.rs @@ -0,0 +1,40 @@ +// TODO (@Techassi): Think about what can be moved into a common impl for both +// fields and variants. + +use darling::{Error, FromVariant}; +use syn::{Ident, Variant}; + +use crate::attrs::{ + common::{AddedAttributes, DeprecatedAttributes, RenamedAttributes}, + container::ContainerAttributes, +}; + +#[derive(Debug, FromVariant)] +#[darling( + attributes(versioned), + forward_attrs(allow, doc, cfg, serde), + and_then = VariantAttributes::validate +)] +pub(crate) struct VariantAttributes { + pub(crate) ident: Ident, + pub(crate) added: Option, + + #[darling(multiple, rename = "renamed")] + pub(crate) renames: Vec, + + pub(crate) deprecated: Option, +} + +impl VariantAttributes { + pub(crate) fn validate(self) -> Result { + Ok(self) + } + + pub(crate) fn validate_versions( + &self, + attributes: &ContainerAttributes, + variant: &Variant, + ) -> Result<(), Error> { + todo!() + } +} diff --git a/crates/stackable-versioned-macros/src/gen/common.rs b/crates/stackable-versioned-macros/src/gen/common.rs new file mode 100644 index 000000000..9d737dce3 --- /dev/null +++ b/crates/stackable-versioned-macros/src/gen/common.rs @@ -0,0 +1,51 @@ +use k8s_version::Version; +use proc_macro2::{Span, TokenStream}; +use syn::Ident; + +use crate::attrs::container::ContainerAttributes; + +#[derive(Debug, Clone)] +pub(crate) struct ContainerVersion { + pub(crate) deprecated: bool, + pub(crate) skip_from: bool, + pub(crate) inner: Version, + pub(crate) ident: Ident, +} + +impl From<&ContainerAttributes> for Vec { + fn from(attributes: &ContainerAttributes) -> Self { + attributes + .versions + .iter() + .map(|v| ContainerVersion { + skip_from: v.skip.as_ref().map_or(false, |s| s.from.is_present()), + ident: Ident::new(&v.name.to_string(), Span::call_site()), + deprecated: v.deprecated.is_present(), + inner: v.name, + }) + .collect() + } +} + +pub(crate) trait VersionedContainer { + type Container; + type Data; + + fn new(ident: Ident, data: Self::Data, attributes: ContainerAttributes) -> Self::Container; + fn generate_tokens(&self) -> TokenStream; +} + +pub(crate) struct Container { + /// The ident, or name, of the versioned enum. + pub(crate) ident: Ident, + + /// List of declared versions for this enum. Each version, except the + /// latest, generates a definition with appropriate fields. + pub(crate) versions: Vec, + + pub(crate) items: Vec, + + /// The name of the enum used in `From` implementations. + pub(crate) from_ident: Ident, + pub(crate) skip_from: bool, +} diff --git a/crates/stackable-versioned-macros/src/gen/mod.rs b/crates/stackable-versioned-macros/src/gen/mod.rs index bf64fa495..7bec874c9 100644 --- a/crates/stackable-versioned-macros/src/gen/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/mod.rs @@ -1,11 +1,14 @@ use proc_macro2::TokenStream; use syn::{spanned::Spanned, Data, DeriveInput, Error, Result}; -use crate::{attrs::container::ContainerAttributes, gen::vstruct::VersionedStruct}; +use crate::{ + attrs::container::ContainerAttributes, + gen::{venum::VersionedEnum, vstruct::VersionedStruct}, +}; -pub(crate) mod field; +pub(crate) mod common; pub(crate) mod neighbors; -pub(crate) mod version; +pub(crate) mod venum; pub(crate) mod vstruct; // NOTE (@Techassi): This derive macro cannot handle multiple structs / enums @@ -20,13 +23,16 @@ pub(crate) mod vstruct; // TODO (@Techassi): Think about how we can handle nested structs / enums which // are also versioned. -pub(crate) fn expand(attrs: ContainerAttributes, input: DeriveInput) -> Result { +pub(crate) fn expand(attributes: ContainerAttributes, input: DeriveInput) -> Result { let expanded = match input.data { - Data::Struct(data) => VersionedStruct::new(input.ident, data, attrs)?.generate_tokens(), + Data::Struct(data) => { + VersionedStruct::new(input.ident, data, attributes)?.generate_tokens() + } + Data::Enum(data) => VersionedEnum::new(input.ident, data, attributes)?.generate_tokens(), _ => { return Err(Error::new( input.span(), - "attribute macro `versioned` only supports structs", + "attribute macro `versioned` only supports structs and enums", )) } }; diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs new file mode 100644 index 000000000..6e8ff9730 --- /dev/null +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -0,0 +1,66 @@ +use std::borrow::Borrow; + +use darling::FromVariant; +use itertools::Itertools; +use proc_macro2::TokenStream; +use syn::{DataEnum, Error, Ident, Result}; + +use crate::{ + attrs::{container::ContainerAttributes, variant::VariantAttributes}, + gen::{common::ContainerVersion, venum::variant::VersionedVariant}, +}; + +mod variant; + +#[derive(Debug)] +pub(crate) struct VersionedEnum { + /// The ident, or name, of the versioned enum. + pub(crate) ident: Ident, + + /// List of declared versions for this enum. Each version, except the + /// latest, generates a definition with appropriate fields. + pub(crate) versions: Vec, + + /// List of variants defined in the base enum. How, and if, a variant should + /// generate code, is decided by the currently generated version. + pub(crate) variants: Vec, + + /// The name of the enum used in `From` implementations. + pub(crate) from_ident: Ident, + pub(crate) skip_from: bool, +} + +impl VersionedEnum { + pub(crate) fn new( + ident: Ident, + data: DataEnum, + attributes: ContainerAttributes, + ) -> Result { + let versions: Vec = attributes.borrow().into(); + let mut variants = Vec::new(); + + for variant in data.variants { + let attrs = VariantAttributes::from_variant(&variant)?; + attrs.validate_versions(&attributes, &variant)?; + + let mut versioned_variant = VersionedVariant::new(); + versioned_variant.insert_container_versions(&versions); + variants.push(versioned_variant); + } + + for version in &versions { + if !variants.iter().map(|v| v.get_ident(version)).all_unique() { + return Err(Error::new( + ident.span(), + format!("enum contains renamed variant which collide with other variant in version {version}", version = version.inner), + )); + } + } + + todo!() + } + + pub(crate) fn generate_tokens(&self) -> TokenStream { + todo!() + } +} diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs new file mode 100644 index 000000000..ddd14fcc8 --- /dev/null +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -0,0 +1,27 @@ +use syn::Ident; + +use crate::gen::common::ContainerVersion; + +#[derive(Debug)] +pub(crate) struct VersionedVariant {} + +impl VersionedVariant { + pub(crate) fn new() -> Self { + todo!() + } + + pub(crate) fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { + todo!() + } + + pub(crate) fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident> { + // match &self.chain { + // Some(chain) => chain + // .get(&version.inner) + // .expect("internal error: chain must contain container version") + // .get_ident(), + // None => self.inner.ident.as_ref(), + // } + todo!() + } +} diff --git a/crates/stackable-versioned-macros/src/gen/version.rs b/crates/stackable-versioned-macros/src/gen/version.rs deleted file mode 100644 index e4375090e..000000000 --- a/crates/stackable-versioned-macros/src/gen/version.rs +++ /dev/null @@ -1,10 +0,0 @@ -use k8s_version::Version; -use syn::Ident; - -#[derive(Debug, Clone)] -pub(crate) struct ContainerVersion { - pub(crate) deprecated: bool, - pub(crate) skip_from: bool, - pub(crate) inner: Version, - pub(crate) ident: Ident, -} diff --git a/crates/stackable-versioned-macros/src/gen/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs similarity index 99% rename from crates/stackable-versioned-macros/src/gen/field.rs rename to crates/stackable-versioned-macros/src/gen/vstruct/field.rs index c9ee4463a..6c7d04208 100644 --- a/crates/stackable-versioned-macros/src/gen/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -9,7 +9,7 @@ use syn::{Field, Ident, Path}; use crate::{ attrs::field::FieldAttributes, consts::DEPRECATED_PREFIX, - gen::{neighbors::Neighbors, version::ContainerVersion}, + gen::{common::ContainerVersion, neighbors::Neighbors}, }; /// A versioned field, which contains contains common [`Field`] data and a chain @@ -158,7 +158,7 @@ impl VersionedField { /// /// This continuous chain ensures that when generating code (tokens), each /// field can lookup the status for a requested version. - pub(crate) fn insert_container_versions(&mut self, versions: &Vec) { + pub(crate) fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { if let Some(chain) = &mut self.chain { for version in versions { if chain.contains_key(&version.inner) { diff --git a/crates/stackable-versioned-macros/src/gen/vstruct.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs similarity index 96% rename from crates/stackable-versioned-macros/src/gen/vstruct.rs rename to crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index fb2a62a71..038758fc9 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -6,9 +6,11 @@ use syn::{DataStruct, Error, Ident, Result}; use crate::{ attrs::{container::ContainerAttributes, field::FieldAttributes}, - gen::{field::VersionedField, version::ContainerVersion}, + gen::{common::ContainerVersion, vstruct::field::VersionedField}, }; +mod field; + /// Stores individual versions of a single struct. Each version tracks field /// actions, which describe if the field was added, renamed or deprecated in /// that version. Fields which are not versioned, are included in every @@ -39,7 +41,7 @@ impl VersionedStruct { attributes: ContainerAttributes, ) -> Result { // Convert the raw version attributes into a container version. - let versions = attributes + let versions: Vec<_> = attributes .versions .iter() .map(|v| ContainerVersion { @@ -69,17 +71,12 @@ impl VersionedStruct { // Collect the idents of all fields for a single version and then // ensure that all idents are unique. If they are not, return an // error. - let mut idents = Vec::new(); // TODO (@Techassi): Report which field(s) use a duplicate ident and // also hint what can be done to fix it based on the field action / // status. - for field in &fields { - idents.push(field.get_ident(version)) - } - - if !idents.iter().all_unique() { + if !fields.iter().map(|f| f.get_ident(version)).all_unique() { return Err(Error::new( ident.span(), format!("struct contains renamed fields which collide with other fields in version {version}", version = version.inner), From f98598aa2c19811b8539be7d152a06474f94c909 Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 20 Jun 2024 10:52:23 +0200 Subject: [PATCH 04/18] Introduce generalized structs for containers and items --- .../src/gen/common.rs | 89 ++++++++-- .../stackable-versioned-macros/src/gen/mod.rs | 2 +- .../src/gen/vstruct/field.rs | 167 ++++++------------ .../src/gen/vstruct/mod.rs | 66 +++---- 4 files changed, 159 insertions(+), 165 deletions(-) diff --git a/crates/stackable-versioned-macros/src/gen/common.rs b/crates/stackable-versioned-macros/src/gen/common.rs index 9d737dce3..98e0a6985 100644 --- a/crates/stackable-versioned-macros/src/gen/common.rs +++ b/crates/stackable-versioned-macros/src/gen/common.rs @@ -1,6 +1,8 @@ +use std::{collections::BTreeMap, ops::Deref}; + use k8s_version::Version; use proc_macro2::{Span, TokenStream}; -use syn::Ident; +use syn::{Field, Ident, Path}; use crate::attrs::container::ContainerAttributes; @@ -27,25 +29,84 @@ impl From<&ContainerAttributes> for Vec { } } -pub(crate) trait VersionedContainer { - type Container; - type Data; +pub(crate) trait Container +where + Self: Sized + Deref, +{ + fn new(ident: Ident, data: D, attributes: ContainerAttributes) -> syn::Result; - fn new(ident: Ident, data: Self::Data, attributes: ContainerAttributes) -> Self::Container; + /// This generates the complete code for a single versioned container. + /// + /// Internally, it will create a module for each declared version which + /// contains the container with the appropriate items (fields or variants) + /// Additionally, it generates `From` implementations, which enable + /// conversion from an older to a newer version. fn generate_tokens(&self) -> TokenStream; } -pub(crate) struct Container { - /// The ident, or name, of the versioned enum. +#[derive(Debug)] +pub(crate) struct VersionedContainer { pub(crate) ident: Ident, - - /// List of declared versions for this enum. Each version, except the - /// latest, generates a definition with appropriate fields. pub(crate) versions: Vec, - - pub(crate) items: Vec, - - /// The name of the enum used in `From` implementations. + pub(crate) items: Vec, pub(crate) from_ident: Ident, pub(crate) skip_from: bool, } + +pub(crate) trait Item +where + Self: Sized, +{ + /// Create a new versioned item (field or variant) by creating a status + /// chain for each version defined in an action in the item attribute. + /// + /// This chain will get extended by the versions defined on the container by + /// calling the [`Item::insert_container_versions`] function. + fn new(field: Field, attributes: A) -> Self; + fn insert_container_versions(&mut self, versions: &[ContainerVersion]); + fn generate_for_container(&self, container_version: &ContainerVersion) -> Option; + fn generate_for_from_impl( + &self, + version: &ContainerVersion, + next_version: &ContainerVersion, + from_ident: &Ident, + ) -> TokenStream; + fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident>; +} + +#[derive(Debug)] +pub(crate) struct VersionedItem { + pub(crate) chain: Option>, + pub(crate) inner: Field, +} + +#[derive(Debug)] +pub(crate) enum ItemStatus { + Added { + ident: Ident, + default_fn: Path, + }, + Renamed { + from: Ident, + to: Ident, + }, + Deprecated { + previous_ident: Ident, + ident: Ident, + note: String, + }, + NoChange(Ident), + NotPresent, +} + +impl ItemStatus { + pub(crate) fn get_ident(&self) -> Option<&Ident> { + match &self { + ItemStatus::Added { ident, .. } => Some(ident), + ItemStatus::Renamed { to, .. } => Some(to), + ItemStatus::Deprecated { ident, .. } => Some(ident), + ItemStatus::NoChange(ident) => Some(ident), + ItemStatus::NotPresent => None, + } + } +} diff --git a/crates/stackable-versioned-macros/src/gen/mod.rs b/crates/stackable-versioned-macros/src/gen/mod.rs index 7bec874c9..980c85869 100644 --- a/crates/stackable-versioned-macros/src/gen/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/mod.rs @@ -3,7 +3,7 @@ use syn::{spanned::Spanned, Data, DeriveInput, Error, Result}; use crate::{ attrs::container::ContainerAttributes, - gen::{venum::VersionedEnum, vstruct::VersionedStruct}, + gen::{common::Container, venum::VersionedEnum, vstruct::VersionedStruct}, }; pub(crate) mod common; diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index 6c7d04208..cffb4ec17 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -1,15 +1,16 @@ use std::{collections::BTreeMap, ops::Deref}; -use darling::Error; -use k8s_version::Version; use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::{Field, Ident, Path}; +use syn::Ident; use crate::{ attrs::field::FieldAttributes, consts::DEPRECATED_PREFIX, - gen::{common::ContainerVersion, neighbors::Neighbors}, + gen::{ + common::{ContainerVersion, Item, ItemStatus, VersionedItem}, + neighbors::Neighbors, + }, }; /// A versioned field, which contains contains common [`Field`] data and a chain @@ -18,19 +19,8 @@ use crate::{ /// The chain of action maps versions to an action and the appropriate field /// name. Additionally, the [`Field`] data can be used to forward attributes, /// generate documentation, etc. -#[derive(Debug)] -pub(crate) struct VersionedField { - chain: Option>, - inner: Field, -} - -impl VersionedField { - /// Create a new versioned field by creating a status chain for each version - /// defined in an action in the field attribute. - /// - /// This chain will get extended by the versions defined on the container by - /// calling the [`VersionedField::insert_container_versions`] function. - pub(crate) fn new(field: Field, attrs: FieldAttributes) -> Result { +impl Item for VersionedItem { + fn new(field: syn::Field, attributes: FieldAttributes) -> Self { // Constructing the action chain requires going through the actions from // the end, because the base struct always represents the latest (most // up-to-date) version of that struct. That's why the following code @@ -42,7 +32,7 @@ impl VersionedField { // rename or addition, which is handled below. // The ident of the deprecated field is guaranteed to include the // 'deprecated_' prefix. The ident can thus be used as is. - if let Some(deprecated) = attrs.deprecated { + if let Some(deprecated) = attributes.deprecated { let deprecated_ident = field.ident.as_ref().unwrap(); // When the field is deprecated, any rename which occurred beforehand @@ -57,18 +47,18 @@ impl VersionedField { actions.insert( *deprecated.since, - FieldStatus::Deprecated { + ItemStatus::Deprecated { previous_ident: ident.clone(), ident: deprecated_ident.clone(), note: deprecated.note.to_string(), }, ); - for rename in attrs.renames.iter().rev() { + for rename in attributes.renames.iter().rev() { let from = format_ident!("{from}", from = *rename.from); actions.insert( *rename.since, - FieldStatus::Renamed { + ItemStatus::Renamed { from: from.clone(), to: ident, }, @@ -78,29 +68,29 @@ impl VersionedField { // After the last iteration above (if any) we use the ident for the // added action if there is any. - if let Some(added) = attrs.added { + if let Some(added) = attributes.added { actions.insert( *added.since, - FieldStatus::Added { + ItemStatus::Added { default_fn: added.default_fn.deref().clone(), ident, }, ); } - Ok(Self { + Self { chain: Some(actions), inner: field, - }) - } else if !attrs.renames.is_empty() { + } + } else if !attributes.renames.is_empty() { let mut actions = BTreeMap::new(); let mut ident = field.ident.clone().unwrap(); - for rename in attrs.renames.iter().rev() { + for rename in attributes.renames.iter().rev() { let from = format_ident!("{from}", from = *rename.from); actions.insert( *rename.since, - FieldStatus::Renamed { + ItemStatus::Renamed { from: from.clone(), to: ident, }, @@ -110,55 +100,46 @@ impl VersionedField { // After the last iteration above (if any) we use the ident for the // added action if there is any. - if let Some(added) = attrs.added { + if let Some(added) = attributes.added { actions.insert( *added.since, - FieldStatus::Added { + ItemStatus::Added { default_fn: added.default_fn.deref().clone(), ident, }, ); } - Ok(Self { + Self { chain: Some(actions), inner: field, - }) + } } else { - if let Some(added) = attrs.added { + if let Some(added) = attributes.added { let mut actions = BTreeMap::new(); actions.insert( *added.since, - FieldStatus::Added { + ItemStatus::Added { default_fn: added.default_fn.deref().clone(), ident: field.ident.clone().unwrap(), }, ); - return Ok(Self { + return Self { chain: Some(actions), inner: field, - }); + }; } - Ok(Self { + Self { chain: None, inner: field, - }) + } } } - /// Inserts container versions not yet present in the status chain. - /// - /// When initially creating a new [`VersionedField`], the code doesn't have - /// access to the versions defined on the container. This function inserts - /// all non-present container versions and decides which status and ident - /// is the right fit based on the status neighbors. - /// - /// This continuous chain ensures that when generating code (tokens), each - /// field can lookup the status for a requested version. - pub(crate) fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { + fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { if let Some(chain) = &mut self.chain { for version in versions { if chain.contains_key(&version.inner) { @@ -167,39 +148,39 @@ impl VersionedField { match chain.get_neighbors(&version.inner) { (None, Some(status)) => match status { - FieldStatus::Added { .. } => { - chain.insert(version.inner, FieldStatus::NotPresent) + ItemStatus::Added { .. } => { + chain.insert(version.inner, ItemStatus::NotPresent) } - FieldStatus::Renamed { from, .. } => { - chain.insert(version.inner, FieldStatus::NoChange(from.clone())) + ItemStatus::Renamed { from, .. } => { + chain.insert(version.inner, ItemStatus::NoChange(from.clone())) } - FieldStatus::Deprecated { previous_ident, .. } => chain - .insert(version.inner, FieldStatus::NoChange(previous_ident.clone())), - FieldStatus::NoChange(ident) => { - chain.insert(version.inner, FieldStatus::NoChange(ident.clone())) + ItemStatus::Deprecated { previous_ident, .. } => chain + .insert(version.inner, ItemStatus::NoChange(previous_ident.clone())), + ItemStatus::NoChange(ident) => { + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) } - FieldStatus::NotPresent => unreachable!(), + ItemStatus::NotPresent => unreachable!(), }, (Some(status), None) => { let ident = match status { - FieldStatus::Added { ident, .. } => ident, - FieldStatus::Renamed { to, .. } => to, - FieldStatus::Deprecated { ident, .. } => ident, - FieldStatus::NoChange(ident) => ident, - FieldStatus::NotPresent => unreachable!(), + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::Deprecated { ident, .. } => ident, + ItemStatus::NoChange(ident) => ident, + ItemStatus::NotPresent => unreachable!(), }; - chain.insert(version.inner, FieldStatus::NoChange(ident.clone())) + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) } (Some(status), Some(_)) => { let ident = match status { - FieldStatus::Added { ident, .. } => ident, - FieldStatus::Renamed { to, .. } => to, - FieldStatus::NoChange(ident) => ident, + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::NoChange(ident) => ident, _ => unreachable!(), }; - chain.insert(version.inner, FieldStatus::NoChange(ident.clone())) + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) } _ => unreachable!(), }; @@ -207,10 +188,7 @@ impl VersionedField { } } - pub(crate) fn generate_for_struct( - &self, - container_version: &ContainerVersion, - ) -> Option { + fn generate_for_container(&self, container_version: &ContainerVersion) -> Option { match &self.chain { Some(chain) => { // Check if the provided container version is present in the map @@ -227,13 +205,13 @@ impl VersionedField { .get(&container_version.inner) .expect("internal error: chain must contain container version") { - FieldStatus::Added { ident, .. } => Some(quote! { + ItemStatus::Added { ident, .. } => Some(quote! { pub #ident: #field_type, }), - FieldStatus::Renamed { from: _, to } => Some(quote! { + ItemStatus::Renamed { from: _, to } => Some(quote! { pub #to: #field_type, }), - FieldStatus::Deprecated { + ItemStatus::Deprecated { ident: field_ident, note, .. @@ -241,8 +219,8 @@ impl VersionedField { #[deprecated = #note] pub #field_ident: #field_type, }), - FieldStatus::NotPresent => None, - FieldStatus::NoChange(field_ident) => Some(quote! { + ItemStatus::NotPresent => None, + ItemStatus::NoChange(field_ident) => Some(quote! { pub #field_ident: #field_type, }), } @@ -261,7 +239,7 @@ impl VersionedField { } } - pub(crate) fn generate_for_from_impl( + fn generate_for_from_impl( &self, version: &ContainerVersion, next_version: &ContainerVersion, @@ -277,7 +255,7 @@ impl VersionedField { .get(&next_version.inner) .expect("internal error: chain must contain container version"), ) { - (_, FieldStatus::Added { ident, default_fn }) => quote! { + (_, ItemStatus::Added { ident, default_fn }) => quote! { #ident: #default_fn(), }, (old, next) => { @@ -299,7 +277,7 @@ impl VersionedField { } } - pub(crate) fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident> { + fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident> { match &self.chain { Some(chain) => chain .get(&version.inner) @@ -309,34 +287,3 @@ impl VersionedField { } } } - -#[derive(Debug)] -pub(crate) enum FieldStatus { - Added { - ident: Ident, - default_fn: Path, - }, - Renamed { - from: Ident, - to: Ident, - }, - Deprecated { - previous_ident: Ident, - ident: Ident, - note: String, - }, - NoChange(Ident), - NotPresent, -} - -impl FieldStatus { - pub(crate) fn get_ident(&self) -> Option<&Ident> { - match &self { - FieldStatus::Added { ident, .. } => Some(ident), - FieldStatus::Renamed { to, .. } => Some(to), - FieldStatus::Deprecated { ident, .. } => Some(ident), - FieldStatus::NoChange(ident) => Some(ident), - FieldStatus::NotPresent => None, - } - } -} diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index 038758fc9..16ad7c81b 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -1,12 +1,14 @@ +use std::ops::Deref; + use darling::FromField; use itertools::Itertools; use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::{DataStruct, Error, Ident, Result}; +use syn::{DataStruct, Error, Ident}; use crate::{ attrs::{container::ContainerAttributes, field::FieldAttributes}, - gen::{common::ContainerVersion, vstruct::field::VersionedField}, + gen::common::{Container, ContainerVersion, Item, VersionedContainer, VersionedItem}, }; mod field; @@ -16,30 +18,18 @@ mod field; /// that version. Fields which are not versioned, are included in every /// version of the struct. #[derive(Debug)] -pub(crate) struct VersionedStruct { - /// The ident, or name, of the versioned struct. - pub(crate) ident: Ident, - - /// The name of the struct used in `From` implementations. - pub(crate) from_ident: Ident, +pub(crate) struct VersionedStruct(VersionedContainer); - /// List of declared versions for this struct. Each version, except the - /// latest, generates a definition with appropriate fields. - pub(crate) versions: Vec, +impl Deref for VersionedStruct { + type Target = VersionedContainer; - /// List of fields defined in the base struct. How, and if, a field should - /// generate code, is decided by the currently generated version. - pub(crate) fields: Vec, - - pub(crate) skip_from: bool, + fn deref(&self) -> &Self::Target { + &self.0 + } } -impl VersionedStruct { - pub(crate) fn new( - ident: Ident, - data: DataStruct, - attributes: ContainerAttributes, - ) -> Result { +impl Container for VersionedStruct { + fn new(ident: Ident, data: DataStruct, attributes: ContainerAttributes) -> syn::Result { // Convert the raw version attributes into a container version. let versions: Vec<_> = attributes .versions @@ -55,15 +45,15 @@ impl VersionedStruct { // Extract the field attributes for every field from the raw token // stream and also validate that each field action version uses a // version declared by the container attribute. - let mut fields = Vec::new(); + let mut items = Vec::new(); for field in data.fields { let attrs = FieldAttributes::from_field(&field)?; attrs.validate_versions(&attributes, &field)?; - let mut versioned_field = VersionedField::new(field, attrs)?; + let mut versioned_field = VersionedItem::new(field, attrs); versioned_field.insert_container_versions(&versions); - fields.push(versioned_field); + items.push(versioned_field); } // Check for field ident collisions @@ -76,7 +66,7 @@ impl VersionedStruct { // also hint what can be done to fix it based on the field action / // status. - if !fields.iter().map(|f| f.get_ident(version)).all_unique() { + if !items.iter().map(|f| f.get_ident(version)).all_unique() { return Err(Error::new( ident.span(), format!("struct contains renamed fields which collide with other fields in version {version}", version = version.inner), @@ -86,25 +76,19 @@ impl VersionedStruct { let from_ident = format_ident!("__sv_{ident}", ident = ident.to_string().to_lowercase()); - Ok(Self { + Ok(Self(VersionedContainer { skip_from: attributes .options .skip .map_or(false, |s| s.from.is_present()), from_ident, versions, - fields, + items, ident, - }) + })) } - /// This generates the complete code for a single versioned struct. - /// - /// Internally, it will create a module for each declared version which - /// contains the struct with the appropriate fields. Additionally, it - /// generated `From` implementations, which enable conversion from an older - /// to a newer version. - pub(crate) fn generate_tokens(&self) -> TokenStream { + fn generate_tokens(&self) -> TokenStream { let mut token_stream = TokenStream::new(); let mut versions = self.versions.iter().peekable(); @@ -114,7 +98,9 @@ impl VersionedStruct { token_stream } +} +impl VersionedStruct { fn generate_version( &self, version: &ContainerVersion, @@ -155,8 +141,8 @@ impl VersionedStruct { fn generate_struct_fields(&self, version: &ContainerVersion) -> TokenStream { let mut token_stream = TokenStream::new(); - for field in &self.fields { - token_stream.extend(field.generate_for_struct(version)); + for item in &self.items { + token_stream.extend(item.generate_for_container(version)); } token_stream @@ -201,8 +187,8 @@ impl VersionedStruct { ) -> TokenStream { let mut token_stream = TokenStream::new(); - for field in &self.fields { - token_stream.extend(field.generate_for_from_impl(version, next_version, from_ident)) + for item in &self.items { + token_stream.extend(item.generate_for_from_impl(version, next_version, from_ident)) } token_stream From 124811fea3b44e4133686b4570c87c0a38917df2 Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 24 Jun 2024 14:36:24 +0200 Subject: [PATCH 05/18] Finish traits and generic types --- .../src/attrs/field.rs | 4 +- .../stackable-versioned-macros/src/consts.rs | 3 +- .../src/gen/common.rs | 49 +++++++--- .../src/gen/venum/mod.rs | 95 ++++++++++++------- .../src/gen/venum/variant.rs | 43 ++++++--- .../src/gen/vstruct/field.rs | 18 +++- .../src/gen/vstruct/mod.rs | 17 ++-- 7 files changed, 154 insertions(+), 75 deletions(-) diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index 30199abb2..c1d5337ff 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -6,7 +6,7 @@ use crate::{ common::{AddedAttributes, DeprecatedAttributes, RenamedAttributes}, container::ContainerAttributes, }, - consts::DEPRECATED_PREFIX, + consts::DEPRECATED_FIELD_PREFIX, }; /// This struct describes all available field attributes, as well as the field @@ -167,7 +167,7 @@ impl FieldAttributes { .as_ref() .unwrap() .to_string() - .starts_with(DEPRECATED_PREFIX); + .starts_with(DEPRECATED_FIELD_PREFIX); if self.deprecated.is_some() && !starts_with { return Err(Error::custom( diff --git a/crates/stackable-versioned-macros/src/consts.rs b/crates/stackable-versioned-macros/src/consts.rs index bb0ff076b..b373331e0 100644 --- a/crates/stackable-versioned-macros/src/consts.rs +++ b/crates/stackable-versioned-macros/src/consts.rs @@ -1 +1,2 @@ -pub(crate) const DEPRECATED_PREFIX: &str = "deprecated_"; +pub(crate) const DEPRECATED_VARIANT_PREFIX: &str = "DEPRECATED_"; +pub(crate) const DEPRECATED_FIELD_PREFIX: &str = "deprecated_"; diff --git a/crates/stackable-versioned-macros/src/gen/common.rs b/crates/stackable-versioned-macros/src/gen/common.rs index 98e0a6985..8d6ccb53f 100644 --- a/crates/stackable-versioned-macros/src/gen/common.rs +++ b/crates/stackable-versioned-macros/src/gen/common.rs @@ -2,10 +2,13 @@ use std::{collections::BTreeMap, ops::Deref}; use k8s_version::Version; use proc_macro2::{Span, TokenStream}; -use syn::{Field, Ident, Path}; +use quote::format_ident; +use syn::{Ident, Path}; use crate::attrs::container::ContainerAttributes; +pub(crate) type VersionChain = BTreeMap; + #[derive(Debug, Clone)] pub(crate) struct ContainerVersion { pub(crate) deprecated: bool, @@ -29,9 +32,9 @@ impl From<&ContainerAttributes> for Vec { } } -pub(crate) trait Container +pub(crate) trait Container where - Self: Sized + Deref, + Self: Sized + Deref>, { fn new(ident: Ident, data: D, attributes: ContainerAttributes) -> syn::Result; @@ -45,15 +48,16 @@ where } #[derive(Debug)] -pub(crate) struct VersionedContainer { - pub(crate) ident: Ident, +pub(crate) struct VersionedContainer { pub(crate) versions: Vec, - pub(crate) items: Vec, + pub(crate) items: Vec, + pub(crate) ident: Ident, + pub(crate) from_ident: Ident, pub(crate) skip_from: bool, } -pub(crate) trait Item +pub(crate) trait Item where Self: Sized, { @@ -62,22 +66,34 @@ where /// /// This chain will get extended by the versions defined on the container by /// calling the [`Item::insert_container_versions`] function. - fn new(field: Field, attributes: A) -> Self; + fn new(item: I, attributes: A) -> Self; + + /// Inserts container versions not yet present in the status chain. + /// + /// When initially creating a new versioned item, the code doesn't have + /// access to the versions defined on the container. This function inserts + /// all non-present container versions and decides which status and ident + /// is the right fit based on the status neighbors. + /// + /// This continuous chain ensures that when generating code (tokens), each + /// field can lookup the status (and ident) for a requested version. fn insert_container_versions(&mut self, versions: &[ContainerVersion]); + + /// Generates tokens for the use inside the container definition, e.g. + /// a struct field or an enum variant. fn generate_for_container(&self, container_version: &ContainerVersion) -> Option; + + /// Generates tokens for the use inside [`From`] implementations for + /// conversion between versions. fn generate_for_from_impl( &self, version: &ContainerVersion, next_version: &ContainerVersion, from_ident: &Ident, ) -> TokenStream; - fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident>; -} -#[derive(Debug)] -pub(crate) struct VersionedItem { - pub(crate) chain: Option>, - pub(crate) inner: Field, + /// Returns the ident of the [`Item`] for a specific [`ContainerVersion`]. + fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident>; } #[derive(Debug)] @@ -110,3 +126,8 @@ impl ItemStatus { } } } + +/// Returns the container ident used in [`From`] implementations. +pub(crate) fn format_container_from_ident(ident: &Ident) -> Ident { + format_ident!("__sv_{ident}", ident = ident.to_string().to_lowercase()) +} diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index 6e8ff9730..f9e9965cd 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -1,66 +1,95 @@ -use std::borrow::Borrow; +use std::ops::Deref; use darling::FromVariant; use itertools::Itertools; use proc_macro2::TokenStream; -use syn::{DataEnum, Error, Ident, Result}; +use quote::format_ident; +use syn::{DataEnum, Error, Ident}; use crate::{ attrs::{container::ContainerAttributes, variant::VariantAttributes}, - gen::{common::ContainerVersion, venum::variant::VersionedVariant}, + gen::{ + common::{ + format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, + }, + venum::variant::VersionedVariant, + }, }; mod variant; #[derive(Debug)] -pub(crate) struct VersionedEnum { - /// The ident, or name, of the versioned enum. - pub(crate) ident: Ident, +pub(crate) struct VersionedEnum(VersionedContainer); - /// List of declared versions for this enum. Each version, except the - /// latest, generates a definition with appropriate fields. - pub(crate) versions: Vec, +impl Container for VersionedEnum { + fn new(ident: Ident, data: DataEnum, attributes: ContainerAttributes) -> syn::Result { + // Convert the raw version attributes into a container version. + let versions: Vec<_> = attributes + .versions + .iter() + .map(|v| ContainerVersion { + skip_from: v.skip.as_ref().map_or(false, |s| s.from.is_present()), + ident: format_ident!("{version}", version = v.name.to_string()), + deprecated: v.deprecated.is_present(), + inner: v.name, + }) + .collect(); - /// List of variants defined in the base enum. How, and if, a variant should - /// generate code, is decided by the currently generated version. - pub(crate) variants: Vec, - - /// The name of the enum used in `From` implementations. - pub(crate) from_ident: Ident, - pub(crate) skip_from: bool, -} - -impl VersionedEnum { - pub(crate) fn new( - ident: Ident, - data: DataEnum, - attributes: ContainerAttributes, - ) -> Result { - let versions: Vec = attributes.borrow().into(); - let mut variants = Vec::new(); + // Extract the field attributes for every field from the raw token + // stream and also validate that each field action version uses a + // version declared by the container attribute. + let mut items = Vec::new(); for variant in data.variants { let attrs = VariantAttributes::from_variant(&variant)?; attrs.validate_versions(&attributes, &variant)?; - let mut versioned_variant = VersionedVariant::new(); - versioned_variant.insert_container_versions(&versions); - variants.push(versioned_variant); + let mut versioned_field = VersionedVariant::new(variant, attrs); + versioned_field.insert_container_versions(&versions); + items.push(versioned_field); } + // Check for field ident collisions for version in &versions { - if !variants.iter().map(|v| v.get_ident(version)).all_unique() { + // Collect the idents of all fields for a single version and then + // ensure that all idents are unique. If they are not, return an + // error. + + // TODO (@Techassi): Report which field(s) use a duplicate ident and + // also hint what can be done to fix it based on the field action / + // status. + + if !items.iter().map(|f| f.get_ident(version)).all_unique() { return Err(Error::new( ident.span(), - format!("enum contains renamed variant which collide with other variant in version {version}", version = version.inner), + format!("struct contains renamed fields which collide with other fields in version {version}", version = version.inner), )); } } - todo!() + let from_ident = format_container_from_ident(&ident); + + Ok(Self(VersionedContainer { + skip_from: attributes + .options + .skip + .map_or(false, |s| s.from.is_present()), + from_ident, + versions, + items, + ident, + })) } - pub(crate) fn generate_tokens(&self) -> TokenStream { + fn generate_tokens(&self) -> TokenStream { todo!() } } + +impl Deref for VersionedEnum { + type Target = VersionedContainer; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs index ddd14fcc8..8a391bb2c 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -1,27 +1,42 @@ -use syn::Ident; +use syn::Variant; -use crate::gen::common::ContainerVersion; +use crate::{ + attrs::variant::VariantAttributes, + gen::common::{Item, VersionChain}, +}; #[derive(Debug)] -pub(crate) struct VersionedVariant {} +pub(crate) struct VersionedVariant { + chain: Option, + inner: Variant, +} + +impl Item for VersionedVariant { + fn new(variant: Variant, attributes: VariantAttributes) -> Self { + todo!() + } + + fn insert_container_versions(&mut self, versions: &[crate::gen::common::ContainerVersion]) { + todo!() + } -impl VersionedVariant { - pub(crate) fn new() -> Self { + fn generate_for_container( + &self, + container_version: &crate::gen::common::ContainerVersion, + ) -> Option { todo!() } - pub(crate) fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { + fn generate_for_from_impl( + &self, + version: &crate::gen::common::ContainerVersion, + next_version: &crate::gen::common::ContainerVersion, + from_ident: &syn::Ident, + ) -> proc_macro2::TokenStream { todo!() } - pub(crate) fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident> { - // match &self.chain { - // Some(chain) => chain - // .get(&version.inner) - // .expect("internal error: chain must contain container version") - // .get_ident(), - // None => self.inner.ident.as_ref(), - // } + fn get_ident(&self, version: &crate::gen::common::ContainerVersion) -> Option<&syn::Ident> { todo!() } } diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index cffb4ec17..e7d3f01b7 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -2,13 +2,13 @@ use std::{collections::BTreeMap, ops::Deref}; use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::Ident; +use syn::{Field, Ident}; use crate::{ attrs::field::FieldAttributes, - consts::DEPRECATED_PREFIX, + consts::DEPRECATED_FIELD_PREFIX, gen::{ - common::{ContainerVersion, Item, ItemStatus, VersionedItem}, + common::{ContainerVersion, Item, ItemStatus, VersionChain}, neighbors::Neighbors, }, }; @@ -19,7 +19,13 @@ use crate::{ /// The chain of action maps versions to an action and the appropriate field /// name. Additionally, the [`Field`] data can be used to forward attributes, /// generate documentation, etc. -impl Item for VersionedItem { +#[derive(Debug)] +pub(crate) struct VersionedField { + pub(crate) chain: Option, + pub(crate) inner: Field, +} + +impl Item for VersionedField { fn new(field: syn::Field, attributes: FieldAttributes) -> Self { // Constructing the action chain requires going through the actions from // the end, because the base struct always represents the latest (most @@ -40,7 +46,9 @@ impl Item for VersionedItem { // the latest rename. let mut ident = format_ident!( "{ident}", - ident = deprecated_ident.to_string().replace(DEPRECATED_PREFIX, "") + ident = deprecated_ident + .to_string() + .replace(DEPRECATED_FIELD_PREFIX, "") ); let mut actions = BTreeMap::new(); diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index 16ad7c81b..11ebe41c9 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -8,7 +8,12 @@ use syn::{DataStruct, Error, Ident}; use crate::{ attrs::{container::ContainerAttributes, field::FieldAttributes}, - gen::common::{Container, ContainerVersion, Item, VersionedContainer, VersionedItem}, + gen::{ + common::{ + format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, + }, + vstruct::field::VersionedField, + }, }; mod field; @@ -18,17 +23,17 @@ mod field; /// that version. Fields which are not versioned, are included in every /// version of the struct. #[derive(Debug)] -pub(crate) struct VersionedStruct(VersionedContainer); +pub(crate) struct VersionedStruct(VersionedContainer); impl Deref for VersionedStruct { - type Target = VersionedContainer; + type Target = VersionedContainer; fn deref(&self) -> &Self::Target { &self.0 } } -impl Container for VersionedStruct { +impl Container for VersionedStruct { fn new(ident: Ident, data: DataStruct, attributes: ContainerAttributes) -> syn::Result { // Convert the raw version attributes into a container version. let versions: Vec<_> = attributes @@ -51,7 +56,7 @@ impl Container for VersionedStruct { let attrs = FieldAttributes::from_field(&field)?; attrs.validate_versions(&attributes, &field)?; - let mut versioned_field = VersionedItem::new(field, attrs); + let mut versioned_field = VersionedField::new(field, attrs); versioned_field.insert_container_versions(&versions); items.push(versioned_field); } @@ -74,7 +79,7 @@ impl Container for VersionedStruct { } } - let from_ident = format_ident!("__sv_{ident}", ident = ident.to_string().to_lowercase()); + let from_ident = format_container_from_ident(&ident); Ok(Self(VersionedContainer { skip_from: attributes From 28048a1e8280c49d832ad28043c624f856bb8c7a Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 24 Jun 2024 15:08:39 +0200 Subject: [PATCH 06/18] Use From<&ContainerAttributes> for Vec implementation --- .../src/gen/venum/mod.rs | 16 ++-------------- .../src/gen/vstruct/mod.rs | 13 ++----------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index f9e9965cd..e2945cf06 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -3,15 +3,12 @@ use std::ops::Deref; use darling::FromVariant; use itertools::Itertools; use proc_macro2::TokenStream; -use quote::format_ident; use syn::{DataEnum, Error, Ident}; use crate::{ attrs::{container::ContainerAttributes, variant::VariantAttributes}, gen::{ - common::{ - format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, - }, + common::{format_container_from_ident, Container, Item, VersionedContainer}, venum::variant::VersionedVariant, }, }; @@ -24,16 +21,7 @@ pub(crate) struct VersionedEnum(VersionedContainer); impl Container for VersionedEnum { fn new(ident: Ident, data: DataEnum, attributes: ContainerAttributes) -> syn::Result { // Convert the raw version attributes into a container version. - let versions: Vec<_> = attributes - .versions - .iter() - .map(|v| ContainerVersion { - skip_from: v.skip.as_ref().map_or(false, |s| s.from.is_present()), - ident: format_ident!("{version}", version = v.name.to_string()), - deprecated: v.deprecated.is_present(), - inner: v.name, - }) - .collect(); + let versions: Vec<_> = (&attributes).into(); // Extract the field attributes for every field from the raw token // stream and also validate that each field action version uses a diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index 11ebe41c9..42b24bf56 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -3,7 +3,7 @@ use std::ops::Deref; use darling::FromField; use itertools::Itertools; use proc_macro2::TokenStream; -use quote::{format_ident, quote}; +use quote::quote; use syn::{DataStruct, Error, Ident}; use crate::{ @@ -36,16 +36,7 @@ impl Deref for VersionedStruct { impl Container for VersionedStruct { fn new(ident: Ident, data: DataStruct, attributes: ContainerAttributes) -> syn::Result { // Convert the raw version attributes into a container version. - let versions: Vec<_> = attributes - .versions - .iter() - .map(|v| ContainerVersion { - skip_from: v.skip.as_ref().map_or(false, |s| s.from.is_present()), - ident: format_ident!("{version}", version = v.name.to_string()), - deprecated: v.deprecated.is_present(), - inner: v.name, - }) - .collect(); + let versions: Vec<_> = (&attributes).into(); // Extract the field attributes for every field from the raw token // stream and also validate that each field action version uses a From b315baf310bb3f3f5905c6828f11ddad8c736788 Mon Sep 17 00:00:00 2001 From: Techassi Date: Tue, 25 Jun 2024 14:34:51 +0200 Subject: [PATCH 07/18] Finish basic enum code generation --- .../src/attrs/field.rs | 2 +- .../src/attrs/variant.rs | 49 ++++- .../src/gen/common.rs | 89 ++++++++- .../src/gen/venum/mod.rs | 66 ++++++- .../src/gen/venum/variant.rs | 172 ++++++++++++++++-- .../src/gen/vstruct/field.rs | 79 ++------ .../stackable-versioned-macros/tests/basic.rs | 2 +- .../stackable-versioned-macros/tests/enum.rs | 10 + 8 files changed, 378 insertions(+), 91 deletions(-) create mode 100644 crates/stackable-versioned-macros/tests/enum.rs diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index c1d5337ff..33d5d9df8 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -61,7 +61,7 @@ impl FieldAttributes { errors.handle(self.validate_deprecated_options()); // TODO (@Techassi): Add validation for renames so that renamed fields - // match up and form a continous chain (eg. foo -> bar -> baz). + // match up and form a continuous chain (eg. foo -> bar -> baz). // TODO (@Techassi): Add hint if a field is added in the first version // that it might be clever to remove the 'added' attribute. diff --git a/crates/stackable-versioned-macros/src/attrs/variant.rs b/crates/stackable-versioned-macros/src/attrs/variant.rs index 1f7614836..f73c7d54c 100644 --- a/crates/stackable-versioned-macros/src/attrs/variant.rs +++ b/crates/stackable-versioned-macros/src/attrs/variant.rs @@ -32,9 +32,54 @@ impl VariantAttributes { pub(crate) fn validate_versions( &self, - attributes: &ContainerAttributes, + container_attrs: &ContainerAttributes, variant: &Variant, ) -> Result<(), Error> { - todo!() + // NOTE (@Techassi): Can we maybe optimize this a little? + // TODO (@Techassi): Unify this with the field impl, e.g. by introducing + // a T: Spanned bound for the second function parameter. + let mut errors = Error::accumulator(); + + if let Some(added) = &self.added { + if !container_attrs + .versions + .iter() + .any(|v| v.name == *added.since) + { + errors.push(Error::custom( + "field action `added` uses version which was not declared via #[versioned(version)]") + .with_span(&variant.ident) + ); + } + } + + for rename in &self.renames { + if !container_attrs + .versions + .iter() + .any(|v| v.name == *rename.since) + { + errors.push( + Error::custom("field action `renamed` uses version which was not declared via #[versioned(version)]") + .with_span(&variant.ident) + ); + } + } + + if let Some(deprecated) = &self.deprecated { + if !container_attrs + .versions + .iter() + .any(|v| v.name == *deprecated.since) + { + errors.push(Error::custom( + "field action `deprecated` uses version which was not declared via #[versioned(version)]") + .with_span(&variant.ident) + ); + } + } + + errors.finish()?; + Ok(()) } } diff --git a/crates/stackable-versioned-macros/src/gen/common.rs b/crates/stackable-versioned-macros/src/gen/common.rs index 8d6ccb53f..3a713b9d7 100644 --- a/crates/stackable-versioned-macros/src/gen/common.rs +++ b/crates/stackable-versioned-macros/src/gen/common.rs @@ -3,9 +3,13 @@ use std::{collections::BTreeMap, ops::Deref}; use k8s_version::Version; use proc_macro2::{Span, TokenStream}; use quote::format_ident; -use syn::{Ident, Path}; +use syn::{Field, Ident, Path, Variant}; -use crate::attrs::container::ContainerAttributes; +use crate::{ + attrs::container::ContainerAttributes, + consts::{DEPRECATED_FIELD_PREFIX, DEPRECATED_VARIANT_PREFIX}, + gen::neighbors::Neighbors, +}; pub(crate) type VersionChain = BTreeMap; @@ -60,6 +64,7 @@ pub(crate) struct VersionedContainer { pub(crate) trait Item where Self: Sized, + I: GetIdent, { /// Create a new versioned item (field or variant) by creating a status /// chain for each version defined in an action in the item attribute. @@ -77,7 +82,54 @@ where /// /// This continuous chain ensures that when generating code (tokens), each /// field can lookup the status (and ident) for a requested version. - fn insert_container_versions(&mut self, versions: &[ContainerVersion]); + fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { + if let Some(chain) = self.chain() { + for version in versions { + if chain.contains_key(&version.inner) { + continue; + } + + match chain.get_neighbors(&version.inner) { + (None, Some(status)) => match status { + ItemStatus::Added { .. } => { + chain.insert(version.inner, ItemStatus::NotPresent) + } + ItemStatus::Renamed { from, .. } => { + chain.insert(version.inner, ItemStatus::NoChange(from.clone())) + } + ItemStatus::Deprecated { previous_ident, .. } => chain + .insert(version.inner, ItemStatus::NoChange(previous_ident.clone())), + ItemStatus::NoChange(ident) => { + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + ItemStatus::NotPresent => unreachable!(), + }, + (Some(status), None) => { + let ident = match status { + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::Deprecated { ident, .. } => ident, + ItemStatus::NoChange(ident) => ident, + ItemStatus::NotPresent => unreachable!(), + }; + + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + (Some(status), Some(_)) => { + let ident = match status { + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::NoChange(ident) => ident, + _ => unreachable!(), + }; + + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + _ => unreachable!(), + }; + } + } + } /// Generates tokens for the use inside the container definition, e.g. /// a struct field or an enum variant. @@ -94,6 +146,24 @@ where /// Returns the ident of the [`Item`] for a specific [`ContainerVersion`]. fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident>; + + fn chain(&mut self) -> Option<&mut VersionChain>; +} + +pub(crate) trait GetIdent { + fn ident(&self) -> Option<&Ident>; +} + +impl GetIdent for Field { + fn ident(&self) -> Option<&Ident> { + self.ident.as_ref() + } +} + +impl GetIdent for Variant { + fn ident(&self) -> Option<&Ident> { + Some(&self.ident) + } } #[derive(Debug)] @@ -131,3 +201,16 @@ impl ItemStatus { pub(crate) fn format_container_from_ident(ident: &Ident) -> Ident { format_ident!("__sv_{ident}", ident = ident.to_string().to_lowercase()) } + +/// Removes the deprecated prefix from field ident. +pub(crate) fn remove_deprecated_field_prefix(ident: &Ident) -> Ident { + remove_ident_prefix(ident, DEPRECATED_FIELD_PREFIX) +} + +pub(crate) fn remove_deprecated_variant_prefix(ident: &Ident) -> Ident { + remove_ident_prefix(ident, DEPRECATED_VARIANT_PREFIX) +} + +pub(crate) fn remove_ident_prefix(ident: &Ident, prefix: &str) -> Ident { + format_ident!("{}", ident.to_string().trim_start_matches(prefix)) +} diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index e2945cf06..f21b52896 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -3,12 +3,15 @@ use std::ops::Deref; use darling::FromVariant; use itertools::Itertools; use proc_macro2::TokenStream; +use quote::quote; use syn::{DataEnum, Error, Ident}; use crate::{ attrs::{container::ContainerAttributes, variant::VariantAttributes}, gen::{ - common::{format_container_from_ident, Container, Item, VersionedContainer}, + common::{ + format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, + }, venum::variant::VersionedVariant, }, }; @@ -18,6 +21,14 @@ mod variant; #[derive(Debug)] pub(crate) struct VersionedEnum(VersionedContainer); +impl Deref for VersionedEnum { + type Target = VersionedContainer; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + impl Container for VersionedEnum { fn new(ident: Ident, data: DataEnum, attributes: ContainerAttributes) -> syn::Result { // Convert the raw version attributes into a container version. @@ -70,14 +81,57 @@ impl Container for VersionedEnum { } fn generate_tokens(&self) -> TokenStream { - todo!() + let mut token_stream = TokenStream::new(); + let mut versions = self.versions.iter().peekable(); + + while let Some(version) = versions.next() { + token_stream.extend(self.generate_version(version, versions.peek().copied())); + } + + token_stream } } -impl Deref for VersionedEnum { - type Target = VersionedContainer; +impl VersionedEnum { + fn generate_version( + &self, + version: &ContainerVersion, + next_version: Option<&ContainerVersion>, + ) -> TokenStream { + let mut token_stream = TokenStream::new(); + let enum_name = &self.ident; + + // Generate variants of the enum for `version`. + let variants = self.generate_enum_variants(version); + + // TODO (@Techassi): Make the generation of the module optional to + // enable the attribute macro to be applied to a module which + // generates versioned versions of all contained containers. + + let deprecated_attr = version.deprecated.then_some(quote! {#[deprecated]}); + let module_name = &version.ident; + + // Generate tokens for the module and the contained struct + token_stream.extend(quote! { + #[automatically_derived] + #deprecated_attr + pub mod #module_name { + pub enum #enum_name { + #variants + } + } + }); - fn deref(&self) -> &Self::Target { - &self.0 + token_stream + } + + fn generate_enum_variants(&self, version: &ContainerVersion) -> TokenStream { + let mut token_stream = TokenStream::new(); + + for variant in &self.items { + token_stream.extend(variant.generate_for_container(version)); + } + + token_stream } } diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs index 8a391bb2c..0529b4cd9 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -1,8 +1,14 @@ +use std::{collections::BTreeMap, ops::Deref}; + +use proc_macro2::TokenStream; +use quote::{format_ident, quote}; use syn::Variant; use crate::{ attrs::variant::VariantAttributes, - gen::common::{Item, VersionChain}, + gen::common::{ + remove_deprecated_variant_prefix, ContainerVersion, Item, ItemStatus, VersionChain, + }, }; #[derive(Debug)] @@ -11,32 +17,168 @@ pub(crate) struct VersionedVariant { inner: Variant, } +// TODO (@Techassi): Figure out a way to be able to only write the following code +// once for both a versioned field and variant, because the are practically +// identical. + impl Item for VersionedVariant { fn new(variant: Variant, attributes: VariantAttributes) -> Self { - todo!() - } + // NOTE (@Techassi): This is straight up copied from the VersionedField + // impl. As mentioned above, unify this. + if let Some(deprecated) = attributes.deprecated { + let deprecated_ident = &variant.ident; - fn insert_container_versions(&mut self, versions: &[crate::gen::common::ContainerVersion]) { - todo!() + // When the field is deprecated, any rename which occurred beforehand + // requires access to the field ident to infer the field ident for + // the latest rename. + let mut ident = remove_deprecated_variant_prefix(&deprecated_ident); + let mut actions = BTreeMap::new(); + + actions.insert( + *deprecated.since, + ItemStatus::Deprecated { + previous_ident: ident.clone(), + ident: deprecated_ident.clone(), + note: deprecated.note.to_string(), + }, + ); + + for rename in attributes.renames.iter().rev() { + let from = format_ident!("{from}", from = *rename.from); + actions.insert( + *rename.since, + ItemStatus::Renamed { + from: from.clone(), + to: ident, + }, + ); + ident = from; + } + + // After the last iteration above (if any) we use the ident for the + // added action if there is any. + if let Some(added) = attributes.added { + actions.insert( + *added.since, + ItemStatus::Added { + default_fn: added.default_fn.deref().clone(), + ident, + }, + ); + } + + Self { + chain: Some(actions), + inner: variant, + } + } else if !attributes.renames.is_empty() { + let mut actions = BTreeMap::new(); + let mut ident = variant.ident.clone(); + + for rename in attributes.renames.iter().rev() { + let from = format_ident!("{from}", from = *rename.from); + actions.insert( + *rename.since, + ItemStatus::Renamed { + from: from.clone(), + to: ident, + }, + ); + ident = from; + } + + // After the last iteration above (if any) we use the ident for the + // added action if there is any. + if let Some(added) = attributes.added { + actions.insert( + *added.since, + ItemStatus::Added { + default_fn: added.default_fn.deref().clone(), + ident, + }, + ); + } + + Self { + chain: Some(actions), + inner: variant, + } + } else { + if let Some(added) = attributes.added { + let mut actions = BTreeMap::new(); + + actions.insert( + *added.since, + ItemStatus::Added { + default_fn: added.default_fn.deref().clone(), + ident: variant.ident.clone(), + }, + ); + + return Self { + chain: Some(actions), + inner: variant, + }; + } + + Self { + chain: None, + inner: variant, + } + } } - fn generate_for_container( - &self, - container_version: &crate::gen::common::ContainerVersion, - ) -> Option { - todo!() + fn generate_for_container(&self, container_version: &ContainerVersion) -> Option { + match &self.chain { + Some(chain) => match chain + .get(&container_version.inner) + .expect("internal error: chain must contain container version") + { + ItemStatus::Added { ident, .. } => Some(quote! { + #ident, + }), + ItemStatus::Renamed { from, to } => todo!(), + ItemStatus::Deprecated { + previous_ident, + ident, + note, + } => todo!(), + ItemStatus::NoChange(_) => todo!(), + ItemStatus::NotPresent => todo!(), + }, + None => { + // If there is no chain of variant actions, the variant is not + // versioned and code generation is straight forward. + // Unversioned variants are always included in versioned enums. + let variant_ident = &self.inner.ident; + + Some(quote! { + #variant_ident, + }) + } + } } fn generate_for_from_impl( &self, - version: &crate::gen::common::ContainerVersion, - next_version: &crate::gen::common::ContainerVersion, + version: &ContainerVersion, + next_version: &ContainerVersion, from_ident: &syn::Ident, - ) -> proc_macro2::TokenStream { + ) -> TokenStream { todo!() } - fn get_ident(&self, version: &crate::gen::common::ContainerVersion) -> Option<&syn::Ident> { - todo!() + fn get_ident(&self, version: &ContainerVersion) -> Option<&syn::Ident> { + match &self.chain { + Some(chain) => chain + .get(&version.inner) + .expect("internal error: chain must contain container version") + .get_ident(), + None => Some(&self.inner.ident), + } + } + + fn chain(&mut self) -> Option<&mut VersionChain> { + self.chain.as_mut() } } diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index e7d3f01b7..7b3f9f2de 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -6,10 +6,8 @@ use syn::{Field, Ident}; use crate::{ attrs::field::FieldAttributes, - consts::DEPRECATED_FIELD_PREFIX, - gen::{ - common::{ContainerVersion, Item, ItemStatus, VersionChain}, - neighbors::Neighbors, + gen::common::{ + remove_deprecated_field_prefix, ContainerVersion, Item, ItemStatus, VersionChain, }, }; @@ -39,18 +37,15 @@ impl Item for VersionedField { // The ident of the deprecated field is guaranteed to include the // 'deprecated_' prefix. The ident can thus be used as is. if let Some(deprecated) = attributes.deprecated { - let deprecated_ident = field.ident.as_ref().unwrap(); + let deprecated_ident = field + .ident + .as_ref() + .expect("internal error: field must have an ident"); // When the field is deprecated, any rename which occurred beforehand // requires access to the field ident to infer the field ident for // the latest rename. - let mut ident = format_ident!( - "{ident}", - ident = deprecated_ident - .to_string() - .replace(DEPRECATED_FIELD_PREFIX, "") - ); - + let mut ident = remove_deprecated_field_prefix(&deprecated_ident); let mut actions = BTreeMap::new(); actions.insert( @@ -92,7 +87,10 @@ impl Item for VersionedField { } } else if !attributes.renames.is_empty() { let mut actions = BTreeMap::new(); - let mut ident = field.ident.clone().unwrap(); + let mut ident = field + .ident + .clone() + .expect("internal error: field must have an ident"); for rename in attributes.renames.iter().rev() { let from = format_ident!("{from}", from = *rename.from); @@ -147,55 +145,6 @@ impl Item for VersionedField { } } - fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { - if let Some(chain) = &mut self.chain { - for version in versions { - if chain.contains_key(&version.inner) { - continue; - } - - match chain.get_neighbors(&version.inner) { - (None, Some(status)) => match status { - ItemStatus::Added { .. } => { - chain.insert(version.inner, ItemStatus::NotPresent) - } - ItemStatus::Renamed { from, .. } => { - chain.insert(version.inner, ItemStatus::NoChange(from.clone())) - } - ItemStatus::Deprecated { previous_ident, .. } => chain - .insert(version.inner, ItemStatus::NoChange(previous_ident.clone())), - ItemStatus::NoChange(ident) => { - chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) - } - ItemStatus::NotPresent => unreachable!(), - }, - (Some(status), None) => { - let ident = match status { - ItemStatus::Added { ident, .. } => ident, - ItemStatus::Renamed { to, .. } => to, - ItemStatus::Deprecated { ident, .. } => ident, - ItemStatus::NoChange(ident) => ident, - ItemStatus::NotPresent => unreachable!(), - }; - - chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) - } - (Some(status), Some(_)) => { - let ident = match status { - ItemStatus::Added { ident, .. } => ident, - ItemStatus::Renamed { to, .. } => to, - ItemStatus::NoChange(ident) => ident, - _ => unreachable!(), - }; - - chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) - } - _ => unreachable!(), - }; - } - } - } - fn generate_for_container(&self, container_version: &ContainerVersion) -> Option { match &self.chain { Some(chain) => { @@ -216,7 +165,7 @@ impl Item for VersionedField { ItemStatus::Added { ident, .. } => Some(quote! { pub #ident: #field_type, }), - ItemStatus::Renamed { from: _, to } => Some(quote! { + ItemStatus::Renamed { to, .. } => Some(quote! { pub #to: #field_type, }), ItemStatus::Deprecated { @@ -294,4 +243,8 @@ impl Item for VersionedField { None => self.inner.ident.as_ref(), } } + + fn chain(&mut self) -> Option<&mut VersionChain> { + self.chain.as_mut() + } } diff --git a/crates/stackable-versioned-macros/tests/basic.rs b/crates/stackable-versioned-macros/tests/basic.rs index ef8a1c55b..a6f2027aa 100644 --- a/crates/stackable-versioned-macros/tests/basic.rs +++ b/crates/stackable-versioned-macros/tests/basic.rs @@ -12,12 +12,12 @@ use stackable_versioned_macros::versioned; version(name = "v3") )] struct Foo { - /// My docs #[versioned( added(since = "v1alpha1"), renamed(since = "v1beta1", from = "jjj"), deprecated(since = "v2", note = "not empty") )] + /// Test deprecated_bar: usize, baz: bool, } diff --git a/crates/stackable-versioned-macros/tests/enum.rs b/crates/stackable-versioned-macros/tests/enum.rs new file mode 100644 index 000000000..20e2797c2 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/enum.rs @@ -0,0 +1,10 @@ +use stackable_versioned_macros::versioned; + +#[test] +fn versioned_enum() { + #[versioned(version(name = "v1alpha1"), version(name = "v1beta1"))] + pub enum Foo { + Bar, + Baz, + } +} From 18e54c145c965830613a34489c44ea288844ccdf Mon Sep 17 00:00:00 2001 From: Techassi Date: Tue, 25 Jun 2024 16:49:10 +0200 Subject: [PATCH 08/18] Use darling(flatten) for field attrs --- .../src/attrs/common.rs | 31 -------- .../src/attrs/{ => common}/container.rs | 0 .../src/attrs/common/item.rs | 71 +++++++++++++++++++ .../src/attrs/common/mod.rs | 5 ++ .../src/attrs/field.rs | 58 ++++++--------- .../src/attrs/mod.rs | 1 - .../src/attrs/variant.rs | 11 ++- .../src/gen/common.rs | 2 +- .../stackable-versioned-macros/src/gen/mod.rs | 2 +- .../src/gen/venum/mod.rs | 2 +- .../src/gen/vstruct/field.rs | 16 ++--- .../src/gen/vstruct/mod.rs | 2 +- crates/stackable-versioned-macros/src/lib.rs | 2 +- .../stackable-versioned-macros/tests/enum.rs | 1 + 14 files changed, 115 insertions(+), 89 deletions(-) delete mode 100644 crates/stackable-versioned-macros/src/attrs/common.rs rename crates/stackable-versioned-macros/src/attrs/{ => common}/container.rs (100%) create mode 100644 crates/stackable-versioned-macros/src/attrs/common/item.rs create mode 100644 crates/stackable-versioned-macros/src/attrs/common/mod.rs diff --git a/crates/stackable-versioned-macros/src/attrs/common.rs b/crates/stackable-versioned-macros/src/attrs/common.rs deleted file mode 100644 index 1b003674a..000000000 --- a/crates/stackable-versioned-macros/src/attrs/common.rs +++ /dev/null @@ -1,31 +0,0 @@ -use darling::{util::SpannedValue, FromMeta}; -use k8s_version::Version; -use proc_macro2::Span; -use syn::Path; - -#[derive(Clone, Debug, FromMeta)] -pub(crate) struct AddedAttributes { - pub(crate) since: SpannedValue, - - #[darling(rename = "default", default = "default_default_fn")] - pub(crate) default_fn: SpannedValue, -} - -fn default_default_fn() -> SpannedValue { - SpannedValue::new( - syn::parse_str("std::default::Default::default").expect("internal error: path must parse"), - Span::call_site(), - ) -} - -#[derive(Clone, Debug, FromMeta)] -pub(crate) struct RenamedAttributes { - pub(crate) since: SpannedValue, - pub(crate) from: SpannedValue, -} - -#[derive(Clone, Debug, FromMeta)] -pub(crate) struct DeprecatedAttributes { - pub(crate) since: SpannedValue, - pub(crate) note: SpannedValue, -} diff --git a/crates/stackable-versioned-macros/src/attrs/container.rs b/crates/stackable-versioned-macros/src/attrs/common/container.rs similarity index 100% rename from crates/stackable-versioned-macros/src/attrs/container.rs rename to crates/stackable-versioned-macros/src/attrs/common/container.rs diff --git a/crates/stackable-versioned-macros/src/attrs/common/item.rs b/crates/stackable-versioned-macros/src/attrs/common/item.rs new file mode 100644 index 000000000..94be3e1a1 --- /dev/null +++ b/crates/stackable-versioned-macros/src/attrs/common/item.rs @@ -0,0 +1,71 @@ +use darling::{util::SpannedValue, Error, FromMeta}; +use k8s_version::Version; +use proc_macro2::Span; +use syn::Path; + +/// These attributes are meant to be used in super structs, which add +/// [`Field`](syn::Field) or [`Variant`](syn::Variant) specific attributes via +/// darling's flatten feature. This struct only provides shared attributes. +#[derive(Debug, FromMeta)] +#[darling(and_then = ItemAttributes::validate)] +pub(crate) struct ItemAttributes { + /// This parses the `added` attribute on items (fields or variants). It can + /// only be present at most once. + pub(crate) added: Option, + + /// This parses the `renamed` attribute on items (fields or variants). It + /// can be present 0..n times. + #[darling(multiple, rename = "renamed")] + pub(crate) renames: Vec, + + /// This parses the `deprecated` attribute on items (fields or variants). It + /// can only be present at most once. + pub(crate) deprecated: Option, +} + +impl ItemAttributes { + fn validate(self) -> Result { + // Validate deprecated options + + // TODO (@Techassi): Make the field 'note' optional, because in the + // future, the macro will generate parts of the deprecation note + // automatically. The user-provided note will then be appended to the + // auto-generated one. + + if let Some(deprecated) = &self.deprecated { + if deprecated.note.is_empty() { + return Err(Error::custom("deprecation note must not be empty") + .with_span(&deprecated.note.span())); + } + } + + Ok(self) + } +} + +#[derive(Clone, Debug, FromMeta)] +pub(crate) struct AddedAttributes { + pub(crate) since: SpannedValue, + + #[darling(rename = "default", default = "default_default_fn")] + pub(crate) default_fn: SpannedValue, +} + +fn default_default_fn() -> SpannedValue { + SpannedValue::new( + syn::parse_str("std::default::Default::default").expect("internal error: path must parse"), + Span::call_site(), + ) +} + +#[derive(Clone, Debug, FromMeta)] +pub(crate) struct RenamedAttributes { + pub(crate) since: SpannedValue, + pub(crate) from: SpannedValue, +} + +#[derive(Clone, Debug, FromMeta)] +pub(crate) struct DeprecatedAttributes { + pub(crate) since: SpannedValue, + pub(crate) note: SpannedValue, +} diff --git a/crates/stackable-versioned-macros/src/attrs/common/mod.rs b/crates/stackable-versioned-macros/src/attrs/common/mod.rs new file mode 100644 index 000000000..175e8eed5 --- /dev/null +++ b/crates/stackable-versioned-macros/src/attrs/common/mod.rs @@ -0,0 +1,5 @@ +mod container; +mod item; + +pub(crate) use container::*; +pub(crate) use item::*; diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index 33d5d9df8..af440c3cd 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -2,10 +2,7 @@ use darling::{Error, FromField}; use syn::{Field, Ident}; use crate::{ - attrs::{ - common::{AddedAttributes, DeprecatedAttributes, RenamedAttributes}, - container::ContainerAttributes, - }, + attrs::common::{ContainerAttributes, ItemAttributes}, consts::DEPRECATED_FIELD_PREFIX, }; @@ -33,13 +30,13 @@ use crate::{ and_then = FieldAttributes::validate )] pub(crate) struct FieldAttributes { - pub(crate) ident: Option, - pub(crate) added: Option, - - #[darling(multiple, rename = "renamed")] - pub(crate) renames: Vec, + #[darling(flatten)] + pub(crate) common: ItemAttributes, - pub(crate) deprecated: Option, + // The ident (automatically extracted by darling) cannot be moved into the + // shared item attributes because for struct fields, the type is + // `Option`, while for enum variants, the type is `Ident`. + pub(crate) ident: Option, } impl FieldAttributes { @@ -57,9 +54,6 @@ impl FieldAttributes { errors.handle(self.validate_action_order()); errors.handle(self.validate_field_name()); - // Code quality validation - errors.handle(self.validate_deprecated_options()); - // TODO (@Techassi): Add validation for renames so that renamed fields // match up and form a continuous chain (eg. foo -> bar -> baz). @@ -84,7 +78,11 @@ impl FieldAttributes { /// - `renamed` and `deprecated` using the same version: Again, the same /// rules from above apply here as well. fn validate_action_combinations(&self) -> Result<(), Error> { - match (&self.added, &self.renames, &self.deprecated) { + match ( + &self.common.added, + &self.common.renames, + &self.common.deprecated, + ) { (Some(added), _, Some(deprecated)) if *added.since == *deprecated.since => { Err(Error::custom( "field cannot be marked as `added` and `deprecated` in the same version", @@ -122,8 +120,8 @@ impl FieldAttributes { /// - All `renamed` actions must use a greater version than `added` but a /// lesser version than `deprecated`. fn validate_action_order(&self) -> Result<(), Error> { - let added_version = self.added.as_ref().map(|a| *a.since); - let deprecated_version = self.deprecated.as_ref().map(|d| *d.since); + let added_version = self.common.added.as_ref().map(|a| *a.since); + let deprecated_version = self.common.deprecated.as_ref().map(|d| *d.since); // First, validate that the added version is less than the deprecated // version. @@ -139,7 +137,7 @@ impl FieldAttributes { // Now, iterate over all renames and ensure that their versions are // between the added and deprecated version. - if !self.renames.iter().all(|r| { + if !self.common.renames.iter().all(|r| { added_version.map_or(true, |a| a < *r.since) && deprecated_version.map_or(true, |d| d > *r.since) }) { @@ -169,13 +167,13 @@ impl FieldAttributes { .to_string() .starts_with(DEPRECATED_FIELD_PREFIX); - if self.deprecated.is_some() && !starts_with { + if self.common.deprecated.is_some() && !starts_with { return Err(Error::custom( "field was marked as `deprecated` and thus must include the `deprecated_` prefix in its name" ).with_span(&self.ident)); } - if self.deprecated.is_none() && starts_with { + if self.common.deprecated.is_none() && starts_with { return Err(Error::custom( "field includes the `deprecated_` prefix in its name but is not marked as `deprecated`" ).with_span(&self.ident)); @@ -184,22 +182,6 @@ impl FieldAttributes { Ok(()) } - fn validate_deprecated_options(&self) -> Result<(), Error> { - // TODO (@Techassi): Make the field 'note' optional, because in the - // future, the macro will generate parts of the deprecation note - // automatically. The user-provided note will then be appended to the - // auto-generated one. - - if let Some(deprecated) = &self.deprecated { - if deprecated.note.is_empty() { - return Err(Error::custom("deprecation note must not be empty") - .with_span(&deprecated.note.span())); - } - } - - Ok(()) - } - /// Validates that each field action version is present in the declared /// container versions. pub(crate) fn validate_versions( @@ -210,7 +192,7 @@ impl FieldAttributes { // NOTE (@Techassi): Can we maybe optimize this a little? let mut errors = Error::accumulator(); - if let Some(added) = &self.added { + if let Some(added) = &self.common.added { if !container_attrs .versions .iter() @@ -223,7 +205,7 @@ impl FieldAttributes { } } - for rename in &self.renames { + for rename in &self.common.renames { if !container_attrs .versions .iter() @@ -236,7 +218,7 @@ impl FieldAttributes { } } - if let Some(deprecated) = &self.deprecated { + if let Some(deprecated) = &self.common.deprecated { if !container_attrs .versions .iter() diff --git a/crates/stackable-versioned-macros/src/attrs/mod.rs b/crates/stackable-versioned-macros/src/attrs/mod.rs index e320be086..6eb6b96bd 100644 --- a/crates/stackable-versioned-macros/src/attrs/mod.rs +++ b/crates/stackable-versioned-macros/src/attrs/mod.rs @@ -1,4 +1,3 @@ pub(crate) mod common; -pub(crate) mod container; pub(crate) mod field; pub(crate) mod variant; diff --git a/crates/stackable-versioned-macros/src/attrs/variant.rs b/crates/stackable-versioned-macros/src/attrs/variant.rs index f73c7d54c..fc40e1733 100644 --- a/crates/stackable-versioned-macros/src/attrs/variant.rs +++ b/crates/stackable-versioned-macros/src/attrs/variant.rs @@ -4,9 +4,8 @@ use darling::{Error, FromVariant}; use syn::{Ident, Variant}; -use crate::attrs::{ - common::{AddedAttributes, DeprecatedAttributes, RenamedAttributes}, - container::ContainerAttributes, +use crate::attrs::common::{ + AddedAttributes, ContainerAttributes, DeprecatedAttributes, RenamedAttributes, }; #[derive(Debug, FromVariant)] @@ -47,7 +46,7 @@ impl VariantAttributes { .any(|v| v.name == *added.since) { errors.push(Error::custom( - "field action `added` uses version which was not declared via #[versioned(version)]") + "variant action `added` uses version which was not declared via #[versioned(version)]") .with_span(&variant.ident) ); } @@ -60,7 +59,7 @@ impl VariantAttributes { .any(|v| v.name == *rename.since) { errors.push( - Error::custom("field action `renamed` uses version which was not declared via #[versioned(version)]") + Error::custom("variant action `renamed` uses version which was not declared via #[versioned(version)]") .with_span(&variant.ident) ); } @@ -73,7 +72,7 @@ impl VariantAttributes { .any(|v| v.name == *deprecated.since) { errors.push(Error::custom( - "field action `deprecated` uses version which was not declared via #[versioned(version)]") + "variant action `deprecated` uses version which was not declared via #[versioned(version)]") .with_span(&variant.ident) ); } diff --git a/crates/stackable-versioned-macros/src/gen/common.rs b/crates/stackable-versioned-macros/src/gen/common.rs index 3a713b9d7..4fbcce1b9 100644 --- a/crates/stackable-versioned-macros/src/gen/common.rs +++ b/crates/stackable-versioned-macros/src/gen/common.rs @@ -6,7 +6,7 @@ use quote::format_ident; use syn::{Field, Ident, Path, Variant}; use crate::{ - attrs::container::ContainerAttributes, + attrs::common::ContainerAttributes, consts::{DEPRECATED_FIELD_PREFIX, DEPRECATED_VARIANT_PREFIX}, gen::neighbors::Neighbors, }; diff --git a/crates/stackable-versioned-macros/src/gen/mod.rs b/crates/stackable-versioned-macros/src/gen/mod.rs index 980c85869..66f2797d1 100644 --- a/crates/stackable-versioned-macros/src/gen/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/mod.rs @@ -2,7 +2,7 @@ use proc_macro2::TokenStream; use syn::{spanned::Spanned, Data, DeriveInput, Error, Result}; use crate::{ - attrs::container::ContainerAttributes, + attrs::common::ContainerAttributes, gen::{common::Container, venum::VersionedEnum, vstruct::VersionedStruct}, }; diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index f21b52896..9a13e9713 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -7,7 +7,7 @@ use quote::quote; use syn::{DataEnum, Error, Ident}; use crate::{ - attrs::{container::ContainerAttributes, variant::VariantAttributes}, + attrs::{common::ContainerAttributes, variant::VariantAttributes}, gen::{ common::{ format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index 7b3f9f2de..53e93bf8f 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -24,7 +24,7 @@ pub(crate) struct VersionedField { } impl Item for VersionedField { - fn new(field: syn::Field, attributes: FieldAttributes) -> Self { + fn new(field: syn::Field, field_attrs: FieldAttributes) -> Self { // Constructing the action chain requires going through the actions from // the end, because the base struct always represents the latest (most // up-to-date) version of that struct. That's why the following code @@ -36,7 +36,7 @@ impl Item for VersionedField { // rename or addition, which is handled below. // The ident of the deprecated field is guaranteed to include the // 'deprecated_' prefix. The ident can thus be used as is. - if let Some(deprecated) = attributes.deprecated { + if let Some(deprecated) = field_attrs.common.deprecated { let deprecated_ident = field .ident .as_ref() @@ -57,7 +57,7 @@ impl Item for VersionedField { }, ); - for rename in attributes.renames.iter().rev() { + for rename in field_attrs.common.renames.iter().rev() { let from = format_ident!("{from}", from = *rename.from); actions.insert( *rename.since, @@ -71,7 +71,7 @@ impl Item for VersionedField { // After the last iteration above (if any) we use the ident for the // added action if there is any. - if let Some(added) = attributes.added { + if let Some(added) = field_attrs.common.added { actions.insert( *added.since, ItemStatus::Added { @@ -85,14 +85,14 @@ impl Item for VersionedField { chain: Some(actions), inner: field, } - } else if !attributes.renames.is_empty() { + } else if !field_attrs.common.renames.is_empty() { let mut actions = BTreeMap::new(); let mut ident = field .ident .clone() .expect("internal error: field must have an ident"); - for rename in attributes.renames.iter().rev() { + for rename in field_attrs.common.renames.iter().rev() { let from = format_ident!("{from}", from = *rename.from); actions.insert( *rename.since, @@ -106,7 +106,7 @@ impl Item for VersionedField { // After the last iteration above (if any) we use the ident for the // added action if there is any. - if let Some(added) = attributes.added { + if let Some(added) = field_attrs.common.added { actions.insert( *added.since, ItemStatus::Added { @@ -121,7 +121,7 @@ impl Item for VersionedField { inner: field, } } else { - if let Some(added) = attributes.added { + if let Some(added) = field_attrs.common.added { let mut actions = BTreeMap::new(); actions.insert( diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index 42b24bf56..2daf41f57 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -7,7 +7,7 @@ use quote::quote; use syn::{DataStruct, Error, Ident}; use crate::{ - attrs::{container::ContainerAttributes, field::FieldAttributes}, + attrs::{common::ContainerAttributes, field::FieldAttributes}, gen::{ common::{ format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, diff --git a/crates/stackable-versioned-macros/src/lib.rs b/crates/stackable-versioned-macros/src/lib.rs index 9c8ce122d..44a8b7e51 100644 --- a/crates/stackable-versioned-macros/src/lib.rs +++ b/crates/stackable-versioned-macros/src/lib.rs @@ -2,7 +2,7 @@ use darling::{ast::NestedMeta, FromMeta}; use proc_macro::TokenStream; use syn::{DeriveInput, Error}; -use crate::attrs::container::ContainerAttributes; +use crate::attrs::common::ContainerAttributes; mod attrs; mod consts; diff --git a/crates/stackable-versioned-macros/tests/enum.rs b/crates/stackable-versioned-macros/tests/enum.rs index 20e2797c2..637cf05d1 100644 --- a/crates/stackable-versioned-macros/tests/enum.rs +++ b/crates/stackable-versioned-macros/tests/enum.rs @@ -4,6 +4,7 @@ use stackable_versioned_macros::versioned; fn versioned_enum() { #[versioned(version(name = "v1alpha1"), version(name = "v1beta1"))] pub enum Foo { + // #[versioned(renamed(since = "v1beta1", from = "bat"))] Bar, Baz, } From e173abab6a66075dfb383fff7f2afd139b24d0b1 Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 26 Jun 2024 15:54:41 +0200 Subject: [PATCH 09/18] Replace unwraps with expects --- .../src/attrs/common/container.rs | 8 +++++++- .../stackable-versioned-macros/src/attrs/field.rs | 2 +- .../src/gen/vstruct/field.rs | 14 +++++++++++--- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/crates/stackable-versioned-macros/src/attrs/common/container.rs b/crates/stackable-versioned-macros/src/attrs/common/container.rs index 918b81042..94c1444b4 100644 --- a/crates/stackable-versioned-macros/src/attrs/common/container.rs +++ b/crates/stackable-versioned-macros/src/attrs/common/container.rs @@ -49,7 +49,13 @@ impl ContainerAttributes { .sort_by(|lhs, rhs| lhs.name.partial_cmp(&rhs.name).unwrap_or(Ordering::Equal)); for (index, version) in original.iter().enumerate() { - if version.name == self.versions.get(index).unwrap().name { + if version.name + == self + .versions + .get(index) + .expect("internal error: version at that index must exist") + .name + { continue; } diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index af440c3cd..73a4945b9 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -163,7 +163,7 @@ impl FieldAttributes { let starts_with = self .ident .as_ref() - .unwrap() + .expect("internal error: to be validated fields must have a name") .to_string() .starts_with(DEPRECATED_FIELD_PREFIX); diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index 53e93bf8f..9fe885aa7 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -128,7 +128,10 @@ impl Item for VersionedField { *added.since, ItemStatus::Added { default_fn: added.default_fn.deref().clone(), - ident: field.ident.clone().unwrap(), + ident: field + .ident + .clone() + .expect("internal error: field must have a name"), }, ); @@ -216,8 +219,13 @@ impl Item for VersionedField { #ident: #default_fn(), }, (old, next) => { - let old_field_ident = old.get_ident().unwrap(); - let next_field_ident = next.get_ident().unwrap(); + let old_field_ident = old + .get_ident() + .expect("internal error: old field must have a name"); + + let next_field_ident = next + .get_ident() + .expect("internal error: new field must have a name"); quote! { #next_field_ident: #from_ident.#old_field_ident, From 96381d763b9e9a31e4781c9ed10e056ae36aefe1 Mon Sep 17 00:00:00 2001 From: Techassi Date: Wed, 26 Jun 2024 15:54:53 +0200 Subject: [PATCH 10/18] Generate code for all item states --- Cargo.lock | 16 ++ Cargo.toml | 1 + crates/stackable-versioned-macros/Cargo.toml | 1 + .../src/attrs/field.rs | 10 +- .../src/attrs/variant.rs | 182 ++++++++++++++++-- .../stackable-versioned-macros/src/consts.rs | 2 +- .../src/gen/venum/variant.rs | 35 ++-- .../stackable-versioned-macros/tests/enum.rs | 10 +- 8 files changed, 221 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c10f94a1d..2a1f148ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -548,6 +548,15 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "convert_case" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec182b0ca2f35d8fc196cf3404988fd8b8c739a4d270ff118a398feb0cbec1ca" +dependencies = [ + "unicode-segmentation", +] + [[package]] name = "core-foundation" version = "0.9.4" @@ -2993,6 +3002,7 @@ dependencies = [ name = "stackable-versioned-macros" version = "0.1.0" dependencies = [ + "convert_case", "darling", "itertools 0.13.0", "k8s-version", @@ -3503,6 +3513,12 @@ dependencies = [ "tinyvec", ] +[[package]] +name = "unicode-segmentation" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202" + [[package]] name = "unicode-xid" version = "0.2.4" diff --git a/Cargo.toml b/Cargo.toml index 2b857392b..9ee3293f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ chrono = { version = "0.4.38", default-features = false } clap = { version = "4.5.4", features = ["derive", "cargo", "env"] } const_format = "0.2.32" const-oid = "0.9.6" +convert_case = "0.6.0" darling = "0.20.9" delegate = "0.12.0" derivative = "2.2.0" diff --git a/crates/stackable-versioned-macros/Cargo.toml b/crates/stackable-versioned-macros/Cargo.toml index 13a5a18e7..5428289d9 100644 --- a/crates/stackable-versioned-macros/Cargo.toml +++ b/crates/stackable-versioned-macros/Cargo.toml @@ -12,6 +12,7 @@ proc-macro = true [dependencies] k8s-version = { path = "../k8s-version", features = ["darling"] } +convert_case.workspace = true darling.workspace = true itertools.workspace = true proc-macro2.workspace = true diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index 73a4945b9..477f17e37 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -40,6 +40,14 @@ pub(crate) struct FieldAttributes { } impl FieldAttributes { + // NOTE (@Techassi): Ideally, these validations should be moved to the + // ItemAttributes impl, because common validation like action combinations + // and action order can be validated without taking the type of attribute + // into account (field vs variant). However, we would loose access to the + // field / variant ident and as such, cannot display the error directly on + // the affected field / variant. This is a significant decrease in DX. + // See https://github.com/TedDriggs/darling/discussions/294 + /// This associated function is called by darling (see and_then attribute) /// after it successfully parsed the attribute. This allows custom /// validation of the attribute which extends the validation already in @@ -128,7 +136,7 @@ impl FieldAttributes { // NOTE (@Techassi): Is this already covered by the code below? if let (Some(added_version), Some(deprecated_version)) = (added_version, deprecated_version) { - if added_version >= deprecated_version { + if added_version > deprecated_version { return Err(Error::custom(format!( "field was marked as `added` in version `{added_version}` while being marked as `deprecated` in an earlier version `{deprecated_version}`" )).with_span(&self.ident)); diff --git a/crates/stackable-versioned-macros/src/attrs/variant.rs b/crates/stackable-versioned-macros/src/attrs/variant.rs index fc40e1733..f34cb1271 100644 --- a/crates/stackable-versioned-macros/src/attrs/variant.rs +++ b/crates/stackable-versioned-macros/src/attrs/variant.rs @@ -1,11 +1,10 @@ -// TODO (@Techassi): Think about what can be moved into a common impl for both -// fields and variants. - +use convert_case::{Case, Casing}; use darling::{Error, FromVariant}; use syn::{Ident, Variant}; -use crate::attrs::common::{ - AddedAttributes, ContainerAttributes, DeprecatedAttributes, RenamedAttributes, +use crate::{ + attrs::common::{ContainerAttributes, ItemAttributes}, + consts::DEPRECATED_VARIANT_PREFIX, }; #[derive(Debug, FromVariant)] @@ -15,20 +14,173 @@ use crate::attrs::common::{ and_then = VariantAttributes::validate )] pub(crate) struct VariantAttributes { - pub(crate) ident: Ident, - pub(crate) added: Option, - - #[darling(multiple, rename = "renamed")] - pub(crate) renames: Vec, + #[darling(flatten)] + pub(crate) common: ItemAttributes, - pub(crate) deprecated: Option, + // The ident (automatically extracted by darling) cannot be moved into the + // shared item attributes because for struct fields, the type is + // `Option`, while for enum variants, the type is `Ident`. + pub(crate) ident: Ident, } impl VariantAttributes { - pub(crate) fn validate(self) -> Result { + // NOTE (@Techassi): Ideally, these validations should be moved to the + // ItemAttributes impl, because common validation like action combinations + // and action order can be validated without taking the type of attribute + // into account (field vs variant). However, we would loose access to the + // field / variant ident and as such, cannot display the error directly on + // the affected field / variant. This is a significant decrease in DX. + // See https://github.com/TedDriggs/darling/discussions/294 + + /// This associated function is called by darling (see and_then attribute) + /// after it successfully parsed the attribute. This allows custom + /// validation of the attribute which extends the validation already in + /// place by darling. + /// + /// Internally, it calls out to other specialized validation functions. + fn validate(self) -> Result { + let mut errors = Error::accumulator(); + + // Semantic validation + errors.handle(self.validate_action_combinations()); + errors.handle(self.validate_action_order()); + errors.handle(self.validate_variant_name()); + + // TODO (@Techassi): Add validation for renames so that renamed items + // match up and form a continuous chain (eg. foo -> bar -> baz). + + // TODO (@Techassi): Add hint if a item is added in the first version + // that it might be clever to remove the 'added' attribute. + + errors.finish()?; Ok(self) } + /// This associated function is called by the top-level validation function + /// and validates that each variant uses a valid combination of actions. + /// Invalid combinations are: + /// + /// - `added` and `deprecated` using the same version: A variant cannot be + /// marked as added in a particular version and then marked as deprecated + /// immediately after. Variants must be included for at least one version + /// before being marked deprecated. + /// - `added` and `renamed` using the same version: The same reasoning from + /// above applies here as well. Variants must be included for at least one + /// version before being renamed. + /// - `renamed` and `deprecated` using the same version: Again, the same + /// rules from above apply here as well. + fn validate_action_combinations(&self) -> Result<(), Error> { + match ( + &self.common.added, + &self.common.renames, + &self.common.deprecated, + ) { + (Some(added), _, Some(deprecated)) if *added.since == *deprecated.since => { + Err(Error::custom( + "variant cannot be marked as `added` and `deprecated` in the same version", + ) + .with_span(&self.ident)) + } + (Some(added), renamed, _) if renamed.iter().any(|r| *r.since == *added.since) => { + Err(Error::custom( + "variant cannot be marked as `added` and `renamed` in the same version", + ) + .with_span(&self.ident)) + } + (_, renamed, Some(deprecated)) + if renamed.iter().any(|r| *r.since == *deprecated.since) => + { + Err(Error::custom( + "variant cannot be marked as `deprecated` and `renamed` in the same version", + ) + .with_span(&self.ident)) + } + _ => Ok(()), + } + } + + /// This associated function is called by the top-level validation function + /// and validates that actions use a chronologically sound chain of + /// versions. + /// + /// The following rules apply: + /// + /// - `deprecated` must use a greater version than `added`: This function + /// ensures that these versions are chronologically sound, that means, + /// that the version of the deprecated action must be greater than the + /// version of the added action. + /// - All `renamed` actions must use a greater version than `added` but a + /// lesser version than `deprecated`. + fn validate_action_order(&self) -> Result<(), Error> { + let added_version = self.common.added.as_ref().map(|a| *a.since); + let deprecated_version = self.common.deprecated.as_ref().map(|d| *d.since); + + // First, validate that the added version is less than the deprecated + // version. + // NOTE (@Techassi): Is this already covered by the code below? + if let (Some(added_version), Some(deprecated_version)) = (added_version, deprecated_version) + { + if added_version > deprecated_version { + return Err(Error::custom(format!( + "variant was marked as `added` in version `{added_version}` while being marked as `deprecated` in an earlier version `{deprecated_version}`" + )).with_span(&self.ident)); + } + } + + // Now, iterate over all renames and ensure that their versions are + // between the added and deprecated version. + if !self.common.renames.iter().all(|r| { + added_version.map_or(true, |a| a < *r.since) + && deprecated_version.map_or(true, |d| d > *r.since) + }) { + return Err(Error::custom( + "all renames must use versions higher than `added` and lower than `deprecated`", + ) + .with_span(&self.ident)); + } + + Ok(()) + } + + /// This associated function is called by the top-level validation function + /// and validates that variants use correct names depending on attached + /// actions. + /// + /// The following naming rules apply: + /// + /// - Variants marked as deprecated need to include the 'deprecated_' prefix + /// in their name. The prefix must not be included for variants which are + /// not deprecated. + fn validate_variant_name(&self) -> Result<(), Error> { + if !self + .common + .renames + .iter() + .all(|r| r.from.is_case(Case::Pascal)) + { + return Err(Error::custom("renamed variants must use PascalCase")); + } + + let starts_with = self + .ident + .to_string() + .starts_with(DEPRECATED_VARIANT_PREFIX); + + if self.common.deprecated.is_some() && !starts_with { + return Err(Error::custom( + "variant was marked as `deprecated` and thus must include the `Deprecated` prefix in its name" + ).with_span(&self.ident)); + } + + if self.common.deprecated.is_none() && starts_with { + return Err(Error::custom( + "variant includes the `Deprecated` prefix in its name but is not marked as `deprecated`" + ).with_span(&self.ident)); + } + + Ok(()) + } + pub(crate) fn validate_versions( &self, container_attrs: &ContainerAttributes, @@ -39,7 +191,7 @@ impl VariantAttributes { // a T: Spanned bound for the second function parameter. let mut errors = Error::accumulator(); - if let Some(added) = &self.added { + if let Some(added) = &self.common.added { if !container_attrs .versions .iter() @@ -52,7 +204,7 @@ impl VariantAttributes { } } - for rename in &self.renames { + for rename in &*self.common.renames { if !container_attrs .versions .iter() @@ -65,7 +217,7 @@ impl VariantAttributes { } } - if let Some(deprecated) = &self.deprecated { + if let Some(deprecated) = &self.common.deprecated { if !container_attrs .versions .iter() diff --git a/crates/stackable-versioned-macros/src/consts.rs b/crates/stackable-versioned-macros/src/consts.rs index b373331e0..d064c344d 100644 --- a/crates/stackable-versioned-macros/src/consts.rs +++ b/crates/stackable-versioned-macros/src/consts.rs @@ -1,2 +1,2 @@ -pub(crate) const DEPRECATED_VARIANT_PREFIX: &str = "DEPRECATED_"; +pub(crate) const DEPRECATED_VARIANT_PREFIX: &str = "Deprecated"; pub(crate) const DEPRECATED_FIELD_PREFIX: &str = "deprecated_"; diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs index 0529b4cd9..71d434c32 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -22,10 +22,10 @@ pub(crate) struct VersionedVariant { // identical. impl Item for VersionedVariant { - fn new(variant: Variant, attributes: VariantAttributes) -> Self { + fn new(variant: Variant, variant_attrs: VariantAttributes) -> Self { // NOTE (@Techassi): This is straight up copied from the VersionedField // impl. As mentioned above, unify this. - if let Some(deprecated) = attributes.deprecated { + if let Some(deprecated) = variant_attrs.common.deprecated { let deprecated_ident = &variant.ident; // When the field is deprecated, any rename which occurred beforehand @@ -43,7 +43,7 @@ impl Item for VersionedVariant { }, ); - for rename in attributes.renames.iter().rev() { + for rename in variant_attrs.common.renames.iter().rev() { let from = format_ident!("{from}", from = *rename.from); actions.insert( *rename.since, @@ -57,7 +57,7 @@ impl Item for VersionedVariant { // After the last iteration above (if any) we use the ident for the // added action if there is any. - if let Some(added) = attributes.added { + if let Some(added) = variant_attrs.common.added { actions.insert( *added.since, ItemStatus::Added { @@ -71,11 +71,11 @@ impl Item for VersionedVariant { chain: Some(actions), inner: variant, } - } else if !attributes.renames.is_empty() { + } else if !variant_attrs.common.renames.is_empty() { let mut actions = BTreeMap::new(); let mut ident = variant.ident.clone(); - for rename in attributes.renames.iter().rev() { + for rename in variant_attrs.common.renames.iter().rev() { let from = format_ident!("{from}", from = *rename.from); actions.insert( *rename.since, @@ -89,7 +89,7 @@ impl Item for VersionedVariant { // After the last iteration above (if any) we use the ident for the // added action if there is any. - if let Some(added) = attributes.added { + if let Some(added) = variant_attrs.common.added { actions.insert( *added.since, ItemStatus::Added { @@ -104,7 +104,7 @@ impl Item for VersionedVariant { inner: variant, } } else { - if let Some(added) = attributes.added { + if let Some(added) = variant_attrs.common.added { let mut actions = BTreeMap::new(); actions.insert( @@ -137,14 +137,17 @@ impl Item for VersionedVariant { ItemStatus::Added { ident, .. } => Some(quote! { #ident, }), - ItemStatus::Renamed { from, to } => todo!(), - ItemStatus::Deprecated { - previous_ident, - ident, - note, - } => todo!(), - ItemStatus::NoChange(_) => todo!(), - ItemStatus::NotPresent => todo!(), + ItemStatus::Renamed { to, .. } => Some(quote! { + #to, + }), + ItemStatus::Deprecated { ident, .. } => Some(quote! { + #[deprecated] + #ident, + }), + ItemStatus::NoChange(ident) => Some(quote! { + #ident, + }), + ItemStatus::NotPresent => None, }, None => { // If there is no chain of variant actions, the variant is not diff --git a/crates/stackable-versioned-macros/tests/enum.rs b/crates/stackable-versioned-macros/tests/enum.rs index 637cf05d1..7a147ad5f 100644 --- a/crates/stackable-versioned-macros/tests/enum.rs +++ b/crates/stackable-versioned-macros/tests/enum.rs @@ -2,10 +2,14 @@ use stackable_versioned_macros::versioned; #[test] fn versioned_enum() { - #[versioned(version(name = "v1alpha1"), version(name = "v1beta1"))] + #[versioned( + version(name = "v1alpha1"), + version(name = "v1beta1"), + version(name = "v1") + )] pub enum Foo { - // #[versioned(renamed(since = "v1beta1", from = "bat"))] - Bar, + #[versioned(added(since = "v1beta1"), deprecated(since = "v1", note = "bye"))] + DeprecatedBar, Baz, } } From c125b98077f3a983d62b9f94fefd6a8f862b0d1d Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 27 Jun 2024 16:17:06 +0200 Subject: [PATCH 11/18] Start adding From ipls for enum conversion --- .../src/gen/common.rs | 108 +----------------- .../src/gen/venum/mod.rs | 52 ++++++++- .../src/gen/venum/variant.rs | 98 +++++++++++++--- .../src/gen/vstruct/field.rs | 102 +++++++++++++---- .../src/gen/vstruct/mod.rs | 13 +-- .../stackable-versioned-macros/tests/enum.rs | 18 +++ 6 files changed, 241 insertions(+), 150 deletions(-) diff --git a/crates/stackable-versioned-macros/src/gen/common.rs b/crates/stackable-versioned-macros/src/gen/common.rs index 4fbcce1b9..6d6b37a75 100644 --- a/crates/stackable-versioned-macros/src/gen/common.rs +++ b/crates/stackable-versioned-macros/src/gen/common.rs @@ -3,12 +3,11 @@ use std::{collections::BTreeMap, ops::Deref}; use k8s_version::Version; use proc_macro2::{Span, TokenStream}; use quote::format_ident; -use syn::{Field, Ident, Path, Variant}; +use syn::{Ident, Path}; use crate::{ attrs::common::ContainerAttributes, consts::{DEPRECATED_FIELD_PREFIX, DEPRECATED_VARIANT_PREFIX}, - gen::neighbors::Neighbors, }; pub(crate) type VersionChain = BTreeMap; @@ -61,111 +60,6 @@ pub(crate) struct VersionedContainer { pub(crate) skip_from: bool, } -pub(crate) trait Item -where - Self: Sized, - I: GetIdent, -{ - /// Create a new versioned item (field or variant) by creating a status - /// chain for each version defined in an action in the item attribute. - /// - /// This chain will get extended by the versions defined on the container by - /// calling the [`Item::insert_container_versions`] function. - fn new(item: I, attributes: A) -> Self; - - /// Inserts container versions not yet present in the status chain. - /// - /// When initially creating a new versioned item, the code doesn't have - /// access to the versions defined on the container. This function inserts - /// all non-present container versions and decides which status and ident - /// is the right fit based on the status neighbors. - /// - /// This continuous chain ensures that when generating code (tokens), each - /// field can lookup the status (and ident) for a requested version. - fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { - if let Some(chain) = self.chain() { - for version in versions { - if chain.contains_key(&version.inner) { - continue; - } - - match chain.get_neighbors(&version.inner) { - (None, Some(status)) => match status { - ItemStatus::Added { .. } => { - chain.insert(version.inner, ItemStatus::NotPresent) - } - ItemStatus::Renamed { from, .. } => { - chain.insert(version.inner, ItemStatus::NoChange(from.clone())) - } - ItemStatus::Deprecated { previous_ident, .. } => chain - .insert(version.inner, ItemStatus::NoChange(previous_ident.clone())), - ItemStatus::NoChange(ident) => { - chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) - } - ItemStatus::NotPresent => unreachable!(), - }, - (Some(status), None) => { - let ident = match status { - ItemStatus::Added { ident, .. } => ident, - ItemStatus::Renamed { to, .. } => to, - ItemStatus::Deprecated { ident, .. } => ident, - ItemStatus::NoChange(ident) => ident, - ItemStatus::NotPresent => unreachable!(), - }; - - chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) - } - (Some(status), Some(_)) => { - let ident = match status { - ItemStatus::Added { ident, .. } => ident, - ItemStatus::Renamed { to, .. } => to, - ItemStatus::NoChange(ident) => ident, - _ => unreachable!(), - }; - - chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) - } - _ => unreachable!(), - }; - } - } - } - - /// Generates tokens for the use inside the container definition, e.g. - /// a struct field or an enum variant. - fn generate_for_container(&self, container_version: &ContainerVersion) -> Option; - - /// Generates tokens for the use inside [`From`] implementations for - /// conversion between versions. - fn generate_for_from_impl( - &self, - version: &ContainerVersion, - next_version: &ContainerVersion, - from_ident: &Ident, - ) -> TokenStream; - - /// Returns the ident of the [`Item`] for a specific [`ContainerVersion`]. - fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident>; - - fn chain(&mut self) -> Option<&mut VersionChain>; -} - -pub(crate) trait GetIdent { - fn ident(&self) -> Option<&Ident>; -} - -impl GetIdent for Field { - fn ident(&self) -> Option<&Ident> { - self.ident.as_ref() - } -} - -impl GetIdent for Variant { - fn ident(&self) -> Option<&Ident> { - Some(&self.ident) - } -} - #[derive(Debug)] pub(crate) enum ItemStatus { Added { diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index 9a13e9713..c2788eb46 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -9,9 +9,7 @@ use syn::{DataEnum, Error, Ident}; use crate::{ attrs::{common::ContainerAttributes, variant::VariantAttributes}, gen::{ - common::{ - format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, - }, + common::{format_container_from_ident, Container, ContainerVersion, VersionedContainer}, venum::variant::VersionedVariant, }, }; @@ -122,6 +120,11 @@ impl VersionedEnum { } }); + // Generate the From impl between this `version` and the next one. + if !self.skip_from && !version.skip_from { + token_stream.extend(self.generate_from_impl(version, next_version)); + } + token_stream } @@ -134,4 +137,47 @@ impl VersionedEnum { token_stream } + + fn generate_from_impl( + &self, + version: &ContainerVersion, + next_version: Option<&ContainerVersion>, + ) -> TokenStream { + if let Some(next_version) = next_version { + let next_module_name = &next_version.ident; + let module_name = &version.ident; + + let from_ident = &self.from_ident; + let enum_ident = &self.ident; + + let mut variants = TokenStream::new(); + + for item in &self.items { + variants.extend(item.generate_for_from_impl( + module_name, + next_module_name, + version, + next_version, + enum_ident, + from_ident, + )) + } + + // TODO (@Techassi): Be a little bit more clever about when to include + // the #[allow(deprecated)] attribute. + return quote! { + #[automatically_derived] + #[allow(deprecated)] + impl From<#module_name::#enum_ident> for #next_module_name::#enum_ident { + fn from(#from_ident: #module_name::#enum_ident) -> Self { + match #from_ident { + #variants + } + } + } + }; + } + + quote! {} + } } diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs index 71d434c32..318c66774 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -2,12 +2,13 @@ use std::{collections::BTreeMap, ops::Deref}; use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::Variant; +use syn::{Ident, Variant}; use crate::{ attrs::variant::VariantAttributes, - gen::common::{ - remove_deprecated_variant_prefix, ContainerVersion, Item, ItemStatus, VersionChain, + gen::{ + common::{remove_deprecated_variant_prefix, ContainerVersion, ItemStatus, VersionChain}, + neighbors::Neighbors, }, }; @@ -21,8 +22,8 @@ pub(crate) struct VersionedVariant { // once for both a versioned field and variant, because the are practically // identical. -impl Item for VersionedVariant { - fn new(variant: Variant, variant_attrs: VariantAttributes) -> Self { +impl VersionedVariant { + pub(crate) fn new(variant: Variant, variant_attrs: VariantAttributes) -> Self { // NOTE (@Techassi): This is straight up copied from the VersionedField // impl. As mentioned above, unify this. if let Some(deprecated) = variant_attrs.common.deprecated { @@ -128,7 +129,68 @@ impl Item for VersionedVariant { } } - fn generate_for_container(&self, container_version: &ContainerVersion) -> Option { + /// Inserts container versions not yet present in the status chain. + /// + /// When initially creating a new versioned item, the code doesn't have + /// access to the versions defined on the container. This function inserts + /// all non-present container versions and decides which status and ident + /// is the right fit based on the status neighbors. + /// + /// This continuous chain ensures that when generating code (tokens), each + /// field can lookup the status (and ident) for a requested version. + pub(crate) fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { + if let Some(chain) = &mut self.chain { + for version in versions { + if chain.contains_key(&version.inner) { + continue; + } + + match chain.get_neighbors(&version.inner) { + (None, Some(status)) => match status { + ItemStatus::Added { .. } => { + chain.insert(version.inner, ItemStatus::NotPresent) + } + ItemStatus::Renamed { from, .. } => { + chain.insert(version.inner, ItemStatus::NoChange(from.clone())) + } + ItemStatus::Deprecated { previous_ident, .. } => chain + .insert(version.inner, ItemStatus::NoChange(previous_ident.clone())), + ItemStatus::NoChange(ident) => { + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + ItemStatus::NotPresent => unreachable!(), + }, + (Some(status), None) => { + let ident = match status { + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::Deprecated { ident, .. } => ident, + ItemStatus::NoChange(ident) => ident, + ItemStatus::NotPresent => unreachable!(), + }; + + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + (Some(status), Some(_)) => { + let ident = match status { + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::NoChange(ident) => ident, + _ => unreachable!(), + }; + + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + _ => unreachable!(), + }; + } + } + } + + pub(crate) fn generate_for_container( + &self, + container_version: &ContainerVersion, + ) -> Option { match &self.chain { Some(chain) => match chain .get(&container_version.inner) @@ -162,16 +224,30 @@ impl Item for VersionedVariant { } } - fn generate_for_from_impl( + pub(crate) fn generate_for_from_impl( &self, + module_name: &Ident, + next_module_name: &Ident, version: &ContainerVersion, next_version: &ContainerVersion, + enum_ident: &Ident, from_ident: &syn::Ident, ) -> TokenStream { - todo!() + match &self.chain { + Some(_) => quote! { + _ => todo!(), + }, + None => { + let variant_ident = &self.inner.ident; + + quote! { + #module_name::#enum_ident::#variant_ident => #next_module_name::#enum_ident::#variant_ident, + } + } + } } - fn get_ident(&self, version: &ContainerVersion) -> Option<&syn::Ident> { + pub(crate) fn get_ident(&self, version: &ContainerVersion) -> Option<&syn::Ident> { match &self.chain { Some(chain) => chain .get(&version.inner) @@ -180,8 +256,4 @@ impl Item for VersionedVariant { None => Some(&self.inner.ident), } } - - fn chain(&mut self) -> Option<&mut VersionChain> { - self.chain.as_mut() - } } diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index 9fe885aa7..9298c75ec 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -6,8 +6,9 @@ use syn::{Field, Ident}; use crate::{ attrs::field::FieldAttributes, - gen::common::{ - remove_deprecated_field_prefix, ContainerVersion, Item, ItemStatus, VersionChain, + gen::{ + common::{remove_deprecated_field_prefix, ContainerVersion, ItemStatus, VersionChain}, + neighbors::Neighbors, }, }; @@ -23,8 +24,12 @@ pub(crate) struct VersionedField { pub(crate) inner: Field, } -impl Item for VersionedField { - fn new(field: syn::Field, field_attrs: FieldAttributes) -> Self { +// TODO (@Techassi): Figure out a way to be able to only write the following code +// once for both a versioned field and variant, because the are practically +// identical. + +impl VersionedField { + pub(crate) fn new(field: syn::Field, field_attrs: FieldAttributes) -> Self { // Constructing the action chain requires going through the actions from // the end, because the base struct always represents the latest (most // up-to-date) version of that struct. That's why the following code @@ -148,7 +153,78 @@ impl Item for VersionedField { } } - fn generate_for_container(&self, container_version: &ContainerVersion) -> Option { + /// Inserts container versions not yet present in the status chain. + /// + /// When initially creating a new versioned item, the code doesn't have + /// access to the versions defined on the container. This function inserts + /// all non-present container versions and decides which status and ident + /// is the right fit based on the status neighbors. + /// + /// This continuous chain ensures that when generating code (tokens), each + /// field can lookup the status (and ident) for a requested version. + pub(crate) fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { + if let Some(chain) = &mut self.chain { + for version in versions { + if chain.contains_key(&version.inner) { + continue; + } + + match chain.get_neighbors(&version.inner) { + (None, Some(status)) => match status { + ItemStatus::Added { .. } => { + chain.insert(version.inner, ItemStatus::NotPresent) + } + ItemStatus::Renamed { from, .. } => { + chain.insert(version.inner, ItemStatus::NoChange(from.clone())) + } + ItemStatus::Deprecated { previous_ident, .. } => chain + .insert(version.inner, ItemStatus::NoChange(previous_ident.clone())), + ItemStatus::NoChange(ident) => { + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + ItemStatus::NotPresent => unreachable!(), + }, + (Some(status), None) => { + let ident = match status { + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::Deprecated { ident, .. } => ident, + ItemStatus::NoChange(ident) => ident, + ItemStatus::NotPresent => unreachable!(), + }; + + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + (Some(status), Some(_)) => { + let ident = match status { + ItemStatus::Added { ident, .. } => ident, + ItemStatus::Renamed { to, .. } => to, + ItemStatus::NoChange(ident) => ident, + _ => unreachable!(), + }; + + chain.insert(version.inner, ItemStatus::NoChange(ident.clone())) + } + _ => unreachable!(), + }; + } + } + } + + pub(crate) fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident> { + match &self.chain { + Some(chain) => chain + .get(&version.inner) + .expect("internal error: chain must contain container version") + .get_ident(), + None => self.inner.ident.as_ref(), + } + } + + pub(crate) fn generate_for_container( + &self, + container_version: &ContainerVersion, + ) -> Option { match &self.chain { Some(chain) => { // Check if the provided container version is present in the map @@ -199,7 +275,7 @@ impl Item for VersionedField { } } - fn generate_for_from_impl( + pub(crate) fn generate_for_from_impl( &self, version: &ContainerVersion, next_version: &ContainerVersion, @@ -241,18 +317,4 @@ impl Item for VersionedField { } } } - - fn get_ident(&self, version: &ContainerVersion) -> Option<&Ident> { - match &self.chain { - Some(chain) => chain - .get(&version.inner) - .expect("internal error: chain must contain container version") - .get_ident(), - None => self.inner.ident.as_ref(), - } - } - - fn chain(&mut self) -> Option<&mut VersionChain> { - self.chain.as_mut() - } } diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index 2daf41f57..b32d2cefe 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -9,9 +9,7 @@ use syn::{DataStruct, Error, Ident}; use crate::{ attrs::{common::ContainerAttributes, field::FieldAttributes}, gen::{ - common::{ - format_container_from_ident, Container, ContainerVersion, Item, VersionedContainer, - }, + common::{format_container_from_ident, Container, ContainerVersion, VersionedContainer}, vstruct::field::VersionedField, }, }; @@ -151,9 +149,10 @@ impl VersionedStruct { ) -> TokenStream { if let Some(next_version) = next_version { let next_module_name = &next_version.ident; - let from_ident = &self.from_ident; let module_name = &version.ident; - let struct_name = &self.ident; + + let from_ident = &self.from_ident; + let struct_ident = &self.ident; let fields = self.generate_from_fields(version, next_version, from_ident); @@ -162,8 +161,8 @@ impl VersionedStruct { return quote! { #[automatically_derived] #[allow(deprecated)] - impl From<#module_name::#struct_name> for #next_module_name::#struct_name { - fn from(#from_ident: #module_name::#struct_name) -> Self { + impl From<#module_name::#struct_ident> for #next_module_name::#struct_ident { + fn from(#from_ident: #module_name::#struct_ident) -> Self { Self { #fields } diff --git a/crates/stackable-versioned-macros/tests/enum.rs b/crates/stackable-versioned-macros/tests/enum.rs index 7a147ad5f..3bc3f456b 100644 --- a/crates/stackable-versioned-macros/tests/enum.rs +++ b/crates/stackable-versioned-macros/tests/enum.rs @@ -12,4 +12,22 @@ fn versioned_enum() { DeprecatedBar, Baz, } + + // There is only one variant in v1alpha1, so we can take a shortcut and thus + // don't need a match statement + // impl From for v1beta1::Foo { + // fn from(__sv_value: v1alpha1::Foo) -> Self { + // Self::Baz + // } + // } + + // We need to match, to do the proper conversion + // impl From for v1::Foo { + // fn from(__sv_value: v1beta1::Foo) -> Self { + // match __sv_value { + // v1beta1::Foo::Bar => Self::DeprecatedBar, + // v1beta1::Foo::Baz => Self::Baz, + // } + // } + // } } From 169d7161a72e8b50aa814afea5c26005463acf2c Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Jun 2024 10:15:39 +0200 Subject: [PATCH 12/18] Finish basic From impls for enums --- .../src/gen/{neighbors.rs => chain.rs} | 20 ++++++++++++++++ .../stackable-versioned-macros/src/gen/mod.rs | 2 +- .../src/gen/venum/mod.rs | 1 - .../src/gen/venum/variant.rs | 24 +++++++++++++++---- .../src/gen/vstruct/field.rs | 4 ++-- .../stackable-versioned-macros/tests/enum.rs | 21 ++++------------ 6 files changed, 47 insertions(+), 25 deletions(-) rename crates/stackable-versioned-macros/src/gen/{neighbors.rs => chain.rs} (85%) diff --git a/crates/stackable-versioned-macros/src/gen/neighbors.rs b/crates/stackable-versioned-macros/src/gen/chain.rs similarity index 85% rename from crates/stackable-versioned-macros/src/gen/neighbors.rs rename to crates/stackable-versioned-macros/src/gen/chain.rs index c37955a39..47675bcb6 100644 --- a/crates/stackable-versioned-macros/src/gen/neighbors.rs +++ b/crates/stackable-versioned-macros/src/gen/chain.rs @@ -51,6 +51,26 @@ where } } +pub(crate) trait BTreeMapExt +where + K: Ord, +{ + const MESSAGE: &'static str; + + fn get_expect(&self, key: &K) -> &V; +} + +impl BTreeMapExt for BTreeMap +where + K: Ord, +{ + const MESSAGE: &'static str = "internal error: chain must contain version"; + + fn get_expect(&self, key: &K) -> &V { + self.get(key).expect(Self::MESSAGE) + } +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/stackable-versioned-macros/src/gen/mod.rs b/crates/stackable-versioned-macros/src/gen/mod.rs index 66f2797d1..ea069d445 100644 --- a/crates/stackable-versioned-macros/src/gen/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/mod.rs @@ -6,8 +6,8 @@ use crate::{ gen::{common::Container, venum::VersionedEnum, vstruct::VersionedStruct}, }; +pub(crate) mod chain; pub(crate) mod common; -pub(crate) mod neighbors; pub(crate) mod venum; pub(crate) mod vstruct; diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index c2788eb46..034a68e53 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -159,7 +159,6 @@ impl VersionedEnum { version, next_version, enum_ident, - from_ident, )) } diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs index 318c66774..472df1e50 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -7,8 +7,8 @@ use syn::{Ident, Variant}; use crate::{ attrs::variant::VariantAttributes, gen::{ + chain::{BTreeMapExt, Neighbors}, common::{remove_deprecated_variant_prefix, ContainerVersion, ItemStatus, VersionChain}, - neighbors::Neighbors, }, }; @@ -32,7 +32,7 @@ impl VersionedVariant { // When the field is deprecated, any rename which occurred beforehand // requires access to the field ident to infer the field ident for // the latest rename. - let mut ident = remove_deprecated_variant_prefix(&deprecated_ident); + let mut ident = remove_deprecated_variant_prefix(deprecated_ident); let mut actions = BTreeMap::new(); actions.insert( @@ -231,11 +231,25 @@ impl VersionedVariant { version: &ContainerVersion, next_version: &ContainerVersion, enum_ident: &Ident, - from_ident: &syn::Ident, ) -> TokenStream { match &self.chain { - Some(_) => quote! { - _ => todo!(), + Some(chain) => match ( + chain.get_expect(&version.inner), + chain.get_expect(&next_version.inner), + ) { + (_, ItemStatus::Added { .. }) => quote! {}, + (old, next) => { + let old_variant_ident = old + .get_ident() + .expect("internal error: old variant must have a name"); + let next_variant_ident = next + .get_ident() + .expect("internal error: next variant must have a name"); + + quote! { + #module_name::#enum_ident::#old_variant_ident => #next_module_name::#enum_ident::#next_variant_ident, + } + } }, None => { let variant_ident = &self.inner.ident; diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index 9298c75ec..44b32478f 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -7,8 +7,8 @@ use syn::{Field, Ident}; use crate::{ attrs::field::FieldAttributes, gen::{ + chain::Neighbors, common::{remove_deprecated_field_prefix, ContainerVersion, ItemStatus, VersionChain}, - neighbors::Neighbors, }, }; @@ -50,7 +50,7 @@ impl VersionedField { // When the field is deprecated, any rename which occurred beforehand // requires access to the field ident to infer the field ident for // the latest rename. - let mut ident = remove_deprecated_field_prefix(&deprecated_ident); + let mut ident = remove_deprecated_field_prefix(deprecated_ident); let mut actions = BTreeMap::new(); actions.insert( diff --git a/crates/stackable-versioned-macros/tests/enum.rs b/crates/stackable-versioned-macros/tests/enum.rs index 3bc3f456b..db5c3d93f 100644 --- a/crates/stackable-versioned-macros/tests/enum.rs +++ b/crates/stackable-versioned-macros/tests/enum.rs @@ -13,21 +13,10 @@ fn versioned_enum() { Baz, } - // There is only one variant in v1alpha1, so we can take a shortcut and thus - // don't need a match statement - // impl From for v1beta1::Foo { - // fn from(__sv_value: v1alpha1::Foo) -> Self { - // Self::Baz - // } - // } + let v1alpha1_foo = v1alpha1::Foo::Baz; + let v1beta1_foo = v1beta1::Foo::from(v1alpha1_foo); + let v1_foo = v1::Foo::from(v1beta1_foo); - // We need to match, to do the proper conversion - // impl From for v1::Foo { - // fn from(__sv_value: v1beta1::Foo) -> Self { - // match __sv_value { - // v1beta1::Foo::Bar => Self::DeprecatedBar, - // v1beta1::Foo::Baz => Self::Baz, - // } - // } - // } + // TODO (@Techassi): Forward derive PartialEq + assert!(matches!(v1_foo, v1::Foo::Baz)) } From 69e497794a29161353e48dd259b9435345824a2e Mon Sep 17 00:00:00 2001 From: Techassi Date: Mon, 1 Jul 2024 11:09:15 +0200 Subject: [PATCH 13/18] Apply suggestions Co-authored-by: Nick --- .../src/attrs/common/item.rs | 13 +++++++++++++ crates/stackable-versioned-macros/src/gen/common.rs | 2 +- .../stackable-versioned-macros/src/gen/venum/mod.rs | 10 +++++----- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/stackable-versioned-macros/src/attrs/common/item.rs b/crates/stackable-versioned-macros/src/attrs/common/item.rs index 94be3e1a1..15b2885da 100644 --- a/crates/stackable-versioned-macros/src/attrs/common/item.rs +++ b/crates/stackable-versioned-macros/src/attrs/common/item.rs @@ -43,6 +43,11 @@ impl ItemAttributes { } } +/// For the added() action +/// +/// Example usage: +/// - `added(since = "...")` +/// - `added(since = "...", default_fn = "custom_fn")` #[derive(Clone, Debug, FromMeta)] pub(crate) struct AddedAttributes { pub(crate) since: SpannedValue, @@ -58,12 +63,20 @@ fn default_default_fn() -> SpannedValue { ) } +/// For the renamed() action +/// +/// Example usage: +/// - `renamed(since = "...", from = "...")` #[derive(Clone, Debug, FromMeta)] pub(crate) struct RenamedAttributes { pub(crate) since: SpannedValue, pub(crate) from: SpannedValue, } +/// For the deprecated() action +/// +/// Example usage: +/// - `deprecated(since = "...", note = "...")` #[derive(Clone, Debug, FromMeta)] pub(crate) struct DeprecatedAttributes { pub(crate) since: SpannedValue, diff --git a/crates/stackable-versioned-macros/src/gen/common.rs b/crates/stackable-versioned-macros/src/gen/common.rs index 6d6b37a75..298b85c11 100644 --- a/crates/stackable-versioned-macros/src/gen/common.rs +++ b/crates/stackable-versioned-macros/src/gen/common.rs @@ -45,7 +45,7 @@ where /// /// Internally, it will create a module for each declared version which /// contains the container with the appropriate items (fields or variants) - /// Additionally, it generates `From` implementations, which enable + /// Additionally, it generates `From` implementations, which enable /// conversion from an older to a newer version. fn generate_tokens(&self) -> TokenStream; } diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index 034a68e53..70b0d6596 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -32,8 +32,8 @@ impl Container for VersionedEnum { // Convert the raw version attributes into a container version. let versions: Vec<_> = (&attributes).into(); - // Extract the field attributes for every field from the raw token - // stream and also validate that each field action version uses a + // Extract the attributes for every variant from the raw token + // stream and also validate that each variant action version uses a // version declared by the container attribute. let mut items = Vec::new(); @@ -48,12 +48,12 @@ impl Container for VersionedEnum { // Check for field ident collisions for version in &versions { - // Collect the idents of all fields for a single version and then + // Collect the idents of all variants for a single version and then // ensure that all idents are unique. If they are not, return an // error. - // TODO (@Techassi): Report which field(s) use a duplicate ident and - // also hint what can be done to fix it based on the field action / + // TODO (@Techassi): Report which variant(s) use a duplicate ident and + // also hint what can be done to fix it based on the variant action / // status. if !items.iter().map(|f| f.get_ident(version)).all_unique() { From 68b28ec58feb8e33b64b193ae59c7f3651240157 Mon Sep 17 00:00:00 2001 From: Techassi Date: Tue, 2 Jul 2024 10:50:47 +0200 Subject: [PATCH 14/18] Apply more suggestions Co-authored-by: Nick --- crates/stackable-versioned-macros/src/gen/venum/variant.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs index 472df1e50..05306724e 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -29,8 +29,8 @@ impl VersionedVariant { if let Some(deprecated) = variant_attrs.common.deprecated { let deprecated_ident = &variant.ident; - // When the field is deprecated, any rename which occurred beforehand - // requires access to the field ident to infer the field ident for + // When the variant is deprecated, any rename which occurred beforehand + // requires access to the variant ident to infer the variant ident for // the latest rename. let mut ident = remove_deprecated_variant_prefix(deprecated_ident); let mut actions = BTreeMap::new(); @@ -137,7 +137,7 @@ impl VersionedVariant { /// is the right fit based on the status neighbors. /// /// This continuous chain ensures that when generating code (tokens), each - /// field can lookup the status (and ident) for a requested version. + /// variant can lookup the status (and ident) for a requested version. pub(crate) fn insert_container_versions(&mut self, versions: &[ContainerVersion]) { if let Some(chain) = &mut self.chain { for version in versions { From 758e88597ce3061847758cd1633a8134838385b0 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 5 Jul 2024 10:04:24 +0200 Subject: [PATCH 15/18] Rename starts_with variable to starts_with_deprecated --- crates/stackable-versioned-macros/src/attrs/field.rs | 6 +++--- crates/stackable-versioned-macros/src/attrs/variant.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index 477f17e37..e17615f5c 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -168,20 +168,20 @@ impl FieldAttributes { /// in their name. The prefix must not be included for fields which are /// not deprecated. fn validate_field_name(&self) -> Result<(), Error> { - let starts_with = self + let starts_with_deprecated = self .ident .as_ref() .expect("internal error: to be validated fields must have a name") .to_string() .starts_with(DEPRECATED_FIELD_PREFIX); - if self.common.deprecated.is_some() && !starts_with { + if self.common.deprecated.is_some() && !starts_with_deprecated { return Err(Error::custom( "field was marked as `deprecated` and thus must include the `deprecated_` prefix in its name" ).with_span(&self.ident)); } - if self.common.deprecated.is_none() && starts_with { + if self.common.deprecated.is_none() && starts_with_deprecated { return Err(Error::custom( "field includes the `deprecated_` prefix in its name but is not marked as `deprecated`" ).with_span(&self.ident)); diff --git a/crates/stackable-versioned-macros/src/attrs/variant.rs b/crates/stackable-versioned-macros/src/attrs/variant.rs index f34cb1271..fd72ca6e8 100644 --- a/crates/stackable-versioned-macros/src/attrs/variant.rs +++ b/crates/stackable-versioned-macros/src/attrs/variant.rs @@ -161,18 +161,18 @@ impl VariantAttributes { return Err(Error::custom("renamed variants must use PascalCase")); } - let starts_with = self + let starts_with_deprecated = self .ident .to_string() .starts_with(DEPRECATED_VARIANT_PREFIX); - if self.common.deprecated.is_some() && !starts_with { + if self.common.deprecated.is_some() && !starts_with_deprecated { return Err(Error::custom( "variant was marked as `deprecated` and thus must include the `Deprecated` prefix in its name" ).with_span(&self.ident)); } - if self.common.deprecated.is_none() && starts_with { + if self.common.deprecated.is_none() && starts_with_deprecated { return Err(Error::custom( "variant includes the `Deprecated` prefix in its name but is not marked as `deprecated`" ).with_span(&self.ident)); From 028fc3c765a9f947e6ad5bced244fe890680f87a Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 5 Jul 2024 10:12:25 +0200 Subject: [PATCH 16/18] Remove old todo comment --- crates/stackable-versioned-macros/src/attrs/field.rs | 3 --- crates/stackable-versioned-macros/src/attrs/variant.rs | 3 --- 2 files changed, 6 deletions(-) diff --git a/crates/stackable-versioned-macros/src/attrs/field.rs b/crates/stackable-versioned-macros/src/attrs/field.rs index e17615f5c..94851a8d1 100644 --- a/crates/stackable-versioned-macros/src/attrs/field.rs +++ b/crates/stackable-versioned-macros/src/attrs/field.rs @@ -62,9 +62,6 @@ impl FieldAttributes { errors.handle(self.validate_action_order()); errors.handle(self.validate_field_name()); - // TODO (@Techassi): Add validation for renames so that renamed fields - // match up and form a continuous chain (eg. foo -> bar -> baz). - // TODO (@Techassi): Add hint if a field is added in the first version // that it might be clever to remove the 'added' attribute. diff --git a/crates/stackable-versioned-macros/src/attrs/variant.rs b/crates/stackable-versioned-macros/src/attrs/variant.rs index fd72ca6e8..75f3ebd59 100644 --- a/crates/stackable-versioned-macros/src/attrs/variant.rs +++ b/crates/stackable-versioned-macros/src/attrs/variant.rs @@ -46,9 +46,6 @@ impl VariantAttributes { errors.handle(self.validate_action_order()); errors.handle(self.validate_variant_name()); - // TODO (@Techassi): Add validation for renames so that renamed items - // match up and form a continuous chain (eg. foo -> bar -> baz). - // TODO (@Techassi): Add hint if a item is added in the first version // that it might be clever to remove the 'added' attribute. From 7cf73962187e193794ca6b1d9b2ae6028f8c7b2d Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 5 Jul 2024 10:37:51 +0200 Subject: [PATCH 17/18] Add auto-generated notes for deprecated versions --- crates/stackable-versioned-macros/src/gen/venum/mod.rs | 10 +++++++--- .../stackable-versioned-macros/src/gen/vstruct/mod.rs | 10 +++++++--- crates/stackable-versioned-macros/tests/basic.rs | 3 ++- crates/stackable-versioned-macros/tests/enum.rs | 3 ++- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index 70b0d6596..34dd18d6b 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -106,14 +106,18 @@ impl VersionedEnum { // enable the attribute macro to be applied to a module which // generates versioned versions of all contained containers. - let deprecated_attr = version.deprecated.then_some(quote! {#[deprecated]}); - let module_name = &version.ident; + let version_ident = &version.ident; + + let deprecated_note = format!("Version {version} is deprecated", version = version_ident); + let deprecated_attr = version + .deprecated + .then_some(quote! {#[deprecated = #deprecated_note]}); // Generate tokens for the module and the contained struct token_stream.extend(quote! { #[automatically_derived] #deprecated_attr - pub mod #module_name { + pub mod #version_ident { pub enum #enum_name { #variants } diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index b32d2cefe..8a913f503 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -110,14 +110,18 @@ impl VersionedStruct { // enable the attribute macro to be applied to a module which // generates versioned versions of all contained containers. - let deprecated_attr = version.deprecated.then_some(quote! {#[deprecated]}); - let module_name = &version.ident; + let version_ident = &version.ident; + + let deprecated_note = format!("Version {version} is deprecated", version = version_ident); + let deprecated_attr = version + .deprecated + .then_some(quote! {#[deprecated = #deprecated_note]}); // Generate tokens for the module and the contained struct token_stream.extend(quote! { #[automatically_derived] #deprecated_attr - pub mod #module_name { + pub mod #version_ident { pub struct #struct_name { #fields } diff --git a/crates/stackable-versioned-macros/tests/basic.rs b/crates/stackable-versioned-macros/tests/basic.rs index a6f2027aa..ce46833c6 100644 --- a/crates/stackable-versioned-macros/tests/basic.rs +++ b/crates/stackable-versioned-macros/tests/basic.rs @@ -5,7 +5,7 @@ use stackable_versioned_macros::versioned; // run `cargo expand --test basic --all-features`. #[allow(dead_code)] #[versioned( - version(name = "v1alpha1"), + version(name = "v1alpha1", deprecated), version(name = "v1beta1"), version(name = "v1"), version(name = "v2"), @@ -24,6 +24,7 @@ struct Foo { #[test] fn basic() { + #[allow(deprecated)] let _ = v1alpha1::Foo { jjj: 0, baz: false }; let _ = v1beta1::Foo { bar: 0, baz: false }; let _ = v1::Foo { bar: 0, baz: false }; diff --git a/crates/stackable-versioned-macros/tests/enum.rs b/crates/stackable-versioned-macros/tests/enum.rs index db5c3d93f..c43b1da1e 100644 --- a/crates/stackable-versioned-macros/tests/enum.rs +++ b/crates/stackable-versioned-macros/tests/enum.rs @@ -4,7 +4,7 @@ use stackable_versioned_macros::versioned; fn versioned_enum() { #[versioned( version(name = "v1alpha1"), - version(name = "v1beta1"), + version(name = "v1beta1", deprecated), version(name = "v1") )] pub enum Foo { @@ -14,6 +14,7 @@ fn versioned_enum() { } let v1alpha1_foo = v1alpha1::Foo::Baz; + #[allow(deprecated)] let v1beta1_foo = v1beta1::Foo::from(v1alpha1_foo); let v1_foo = v1::Foo::from(v1beta1_foo); From de1cde3e50e2f497c57332a8891c2a23bc6c37b9 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 5 Jul 2024 11:43:31 +0200 Subject: [PATCH 18/18] Move attribute parsing into new() functions --- .../src/gen/venum/mod.rs | 8 ++---- .../src/gen/venum/variant.rs | 28 ++++++++++++------- .../src/gen/vstruct/field.rs | 27 +++++++++++------- .../src/gen/vstruct/mod.rs | 8 ++---- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/crates/stackable-versioned-macros/src/gen/venum/mod.rs b/crates/stackable-versioned-macros/src/gen/venum/mod.rs index 34dd18d6b..3d417ecef 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/mod.rs @@ -1,13 +1,12 @@ use std::ops::Deref; -use darling::FromVariant; use itertools::Itertools; use proc_macro2::TokenStream; use quote::quote; use syn::{DataEnum, Error, Ident}; use crate::{ - attrs::{common::ContainerAttributes, variant::VariantAttributes}, + attrs::common::ContainerAttributes, gen::{ common::{format_container_from_ident, Container, ContainerVersion, VersionedContainer}, venum::variant::VersionedVariant, @@ -38,10 +37,7 @@ impl Container for VersionedEnum { let mut items = Vec::new(); for variant in data.variants { - let attrs = VariantAttributes::from_variant(&variant)?; - attrs.validate_versions(&attributes, &variant)?; - - let mut versioned_field = VersionedVariant::new(variant, attrs); + let mut versioned_field = VersionedVariant::new(variant, &attributes)?; versioned_field.insert_container_versions(&versions); items.push(versioned_field); } diff --git a/crates/stackable-versioned-macros/src/gen/venum/variant.rs b/crates/stackable-versioned-macros/src/gen/venum/variant.rs index 05306724e..d06c29651 100644 --- a/crates/stackable-versioned-macros/src/gen/venum/variant.rs +++ b/crates/stackable-versioned-macros/src/gen/venum/variant.rs @@ -1,11 +1,12 @@ use std::{collections::BTreeMap, ops::Deref}; +use darling::FromVariant; use proc_macro2::TokenStream; use quote::{format_ident, quote}; use syn::{Ident, Variant}; use crate::{ - attrs::variant::VariantAttributes, + attrs::{common::ContainerAttributes, variant::VariantAttributes}, gen::{ chain::{BTreeMapExt, Neighbors}, common::{remove_deprecated_variant_prefix, ContainerVersion, ItemStatus, VersionChain}, @@ -23,9 +24,16 @@ pub(crate) struct VersionedVariant { // identical. impl VersionedVariant { - pub(crate) fn new(variant: Variant, variant_attrs: VariantAttributes) -> Self { + pub(crate) fn new( + variant: Variant, + container_attrs: &ContainerAttributes, + ) -> syn::Result { // NOTE (@Techassi): This is straight up copied from the VersionedField // impl. As mentioned above, unify this. + + let variant_attrs = VariantAttributes::from_variant(&variant)?; + variant_attrs.validate_versions(container_attrs, &variant)?; + if let Some(deprecated) = variant_attrs.common.deprecated { let deprecated_ident = &variant.ident; @@ -68,10 +76,10 @@ impl VersionedVariant { ); } - Self { + Ok(Self { chain: Some(actions), inner: variant, - } + }) } else if !variant_attrs.common.renames.is_empty() { let mut actions = BTreeMap::new(); let mut ident = variant.ident.clone(); @@ -100,10 +108,10 @@ impl VersionedVariant { ); } - Self { + Ok(Self { chain: Some(actions), inner: variant, - } + }) } else { if let Some(added) = variant_attrs.common.added { let mut actions = BTreeMap::new(); @@ -116,16 +124,16 @@ impl VersionedVariant { }, ); - return Self { + return Ok(Self { chain: Some(actions), inner: variant, - }; + }); } - Self { + Ok(Self { chain: None, inner: variant, - } + }) } } diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs index 44b32478f..c3b77a7e7 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/field.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/field.rs @@ -1,11 +1,12 @@ use std::{collections::BTreeMap, ops::Deref}; +use darling::FromField; use proc_macro2::TokenStream; use quote::{format_ident, quote}; use syn::{Field, Ident}; use crate::{ - attrs::field::FieldAttributes, + attrs::{common::ContainerAttributes, field::FieldAttributes}, gen::{ chain::Neighbors, common::{remove_deprecated_field_prefix, ContainerVersion, ItemStatus, VersionChain}, @@ -29,7 +30,13 @@ pub(crate) struct VersionedField { // identical. impl VersionedField { - pub(crate) fn new(field: syn::Field, field_attrs: FieldAttributes) -> Self { + /// Create a new versioned field (of a versioned struct) by consuming the + /// parsed [Field] and validating the versions of field actions against + /// versions attached on the container. + pub(crate) fn new(field: Field, container_attrs: &ContainerAttributes) -> syn::Result { + let field_attrs = FieldAttributes::from_field(&field)?; + field_attrs.validate_versions(container_attrs, &field)?; + // Constructing the action chain requires going through the actions from // the end, because the base struct always represents the latest (most // up-to-date) version of that struct. That's why the following code @@ -86,10 +93,10 @@ impl VersionedField { ); } - Self { + Ok(Self { chain: Some(actions), inner: field, - } + }) } else if !field_attrs.common.renames.is_empty() { let mut actions = BTreeMap::new(); let mut ident = field @@ -121,10 +128,10 @@ impl VersionedField { ); } - Self { + Ok(Self { chain: Some(actions), inner: field, - } + }) } else { if let Some(added) = field_attrs.common.added { let mut actions = BTreeMap::new(); @@ -140,16 +147,16 @@ impl VersionedField { }, ); - return Self { + return Ok(Self { chain: Some(actions), inner: field, - }; + }); } - Self { + Ok(Self { chain: None, inner: field, - } + }) } } diff --git a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs index 8a913f503..6bbf6b98e 100644 --- a/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs +++ b/crates/stackable-versioned-macros/src/gen/vstruct/mod.rs @@ -1,13 +1,12 @@ use std::ops::Deref; -use darling::FromField; use itertools::Itertools; use proc_macro2::TokenStream; use quote::quote; use syn::{DataStruct, Error, Ident}; use crate::{ - attrs::{common::ContainerAttributes, field::FieldAttributes}, + attrs::common::ContainerAttributes, gen::{ common::{format_container_from_ident, Container, ContainerVersion, VersionedContainer}, vstruct::field::VersionedField, @@ -42,10 +41,7 @@ impl Container for VersionedStruct { let mut items = Vec::new(); for field in data.fields { - let attrs = FieldAttributes::from_field(&field)?; - attrs.validate_versions(&attributes, &field)?; - - let mut versioned_field = VersionedField::new(field, attrs); + let mut versioned_field = VersionedField::new(field, &attributes)?; versioned_field.insert_container_versions(&versions); items.push(versioned_field); }