search

elixir-grpc / grpc-reflection

elixir graph reflection support
Apache License 2.0
9 stars 6 forks source link

Incorrect Package Name Parsing #20

Closed zhihuizhang17 closed 8 months ago

zhihuizhang17 commented 8 months ago

Issue

The package_from_name/1 method in Builder.Util currently extracts an incorrect package name when given a nested message name.

For example:

>  Util.package_from_name("testServiceV3.TestRequest.Payload")
"testServiceV3.TestRequest"

It's supposed to return: testServiceV3

Cause

Unlike protoc-gen-golang that retains file package info in a .pb.go FileDescriptorProto, protoc-gen-elixir builds a DescriptorProto in .pb.ex, losing the original package name and options from the .proto.

No clear way at present to parse correct package name.

Proposal

Potential options:

mjheilmann commented 8 months ago

@zhihuizhang17 I did a quick dig, can you update your listed problem with this result instead of the result you listed?

>  Util.package_from_name("TestServiceV3.TestRequest.Payload")
"TestServiceV3.TestRequest"

Can you also specify how you determined what your expected package name is? from https://protobuf.dev/programming-guides/proto3/#packages:

The way a package specifier affects the generated code depends on your chosen language:

  • In C++ the generated classes are wrapped inside a C++ namespace. For example, Open would be in the namespace foo::bar.
  • In Java and Kotlin, the package is used as the Java package, unless you explicitly provide an option java_package in your .proto file. In Python, the package directive is ignored, since Python modules are organized according to their location in the file system.
  • In Go, the package directive is ignored, and the generated .pb.go file is in the package named after the corresponding go_proto_library Bazel rule. For open source projects, you must provide either a gopackage option or set the Bazel -M flag. In Ruby, the generated classes are wrapped inside nested Ruby namespaces, converted to the required Ruby capitalization style (first letter capitalized; if the first character is not a letter, PB is prepended). For example, Open would be in the namespace Foo::Bar.
  • In PHP the package is used as the namespace after converting to PascalCase, unless you explicitly provide an option php_namespace in your .proto file. For example, Open would be in the namespace Foo\Bar. In C# the package is used as the namespace after converting to PascalCase, unless you explicitly provide an option csharp_namespace in your .proto file. For example, Open would be in the namespace Foo.Bar.

This reads like the handling of the package directive, or the absence of it, is messy at best. It also states that the package directive is optional and is only used to prevent name clashes.

zhihuizhang17 commented 8 months ago

Can you also specify how you determined what your expected package name is

I had previously thought it was a recommended best practice to retain consistency between the file descriptor package names and those defined in the original .proto files. Now I realize that maybe there does not seem to be an official specification requiring this consistency.