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

feat: generate enum default as newtype #233

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

Millione
Copy link
Member

Motivation

Previously, thrift/protobuf enum type is generated to rust enum type by default, and we provided an annotation pilota.enum_mode="new_type" for users to change the rust generated type to a wrapper of i32. Over time, we found that it is more correct to make the newtype the default behaviour, as when users add a new variant in the enum definition, it will not cause the old idl generated code to fail to decode.

Solution

Deprecate annotation pilota.enum_mode="new_type" and make it as default behaviour.

enum Index {
    A = 0x01,
    B = 0x10,
}

Generated:

pub struct Index(i32);

impl Index {
    pub const A: Self = Self(1);
    pub const B: Self = Self(16);

    pub fn inner(&self) -> i32 {
        self.0
    }
}

impl ::std::convert::From<i32> for Index {
    fn from(value: i32) -> Self {
        Self(value)
    }
}

@Millione Millione added A-pilota-build This issue concerns the `pilota-build` crate. C-enhancement This is a PR that adds a new feature or fixes a bug. labels Mar 28, 2024
@Millione Millione self-assigned this Mar 28, 2024
@Millione Millione requested review from a team as code owners March 28, 2024 03:25
@Millione Millione merged commit 7f088f7 into cloudwego:main Mar 28, 2024
9 checks passed
@Millione Millione deleted the feat/enum-default-newtype branch March 28, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pilota-build This issue concerns the `pilota-build` crate. C-enhancement This is a PR that adds a new feature or fixes a bug.
Development

Successfully merging this pull request may close these issues.

2 participants