From 99c1cdd440b3b4099f3b418ae8a56ceed5db17a5 Mon Sep 17 00:00:00 2001
From: Techassi <sascha.lautenschlaeger@stackable.tech>
Date: Tue, 17 Sep 2024 12:06:53 +0200
Subject: [PATCH 1/2] feat: Emit error when both from_name and from_type are
empty
---
.../src/attrs/common/item.rs | 27 ++++++++++++++-----
.../tests/default/fail/changed.rs | 2 +-
.../tests/default/fail/changed.stderr | 8 +++---
3 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/crates/stackable-versioned-macros/src/attrs/common/item.rs b/crates/stackable-versioned-macros/src/attrs/common/item.rs
index 42b6ac953..7acda8894 100644
--- a/crates/stackable-versioned-macros/src/attrs/common/item.rs
+++ b/crates/stackable-versioned-macros/src/attrs/common/item.rs
@@ -143,20 +143,20 @@ impl ItemAttributes {
let mut errors = Error::accumulator();
- // Semantic validation
+ // Common validation
errors.handle(self.validate_action_combinations(item_ident, item_type));
errors.handle(self.validate_action_order(item_ident, item_type));
errors.handle(self.validate_item_name(item_ident, item_type));
- errors.handle(self.validate_changed_item_name(item_type));
errors.handle(self.validate_item_attributes(item_attrs));
+ // Action specific validation
+ errors.handle(self.validate_changed_action(item_ident, item_type));
+
// TODO (@Techassi): Add hint if a field or variant is added in the
// first version that it might be clever to remove the 'added'
// attribute.
- errors.finish()?;
-
- Ok(())
+ errors.finish()
}
/// This associated function is called by the top-level validation function
@@ -293,13 +293,18 @@ impl ItemAttributes {
}
}
}
+
Ok(())
}
/// This associated function is called by the top-level validation function
/// and validates that parameters provided to the `changed` actions are
/// valid.
- fn validate_changed_item_name(&self, item_type: &ItemType) -> Result<(), Error> {
+ fn validate_changed_action(
+ &self,
+ item_ident: &Ident,
+ item_type: &ItemType,
+ ) -> Result<(), Error> {
let prefix = match item_type {
ItemType::Field => DEPRECATED_FIELD_PREFIX,
ItemType::Variant => DEPRECATED_VARIANT_PREFIX,
@@ -307,8 +312,16 @@ impl ItemAttributes {
let mut errors = Error::accumulator();
- // This ensures that `from_name` doesn't include the deprecation prefix.
for change in &self.changes {
+ // Ensure that from_name and from_type are not empty at the same
+ // time.
+ if change.from_name.is_none() && change.from_type.is_none() {
+ errors.push(Error::custom(format!(
+ "{item_type} was marked as `changed`, but both `from_name` and `from_type` are unset"
+ )).with_span(item_ident));
+ }
+
+ // Ensure that `from_name` doesn't include the deprecation prefix.
if let Some(from_name) = change.from_name.as_ref() {
if from_name.starts_with(prefix) {
errors.push(
diff --git a/crates/stackable-versioned-macros/tests/default/fail/changed.rs b/crates/stackable-versioned-macros/tests/default/fail/changed.rs
index 2d17aa768..61aba4e0e 100644
--- a/crates/stackable-versioned-macros/tests/default/fail/changed.rs
+++ b/crates/stackable-versioned-macros/tests/default/fail/changed.rs
@@ -9,7 +9,7 @@ fn main() {
struct Foo {
#[versioned(
changed(since = "v1beta1", from_name = "deprecated_bar"),
- changed(since = "v1", from_name = "deprecated_baz")
+ changed(since = "v1")
)]
bar: usize,
}
diff --git a/crates/stackable-versioned-macros/tests/default/fail/changed.stderr b/crates/stackable-versioned-macros/tests/default/fail/changed.stderr
index a6d0d4070..50b88411e 100644
--- a/crates/stackable-versioned-macros/tests/default/fail/changed.stderr
+++ b/crates/stackable-versioned-macros/tests/default/fail/changed.stderr
@@ -4,8 +4,8 @@ error: the previous field name must not start with the deprecation prefix
11 | changed(since = "v1beta1", from_name = "deprecated_bar"),
| ^^^^^^^^^^^^^^^^
-error: the previous field name must not start with the deprecation prefix
- --> tests/default/fail/changed.rs:12:47
+error: field was marked as `changed`, but both `from_name` and `from_type` are unset
+ --> tests/default/fail/changed.rs:14:9
|
-12 | changed(since = "v1", from_name = "deprecated_baz")
- | ^^^^^^^^^^^^^^^^
+14 | bar: usize,
+ | ^^^
From 73f84e606260129b88854fb088c646370aca720a Mon Sep 17 00:00:00 2001
From: Techassi <sascha.lautenschlaeger@stackable.tech>
Date: Tue, 17 Sep 2024 12:07:51 +0200
Subject: [PATCH 2/2] refactor: Centralize doc comment handling
---
.../src/codegen/common/mod.rs | 67 +++++++++++++------
.../src/codegen/venum/mod.rs | 26 +------
.../src/codegen/vstruct/mod.rs | 26 +------
3 files changed, 50 insertions(+), 69 deletions(-)
diff --git a/crates/stackable-versioned-macros/src/codegen/common/mod.rs b/crates/stackable-versioned-macros/src/codegen/common/mod.rs
index d46cfd160..74ac60483 100644
--- a/crates/stackable-versioned-macros/src/codegen/common/mod.rs
+++ b/crates/stackable-versioned-macros/src/codegen/common/mod.rs
@@ -1,8 +1,8 @@
use std::collections::BTreeMap;
use k8s_version::Version;
-use proc_macro2::Span;
-use quote::format_ident;
+use proc_macro2::{Span, TokenStream};
+use quote::{format_ident, quote, ToTokens};
use syn::Ident;
use crate::{
@@ -34,24 +34,7 @@ pub(crate) struct ContainerVersion {
pub(crate) ident: Ident,
/// Store additional doc-comment lines for this version.
- pub(crate) version_specific_docs: Vec<String>,
-}
-
-/// Converts lines of doc-comments into a trimmed list.
-fn process_docs(input: &Option<String>) -> Vec<String> {
- if let Some(input) = input {
- input
- // Trim the leading and trailing whitespace, deleting suprefluous
- // empty lines.
- .trim()
- .lines()
- // Trim the leading and trailing whitespace on each line that can be
- // introduced when the developer indents multi-line comments.
- .map(|line| line.trim().to_owned())
- .collect()
- } else {
- Vec::new()
- }
+ pub(crate) docs: Docs,
}
impl From<&ContainerAttributes> for Vec<ContainerVersion> {
@@ -63,13 +46,55 @@ impl From<&ContainerAttributes> for Vec<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(),
+ docs: v.doc.clone().into(),
inner: v.name,
- version_specific_docs: process_docs(&v.doc),
})
.collect()
}
}
+#[derive(Clone, Debug)]
+pub(crate) struct Docs(Vec<String>);
+
+impl From<Option<String>> for Docs {
+ fn from(doc: Option<String>) -> Self {
+ let lines = if let Some(doc) = doc {
+ doc
+ // Trim the leading and trailing whitespace, deleting
+ // superfluous empty lines.
+ .trim()
+ .lines()
+ // Trim the leading and trailing whitespace on each line that
+ // can be introduced when the developer indents multi-line
+ // comments.
+ .map(|line| line.trim().into())
+ .collect()
+ } else {
+ Vec::new()
+ };
+
+ Self(lines)
+ }
+}
+
+impl ToTokens for Docs {
+ fn to_tokens(&self, tokens: &mut TokenStream) {
+ for (index, line) in self.0.iter().enumerate() {
+ if index == 0 {
+ // Prepend an empty line to clearly separate the version/action
+ // specific docs.
+ tokens.extend(quote! {
+ #[doc = ""]
+ })
+ }
+
+ tokens.extend(quote! {
+ #[doc = #line]
+ })
+ }
+ }
+}
+
/// Removes the deprecated prefix from a field ident.
///
/// See [`DEPRECATED_FIELD_PREFIX`].
diff --git a/crates/stackable-versioned-macros/src/codegen/venum/mod.rs b/crates/stackable-versioned-macros/src/codegen/venum/mod.rs
index d6b691afb..a0ada2c65 100644
--- a/crates/stackable-versioned-macros/src/codegen/venum/mod.rs
+++ b/crates/stackable-versioned-macros/src/codegen/venum/mod.rs
@@ -106,6 +106,7 @@ impl VersionedEnum {
// enable the attribute macro to be applied to a module which
// generates versioned versions of all contained containers.
+ let version_specific_docs = &version.docs;
let version_ident = &version.ident;
let deprecated_note = format!("Version {version} is deprecated", version = version_ident);
@@ -113,9 +114,6 @@ impl VersionedEnum {
.deprecated
.then_some(quote! {#[deprecated = #deprecated_note]});
- // Generate doc comments for the container (enum)
- let version_specific_docs = self.generate_enum_docs(version);
-
// Generate tokens for the module and the contained enum
token_stream.extend(quote! {
#[automatically_derived]
@@ -123,8 +121,8 @@ impl VersionedEnum {
#visibility mod #version_ident {
use super::*;
- #version_specific_docs
#(#original_attributes)*
+ #version_specific_docs
pub enum #enum_name {
#variants
}
@@ -139,26 +137,6 @@ impl VersionedEnum {
token_stream
}
- /// Generates version specific doc comments for the enum.
- fn generate_enum_docs(&self, version: &ContainerVersion) -> TokenStream {
- let mut tokens = TokenStream::new();
-
- for (i, doc) in version.version_specific_docs.iter().enumerate() {
- if i == 0 {
- // Prepend an empty line to clearly separate the version
- // specific docs.
- tokens.extend(quote! {
- #[doc = ""]
- })
- }
- tokens.extend(quote! {
- #[doc = #doc]
- })
- }
-
- tokens
- }
-
fn generate_enum_variants(&self, version: &ContainerVersion) -> TokenStream {
let mut token_stream = TokenStream::new();
diff --git a/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs b/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs
index 6dd493906..220e5c3c0 100644
--- a/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs
+++ b/crates/stackable-versioned-macros/src/codegen/vstruct/mod.rs
@@ -126,6 +126,7 @@ impl VersionedStruct {
// enable the attribute macro to be applied to a module which
// generates versioned versions of all contained containers.
+ let version_specific_docs = &version.docs;
let version_ident = &version.ident;
let deprecated_note = format!("Version {version} is deprecated", version = version_ident);
@@ -133,9 +134,6 @@ impl VersionedStruct {
.deprecated
.then_some(quote! {#[deprecated = #deprecated_note]});
- // Generate doc comments for the container (struct)
- let version_specific_docs = self.generate_struct_docs(version);
-
// Generate K8s specific code
let kubernetes_cr_derive = self.generate_kubernetes_cr_derive(version);
@@ -146,8 +144,8 @@ impl VersionedStruct {
#visibility mod #version_ident {
use super::*;
- #version_specific_docs
#(#original_attributes)*
+ #version_specific_docs
#kubernetes_cr_derive
pub struct #struct_name {
#fields
@@ -163,26 +161,6 @@ impl VersionedStruct {
token_stream
}
- /// Generates version specific doc comments for the struct.
- fn generate_struct_docs(&self, version: &ContainerVersion) -> TokenStream {
- let mut tokens = TokenStream::new();
-
- for (i, doc) in version.version_specific_docs.iter().enumerate() {
- if i == 0 {
- // Prepend an empty line to clearly separate the version
- // specific docs.
- tokens.extend(quote! {
- #[doc = ""]
- })
- }
- tokens.extend(quote! {
- #[doc = #doc]
- })
- }
-
- tokens
- }
-
/// Generates struct fields following the `name: type` format which includes
/// a trailing comma.
fn generate_struct_fields(&self, version: &ContainerVersion) -> TokenStream {