Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Splits MakeValueWriter into two traits #867

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Splits MakeValueWriter into two traits #867

merged 2 commits into from
Dec 3, 2024

Conversation

zslayton
Copy link
Contributor

@zslayton zslayton commented Dec 3, 2024

This patch contains no logic changes, only a refactoring of the MakeValueWriter trait.

Instances of ValueWriter are single-use by design, as this allows the crate to
statically guarantee that an application cannot (for example) try to encode multiple
values following an annotations sequence or inside a struct field.

Prior to this PR, the MakeValueWriter trait was public for testing purposes.
This allowed applications to refer to a container's associated ValueWriter family of types,
which was needed for custom writer methods leveraging generics.
For example, a list writer at the top level of a stream might be written as:

let mut list_writer: 
    <
        <Writer<v1_1::Binary, File> as MakeValueWriter>
            ::ValueWriter<'_> as ValueWriter
    >
    ::ListWriter 
= // ...;

(This is admittedly very ugly, but alas there is no way to offer a nice shortcut for this case.)

While exposing MakeValueWriter allowed applications to concretely reference associated type paths,
it also exposed the make_value_writer method. make_value_writer needs to be kept private to
prevent users from circumventing ValueWriter's statically enforced single-use restriction,
creating and then using multiple value writers in situations where that would produce illegal data.

This PR splits MakeValueWriter into two traits:

  1. ContextWriter, which houses the associated types
    and
  2. MakeValueWriter, which houses the make_value_writer method and, by extending ContextWriter, has access to the necessary associated types

ContextWriter is public/re-exported, allowing applications to refer to associated types as needed.
MakeValueWriter is now pub(crate) and is no longer re-exported at the top level.

The example variable declaration above would now be written:

let mut list_writer: 
    <
        <Writer<v1_1::Binary, File> as ContextWriter>
            ::NestedValueWriter<'_> as ValueWriter
    >
    ::ListWriter
= // ...;

This PR supersedes #864 and fixes #863.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@zslayton zslayton merged commit f3b6cca into main Dec 3, 2024
25 of 33 checks passed
@zslayton zslayton deleted the context-writer branch December 3, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MakeValueWriter should be pub and feature-gated
2 participants