Allow default json_name on extensions (#164)

In some cases, generated code will contain a `json_name` on extension values although this is not generally allowed. Update protocompile to relax this check to match protoc's behavior when the `json_name` value is equal to the default computed value for the field: * https://github.com/protocolbuffers/protobuf/blob/3e1967e10be786062ccd026275866c3aef487eba/src/google/protobuf/descriptor.cc#L6717-L6725
bufbuild · Jul 25, 2023 · 4b4eff5 · 4b4eff5
1 parent 472c6fb
commit 4b4eff5
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 7 deletions.
diff --git a/linker/linker_test.go b/linker/linker_test.go
@@ -706,6 +706,16 @@ func TestLinkerValidation(t *testing.T) {
 			},
 			expectedErr: "foo.proto:4:26: field foobar: option json_name is not allowed on extensions",
 		},
+		"success_json_name_extension_default": {
+			input: map[string]string{
+				"foo.proto": `
+					syntax = "proto3";
+					import "google/protobuf/descriptor.proto";
+					extend google.protobuf.MessageOptions {
+					  string foobar = 10001 [json_name="foobar"];
+					}`,
+			},
+		},
 		"failure_json_name_looks_like_extension": {
 			input: map[string]string{
 				"foo.proto": `

diff --git a/options/options.go b/options/options.go
@@ -325,22 +325,24 @@ func (interp *interpreter) interpretFieldOptions(fqn string, fld *descriptorpb.F
 	if index >= 0 {
 		opt := uo[index]
 		optNode := interp.file.OptionNode(opt)
-		if fld.GetExtendee() != "" {
+		if opt.StringValue == nil {
+			return interp.reporter.HandleErrorf(interp.nodeInfo(optNode.GetValue()).Start(), "%s: expecting string value for json_name option", scope)
+		}
+		jsonName := string(opt.StringValue)
+		// Extensions don't support custom json_name values.
+		// If the value is already set (via the descriptor) and doesn't match the default value, return an error.
+		if fld.GetExtendee() != "" && jsonName != "" && jsonName != internal.JSONName(fld.GetName()) {
 			return interp.reporter.HandleErrorf(interp.nodeInfo(optNode.GetName()).Start(), "%s: option json_name is not allowed on extensions", scope)
 		}
 		// attribute source code info
 		if on, ok := optNode.(*ast.OptionNode); ok {
 			interp.index[on] = &sourceinfo.OptionSourceInfo{Path: []int32{-1, internal.FieldJSONNameTag}}
 		}
 		uo = internal.RemoveOption(uo, index)
-		if opt.StringValue == nil {
-			return interp.reporter.HandleErrorf(interp.nodeInfo(optNode.GetValue()).Start(), "%s: expecting string value for json_name option", scope)
-		}
-		name := string(opt.StringValue)
-		if strings.HasPrefix(name, "[") && strings.HasSuffix(name, "]") {
+		if strings.HasPrefix(jsonName, "[") && strings.HasSuffix(jsonName, "]") {
 			return interp.reporter.HandleErrorf(interp.nodeInfo(optNode.GetValue()).Start(), "%s: option json_name value cannot start with '[' and end with ']'; that is reserved for representing extensions", scope)
 		}
-		fld.JsonName = proto.String(name)
+		fld.JsonName = proto.String(jsonName)
 	}
 
 	// and process default pseudo-option