many validation rules previously unimplemented are now implemented (#542

)

thanks to underlying protoreflect.Descriptor impl being used to construct descriptors

desc/builder/builder_test.go

@@ -1576,3 +1576,111 @@ func TestPruneDependencies(t *testing.T) {
 	testutil.Eq(t, 1, len(newFileDesc.GetDependencies()))
 	testutil.Eq(t, extDesc.GetFile().GetName(), newFileDesc.GetDependencies()[0].GetName())
 }
+
+func TestInvalid(t *testing.T) {
+	testCases := []struct {
+		name          string
+		builder       func() Builder
+		expectedError string
+	}{
+		{
+			name: "required in proto3",
+			builder: func() Builder {
+				return NewFile("foo.proto").SetProto3(true).
+					AddMessage(
+						NewMessage("Foo").AddField(NewField("foo", FieldTypeBool()).SetRequired()),
+					)
+			},
+			expectedError: "proto3 does not allow required fields",
+		},
+		{
+			name: "extension range in proto3",
+			builder: func() Builder {
+				return NewFile("foo.proto").SetProto3(true).
+					AddMessage(
+						NewMessage("Foo").AddExtensionRange(100, 1000),
+					)
+			},
+			expectedError: "proto3 semantics cannot have extension ranges",
+		},
+		{
+			name: "group in proto3",
+			builder: func() Builder {
+				return NewFile("foo.proto").SetProto3(true).
+					AddMessage(
+						NewMessage("Foo").AddField(NewGroupField(NewMessage("Bar"))),
+					)
+			},
+			// NB: This is the actual error message returned by the protobuf runtime. It is
+			//     misleading since it says proto2 instead of proto3.
+			expectedError: "invalid group: invalid under proto2 semantics",
+		},
+		{
+			name: "default value in proto3",
+			builder: func() Builder {
+				return NewFile("foo.proto").SetProto3(true).
+					AddMessage(
+						NewMessage("Foo").AddField(NewField("foo", FieldTypeString()).SetDefaultValue("abc")),
+					)
+			},
+			expectedError: "invalid default: cannot be specified under proto3 semantics",
+		},
+		{
+			name: "extension tag outside range",
+			builder: func() Builder {
+				msg := NewMessage("Foo").AddExtensionRange(100, 1000)
+				return NewFile("foo.proto").
+					AddMessage(msg).
+					AddExtension(NewExtension("foo", 1, FieldTypeString(), msg))
+			},
+			expectedError: "non-extension field number: 1",
+		},
+		{
+			name: "non-extension tag in extension range",
+			builder: func() Builder {
+				return NewFile("foo.proto").
+					AddMessage(NewMessage("Foo").
+						AddField(NewField("foo", FieldTypeBool()).SetNumber(100)).
+						AddExtensionRange(100, 1000))
+			},
+			expectedError: "number 100 in extension range",
+		},
+		{
+			name: "tag in reserved range",
+			builder: func() Builder {
+				return NewFile("foo.proto").
+					AddMessage(NewMessage("Foo").
+						AddField(NewField("foo", FieldTypeBool()).SetNumber(100)).
+						AddReservedRange(100, 1000))
+			},
+			expectedError: "must not use reserved number 100",
+		},
+		{
+			name: "field has reserved name",
+			builder: func() Builder {
+				return NewFile("foo.proto").
+					AddMessage(NewMessage("Foo").
+						AddField(NewField("foo", FieldTypeBool())).
+						AddReservedName("foo"))
+			},
+			expectedError: "must not use reserved name",
+		},
+		{
+			name: "ranges overlap",
+			builder: func() Builder {
+				return NewFile("foo.proto").
+					AddMessage(NewMessage("Foo").
+						AddReservedRange(100, 1000).
+						AddExtensionRange(200, 300))
+			},
+			expectedError: "reserved and extension ranges has overlapping ranges",
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			_, err := testCase.builder().BuildDescriptor()
+			testutil.Nok(t, err)
+			testutil.Require(t, strings.Contains(err.Error(), testCase.expectedError), "unexpected error: want %q, got %q", testCase.expectedError, err.Error())
+		})
+	}
+}

desc/builder/doc.go

@@ -160,28 +160,25 @@
 //     cannot have negative values.
 //  6. Element names should include only underscore, letters, and numbers, and
 //     must begin with an underscore or letter.
-//
-// Validation rules that are *not* enforced by builders, and thus would be
-// allowed and result in illegal constructs, include the following:
-//
-//  1. Files with a syntax of proto3 are not allowed to have required fields.
-//  2. Files with a syntax of proto3 are not allowed to have messages that
+//  7. Files with a syntax of proto3 are not allowed to have required fields.
+//  8. Files with a syntax of proto3 are not allowed to have messages that
 //     define extension ranges.
-//  3. Files with a syntax of proto3 are not allowed to use groups.
-//  4. Files with a syntax of proto3 are not allowed to declare default values
+//  9. Files with a syntax of proto3 are not allowed to use groups.
+//  10. Files with a syntax of proto3 are not allowed to declare default values
 //     for fields.
-//  5. Names are supposed to be globally unique, even across multiple files
-//     if multiple files are defined in the same package.
-//  6. Extension fields must use tag numbers that are in an extension range
+//  11. Extension fields must use tag numbers that are in an extension range
 //     defined on the extended message.
-//  7. Multiple extensions for the same message cannot re-use tag numbers, even
-//     across multiple files.
-//  8. Non-extension fields are not allowed to use tags that lie in a message's
+//  12. Non-extension fields are not allowed to use tags that lie in a message's
 //     extension ranges or reserved ranges.
-//  9. Non-extension fields are not allowed to use names that the message has
+//  13. Non-extension fields are not allowed to use names that the message has
 //     marked as reserved.
-//  10. Extension ranges and reserved ranges must not overlap.
+//  14. Extension ranges and reserved ranges must not overlap.
 //
-// This list may change in the future, as more validation rules may be
-// implemented in the builders.
+// Validation rules that are *not* enforced by builders, and thus would be
+// allowed and result in illegal constructs, include the following:
+//
+//  1. Names are supposed to be globally unique, even across multiple files
+//     if multiple files are defined in the same package.
+//  2. Multiple extensions for the same message cannot re-use tag numbers, even
+//     across multiple files.
 package builder