diff --git a/confik-macros/src/lib.rs b/confik-macros/src/lib.rs index f1568f3..ddd25d1 100644 --- a/confik-macros/src/lib.rs +++ b/confik-macros/src/lib.rs @@ -46,6 +46,26 @@ impl FromMeta for FieldFrom { } } +/// Handles `try_from` attributes for dealing with foreign types. +#[derive(Debug)] +struct FieldTryFrom { + ty: Type, +} + +impl FromMeta for FieldTryFrom { + fn from_expr(ty: &Expr) -> darling::Result { + let Ok(ty) = parse2(ty.to_token_stream()) else { + return Err(syn::Error::new( + ty.span(), + format!("Unable to parse type from: {}", ty.to_token_stream()), + ) + .into()); + }; + + Ok(Self { ty }) + } +} + /// Handles requesting to forward `serde` attributes. #[derive(Debug)] struct ForwardSerde { @@ -115,7 +135,7 @@ struct VariantImplementer { impl VariantImplementer { /// Define the builder variant for a given target variant - fn define_builder(var_impl: &SpannedValue) -> TokenStream { + fn define_builder(var_impl: &SpannedValue) -> syn::Result { let Self { ident, fields, @@ -126,17 +146,17 @@ impl VariantImplementer { let field_vec = fields .iter() .map(FieldImplementer::define_builder) - .collect::>(); + .collect::, _>>()?; let fields = ast::Fields::new(fields.style, field_vec).into_token_stream(); let discriminant = discriminant .as_ref() .map(|disc| quote_spanned!(disc.span() => = discriminant)); - quote_spanned! { var_impl.span() => + Ok(quote_spanned! { var_impl.span() => #forward_serde #ident #fields #discriminant - } + }) } fn impl_merge(var_impl: &SpannedValue) -> TokenStream { @@ -297,6 +317,10 @@ struct FieldImplementer { /// Enables handling foreign types. from: Option, + /// A type which implements `Configuration`, for which the field implements `TryFrom`. + /// Enables handling foreign types. + try_from: Option, + /// The field name, if a named field. /// /// If not, then you will probably want to enumerate through the list of these and @@ -351,13 +375,14 @@ impl FieldImplementer { } /// Define the builder field for a given target field. - fn define_builder(field_impl: &SpannedValue) -> TokenStream { + fn define_builder(field_impl: &SpannedValue) -> syn::Result { let Self { ty, ident, secret, forward_serde, from, + try_from, .. } = field_impl.as_ref(); @@ -367,7 +392,17 @@ impl FieldImplementer { // Builder type based on original field type via [`confik::Configuration`] // If `from` is set, then use that type instead. - let ty = from.as_ref().map_or(ty, |from| &from.ty); + let ty = match (from, try_from) { + (Some(from), Some(try_from)) => { + let msg = "Cannot support both `try_from` and `from` confik attributes"; + let mut err = syn::Error::new(try_from.ty.span(), msg); + err.combine(syn::Error::new(from.ty.span(), msg)); + return Err(err); + } + (Some(FieldFrom { ty }), None) | (None, Some(FieldTryFrom { ty })) => ty, + (None, None) => ty, + }; + let ty = quote_spanned!(ty.span() => <#ty as ::confik::Configuration>::Builder); // If secret then wrap in [`confik::SecretBuilder`] @@ -377,11 +412,11 @@ impl FieldImplementer { ty }; - quote_spanned! { ident.span() => + Ok(quote_spanned! { ident.span() => #[serde(default)] #forward_serde #ident #ty - } + }) } /// Define how to merge the given field in a struct impl. @@ -477,6 +512,12 @@ impl FieldImplementer { field_build = quote_spanned! { field_build.span() => #field_build.into() } + } else if field_impl.try_from.is_some() { + field_build = quote_spanned! { + field_build.span() => #field_build.try_into().map_err(|e| + ::confik::FailedTryInto::new(e) + )? + } } match style { @@ -618,7 +659,7 @@ impl RootImplementer { } /// Defines the builder for the target. - fn define_builder(&self) -> TokenStream { + fn define_builder(&self) -> syn::Result { let Self { ident: target_name, data, @@ -648,7 +689,7 @@ impl RootImplementer { let variants = variants .iter() .map(VariantImplementer::define_builder) - .collect::>(); + .collect::, _>>()?; quote_spanned! { target_name.span() => { @@ -665,7 +706,7 @@ impl RootImplementer { let field_vec = fields .iter() .map(FieldImplementer::define_builder) - .collect::>(); + .collect::, _>>()?; ast::Fields::new(fields.style, field_vec).into_token_stream() } }; @@ -684,14 +725,14 @@ impl RootImplementer { let (_impl_generics, type_generics, where_clause) = generics.split_for_impl(); - quote_spanned! { target_name.span() => + Ok(quote_spanned! { target_name.span() => #[derive(::std::default::Default, ::confik::__exports::__serde::Deserialize, #additional_derives )] #[serde(crate = "::confik::__exports::__serde")] #forward_serde #vis #enum_or_struct_token #builder_name #type_generics #where_clause #bracketed_data #terminator - } + }) } /// Implement the `ConfigurationBuilder::merge` method for our builder. @@ -757,7 +798,7 @@ impl RootImplementer { .collect::>(); quote! { Ok(match self { - Self::ConfigBuilderUndefined => return Err(<::confik::MissingValue as ::std::default::Default>::default()), + Self::ConfigBuilderUndefined => return Err(::confik::Error::MissingValue(<::confik::MissingValue as ::std::default::Default>::default())), #( #variants, )* }) } @@ -767,7 +808,7 @@ impl RootImplementer { quote! { // Allow useless conversions as the default handling may call `.into()` unnecessarily. #[allow(clippy::useless_conversion)] - fn try_build(self) -> ::std::result::Result { + fn try_build(self) -> ::std::result::Result { #field_build } } @@ -859,7 +900,7 @@ impl RootImplementer { fn derive_macro_builder_inner(target_struct: DeriveInput) -> syn::Result { let implementer = RootImplementer::from_derive_input(&target_struct)?; implementer.check_valid()?; - let builder_struct = implementer.define_builder(); + let builder_struct = implementer.define_builder()?; let builder_impl = implementer.impl_builder(); let target_impl = implementer.impl_target(); diff --git a/confik-macros/tests/trybuild.rs b/confik-macros/tests/trybuild.rs index 267cb23..6663a56 100644 --- a/confik-macros/tests/trybuild.rs +++ b/confik-macros/tests/trybuild.rs @@ -1,5 +1,5 @@ // only run on MSRV to avoid changes to compiler output causing CI failures -#[rustversion::stable(1.66)] +// #[rustversion::stable(1.75)] #[test] fn compile_macros() { let t = trybuild::TestCases::new(); @@ -27,6 +27,7 @@ fn compile_macros() { t.pass("tests/trybuild/21-field-from.rs"); t.pass("tests/trybuild/22-dataless-types.rs"); t.pass("tests/trybuild/23-where-clause.rs"); + t.pass("tests/trybuild/24-field-try_from.rs"); t.compile_fail("tests/trybuild/fail-default-parse.rs"); t.compile_fail("tests/trybuild/fail-default-invalid-expr.rs"); @@ -37,4 +38,5 @@ fn compile_macros() { t.compile_fail("tests/trybuild/fail-uncreatable-type.rs"); t.compile_fail("tests/trybuild/fail-not-a-type.rs"); t.compile_fail("tests/trybuild/fail-default-not-expression.rs"); + t.compile_fail("tests/trybuild/fail-from-and-try_from.rs"); } diff --git a/confik-macros/tests/trybuild/fail-default-parse.stderr b/confik-macros/tests/trybuild/fail-default-parse.stderr index 47b9913..080f366 100644 --- a/confik-macros/tests/trybuild/fail-default-parse.stderr +++ b/confik-macros/tests/trybuild/fail-default-parse.stderr @@ -5,16 +5,12 @@ error[E0277]: the trait bound `usize: From` is not satisfied | ^---- | | | the trait `From` is not implemented for `usize` - | this tail expression is of type `_` + | this tail expression is of type `i32` | = help: the following other types implement trait `From`: - > - > - > - > - > - > - > - > - and $N others + > + > + > + > + > = note: required for `i32` to implement `Into` diff --git a/confik-macros/tests/trybuild/fail-field-from-unknown-type.stderr b/confik-macros/tests/trybuild/fail-field-from-unknown-type.stderr index 41e3885..b2cd346 100644 --- a/confik-macros/tests/trybuild/fail-field-from-unknown-type.stderr +++ b/confik-macros/tests/trybuild/fail-field-from-unknown-type.stderr @@ -1,26 +1,16 @@ error[E0412]: cannot find type `A` in this scope --> tests/trybuild/fail-field-from-unknown-type.rs:6:21 | -5 | struct Config { - | - help: you might be missing a type parameter: `` 6 | #[confik(from = A)] | ^ not found in this scope + | +help: you might be missing a type parameter + | +5 | struct Config { + | +++ error[E0412]: cannot find type `A` in this scope --> tests/trybuild/fail-field-from-unknown-type.rs:6:21 | 6 | #[confik(from = A)] | ^ not found in this scope - -error[E0283]: type annotations needed - --> tests/trybuild/fail-field-from-unknown-type.rs:6:21 - | -5 | struct Config { - | ------ in this derive macro expansion -6 | #[confik(from = A)] - | _____________________^ -7 | | param: String, - | |_________^ cannot infer type - | - = note: cannot satisfy `_: Default` - = note: this error originates in the derive macro `::std::default::Default` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/confik/CHANGELOG.md b/confik/CHANGELOG.md index 412595f..4e34418 100644 --- a/confik/CHANGELOG.md +++ b/confik/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Add support for `try_from` attribute, following the rules of `from` but using `TryFrom`. Note: This required a change in the return type of `try_build` and a new `Error` variant. + - This will not break existing code unless it contains manual implementations of `Configuration`. + ## 0.10.2 - Remove `Debug` implementation from configuration builders. diff --git a/confik/src/errors.rs b/confik/src/errors.rs index 7696ff8..d5bf592 100644 --- a/confik/src/errors.rs +++ b/confik/src/errors.rs @@ -3,11 +3,11 @@ //! Although in theory [`UnexpectedSecret`] and [`MissingValue`] are also user facing, they are entirely //! handled by the `derive` internals, so is counted as internal. -use std::error::Error as StdError; +use std::{borrow::Cow, error::Error as StdError}; use thiserror::Error; -use crate::{MissingValue, UnexpectedSecret}; +use crate::{FailedTryInto, MissingValue, UnexpectedSecret}; /// Possible error values. #[derive(Debug, Error)] @@ -23,9 +23,30 @@ pub enum Error { #[error("Source {1} returned an error")] Source(#[source] Box, String), - /// The value contained in the `path` was marked as a [`SecretBuilder`](crate::SecretBuilder) but - /// was parsed from a [`Source`](crate::Source) that was not marked as a secret + /// The value contained in the `path` was marked as a [`SecretBuilder`](crate::SecretBuilder) + /// but was parsed from a [`Source`](crate::Source) that was not marked as a secret /// (see [`Source::allows_secrets`](crate::Source::allows_secrets)). #[error("Found a secret in source {1} that does not permit secrets")] UnexpectedSecret(#[source] UnexpectedSecret, String), + + /// The value contained in the `path` was attempted to be converted and that conversion failed. + #[error(transparent)] + TryInto(#[from] FailedTryInto), +} + +impl Error { + /// Used in chaining [`MissingValue`] errors during [`crate::Configuration::try_build`]. + #[doc(hidden)] + pub fn prepend(self, path_segment: impl Into>) -> Self { + match self { + Self::MissingValue(err) => Self::MissingValue(err.prepend(path_segment)), + Self::TryInto(err) => Self::TryInto(err.prepend(path_segment)), + // This branch will probably never be hit but exists so that the function works the way + // a caller would expect if there is a use case for it in future. + Self::UnexpectedSecret(err, source) => { + Self::UnexpectedSecret(err.prepend(path_segment), source) + } + Self::Source(err, source) => Self::Source(err, source), + } + } } diff --git a/confik/src/lib.md b/confik/src/lib.md index 2abc535..ee13c57 100644 --- a/confik/src/lib.md +++ b/confik/src/lib.md @@ -188,7 +188,7 @@ This crate provides implementations of [`Configuration`] for a number of `std` t - `secrecy`: v0.8 - Note: `#[config(secret)]` is not needed (although it is harmless) for `secrecy`'s types as they are always treated as secrets. -If there's another foreign type used in your config, then you will not be able to implement [`Configuration`] for it. Instead any type that implements [`Into`] can be used. +If there's another foreign type used in your config, then you will not be able to implement [`Configuration`] for it. Instead any type that implements [`Into`] or [`TryInto`] can be used. ``` struct ForeignType { @@ -208,10 +208,28 @@ impl From for ForeignType { } } +#[derive(confik::Configuration)] +struct MyForeignTypeIsize { + data: isize +} + +impl TryFrom for ForeignType { + type Error = >::Error; + + fn try_from(copy: MyForeignTypeIsize) -> Result { + Ok(Self { + data: copy.data.try_into()?, + }) + } +} + #[derive(confik::Configuration)] struct Config { #[confik(from = MyForeignTypeCopy)] - foreign_data: ForeignType + foreign_data: ForeignType, + + #[confik(try_from = MyForeignTypeIsize)] + foreign_data_isized: ForeignType, } ``` diff --git a/confik/src/lib.rs b/confik/src/lib.rs index f3be83f..1e1585c 100644 --- a/confik/src/lib.rs +++ b/confik/src/lib.rs @@ -2,7 +2,7 @@ #![deny(rust_2018_idioms, nonstandard_style, future_incompatible)] #![cfg_attr(docsrs, feature(doc_auto_cfg))] -use std::{borrow::Cow, ops::Not}; +use std::{borrow::Cow, error::Error as StdError, ops::Not}; #[doc(hidden)] pub use confik_macros::*; @@ -63,6 +63,25 @@ impl MissingValue { } } +/// Captures the path and error of a failed conversion. +#[derive(Debug, thiserror::Error)] +#[error("Failed try_into for path `{0}`: {1}")] +pub struct FailedTryInto(Path, #[source] Box); + +impl FailedTryInto { + /// Creates a new [`Self`] with a blank path. + pub fn new(err: impl StdError + 'static) -> Self { + Self(Path::new(), Box::new(err)) + } + + /// Prepends a path segment as we return back up the call-stack. + #[must_use] + pub fn prepend(mut self, path_segment: impl Into>) -> Self { + self.0 .0.push(path_segment.into()); + self + } +} + /// Converts the sources, in order, into [`Configuration::Builder`] and /// [`ConfigurationBuilder::merge`]s them, passing any errors back. fn build_from_sources<'a, Target, Iter>(sources: Iter) -> Result @@ -140,7 +159,7 @@ pub trait ConfigurationBuilder: Default + DeserializeOwned { /// This will probably delegate to `TryInto` but allows it to be implemented for types foreign /// to the library. - fn try_build(self) -> Result; + fn try_build(self) -> Result; /// Called recursively on each field, aiming to hit all [`SecretBuilder`]s. This is only called /// when [`Source::allows_secrets`] is `false`. @@ -165,8 +184,8 @@ where self.or(other) } - fn try_build(self) -> Result { - self.ok_or_else(|| MissingValue(Path::new())) + fn try_build(self) -> Result { + self.ok_or_else(|| Error::MissingValue(MissingValue::default())) } /// Should not have an `Option` wrapping a secret as ` as ConfigurationBuilder` is diff --git a/confik/src/secrets.rs b/confik/src/secrets.rs index dc6fbf3..eecc88c 100644 --- a/confik/src/secrets.rs +++ b/confik/src/secrets.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use serde::{de::DeserializeOwned, Deserialize}; use thiserror::Error; -use crate::{path::Path, Configuration, ConfigurationBuilder, MissingValue}; +use crate::{path::Path, Configuration, ConfigurationBuilder, Error}; /// Captures the path of a secret found in a non-secret source. #[derive(Debug, Default, Error)] @@ -37,7 +37,7 @@ impl SecretBuilder { Self(self.0.merge(other.0)) } - pub fn try_build(self) -> Result { + pub fn try_build(self) -> Result { self.0.try_build() } @@ -77,8 +77,9 @@ where Self(self.0.or(other.0)) } - fn try_build(self) -> Result { - self.0.ok_or_else(|| MissingValue(Path::new())) + fn try_build(self) -> Result { + self.0 + .ok_or_else(|| Error::MissingValue(Default::default())) } /// Should not have an `Option` wrapping a secret as ` as ConfigurationBuilder` is diff --git a/confik/src/std_impls.rs b/confik/src/std_impls.rs index 501f555..1836378 100644 --- a/confik/src/std_impls.rs +++ b/confik/src/std_impls.rs @@ -13,7 +13,9 @@ use std::{ use serde::{de::DeserializeOwned, Deserialize}; -use crate::{path::Path, Configuration, ConfigurationBuilder, MissingValue, UnexpectedSecret}; +use crate::{ + path::Path, Configuration, ConfigurationBuilder, Error, MissingValue, UnexpectedSecret, +}; /// Convenience macro for the large number of foreign library types to implement the /// [`Configuration`] using an [`Option`] as their [`ConfigurationBuilder`]. @@ -104,9 +106,9 @@ where } } - fn try_build(self) -> Result { + fn try_build(self) -> Result { match self { - Self::Unspecified => Err(MissingValue(Path::new())), + Self::Unspecified => Err(Error::MissingValue(Default::default())), Self::Some(val) => val .into_iter() .map(ConfigurationBuilder::try_build) @@ -229,9 +231,9 @@ where } } - fn try_build(self) -> Result { + fn try_build(self) -> Result { match self { - Self::Unspecified => Err(MissingValue(Path::new())), + Self::Unspecified => Err(Error::MissingValue(Default::default())), Self::Some(val) => val .into_iter() .map(|(key, value)| Ok((key, value.try_build()?))) @@ -331,16 +333,20 @@ where self.map(|us| us.merge(iter.next().unwrap())) } - fn try_build(self) -> Result { + fn try_build(self) -> Result { self.into_iter() .enumerate() .map(|(index, val)| { - val.try_build() - .map_err(|err| err.prepend(index.to_string())) + val.try_build().map_err(|err| match err { + Error::MissingValue(err) => Error::MissingValue(err.prepend(index.to_string())), + err => err, + }) }) .collect::, _>>()? .try_into() - .map_err(|vec: Vec<_>| MissingValue(Path::new()).prepend(vec.len().to_string())) + .map_err(|vec: Vec<_>| { + Error::MissingValue(MissingValue(Path::new()).prepend(vec.len().to_string())) + }) } fn contains_non_secret_data(&self) -> Result { @@ -370,7 +376,7 @@ impl ConfigurationBuilder for PhantomData { self } - fn try_build(self) -> Result { + fn try_build(self) -> Result { Ok(self) } @@ -437,7 +443,7 @@ where } } - fn try_build(self) -> Result { + fn try_build(self) -> Result { match self { Self::Unspecified | Self::None => Ok(None), Self::Some(val) => Ok(Some(val.try_build()?)),