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): Generics on Enums #42

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

cmuramoto
Copy link
Contributor

@cmuramoto cmuramoto commented Apr 13, 2023

Consider the following proto file:

syntax = "proto3";

package foo;

option java_multiple_files = true;

enum CurrencyType {
    USD = 0;
    BRL = 1;
}

message Value {
    uint32 decimal = 1;
    uint32 cents = 2;
    CurrencyType type = 3;
}

The generated enum implements ProtoEnum with no type, but its associated converter is typed:

public enum CurrencyType implements ProtoEnum {
...
  enum CurrencyTypeConverter implements ProtoEnum.EnumConverter<CurrencyType> {
    INSTANCE;
  }

}

This leads to compiler issues at JsonSource::readEnum call sites. Excerpt for mergeFrom

 case 3575610: {
          if (input.isAtField(FieldNames.type)) {
            if (!input.trySkipNullValue()) {
              final ProtoEnum value = input.readEnum(CurrencyType.converter()); // compiler error on eclipse
              if (value != null) {
                type = value.getNumber();
                bitField0_ |= 0x00000004;
              } else {
                input.skipUnknownEnumValue();
              }
            }
          } else {
            input.skipUnknownField();
          }
          break;
        }

Either the converter should be implemented as a raw type or enum should not implement ProtoMessage as a raw type.

@ennerf ennerf changed the base branch from main to develop April 14, 2023 10:21
@ennerf ennerf merged commit 76922de into HebiRobotics:develop Apr 14, 2023
@ennerf
Copy link
Collaborator

ennerf commented Apr 14, 2023

That was an oversight. Thanks.

@ennerf ennerf mentioned this pull request Apr 14, 2023
@ennerf
Copy link
Collaborator

ennerf commented Apr 14, 2023

I just did a 1.1.1 release that among other things includes this PR and the collisions you mentioned before

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.

2 participants