Fixes #3917: Add package openapi naming strategy. (#4372)

* Fix 3917: Add package openapi naming strategy.

Support passing "package" for openapi_naming_strategy to create
in-package unique names by qualifying nested types.

This strategy operates like "simple" except that we start by qualifying
nested types, so that nested type names stay stable as new types are
added in the same package.

* Move regexp compilation to initialization of global var.

* Tweak comment wording for consistency.

* Add package option to protoc_gen_openapiv2 bazel rule.

protoc-gen-openapiv2/defs.bzl

@@ -311,9 +311,9 @@ protoc_gen_openapiv2 = rule(
         "openapi_naming_strategy": attr.string(
             default = "",
             mandatory = False,
-            values = ["", "simple", "legacy", "fqn"],
+            values = ["", "simple", "package", "legacy", "fqn"],
             doc = "configures how OpenAPI names are determined." +
-                  " Allowed values are `` (empty), `simple`, `legacy` and `fqn`." +
+                  " Allowed values are `` (empty), `simple`, `package`, `legacy` and `fqn`." +
                   " If unset, either `legacy` or `fqn` are selected, depending" +
                   " on the value of the `fqn_for_openapi_name` setting",
         ),

protoc-gen-openapiv2/internal/genopenapi/naming.go

@@ -2,6 +2,7 @@ package genopenapi
 
 import (
 	"reflect"
+	"regexp"
 	"strings"
 )
 
@@ -17,6 +18,8 @@ func LookupNamingStrategy(strategyName string) func([]string) map[string]string
 		return resolveNamesLegacy
 	case "simple":
 		return resolveNamesSimple
+	case "package":
+		return resolveNamesPackage
 	}
 	return nil
 }
@@ -41,7 +44,7 @@ func resolveNamesFQN(messages []string) map[string]string {
 // E.g., if the fully qualified name is `.a.b.C.D`, and there are other messages with fully
 // qualified names ending in `.D` but not in `.C.D`, it assigns the unique name `bCD`.
 func resolveNamesLegacy(messages []string) map[string]string {
-	return resolveNamesUniqueWithContext(messages, 1, "")
+	return resolveNamesUniqueWithContext(messages, 1, "", false)
 }
 
 // resolveNamesSimple takes the names of all proto messages and generates unique references by using a simple
@@ -52,20 +55,48 @@ func resolveNamesLegacy(messages []string) map[string]string {
 // E.g., if the fully qualified name is `.a.b.C.D`, and there are other messages with
 // fully qualified names ending in `.D` but not in `.C.D`, it assigns the unique name `C.D`.
 func resolveNamesSimple(messages []string) map[string]string {
-	return resolveNamesUniqueWithContext(messages, 0, ".")
+	return resolveNamesUniqueWithContext(messages, 0, ".", false)
 }
 
+// resolveNamesPackage takes the names of all proto messages and generates unique references by
+// starting with the package-scoped name (with nested message types qualified by their containing
+// "parent" types), and then following the "simple" heuristic above to add package name components
+// until each message has a unique name with a "." between each component.
+//
+// E.g., if the fully qualified name is `.a.b.C.D`, the name is `C.D` unless there is another
+// package-scoped name ending in "C.D", in  which case it would be `b.C.D` (unless that also
+// conflicted, in which case the name would be the fully-qualified `a.b.C`).
+func resolveNamesPackage(messages []string) map[string]string {
+	return resolveNamesUniqueWithContext(messages, 0, ".", true)
+}
+
+// For the "package" naming strategy, we rely on the convention that package names are lowercase
+// but message names are capitalized.
+var pkgEndRegexp = regexp.MustCompile(`\.[A-Z]`)
+
 // Take the names of every proto message and generates a unique reference by:
 // first, separating each message name into its components by splitting at dots. Then,
 // take the shortest suffix slice from each components slice that is unique among all
 // messages, and convert it into a component name by taking extraContext additional
 // components into consideration and joining all components with componentSeparator.
-func resolveNamesUniqueWithContext(messages []string, extraContext int, componentSeparator string) map[string]string {
+func resolveNamesUniqueWithContext(messages []string, extraContext int, componentSeparator string, qualifyNestedMessages bool) map[string]string {
 	packagesByDepth := make(map[int][][]string)
 	uniqueNames := make(map[string]string)
 
 	hierarchy := func(pkg string) []string {
-		return strings.Split(pkg, ".")
+		if !qualifyNestedMessages {
+			return strings.Split(pkg, ".")
+		}
+		pkgEnd := pkgEndRegexp.FindStringIndex(pkg)
+		if pkgEnd == nil {
+			// Fall back to non-qualified behavior if search based on convention fails.
+			return strings.Split(pkg, ".")
+		}
+		// Return each package component as an element, followed by the full message name
+		// (potentially qualified, if nested) as a single element.
+		qualifiedPkgName := pkg[:pkgEnd[0]]
+		nestedTypeName := pkg[pkgEnd[0]+1:]
+		return append(strings.Split(qualifiedPkgName, "."), nestedTypeName)
 	}
 
 	for _, p := range messages {

protoc-gen-openapiv2/internal/genopenapi/naming_test.go

@@ -4,15 +4,15 @@ import "testing"
 
 func TestNaming(t *testing.T) {
 	type expectedNames struct {
-		fqn, legacy, simple string
+		fqn, legacy, simple, pkg string
 	}
 	messageNameToExpected := map[string]expectedNames{
-		".A":     {"A", "A", "A"},
-		".a.B.C": {"a.B.C", "aBC", "B.C"},
-		".a.D.C": {"a.D.C", "aDC", "D.C"},
-		".a.E.F": {"a.E.F", "aEF", "a.E.F"},
-		".b.E.F": {"b.E.F", "bEF", "b.E.F"},
-		".c.G.H": {"c.G.H", "GH", "H"},
+		".A":     {"A", "A", "A", "A"},
+		".a.B.C": {"a.B.C", "aBC", "B.C", "B.C"},
+		".a.D.C": {"a.D.C", "aDC", "D.C", "D.C"},
+		".a.E.F": {"a.E.F", "aEF", "a.E.F", "a.E.F"},
+		".b.E.F": {"b.E.F", "bEF", "b.E.F", "b.E.F"},
+		".c.G.H": {"c.G.H", "GH", "H", "G.H"},
 	}
 
 	allMessageNames := make([]string, 0, len(messageNameToExpected))
@@ -50,4 +50,14 @@ func TestNaming(t *testing.T) {
 			}
 		}
 	})
+	t.Run("package", func(t *testing.T) {
+		uniqueNames := resolveNamesPackage(allMessageNames)
+		for _, msgName := range allMessageNames {
+			expected := messageNameToExpected[msgName].pkg
+			actual := uniqueNames[msgName]
+			if expected != actual {
+				t.Errorf("package unique name %q does not match expected name %q", actual, expected)
+			}
+		}
+	})
 }

protoc-gen-openapiv2/main.go

@@ -29,7 +29,7 @@ var (
 	_                              = flag.Bool("allow_repeated_fields_in_body", true, "allows to use repeated field in `body` and `response_body` field of `google.api.http` annotation option. DEPRECATED: the value is ignored and always behaves as `true`.")
 	includePackageInTags           = flag.Bool("include_package_in_tags", false, "if unset, the gRPC service name is added to the `Tags` field of each operation. If set and the `package` directive is shown in the proto file, the package name will be prepended to the service name")
 	useFQNForOpenAPIName           = flag.Bool("fqn_for_openapi_name", false, "if set, the object's OpenAPI names will use the fully qualified names from the proto definition (ie my.package.MyMessage.MyInnerMessage). DEPRECATED: prefer `openapi_naming_strategy=fqn`")
-	openAPINamingStrategy          = flag.String("openapi_naming_strategy", "", "use the given OpenAPI naming strategy. Allowed values are `legacy`, `fqn`, `simple`. If unset, either `legacy` or `fqn` are selected, depending on the value of the `fqn_for_openapi_name` flag")
+	openAPINamingStrategy          = flag.String("openapi_naming_strategy", "", "use the given OpenAPI naming strategy. Allowed values are `legacy`, `fqn`, `simple`, `package`. If unset, either `legacy` or `fqn` are selected, depending on the value of the `fqn_for_openapi_name` flag")
 	useGoTemplate                  = flag.Bool("use_go_templates", false, "if set, you can use Go templates in protofile comments")
 	goTemplateArgs                 = utilities.StringArrayFlag(flag.CommandLine, "go_template_args", "provide a custom value that can override a key in the Go template. Requires the `use_go_templates` option to be set")
 	ignoreComments                 = flag.Bool("ignore_comments", false, "if set, all protofile comments are excluded from output")