Skip to content

feat(stackable-versioned): Add support for versioned enums #813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cb8fbc9
Update changelog
Techassi Jun 17, 2024
d2641d4
Update PR link in the changelog
Techassi Jun 17, 2024
52b4536
Merge branch 'main' into feat/crd-versioning-enum-support
Techassi Jun 18, 2024
6721b0c
Start to move code, add enum impls
Techassi Jun 19, 2024
f98598a
Introduce generalized structs for containers and items
Techassi Jun 20, 2024
124811f
Finish traits and generic types
Techassi Jun 24, 2024
28048a1
Use From<&ContainerAttributes> for Vec<ContainerVersion> implementation
Techassi Jun 24, 2024
f524d32
Merge branch 'main' into feat/crd-versioning-enum-support
Techassi Jun 24, 2024
b315baf
Finish basic enum code generation
Techassi Jun 25, 2024
18e54c1
Use darling(flatten) for field attrs
Techassi Jun 25, 2024
e173aba
Replace unwraps with expects
Techassi Jun 26, 2024
96381d7
Generate code for all item states
Techassi Jun 26, 2024
c125b98
Start adding From ipls for enum conversion
Techassi Jun 27, 2024
161380c
Merge branch 'main' into feat/crd-versioning-enum-support
Techassi Jun 27, 2024
169d716
Finish basic From impls for enums
Techassi Jun 28, 2024
0fed8bc
Merge branch 'main' into feat/crd-versioning-enum-support
Techassi Jun 28, 2024
69e4977
Apply suggestions
Techassi Jul 1, 2024
68b28ec
Apply more suggestions
Techassi Jul 2, 2024
758e885
Rename starts_with variable to starts_with_deprecated
Techassi Jul 5, 2024
028fc3c
Remove old todo comment
Techassi Jul 5, 2024
7cf7396
Add auto-generated notes for deprecated versions
Techassi Jul 5, 2024
de1cde3
Move attribute parsing into new() functions
Techassi Jul 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions crates/stackable-versioned-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
71 changes: 71 additions & 0 deletions crates/stackable-versioned-macros/src/attrs/common/item.rs
Original file line number Diff line number Diff line change
@@ -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<AddedAttributes>,

/// 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<RenamedAttributes>,

/// This parses the `deprecated` attribute on items (fields or variants). It
/// can only be present at most once.
pub(crate) deprecated: Option<DeprecatedAttributes>,
}

impl ItemAttributes {
fn validate(self) -> Result<Self, Error> {
// 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<Version>,

#[darling(rename = "default", default = "default_default_fn")]
pub(crate) default_fn: SpannedValue<Path>,
}

fn default_default_fn() -> SpannedValue<Path> {
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<Version>,
pub(crate) from: SpannedValue<String>,
}

#[derive(Clone, Debug, FromMeta)]
pub(crate) struct DeprecatedAttributes {
pub(crate) since: SpannedValue<Version>,
pub(crate) note: SpannedValue<String>,
}
5 changes: 5 additions & 0 deletions crates/stackable-versioned-macros/src/attrs/common/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
mod container;
mod item;

pub(crate) use container::*;
pub(crate) use item::*;
107 changes: 37 additions & 70 deletions crates/stackable-versioned-macros/src/attrs/field.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
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::{ContainerAttributes, ItemAttributes},
consts::DEPRECATED_FIELD_PREFIX,
};

/// This struct describes all available field attributes, as well as the field
/// name to display better diagnostics.
Expand All @@ -29,43 +30,24 @@ use crate::{attrs::container::ContainerAttributes, consts::DEPRECATED_PREFIX};
and_then = FieldAttributes::validate
)]
pub(crate) struct FieldAttributes {
pub(crate) ident: Option<Ident>,
pub(crate) added: Option<AddedAttributes>,

#[darling(multiple, rename = "renamed")]
pub(crate) renames: Vec<RenamedAttributes>,

pub(crate) deprecated: Option<DeprecatedAttributes>,
}

#[derive(Clone, Debug, FromMeta)]
pub(crate) struct AddedAttributes {
pub(crate) since: SpannedValue<Version>,

#[darling(rename = "default", default = "default_default_fn")]
pub(crate) default_fn: SpannedValue<Path>,
}

fn default_default_fn() -> SpannedValue<Path> {
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<Version>,
pub(crate) from: SpannedValue<String>,
}
#[darling(flatten)]
pub(crate) common: ItemAttributes,

#[derive(Clone, Debug, FromMeta)]
pub(crate) struct DeprecatedAttributes {
pub(crate) since: SpannedValue<Version>,
pub(crate) note: SpannedValue<String>,
// The ident (automatically extracted by darling) cannot be moved into the
// shared item attributes because for struct fields, the type is
// `Option<Ident>`, while for enum variants, the type is `Ident`.
pub(crate) ident: Option<Ident>,
}

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
Expand All @@ -80,11 +62,8 @@ 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 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.
Expand All @@ -107,7 +86,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",
Expand Down Expand Up @@ -145,15 +128,15 @@ 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.
// 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));
Expand All @@ -162,7 +145,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)
}) {
Expand All @@ -188,17 +171,17 @@ 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_PREFIX);
.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));
Expand All @@ -207,22 +190,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(
Expand All @@ -233,7 +200,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()
Expand All @@ -246,7 +213,7 @@ impl FieldAttributes {
}
}

for rename in &self.renames {
for rename in &self.common.renames {
if !container_attrs
.versions
.iter()
Expand All @@ -259,7 +226,7 @@ impl FieldAttributes {
}
}

if let Some(deprecated) = &self.deprecated {
if let Some(deprecated) = &self.common.deprecated {
if !container_attrs
.versions
.iter()
Expand Down
3 changes: 2 additions & 1 deletion crates/stackable-versioned-macros/src/attrs/mod.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
pub(crate) mod container;
pub(crate) mod common;
pub(crate) mod field;
pub(crate) mod variant;
Loading