From 5edd85190f0451246cdf18a611487c93603b9f1a Mon Sep 17 00:00:00 2001 From: Techassi Date: Thu, 27 Feb 2025 16:49:42 +0100 Subject: [PATCH 1/8] chore: Add changelog entry --- 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 0060a3a6b..3f59adc9a 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. ### Added +- Add support for re-emitting and merging modules defined in versioned modules ([#xxx]). - Add basic support for generic types in struct and enum definitions ([#969]). ### Changed @@ -14,6 +15,7 @@ All notable changes to this project will be documented in this file. [#961]: https://github.com/stackabletech/operator-rs/pull/961 [#969]: https://github.com/stackabletech/operator-rs/pull/969 +[#xxx]: https://github.com/stackabletech/operator-rs/pull/xxx ## [0.5.1] - 2025-02-14 From 98c086835af868d95cc5cffc742244da76ad4082 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Feb 2025 11:13:53 +0100 Subject: [PATCH 2/8] feat: Support to re-emit and merge modules in versioned modules --- .../src/codegen/module.rs | 132 +++++++++++++++--- crates/stackable-versioned-macros/src/lib.rs | 63 +-------- 2 files changed, 114 insertions(+), 81 deletions(-) diff --git a/crates/stackable-versioned-macros/src/codegen/module.rs b/crates/stackable-versioned-macros/src/codegen/module.rs index 4ae950c1b..cb1ef474a 100644 --- a/crates/stackable-versioned-macros/src/codegen/module.rs +++ b/crates/stackable-versioned-macros/src/codegen/module.rs @@ -1,53 +1,114 @@ use std::{collections::HashMap, ops::Not}; -use darling::util::IdentString; +use darling::{util::IdentString, Error, Result}; use proc_macro2::TokenStream; use quote::quote; -use syn::{token::Pub, Ident, Visibility}; +use syn::{token::Pub, Ident, Item, ItemMod, ItemUse, Visibility}; -use crate::codegen::{container::Container, VersionDefinition}; +use crate::{ + codegen::{container::Container, VersionDefinition}, + ModuleAttributes, +}; pub(crate) type KubernetesItems = (Vec, Vec, Vec); -pub(crate) struct ModuleInput { - pub(crate) vis: Visibility, - pub(crate) ident: Ident, -} - /// A versioned module. /// /// Versioned modules allow versioning multiple containers at once without introducing conflicting /// version module definitions. -pub(crate) struct Module { +pub struct Module { versions: Vec, + + // Recognized items of the module containers: Vec, - preserve_module: bool, - skip_from: bool, + submodules: Vec, + ident: IdentString, vis: Visibility, + + // Flags which influence generation + preserve_module: bool, + skip_from: bool, } impl Module { /// Creates a new versioned module containing versioned containers. - pub(crate) fn new( - ModuleInput { ident, vis, .. }: ModuleInput, - preserve_module: bool, - skip_from: bool, - versions: Vec, - containers: Vec, - ) -> Self { - Self { - ident: ident.into(), + pub fn new(item_mod: ItemMod, module_attributes: ModuleAttributes) -> Result { + let Some((_, items)) = item_mod.content else { + return Err( + Error::custom("the macro can only be used on module blocks").with_span(&item_mod) + ); + }; + + let versions: Vec = (&module_attributes).into(); + + let preserve_module = module_attributes + .common + .options + .preserve_module + .is_present(); + + let skip_from = module_attributes + .common + .options + .skip + .as_ref() + .map_or(false, |opts| opts.from.is_present()); + + let mut errors = Error::accumulator(); + let mut containers = Vec::new(); + let mut submodules = Vec::new(); + + for item in items { + match item { + Item::Enum(item_enum) => { + let container = Container::new_enum_nested(item_enum, &versions)?; + containers.push(container); + } + Item::Struct(item_struct) => { + let container = Container::new_struct_nested(item_struct, &versions)?; + containers.push(container); + } + Item::Mod(item_module) => match item_module.content { + Some((_, items)) => { + let imports: Vec = items + .into_iter() + // We are only interested in use statements. Everything else is ignored. + // NOTE (@Techassi): We might want to error instead. + .filter_map(|item| match item { + Item::Use(item_use) => Some(item_use), + _ => None, + }) + .collect(); + + // NOTE (@Techassi): Do we wanna enforce that modules must use version names? + + submodules.push(Submodule { + ident: item_module.ident.into(), + imports, + }); + } + None => errors.push( + Error::custom("submodules must be module blocks").with_span(&item_module), + ), + }, + _ => continue, + }; + } + + errors.finish_with(Self { + ident: item_mod.ident.into(), + vis: item_mod.vis, preserve_module, containers, + submodules, skip_from, versions, - vis, - } + }) } /// Generates tokens for all versioned containers. - pub(crate) fn generate_tokens(&self) -> TokenStream { + pub fn generate_tokens(&self) -> TokenStream { if self.containers.is_empty() { return quote! {}; } @@ -103,6 +164,8 @@ impl Module { } } + let submodule_imports = self.generate_submodule_imports(version); + // Only add #[automatically_derived] here if the user doesn't want to preserve the // module. let automatically_derived = self @@ -122,6 +185,8 @@ impl Module { #version_module_vis mod #version_ident { use super::*; + #submodule_imports + #container_definitions } @@ -163,4 +228,25 @@ impl Module { } } } + + /// Optionally generates imports (which can be re-exports) located in submodules for the + /// specified `version`. + fn generate_submodule_imports(&self, version: &VersionDefinition) -> Option { + for submodule in &self.submodules { + if submodule.ident == version.ident { + let imports = &submodule.imports; + + return Some(quote! { + #(#imports)* + }); + } + } + + None + } +} + +pub struct Submodule { + ident: IdentString, + imports: Vec, } diff --git a/crates/stackable-versioned-macros/src/lib.rs b/crates/stackable-versioned-macros/src/lib.rs index b27f12ce8..4bb199770 100644 --- a/crates/stackable-versioned-macros/src/lib.rs +++ b/crates/stackable-versioned-macros/src/lib.rs @@ -4,11 +4,7 @@ use syn::{spanned::Spanned, Error, Item}; use crate::{ attrs::{container::StandaloneContainerAttributes, module::ModuleAttributes}, - codegen::{ - container::{Container, StandaloneContainer}, - module::{Module, ModuleInput}, - VersionDefinition, - }, + codegen::{container::StandaloneContainer, module::Module}, }; #[cfg(test)] @@ -802,61 +798,12 @@ fn versioned_impl(attrs: proc_macro2::TokenStream, input: Item) -> proc_macro2:: Err(err) => return err.write_errors(), }; - let versions: Vec = (&module_attributes).into(); - let preserve_modules = module_attributes - .common - .options - .preserve_module - .is_present(); - - let skip_from = module_attributes - .common - .options - .skip - .as_ref() - .map_or(false, |opts| opts.from.is_present()); - - let module_span = item_mod.span(); - let module_input = ModuleInput { - ident: item_mod.ident, - vis: item_mod.vis, - }; - - let Some((_, items)) = item_mod.content else { - return Error::new(module_span, "the macro can only be used on module blocks") - .into_compile_error(); + let module = match Module::new(item_mod, module_attributes) { + Ok(module) => module, + Err(err) => return err.write_errors(), }; - let mut containers = Vec::new(); - - for item in items { - let container = match item { - Item::Enum(item_enum) => { - match Container::new_enum_nested(item_enum, &versions) { - Ok(container) => container, - Err(err) => return err.write_errors(), - } - } - Item::Struct(item_struct) => { - match Container::new_struct_nested(item_struct, &versions) { - Ok(container) => container, - Err(err) => return err.write_errors(), - } - } - _ => continue, - }; - - containers.push(container); - } - - Module::new( - module_input, - preserve_modules, - skip_from, - versions, - containers, - ) - .generate_tokens() + module.generate_tokens() } Item::Enum(item_enum) => { let container_attributes: StandaloneContainerAttributes = From 782b339cf0d00e01f20b24a4d356a3788fa27374 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Feb 2025 11:14:43 +0100 Subject: [PATCH 3/8] chore: Update changelog link --- 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 3f59adc9a..72be15bb7 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. ### Added -- Add support for re-emitting and merging modules defined in versioned modules ([#xxx]). +- Add support for re-emitting and merging modules defined in versioned modules ([#971]). - Add basic support for generic types in struct and enum definitions ([#969]). ### Changed @@ -15,7 +15,7 @@ All notable changes to this project will be documented in this file. [#961]: https://github.com/stackabletech/operator-rs/pull/961 [#969]: https://github.com/stackabletech/operator-rs/pull/969 -[#xxx]: https://github.com/stackabletech/operator-rs/pull/xxx +[#971]: https://github.com/stackabletech/operator-rs/pull/971 ## [0.5.1] - 2025-02-14 From 828381ac13326fd8b899c7d64bf114d9437e5059 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Feb 2025 11:46:40 +0100 Subject: [PATCH 4/8] test: Add new submodule snapshot test --- .../fixtures/inputs/default/submodule.rs | 11 ++++++++ ..._test__default_snapshots@submodule.rs.snap | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 crates/stackable-versioned-macros/fixtures/inputs/default/submodule.rs create mode 100644 crates/stackable-versioned-macros/fixtures/snapshots/stackable_versioned_macros__test__default_snapshots@submodule.rs.snap diff --git a/crates/stackable-versioned-macros/fixtures/inputs/default/submodule.rs b/crates/stackable-versioned-macros/fixtures/inputs/default/submodule.rs new file mode 100644 index 000000000..b1665c729 --- /dev/null +++ b/crates/stackable-versioned-macros/fixtures/inputs/default/submodule.rs @@ -0,0 +1,11 @@ +#[versioned(version(name = "v1alpha1"), version(name = "v1"))] +// --- +mod versioned { + mod v1alpha1 { + pub use my::reexport::v1alpha1::*; + } + + struct Foo { + bar: usize, + } +} diff --git a/crates/stackable-versioned-macros/fixtures/snapshots/stackable_versioned_macros__test__default_snapshots@submodule.rs.snap b/crates/stackable-versioned-macros/fixtures/snapshots/stackable_versioned_macros__test__default_snapshots@submodule.rs.snap new file mode 100644 index 000000000..7b6b9bc3d --- /dev/null +++ b/crates/stackable-versioned-macros/fixtures/snapshots/stackable_versioned_macros__test__default_snapshots@submodule.rs.snap @@ -0,0 +1,26 @@ +--- +source: crates/stackable-versioned-macros/src/lib.rs +expression: formatted +input_file: crates/stackable-versioned-macros/fixtures/inputs/default/submodule.rs +--- +#[automatically_derived] +mod v1alpha1 { + use super::*; + pub use my::reexport::v1alpha1::*; + pub struct Foo { + pub bar: usize, + } +} +#[automatically_derived] +impl ::std::convert::From for v1::Foo { + fn from(__sv_foo: v1alpha1::Foo) -> Self { + Self { bar: __sv_foo.bar.into() } + } +} +#[automatically_derived] +mod v1 { + use super::*; + pub struct Foo { + pub bar: usize, + } +} From 6c7f353f4ea5fc0b33976c8eff13d9785d1f313e Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Feb 2025 13:11:49 +0100 Subject: [PATCH 5/8] docs: Add submodule section to doc comments --- crates/stackable-versioned-macros/src/lib.rs | 69 ++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/crates/stackable-versioned-macros/src/lib.rs b/crates/stackable-versioned-macros/src/lib.rs index 4bb199770..087b7bc6e 100644 --- a/crates/stackable-versioned-macros/src/lib.rs +++ b/crates/stackable-versioned-macros/src/lib.rs @@ -261,6 +261,75 @@ mod utils; /// } /// ``` /// +/// ### Re-emitting and merging Submodules +/// +/// Modules defined in the versioned module will be re-emitted. This allows for +/// composition of re-exports to compose easier to use imports for downstream +/// consumers of versioned containers. The following rules apply: +/// +/// 1. Only modules named the same like defined versions will be re-emitted. +/// Modules named differently will be ignored and won't produce any code. In +/// the future, this might return an error instead. +/// 2. Only `use` statements defined in the module will be emitted. Other items +/// will be ignored and won't produce any code. In the future, this might +/// return an error instead. +/// +/// ``` +/// # use stackable_versioned_macros::versioned; +/// # mod a { +/// # pub mod v1alpha1 {} +/// # } +/// # mod b { +/// # pub mod v1alpha1 {} +/// # } +/// #[versioned( +/// version(name = "v1alpha1"), +/// version(name = "v1") +/// )] +/// mod versioned { +/// mod v1alpha1 { +/// pub use a::v1alpha1::*; +/// pub use b::v1alpha1::*; +/// } +/// +/// struct Foo { +/// bar: usize, +/// } +/// } +/// # fn main() {} +/// ``` +/// +///
+/// Expand Generated Code +/// +/// ```ignore +/// mod v1alpha1 { +/// use super::*; +/// pub use a::v1alpha1::*; +/// pub use b::v1alpha1::*; +/// pub struct Foo { +/// pub bar: usize, +/// } +/// } +/// +/// impl ::std::convert::From for v1::Foo { +/// fn from(__sv_foo: v1alpha1::Foo) -> Self { +/// Self { +/// bar: __sv_foo.bar.into(), +/// } +/// } +/// } +/// +/// mod v1 { +/// use super::*; +/// pub struct Foo { +/// pub bar: usize, +/// } +/// } +/// ``` +/// +///
+/// /// ## Item Actions /// /// This crate currently supports three different item actions. Items can From 12b9883daeb654a267aa41f486a2fa2e31525439 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Feb 2025 14:42:18 +0100 Subject: [PATCH 6/8] refactor: Emit errors instead of ignoring invalid code --- .../src/codegen/module.rs | 85 ++++++++++--------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/crates/stackable-versioned-macros/src/codegen/module.rs b/crates/stackable-versioned-macros/src/codegen/module.rs index cb1ef474a..0de782f57 100644 --- a/crates/stackable-versioned-macros/src/codegen/module.rs +++ b/crates/stackable-versioned-macros/src/codegen/module.rs @@ -21,7 +21,7 @@ pub struct Module { // Recognized items of the module containers: Vec, - submodules: Vec, + submodules: HashMap>, ident: IdentString, vis: Visibility, @@ -56,8 +56,8 @@ impl Module { .map_or(false, |opts| opts.from.is_present()); let mut errors = Error::accumulator(); + let mut submodules = HashMap::new(); let mut containers = Vec::new(); - let mut submodules = Vec::new(); for item in items { match item { @@ -69,29 +69,47 @@ impl Module { let container = Container::new_struct_nested(item_struct, &versions)?; containers.push(container); } - Item::Mod(item_module) => match item_module.content { - Some((_, items)) => { - let imports: Vec = items - .into_iter() - // We are only interested in use statements. Everything else is ignored. - // NOTE (@Techassi): We might want to error instead. - .filter_map(|item| match item { - Item::Use(item_use) => Some(item_use), - _ => None, - }) - .collect(); - - // NOTE (@Techassi): Do we wanna enforce that modules must use version names? - - submodules.push(Submodule { - ident: item_module.ident.into(), - imports, - }); + Item::Mod(submodule) => { + if !versions + .iter() + .any(|v| v.ident.as_ident() == &submodule.ident) + { + errors.push( + Error::custom( + "submodules must use names which are defined as `version`s", + ) + .with_span(&submodule), + ); + continue; } - None => errors.push( - Error::custom("submodules must be module blocks").with_span(&item_module), - ), - }, + + match submodule.content { + Some((_, items)) => { + let use_statements: Vec = items + .into_iter() + // We are only interested in use statements. Everything else is ignored. + .filter_map(|item| match item { + Item::Use(item_use) => Some(item_use), + item => { + errors.push( + Error::custom( + "submodules must only define `use` statements", + ) + .with_span(&item), + ); + + None + } + }) + .collect(); + + submodules.insert(submodule.ident.into(), use_statements); + } + None => errors.push( + Error::custom("submodules must be module blocks").with_span(&submodule), + ), + } + } _ => continue, }; } @@ -232,21 +250,10 @@ impl Module { /// Optionally generates imports (which can be re-exports) located in submodules for the /// specified `version`. fn generate_submodule_imports(&self, version: &VersionDefinition) -> Option { - for submodule in &self.submodules { - if submodule.ident == version.ident { - let imports = &submodule.imports; - - return Some(quote! { - #(#imports)* - }); + self.submodules.get(&version.ident).map(|use_statements| { + quote! { + #(#use_statements)* } - } - - None + }) } } - -pub struct Submodule { - ident: IdentString, - imports: Vec, -} From 07fd93cb3405dada5de6993fce18da851d554a2e Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Feb 2025 14:43:01 +0100 Subject: [PATCH 7/8] test: Add UI tests for invalid submodules --- .../tests/default/fail/submodule_invalid_name.rs | 8 ++++++++ .../tests/default/fail/submodule_invalid_name.stderr | 5 +++++ .../tests/default/fail/submodule_use_statement.rs | 10 ++++++++++ .../tests/default/fail/submodule_use_statement.stderr | 5 +++++ 4 files changed, 28 insertions(+) create mode 100644 crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.rs create mode 100644 crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.stderr create mode 100644 crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.rs create mode 100644 crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.stderr diff --git a/crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.rs b/crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.rs new file mode 100644 index 000000000..2ce7a5997 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.rs @@ -0,0 +1,8 @@ +use stackable_versioned_macros::versioned; + +fn main() { + #[versioned(version(name = "v1alpha1"))] + mod versioned { + mod v1alpha2 {} + } +} diff --git a/crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.stderr b/crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.stderr new file mode 100644 index 000000000..b213f2ebd --- /dev/null +++ b/crates/stackable-versioned-macros/tests/default/fail/submodule_invalid_name.stderr @@ -0,0 +1,5 @@ +error: submodules must use names which are defined as `version`s + --> tests/default/fail/submodule_invalid_name.rs:6:9 + | +6 | mod v1alpha2 {} + | ^^^ diff --git a/crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.rs b/crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.rs new file mode 100644 index 000000000..041451ab0 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.rs @@ -0,0 +1,10 @@ +use stackable_versioned::versioned; + +fn main() { + #[versioned(version(name = "v1alpha1"))] + mod versioned { + mod v1alpha1 { + struct Foo; + } + } +} diff --git a/crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.stderr b/crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.stderr new file mode 100644 index 000000000..966b1061b --- /dev/null +++ b/crates/stackable-versioned-macros/tests/default/fail/submodule_use_statement.stderr @@ -0,0 +1,5 @@ +error: submodules must only define `use` statements + --> tests/default/fail/submodule_use_statement.rs:7:13 + | +7 | struct Foo; + | ^^^^^^ From a3772df19b1fc167b7c635b03f06f626629a6349 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 28 Feb 2025 14:47:09 +0100 Subject: [PATCH 8/8] docs: Adjust doc comments --- crates/stackable-versioned-macros/src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/stackable-versioned-macros/src/lib.rs b/crates/stackable-versioned-macros/src/lib.rs index 087b7bc6e..0d67b8ef7 100644 --- a/crates/stackable-versioned-macros/src/lib.rs +++ b/crates/stackable-versioned-macros/src/lib.rs @@ -268,11 +268,9 @@ mod utils; /// consumers of versioned containers. The following rules apply: /// /// 1. Only modules named the same like defined versions will be re-emitted. -/// Modules named differently will be ignored and won't produce any code. In -/// the future, this might return an error instead. -/// 2. Only `use` statements defined in the module will be emitted. Other items -/// will be ignored and won't produce any code. In the future, this might -/// return an error instead. +/// Using modules with invalid names will return an error. +/// 2. Only `use` statements defined in the module will be emitted. Declaring +/// other items will return an error. /// /// ``` /// # use stackable_versioned_macros::versioned;