Fix Import collisions (take default imports into an account) (#859)

.gitignore

@@ -22,6 +22,8 @@ __pycache__/
 /tests/harness/cases/other_package/gogo
 /tests/harness/cases/yet_another_package/go
 /tests/harness/cases/yet_another_package/gogo
+/tests/harness/cases/sort/go
+/tests/harness/cases/sort/gogo
 /tests/harness/go/harness.pb.go
 /tests/harness/go/main/go-harness
 /tests/harness/go/main/go-harness.exe

Makefile

@@ -78,6 +78,8 @@ testcases: bin/protoc-gen-go ## generate the test harness case protos
 	mkdir tests/harness/cases/other_package/go
 	rm -r tests/harness/cases/yet_another_package/go || true
 	mkdir tests/harness/cases/yet_another_package/go
+	rm -r tests/harness/cases/sort/go || true
+	mkdir tests/harness/cases/sort/go
 	# protoc-gen-go makes us go a package at a time
 	cd tests/harness/cases/other_package && \
 	protoc \
@@ -95,6 +97,14 @@ testcases: bin/protoc-gen-go ## generate the test harness case protos
 		--plugin=protoc-gen-go=$(shell pwd)/bin/protoc-gen-go \
 		--validate_out="module=${PACKAGE}/tests/harness/cases/yet_another_package/go,lang=go:./go" \
 		./*.proto
+	cd tests/harness/cases/sort && \
+	protoc \
+		-I . \
+		-I ../../../.. \
+		--go_out="module=${PACKAGE}/tests/harness/cases/sort/go,${GO_IMPORT}:./go" \
+		--plugin=protoc-gen-go=$(shell pwd)/bin/protoc-gen-go \
+		--validate_out="module=${PACKAGE}/tests/harness/cases/sort/go,lang=go:./go" \
+		./*.proto
 	cd tests/harness/cases && \
 	protoc \
 		-I . \

java/pgv-java-validation/pom.xml

@@ -66,6 +66,7 @@
                                 <include>tests/harness/cases/*.proto</include>
                                 <include>tests/harness/cases/other_package/*.proto</include>
                                 <include>tests/harness/cases/yet_another_package/*.proto</include>
+                                <include>tests/harness/cases/sort/*.proto</include>
                             </includes>
                         </configuration>
                     </execution>

java/pgv-java-validation/src/main/java/io/envoyproxy/pgv/validation/BUILD

@@ -14,6 +14,8 @@ java_library(
         "//tests/harness/cases/other_package:java",
         "//tests/harness/cases/yet_another_package:embed_java_proto",
         "//tests/harness/cases/yet_another_package:java",
+        "//tests/harness/cases/sort:sort_java_proto",
+        "//tests/harness/cases/sort:java",
         "//validate:validate_java",
         "@com_google_guava//jar",
         "@com_google_protobuf//:protobuf_java",

templates/goshared/register.go

@@ -367,7 +367,21 @@ type NormalizedEnum struct {
 func (fns goSharedFuncs) enumPackages(enums []pgs.Enum) map[pgs.Name]NormalizedEnum {
 	out := make(map[pgs.Name]NormalizedEnum, len(enums))
 
-	nameCollision := make(map[pgs.Name]int)
+	// Start point from ./templates/go/file.go
+	nameCollision := map[pgs.Name]int{
+		"bytes":   0,
+		"errors":  0,
+		"fmt":     0,
+		"net":     0,
+		"mail":    0,
+		"url":     0,
+		"regexp":  0,
+		"sort":    0,
+		"strings": 0,
+		"time":    0,
+		"utf8":    0,
+		"anypb":   0,
+	}
 	nameNormalized := make(map[pgs.FilePath]struct{})
 
 	for _, en := range enums {
@@ -378,11 +392,11 @@ func (fns goSharedFuncs) enumPackages(enums []pgs.Enum) map[pgs.Name]NormalizedE
 
 		pkgName := fns.PackageName(en)
 
-		if normalized, ok := out[pkgName]; ok {
-			if normalized.FilePath != enImportPath {
-				nameCollision[pkgName] = nameCollision[pkgName] + 1
-				pkgName = pkgName + pgs.Name(strconv.Itoa(nameCollision[pkgName]))
-			}
+		if collision, ok := nameCollision[pkgName]; ok {
+			nameCollision[pkgName] = collision + 1
+			pkgName = pkgName + pgs.Name(strconv.Itoa(nameCollision[pkgName]))
+		} else {
+			nameCollision[pkgName] = 0
 		}
 
 		nameNormalized[enImportPath] = struct{}{}

tests/harness/cases/BUILD

@@ -35,6 +35,7 @@ proto_library(
     deps = [
         "//tests/harness/cases/other_package:embed_proto",
         "//tests/harness/cases/yet_another_package:embed_proto",
+        "//tests/harness/cases/sort:sort_proto",
         "//validate:validate_proto",
         "@com_google_protobuf//:any_proto",
         "@com_google_protobuf//:duration_proto",
@@ -50,6 +51,7 @@ pgv_go_proto_library(
     deps = [
         "//tests/harness/cases/other_package:go",
         "//tests/harness/cases/yet_another_package:go",
+        "//tests/harness/cases/sort:go",
         "@org_golang_google_protobuf//types/known/anypb:go_default_library",
         "@org_golang_google_protobuf//types/known/durationpb:go_default_library",
         "@org_golang_google_protobuf//types/known/timestamppb:go_default_library",
@@ -62,6 +64,7 @@ pgv_cc_proto_library(
     cc_deps = [
         "//tests/harness/cases/other_package:cc",
         "//tests/harness/cases/yet_another_package:cc",
+        "//tests/harness/cases/sort:cc",
     ],
     visibility = ["//tests:__subpackages__"],
     deps = [":cases_proto"],
@@ -79,6 +82,7 @@ pgv_java_proto_library(
         ":cases_java_proto",
         "//tests/harness/cases/other_package:java",
         "//tests/harness/cases/yet_another_package:java",
+        "//tests/harness/cases/sort:java",
     ],
     visibility = ["//visibility:public"],
     deps = [":cases_proto"],
@@ -111,6 +115,7 @@ py_proto_library(
         "//validate:validate_py",
         "//tests/harness/cases/other_package:embed_python_proto",
         "//tests/harness/cases/yet_another_package:embed_python_proto",
+        "//tests/harness/cases/sort:sort_python_proto",
         "@com_google_protobuf//:protobuf_python",
     ],
 )

tests/harness/cases/enums.proto

@@ -5,6 +5,7 @@ option go_package = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cas
 import "validate/validate.proto";
 import "tests/harness/cases/other_package/embed.proto";
 import "tests/harness/cases/yet_another_package/embed.proto";
+import "tests/harness/cases/sort/sort.proto";
 
 enum TestEnum {
     ZERO = 0;
@@ -44,6 +45,9 @@ message EnumExternal3 {
     other_package.Embed.FooNumber foo = 1 [(validate.rules).enum = { in: [0, 2] }];
     yet_another_package.Embed.BarNumber bar = 2 [(validate.rules).enum = { not_in: [1] }];
 }
+message EnumExternal4 {
+    sort.Direction sort_direction = 1 [(validate.rules).enum.const = 1];
+}
 
 message RepeatedEnumDefined { repeated TestEnum val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; }
 message RepeatedExternalEnumDefined { repeated other_package.Embed.Enumerated val = 1 [(validate.rules).repeated.items.enum.defined_only = true]; }

tests/harness/cases/sort/BUILD

@@ -0,0 +1,80 @@
+load("@rules_java//java:defs.bzl", "java_proto_library")
+load("@com_google_protobuf//:protobuf.bzl", "py_proto_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+load(
+    "//bazel:pgv_proto_library.bzl",
+    "pgv_cc_proto_library",
+    "pgv_go_proto_library",
+    "pgv_java_proto_library",
+)
+
+proto_library(
+    name = "sort_proto",
+    srcs = [
+        "sort.proto",
+    ],
+    visibility = ["//visibility:public"],
+    deps = ["//validate:validate_proto"],
+)
+
+pgv_go_proto_library(
+    name = "go",
+    importpath = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/sort/go",
+    proto = ":sort_proto",
+    deps = [
+        "@org_golang_google_protobuf//types/known/anypb:go_default_library",
+    ],
+)
+
+pgv_cc_proto_library(
+    name = "cc",
+    visibility = ["//tests:__subpackages__"],
+    deps = [":sort_proto"],
+)
+
+proto_library(
+    name = "sort_sort_proto",
+    srcs = ["sort.proto"],
+    visibility = ["//visibility:public"],
+    deps = ["//validate:validate_proto"],
+)
+
+go_proto_library(
+    name = "sort_sort_go_proto",
+    importpath = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/sort",
+    proto = ":sort_proto",
+    visibility = ["//visibility:public"],
+    deps = ["//validate:go_default_library"],
+)
+
+go_library(
+    name = "go_default_library",
+    embed = [":sort_sort_go_proto"],
+    importpath = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/sort",
+    visibility = ["//visibility:public"],
+)
+
+java_proto_library(
+    name = "sort_java_proto",
+    visibility = ["//visibility:public"],
+    deps = [":sort_proto"],
+)
+
+pgv_java_proto_library(
+    name = "java",
+    java_deps = [":sort_java_proto"],
+    visibility = ["//visibility:public"],
+    deps = [":sort_proto"],
+)
+
+py_proto_library(
+    name = "sort_python_proto",
+    srcs = ["sort.proto"],
+    visibility = ["//visibility:public"],
+    deps = [
+        "//validate:validate_py",
+        "@com_google_protobuf//:protobuf_python",
+    ],
+)

tests/harness/cases/sort/sort.proto

@@ -0,0 +1,10 @@
+syntax = "proto3";
+
+package tests.harness.cases.sort;
+option go_package = "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/sort/go;sort";
+
+enum Direction {
+    UNKNOWN_DIRECTION = 0;
+    ASC = 1;
+    DESC = 2;
+}

tests/harness/executor/BUILD

@@ -20,6 +20,7 @@ go_library(
         "//tests/harness/cases:go",
         "//tests/harness/cases/other_package:go",
         "//tests/harness/cases/yet_another_package:go",
+        "//tests/harness/cases/sort:go",
         "@org_golang_google_protobuf//proto:go_default_library",
         "@org_golang_google_protobuf//types/known/anypb:go_default_library",
         "@org_golang_google_protobuf//types/known/durationpb:go_default_library",

tests/harness/executor/cases.go

@@ -6,6 +6,7 @@ import (
 
 	cases "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/go"
 	other_package "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/other_package/go"
+	sort "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/sort/go"
 	yet_another_package "github.com/envoyproxy/protoc-gen-validate/tests/harness/cases/yet_another_package/go"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/anypb"
@@ -1025,6 +1026,8 @@ var enumCases = []TestCase{
 	{"enum external - in - invalid", &cases.EnumExternal3{Foo: other_package.Embed_ONE}, 1},
 	{"enum external - not in - valid", &cases.EnumExternal3{Bar: yet_another_package.Embed_ZERO}, 0},
 	{"enum external - not in - invalid", &cases.EnumExternal3{Bar: yet_another_package.Embed_ONE}, 1},
+	{"enum external - const - valid", &cases.EnumExternal4{SortDirection: sort.Direction_ASC}, 0},
+	{"enum external - const - invalid", &cases.EnumExternal4{SortDirection: sort.Direction_DESC}, 1},
 
 	{"enum repeated - defined_only - valid", &cases.RepeatedEnumDefined{Val: []cases.TestEnum{cases.TestEnum_ONE, cases.TestEnum_TWO}}, 0},
 	{"enum repeated - defined_only - invalid", &cases.RepeatedEnumDefined{Val: []cases.TestEnum{cases.TestEnum_ONE, math.MaxInt32}}, 1},

tests/harness/go/main/BUILD

@@ -10,6 +10,7 @@ go_library(
         "//tests/harness/cases:go",
         "//tests/harness/cases/other_package:go",
         "//tests/harness/cases/yet_another_package:go",
+        "//tests/harness/cases/sort:go",
         "@org_golang_google_protobuf//proto:go_default_library",
         "@org_golang_google_protobuf//types/known/anypb:go_default_library",
         "@org_golang_google_protobuf//types/known/durationpb:go_default_library",