From 2979e55054d8258aa3e970bfe4852e287d23c90b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 11:16:05 +0200 Subject: [PATCH 01/15] derive: require enum variants to be flattened --- CHANGELOG.md | 8 +++++ miniconf/tests/enum.rs | 3 +- miniconf_derive/src/tree.rs | 61 ++++++++++++++++++++++++++----------- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32babdb1..34571818 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased](https://github.com/quartiq/miniconf/compare/v0.13.0...HEAD) - DATE + +### Changed + +* Enum variants (which currently must be newtypes) must be flattened. This doesn't change + behavior as they have always been flattened, it just required adding the attribute to make it + future proof. + ## [0.14.0](https://github.com/quartiq/miniconf/compare/v0.13.0...v0.14.0) - 2024-09-26 ### Added diff --git a/miniconf/tests/enum.rs b/miniconf/tests/enum.rs index fa3014d4..97f827bb 100644 --- a/miniconf/tests/enum.rs +++ b/miniconf/tests/enum.rs @@ -11,8 +11,9 @@ enum Enum { #[default] None, #[strum(serialize = "foo")] - #[tree(rename = "foo")] + #[tree(rename = "foo", flatten)] A(i32), + #[tree(flatten)] B(#[tree(depth = 1)] Inner), } diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 510fd88c..7a810957 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -15,12 +15,14 @@ pub struct TreeVariant { ident: syn::Ident, rename: Option, skip: Flag, + flatten: Flag, fields: ast::Fields>, } impl TreeVariant { fn field(&self) -> &SpannedValue { - assert!(self.fields.len() == 1); + // assert!(self.fields.is_newtype()); // Don't do this since we modified it with skip + assert!(self.fields.len() == 1); // Only newtypes currently self.fields.fields.first().unwrap() } @@ -34,22 +36,21 @@ impl TreeVariant { #[darling(supports(any))] pub struct Tree { ident: syn::Ident, - // pub flatten: Flag, // FIXME: implement + flatten: Flag, generics: syn::Generics, data: Data, SpannedValue>, } impl Tree { fn depth(&self) -> usize { - match &self.data { - Data::Struct(fields) => fields.fields.iter().fold(0, |d, field| d.max(field.depth)) + 1, - Data::Enum(variants) => { - variants - .iter() - .fold(0, |d, variant| d.max(variant.field().depth)) - + 1 - } - } + let level = if self.flatten.is_present() { 0 } else { 1 }; + let inner = match &self.data { + Data::Struct(fields) => fields.fields.iter().fold(0, |d, field| d.max(field.depth)), + Data::Enum(variants) => variants + .iter() + .fold(0, |d, variant| d.max(variant.field().depth)), + }; + (level + inner).max(1) // We need to eat at least one level. C.f. impl TreeKey for Option. } pub fn parse(input: &syn::DeriveInput) -> Result { @@ -61,7 +62,7 @@ impl Tree { while fields .fields .last() - .map(|f| f.skip.is_present()) + .map(|f| f.ident.is_none() && f.skip.is_present()) .unwrap_or_default() { fields.fields.pop(); @@ -71,27 +72,41 @@ impl Tree { .retain(|f| f.ident.is_none() || !f.skip.is_present()); if let Some(f) = fields.fields.iter().find(|f| f.skip.is_present()) { return Err( + // Note(design) If non-terminal fields are skipped, there is a gap in the indices. + // This could be lifted with a index map. Error::custom("Can only `skip` terminal tuple struct fields") .with_span(&f.skip.span()), ); } + if tree.flatten.is_present() && fields.len() > 1 { + return Err(Error::custom("Can only flatten single-field structs") + .with_span(&tree.flatten.span())); + } } Data::Enum(variants) => { variants.retain(|v| !(v.skip.is_present() || v.fields.is_unit())); for v in variants.iter_mut() { if v.fields.is_struct() { // Note(design) For tuple or named struct variants we'd have to create proxy - // structs anyway to support KeyLookup on that level. - return Err( - Error::custom("Struct variants not supported").with_span(&v.span()) - ); + // structs anyway (or use a FIELD_NAMES: &[&[&str]]) for KeyLookup. + return Err(Error::custom( + "Struct variants with named fields are not supported", + ) + .with_span(&v.span())); + } + if !v.flatten.is_present() { + // Note(design) + return Err(Error::custom( + "Enum variants must be flattened. Add `#[tree(flatten)].", + ) + .with_span(&v.span())); } // unnamed fields can only be skipped if they are terminal while v .fields .fields .last() - .map(|f| f.skip.is_present()) + .map(|f| f.ident.is_none() && f.skip.is_present()) .unwrap_or_default() { v.fields.fields.pop(); @@ -102,6 +117,18 @@ impl Tree { .with_span(&f.skip.span()), ); } + if v.fields.len() != 1 { + return Err(Error::custom( + "Only newtype (single field tuple) enum variants are supported.", + ) + .with_span(&v.span())); + } + } + if tree.flatten.is_present() && variants.len() > 1 { + return Err(Error::custom( + "Can only flatten enums with a single non-unit, non-skipped variant.", + ) + .with_span(&tree.flatten.span())); } } } From 392e1417038e1c56c058ea064e559fcba6839550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 11:35:57 +0200 Subject: [PATCH 02/15] derive: refactor getter/getter_mut --- CHANGELOG.md | 6 +++--- miniconf_derive/src/field.rs | 24 ++++++++++++------------ miniconf_derive/src/tree.rs | 6 ++++++ 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34571818..7560a95f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -* Enum variants (which currently must be newtypes) must be flattened. This doesn't change - behavior as they have always been flattened, it just required adding the attribute to make it - future proof. +* Enum variants must be flattened. This doesn't change behavior as they have always been flattened, + it just requires adding the attribute now to make it clear what's happening and avoid a later + breakage when/if multi-field variants are supported. ## [0.14.0](https://github.com/quartiq/miniconf/compare/v0.13.0...v0.14.0) - 2024-09-26 diff --git a/miniconf_derive/src/field.rs b/miniconf_derive/src/field.rs index 52b74062..1f0fb41d 100644 --- a/miniconf_derive/src/field.rs +++ b/miniconf_derive/src/field.rs @@ -70,29 +70,29 @@ impl TreeField { } } - fn getter(&self, i: usize, is_enum: bool) -> TokenStream { + fn getter(&self, i: Option) -> TokenStream { if let Some(get) = &self.get { quote_spanned! { get.span()=> #get(self).map_err(|msg| ::miniconf::Traversal::Access(0, msg).into()) } - } else if is_enum { - quote_spanned!(self.span()=> Ok(value)) - } else { + } else if let Some(i) = i { let ident = self.ident_or_index(i); quote_spanned!(self.span()=> Ok(&self.#ident)) + } else { + quote_spanned!(self.span()=> Ok(value)) } } - fn getter_mut(&self, i: usize, is_enum: bool) -> TokenStream { + fn getter_mut(&self, i: Option) -> TokenStream { if let Some(get_mut) = &self.get_mut { quote_spanned! { get_mut.span()=> #get_mut(self).map_err(|msg| ::miniconf::Traversal::Access(0, msg).into()) } - } else if is_enum { - quote_spanned!(self.span()=> Ok(value)) - } else { + } else if let Some(i) = i { let ident = self.ident_or_index(i); quote_spanned!(self.span()=> Ok(&mut self.#ident)) + } else { + quote_spanned!(self.span()=> Ok(value)) } } @@ -120,7 +120,7 @@ impl TreeField { // Quote context is a match of the field index with `serialize_by_key()` args available. let lhs = self.lhs(i, ident); let depth = self.depth; - let getter = self.getter(i, ident.is_some()); + let getter = self.getter(ident.is_none().then_some(i)); if depth > 0 { quote_spanned! { self.span()=> #lhs => #getter @@ -143,7 +143,7 @@ impl TreeField { // Quote context is a match of the field index with `deserialize_by_key()` args available. let lhs = self.lhs(i, ident); let depth = self.depth; - let getter_mut = self.getter_mut(i, ident.is_some()); + let getter_mut = self.getter_mut(ident.is_none().then_some(i)); let validator = self.validator(); if depth > 0 { quote_spanned! { self.span()=> @@ -172,7 +172,7 @@ impl TreeField { // Quote context is a match of the field index with `get_mut_by_key()` args available. let lhs = self.lhs(i, ident); let depth = self.depth; - let getter = self.getter(i, ident.is_some()); + let getter = self.getter(ident.is_none().then_some(i)); if depth > 0 { quote_spanned! { self.span()=> #lhs => #getter @@ -189,7 +189,7 @@ impl TreeField { // Quote context is a match of the field index with `get_mut_by_key()` args available. let lhs = self.lhs(i, ident); let depth = self.depth; - let getter_mut = self.getter_mut(i, ident.is_some()); + let getter_mut = self.getter_mut(ident.is_none().then_some(i)); if depth > 0 { quote_spanned! { self.span()=> #lhs => #getter_mut diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 7a810957..8ef827db 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -56,6 +56,12 @@ impl Tree { pub fn parse(input: &syn::DeriveInput) -> Result { let mut tree = Self::from_derive_input(input)?; + if tree.flatten.is_present() { + return Err( + Error::custom("Flattening structs/enums is not yet supported.") + .with_span(&tree.flatten.span()), + ); + } match &mut tree.data { Data::Struct(fields) => { // unnamed fields can only be skipped if they are terminal From 0cf77d032daf4bd1c6e256f0c7062515bca5882a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 13:00:36 +0200 Subject: [PATCH 03/15] style, comments --- miniconf/src/key.rs | 6 +++--- miniconf/src/node.rs | 2 +- miniconf/src/packed.rs | 16 ++++++---------- miniconf_derive/src/tree.rs | 2 +- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/miniconf/src/key.rs b/miniconf/src/key.rs index cec1f95a..fdb91bba 100644 --- a/miniconf/src/key.rs +++ b/miniconf/src/key.rs @@ -28,20 +28,20 @@ pub trait KeyLookup { fn name_to_index(value: &str) -> Option; } -/// Convert a `&str` key into a node index on a `TreeKey` +/// Convert a `&str` key into a node index on a `KeyLookup` pub trait Key { /// Convert the key `self` to a `usize` index fn find(&self) -> Option; } -// `usize` index as Key +// index impl Key for usize { fn find(&self) -> Option { Some(*self) } } -// &str name as Key +// name impl Key for &str { fn find(&self) -> Option { M::name_to_index(self) diff --git a/miniconf/src/node.rs b/miniconf/src/node.rs index f5e3a12c..1255dfcd 100644 --- a/miniconf/src/node.rs +++ b/miniconf/src/node.rs @@ -86,7 +86,7 @@ impl From for usize { } } -/// Map a `TreeKey::traverse_by_key()` `Result` to a `NodeLookup::lookup()` `Result`. +/// Map a `TreeKey::traverse_by_key()` `Result` to a `Transcode::transcode()` `Result`. impl TryFrom>> for Node { type Error = Traversal; fn try_from(value: Result>) -> Result { diff --git a/miniconf/src/packed.rs b/miniconf/src/packed.rs index 55d15338..8cdd77ad 100644 --- a/miniconf/src/packed.rs +++ b/miniconf/src/packed.rs @@ -208,14 +208,12 @@ impl Packed { pub fn pop_msb(&mut self, bits: u32) -> Option { let s = self.get(); // Remove value from self - if let Some(new) = Self::new(s << bits) { + Self::new(s << bits).map(|new| { *self = new; // Extract value from old self // Done in two steps as bits + 1 can be Self::BITS which would wrap. - Some((s >> (Self::CAPACITY - bits)) >> 1) - } else { - None - } + (s >> (Self::CAPACITY - bits)) >> 1 + }) } /// Push the given number `bits` of `value` as new LSBs. @@ -229,17 +227,15 @@ impl Packed { debug_assert_eq!(value >> bits, 0); let mut n = self.trailing_zeros(); let old_marker = 1 << n; - if let Some(new_marker) = Self::new(old_marker >> bits) { + Self::new(old_marker >> bits).map(|new_marker| { n -= bits; // * Remove old marker // * Add value at offset n + 1 // Done in two steps as n + 1 can be Self::BITS, which would wrap. // * Add new marker self.0 = (self.get() ^ old_marker) | ((value << n) << 1) | new_marker.0; - Some(n) - } else { - None - } + n + }) } } diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 8ef827db..1d82368e 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -94,7 +94,7 @@ impl Tree { for v in variants.iter_mut() { if v.fields.is_struct() { // Note(design) For tuple or named struct variants we'd have to create proxy - // structs anyway (or use a FIELD_NAMES: &[&[&str]]) for KeyLookup. + // structs for KeyLookup. return Err(Error::custom( "Struct variants with named fields are not supported", ) From 9a5ff372caec92d4d975ce30039ff04bfaf79a89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 14:23:13 +0200 Subject: [PATCH 04/15] derive: fix skipped enum variant fields --- miniconf/tests/enum.rs | 14 ++++++++++++++ miniconf_derive/src/field.rs | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/miniconf/tests/enum.rs b/miniconf/tests/enum.rs index 97f827bb..5cd517f0 100644 --- a/miniconf/tests/enum.rs +++ b/miniconf/tests/enum.rs @@ -68,3 +68,17 @@ fn enum_switch() { vec!["/tag", "/en/foo", "/en/B/a"] ); } + +#[test] +fn enum_skip() { + struct S; + + #[allow(dead_code)] + #[derive(Tree)] + enum E { + #[tree(flatten)] + A(i32, #[tree(skip)] i32), + #[tree(skip)] + B(S), + } +} diff --git a/miniconf_derive/src/field.rs b/miniconf_derive/src/field.rs index 1f0fb41d..b3ffc1eb 100644 --- a/miniconf_derive/src/field.rs +++ b/miniconf_derive/src/field.rs @@ -110,7 +110,7 @@ impl TreeField { fn lhs(&self, i: usize, ident: Option<&syn::Ident>) -> TokenStream { if let Some(ident) = ident { - quote_spanned!(ident.span()=> (Self::#ident(value), #i)) + quote_spanned!(ident.span()=> (Self::#ident(value, ..), #i)) } else { quote_spanned!(self.span()=> #i) } From 30557a996426da984be58cfba543abbfb70272c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 14:56:56 +0200 Subject: [PATCH 05/15] remove flatten all over assume that single-variant enums and newtypes will always auto-flattened like newtype enum variants are already. --- CHANGELOG.md | 6 --- miniconf/tests/enum.rs | 4 +- miniconf_derive/src/lib.rs | 11 ++-- miniconf_derive/src/tree.rs | 102 +++++++++++++----------------------- 4 files changed, 44 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7560a95f..d383b392 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,12 +8,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/quartiq/miniconf/compare/v0.13.0...HEAD) - DATE -### Changed - -* Enum variants must be flattened. This doesn't change behavior as they have always been flattened, - it just requires adding the attribute now to make it clear what's happening and avoid a later - breakage when/if multi-field variants are supported. - ## [0.14.0](https://github.com/quartiq/miniconf/compare/v0.13.0...v0.14.0) - 2024-09-26 ### Added diff --git a/miniconf/tests/enum.rs b/miniconf/tests/enum.rs index 5cd517f0..e3991ee5 100644 --- a/miniconf/tests/enum.rs +++ b/miniconf/tests/enum.rs @@ -11,9 +11,8 @@ enum Enum { #[default] None, #[strum(serialize = "foo")] - #[tree(rename = "foo", flatten)] + #[tree(rename = "foo")] A(i32), - #[tree(flatten)] B(#[tree(depth = 1)] Inner), } @@ -76,7 +75,6 @@ fn enum_skip() { #[allow(dead_code)] #[derive(Tree)] enum E { - #[tree(flatten)] A(i32, #[tree(skip)] i32), #[tree(skip)] B(S), diff --git a/miniconf_derive/src/lib.rs b/miniconf_derive/src/lib.rs index afa06a26..231cf343 100644 --- a/miniconf_derive/src/lib.rs +++ b/miniconf_derive/src/lib.rs @@ -1,3 +1,4 @@ +use darling::FromDeriveInput; use proc_macro::TokenStream; use syn::{parse_macro_input, DeriveInput}; @@ -8,7 +9,7 @@ use tree::Tree; /// Derive the `TreeKey` trait for a struct. #[proc_macro_derive(TreeKey, attributes(tree))] pub fn derive_tree_key(input: TokenStream) -> TokenStream { - match Tree::parse(&parse_macro_input!(input as DeriveInput)) { + match Tree::from_derive_input(&parse_macro_input!(input as DeriveInput)) { Ok(t) => t.tree_key(), Err(e) => e.write_errors(), } @@ -18,7 +19,7 @@ pub fn derive_tree_key(input: TokenStream) -> TokenStream { /// Derive the `TreeSerialize` trait for a struct. #[proc_macro_derive(TreeSerialize, attributes(tree))] pub fn derive_tree_serialize(input: TokenStream) -> TokenStream { - match Tree::parse(&parse_macro_input!(input as DeriveInput)) { + match Tree::from_derive_input(&parse_macro_input!(input as DeriveInput)) { Ok(t) => t.tree_serialize(), Err(e) => e.write_errors(), } @@ -28,7 +29,7 @@ pub fn derive_tree_serialize(input: TokenStream) -> TokenStream { /// Derive the `TreeDeserialize` trait for a struct. #[proc_macro_derive(TreeDeserialize, attributes(tree))] pub fn derive_tree_deserialize(input: TokenStream) -> TokenStream { - match Tree::parse(&parse_macro_input!(input as DeriveInput)) { + match Tree::from_derive_input(&parse_macro_input!(input as DeriveInput)) { Ok(t) => t.tree_deserialize(), Err(e) => e.write_errors(), } @@ -38,7 +39,7 @@ pub fn derive_tree_deserialize(input: TokenStream) -> TokenStream { /// Derive the `TreeAny` trait for a struct. #[proc_macro_derive(TreeAny, attributes(tree))] pub fn derive_tree_any(input: TokenStream) -> TokenStream { - match Tree::parse(&parse_macro_input!(input as DeriveInput)) { + match Tree::from_derive_input(&parse_macro_input!(input as DeriveInput)) { Ok(t) => t.tree_any(), Err(e) => e.write_errors(), } @@ -48,7 +49,7 @@ pub fn derive_tree_any(input: TokenStream) -> TokenStream { /// Shorthand to derive the `TreeKey`, `TreeAny`, `TreeSerialize`, and `TreeDeserialize` traits for a struct. #[proc_macro_derive(Tree, attributes(tree))] pub fn derive_tree(input: TokenStream) -> TokenStream { - match Tree::parse(&parse_macro_input!(input as DeriveInput)) { + match Tree::from_derive_input(&parse_macro_input!(input as DeriveInput)) { Ok(t) => [ t.tree_key(), t.tree_any(), diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 1d82368e..45f6ef27 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -1,6 +1,6 @@ use darling::{ ast::{self, Data}, - util::{Flag, SpannedValue}, + util::Flag, Error, FromDeriveInput, FromVariant, }; use proc_macro2::TokenStream; @@ -10,17 +10,39 @@ use syn::parse_quote; use crate::field::TreeField; #[derive(Debug, FromVariant, Clone)] -#[darling(attributes(tree))] +#[darling(attributes(tree), supports(newtype, tuple, unit), and_then=Self::parse)] pub struct TreeVariant { ident: syn::Ident, rename: Option, skip: Flag, - flatten: Flag, - fields: ast::Fields>, + fields: ast::Fields, } impl TreeVariant { - fn field(&self) -> &SpannedValue { + fn parse(mut self) -> darling::Result { + assert!(!self.fields.is_struct()); + // unnamed fields can only be skipped if they are terminal + while self + .fields + .fields + .last() + .map(|f| f.skip.is_present()) + .unwrap_or_default() + { + self.fields.fields.pop(); + } + if let Some(f) = self.fields.iter().find(|f| f.skip.is_present()) { + return Err( + Error::custom("Can only `skip` terminal tuple struct fields") + .with_span(&f.skip.span()), + ); + } + Ok(self) + } +} + +impl TreeVariant { + fn field(&self) -> &TreeField { // assert!(self.fields.is_newtype()); // Don't do this since we modified it with skip assert!(self.fields.len() == 1); // Only newtypes currently self.fields.fields.first().unwrap() @@ -32,18 +54,17 @@ impl TreeVariant { } #[derive(Debug, FromDeriveInput, Clone)] -#[darling(attributes(tree))] -#[darling(supports(any))] +#[darling(attributes(tree), supports(any), and_then=Self::parse)] +#[darling()] pub struct Tree { ident: syn::Ident, - flatten: Flag, generics: syn::Generics, - data: Data, SpannedValue>, + data: Data, } impl Tree { fn depth(&self) -> usize { - let level = if self.flatten.is_present() { 0 } else { 1 }; + let level = 1; // flatten single-variant enums! let inner = match &self.data { Data::Struct(fields) => fields.fields.iter().fold(0, |d, field| d.max(field.depth)), Data::Enum(variants) => variants @@ -53,16 +74,8 @@ impl Tree { (level + inner).max(1) // We need to eat at least one level. C.f. impl TreeKey for Option. } - pub fn parse(input: &syn::DeriveInput) -> Result { - let mut tree = Self::from_derive_input(input)?; - - if tree.flatten.is_present() { - return Err( - Error::custom("Flattening structs/enums is not yet supported.") - .with_span(&tree.flatten.span()), - ); - } - match &mut tree.data { + fn parse(mut self) -> darling::Result { + match &mut self.data { Data::Struct(fields) => { // unnamed fields can only be skipped if they are terminal while fields @@ -84,64 +97,23 @@ impl Tree { .with_span(&f.skip.span()), ); } - if tree.flatten.is_present() && fields.len() > 1 { - return Err(Error::custom("Can only flatten single-field structs") - .with_span(&tree.flatten.span())); - } } Data::Enum(variants) => { variants.retain(|v| !(v.skip.is_present() || v.fields.is_unit())); - for v in variants.iter_mut() { - if v.fields.is_struct() { - // Note(design) For tuple or named struct variants we'd have to create proxy - // structs for KeyLookup. - return Err(Error::custom( - "Struct variants with named fields are not supported", - ) - .with_span(&v.span())); - } - if !v.flatten.is_present() { - // Note(design) - return Err(Error::custom( - "Enum variants must be flattened. Add `#[tree(flatten)].", - ) - .with_span(&v.span())); - } - // unnamed fields can only be skipped if they are terminal - while v - .fields - .fields - .last() - .map(|f| f.ident.is_none() && f.skip.is_present()) - .unwrap_or_default() - { - v.fields.fields.pop(); - } - if let Some(f) = v.fields.iter().find(|f| f.skip.is_present()) { - return Err( - Error::custom("Can only `skip` terminal tuple struct fields") - .with_span(&f.skip.span()), - ); - } + for v in variants.iter() { if v.fields.len() != 1 { return Err(Error::custom( "Only newtype (single field tuple) enum variants are supported.", ) - .with_span(&v.span())); + .with_span(&v.ident.span())); } } - if tree.flatten.is_present() && variants.len() > 1 { - return Err(Error::custom( - "Can only flatten enums with a single non-unit, non-skipped variant.", - ) - .with_span(&tree.flatten.span())); - } } } - Ok(tree) + Ok(self) } - fn fields(&self) -> Vec<&SpannedValue> { + fn fields(&self) -> Vec<&TreeField> { match &self.data { Data::Struct(fields) => fields.iter().collect(), Data::Enum(variants) => variants.iter().map(|v| v.field()).collect(), From bf9ad798313361d66a29b6eb42e88ee7c76ef3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 15:06:29 +0200 Subject: [PATCH 06/15] derive: style --- miniconf_derive/src/tree.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 45f6ef27..3a7b0da7 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -39,9 +39,7 @@ impl TreeVariant { } Ok(self) } -} -impl TreeVariant { fn field(&self) -> &TreeField { // assert!(self.fields.is_newtype()); // Don't do this since we modified it with skip assert!(self.fields.len() == 1); // Only newtypes currently @@ -63,17 +61,6 @@ pub struct Tree { } impl Tree { - fn depth(&self) -> usize { - let level = 1; // flatten single-variant enums! - let inner = match &self.data { - Data::Struct(fields) => fields.fields.iter().fold(0, |d, field| d.max(field.depth)), - Data::Enum(variants) => variants - .iter() - .fold(0, |d, variant| d.max(variant.field().depth)), - }; - (level + inner).max(1) // We need to eat at least one level. C.f. impl TreeKey for Option. - } - fn parse(mut self) -> darling::Result { match &mut self.data { Data::Struct(fields) => { @@ -99,7 +86,7 @@ impl Tree { } } Data::Enum(variants) => { - variants.retain(|v| !(v.skip.is_present() || v.fields.is_unit())); + variants.retain(|v| !(v.skip.is_present() || v.fields.is_empty())); for v in variants.iter() { if v.fields.len() != 1 { return Err(Error::custom( @@ -113,6 +100,12 @@ impl Tree { Ok(self) } + fn depth(&self) -> usize { + let level = 1; // flatten single-variant enums! + let inner = self.fields().iter().fold(0, |d, field| d.max(field.depth)); + (level + inner).max(1) // We need to eat at least one level. C.f. impl TreeKey for Option. + } + fn fields(&self) -> Vec<&TreeField> { match &self.data { Data::Struct(fields) => fields.iter().collect(), From 82a530810383c806e47f3f7f4c67162399c056bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 15:23:42 +0200 Subject: [PATCH 07/15] derive: simplify expand for metadata --- miniconf_derive/src/field.rs | 16 +++++++++++----- miniconf_derive/src/tree.rs | 24 ++++-------------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/miniconf_derive/src/field.rs b/miniconf_derive/src/field.rs index b3ffc1eb..586c0c34 100644 --- a/miniconf_derive/src/field.rs +++ b/miniconf_derive/src/field.rs @@ -57,16 +57,22 @@ impl TreeField { } } - pub fn metadata(&self, i: usize) -> Option { + pub fn metadata(&self, i: usize) -> TokenStream { // Quote context is a match of the field index with `metadata()` args available. let depth = self.depth; if depth > 0 { let typ = self.typ(); - Some(quote_spanned! { self.span()=> - #i => <#typ as ::miniconf::TreeKey<#depth>>::metadata() - }) + quote_spanned! { self.span()=> + let m = <#typ as ::miniconf::TreeKey<#depth>>::metadata(); + meta.max_length = meta.max_length.max(ident_len(#i) + m.max_length); + meta.max_depth = meta.max_depth.max(m.max_depth); + meta.count += m.count; + } } else { - None + quote_spanned! { self.span()=> + meta.max_length = meta.max_length.max(ident_len(#i)); + meta.count += 1; + } } } diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 3a7b0da7..c3bdc1f3 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -133,7 +133,7 @@ impl Tree { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let fields = self.fields(); let fields_len = fields.len(); - let metadata_arms = fields.iter().enumerate().filter_map(|(i, f)| f.metadata(i)); + let metadata_arms = fields.iter().enumerate().map(|(i, f)| f.metadata(i)); let traverse_arms = fields .iter() .enumerate() @@ -161,7 +161,7 @@ impl Tree { ), _ => None, }; - let (names, name_to_index, index_to_name, index_len) = if let Some(names) = names { + let (names, name_to_index, index_to_name, ident_len) = if let Some(names) = names { ( Some(quote!( const __MINICONF_NAMES: [&'static str; #fields_len] = [#(#names ,)*]; @@ -220,24 +220,8 @@ impl Tree { impl #impl_generics ::miniconf::TreeKey<#depth> for #ident #ty_generics #where_clause { fn metadata() -> ::miniconf::Metadata { let mut meta = ::miniconf::Metadata::default(); - for index in 0..#fields_len { - let item_meta = match index { - #(#metadata_arms ,)* - _ => { - let mut m = ::miniconf::Metadata::default(); - m.count = 1; - m - } - }; - meta.max_length = meta.max_length.max( - #index_len + - item_meta.max_length - ); - meta.max_depth = meta.max_depth.max( - item_meta.max_depth - ); - meta.count += item_meta.count; - } + let ident_len = |index: usize| { #ident_len }; + #(#metadata_arms)* meta.max_depth += 1; meta } From bda84c0d6a18ee0c80a5538ce1a626f0dcc5bb9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 27 Sep 2024 15:52:39 +0200 Subject: [PATCH 08/15] refactor for flattened --- miniconf_derive/src/field.rs | 44 +++++++++++----------------- miniconf_derive/src/tree.rs | 56 ++++++++++++++++++++++++------------ 2 files changed, 53 insertions(+), 47 deletions(-) diff --git a/miniconf_derive/src/field.rs b/miniconf_derive/src/field.rs index 586c0c34..fd15b71e 100644 --- a/miniconf_derive/src/field.rs +++ b/miniconf_derive/src/field.rs @@ -114,28 +114,19 @@ impl TreeField { } } - fn lhs(&self, i: usize, ident: Option<&syn::Ident>) -> TokenStream { - if let Some(ident) = ident { - quote_spanned!(ident.span()=> (Self::#ident(value, ..), #i)) - } else { - quote_spanned!(self.span()=> #i) - } - } - - pub fn serialize_by_key(&self, i: usize, ident: Option<&syn::Ident>) -> TokenStream { + pub fn serialize_by_key(&self, i: Option) -> TokenStream { // Quote context is a match of the field index with `serialize_by_key()` args available. - let lhs = self.lhs(i, ident); let depth = self.depth; - let getter = self.getter(ident.is_none().then_some(i)); + let getter = self.getter(i); if depth > 0 { quote_spanned! { self.span()=> - #lhs => #getter + #getter .and_then(|value| ::miniconf::TreeSerialize::<#depth>::serialize_by_key(value, keys, ser)) } } else { quote_spanned! { self.span()=> - #lhs => #getter + #getter .and_then(|value| ::miniconf::Serialize::serialize(value, ser) .map_err(|err| ::miniconf::Error::Inner(0, err)) @@ -145,15 +136,14 @@ impl TreeField { } } - pub fn deserialize_by_key(&self, i: usize, ident: Option<&syn::Ident>) -> TokenStream { + pub fn deserialize_by_key(&self, i: Option) -> TokenStream { // Quote context is a match of the field index with `deserialize_by_key()` args available. - let lhs = self.lhs(i, ident); let depth = self.depth; - let getter_mut = self.getter_mut(ident.is_none().then_some(i)); + let getter_mut = self.getter_mut(i); let validator = self.validator(); if depth > 0 { quote_spanned! { self.span()=> - #lhs => #getter_mut + #getter_mut .and_then(|item| ::miniconf::TreeDeserialize::<'de, #depth>::deserialize_by_key(item, keys, de) ) @@ -161,7 +151,7 @@ impl TreeField { } } else { quote_spanned! { self.span()=> - #lhs => ::miniconf::Deserialize::deserialize(de) + ::miniconf::Deserialize::deserialize(de) .map_err(|err| ::miniconf::Error::Inner(0, err)) #validator .and_then(|new| @@ -174,36 +164,34 @@ impl TreeField { } } - pub fn ref_any_by_key(&self, i: usize, ident: Option<&syn::Ident>) -> TokenStream { + pub fn ref_any_by_key(&self, i: Option) -> TokenStream { // Quote context is a match of the field index with `get_mut_by_key()` args available. - let lhs = self.lhs(i, ident); let depth = self.depth; - let getter = self.getter(ident.is_none().then_some(i)); + let getter = self.getter(i); if depth > 0 { quote_spanned! { self.span()=> - #lhs => #getter + #getter .and_then(|item| ::miniconf::TreeAny::<#depth>::ref_any_by_key(item, keys)) } } else { quote_spanned! { self.span()=> - #lhs => #getter.map(|item| item as &dyn ::core::any::Any) + #getter.map(|item| item as &dyn ::core::any::Any) } } } - pub fn mut_any_by_key(&self, i: usize, ident: Option<&syn::Ident>) -> TokenStream { + pub fn mut_any_by_key(&self, i: Option) -> TokenStream { // Quote context is a match of the field index with `get_mut_by_key()` args available. - let lhs = self.lhs(i, ident); let depth = self.depth; - let getter_mut = self.getter_mut(ident.is_none().then_some(i)); + let getter_mut = self.getter_mut(i); if depth > 0 { quote_spanned! { self.span()=> - #lhs => #getter_mut + #getter_mut .and_then(|item| ::miniconf::TreeAny::<#depth>::mut_any_by_key(item, keys)) } } else { quote_spanned! { self.span()=> - #lhs => #getter_mut.map(|item| item as &mut dyn ::core::any::Any) + #getter_mut.map(|item| item as &mut dyn ::core::any::Any) } } } diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index c3bdc1f3..b25ace42 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -270,7 +270,10 @@ impl Tree { fields .iter() .enumerate() - .map(|(i, f)| f.serialize_by_key(i, None)) + .map(|(i, f)| { + let rhs = f.serialize_by_key(Some(i)); + quote!(#i => #rhs) + }) .collect(), quote!(unreachable!()), ), @@ -279,7 +282,11 @@ impl Tree { variants .iter() .enumerate() - .map(|(i, v)| v.field().serialize_by_key(i, Some(&v.ident))) + .map(|(i, v)| { + let ident = &v.ident; + let rhs = v.field().serialize_by_key(None); + quote!((Self::#ident(value, ..), #i) => #rhs) + }) .collect(), quote!(Err(::miniconf::Traversal::Absent(0).into())), ), @@ -336,7 +343,10 @@ impl Tree { fields .iter() .enumerate() - .map(|(i, f)| f.deserialize_by_key(i, None)) + .map(|(i, f)| { + let rhs = f.deserialize_by_key(Some(i)); + quote!(#i => #rhs) + }) .collect(), quote!(unreachable!()), ), @@ -345,7 +355,11 @@ impl Tree { variants .iter() .enumerate() - .map(|(i, v)| v.field().deserialize_by_key(i, Some(&v.ident))) + .map(|(i, v)| { + let ident = &v.ident; + let rhs = v.field().deserialize_by_key(None); + quote!((Self::#ident(value, ..), #i) => #rhs) + }) .collect(), quote!(Err(::miniconf::Traversal::Absent(0).into())), ), @@ -384,19 +398,18 @@ impl Tree { let ident = &self.ident; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let (mat, ref_arms, mut_arms, default): (_, Vec<_>, Vec<_>, _) = match &self.data { + let (mat, (ref_arms, mut_arms), default): (_, (Vec<_>, Vec<_>), _) = match &self.data { Data::Struct(fields) => ( quote!(index), fields .iter() .enumerate() - .map(|(i, f)| f.ref_any_by_key(i, None)) - .collect(), - fields - .iter() - .enumerate() - .map(|(i, f)| f.mut_any_by_key(i, None)) - .collect(), + .map(|(i, f)| { + let (ref_rhs, mut_rhs) = + (f.ref_any_by_key(Some(i)), f.mut_any_by_key(Some(i))); + (quote!(#i => #ref_rhs), quote!(#i => #mut_rhs)) + }) + .unzip(), quote!(unreachable!()), ), Data::Enum(variants) => ( @@ -404,13 +417,18 @@ impl Tree { variants .iter() .enumerate() - .map(|(i, v)| v.field().ref_any_by_key(i, Some(&v.ident))) - .collect(), - variants - .iter() - .enumerate() - .map(|(i, v)| v.field().mut_any_by_key(i, Some(&v.ident))) - .collect(), + .map(|(i, v)| { + let ident = &v.ident; + let (ref_rhs, mut_rhs) = ( + v.field().ref_any_by_key(None), + v.field().mut_any_by_key(None), + ); + ( + quote!((Self::#ident(value, ..), #i) => #ref_rhs), + quote!((Self::#ident(value, ..), #i) => #mut_rhs), + ) + }) + .unzip(), quote!(Err(::miniconf::Traversal::Absent(0).into())), ), }; From cd5b392e09456a37b8cec5b8eb38c1f50075476e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Sat, 28 Sep 2024 15:21:18 +0200 Subject: [PATCH 09/15] derive: support flatten --- CHANGELOG.md | 4 ++ miniconf/Cargo.toml | 4 ++ miniconf/tests/common/mod.rs | 26 ++++++++++++ miniconf/tests/enum.rs | 26 +++++------- miniconf/tests/flatten.rs | 73 +++++++++++++++++++++++++++++++++ miniconf_derive/src/tree.rs | 78 +++++++++++++++++++++++++++--------- 6 files changed, 177 insertions(+), 34 deletions(-) create mode 100644 miniconf/tests/common/mod.rs create mode 100644 miniconf/tests/flatten.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index d383b392..989d847d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/quartiq/miniconf/compare/v0.13.0...HEAD) - DATE +### Added + +* `flatten` support for structs/enums with a single non-skip/non-unit variant/field. + ## [0.14.0](https://github.com/quartiq/miniconf/compare/v0.13.0...v0.14.0) - 2024-09-26 ### Added diff --git a/miniconf/Cargo.toml b/miniconf/Cargo.toml index bbf24d3d..b6c99729 100644 --- a/miniconf/Cargo.toml +++ b/miniconf/Cargo.toml @@ -81,6 +81,10 @@ required-features = ["json-core", "derive"] name = "validate" required-features = ["json-core", "derive"] +[[test]] +name = "flatten" +required-features = ["json-core", "derive"] + [[example]] name = "common" crate-type = ["lib"] diff --git a/miniconf/tests/common/mod.rs b/miniconf/tests/common/mod.rs new file mode 100644 index 00000000..d21cd196 --- /dev/null +++ b/miniconf/tests/common/mod.rs @@ -0,0 +1,26 @@ +use miniconf::{JsonCoreSlashOwned, Path, TreeKey}; + +pub fn paths() -> Vec +where + M: TreeKey, +{ + M::nodes::>() + .exact_size() + .map(|pn| { + let (p, n) = pn.unwrap(); + assert!(n.is_leaf()); + assert_eq!(p.chars().filter(|c| *c == p.separator()).count(), n.depth()); + p.into_inner() + }) + .collect() +} + +pub fn set_get(s: &mut M, path: &str, value: &[u8]) +where + M: JsonCoreSlashOwned, +{ + s.set_json(path, value).unwrap(); + let mut buf = vec![0; value.len()]; + let len = s.get_json(path, &mut buf[..]).unwrap(); + assert_eq!(&buf[..len], value); +} diff --git a/miniconf/tests/enum.rs b/miniconf/tests/enum.rs index e3991ee5..36bc06df 100644 --- a/miniconf/tests/enum.rs +++ b/miniconf/tests/enum.rs @@ -1,6 +1,9 @@ -use miniconf::{JsonCoreSlash, Path, Tree, TreeDeserialize, TreeKey, TreeSerialize}; +use miniconf::{JsonCoreSlash, Tree, TreeDeserialize, TreeKey, TreeSerialize}; use strum::{AsRefStr, EnumString}; +mod common; +use common::*; + #[derive(Tree, Default, PartialEq, Debug)] struct Inner { a: i32, @@ -39,33 +42,23 @@ impl Settings { fn enum_switch() { let mut s = Settings::default(); assert_eq!(s.en, Enum::None); - s.set_json("/tag", b"\"foo\"").unwrap(); + set_get(&mut s, "/tag", b"\"foo\""); assert_eq!( s.set_json("/tag", b"\"bar\""), Err(miniconf::Traversal::Invalid(1, "invalid tag").into()) ); assert_eq!(s.en, Enum::A(0)); - s.set_json("/en/foo", b"99").unwrap(); + set_get(&mut s, "/en/foo", b"99"); assert_eq!(s.en, Enum::A(99)); assert_eq!( s.set_json("/en/B/a", b"99"), Err(miniconf::Traversal::Absent(2).into()) ); - s.set_json("/tag", b"\"B\"").unwrap(); - s.set_json("/en/B/a", b"8").unwrap(); + set_get(&mut s, "/tag", b"\"B\""); + set_get(&mut s, "/en/B/a", b"8"); assert_eq!(s.en, Enum::B(Inner { a: 8 })); - assert_eq!( - Settings::nodes::>() - .exact_size() - .map(|pn| { - let (p, n) = pn.unwrap(); - assert!(n.is_leaf()); - p.into_inner() - }) - .collect::>(), - vec!["/tag", "/en/foo", "/en/B/a"] - ); + assert_eq!(paths::(), ["/tag", "/en/foo", "/en/B/a"]); } #[test] @@ -79,4 +72,5 @@ fn enum_skip() { #[tree(skip)] B(S), } + assert_eq!(paths::(), ["/A"]); } diff --git a/miniconf/tests/flatten.rs b/miniconf/tests/flatten.rs new file mode 100644 index 00000000..b82f4253 --- /dev/null +++ b/miniconf/tests/flatten.rs @@ -0,0 +1,73 @@ +use miniconf::{JsonCoreSlash, Tree}; + +mod common; +use common::*; + +#[derive(Tree, Default, PartialEq, Debug)] +struct Inner { + a: i32, +} + +#[test] +fn struct_flatten() { + #[derive(Tree, Default, PartialEq, Debug)] + #[tree(flatten)] + struct S1 { + a: i32, + } + assert_eq!(paths::(), [""]); + let mut s = S1::default(); + set_get(&mut s, "", b"1"); + assert_eq!(s.a, 1); + + #[derive(Tree, Default, PartialEq, Debug)] + #[tree(flatten)] + struct S2(i32); + assert_eq!(paths::(), [""]); + let mut s = S2::default(); + set_get(&mut s, "", b"1"); + assert_eq!(s.0, 1); + + #[derive(Tree, Default, PartialEq, Debug)] + #[tree(flatten)] + struct S3(#[tree(depth = 1)] Inner); + assert_eq!(paths::(), ["/a"]); + let mut s = S3::default(); + set_get(&mut s, "/a", b"1"); + assert_eq!(s.0.a, 1); +} + +#[test] +fn enum_flatten() { + #[derive(Tree, Default, PartialEq, Debug)] + #[tree(flatten)] + enum E1 { + #[default] + None, + A(i32), + } + assert_eq!(paths::(), [""]); + let mut e = E1::A(0); + set_get(&mut e, "", b"1"); + assert_eq!(e, E1::A(1)); + assert_eq!( + E1::None.set_json("", b"1").unwrap_err(), + miniconf::Traversal::Absent(0).into() + ); + + #[derive(Tree, Default, PartialEq, Debug)] + #[tree(flatten)] + enum E2 { + #[default] + None, + A(#[tree(depth = 1)] Inner), + } + assert_eq!(paths::(), ["/a"]); + let mut e = E2::A(Inner::default()); + set_get(&mut e, "/a", b"1"); + assert_eq!(e, E2::A(Inner { a: 1 })); + assert_eq!( + E2::None.set_json("/a", b"1").unwrap_err(), + miniconf::Traversal::Absent(0).into() + ); +} diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index b25ace42..bf9b7220 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -57,6 +57,7 @@ impl TreeVariant { pub struct Tree { ident: syn::Ident, generics: syn::Generics, + flatten: Flag, data: Data, } @@ -97,13 +98,24 @@ impl Tree { } } } + if self.flatten.is_present() && self.fields().len() != 1 { + return Err(Error::custom("Can't flatten multiple fields/variants") + .with_span(&self.flatten.span())); + } Ok(self) } + fn level(&self) -> usize { + if self.flatten.is_present() { + 0 + } else { + 1 + } + } + fn depth(&self) -> usize { - let level = 1; // flatten single-variant enums! let inner = self.fields().iter().fold(0, |d, field| d.max(field.depth)); - (level + inner).max(1) // We need to eat at least one level. C.f. impl TreeKey for Option. + (self.level() + inner).max(1) // We need to eat at least one level. C.f. impl TreeKey for Option. } fn fields(&self) -> Vec<&TreeField> { @@ -161,7 +173,7 @@ impl Tree { ), _ => None, }; - let (names, name_to_index, index_to_name, ident_len) = if let Some(names) = names { + let (names, name_to_index, index_to_name, mut ident_len) = if let Some(names) = names { ( Some(quote!( const __MINICONF_NAMES: [&'static str; #fields_len] = [#(#names ,)*]; @@ -187,13 +199,29 @@ impl Tree { ) }; + let level = self.level(); + let (index, traverse, increment) = if self.flatten.is_present() { + ident_len = quote!(0); + (quote!(0), quote!(), quote!()) + } else { + ( + quote!(::miniconf::Keys::next::(&mut keys)?), + quote! { + func(index, #index_to_name, #fields_len).map_err(|err| ::miniconf::Error::Inner(1, err))?; + }, + quote!(::miniconf::Error::increment_result), + ) + }; + + // FIXME: don't increment result in flatten in other traits + quote! { #[automatically_derived] impl #impl_generics #ident #ty_generics #where_clause { // TODO: can these be hidden and disambiguated w.r.t. collision? - fn __miniconf_lookup(keys: &mut K) -> Result { + fn __miniconf_lookup(mut keys: K) -> Result { const DEFERS: [bool; #fields_len] = [#(#defers ,)*]; - let index = ::miniconf::Keys::next::(keys)?; + let index = #index; let defer = DEFERS.get(index) .ok_or(::miniconf::Traversal::NotFound(1))?; if !defer && !keys.finalize() { @@ -222,22 +250,18 @@ impl Tree { let mut meta = ::miniconf::Metadata::default(); let ident_len = |index: usize| { #ident_len }; #(#metadata_arms)* - meta.max_depth += 1; + meta.max_depth += #level; meta } - fn traverse_by_key( - mut keys: K, - mut func: F, - ) -> Result> + fn traverse_by_key(mut keys: K, mut func: F) -> Result> where K: ::miniconf::Keys, F: FnMut(usize, Option<&'static str>, usize) -> Result<(), E>, { - let index = ::miniconf::Keys::next::(&mut keys)?; - let name = #index_to_name; - func(index, name, #fields_len).map_err(|err| ::miniconf::Error::Inner(1, err))?; - ::miniconf::Error::increment_result(match index { + let index = #index; + #traverse + #increment(match index { #(#traverse_arms ,)* _ => { if !keys.finalize() { @@ -292,6 +316,12 @@ impl Tree { ), }; + let increment = if self.flatten.is_present() { + quote!() + } else { + quote!(::miniconf::Error::increment_result) + }; + quote! { #[automatically_derived] impl #impl_generics ::miniconf::TreeSerialize<#depth> for #ident #ty_generics #where_clause { @@ -303,7 +333,7 @@ impl Tree { let index = Self::__miniconf_lookup(&mut keys)?; // Note(unreachable) empty structs have diverged by now #[allow(unreachable_code)] - ::miniconf::Error::increment_result(match #mat { + #increment(match #mat { #(#arms ,)* _ => #default }) @@ -365,6 +395,12 @@ impl Tree { ), }; + let increment = if self.flatten.is_present() { + quote!() + } else { + quote!(::miniconf::Error::increment_result) + }; + quote! { #[automatically_derived] impl #impl_generics ::miniconf::TreeDeserialize<'de, #depth> for #ident #ty_generics #where_clause { @@ -376,7 +412,7 @@ impl Tree { let index = Self::__miniconf_lookup(&mut keys)?; // Note(unreachable) empty structs have diverged by now #[allow(unreachable_code)] - ::miniconf::Error::increment_result(match #mat { + #increment(match #mat { #(#arms ,)* _ => #default }) @@ -433,6 +469,12 @@ impl Tree { ), }; + let increment = if self.flatten.is_present() { + quote!(ret) + } else { + quote!(ret.map_err(::miniconf::Traversal::increment)) + }; + quote! { #[automatically_derived] impl #impl_generics ::miniconf::TreeAny<#depth> for #ident #ty_generics #where_clause { @@ -448,7 +490,7 @@ impl Tree { #(#ref_arms ,)* _ => #default }; - ret.map_err(::miniconf::Traversal::increment) + #increment } } @@ -464,7 +506,7 @@ impl Tree { #(#mut_arms ,)* _ => #default }; - ret.map_err(::miniconf::Traversal::increment) + #increment } } } From 4f2de58e35abf757c5ca99e0e1c28c8cf6954e6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Sun, 29 Sep 2024 03:06:00 +0200 Subject: [PATCH 10/15] add Option like integration test --- miniconf/tests/enum.rs | 13 +++++++++++++ miniconf_derive/src/tree.rs | 16 +++++++--------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/miniconf/tests/enum.rs b/miniconf/tests/enum.rs index 36bc06df..223b7934 100644 --- a/miniconf/tests/enum.rs +++ b/miniconf/tests/enum.rs @@ -74,3 +74,16 @@ fn enum_skip() { } assert_eq!(paths::(), ["/A"]); } + +#[test] +fn option() { + #[allow(dead_code)] + #[derive(Tree, Copy, Clone, PartialEq, Default, Debug)] + #[tree(flatten)] + enum MOption { + #[default] + None, + Some(#[tree(depth = 1)] T), + } + assert_eq!(paths::, 1>(), ["/0"]); +} diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index bf9b7220..44636ba6 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -138,6 +138,7 @@ impl Tree { pub fn tree_key(&self) -> TokenStream { let depth = self.depth(); + let level = self.level(); let ident = &self.ident; let generics = self.bound_generics(&mut |depth| { (depth > 0).then_some(parse_quote!(::miniconf::TreeKey<#depth>)) @@ -199,7 +200,6 @@ impl Tree { ) }; - let level = self.level(); let (index, traverse, increment) = if self.flatten.is_present() { ident_len = quote!(0); (quote!(0), quote!(), quote!()) @@ -213,8 +213,6 @@ impl Tree { ) }; - // FIXME: don't increment result in flatten in other traits - quote! { #[automatically_derived] impl #impl_generics #ident #ty_generics #where_clause { @@ -288,7 +286,7 @@ impl Tree { }); let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let (mat, arms, default): (_, Vec<_>, _) = match &self.data { + let (mat, arms, default) = match &self.data { Data::Struct(fields) => ( quote!(index), fields @@ -298,7 +296,7 @@ impl Tree { let rhs = f.serialize_by_key(Some(i)); quote!(#i => #rhs) }) - .collect(), + .collect::>(), quote!(unreachable!()), ), Data::Enum(variants) => ( @@ -367,7 +365,7 @@ impl Tree { } let (impl_generics, _, _) = generics.split_for_impl(); - let (mat, arms, default): (_, Vec<_>, _) = match &self.data { + let (mat, arms, default) = match &self.data { Data::Struct(fields) => ( quote!(index), fields @@ -377,7 +375,7 @@ impl Tree { let rhs = f.deserialize_by_key(Some(i)); quote!(#i => #rhs) }) - .collect(), + .collect::>(), quote!(unreachable!()), ), Data::Enum(variants) => ( @@ -434,7 +432,7 @@ impl Tree { let ident = &self.ident; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let (mat, (ref_arms, mut_arms), default): (_, (Vec<_>, Vec<_>), _) = match &self.data { + let (mat, (ref_arms, mut_arms), default) = match &self.data { Data::Struct(fields) => ( quote!(index), fields @@ -445,7 +443,7 @@ impl Tree { (f.ref_any_by_key(Some(i)), f.mut_any_by_key(Some(i))); (quote!(#i => #ref_rhs), quote!(#i => #mut_rhs)) }) - .unzip(), + .unzip::<_, _, Vec<_>, Vec<_>>(), quote!(unreachable!()), ), Data::Enum(variants) => ( From c6142b4f081e98f3e213d7694395f40988ac2fd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 30 Sep 2024 00:27:34 +0200 Subject: [PATCH 11/15] docs --- miniconf/README.md | 22 ++++++++++------------ miniconf/src/error.rs | 9 +++++---- miniconf/src/tree.rs | 3 --- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/miniconf/README.md b/miniconf/README.md index 1f4e1c32..967ebe5c 100644 --- a/miniconf/README.md +++ b/miniconf/README.md @@ -130,7 +130,7 @@ One possible use of `miniconf` is a backend for run-time settings management in It was originally designed to work with JSON ([`serde_json_core`](https://docs.rs/serde-json-core)) payloads over MQTT ([`minimq`](https://docs.rs/minimq)) and provides a MQTT settings management client in the `miniconf_mqtt` crate and a Python reference implementation to interact with it. -Miniconf is agnostic of the `serde` backend/format, hierarchy separator, and transport/protocol. +Miniconf is agnostic of the `serde` backend/format, key type/format, and transport/protocol. ## Formats @@ -162,11 +162,11 @@ python -m miniconf -d quartiq/application/+ /foo=true ## Derive macros -For structs `miniconf` offers derive macros for [`macro@TreeKey`], [`macro@TreeSerialize`], and [`macro@TreeDeserialize`]. -The macros implements the [`TreeKey`], [`TreeSerialize`], and [`TreeDeserialize`] traits. -Fields/items that form internal nodes (non-leaf) need to implement the respective `Tree{Key,Serialize,Deserialize}` trait. -Leaf fields/items need to support the respective [`serde`] trait (and the desired `serde::Serializer`/`serde::Deserializer` -backend). +For structs `miniconf` offers derive macros for [`macro@TreeKey`], [`macro@TreeSerialize`], [`macro@TreeDeserialize`], and [`macro@TreeAny`]. +The macros implements the [`TreeKey`], [`TreeSerialize`], [`TreeDeserialize`], and [`TreeAny`] traits. +Fields/variants that form internal nodes (non-leaf) need to implement the respective `Tree{Key,Serialize,Deserialize,Any}` trait. +Leaf fields/items need to support the respective [`serde`] (and the desired `serde::Serializer`/`serde::Deserializer` +backend) or [`core::any`] trait. Structs, enums, arrays, and Options can then be cascaded to construct more complex trees. When using the derive macro, the behavior and tree recursion depth can be configured for each @@ -180,18 +180,16 @@ Lookup into the tree is done using a [`Keys`] implementation. A blanket implemen is provided for `IntoIterator`s over [`Key`] items. The [`Key`] lookup capability is implemented for `usize` indices and `&str` names. -Path iteration is supported with arbitrary separator between names. +Path iteration is supported with arbitrary separator `char`s between names. Very compact hierarchical indices encodings can be obtained from the [`Packed`] structure. It implements [`Keys`]. ## Limitations -The derive macros don't support enums with record (named fields) variants or tuple (non-newtype) variants. -These are still however usable in their atomic `serde` form as leaf nodes. - -The derive macros don't handle `std`/`alloc` smart pointers ( `Box`, `Rc`, `Arc`) in any special way. -They however still be handled with accessors (`get`, `get_mut`, `validate`). +* `enum`: The derive macros don't support enums with record (named fields) variants or tuple variants with more than one field. Only unit, newtype and skipped variants are supported. Without the derive macros, these `enums` are still however usable in their atomic `serde` form as leaf nodes. +* The derive macros don't handle `std`/`alloc` smart pointers ( `Box`, `Rc`, `Arc`) in any special way. They however still be handled with accessors (`get`, `get_mut`, `validate`). +* The derive macros only support flattening in non-ambiguous situations (single field structs and single variant enums, both modulo skipped fields/variants and unit variants). ## Features diff --git a/miniconf/src/error.rs b/miniconf/src/error.rs index da0d7fff..7b6ea0ee 100644 --- a/miniconf/src/error.rs +++ b/miniconf/src/error.rs @@ -13,10 +13,11 @@ use core::fmt::{Display, Formatter}; #[non_exhaustive] #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Traversal { - /// The key is valid, but does not exist at runtime. + /// The does not exist at runtime. /// - /// This is the case if an [`Option`] using the `Tree*` traits - /// is `None` at runtime. See also [`crate::TreeKey#option`]. + /// The `enum` variant is currently absent. + /// This is for example the case if an [`Option`] using the `Tree*` + /// traits is `None` at runtime. See also [`crate::TreeKey#option`]. Absent(usize), /// The key ends early and does not reach a leaf node. @@ -44,7 +45,7 @@ impl Display for Traversal { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { match self { Traversal::Absent(depth) => { - write!(f, "Key not currently present (depth: {depth})") + write!(f, "Variant absent (depth: {depth})") } Traversal::TooShort(depth) => { write!(f, "Key too short (depth: {depth})") diff --git a/miniconf/src/tree.rs b/miniconf/src/tree.rs index ce811f41..e4f5a6f8 100644 --- a/miniconf/src/tree.rs +++ b/miniconf/src/tree.rs @@ -278,9 +278,6 @@ impl Metadata { /// to access it (e.g. [`TreeSerialize::serialize_by_key()`], [`TreeDeserialize::deserialize_by_key()`], /// [`TreeAny::ref_any_by_key()`], or [`TreeAny::mut_any_by_key()`]) /// return the special [`Traversal::Absent`]. -/// This is intended as a mechanism to provide run-time construction of the namespace. In some -/// cases, run-time detection may indicate that some component is not present. In this case, -/// the nodes will not be exposed for serialization/deserialization. /// /// If the depth specified by the `#[tree(depth=Y)]` attribute exceeds 1, /// the `Option` can be used to access the inner type using its `TreeKey<{Y - 1}>` trait. From c9c7784875f288e44a7f09e3be29df7243adf210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 30 Sep 2024 12:43:50 +0200 Subject: [PATCH 12/15] derive: be more hygienic --- miniconf/tests/enum.rs | 7 ++- miniconf_derive/src/field.rs | 10 ++-- miniconf_derive/src/tree.rs | 88 ++++++++++++++++++++---------------- 3 files changed, 60 insertions(+), 45 deletions(-) diff --git a/miniconf/tests/enum.rs b/miniconf/tests/enum.rs index 223b7934..09f798f8 100644 --- a/miniconf/tests/enum.rs +++ b/miniconf/tests/enum.rs @@ -77,13 +77,16 @@ fn enum_skip() { #[test] fn option() { + // Also tests macro hygiene a bit #[allow(dead_code)] #[derive(Tree, Copy, Clone, PartialEq, Default, Debug)] #[tree(flatten)] - enum MOption { + enum Option { #[default] None, + // #192 Some(#[tree(depth = 1)] T), } - assert_eq!(paths::, 1>(), ["/0"]); + assert_eq!(paths::, 1>(), ["/0"]); + assert_eq!(paths::>, 1>(), [""]); } diff --git a/miniconf_derive/src/field.rs b/miniconf_derive/src/field.rs index fd15b71e..aaa52654 100644 --- a/miniconf_derive/src/field.rs +++ b/miniconf_derive/src/field.rs @@ -83,9 +83,9 @@ impl TreeField { } } else if let Some(i) = i { let ident = self.ident_or_index(i); - quote_spanned!(self.span()=> Ok(&self.#ident)) + quote_spanned!(self.span()=> ::core::result::Result::Ok(&self.#ident)) } else { - quote_spanned!(self.span()=> Ok(value)) + quote_spanned!(self.span()=> ::core::result::Result::Ok(value)) } } @@ -96,9 +96,9 @@ impl TreeField { } } else if let Some(i) = i { let ident = self.ident_or_index(i); - quote_spanned!(self.span()=> Ok(&mut self.#ident)) + quote_spanned!(self.span()=> ::core::result::Result::Ok(&mut self.#ident)) } else { - quote_spanned!(self.span()=> Ok(value)) + quote_spanned!(self.span()=> ::core::result::Result::Ok(value)) } } @@ -130,7 +130,7 @@ impl TreeField { .and_then(|value| ::miniconf::Serialize::serialize(value, ser) .map_err(|err| ::miniconf::Error::Inner(0, err)) - .and(Ok(0)) + .and(::core::result::Result::Ok(0)) ) } } diff --git a/miniconf_derive/src/tree.rs b/miniconf_derive/src/tree.rs index 44636ba6..a8c3e8f3 100644 --- a/miniconf_derive/src/tree.rs +++ b/miniconf_derive/src/tree.rs @@ -180,7 +180,7 @@ impl Tree { const __MINICONF_NAMES: [&'static str; #fields_len] = [#(#names ,)*]; )), quote!(Self::__MINICONF_NAMES.iter().position(|&n| n == value)), - quote!(Some( + quote!(::core::option::Option::Some( *Self::__MINICONF_NAMES .get(index) .ok_or(::miniconf::Traversal::NotFound(1))? @@ -192,24 +192,40 @@ impl Tree { None, quote!(str::parse(value).ok()), quote!(if index >= #fields_len { - Err(::miniconf::Traversal::NotFound(1))? + ::core::result::Result::Err(::miniconf::Traversal::NotFound(1))? } else { - None + ::core::option::Option::None }), quote!(index.checked_ilog10().unwrap_or_default() as usize + 1), ) }; - let (index, traverse, increment) = if self.flatten.is_present() { + let (index, traverse, increment, lookup) = if self.flatten.is_present() { ident_len = quote!(0); - (quote!(0), quote!(), quote!()) + (quote!(0), None, None, None) } else { ( quote!(::miniconf::Keys::next::(&mut keys)?), - quote! { + Some(quote! { func(index, #index_to_name, #fields_len).map_err(|err| ::miniconf::Error::Inner(1, err))?; - }, - quote!(::miniconf::Error::increment_result), + }), + Some(quote!(::miniconf::Error::increment_result)), + Some(quote! { + #[automatically_derived] + impl #impl_generics #ident #ty_generics #where_clause { + #names + } + + #[automatically_derived] + impl #impl_generics ::miniconf::KeyLookup for #ident #ty_generics #where_clause { + const LEN: usize = #fields_len; + + #[inline] + fn name_to_index(value: &str) -> ::core::option::Option { + #name_to_index + } + } + }), ) }; @@ -217,30 +233,20 @@ impl Tree { #[automatically_derived] impl #impl_generics #ident #ty_generics #where_clause { // TODO: can these be hidden and disambiguated w.r.t. collision? - fn __miniconf_lookup(mut keys: K) -> Result { + fn __miniconf_lookup(mut keys: K) -> ::core::result::Result { const DEFERS: [bool; #fields_len] = [#(#defers ,)*]; let index = #index; let defer = DEFERS.get(index) .ok_or(::miniconf::Traversal::NotFound(1))?; if !defer && !keys.finalize() { - Err(::miniconf::Traversal::TooLong(1)) + ::core::result::Result::Err(::miniconf::Traversal::TooLong(1)) } else { - Ok(index) + ::core::result::Result::Ok(index) } } - - #names } - #[automatically_derived] - impl #impl_generics ::miniconf::KeyLookup for #ident #ty_generics #where_clause { - const LEN: usize = #fields_len; - - #[inline] - fn name_to_index(value: &str) -> Option { - #name_to_index - } - } + #lookup #[automatically_derived] impl #impl_generics ::miniconf::TreeKey<#depth> for #ident #ty_generics #where_clause { @@ -252,10 +258,10 @@ impl Tree { meta } - fn traverse_by_key(mut keys: K, mut func: F) -> Result> + fn traverse_by_key(mut keys: K, mut func: F) -> ::core::result::Result> where K: ::miniconf::Keys, - F: FnMut(usize, Option<&'static str>, usize) -> Result<(), E>, + F: ::core::ops::FnMut(usize, ::core::option::Option<&'static str>, usize) -> ::core::result::Result<(), E>, { let index = #index; #traverse @@ -263,9 +269,9 @@ impl Tree { #(#traverse_arms ,)* _ => { if !keys.finalize() { - Err(::miniconf::Traversal::TooLong(0).into()) + ::core::result::Result::Err(::miniconf::Traversal::TooLong(0).into()) } else { - Ok(0) + ::core::result::Result::Ok(0) } } }) @@ -297,7 +303,7 @@ impl Tree { quote!(#i => #rhs) }) .collect::>(), - quote!(unreachable!()), + quote!(::core::unreachable!()), ), Data::Enum(variants) => ( quote!((self, index)), @@ -310,7 +316,9 @@ impl Tree { quote!((Self::#ident(value, ..), #i) => #rhs) }) .collect(), - quote!(Err(::miniconf::Traversal::Absent(0).into())), + quote!(::core::result::Result::Err( + ::miniconf::Traversal::Absent(0).into() + )), ), }; @@ -323,7 +331,7 @@ impl Tree { quote! { #[automatically_derived] impl #impl_generics ::miniconf::TreeSerialize<#depth> for #ident #ty_generics #where_clause { - fn serialize_by_key(&self, mut keys: K, ser: S) -> Result> + fn serialize_by_key(&self, mut keys: K, ser: S) -> ::core::result::Result> where K: ::miniconf::Keys, S: ::miniconf::Serializer, @@ -376,7 +384,7 @@ impl Tree { quote!(#i => #rhs) }) .collect::>(), - quote!(unreachable!()), + quote!(::core::unreachable!()), ), Data::Enum(variants) => ( quote!((self, index)), @@ -389,7 +397,9 @@ impl Tree { quote!((Self::#ident(value, ..), #i) => #rhs) }) .collect(), - quote!(Err(::miniconf::Traversal::Absent(0).into())), + quote!(::core::result::Result::Err( + ::miniconf::Traversal::Absent(0).into() + )), ), }; @@ -402,7 +412,7 @@ impl Tree { quote! { #[automatically_derived] impl #impl_generics ::miniconf::TreeDeserialize<'de, #depth> for #ident #ty_generics #where_clause { - fn deserialize_by_key(&mut self, mut keys: K, de: D) -> Result> + fn deserialize_by_key(&mut self, mut keys: K, de: D) -> ::core::result::Result> where K: ::miniconf::Keys, D: ::miniconf::Deserializer<'de>, @@ -444,7 +454,7 @@ impl Tree { (quote!(#i => #ref_rhs), quote!(#i => #mut_rhs)) }) .unzip::<_, _, Vec<_>, Vec<_>>(), - quote!(unreachable!()), + quote!(::core::unreachable!()), ), Data::Enum(variants) => ( quote!((self, index)), @@ -463,7 +473,9 @@ impl Tree { ) }) .unzip(), - quote!(Err(::miniconf::Traversal::Absent(0).into())), + quote!(::core::result::Result::Err( + ::miniconf::Traversal::Absent(0).into() + )), ), }; @@ -476,7 +488,7 @@ impl Tree { quote! { #[automatically_derived] impl #impl_generics ::miniconf::TreeAny<#depth> for #ident #ty_generics #where_clause { - fn ref_any_by_key(&self, mut keys: K) -> Result<&dyn ::core::any::Any, ::miniconf::Traversal> + fn ref_any_by_key(&self, mut keys: K) -> ::core::result::Result<&dyn ::core::any::Any, ::miniconf::Traversal> where K: ::miniconf::Keys, { @@ -484,7 +496,7 @@ impl Tree { // Note(unreachable) empty structs have diverged by now #[allow(unreachable_code)] { - let ret: Result<_, _> = match #mat { + let ret: ::core::result::Result<_, _> = match #mat { #(#ref_arms ,)* _ => #default }; @@ -492,7 +504,7 @@ impl Tree { } } - fn mut_any_by_key(&mut self, mut keys: K) -> Result<&mut dyn ::core::any::Any, ::miniconf::Traversal> + fn mut_any_by_key(&mut self, mut keys: K) -> ::core::result::Result<&mut dyn ::core::any::Any, ::miniconf::Traversal> where K: ::miniconf::Keys, { @@ -500,7 +512,7 @@ impl Tree { // Note(unreachable) empty structs have diverged by now #[allow(unreachable_code)] { - let ret: Result<_, _> = match #mat { + let ret: ::core::result::Result<_, _> = match #mat { #(#mut_arms ,)* _ => #default }; From d0432e8936bea3a143bb2aa5058ff6c54d446341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 30 Sep 2024 12:49:24 +0200 Subject: [PATCH 13/15] add core::erro::Error impls --- miniconf/src/error.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/miniconf/src/error.rs b/miniconf/src/error.rs index 7b6ea0ee..84cc1f0c 100644 --- a/miniconf/src/error.rs +++ b/miniconf/src/error.rs @@ -1,4 +1,4 @@ -use core::fmt::{Display, Formatter}; +use core::fmt::{Debug, Display, Formatter}; /// Errors that can occur when using the Tree traits. /// @@ -66,6 +66,8 @@ impl Display for Traversal { } } +impl ::core::error::Error for Traversal {} + impl Traversal { /// Pass it up one hierarchy depth level, incrementing its usize depth field by one. #[inline] @@ -117,10 +119,10 @@ pub enum Error { Finalization(E), } -impl Display for Error { +impl Display for Error { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { match self { - Self::Traversal(t) => t.fmt(f), + Self::Traversal(t) => Display::fmt(t, f), Self::Inner(depth, error) => { write!(f, "(De)serialization error (depth: {depth}): {error}") } @@ -131,6 +133,15 @@ impl Display for Error { } } +impl core::error::Error for Error { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Self::Traversal(t) => Some(t), + Self::Inner(_, e) | Self::Finalization(e) => Some(e), + } + } +} + // Try to extract the Traversal from an Error impl TryFrom> for Traversal { type Error = Error; From 8ba589cba729f55a51691513beb8b97a48bc1fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 30 Sep 2024 13:50:18 +0200 Subject: [PATCH 14/15] add trybuild ui tests --- miniconf/Cargo.toml | 1 + miniconf/src/error.rs | 8 ++++---- miniconf/tests/compiletest.rs | 5 +++++ miniconf/tests/skipped.rs | 17 +++++++++++++++++ miniconf/tests/ui/enum-named.rs | 8 ++++++++ miniconf/tests/ui/enum-named.stderr | 7 +++++++ miniconf/tests/ui/enum-non-newtype.rs | 13 +++++++++++++ miniconf/tests/ui/enum-non-newtype.stderr | 13 +++++++++++++ miniconf/tests/ui/enum-skip.rs | 11 +++++++++++ miniconf/tests/ui/enum-skip.stderr | 11 +++++++++++ miniconf/tests/ui/flatten-ambiguous.rs | 17 +++++++++++++++++ miniconf/tests/ui/flatten-ambiguous.stderr | 11 +++++++++++ 12 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 miniconf/tests/compiletest.rs create mode 100644 miniconf/tests/ui/enum-named.rs create mode 100644 miniconf/tests/ui/enum-named.stderr create mode 100644 miniconf/tests/ui/enum-non-newtype.rs create mode 100644 miniconf/tests/ui/enum-non-newtype.stderr create mode 100644 miniconf/tests/ui/enum-skip.rs create mode 100644 miniconf/tests/ui/enum-skip.stderr create mode 100644 miniconf/tests/ui/flatten-ambiguous.rs create mode 100644 miniconf/tests/ui/flatten-ambiguous.stderr diff --git a/miniconf/Cargo.toml b/miniconf/Cargo.toml index b6c99729..fa991e64 100644 --- a/miniconf/Cargo.toml +++ b/miniconf/Cargo.toml @@ -36,6 +36,7 @@ heapless = "0.8.0" yafnv = "3.0.0" tokio = { version = "1.38.0", features = ["io-std", "rt", "macros"] } strum = { version = "0.26.3", features = ["derive"] } +trybuild = { version = "1.0.99", features = ["diff"] } [[test]] name = "arrays" diff --git a/miniconf/src/error.rs b/miniconf/src/error.rs index 84cc1f0c..f0bc8916 100644 --- a/miniconf/src/error.rs +++ b/miniconf/src/error.rs @@ -135,10 +135,10 @@ impl Display for Error { impl core::error::Error for Error { fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { - match self { - Self::Traversal(t) => Some(t), - Self::Inner(_, e) | Self::Finalization(e) => Some(e), - } + Some(match self { + Self::Traversal(t) => t, + Self::Inner(_, e) | Self::Finalization(e) => e, + }) } } diff --git a/miniconf/tests/compiletest.rs b/miniconf/tests/compiletest.rs new file mode 100644 index 00000000..870c2f95 --- /dev/null +++ b/miniconf/tests/compiletest.rs @@ -0,0 +1,5 @@ +#[test] +fn ui() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/ui/*.rs"); +} diff --git a/miniconf/tests/skipped.rs b/miniconf/tests/skipped.rs index d18cbd22..ed42f71b 100644 --- a/miniconf/tests/skipped.rs +++ b/miniconf/tests/skipped.rs @@ -30,3 +30,20 @@ fn path() { Err(Traversal::NotFound(1)) ); } + +#[test] +fn skip_enum() { + #[allow(dead_code)] + #[derive(Tree)] + pub enum E { + A(i32, #[tree(skip)] i32), + } +} + +#[test] +fn skip_struct() { + #[allow(dead_code)] + #[derive(Tree)] + #[tree(flatten)] + pub struct S(i32, #[tree(skip)] i32); +} diff --git a/miniconf/tests/ui/enum-named.rs b/miniconf/tests/ui/enum-named.rs new file mode 100644 index 00000000..3e193c46 --- /dev/null +++ b/miniconf/tests/ui/enum-named.rs @@ -0,0 +1,8 @@ +use miniconf::Tree; + +#[derive(Tree)] +pub enum E { + A { a: i32 }, +} + +fn main() {} diff --git a/miniconf/tests/ui/enum-named.stderr b/miniconf/tests/ui/enum-named.stderr new file mode 100644 index 00000000..8454ec19 --- /dev/null +++ b/miniconf/tests/ui/enum-named.stderr @@ -0,0 +1,7 @@ +error: Unsupported shape `named fields`. Expected unnamed fields or no fields. + --> tests/ui/enum-named.rs:3:10 + | +3 | #[derive(Tree)] + | ^^^^ + | + = note: this error originates in the derive macro `Tree` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/miniconf/tests/ui/enum-non-newtype.rs b/miniconf/tests/ui/enum-non-newtype.rs new file mode 100644 index 00000000..45b57588 --- /dev/null +++ b/miniconf/tests/ui/enum-non-newtype.rs @@ -0,0 +1,13 @@ +use miniconf::Tree; + +#[derive(Tree)] +pub enum E1 { + A(i32, i32), +} + +#[derive(Tree)] +pub enum E2 { + A { a: i32, b: i32 }, +} + +fn main() {} diff --git a/miniconf/tests/ui/enum-non-newtype.stderr b/miniconf/tests/ui/enum-non-newtype.stderr new file mode 100644 index 00000000..f40b96e2 --- /dev/null +++ b/miniconf/tests/ui/enum-non-newtype.stderr @@ -0,0 +1,13 @@ +error: Only newtype (single field tuple) enum variants are supported. + --> tests/ui/enum-non-newtype.rs:5:5 + | +5 | A(i32, i32), + | ^ + +error: Unsupported shape `named fields`. Expected unnamed fields or no fields. + --> tests/ui/enum-non-newtype.rs:8:10 + | +8 | #[derive(Tree)] + | ^^^^ + | + = note: this error originates in the derive macro `Tree` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/miniconf/tests/ui/enum-skip.rs b/miniconf/tests/ui/enum-skip.rs new file mode 100644 index 00000000..fc3ff081 --- /dev/null +++ b/miniconf/tests/ui/enum-skip.rs @@ -0,0 +1,11 @@ +use miniconf::Tree; + +#[derive(Tree)] +pub struct S(#[tree(skip)] i32, i32); + +#[derive(Tree)] +pub enum E { + A(#[tree(skip)] i32, i32), +} + +fn main() {} diff --git a/miniconf/tests/ui/enum-skip.stderr b/miniconf/tests/ui/enum-skip.stderr new file mode 100644 index 00000000..661cac0f --- /dev/null +++ b/miniconf/tests/ui/enum-skip.stderr @@ -0,0 +1,11 @@ +error: Can only `skip` terminal tuple struct fields + --> tests/ui/enum-skip.rs:4:21 + | +4 | pub struct S(#[tree(skip)] i32, i32); + | ^^^^ + +error: Can only `skip` terminal tuple struct fields + --> tests/ui/enum-skip.rs:8:14 + | +8 | A(#[tree(skip)] i32, i32), + | ^^^^ diff --git a/miniconf/tests/ui/flatten-ambiguous.rs b/miniconf/tests/ui/flatten-ambiguous.rs new file mode 100644 index 00000000..d0e8760f --- /dev/null +++ b/miniconf/tests/ui/flatten-ambiguous.rs @@ -0,0 +1,17 @@ +use miniconf::Tree; + +#[derive(Tree)] +#[tree(flatten)] +pub struct S { + a: i32, + b: i32, +} + +#[derive(Tree)] +#[tree(flatten)] +pub enum E { + A(i32), + B(i32), +} + +fn main() {} diff --git a/miniconf/tests/ui/flatten-ambiguous.stderr b/miniconf/tests/ui/flatten-ambiguous.stderr new file mode 100644 index 00000000..9f870d15 --- /dev/null +++ b/miniconf/tests/ui/flatten-ambiguous.stderr @@ -0,0 +1,11 @@ +error: Can't flatten multiple fields/variants + --> tests/ui/flatten-ambiguous.rs:4:8 + | +4 | #[tree(flatten)] + | ^^^^^^^ + +error: Can't flatten multiple fields/variants + --> tests/ui/flatten-ambiguous.rs:11:8 + | +11 | #[tree(flatten)] + | ^^^^^^^ From 7353e43eb82533ab9e35fbb3520d3f4776181a79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 30 Sep 2024 13:52:30 +0200 Subject: [PATCH 15/15] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 989d847d..b0c36ca4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * `flatten` support for structs/enums with a single non-skip/non-unit variant/field. +* `core::error::Error` implementations added to `Error` and `Traversal` ## [0.14.0](https://github.com/quartiq/miniconf/compare/v0.13.0...v0.14.0) - 2024-09-26