Enabled composed/nested structs by impl'ing Deserialize ourselves#28
Open
FuegoFro wants to merge 1 commit into
Open
Enabled composed/nested structs by impl'ing Deserialize ourselves#28FuegoFro wants to merge 1 commit into
FuegoFro wants to merge 1 commit into
Conversation
This enables Recap to work for multiple layers of nested structs, as seen in the `custom_deserialize_allows_nested_structs` test. The impetus for this came from things like https://adventofcode.com/2023/day/19, where parsing the workflow with rules with optional conditions gets much easier if you can just describe it as 3 nested structs. There's details on the implementation written in the `derive_impl_deserialize` function, but the high level is that, when enabled, this will create a helper struct that mirrors the original one and then handle most deserialization requests by first deserializing into the helper struct and then moving the data to an instance of the original struct. The main difference is custom behavior for `str`, which allows us to parse the `str` using the regex and *then* forward to the helper struct. This also adds integration tests for the derive definition and updates the documentation to describe the new behavior. Note that currently the `Deserialize` implementation has some limitations like not working with zero-copy structs (structs that have lifetimes) or generic types (though generics already seems to not work with the existing Recap derivation). This also changes the `Recap` proc-macro to allow/handle `serde` attributes, which could lead to false negatives where people are putting attributes that end up being unused and not getting compiler warnings for it when the `handle_deserialize` attribute is *not* present. The only way around this that I'm aware of would be to use a different derive name (eg something like `#[derive(RecapDeserialize)]`) which I'm certainly open to as an alternative to the current `#[recap(handle_deserialize)]` approach if you'd prefer. Fixes softprops#10.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What did you implement:
This enables Recap to work for multiple layers of nested structs, as seen in the
custom_deserialize_allows_nested_structstest. The impetus for this came from things like https://adventofcode.com/2023/day/19, where parsing the workflow with rules with optional conditions gets much easier if you can just describe it as 3 nested structs.There's details on the implementation written in the
derive_impl_deserializefunction, but the high level is that, when enabled, this will create a helper struct that mirrors the original one and then handle most deserialization requests by first deserializing into the helper struct and then moving the data to an instance of the original struct. The main difference is custom behavior forstr, which allows us to parse thestrusing the regex and then forward to the helper struct.Note that currently the
Deserializeimplementation has some limitations like not working with zero-copy structs (structs that have lifetimes) or generic types (though generics already seems to not work with the existing Recap derivation). This also changes theRecapproc-macro to allow/handleserdeattributes, which could lead to false negatives where people are putting attributes that end up being unused and not getting compiler warnings for it when thehandle_deserializeattribute is not present. The only way around this that I'm aware of would be to use a different derive name (eg something like#[derive(RecapDeserialize)]) which I'm certainly open to as an alternative to the current#[recap(handle_deserialize)]approach if you'd prefer.Closes: #10
How did you verify your change:
This also adds integration tests for the derive definition and updates the documentation to describe the new behavior.
What (if anything) would need to be called out in the CHANGELOG for the next release:
The new behavior accessible via the
#[recap(handle_deserialize)]attribute and likely the change in allowingserdeattributes as well.