Skip to content

Commit

Permalink
Remove soft merge mode.
Browse files Browse the repository at this point in the history 
Soft merging has been removed, as there are some footguns that cannot
easily be fixed.

soft mode would never merge, if the target is non-optional.
Due to aliasing however, we can never be actually sure that any type is
not optional, since it's impractically to do full type resolution in the
proc macro state!
  • Loading branch information
@Nukesor
Nukesor committed Mar 26, 2022
1 parent be0499b commit 44c94c8
Show file tree
Hide file tree
Showing 6 changed files with 0 additions and 431 deletions.
4 changes: 0 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ It's not trivial to implement code for two structs in the same codebase.
pub trait StructMerge<Src> {
/// Merge the given struct into self.
fn merge(&mut self, src: Src);
/// Nearly the same as `merge`, but any `Self::Option<T>` fields will only get merged, if the
/// value of the field is `None`.
fn merge_soft(&mut self, src: Src);
}
```

Expand Down
113 changes: 0 additions & 113 deletions codegen/src/generate/merge/borrowed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ pub(crate) fn impl_borrowed(params: &Parameters, fields: Vec<(Field, Field)>) ->
let stream = merge_ref(params, fields.clone());
functions_tokens.extend(vec![stream]);

// Add `merge_ref_soft` impl.
let stream = merge_ref_soft(params, fields);
functions_tokens.extend(vec![stream]);

// Surround functions with `impl` block.
let src_ident = &params.src_struct.ident;
let target_path = &params.target_path;
Expand Down Expand Up @@ -155,112 +151,3 @@ fn merge_ref(params: &Parameters, fields: Vec<(Field, Field)>) -> TokenStream {
}
}
}

// See the merge/owned.rs implementation for info on why this is commented.
/// Generate the [inter_struct::merge::StructMergeRef::merge_ref_soft] function for given structs.
///
/// All fields must implement `Clone`.
fn merge_ref_soft(params: &Parameters, fields: Vec<(Field, Field)>) -> TokenStream {
let mut merge_code = TokenStream::new();
for (src_field, target_field) in fields {
let src_field_ident = src_field.ident;
let target_field_ident = target_field.ident;

// Find out, whether the fields are optional or not.
let src_field_type = match determine_field_type(src_field.ty) {
Ok(field) => field,
Err(err) => {
merge_code.extend(vec![err]);
continue;
}
};
let target_field_type = match determine_field_type(target_field.ty) {
Ok(field) => field,
Err(err) => {
merge_code.extend(vec![err]);
continue;
}
};

let snippet = match (src_field_type, target_field_type) {
// Soft merge only applies if the target field is `Optional`.
(FieldType::Normal(_), FieldType::Normal(_))
| (FieldType::Optional { .. }, FieldType::Normal(_)) => continue,
// The target is optional and needs to be wrapped in `Some(T)` to be merged.
(
FieldType::Normal(src_type),
FieldType::Optional {
inner: target_type, ..
},
) => {
equal_type_or_err!(
src_type,
target_type,
"",
quote! {
if target.#target_field_ident.is_none() {
target.#target_field_ident = Some(self.#src_field_ident.clone());
}
}
)
}
// Both fields are optional. It can now be either of these:
// - (Option<T>, Option<T>)
// - (Option<Option<T>>, Option<T>)
// - (Option<T>, Option<Option<T>>)
(
FieldType::Optional {
inner: inner_src_type,
outer: outer_src_type,
},
FieldType::Optional {
inner: inner_target_type,
outer: outer_target_type,
},
) => {
// Handling the (Option<T>, Option<T>) case
if is_equal_type(&inner_src_type, &inner_target_type) {
quote! {
if target.#target_field_ident.is_none() {
target.#target_field_ident = self.#src_field_ident.clone();
}
}
// Handling the (Option<Option<<T>>, Option<T>) case
} else if is_equal_type(&inner_src_type, &outer_target_type) {
quote! {
if let Some(value) = self.#src_field_ident.as_ref() {
if target.#target_field_ident.is_none() {
target.#target_field_ident = value.clone();
}
}
}
// Handling the (Option<<T>, Option<Option<T>)> case
} else {
equal_type_or_err!(
outer_src_type,
inner_target_type,
"",
quote! {
if target.#target_field_ident.is_none() {
target.#target_field_ident = Some(self.#src_field_ident.clone());
}
}
)
}
}
// Skip anything where either of the fields are invalid
(FieldType::Invalid, _) | (_, FieldType::Invalid) => continue,
};

merge_code.extend(vec![snippet]);
}

let merge_code = merge_code.to_token_stream();

let target_path = &params.target_path;
quote! {
fn merge_into_ref_soft(&self, target: &mut #target_path) {
#merge_code
}
}
}
114 changes: 0 additions & 114 deletions codegen/src/generate/merge/owned.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ pub(crate) fn impl_owned(params: &Parameters, fields: Vec<(Field, Field)>) -> To
let stream = merge(params, fields.clone());
functions_tokens.extend(vec![stream]);

// Add `merge_soft` impl.
let stream = merge_soft(params, fields);
functions_tokens.extend(vec![stream]);

// Surround functions with `impl` block.
let src_ident = &params.src_struct.ident;
let target_path = &params.target_path;
Expand Down Expand Up @@ -128,113 +124,3 @@ fn merge(params: &Parameters, fields: Vec<(Field, Field)>) -> TokenStream {
}
}
}

// The Merge soft code branch is removed for now, since this code contains a few footguns.
//
// - If people work with type aliases such as `type nice = Option<String>`, the `Option` detection
// no longer works and thereby the `merge_soft*` functions won't work as expected.
/// Generate the [inter_struct::merge::StructMerge::merge_soft] function for the given structs.
fn merge_soft(params: &Parameters, fields: Vec<(Field, Field)>) -> TokenStream {
let mut merge_code = TokenStream::new();
for (src_field, dest_field) in fields {
let src_ident = src_field.ident;
let dest_ident = dest_field.ident;

// Find out, whether the fields are optional or not.
let src_field_type = match determine_field_type(src_field.ty) {
Ok(field) => field,
Err(err) => {
merge_code.extend(vec![err]);
continue;
}
};
let dest_field_type = match determine_field_type(dest_field.ty) {
Ok(field) => field,
Err(err) => {
merge_code.extend(vec![err]);
continue;
}
};

let snippet = match (src_field_type, dest_field_type) {
// Soft merge only applies if the dest field is `Optional`.
(FieldType::Normal(_), FieldType::Normal(_))
| (FieldType::Optional { .. }, FieldType::Normal(_)) => continue,
// The dest is optional and needs to be wrapped in `Some(T)` to be merged.
(
FieldType::Normal(src_type),
FieldType::Optional {
inner: dest_type, ..
},
) => {
equal_type_or_err!(
src_type,
dest_type,
"",
quote! {
if dest.#dest_ident.is_none() {
dest.#dest_ident = Some(self.#src_ident);
}
}
)
}
// Both fields are optional. It can now be either of these:
// - (Option<T>, Option<T>)
// - (Option<Option<T>>, Option<T>)
// - (Option<T>, Option<Option<T>>)
(
FieldType::Optional {
inner: inner_src_type,
outer: outer_src_type,
},
FieldType::Optional {
inner: inner_dest_type,
outer: outer_dest_type,
},
) => {
// Handling the (Option<T>, Option<T>) case
if is_equal_type(&inner_src_type, &inner_dest_type) {
quote! {
if dest.#dest_ident.is_none() {
dest.#dest_ident = self.#src_ident;
}
}
// Handling the (Option<Option<<T>>, Option<T>) case
} else if is_equal_type(&inner_src_type, &outer_dest_type) {
quote! {
if let Some(value) = self.#src_ident {
if dest.#dest_ident.is_none() {
dest.#dest_ident = value;
}
}
}
// Handling the (Option<<T>, Option<Option<T>)> case
} else {
equal_type_or_err!(
outer_src_type,
inner_dest_type,
"",
quote! {
if dest.#dest_ident.is_none() {
dest.#dest_ident = Some(self.#src_ident);
}
}
)
}
}
// Skip anything where either of the fields are invalid
(FieldType::Invalid, _) | (_, FieldType::Invalid) => continue,
};

merge_code.extend(vec![snippet]);
}

let merge_code = merge_code.to_token_stream();

let target_path = &params.target_path;
quote! {
fn merge_into_soft(self, dest: &mut #target_path) {
#merge_code
}
}
}
94 changes: 0 additions & 94 deletions src/merge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,93 +49,25 @@
//! target.test = value;
//! }
//! ```
//!
//! ## Merge behavior of `merge_soft` and `merge_ref_soft`
//!
//! #### Target is not Optional
//!
//! As long as a target field is not optional **it won't be touched**!
//!
//! #### Target is Optional
//!
//! ```rust,ignore
//! struct Src {
//! test: T
//! }
//! struct Target {
//! test: Option<T>
//! }
//! ```
//!
//! This will wrap `src.test` into an `Option` and merge it into `target.test` but only if `target.test` is `None`: \
//! ```rust,ignore
//! if target.test.is_none() {
//! target.test = Some(src.test);
//! }
//! ```
//!
//! #### Both are Optional
//!
//! ```rust,ignore
//! struct Src {
//! test: Option<T>
//! }
//! struct Target {
//! test: Option<T>
//! }
//! ```
//!
//! This will only merge `src.test` into `target.test` if `target.test` is `None`: \
//! ```rust,ignore
//! if target.test.is_none() {
//! target.test = src.test;
//! }
//! ```
/// Merge another struct into `Self`.
pub trait StructMerge<Src> {
/// Merge the given struct into `Self` whilst consuming it.
fn merge(&mut self, src: Src);

/// Merge the given struct into `Self` whilst consuming it.
///
/// Nearly the same as `merge`, but any `Self::Option<T>` fields will only get merged if the
/// value of the field is `None`.
///
/// For example:
/// ```ignore
/// struct Target { a: Option<String> };
/// struct Src { a: String };
///
/// let target = Target { a: Some("test".to_string()) };
/// let src = Src { a: "test2".to_string() };
///
/// target.merge_soft(src);
/// // Value didn't get merged as `target.a` was `Some`
/// assert_eq!(target.a, "test".to_string());
/// ```
fn merge_soft(&mut self, src: Src);
}

/// Counterpart of [StructMerge].
/// This will merge `Self` into a given target.
pub trait StructMergeInto<Target: ?Sized> {
/// Check the [StructMerge::merge] docs.
fn merge_into(self, target: &mut Target);

/// Check the [StructMerge::merge_soft] docs.
fn merge_into_soft(self, target: &mut Target);
}

/// Implement the [StructMerge] trait for all types that provide [StructMergeInto] for it.
impl<Target, Src: StructMergeInto<Target>> StructMerge<Src> for Target {
fn merge(&mut self, src: Src) {
src.merge_into(self);
}

fn merge_soft(&mut self, src: Src) {
src.merge_into_soft(self);
}
}

/// Merge another borrowed struct into `Self`.
Expand All @@ -144,44 +76,18 @@ impl<Target, Src: StructMergeInto<Target>> StructMerge<Src> for Target {
pub trait StructMergeRef<Src> {
/// Merge the given struct into `Self`.
fn merge_ref(&mut self, src: &Src);

/// Merge the given struct into `Self`.
///
/// Nearly the same as `merge_ref`, but any `Self::Option<T>` fields will only get merged if the
/// value of the field is `None`.
///
/// For example:
/// ```ignore
/// struct Target { a: Option<String> };
/// struct Src { a: String };
///
/// let target = Target { a: Some("test".to_string()) };
/// let src = Src { a: "test2".to_string() };
///
/// target.merge_ref_soft(&src);
/// // Value didn't get merged as `target.a` was `Some`
/// assert_eq!(target.a, "test".to_string());
/// ```
fn merge_ref_soft(&mut self, src: &Src);
}

/// Counterpart of [StructMergeRef].
/// This will merge `&Self` into a given target.
pub trait StructMergeIntoRef<Target: ?Sized> {
/// Check the [StructMergeRef::merge_ref] docs.
fn merge_into_ref(&self, target: &mut Target);

/// Check the [StructMergeRef::merge_ref_soft] docs.
fn merge_into_ref_soft(&self, target: &mut Target);
}

/// Implement the [StructMergeRef] trait for all types that provide [StructMergeInto] for it.
impl<Target, Src: StructMergeIntoRef<Target>> StructMergeRef<Src> for Target {
fn merge_ref(&mut self, src: &Src) {
src.merge_into_ref(self);
}

fn merge_ref_soft(&mut self, src: &Src) {
src.merge_into_ref_soft(self);
}
}
Loading

0 comments on commit 44c94c8

Please sign in to comment.