alta / protopatch Goto Github PK

Proto3 support for optional has been added, see https://github.com/protocolbuffers/protobuf/blob/master/docs/implementing_proto3_presence.md

protopatch does not have support for it:

tests/plugin/validate.proto: is a proto3 file that contains optional fields, but code generator protoc-gen-go-patch hasn't been updated to support optional fields in proto3. Please ask the owner of this code generator to support proto3 optional.--go-patch_out: 
make: *** [tests/plugin/validate.proto] Error 1

Support can be declared using this snippet in main.go:

supportedFeatures := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL)
res.SupportedFeatures = &supportedFeatures

// Write the patched CodeGeneratorResponse to stdout.
return patch.WriteResponse(os.Stdout, res)

But introducing optional keyword in the test proto files breaks code generation.

Firstly, thanks a lot for this project, it's really great.

Provided awesome/v1/entity.proto:

package awesome.v1;

import "patch/go.proto";

option (go.lint).all = true;

enum MyEnum {
   MY_ENUM_UNSPECIFIED = 0;
}

It generates idiomatic Go identifiers 👍 on entity.pb.go:

const (
      MyEnumUnspecified MyEnum = 0
)

And then awesome/api/v1/api.proto:

package awesome.api.v1;

import "patch/go.proto";
import "awesome/v1/entity.proto"

option (go.lint).all = true;

message MessageWithMyEnum {
    awesome.v1.MyEnum my_enum = 0;
}

But at generate this message, it fails to use the patched identifier on api.pb.go:

func (x *MessageWithMyEnum) GetMyEnum() v1.MyEnum {
    if x != nil {
          return x.MyEnum
    }
    return v1.MY_ENUM_UNSPECIFIED
}

This is worked around with this code:

enum MyEnum {
   MY_ENUM_UNSPECIFIED = 0 [(go.value) = {name:'MyEnumUnspecified'}];
}

Thanks!

Errors in Go are usually prefixed with Err, and adding a custom name for each enum value is cumbersome and error prone

It seems something like "prefix" feature would make life a bit easier

Compare

enum Errors {
	option (go.enum.options) = {name: 'ProtocolErrors'};
	INVALID = 1 [(go.value.options) = {name: 'ErrInvalid'}];
	NOT_FOUND = 2 [(go.value.options) = {name: 'ErrNotFound'}];
	TOO_FUN = 3 [(go.value.options) = {name: 'ErrTooFun'}];
}

and

enum Errors {
	option (go.enum.options) = {name: 'ProtocolErrors', prefix: 'Err'};
	INVALID = 1;
	NOT_FOUND = 2;
	TOO_FUN = 3;
}

should produce same results.

(I think (might be mistaken though) that contraction rules will replace NOT_FOUND and TOO_FUN with NotFound and TooFun correspondingly.)

Thank you very much!

(Found through gogo/protobuf#691 (comment))

sorry

I am trying to use this package by importing using go mod. I see the it requires following from go.mod

google.golang.org/protobuf v1.31.0

But, I checked from the source that descriptor.proto is missing in that

hence I see compile errors when I do protoc

google/protobuf/descriptor.proto: File not found.
patch/go.proto: Import "google/protobuf/descriptor.proto" was not found or had errors.
patch/go.proto:46:8: "google.protobuf.MessageOptions" is not defined.
patch/go.proto:50:8: "google.protobuf.FieldOptions" is not defined.
patch/go.proto:54:8: "google.protobuf.OneofOptions" is not defined.
patch/go.proto:58:8: "google.protobuf.EnumOptions" is not defined.
patch/go.proto:62:8: "google.protobuf.EnumValueOptions" is not defined.
patch/go.proto:95:8: "google.protobuf.FileOptions" is not defined.
greet.proto: Import "patch/go.proto" was not found or had errors.

How to fix this?

A proto file imported from another proto file using import public ... will have its exported types aliased in the generated Go code, and vars copied. Currently the Go code generator doesn’t check to see if the imported symbols clobber existing symbols in the importing package.

• Ensure symbols imported using import public are renamed when aliased or copied into the destination package.
• Consider an option to rename the symbol in an importing package.

We often use other plugins like protoc-gen-validate or protoc-gen-grpc-gateway, but tools renaming fields usually breaks the generated code. The chosen approach by protopatch may solve this issue but does not work right now.
I did implement a naive cache that store protoc-gen-go output and reload it before patching generated code by other plugins, allowing the ast tree to find the type's declarations. I can create a pull request if you wish.
Another approach would have been to re-run protoc-gen-go plugin before each (other) plugin call to retrieve the types declaration.
I don't know which would be best.
Any thought ?

If multiple packages have the same "leaf" package name (package1/messages/, package2/messages/), then the linting will sometimes be skipped for one of the packages.

It's easiest to describe with a small example:

v1/a/test.proto:

syntax = "proto3";
package test.v1.a;
option go_package = "foo/bar/v1/a";

v2/a/anothertest.proto:

syntax = "proto3";
package test.v1.a;
option go_package = "foo/bar/v2/a";

import "patch/go.proto";
option (go.lint).all = true;

enum SomeEnum {
  SOME_ENUM_UNDEFINED = 0;
  SOME_ENUM_FOO = 1;
}

Then, depending on the order the proto files are supplied to protoc, the generated enum might not be linted.

this failed to lint the enum:
protoc -I . --go-patch_out=plugin=go,paths=source_relative:.make v2\a\anothertest.proto v1\a\test.proto
this works:
protoc -I . --go-patch_out=plugin=go,paths=source_relative:.make v2\a\test.proto v1\a\anothertest.proto

I think this is because Patcher.packagesByName is only keyed by the "leaf" package name, and thus gets overwritten.

Is it possible to write protopatch annotations in a separate YML file, instead of in the Proto file? That way I can avoid importing the protopatch Proto file, which is great if I want to distribute the Proto files to others so they can compile them for their platform/language.

This looks great @ydnar ... thanks for doing this.

I’m wondering why use several different options for this; can’t this be done with a single option go.options for all the things, and figure out at “patch” time which type of patch it should be?

Alternatively, if this isn’t possible or desirable, I think it would be nicer to have a single import for all options.

--go-patch_out: protoc-gen-go-patch: Plugin failed with status code 1.
makefile:

.PHONY: enums
# generate enums
enums:
	protoc --proto_path=./api \
           --proto_path=./third_party \
           --experimental_allow_proto3_optional \
           --go-patch_out=plugin=go,paths=source_relative:./api \
	       $(ENUM_PROTO_FILES)

For example...

• expected:example_ids -> ExampleIDs
• actual: example_ids -> ExampleIds

Doesn't look like you can fix this with the lint's initialisms setting, either.