grpc-gateway icon indicating copy to clipboard operation
grpc-gateway copied to clipboard

protoc-gen-swagger generates odd names for definitions.

Open GeertJohan opened this issue 7 years ago • 1 comments

I believe the names that protoc-gen-swagger generates for definitions can be improved.

Let's take the following proto3 doc:

package mypackage;

message Abc {}

message Foo{
	message Bar{
		message Qux {
			string username = 1;
		}
		Qux qux = 1;
	}
	Bar bar = 1;
}

sevice MyService{
	rpc MyMethod (Foo) returns (Abc) {
		option (google.api.http) = {
			post: "/myService/myMethod"
			body: "*"
		};
	}
}

In Go, this will generate the following structs:

struct Abc{….}
struct Foo{…}
struct Foo_Bar{…}
struct Foo_Bar_Qux{}

Which makes perfect sense to me. The names are

However, proto-gen-swagger generates these definitions:

  • mypackageAbc
  • mypackageFoo
  • FooBar
  • BarQux

It works.. But it's not as nice.. It has inconsistent prefixing of the package name and sub-messages don't have the full 'path' in their swagger definition name.

Perhaps the packagename as prefix to the message name is indeed a good idea to avoid cross-package naming problems. For example: rpc MyMethod (otherpackage.Foo)… could add a second Foo in the same swagger definitions list (?). So some form of package qualification is required. But I think it should then be done consistently. Also the complete message/submessage "path" should be in the definition name. Because what if Abc also has a sub-message Bar which has a submessage Qux? We would get two BarQux definitions in the same swagger.json.. That probably won't work well.

Just a quick though on how this maybe could work:

  • mypackageFoo
  • mypackageFoo_Bar
  • mypackageFoo_Bar_Qux
  • mypackageAbc

Possibly with an extra underscore between package name and first message name.

GeertJohan avatar Jul 03 '17 09:07 GeertJohan

This seems to me like a really great starter project for someone who wants to get involved. It seems to me that the problem comes from nested messages. I think the way you would go about doing this would be to modify findNestedMessagesAndEnumerations to take a current path parameter and have it record its recursion as it goes down.

@GeertJohan, would you be interested in taking this task on?

achew22 avatar Jul 04 '17 23:07 achew22