protoc-gen-validate icon indicating copy to clipboard operation
protoc-gen-validate copied to clipboard

0.6.3 importing enums causes compile to fail

Open jonrose-dev opened this issue 3 years ago • 11 comments
trafficstars

When importing an enum, compilation fails with the follow error:

[error] failed post-processing: 46:29: expected operand, found 'type' (and 10 more errors)
--validate_go_plugin_out: protoc-gen-validate_go_plugin: Plugin failed with status code 1.

Steps to reproduce:

  1. Proto structure:
/common/apis/type/a/a.proto
/common/apis/type/b/b.proto
/path/to/non-common/proto/c.proto
  1. a.proto looks like:
syntax = "proto3";

package common.apis.type;

option go_package = "github.com/ORG/common/apis/type/a;a";

enum A {
  A0 = 0;
  A1 = 1;
}
  1. b.proto looks like:
syntax = "proto3";

package common.apis.type;

option go_package = "github.com/ORG/common/apis/type/b;b";

enum B {
  B0 = 0;
  B1 = 1;
}
  1. c.proto looks like:
syntax = "proto3";
package c;
import "validate/validate.proto";
import "common/apis/type/a/a.proto";
import "common/apis/type/b/b.proto";

option go_package = "github.com/ORG/path/to/non-common/proto;cpb";

message Cs {
    repeated C items = 1;
}

message C {
    common.apis.type.A a = 0;
    common.apis.type.B b = 1;
}

service CService {
    rpc CreateBatch(Cs) returns (Cs) {};
}
  1. Generate Go code

Expected Result: c.pb.validate.go is generated Actual Result: c.pb.validate.go fails to generate with the error above

Notes

  1. This does work as expected with version 0.6.2
  2. This does work if I combine my enums into a single message and then import that message. e.g.
syntax = "proto3";

package common.apis.type;

option go_package = "github.com/ORG/common/apis/type/enums;enums";

message Enums {
    enum A {
        A0 = 0;
        A1 = 1;
    }

    enum B {
        B0 = 0;
        B1 = 1;
    }
} 

And use it in c.proto like:

syntax = "proto3";
package c;
import "validate/validate.proto";
import "common/apis/type/enums/enums.proto";

option go_package = "github.com/ORG/path/to/non-common/proto;cpb";

message Cs {
    repeated C items = 1;
}

message C {
    common.apis.type.Enums.A a = 0;
    common.apis.type.Enums.B b = 1;
}

service CService {
    rpc CreateBatch(Cs) returns (Cs) {};
}

This is not an ideal solution as it forces us to combine all of our common enums into a single file, and causes some redundancies in our usage of these enums in our Go code.

jonrose-dev avatar Jan 21 '22 17:01 jonrose-dev

👍

dgocoder avatar Jan 21 '22 17:01 dgocoder

👍

adriancodes avatar Jan 21 '22 18:01 adriancodes

Seeing the same issue

mgreg90 avatar Jan 21 '22 18:01 mgreg90

Seems to be caused by an assumption in the go file template that assumes enums are defined under a message type.

Following is an _ variable that tries to make sure the import is used somewhere.

	{{ range $pkg, $path := enumPackages (externalEnums .) }}
		_ = {{ $pkg }}.{{ (index (externalEnums $) 0).Parent.Name }}_{{ (index (externalEnums $) 0).Name }}(0)
	{{ end }}

The Parent.Name_Name pattern is only valid in one case, when the enum is defined under a message. Any other place an enum can be defined will break, producing different kinds of failures.

Error case 1: Enum on its own

If the enum is on its own, the Parent is a file and you get something like

// pkg1/foo.proto
enum Enum { }

// pkg2/foo.proto
import "pkg1/foo.proto"

// pkg2/foo.pb.validate.go
_ = pkg1.pkg1/pkg1.proto_Enum(0)

This is not valid go code, and the GoFmt post-processor cannot format it correctly, hence the error.

EDIT: on closer thought, it looks valid (valid division), but it won't compile. I think it becomes invalid when a keyword like type appears as one of the path elements, since go will try and interpret it as a divisor.

Error case 2: enum is under a nested message (two messages deep)

OTOH, if the enum is defined under a nested message (message.message.enum), The Parent is just M2, however the enum type includes both M1 and M2 (type M1_M2_Enum). The result is you get a reference to something that does not exist.

// pkg1/foo.proto
message M1 { message M2 { enum Enum { } } }

// pkg2/foo.proto
import "pkg1/foo.proto"

// pkg2/foo.pb.validate.go
_ = pkg1.M2_Enum(0) // This does not exist, instead 'M1_M2_Enum(0)' is the correct value to use.

This causes code that generates successfully but will not compile.

anzboi avatar Feb 06 '22 12:02 anzboi

Any suggestion?

wzyjerry avatar Mar 07 '22 02:03 wzyjerry

We've opted to group our enums into single enums message. This is far from an ideal solution and causes other problems, but they are at least problems we can work around. Wondering if there's any update on the actual fix for this issue?

jonrose-dev avatar Mar 22 '22 20:03 jonrose-dev

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 27 '22 19:04 stale[bot]

This is VERY much not a stale issue

jonrose-dev avatar May 05 '22 19:05 jonrose-dev

I got the same problem too.

leafs99 avatar May 13 '22 02:05 leafs99

So my solution was to clone the repo, fix it myself, build a binary and distribute that within my org. I was going to contribute the fix but never got round to it.

anzboi avatar May 13 '22 04:05 anzboi

hhh, I found it was fixed in https://github.com/envoyproxy/protoc-gen-validate/pull/573.

leafs99 avatar May 13 '22 08:05 leafs99