dropbox / djinni

A tool for generating cross-language type declarations and interface bindings.
Apache License 2.0
2.88k stars 488 forks source link

Djinni don't write enums correctly using ordinal value #431

Closed paulocoutinhox closed 5 years ago

paulocoutinhox commented 5 years ago

Hi,

Djinni don't write on parcel enum value as ordinal. In my example:

/*package*/ final com.ubook.library.core.enums.SearchParamOrderAscendingTypeEnum mOrderAscensing;

    @Override
    public void writeToParcel(android.os.Parcel out, int flags) {
        out.writeList(this.mCatalogTypeList);
        out.writeString(this.mOrderField);
        out.writeInt((int)this.mOrderAscensing); // error when compile
        out.writeString(this.mSource);
        out.writeInt((int)this.mDownloaded);  // error when compile
        out.writeString(this.mText);
    }

Java generate enum:

public enum SearchParamOrderAscendingTypeEnum {
    IGNORE,
    ASCENDING,
    DESCENDING,
    ;
}

I fix it manually using:

    @Override
    public void writeToParcel(android.os.Parcel out, int flags) {
        out.writeList(this.mCatalogTypeList);
        out.writeString(this.mOrderField);
        out.writeInt((int)this.mOrderAscensing.ordinal());
        out.writeString(this.mSource);
        out.writeInt((int)this.mDownloaded.ordinal());
        out.writeString(this.mText);
    }

Can you fix it please?

Thanks.

artwyman commented 5 years ago

This feature was contributed by @4brunu in #308, so perhaps he can take a look.

4brunu commented 5 years ago

Hum, maybe I didn't test for this case, @prsolucoes do you want to try to fix it and create a PR? I'm a bit busy, but I can try to fix it once I have time.

4brunu commented 5 years ago

@prsolucoes Are you using the last version of djinni, from master branch? I was testing this, and it's working in my test case

    public ItemList(android.os.Parcel in) {
        this.mOrder = SortOrder.values()[in.readInt()];
    }

    @Override
    public void writeToParcel(android.os.Parcel out, int flags) {
        out.writeInt(this.mOrder.ordinal());
    }

Can you share a djinni example file? Thanks

paulocoutinhox commented 5 years ago

HI,

Yes, im using latest version: image

Djinni file:

search_param_downloaded_type_enum = enum {
    ignore;
    downloaded;
    not_downloaded;
}

search_param_order_ascending_type_enum = enum {
    ignore;
    ascending;
    descending;
}

my_product_search_options = record {
    catalog_type_list: list<string>;
    order_field: string;
    order_ascensing: search_param_order_ascending_type_enum;
    source: string;
    downloaded: search_param_downloaded_type_enum;
    text: string;
} deriving(parcelable)

Thanks.

4brunu commented 5 years ago

I just run the djinni file that you sent, and it's working in my computer.

    public MyProductSearchOptions(android.os.Parcel in) {
        this.mCatalogTypeList = new ArrayList<String>();
        in.readList(this.mCatalogTypeList, getClass().getClassLoader());
        this.mOrderField = in.readString();
        this.mOrderAscensing = SearchParamOrderAscendingTypeEnum.values()[in.readInt()];
        this.mSource = in.readString();
        this.mDownloaded = SearchParamDownloadedTypeEnum.values()[in.readInt()];
        this.mText = in.readString();
    }

    @Override
    public void writeToParcel(android.os.Parcel out, int flags) {
        out.writeList(this.mCatalogTypeList);
        out.writeString(this.mOrderField);
        out.writeInt(this.mOrderAscensing.ordinal());
        out.writeString(this.mSource);
        out.writeInt(this.mDownloaded.ordinal());
        out.writeString(this.mText);
    }
paulocoutinhox commented 5 years ago

Hi,

The two enums are imported from yaml files. Can be it?

@extern "../build/djinni/yaml/enums/search_param_downloaded_type_enum.yaml"
@extern "../build/djinni/yaml/enums/search_param_order_ascending_type_enum.yaml"

Thanks.

4brunu commented 5 years ago

Yes, that was the problem. I just created a PR https://github.com/dropbox/djinni/pull/435 that I think that fixes this issue. @prsolucoes can you please confirm that this PR fixes the issue for you? Thanks

paulocoutinhox commented 5 years ago

Hi,

Yes, i have generated all our djinni things with your new branch and now the java files are correct:

image

Thanks.

paulocoutinhox commented 5 years ago

@artwyman can you merge it?