all: split flags.ProtoLegacyWeak out of flags.ProtoLegacy

This is a no-op change in preparation of removing support for weak fields. It allows users to keep enabling ProtoLegacy in general, but also disable weak field support independently. For golang/protobuf#1666 Change-Id: Ic3cb90d937e21a817ddbbb36029274be8b5f2513 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/641635 Reviewed-by: Chressie Himpel <chressie@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
protocolbuffers · Jan 9, 2025 · 42e0fa9 · 42e0fa9
1 parent 5fee2a7
commit 42e0fa9
Show file tree

Hide file tree

Showing 11 changed files with 22 additions and 17 deletions.
diff --git a/encoding/protojson/decode.go b/encoding/protojson/decode.go
@@ -192,7 +192,7 @@ func (d decoder) unmarshalMessage(m protoreflect.Message, skipTypeURL bool) erro
 				fd = fieldDescs.ByTextName(name)
 			}
 		}
-		if flags.ProtoLegacy {
+		if flags.ProtoLegacyWeak {
 			if fd != nil && fd.IsWeak() && fd.Message().IsPlaceholder() {
 				fd = nil // reset since the weak reference is not linked in
 			}

diff --git a/encoding/protojson/decode_test.go b/encoding/protojson/decode_test.go
@@ -2684,13 +2684,13 @@ func TestUnmarshal(t *testing.T) {
 			m.SetWeakMessage1(&weakpb.WeakImportMessage1{A: proto.Int32(1)})
 			return m
 		}(),
-		skip: !flags.ProtoLegacy,
+		skip: !flags.ProtoLegacyWeak,
 	}, {
 		desc:         "weak fields; unknown field",
 		inputMessage: &testpb.TestWeak{},
 		inputText:    `{"weak_message1":{"a":1}, "weak_message2":{"a":1}}`,
 		wantErr:      `unknown field "weak_message2"`, // weak_message2 is unknown since the package containing it is not imported
-		skip:         !flags.ProtoLegacy,
+		skip:         !flags.ProtoLegacyWeak,
 	}, {
 		desc:         "just at recursion limit: nested messages",
 		inputMessage: &testpb.TestAllTypes{},

diff --git a/encoding/prototext/decode.go b/encoding/prototext/decode.go
@@ -185,7 +185,7 @@ func (d decoder) unmarshalMessage(m protoreflect.Message, checkDelims bool) erro
 		} else if xtErr != nil && xtErr != protoregistry.NotFound {
 			return d.newError(tok.Pos(), "unable to resolve [%s]: %v", tok.RawString(), xtErr)
 		}
-		if flags.ProtoLegacy {
+		if flags.ProtoLegacyWeak {
 			if fd != nil && fd.IsWeak() && fd.Message().IsPlaceholder() {
 				fd = nil // reset since the weak reference is not linked in
 			}

diff --git a/encoding/prototext/decode_test.go b/encoding/prototext/decode_test.go
@@ -1625,13 +1625,13 @@ type_url: "pb2.Nested"
 			m.SetWeakMessage1(&weakpb.WeakImportMessage1{A: proto.Int32(1)})
 			return m
 		}(),
-		skip: !flags.ProtoLegacy,
+		skip: !flags.ProtoLegacyWeak,
 	}, {
 		desc:         "weak fields; unknown field",
 		inputMessage: &testpb.TestWeak{},
 		inputText:    `weak_message1:{a:1} weak_message2:{a:1}`,
 		wantErr:      "unknown field: weak_message2", // weak_message2 is unknown since the package containing it is not imported
-		skip:         !flags.ProtoLegacy,
+		skip:         !flags.ProtoLegacyWeak,
 	}}
 
 	for _, msg := range makeMessages(protobuild.Message{},

diff --git a/internal/flags/flags.go b/internal/flags/flags.go
@@ -22,3 +22,8 @@ const ProtoLegacy = protoLegacy
 // extension fields at unmarshal time, but defers creating the message
 // structure until the extension is first accessed.
 const LazyUnmarshalExtensions = ProtoLegacy
+
+// ProtoLegacyWeak specifies whether to enable support for weak fields.
+// This flag was split out of ProtoLegacy in preparation for removing
+// support for weak fields (independent of the other protolegacy features).
+const ProtoLegacyWeak = ProtoLegacy
diff --git a/internal/impl/message_reflect_field.go b/internal/impl/message_reflect_field.go
@@ -333,7 +333,7 @@ func fieldInfoForScalar(fd protoreflect.FieldDescriptor, fs reflect.StructField,
 }
 
 func fieldInfoForWeakMessage(fd protoreflect.FieldDescriptor, weakOffset offset) fieldInfo {
-	if !flags.ProtoLegacy {
+	if !flags.ProtoLegacyWeak {
 		panic("no support for proto1 weak fields")
 	}
 

diff --git a/proto/checkinit_test.go b/proto/checkinit_test.go
@@ -73,15 +73,15 @@ func TestCheckInitializedErrors(t *testing.T) {
 	}, {
 		m:    &testpb.TestWeak{},
 		want: `<nil>`,
-		skip: !flags.ProtoLegacy,
+		skip: !flags.ProtoLegacyWeak,
 	}, {
 		m: func() proto.Message {
 			m := &testpb.TestWeak{}
 			m.SetWeakMessage1(&weakpb.WeakImportMessage1{})
 			return m
 		}(),
 		want: `goproto.proto.test.weak.WeakImportMessage1.a`,
-		skip: !flags.ProtoLegacy,
+		skip: !flags.ProtoLegacyWeak,
 	}, {
 		m: func() proto.Message {
 			m := &testpb.TestWeak{}
@@ -91,7 +91,7 @@ func TestCheckInitializedErrors(t *testing.T) {
 			return m
 		}(),
 		want: `<nil>`,
-		skip: !flags.ProtoLegacy,
+		skip: !flags.ProtoLegacyWeak,
 	}}
 
 	for _, tt := range tests {

diff --git a/proto/decode.go b/proto/decode.go
@@ -172,7 +172,7 @@ func (o UnmarshalOptions) unmarshalMessageSlow(b []byte, m protoreflect.Message)
 		var err error
 		if fd == nil {
 			err = errUnknown
-		} else if flags.ProtoLegacy {
+		} else if flags.ProtoLegacyWeak {
 			if fd.IsWeak() && fd.Message().IsPlaceholder() {
 				err = errUnknown // weak referent is not linked in
 			}

diff --git a/proto/weak_test.go b/proto/weak_test.go
@@ -17,7 +17,7 @@ import (
 )
 
 func init() {
-	if flags.ProtoLegacy {
+	if flags.ProtoLegacyWeak {
 		testValidMessages = append(testValidMessages, testWeakValidMessages...)
 		testInvalidMessages = append(testInvalidMessages, testWeakInvalidMessages...)
 		testMerges = append(testMerges, testWeakMerges...)
@@ -29,7 +29,7 @@ var testWeakValidMessages = []testProto{
 		desc: "weak message",
 		decodeTo: []proto.Message{
 			func() proto.Message {
-				if !flags.ProtoLegacy {
+				if !flags.ProtoLegacyWeak {
 					return nil
 				}
 				m := &testpb.TestWeak{}
@@ -98,7 +98,7 @@ var testWeakMerges = []testMerge{
 }
 
 func TestWeakNil(t *testing.T) {
-	if !flags.ProtoLegacy {
+	if !flags.ProtoLegacyWeak {
 		t.SkipNow()
 	}
 
@@ -109,7 +109,7 @@ func TestWeakNil(t *testing.T) {
 }
 
 func TestWeakMarshalNil(t *testing.T) {
-	if !flags.ProtoLegacy {
+	if !flags.ProtoLegacyWeak {
 		t.SkipNow()
 	}
 

diff --git a/reflect/protodesc/desc_validate.go b/reflect/protodesc/desc_validate.go
@@ -149,7 +149,7 @@ func validateMessageDeclarations(file *filedesc.File, ms []filedesc.Message, mds
 					return errors.New("message field %q under proto3 optional semantics must be within a single element oneof", f.FullName())
 				}
 			}
-			if f.IsWeak() && !flags.ProtoLegacy {
+			if f.IsWeak() && !flags.ProtoLegacyWeak {
 				return errors.New("message field %q is a weak field, which is a legacy proto1 feature that is no longer supported", f.FullName())
 			}
 			if f.IsWeak() && (!f.HasPresence() || !isOptionalMessage(f) || f.ContainingOneof() != nil) {

diff --git a/testing/prototest/prototest_test.go b/testing/prototest/prototest_test.go
@@ -36,7 +36,7 @@ func Test(t *testing.T) {
 		(*legacypb.Legacy)(nil),
 		protoimpl.X.MessageOf((*legacy1pb.Message)(nil)).Interface(),
 	}
-	if flags.ProtoLegacy {
+	if flags.ProtoLegacyWeak {
 		ms = append(ms, (*testpb.TestWeak)(nil))
 	}