Repository: jhump/protoreflect (go)

Issue:
protoparse: ParseFiles doesn't catch duplicate oneof names
ParseFiles does not oneof names for duplicates.

```protobuf
syntax = "proto3";
message m{
  oneof z{
    int64 z=1;
  }
}

// protoc error:
// a.proto:4:11: "z" is already defined in "m".

```

```proto
syntax="proto3";
message m{
  string z = 1;
  oneof z{int64 b=2;}
}

// protoc error:
// a.proto:3:10: "z" is already defined in "m".

```

```proto
syntax="proto3";
message m{
  oneof z{int64 a=1;}
  oneof z{int64 b=2;}
}

// protoc error:
// a.proto: "z" is already defined in "m".

```




Pull request description:
protoparse: disallow duplicate oneof names
fixes #454

This makes ParseFiles error on duplicate oneof names.

Here's the examples from #454 with protoparse error messages:

```protobuf
syntax = "proto3";
message m{
  oneof z{
    int64 z=1;
  }
}

// protoc error:
// a.proto:4:11: "z" is already defined in "m".

// protoparse error:
// a.proto:4:5: duplicate symbol m.z: already defined as oneof
```

```proto
syntax="proto3";
message m{
  string z = 1;
  oneof z{int64 b=2;}
}

// protoc error:
// a.proto:3:10: "z" is already defined in "m".

// protoparse error:
// a.proto:3:3: duplicate symbol m.z: already defined as oneof

```

```proto
syntax="proto3";
message m{
  oneof z{int64 a=1;}
  oneof z{int64 b=2;}
}

// protoc error:
// a.proto: "z" is already defined in "m".

// protoparse error:
// a.proto:4:3: duplicate symbol m.z: already defined as oneof
```

Patch:
diff --git a/desc/protoparse/linker.go b/desc/protoparse/linker.go
index 4b4c1d8..4a1a4a5 100644
--- a/desc/protoparse/linker.go
+++ b/desc/protoparse/linker.go
@@ -180,6 +180,11 @@ func addMessageToPool(r *parseResult, pool map[string]proto.Message, errs *error
 		return err
 	}
 	prefix = fqn + "."
+	for _, ood := range md.OneofDecl {
+		if err := addOneofToPool(r, pool, errs, prefix, ood); err != nil {
+			return err
+		}
+	}
 	for _, fld := range md.Field {
 		if err := addFieldToPool(r, pool, errs, prefix, fld); err != nil {
 			return err
@@ -208,6 +213,11 @@ func addFieldToPool(r *parseResult, pool map[string]proto.Message, errs *errorHa
 	return addToPool(r, pool, errs, fqn, fld)
 }
 
+func addOneofToPool(r *parseResult, pool map[string]proto.Message, errs *errorHandler, prefix string, ood *dpb.OneofDescriptorProto) error {
+	fqn := prefix + ood.GetName()
+	return addToPool(r, pool, errs, fqn, ood)
+}
+
 func addEnumToPool(r *parseResult, pool map[string]proto.Message, errs *errorHandler, prefix string, ed *dpb.EnumDescriptorProto) error {
 	fqn := prefix + ed.GetName()
 	if err := addToPool(r, pool, errs, fqn, ed); err != nil {
@@ -281,6 +291,8 @@ func descriptorType(m proto.Message) string {
 		return "method"
 	case *dpb.FileDescriptorProto:
 		return "file"
+	case *dpb.OneofDescriptorProto:
+		return "oneof"
 	default:
 		// shouldn't be possible
 		return fmt.Sprintf("%T", m)


Test patch:
diff --git a/desc/protoparse/linker_test.go b/desc/protoparse/linker_test.go
index 657ac26..3324acd 100644
--- a/desc/protoparse/linker_test.go
+++ b/desc/protoparse/linker_test.go
@@ -535,6 +535,35 @@ func TestLinkerValidation(t *testing.T) {
 			},
 			`foo.proto:7:16: message Baz: option (foo).buzz: oneof "bar" already has field "baz" set`,
 		},
+		{
+			map[string]string{
+				"a.proto": "syntax = \"proto3\";\n" +
+					"message m{\n" +
+					"  oneof z{\n" +
+					"    int64 z=1;\n" +
+					"  }\n" +
+					"}",
+			},
+			`a.proto:4:5: duplicate symbol m.z: already defined as oneof`,
+		},
+		{
+			map[string]string{
+				"a.proto": "syntax=\"proto3\";\nmessage m{\n" +
+					"  string z = 1;\n" +
+					"  oneof z{int64 b=2;}\n" +
+					"}",
+			},
+			`a.proto:3:3: duplicate symbol m.z: already defined as oneof`,
+		},
+		{
+			map[string]string{
+				"a.proto": "syntax=\"proto3\";\nmessage m{\n" +
+					"  oneof z{int64 a=1;}\n" +
+					"  oneof z{int64 b=2;}\n" +
+					"}",
+			},
+			`a.proto:4:3: duplicate symbol m.z: already defined as oneof`,
+		},
 		{
 			map[string]string{
 				"foo.proto": "syntax = \"proto3\";\n" +