diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c49d7d..49fce8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ ## Unreleased +## 1.1.0 + +- Add `#[cache_diff(custom = )]` to containers (structs) to allow for customizing + deriving diffs. (https://github.com/heroku-buildpacks/cache_diff/pull/6) +- Add: Allow annotating ignored fields with `#[cache_diff(ignore = "")]`. Using `ignore = "custom"` requires the container (struct) to implement `custom = `. (https://github.com/heroku-buildpacks/cache_diff/pull/6) + +## 1.0.1 + - Fix: Multiple `#[derive(CachDiff)]` calls in the same file now work (https://github.com/heroku-buildpacks/cache_diff/pull/4) ## 1.0.0 diff --git a/Cargo.lock b/Cargo.lock index f79ba14..ae2c641 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10,7 +10,7 @@ checksum = "1e038a9e6b7e36319ab42141434b0c7a93f7714aa90abf63cc5086e106027bb7" [[package]] name = "cache_diff" -version = "1.0.1" +version = "1.1.0" dependencies = [ "bullet_stream", "cache_diff_derive", @@ -18,15 +18,23 @@ dependencies = [ [[package]] name = "cache_diff_derive" -version = "1.0.1" +version = "1.1.0" dependencies = [ "bullet_stream", + "indoc", + "pretty_assertions", "proc-macro2", "quote", "strum", "syn", ] +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "equivalent" version = "1.0.1" @@ -61,6 +69,12 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "indoc" +version = "2.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b248f5224d1d606005e02c97f5aa4e88eeb230488bcc03bc9ca4d7991399f2b5" + [[package]] name = "itoa" version = "1.0.11" @@ -73,6 +87,16 @@ version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" +[[package]] +name = "pretty_assertions" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ae130e2f271fbc2ac3a40fb1d07180839cdbbe443c7a27e1e3c13c5cac0116d" +dependencies = [ + "diff", + "yansi", +] + [[package]] name = "proc-macro2" version = "1.0.89" @@ -252,6 +276,9 @@ name = "usage" version = "0.1.0" dependencies = [ "cache_diff", + "indoc", + "pretty_assertions", + "serde", "trybuild", ] @@ -345,3 +372,9 @@ checksum = "36c1fec1a2bb5866f07c25f68c26e565c4c200aebb96d7e55710c19d3e8ac49b" dependencies = [ "memchr", ] + +[[package]] +name = "yansi" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" diff --git a/Cargo.toml b/Cargo.toml index 6bdfabc..0796f4a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,9 +7,14 @@ members = [ ] [workspace.package] -version = "1.0.1" +version = "1.1.0" rust-version = "1.82" edition = "2021" license = "Apache-2.0" repository = "https://github.com/heroku-buildpacks/cache_diff" documentation = "https://docs.rs/cache_diff" + +[workspace.dependencies] +pretty_assertions = "1.4.1" +indoc = "2" +serde = {version = "1.0", features = ["derive"] } diff --git a/cache_diff/README.md b/cache_diff/README.md index 7a1311d..ee67154 100644 --- a/cache_diff/README.md +++ b/cache_diff/README.md @@ -26,11 +26,16 @@ When it returns an empty list, the two structs are identical. You can manually implement the trait, or you can use the `#[derive(CacheDiff)]` macro to automatically generate the implementation. -Attributes are: +Top level struct configuration (Container attributes): - - `cache_diff(rename = "")` Specify custom name for the field - - `cache_diff(ignore)` Ignores the given field - - `cache_diff(display = )` Specify a function to call to display the field +- `#[cache_diff(custom = )]` Specify a function that receives references to both current and old values and returns a Vec of strings if there are any differences. This function is only called once. It can be in combination with `#[cache_diff(custom)]` on fields to combine multiple related fields into one diff (for example OS distribution and version) or to split apart a monolithic field into multiple differences (for example an "inventory" struct that contains a version and CPU architecture information). + +Attributes for fields are: + +- `#[cache_diff(rename = "")]` Specify custom name for the field +- `#[cache_diff(ignore)]` or `#[cache_diff(ignore = "")]` Ignores the given field with an optional comment string. + If the field is ignored because you're using a custom diff function (see container attributes) you can use + `cache_diff(ignore = "custom")` which will check that the container implements a custom function. ### Why @@ -189,6 +194,58 @@ let diff = now.diff(&Metadata { version: NoDisplay("3.3.0".to_string())}); assert_eq!(diff.join(" "), "version (`custom 3.3.0` to `custom 3.4.0`)"); ``` +### Customize one or more field differences + +You can provide a custom implementation for a diffing a subset of fields without having to roll your own implementation. + +#### Custom logic for one field example + +Here's an example where someone wants to bust the cache after N cache calls. Everything else other than `cache_usage_count` can be derived. If you want to keep the existing derived difference checks, but add on a custom one you can do it like this: + +```rust +use cache_diff::CacheDiff; +const MAX: f32 = 200.0; + +#[derive(Debug, CacheDiff)] +#[cache_diff(custom = diff_cache_usage_count)] +pub(crate) struct Metadata { + #[cache_diff(ignore = "custom")] + cache_usage_count: f32, + + binary_version: String, + target_arch: String, + os_distribution: String, + os_version: String, +} + +fn diff_cache_usage_count(_old: &Metadata, now: &Metadata) -> Vec { + let Metadata { + cache_usage_count, + binary_version: _, + target_arch: _, + os_distribution: _, + os_version: _, + } = now; + + if cache_usage_count > &MAX { + vec![format!("Cache count ({}) exceeded limit {MAX}", cache_usage_count)] + } else { + Vec::new() + } +} +``` + +In this example, four fields are derived automatically, saving us time, while one field is custom +using the `#[cache_diff(custom = diff_cache_usage_count)]` attribute on the struct. This tells +[CacheDiff] to call this function and pass in the old and current values. It expects a vector +with some strings if there is a difference and an empty vector if there are none. + +Don't forget to "ignore" any fields you're implementing yourself. You can also use this feature to +combine several fields into a single diff output, for example using the previous struct, if +you only wanted to have one output for a combined `os_distribution` and `os_version` in one output +like "OS (ubuntu-22 to ubuntu-24)". Alternatively, you can use to +re-arrange your struct to only have one field with a custom display. + ## Releasing diff --git a/cache_diff/src/lib.rs b/cache_diff/src/lib.rs index 38b91dd..82ea471 100644 --- a/cache_diff/src/lib.rs +++ b/cache_diff/src/lib.rs @@ -7,11 +7,16 @@ //! //! You can manually implement the trait, or you can use the `#[derive(CacheDiff)]` macro to automatically generate the implementation. //! -//! Attributes are: +//! Top level struct configuration (Container attributes): //! -//! - `cache_diff(rename = "")` Specify custom name for the field -//! - `cache_diff(ignore)` Ignores the given field -//! - `cache_diff(display = )` Specify a function to call to display the field +//! - `#[cache_diff(custom = )]` Specify a function that receives references to both current and old values and returns a Vec of strings if there are any differences. This function is only called once. It can be in combination with `#[cache_diff(custom)]` on fields to combine multiple related fields into one diff (for example OS distribution and version) or to split apart a monolithic field into multiple differences (for example an "inventory" struct that contains a version and CPU architecture information). +//! +//! Attributes for fields are: +//! +//! - `#[cache_diff(rename = "")]` Specify custom name for the field +//! - `#[cache_diff(ignore)]` or `#[cache_diff(ignore = "")]` Ignores the given field with an optional comment string. +//! If the field is ignored because you're using a custom diff function (see container attributes) you can use +//! `cache_diff(ignore = "custom")` which will check that the container implements a custom function. //! //! ## Why //! @@ -169,6 +174,58 @@ //! //! assert_eq!(diff.join(" "), "version (`custom 3.3.0` to `custom 3.4.0`)"); //! ``` +//! +//! ## Customize one or more field differences +//! +//! You can provide a custom implementation for a diffing a subset of fields without having to roll your own implementation. +//! +//! ### Custom logic for one field example +//! +//! Here's an example where someone wants to bust the cache after N cache calls. Everything else other than `cache_usage_count` can be derived. If you want to keep the existing derived difference checks, but add on a custom one you can do it like this: +//! +//! ```rust +//! use cache_diff::CacheDiff; +//! const MAX: f32 = 200.0; +//! +//! #[derive(Debug, CacheDiff)] +//! #[cache_diff(custom = diff_cache_usage_count)] +//! pub(crate) struct Metadata { +//! #[cache_diff(ignore = "custom")] +//! cache_usage_count: f32, +//! +//! binary_version: String, +//! target_arch: String, +//! os_distribution: String, +//! os_version: String, +//! } +//! +//! fn diff_cache_usage_count(_old: &Metadata, now: &Metadata) -> Vec { +//! let Metadata { +//! cache_usage_count, +//! binary_version: _, +//! target_arch: _, +//! os_distribution: _, +//! os_version: _, +//! } = now; +//! +//! if cache_usage_count > &MAX { +//! vec![format!("Cache count ({}) exceeded limit {MAX}", cache_usage_count)] +//! } else { +//! Vec::new() +//! } +//! } +//! ``` +//! +//! In this example, four fields are derived automatically, saving us time, while one field is custom +//! using the `#[cache_diff(custom = diff_cache_usage_count)]` attribute on the struct. This tells +//! [CacheDiff] to call this function and pass in the old and current values. It expects a vector +//! with some strings if there is a difference and an empty vector if there are none. +//! +//! Don't forget to "ignore" any fields you're implementing yourself. You can also use this feature to +//! combine several fields into a single diff output, for example using the previous struct, if +//! you only wanted to have one output for a combined `os_distribution` and `os_version` in one output +//! like "OS (ubuntu-22 to ubuntu-24)". Alternatively, you can use to +//! re-arrange your struct to only have one field with a custom display. /// Centralized cache invalidation logic with human readable differences /// diff --git a/cache_diff_derive/Cargo.toml b/cache_diff_derive/Cargo.toml index 4bc614a..e03078d 100644 --- a/cache_diff_derive/Cargo.toml +++ b/cache_diff_derive/Cargo.toml @@ -18,3 +18,7 @@ strum = {version = "0.26", features = ["derive"] } [lib] proc-macro = true + +[dev-dependencies] +pretty_assertions.workspace = true +indoc.workspace = true diff --git a/cache_diff_derive/src/attributes.rs b/cache_diff_derive/src/attributes.rs index a8008dd..f0ae441 100644 --- a/cache_diff_derive/src/attributes.rs +++ b/cache_diff_derive/src/attributes.rs @@ -37,7 +37,7 @@ pub(crate) struct CacheDiffAttributes { pub(crate) display: Option, /// When `Some` indicates the field should be ignored in the diff comparison - pub(crate) ignore: Option<()>, + pub(crate) ignore: Option, } impl CacheDiffAttributes { @@ -89,14 +89,19 @@ impl syn::parse::Parse for CacheDiffAttributes { let name_str = name.to_string(); let mut attribute = CacheDiffAttributes::default(); match Key::from_str(&name_str).map_err(|_| { + let extra = match name_str.as_ref() { + "custom" => "\nThe cache_diff attribute `custom` is available on the struct, not the field", + _ => "" + }; + syn::Error::new( name.span(), format!( - "Unknown cache_diff attribute: `{name_str}`. Must be one of {}", - Key::iter() - .map(|k| format!("`{k}`")) - .collect::>() - .join(", ") + "Unknown cache_diff attribute: `{name_str}`. Must be one of {valid_keys}{extra}", + valid_keys = Key::iter() + .map(|k| format!("`{k}`")) + .collect::>() + .join(", ") ), ) })? { @@ -110,7 +115,12 @@ impl syn::parse::Parse for CacheDiffAttributes { attribute.display = Some(input.parse()?); } Key::ignore => { - attribute.ignore = Some(()); + if input.peek(syn::Token![=]) { + input.parse::()?; + attribute.ignore = Some(input.parse::()?.value()); + } else { + attribute.ignore = Some("_ignored".to_string()); + } } } Ok(attribute) @@ -120,6 +130,8 @@ impl syn::parse::Parse for CacheDiffAttributes { #[cfg(test)] mod test { use super::*; + use indoc::formatdoc; + use pretty_assertions::assert_eq; #[test] fn test_parse_all_rename() { @@ -145,18 +157,47 @@ mod test { assert_eq!(CacheDiffAttributes::parse_all(&input).unwrap(), expected); } + #[test] + fn test_ignore_with_value() { + let input = syn::parse_quote! { + #[cache_diff(ignore = "value")] + }; + let expected = CacheDiffAttributes { + ignore: Some("value".to_string()), + ..Default::default() + }; + assert_eq!(CacheDiffAttributes::parse_all(&input).unwrap(), expected); + } + #[test] fn test_parse_all_ignore() { let input = syn::parse_quote! { #[cache_diff(ignore)] }; let expected = CacheDiffAttributes { - ignore: Some(()), + ignore: Some("_ignored".to_string()), ..Default::default() }; assert_eq!(CacheDiffAttributes::parse_all(&input).unwrap(), expected); } + #[test] + fn test_parse_accidental_custom() { + let input = syn::parse_quote! { + #[cache_diff(custom = "IDK")] + }; + let result = CacheDiffAttributes::parse_all(&input); + assert!(result.is_err(), "Expected an error, got {:?}", result); + assert_eq!( + format!("{}", result.err().unwrap()).trim(), + formatdoc! {" + Unknown cache_diff attribute: `custom`. Must be one of `rename`, `display`, `ignore` + The cache_diff attribute `custom` is available on the struct, not the field + "} + .trim() + ); + } + #[test] fn test_parse_all_unknown() { let input = syn::parse_quote! { diff --git a/cache_diff_derive/src/fields.rs b/cache_diff_derive/src/fields.rs index e33e17a..a1d2a31 100644 --- a/cache_diff_derive/src/fields.rs +++ b/cache_diff_derive/src/fields.rs @@ -1,7 +1,10 @@ use crate::attributes::CacheDiffAttributes; use proc_macro2::TokenStream; use quote::quote; +use syn::parse::Parse; +use syn::punctuated::Punctuated; use syn::spanned::Spanned; +use syn::token::Comma; use syn::Data::Struct; use syn::Fields::Named; use syn::{DataStruct, DeriveInput, Field, FieldsNamed, Ident, PathArguments}; @@ -22,9 +25,24 @@ struct CacheDiffField { } impl CacheDiffField { - fn new(field: &Field, attributes: CacheDiffAttributes) -> syn::Result> { - if attributes.ignore.is_some() { - Ok(None) + fn new( + field: &Field, + attributes: CacheDiffAttributes, + container: &Container, + ) -> syn::Result> { + if let Some(ignore) = attributes.ignore { + if ignore == "custom" && container.custom.is_none() { + Err(syn::Error::new( + container.identifier.span(), + format!( + "field `{field}` on {container} marked ignored as custom, but no `#[cache_diff(custom = )]` found on `{container}`", + field = field.ident.as_ref().expect("only named structs supported"), + container = &container.identifier, + ), + )) + } else { + Ok(None) + } } else { let field_identifier = field.ident.clone().ok_or_else(|| { syn::Error::new( @@ -63,20 +81,94 @@ fn is_pathbuf(ty: &syn::Type) -> bool { false } +/// Represents a single attribute on a container AKA a struct +#[derive(Debug, Default)] +struct ContainerAttribute { + custom: Option, +} + +/// Represents the Struct +struct Container { + identifier: Ident, + custom: Option, + fields: Punctuated, +} + +impl Container { + fn from_ast(input: &syn::DeriveInput) -> syn::Result { + let identifier = input.ident.clone(); + let attrs = input.attrs.clone(); + + let attributes = attrs + .iter() + .filter(|attr| attr.path().is_ident("cache_diff")) + .map(|attr| attr.parse_args_with(ContainerAttribute::parse)) + .collect::>>()?; + + if attributes.len() > 1 { + return Err(syn::Error::new( + input.attrs.last().span(), + "Too many attributes", + )); + } + + let custom = attributes.into_iter().next().unwrap_or_default().custom; + let fields = match input.data { + Struct(DataStruct { + fields: Named(FieldsNamed { ref named, .. }), + .. + }) => named, + _ => unimplemented!("Only implemented for structs"), + } + .to_owned(); + + Ok(Container { + identifier, + custom, + fields, + }) + } +} + +impl Parse for ContainerAttribute { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let name: Ident = input.parse()?; + let name_str = name.to_string(); + match name_str.as_ref() { + "custom" => { + input.parse::()?; + Ok(ContainerAttribute { custom: Some(input.parse()?) }) + } + _ => Err(syn::Error::new( + name.span(), + format!( + "Unknown cache_diff attribute on struct: `{name_str}`. Must be one of `custom = `", + ), + )), + } + } +} + pub fn create_cache_diff(item: TokenStream) -> syn::Result { let ast: DeriveInput = syn::parse2(item).unwrap(); - let struct_identifier = ast.ident; - let fields = match ast.data { - Struct(DataStruct { - fields: Named(FieldsNamed { ref named, .. }), - .. - }) => named, - _ => unimplemented!("Only implemented for structs"), + let container = Container::from_ast(&ast)?; + let struct_identifier = &container.identifier; + + let custom_diff = if let Some(ref custom_fn) = container.custom { + quote! { + let custom_diff = #custom_fn(old, self); + for diff in &custom_diff { + differences.push(diff.to_string()) + } + } + } else { + quote! {} }; + let mut comparisons = Vec::new(); - for f in fields.iter() { + for f in container.fields.iter() { let attributes = CacheDiffAttributes::from(f)?; - let field = CacheDiffField::new(f, attributes)?; + let field = CacheDiffField::new(f, attributes, &container)?; if let Some(CacheDiffField { field_identifier: field_ident, @@ -108,6 +200,7 @@ pub fn create_cache_diff(item: TokenStream) -> syn::Result { impl cache_diff::CacheDiff for #struct_identifier { fn diff(&self, old: &Self) -> Vec { let mut differences = Vec::new(); + #custom_diff #(#comparisons)* differences } diff --git a/usage/Cargo.toml b/usage/Cargo.toml index 0fd6b9e..e8bf868 100644 --- a/usage/Cargo.toml +++ b/usage/Cargo.toml @@ -9,3 +9,6 @@ cache_diff = { path = "../cache_diff" } [dev-dependencies] trybuild = "1.0.101" +pretty_assertions.workspace = true +indoc.workspace = true +serde.workspace = true diff --git a/usage/src/main.rs b/usage/src/main.rs index d83de1f..7604c49 100644 --- a/usage/src/main.rs +++ b/usage/src/main.rs @@ -4,20 +4,58 @@ use cache_diff::CacheDiff; struct Hello { name: String, } + +#[derive(CacheDiff)] +#[cache_diff(custom = diff_fn)] +struct CustomDiffFn { + name: String, +} + +fn diff_fn(old: &CustomDiffFn, now: &CustomDiffFn) -> Vec { + let mut diff = Vec::new(); + diff.push(format!( + "Totally custom old: {} now: {}", + old.name, now.name + )); + diff +} + fn main() { let _ = Hello { name: "world".to_string(), }; println!("Hello, world!"); + let _ = CustomDiffFn { + name: "Hello".to_string(), + }; } #[cfg(test)] mod tests { use super::*; + use pretty_assertions::assert_eq; use std::path::PathBuf; fn is_diff(_in: &T) {} + #[test] + fn custom_diff_function() { + let diff = CustomDiffFn { + name: "Richard".to_string(), + } + .diff(&CustomDiffFn { + name: "Schneems".to_string(), + }); + + assert_eq!( + [ + "Totally custom old: Schneems now: Richard".to_string(), + "name (`Schneems` to `Richard`)".to_string() + ], + diff[..] + ); + } + #[test] fn ignore_a_field() { #[derive(CacheDiff)] diff --git a/usage/tests/compliation_tests.rs b/usage/tests/compliation_tests.rs index fc951f5..8df999c 100644 --- a/usage/tests/compliation_tests.rs +++ b/usage/tests/compliation_tests.rs @@ -3,3 +3,9 @@ fn should_not_compile() { let t = trybuild::TestCases::new(); t.compile_fail("tests/fails/*.rs"); } + +#[test] +fn should_compile() { + let t = trybuild::TestCases::new(); + t.pass("tests/pass/*.rs"); +} diff --git a/usage/tests/fails/accidental_custom_field.rs b/usage/tests/fails/accidental_custom_field.rs new file mode 100644 index 0000000..3141098 --- /dev/null +++ b/usage/tests/fails/accidental_custom_field.rs @@ -0,0 +1,15 @@ +use cache_diff::CacheDiff; + +#[derive(CacheDiff)] +struct AccidentalCustom { + #[cache_diff(custom = function)] + i_am_a_custom_field: String, + + normal: String, +} + +fn function(_old: &AccidentalCustom, _now: &AccidentalCustom) -> Vec { + todo!() +} + +fn main() {} diff --git a/usage/tests/fails/accidental_custom_field.stderr b/usage/tests/fails/accidental_custom_field.stderr new file mode 100644 index 0000000..33f66cd --- /dev/null +++ b/usage/tests/fails/accidental_custom_field.stderr @@ -0,0 +1,6 @@ +error: Unknown cache_diff attribute: `custom`. Must be one of `rename`, `display`, `ignore` + The cache_diff attribute `custom` is available on the struct, not the field + --> tests/fails/accidental_custom_field.rs:5:18 + | +5 | #[cache_diff(custom = function)] + | ^^^^^^ diff --git a/usage/tests/fails/missing_custom.rs b/usage/tests/fails/missing_custom.rs new file mode 100644 index 0000000..6926359 --- /dev/null +++ b/usage/tests/fails/missing_custom.rs @@ -0,0 +1,11 @@ +use cache_diff::CacheDiff; + +#[derive(CacheDiff)] +struct MissingCustom { + #[cache_diff(ignore = "custom")] + i_am_a_custom_field: String, + + normal: String, +} + +fn main() {} diff --git a/usage/tests/fails/missing_custom.stderr b/usage/tests/fails/missing_custom.stderr new file mode 100644 index 0000000..1ee98bf --- /dev/null +++ b/usage/tests/fails/missing_custom.stderr @@ -0,0 +1,5 @@ +error: field `i_am_a_custom_field` on MissingCustom marked ignored as custom, but no `#[cache_diff(custom = )]` found on `MissingCustom` + --> tests/fails/missing_custom.rs:4:8 + | +4 | struct MissingCustom { + | ^^^^^^^^^^^^^ diff --git a/usage/tests/pass/custom_with_serde_attribute.rs b/usage/tests/pass/custom_with_serde_attribute.rs new file mode 100644 index 0000000..c9f48cf --- /dev/null +++ b/usage/tests/pass/custom_with_serde_attribute.rs @@ -0,0 +1,15 @@ +use cache_diff::CacheDiff; +use serde::Deserialize; + +#[derive(Debug, Deserialize, CacheDiff)] +#[cache_diff(custom = custom_diff)] +#[serde(deny_unknown_fields)] +struct Example { + name: String, +} + +fn custom_diff(_old: &Example, _now: &Example) -> Vec { + todo!() +} + +fn main() {}