idanarye / rust-typed-builder

Compile-time type-checked builder derive
https://crates.io/crates/typed-builder
Apache License 2.0
904 stars 52 forks source link

Fix private type leak #97

Closed AldaronLau closed 1 year ago

AldaronLau commented 1 year ago

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 commented 1 year ago

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

AldaronLau commented 1 year ago

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.