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

Fix private type leak #97

Merged
merged 3 commits into from Jun 24, 2023
Merged

Conversation

AldaronLau
Copy link
Contributor

This PR fixes a private type leak bug in the expanded macro output, by making a small simplification in the implementation. I also added a test that triggers the bug.

@idanarye
Copy link
Owner

Can you explain what the bug is? I'm not sure I 100% follow.

@AldaronLau
Copy link
Contributor Author

The bug happens to when

  1. The builder method is public
  2. The builder type is public and converts to a public type with build_method(into =
  3. The type being built is private
  4. At least one field is required / non-default

The generated code creates a method that produces a compiler error.

error[E0446]: private type `Foo` in public interface
  --> tests/no_type_leakage.rs:5:21
   |
5  | #[derive(PartialEq, TypedBuilder)]
   |                     ^^^^^^^^^^^^ can't leak private type
...
11 | struct Foo {
   | ---------- `Foo` declared as private
   |
   = note: this error originates in the derive macro `TypedBuilder` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0446`.
error: could not compile `typed-builder` (test "no_type_leakage") due to previous error

A snippet of expanded code showing where the error is:

#[doc(hidden)]
#[allow(dead_code, non_camel_case_types, non_snake_case)]
pub enum BarBuilder_Error_Missing_required_field_x {}
#[doc(hidden)]
#[allow(dead_code, non_camel_case_types, missing_docs, clippy::panic)]
impl BarBuilder<((),)> {
    #[deprecated(note = "Missing required field x")]
    pub fn build(self, _: BarBuilder_Error_Missing_required_field_x) -> Foo {
        ::core::panicking::panic("explicit panic");
    }
}
#[allow(dead_code, non_camel_case_types, missing_docs)]
impl BarBuilder<((i32,),)> {
    #[allow(clippy::default_trait_access)]
    pub fn build(self) -> Bar {
        let (x,) = self.fields;
        let x = x.0;
        #[allow(deprecated)] Foo { x }.into()
    }
}

As you can see, the missing required field build() method has the incorrect return type (Foo, when it should be Bar). But, since it always panics, I opted to return ! instead.

@idanarye idanarye merged commit 7d7b54b into idanarye:master Jun 24, 2023
8 checks passed
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.

None yet

2 participants