Browse Source

reflect/protodesc: check file imports by name

Weak imports are added to Imports as placeholders even if they can be
found in the Files registry, so we have to look at the name rather than
the actual FileDescriptor.

Change-Id: I28f62e945f233119014e5e8cb1bcbde7dca831a7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/175897
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
John Wright 6 years ago
parent
commit
bac4cd44d4
2 changed files with 41 additions and 6 deletions
  1. 6 6
      reflect/protodesc/protodesc.go
  2. 35 0
      reflect/protodesc/protodesc_test.go

+ 6 - 6
reflect/protodesc/protodesc.go

@@ -112,13 +112,13 @@ func NewFile(fd *descriptorpb.FileDescriptorProto, r *protoregistry.Files) (prot
 	return prototype.NewFile(&f)
 	return prototype.NewFile(&f)
 }
 }
 
 
-type importSet map[protoreflect.FileDescriptor]bool
+type importSet map[string]bool
 
 
 func importedFiles(imps []protoreflect.FileImport) importSet {
 func importedFiles(imps []protoreflect.FileImport) importSet {
 	ret := make(importSet)
 	ret := make(importSet)
 	for _, imp := range imps {
 	for _, imp := range imps {
-		ret[imp.FileDescriptor] = true
-		addPublicImports(imp.FileDescriptor, ret)
+		ret[imp.Path()] = true
+		addPublicImports(imp, ret)
 	}
 	}
 	return ret
 	return ret
 }
 }
@@ -128,8 +128,8 @@ func addPublicImports(fd protoreflect.FileDescriptor, out importSet) {
 	for i := 0; i < imps.Len(); i++ {
 	for i := 0; i < imps.Len(); i++ {
 		imp := imps.Get(i)
 		imp := imps.Get(i)
 		if imp.IsPublic {
 		if imp.IsPublic {
-			out[imp.FileDescriptor] = true
-			addPublicImports(imp.FileDescriptor, out)
+			out[imp.Path()] = true
+			addPublicImports(imp, out)
 		}
 		}
 	}
 	}
 }
 }
@@ -381,7 +381,7 @@ func validateFileInImports(d protoreflect.Descriptor, imps importSet) error {
 	if fd == nil {
 	if fd == nil {
 		return errors.New("%v has no parent FileDescriptor", d.FullName())
 		return errors.New("%v has no parent FileDescriptor", d.FullName())
 	}
 	}
-	if !imps[fd] {
+	if !imps[fd.Path()] {
 		return errors.New("reference to type %v without import of %v", d.FullName(), fd.Path())
 		return errors.New("reference to type %v without import of %v", d.FullName(), fd.Path())
 	}
 	}
 	return nil
 	return nil

+ 35 - 0
reflect/protodesc/protodesc_test.go

@@ -553,6 +553,41 @@ func TestNewFile_ValidationOK(t *testing.T) {
 				}},
 				}},
 			}},
 			}},
 		},
 		},
+	}, {
+		name: "external type from weak import",
+		deps: []*descriptorpb.FileDescriptorProto{{
+			Name:    scalar.String("weak.proto"),
+			Syntax:  scalar.String("proto2"),
+			Package: scalar.String("foo"),
+			MessageType: []*descriptorpb.DescriptorProto{{
+				Name: scalar.String("WeakMessage"),
+			}},
+		}},
+		fd: &descriptorpb.FileDescriptorProto{
+			Name:           scalar.String("external-type-from-weak-import.proto"),
+			Syntax:         scalar.String("proto2"),
+			Package:        scalar.String("bar"),
+			Dependency:     []string{"weak.proto"},
+			WeakDependency: []int32{0},
+			MessageType: []*descriptorpb.DescriptorProto{{
+				Name: scalar.String("Bar"),
+				Field: []*descriptorpb.FieldDescriptorProto{{
+					Name:   scalar.String("id"),
+					Number: scalar.Int32(1),
+					Label:  descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
+					Type:   descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
+				}, {
+					Name:     scalar.String("weak_message"),
+					Number:   scalar.Int32(2),
+					Label:    descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
+					Type:     descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
+					TypeName: scalar.String(".foo.WeakMessage"),
+					Options: &descriptorpb.FieldOptions{
+						Weak: scalar.Bool(true),
+					},
+				}},
+			}},
+		},
 	}}
 	}}
 
 
 	for _, tc := range testCases {
 	for _, tc := range testCases {