diff options
Diffstat (limited to 'src/google/protobuf/descriptor_unittest.cc')
| -rw-r--r-- | src/google/protobuf/descriptor_unittest.cc | 1705 |
1 files changed, 1625 insertions, 80 deletions
diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index ec2c815..9b77fbe 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -36,13 +36,15 @@ #include <vector> +#include <google/protobuf/compiler/importer.h> +#include <google/protobuf/unittest.pb.h> +#include <google/protobuf/unittest_custom_options.pb.h> +#include <google/protobuf/io/zero_copy_stream_impl.h> +#include <google/protobuf/descriptor.pb.h> #include <google/protobuf/descriptor.h> #include <google/protobuf/descriptor_database.h> #include <google/protobuf/dynamic_message.h> -#include <google/protobuf/descriptor.pb.h> #include <google/protobuf/text_format.h> -#include <google/protobuf/unittest.pb.h> -#include <google/protobuf/unittest_custom_options.pb.h> #include <google/protobuf/stubs/strutil.h> #include <google/protobuf/stubs/substitute.h> @@ -644,6 +646,104 @@ TEST_F(DescriptorTest, FieldEnumType) { // =================================================================== +// Test simple flat messages and fields. +class OneofDescriptorTest : public testing::Test { + protected: + virtual void SetUp() { + // Build descriptors for the following definitions: + // + // package garply; + // message TestOneof { + // optional int32 a = 1; + // oneof foo { + // string b = 2; + // TestOneof c = 3; + // } + // oneof bar { + // float d = 4; + // } + // } + + FileDescriptorProto baz_file; + baz_file.set_name("baz.proto"); + baz_file.set_package("garply"); + + DescriptorProto* oneof_message = AddMessage(&baz_file, "TestOneof"); + oneof_message->add_oneof_decl()->set_name("foo"); + oneof_message->add_oneof_decl()->set_name("bar"); + + AddField(oneof_message, "a", 1, + FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_INT32); + AddField(oneof_message, "b", 2, + FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_STRING); + oneof_message->mutable_field(1)->set_oneof_index(0); + AddField(oneof_message, "c", 3, + FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_MESSAGE); + oneof_message->mutable_field(2)->set_oneof_index(0); + oneof_message->mutable_field(2)->set_type_name("TestOneof"); + + AddField(oneof_message, "d", 4, + FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_FLOAT); + oneof_message->mutable_field(3)->set_oneof_index(1); + + // Build the descriptors and get the pointers. + baz_file_ = pool_.BuildFile(baz_file); + ASSERT_TRUE(baz_file_ != NULL); + + ASSERT_EQ(1, baz_file_->message_type_count()); + oneof_message_ = baz_file_->message_type(0); + + ASSERT_EQ(2, oneof_message_->oneof_decl_count()); + oneof_ = oneof_message_->oneof_decl(0); + oneof2_ = oneof_message_->oneof_decl(1); + + ASSERT_EQ(4, oneof_message_->field_count()); + a_ = oneof_message_->field(0); + b_ = oneof_message_->field(1); + c_ = oneof_message_->field(2); + d_ = oneof_message_->field(3); + } + + DescriptorPool pool_; + + const FileDescriptor* baz_file_; + + const Descriptor* oneof_message_; + + const OneofDescriptor* oneof_; + const OneofDescriptor* oneof2_; + const FieldDescriptor* a_; + const FieldDescriptor* b_; + const FieldDescriptor* c_; + const FieldDescriptor* d_; + const FieldDescriptor* e_; + const FieldDescriptor* f_; +}; + +TEST_F(OneofDescriptorTest, Normal) { + EXPECT_EQ("foo", oneof_->name()); + EXPECT_EQ("garply.TestOneof.foo", oneof_->full_name()); + EXPECT_EQ(0, oneof_->index()); + ASSERT_EQ(2, oneof_->field_count()); + EXPECT_EQ(b_, oneof_->field(0)); + EXPECT_EQ(c_, oneof_->field(1)); + EXPECT_TRUE(a_->containing_oneof() == NULL); + EXPECT_EQ(oneof_, b_->containing_oneof()); + EXPECT_EQ(oneof_, c_->containing_oneof()); +} + +TEST_F(OneofDescriptorTest, FindByName) { + EXPECT_EQ(oneof_, oneof_message_->FindOneofByName("foo")); + EXPECT_EQ(oneof2_, oneof_message_->FindOneofByName("bar")); + EXPECT_TRUE(oneof_message_->FindOneofByName("no_such_oneof") == NULL); +} + +// =================================================================== + class StylizedFieldNamesTest : public testing::Test { protected: void SetUp() { @@ -1514,13 +1614,46 @@ TEST_F(ExtensionDescriptorTest, FindAllExtensions) { EXPECT_EQ(39, extensions[3]->number()); } +TEST_F(ExtensionDescriptorTest, DuplicateFieldNumber) { + DescriptorPool pool; + FileDescriptorProto file_proto; + // Add "google/protobuf/descriptor.proto". + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + // Add "foo.proto": + // import "google/protobuf/descriptor.proto"; + // extend google.protobuf.FieldOptions { + // optional int32 option1 = 1000; + // } + file_proto.Clear(); + file_proto.set_name("foo.proto"); + file_proto.add_dependency("google/protobuf/descriptor.proto"); + AddExtension(&file_proto, "google.protobuf.FieldOptions", "option1", 1000, + FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_INT32); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + // Add "bar.proto": + // import "google/protobuf/descriptor.proto"; + // extend google.protobuf.FieldOptions { + // optional int32 option2 = 1000; + // } + file_proto.Clear(); + file_proto.set_name("bar.proto"); + file_proto.add_dependency("google/protobuf/descriptor.proto"); + AddExtension(&file_proto, "google.protobuf.FieldOptions", "option2", 1000, + FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_INT32); + // Currently we only generate a warning for conflicting extension numbers. + // TODO(xiaofeng): Change it to an error. + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); +} + // =================================================================== class MiscTest : public testing::Test { protected: - // Function which makes a field of the given type just to find out what its - // cpp_type is. - FieldDescriptor::CppType GetCppTypeForFieldType(FieldDescriptor::Type type) { + // Function which makes a field descriptor of the given type. + const FieldDescriptor* GetFieldDescriptorOfType(FieldDescriptor::Type type) { FileDescriptorProto file_proto; file_proto.set_name("foo.proto"); AddEmptyEnum(&file_proto, "DummyEnum"); @@ -1538,19 +1671,99 @@ class MiscTest : public testing::Test { } // Build the descriptors and get the pointers. - DescriptorPool pool; - const FileDescriptor* file = pool.BuildFile(file_proto); + pool_.reset(new DescriptorPool()); + const FileDescriptor* file = pool_->BuildFile(file_proto); if (file != NULL && file->message_type_count() == 1 && file->message_type(0)->field_count() == 1) { - return file->message_type(0)->field(0)->cpp_type(); + return file->message_type(0)->field(0); } else { - return static_cast<FieldDescriptor::CppType>(0); + return NULL; } } + + const char* GetTypeNameForFieldType(FieldDescriptor::Type type) { + const FieldDescriptor* field = GetFieldDescriptorOfType(type); + return field != NULL ? field->type_name() : ""; + } + + FieldDescriptor::CppType GetCppTypeForFieldType(FieldDescriptor::Type type) { + const FieldDescriptor* field = GetFieldDescriptorOfType(type); + return field != NULL ? field->cpp_type() : + static_cast<FieldDescriptor::CppType>(0); + } + + const char* GetCppTypeNameForFieldType(FieldDescriptor::Type type) { + const FieldDescriptor* field = GetFieldDescriptorOfType(type); + return field != NULL ? field->cpp_type_name() : ""; + } + + const Descriptor* GetMessageDescriptorForFieldType( + FieldDescriptor::Type type) { + const FieldDescriptor* field = GetFieldDescriptorOfType(type); + return field != NULL ? field->message_type() : NULL; + } + + const EnumDescriptor* GetEnumDescriptorForFieldType( + FieldDescriptor::Type type) { + const FieldDescriptor* field = GetFieldDescriptorOfType(type); + return field != NULL ? field->enum_type() : NULL; + } + + scoped_ptr<DescriptorPool> pool_; }; +TEST_F(MiscTest, TypeNames) { + // Test that correct type names are returned. + + typedef FieldDescriptor FD; // avoid ugly line wrapping + + EXPECT_STREQ("double" , GetTypeNameForFieldType(FD::TYPE_DOUBLE )); + EXPECT_STREQ("float" , GetTypeNameForFieldType(FD::TYPE_FLOAT )); + EXPECT_STREQ("int64" , GetTypeNameForFieldType(FD::TYPE_INT64 )); + EXPECT_STREQ("uint64" , GetTypeNameForFieldType(FD::TYPE_UINT64 )); + EXPECT_STREQ("int32" , GetTypeNameForFieldType(FD::TYPE_INT32 )); + EXPECT_STREQ("fixed64" , GetTypeNameForFieldType(FD::TYPE_FIXED64 )); + EXPECT_STREQ("fixed32" , GetTypeNameForFieldType(FD::TYPE_FIXED32 )); + EXPECT_STREQ("bool" , GetTypeNameForFieldType(FD::TYPE_BOOL )); + EXPECT_STREQ("string" , GetTypeNameForFieldType(FD::TYPE_STRING )); + EXPECT_STREQ("group" , GetTypeNameForFieldType(FD::TYPE_GROUP )); + EXPECT_STREQ("message" , GetTypeNameForFieldType(FD::TYPE_MESSAGE )); + EXPECT_STREQ("bytes" , GetTypeNameForFieldType(FD::TYPE_BYTES )); + EXPECT_STREQ("uint32" , GetTypeNameForFieldType(FD::TYPE_UINT32 )); + EXPECT_STREQ("enum" , GetTypeNameForFieldType(FD::TYPE_ENUM )); + EXPECT_STREQ("sfixed32", GetTypeNameForFieldType(FD::TYPE_SFIXED32)); + EXPECT_STREQ("sfixed64", GetTypeNameForFieldType(FD::TYPE_SFIXED64)); + EXPECT_STREQ("sint32" , GetTypeNameForFieldType(FD::TYPE_SINT32 )); + EXPECT_STREQ("sint64" , GetTypeNameForFieldType(FD::TYPE_SINT64 )); +} + +TEST_F(MiscTest, StaticTypeNames) { + // Test that correct type names are returned. + + typedef FieldDescriptor FD; // avoid ugly line wrapping + + EXPECT_STREQ("double" , FD::TypeName(FD::TYPE_DOUBLE )); + EXPECT_STREQ("float" , FD::TypeName(FD::TYPE_FLOAT )); + EXPECT_STREQ("int64" , FD::TypeName(FD::TYPE_INT64 )); + EXPECT_STREQ("uint64" , FD::TypeName(FD::TYPE_UINT64 )); + EXPECT_STREQ("int32" , FD::TypeName(FD::TYPE_INT32 )); + EXPECT_STREQ("fixed64" , FD::TypeName(FD::TYPE_FIXED64 )); + EXPECT_STREQ("fixed32" , FD::TypeName(FD::TYPE_FIXED32 )); + EXPECT_STREQ("bool" , FD::TypeName(FD::TYPE_BOOL )); + EXPECT_STREQ("string" , FD::TypeName(FD::TYPE_STRING )); + EXPECT_STREQ("group" , FD::TypeName(FD::TYPE_GROUP )); + EXPECT_STREQ("message" , FD::TypeName(FD::TYPE_MESSAGE )); + EXPECT_STREQ("bytes" , FD::TypeName(FD::TYPE_BYTES )); + EXPECT_STREQ("uint32" , FD::TypeName(FD::TYPE_UINT32 )); + EXPECT_STREQ("enum" , FD::TypeName(FD::TYPE_ENUM )); + EXPECT_STREQ("sfixed32", FD::TypeName(FD::TYPE_SFIXED32)); + EXPECT_STREQ("sfixed64", FD::TypeName(FD::TYPE_SFIXED64)); + EXPECT_STREQ("sint32" , FD::TypeName(FD::TYPE_SINT32 )); + EXPECT_STREQ("sint64" , FD::TypeName(FD::TYPE_SINT64 )); +} + TEST_F(MiscTest, CppTypes) { // Test that CPP types are assigned correctly. @@ -1576,6 +1789,99 @@ TEST_F(MiscTest, CppTypes) { EXPECT_EQ(FD::CPPTYPE_INT64 , GetCppTypeForFieldType(FD::TYPE_SINT64 )); } +TEST_F(MiscTest, CppTypeNames) { + // Test that correct CPP type names are returned. + + typedef FieldDescriptor FD; // avoid ugly line wrapping + + EXPECT_STREQ("double" , GetCppTypeNameForFieldType(FD::TYPE_DOUBLE )); + EXPECT_STREQ("float" , GetCppTypeNameForFieldType(FD::TYPE_FLOAT )); + EXPECT_STREQ("int64" , GetCppTypeNameForFieldType(FD::TYPE_INT64 )); + EXPECT_STREQ("uint64" , GetCppTypeNameForFieldType(FD::TYPE_UINT64 )); + EXPECT_STREQ("int32" , GetCppTypeNameForFieldType(FD::TYPE_INT32 )); + EXPECT_STREQ("uint64" , GetCppTypeNameForFieldType(FD::TYPE_FIXED64 )); + EXPECT_STREQ("uint32" , GetCppTypeNameForFieldType(FD::TYPE_FIXED32 )); + EXPECT_STREQ("bool" , GetCppTypeNameForFieldType(FD::TYPE_BOOL )); + EXPECT_STREQ("string" , GetCppTypeNameForFieldType(FD::TYPE_STRING )); + EXPECT_STREQ("message", GetCppTypeNameForFieldType(FD::TYPE_GROUP )); + EXPECT_STREQ("message", GetCppTypeNameForFieldType(FD::TYPE_MESSAGE )); + EXPECT_STREQ("string" , GetCppTypeNameForFieldType(FD::TYPE_BYTES )); + EXPECT_STREQ("uint32" , GetCppTypeNameForFieldType(FD::TYPE_UINT32 )); + EXPECT_STREQ("enum" , GetCppTypeNameForFieldType(FD::TYPE_ENUM )); + EXPECT_STREQ("int32" , GetCppTypeNameForFieldType(FD::TYPE_SFIXED32)); + EXPECT_STREQ("int64" , GetCppTypeNameForFieldType(FD::TYPE_SFIXED64)); + EXPECT_STREQ("int32" , GetCppTypeNameForFieldType(FD::TYPE_SINT32 )); + EXPECT_STREQ("int64" , GetCppTypeNameForFieldType(FD::TYPE_SINT64 )); +} + +TEST_F(MiscTest, StaticCppTypeNames) { + // Test that correct CPP type names are returned. + + typedef FieldDescriptor FD; // avoid ugly line wrapping + + EXPECT_STREQ("int32" , FD::CppTypeName(FD::CPPTYPE_INT32 )); + EXPECT_STREQ("int64" , FD::CppTypeName(FD::CPPTYPE_INT64 )); + EXPECT_STREQ("uint32" , FD::CppTypeName(FD::CPPTYPE_UINT32 )); + EXPECT_STREQ("uint64" , FD::CppTypeName(FD::CPPTYPE_UINT64 )); + EXPECT_STREQ("double" , FD::CppTypeName(FD::CPPTYPE_DOUBLE )); + EXPECT_STREQ("float" , FD::CppTypeName(FD::CPPTYPE_FLOAT )); + EXPECT_STREQ("bool" , FD::CppTypeName(FD::CPPTYPE_BOOL )); + EXPECT_STREQ("enum" , FD::CppTypeName(FD::CPPTYPE_ENUM )); + EXPECT_STREQ("string" , FD::CppTypeName(FD::CPPTYPE_STRING )); + EXPECT_STREQ("message", FD::CppTypeName(FD::CPPTYPE_MESSAGE)); +} + +TEST_F(MiscTest, MessageType) { + // Test that message_type() is NULL for non-aggregate fields + + typedef FieldDescriptor FD; // avoid ugly line wrapping + + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_DOUBLE )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_FLOAT )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_INT64 )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_UINT64 )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_INT32 )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_FIXED64 )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_FIXED32 )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_BOOL )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_STRING )); + EXPECT_TRUE(NULL != GetMessageDescriptorForFieldType(FD::TYPE_GROUP )); + EXPECT_TRUE(NULL != GetMessageDescriptorForFieldType(FD::TYPE_MESSAGE )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_BYTES )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_UINT32 )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_ENUM )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_SFIXED32)); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_SFIXED64)); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_SINT32 )); + EXPECT_TRUE(NULL == GetMessageDescriptorForFieldType(FD::TYPE_SINT64 )); +} + +TEST_F(MiscTest, EnumType) { + // Test that enum_type() is NULL for non-enum fields + + typedef FieldDescriptor FD; // avoid ugly line wrapping + + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_DOUBLE )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_FLOAT )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_INT64 )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_UINT64 )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_INT32 )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_FIXED64 )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_FIXED32 )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_BOOL )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_STRING )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_GROUP )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_MESSAGE )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_BYTES )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_UINT32 )); + EXPECT_TRUE(NULL != GetEnumDescriptorForFieldType(FD::TYPE_ENUM )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_SFIXED32)); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_SFIXED64)); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_SINT32 )); + EXPECT_TRUE(NULL == GetEnumDescriptorForFieldType(FD::TYPE_SINT64 )); +} + + TEST_F(MiscTest, DefaultValues) { // Test that setting default values works. FileDescriptorProto file_proto; @@ -1670,7 +1976,7 @@ TEST_F(MiscTest, DefaultValues) { message->field(3)->default_value_uint64()); EXPECT_EQ(4.5 , message->field(4)->default_value_float ()); EXPECT_EQ(10e100 , message->field(5)->default_value_double()); - EXPECT_EQ(true , message->field(6)->default_value_bool ()); + EXPECT_TRUE( message->field(6)->default_value_bool ()); EXPECT_EQ("hello" , message->field(7)->default_value_string()); EXPECT_EQ("\001\002\003" , message->field(8)->default_value_string()); EXPECT_EQ(enum_value_b , message->field(9)->default_value_enum ()); @@ -1693,7 +1999,7 @@ TEST_F(MiscTest, DefaultValues) { EXPECT_EQ(0 , message->field(14)->default_value_uint64()); EXPECT_EQ(0.0f , message->field(15)->default_value_float ()); EXPECT_EQ(0.0 , message->field(16)->default_value_double()); - EXPECT_EQ(false, message->field(17)->default_value_bool ()); + EXPECT_FALSE( message->field(17)->default_value_bool ()); EXPECT_EQ("" , message->field(18)->default_value_string()); EXPECT_EQ("" , message->field(19)->default_value_string()); EXPECT_EQ(enum_value_a, message->field(20)->default_value_enum()); @@ -1739,13 +2045,31 @@ TEST_F(MiscTest, FieldOptions) { } // =================================================================== +enum DescriptorPoolMode { + NO_DATABASE, + FALLBACK_DATABASE +}; -class AllowUnknownDependenciesTest : public testing::Test { +class AllowUnknownDependenciesTest + : public testing::TestWithParam<DescriptorPoolMode> { protected: + DescriptorPoolMode mode() { + return GetParam(); + } + virtual void SetUp() { FileDescriptorProto foo_proto, bar_proto; - pool_.AllowUnknownDependencies(); + switch (mode()) { + case NO_DATABASE: + pool_.reset(new DescriptorPool); + break; + case FALLBACK_DATABASE: + pool_.reset(new DescriptorPool(&db_)); + break; + } + + pool_->AllowUnknownDependencies(); ASSERT_TRUE(TextFormat::ParseFromString( "name: 'foo.proto'" @@ -1776,13 +2100,13 @@ class AllowUnknownDependenciesTest : public testing::Test { &bar_proto)); // Collect pointers to stuff. - bar_file_ = pool_.BuildFile(bar_proto); + bar_file_ = BuildFile(bar_proto); ASSERT_TRUE(bar_file_ != NULL); ASSERT_EQ(1, bar_file_->message_type_count()); bar_type_ = bar_file_->message_type(0); - foo_file_ = pool_.BuildFile(foo_proto); + foo_file_ = BuildFile(foo_proto); ASSERT_TRUE(foo_file_ != NULL); ASSERT_EQ(1, foo_file_->message_type_count()); @@ -1794,6 +2118,20 @@ class AllowUnknownDependenciesTest : public testing::Test { qux_field_ = foo_type_->field(2); } + const FileDescriptor* BuildFile(const FileDescriptorProto& proto) { + switch (mode()) { + case NO_DATABASE: + return pool_->BuildFile(proto); + break; + case FALLBACK_DATABASE: { + EXPECT_TRUE(db_.Add(proto)); + return pool_->FindFileByName(proto.name()); + } + } + GOOGLE_LOG(FATAL) << "Can't get here."; + return NULL; + } + const FileDescriptor* bar_file_; const Descriptor* bar_type_; const FileDescriptor* foo_file_; @@ -1802,46 +2140,50 @@ class AllowUnknownDependenciesTest : public testing::Test { const FieldDescriptor* baz_field_; const FieldDescriptor* qux_field_; - DescriptorPool pool_; + SimpleDescriptorDatabase db_; // used if in FALLBACK_DATABASE mode. + scoped_ptr<DescriptorPool> pool_; }; -TEST_F(AllowUnknownDependenciesTest, PlaceholderFile) { +TEST_P(AllowUnknownDependenciesTest, PlaceholderFile) { ASSERT_EQ(2, foo_file_->dependency_count()); EXPECT_EQ(bar_file_, foo_file_->dependency(0)); + EXPECT_FALSE(bar_file_->is_placeholder()); const FileDescriptor* baz_file = foo_file_->dependency(1); EXPECT_EQ("baz.proto", baz_file->name()); EXPECT_EQ(0, baz_file->message_type_count()); + EXPECT_TRUE(baz_file->is_placeholder()); // Placeholder files should not be findable. - EXPECT_EQ(bar_file_, pool_.FindFileByName(bar_file_->name())); - EXPECT_TRUE(pool_.FindFileByName(baz_file->name()) == NULL); + EXPECT_EQ(bar_file_, pool_->FindFileByName(bar_file_->name())); + EXPECT_TRUE(pool_->FindFileByName(baz_file->name()) == NULL); } -TEST_F(AllowUnknownDependenciesTest, PlaceholderTypes) { +TEST_P(AllowUnknownDependenciesTest, PlaceholderTypes) { ASSERT_EQ(FieldDescriptor::TYPE_MESSAGE, bar_field_->type()); EXPECT_EQ(bar_type_, bar_field_->message_type()); + EXPECT_FALSE(bar_type_->is_placeholder()); ASSERT_EQ(FieldDescriptor::TYPE_MESSAGE, baz_field_->type()); const Descriptor* baz_type = baz_field_->message_type(); EXPECT_EQ("Baz", baz_type->name()); EXPECT_EQ("Baz", baz_type->full_name()); - EXPECT_EQ("Baz.placeholder.proto", baz_type->file()->name()); EXPECT_EQ(0, baz_type->extension_range_count()); + EXPECT_TRUE(baz_type->is_placeholder()); ASSERT_EQ(FieldDescriptor::TYPE_ENUM, qux_field_->type()); const EnumDescriptor* qux_type = qux_field_->enum_type(); EXPECT_EQ("Qux", qux_type->name()); EXPECT_EQ("corge.Qux", qux_type->full_name()); - EXPECT_EQ("corge.Qux.placeholder.proto", qux_type->file()->name()); + EXPECT_TRUE(qux_type->is_placeholder()); // Placeholder types should not be findable. - EXPECT_EQ(bar_type_, pool_.FindMessageTypeByName(bar_type_->full_name())); - EXPECT_TRUE(pool_.FindMessageTypeByName(baz_type->full_name()) == NULL); - EXPECT_TRUE(pool_.FindEnumTypeByName(qux_type->full_name()) == NULL); + EXPECT_EQ(bar_type_, pool_->FindMessageTypeByName(bar_type_->full_name())); + EXPECT_TRUE(pool_->FindMessageTypeByName(baz_type->full_name()) == NULL); + EXPECT_TRUE(pool_->FindEnumTypeByName(qux_type->full_name()) == NULL); } -TEST_F(AllowUnknownDependenciesTest, CopyTo) { +TEST_P(AllowUnknownDependenciesTest, CopyTo) { // FieldDescriptor::CopyTo() should write non-fully-qualified type names // for placeholder types which were not originally fully-qualified. FieldDescriptorProto proto; @@ -1864,7 +2206,7 @@ TEST_F(AllowUnknownDependenciesTest, CopyTo) { EXPECT_EQ(FieldDescriptorProto::TYPE_ENUM, proto.type()); } -TEST_F(AllowUnknownDependenciesTest, CustomOptions) { +TEST_P(AllowUnknownDependenciesTest, CustomOptions) { // Qux should still have the uninterpreted option attached. ASSERT_EQ(1, qux_field_->options().uninterpreted_option_size()); const UninterpretedOption& option = @@ -1873,7 +2215,7 @@ TEST_F(AllowUnknownDependenciesTest, CustomOptions) { EXPECT_EQ("grault", option.name(0).name_part()); } -TEST_F(AllowUnknownDependenciesTest, UnknownExtendee) { +TEST_P(AllowUnknownDependenciesTest, UnknownExtendee) { // Test that we can extend an unknown type. This is slightly tricky because // it means that the placeholder type must have an extension range. @@ -1884,19 +2226,20 @@ TEST_F(AllowUnknownDependenciesTest, UnknownExtendee) { "extension { extendee: 'UnknownType' name:'some_extension' number:123" " label:LABEL_OPTIONAL type:TYPE_INT32 }", &extension_proto)); - const FileDescriptor* file = pool_.BuildFile(extension_proto); + const FileDescriptor* file = BuildFile(extension_proto); ASSERT_TRUE(file != NULL); ASSERT_EQ(1, file->extension_count()); const Descriptor* extendee = file->extension(0)->containing_type(); EXPECT_EQ("UnknownType", extendee->name()); + EXPECT_TRUE(extendee->is_placeholder()); ASSERT_EQ(1, extendee->extension_range_count()); EXPECT_EQ(1, extendee->extension_range(0)->start); EXPECT_EQ(FieldDescriptor::kMaxNumber + 1, extendee->extension_range(0)->end); } -TEST_F(AllowUnknownDependenciesTest, CustomOption) { +TEST_P(AllowUnknownDependenciesTest, CustomOption) { // Test that we can use a custom option without having parsed // descriptor.proto. @@ -1937,7 +2280,7 @@ TEST_F(AllowUnknownDependenciesTest, CustomOption) { "}", &option_proto)); - const FileDescriptor* file = pool_.BuildFile(option_proto); + const FileDescriptor* file = BuildFile(option_proto); ASSERT_TRUE(file != NULL); // Verify that no extension options were set, but they were left as @@ -1949,6 +2292,81 @@ TEST_F(AllowUnknownDependenciesTest, CustomOption) { EXPECT_EQ(2, file->options().uninterpreted_option_size()); } +TEST_P(AllowUnknownDependenciesTest, + UndeclaredDependencyTriggersBuildOfDependency) { + // Crazy case: suppose foo.proto refers to a symbol without declaring the + // dependency that finds it. In the event that the pool is backed by a + // DescriptorDatabase, the pool will attempt to find the symbol in the + // database. If successful, it will build the undeclared dependency to verify + // that the file does indeed contain the symbol. If that file fails to build, + // then its descriptors must be rolled back. However, we still want foo.proto + // to build successfully, since we are allowing unknown dependencies. + + FileDescriptorProto undeclared_dep_proto; + // We make this file fail to build by giving it two fields with tag 1. + ASSERT_TRUE(TextFormat::ParseFromString( + "name: \"invalid_file_as_undeclared_dep.proto\" " + "package: \"undeclared\" " + "message_type: { " + " name: \"Quux\" " + " field { " + " name:'qux' number:1 label:LABEL_OPTIONAL type: TYPE_INT32 " + " }" + " field { " + " name:'quux' number:1 label:LABEL_OPTIONAL type: TYPE_INT64 " + " }" + "}", + &undeclared_dep_proto)); + // We can't use the BuildFile() helper because we don't actually want to build + // it into the descriptor pool in the fallback database case: it just needs to + // be sitting in the database so that it gets built during the building of + // test.proto below. + switch (mode()) { + case NO_DATABASE: { + ASSERT_TRUE(pool_->BuildFile(undeclared_dep_proto) == NULL); + break; + } + case FALLBACK_DATABASE: { + ASSERT_TRUE(db_.Add(undeclared_dep_proto)); + } + } + + FileDescriptorProto test_proto; + ASSERT_TRUE(TextFormat::ParseFromString( + "name: \"test.proto\" " + "message_type: { " + " name: \"Corge\" " + " field { " + " name:'quux' number:1 label: LABEL_OPTIONAL " + " type_name:'undeclared.Quux' type: TYPE_MESSAGE " + " }" + "}", + &test_proto)); + + const FileDescriptor* file = BuildFile(test_proto); + ASSERT_TRUE(file != NULL); + GOOGLE_LOG(INFO) << file->DebugString(); + + EXPECT_EQ(0, file->dependency_count()); + ASSERT_EQ(1, file->message_type_count()); + const Descriptor* corge_desc = file->message_type(0); + ASSERT_EQ("Corge", corge_desc->name()); + ASSERT_EQ(1, corge_desc->field_count()); + EXPECT_FALSE(corge_desc->is_placeholder()); + + const FieldDescriptor* quux_field = corge_desc->field(0); + ASSERT_EQ(FieldDescriptor::TYPE_MESSAGE, quux_field->type()); + ASSERT_EQ("Quux", quux_field->message_type()->name()); + ASSERT_EQ("undeclared.Quux", quux_field->message_type()->full_name()); + EXPECT_TRUE(quux_field->message_type()->is_placeholder()); + // The place holder type should not be findable. + ASSERT_TRUE(pool_->FindMessageTypeByName("undeclared.Quux") == NULL); +} + +INSTANTIATE_TEST_CASE_P(DatabaseSource, + AllowUnknownDependenciesTest, + testing::Values(NO_DATABASE, FALLBACK_DATABASE)); + // =================================================================== TEST(CustomOptions, OptionLocations) { @@ -2212,6 +2630,230 @@ TEST(CustomOptions, MessageOptionThreeFieldsSet) { EXPECT_EQ(1234, options.GetExtension(protobuf_unittest::complex_opt1).foo()); } +TEST(CustomOptions, MessageOptionRepeatedLeafFieldSet) { + // This test verifies that repeated fields in custom options can be + // given multiple values by repeating the option with a different value. + // This test checks repeated leaf values. Each repeated custom value + // appears in a different uninterpreted_option, which will be concatenated + // when they are merged into the final option value. + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + + protobuf_unittest::TestMessageWithCustomOptions::descriptor() + ->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + + // The following represents the definition: + // + // import "google/protobuf/unittest_custom_options.proto" + // package protobuf_unittest; + // message Foo { + // option (complex_opt1).foo4 = 12; + // option (complex_opt1).foo4 = 34; + // option (complex_opt1).foo4 = 56; + // } + ASSERT_TRUE(TextFormat::ParseFromString( + "name: \"custom_options_import.proto\" " + "package: \"protobuf_unittest\" " + "dependency: \"google/protobuf/unittest_custom_options.proto\" " + "message_type { " + " name: \"Foo\" " + " options { " + " uninterpreted_option { " + " name { " + " name_part: \"complex_opt1\" " + " is_extension: true " + " } " + " name { " + " name_part: \"foo4\" " + " is_extension: false " + " } " + " positive_int_value: 12 " + " } " + " uninterpreted_option { " + " name { " + " name_part: \"complex_opt1\" " + " is_extension: true " + " } " + " name { " + " name_part: \"foo4\" " + " is_extension: false " + " } " + " positive_int_value: 34 " + " } " + " uninterpreted_option { " + " name { " + " name_part: \"complex_opt1\" " + " is_extension: true " + " } " + " name { " + " name_part: \"foo4\" " + " is_extension: false " + " } " + " positive_int_value: 56 " + " } " + " } " + "}", + &file_proto)); + + const FileDescriptor* file = pool.BuildFile(file_proto); + ASSERT_TRUE(file != NULL); + ASSERT_EQ(1, file->message_type_count()); + + const MessageOptions& options = file->message_type(0)->options(); + EXPECT_EQ(3, options.GetExtension(protobuf_unittest::complex_opt1).foo4_size()); + EXPECT_EQ(12, options.GetExtension(protobuf_unittest::complex_opt1).foo4(0)); + EXPECT_EQ(34, options.GetExtension(protobuf_unittest::complex_opt1).foo4(1)); + EXPECT_EQ(56, options.GetExtension(protobuf_unittest::complex_opt1).foo4(2)); +} + +TEST(CustomOptions, MessageOptionRepeatedMsgFieldSet) { + // This test verifies that repeated fields in custom options can be + // given multiple values by repeating the option with a different value. + // This test checks repeated message values. Each repeated custom value + // appears in a different uninterpreted_option, which will be concatenated + // when they are merged into the final option value. + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + + protobuf_unittest::TestMessageWithCustomOptions::descriptor() + ->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + + // The following represents the definition: + // + // import "google/protobuf/unittest_custom_options.proto" + // package protobuf_unittest; + // message Foo { + // option (complex_opt2).barney = {waldo: 1}; + // option (complex_opt2).barney = {waldo: 10}; + // option (complex_opt2).barney = {waldo: 100}; + // } + ASSERT_TRUE(TextFormat::ParseFromString( + "name: \"custom_options_import.proto\" " + "package: \"protobuf_unittest\" " + "dependency: \"google/protobuf/unittest_custom_options.proto\" " + "message_type { " + " name: \"Foo\" " + " options { " + " uninterpreted_option { " + " name { " + " name_part: \"complex_opt2\" " + " is_extension: true " + " } " + " name { " + " name_part: \"barney\" " + " is_extension: false " + " } " + " aggregate_value: \"waldo: 1\" " + " } " + " uninterpreted_option { " + " name { " + " name_part: \"complex_opt2\" " + " is_extension: true " + " } " + " name { " + " name_part: \"barney\" " + " is_extension: false " + " } " + " aggregate_value: \"waldo: 10\" " + " } " + " uninterpreted_option { " + " name { " + " name_part: \"complex_opt2\" " + " is_extension: true " + " } " + " name { " + " name_part: \"barney\" " + " is_extension: false " + " } " + " aggregate_value: \"waldo: 100\" " + " } " + " } " + "}", + &file_proto)); + + const FileDescriptor* file = pool.BuildFile(file_proto); + ASSERT_TRUE(file != NULL); + ASSERT_EQ(1, file->message_type_count()); + + const MessageOptions& options = file->message_type(0)->options(); + EXPECT_EQ(3, options.GetExtension( + protobuf_unittest::complex_opt2).barney_size()); + EXPECT_EQ(1,options.GetExtension( + protobuf_unittest::complex_opt2).barney(0).waldo()); + EXPECT_EQ(10, options.GetExtension( + protobuf_unittest::complex_opt2).barney(1).waldo()); + EXPECT_EQ(100, options.GetExtension( + protobuf_unittest::complex_opt2).barney(2).waldo()); +} + +// Check that aggregate options were parsed and saved correctly in +// the appropriate descriptors. +TEST(CustomOptions, AggregateOptions) { + const Descriptor* msg = protobuf_unittest::AggregateMessage::descriptor(); + const FileDescriptor* file = msg->file(); + const FieldDescriptor* field = msg->FindFieldByName("fieldname"); + const EnumDescriptor* enumd = file->FindEnumTypeByName("AggregateEnum"); + const EnumValueDescriptor* enumv = enumd->FindValueByName("VALUE"); + const ServiceDescriptor* service = file->FindServiceByName( + "AggregateService"); + const MethodDescriptor* method = service->FindMethodByName("Method"); + + // Tests for the different types of data embedded in fileopt + const protobuf_unittest::Aggregate& file_options = + file->options().GetExtension(protobuf_unittest::fileopt); + EXPECT_EQ(100, file_options.i()); + EXPECT_EQ("FileAnnotation", file_options.s()); + EXPECT_EQ("NestedFileAnnotation", file_options.sub().s()); + EXPECT_EQ("FileExtensionAnnotation", + file_options.file().GetExtension(protobuf_unittest::fileopt).s()); + EXPECT_EQ("EmbeddedMessageSetElement", + file_options.mset().GetExtension( + protobuf_unittest::AggregateMessageSetElement + ::message_set_extension).s()); + + // Simple tests for all the other types of annotations + EXPECT_EQ("MessageAnnotation", + msg->options().GetExtension(protobuf_unittest::msgopt).s()); + EXPECT_EQ("FieldAnnotation", + field->options().GetExtension(protobuf_unittest::fieldopt).s()); + EXPECT_EQ("EnumAnnotation", + enumd->options().GetExtension(protobuf_unittest::enumopt).s()); + EXPECT_EQ("EnumValueAnnotation", + enumv->options().GetExtension(protobuf_unittest::enumvalopt).s()); + EXPECT_EQ("ServiceAnnotation", + service->options().GetExtension(protobuf_unittest::serviceopt).s()); + EXPECT_EQ("MethodAnnotation", + method->options().GetExtension(protobuf_unittest::methodopt).s()); +} + +TEST(CustomOptions, UnusedImportWarning) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + + protobuf_unittest::TestMessageWithCustomOptions::descriptor() + ->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != NULL); + + + pool.AddUnusedImportTrackFile("custom_options_import.proto"); + ASSERT_TRUE(TextFormat::ParseFromString( + "name: \"custom_options_import.proto\" " + "package: \"protobuf_unittest\" " + "dependency: \"google/protobuf/unittest_custom_options.proto\" ", + &file_proto)); + pool.BuildFile(file_proto); +} // =================================================================== @@ -2226,6 +2868,7 @@ class MockErrorCollector : public DescriptorPool::ErrorCollector { ~MockErrorCollector() {} string text_; + string warning_text_; // implements ErrorCollector --------------------------------------- void AddError(const string& filename, @@ -2249,6 +2892,29 @@ class MockErrorCollector : public DescriptorPool::ErrorCollector { &text_, "$0: $1: $2: $3\n", filename, element_name, location_name, message); } + + // implements ErrorCollector --------------------------------------- + void AddWarning(const string& filename, const string& element_name, + const Message* descriptor, ErrorLocation location, + const string& message) { + const char* location_name = NULL; + switch (location) { + case NAME : location_name = "NAME" ; break; + case NUMBER : location_name = "NUMBER" ; break; + case TYPE : location_name = "TYPE" ; break; + case EXTENDEE : location_name = "EXTENDEE" ; break; + case DEFAULT_VALUE: location_name = "DEFAULT_VALUE"; break; + case OPTION_NAME : location_name = "OPTION_NAME" ; break; + case OPTION_VALUE : location_name = "OPTION_VALUE" ; break; + case INPUT_TYPE : location_name = "INPUT_TYPE" ; break; + case OUTPUT_TYPE : location_name = "OUTPUT_TYPE" ; break; + case OTHER : location_name = "OTHER" ; break; + } + + strings::SubstituteAndAppend( + &warning_text_, "$0: $1: $2: $3\n", + filename, element_name, location_name, message); + } }; class ValidationErrorTest : public testing::Test { @@ -2275,6 +2941,19 @@ class ValidationErrorTest : public testing::Test { EXPECT_EQ(expected_errors, error_collector.text_); } + // Parse file_text as a FileDescriptorProto in text format and add it + // to the DescriptorPool. Expect errors to be produced which match the + // given warning text. + void BuildFileWithWarnings(const string& file_text, + const string& expected_warnings) { + FileDescriptorProto file_proto; + ASSERT_TRUE(TextFormat::ParseFromString(file_text, &file_proto)); + + MockErrorCollector error_collector; + EXPECT_TRUE(pool_.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(expected_warnings, error_collector.warning_text_); + } + // Builds some already-parsed file in our test pool. void BuildFileInTestPool(const FileDescriptor* file) { FileDescriptorProto file_proto; @@ -2412,6 +3091,15 @@ TEST_F(ValidationErrorTest, UnknownDependency) { "bar.proto: bar.proto: OTHER: Import \"foo.proto\" has not been loaded.\n"); } +TEST_F(ValidationErrorTest, InvalidPublicDependencyIndex) { + BuildFile("name: \"foo.proto\""); + BuildFileWithErrors( + "name: \"bar.proto\" " + "dependency: \"foo.proto\" " + "public_dependency: 1", + "bar.proto: bar.proto: OTHER: Invalid public dependency index.\n"); +} + TEST_F(ValidationErrorTest, ForeignUnimportedPackageNoCrash) { // Used to crash: If we depend on a non-existent file and then refer to a // package defined in a file that we didn't import, and that package is @@ -2519,8 +3207,8 @@ TEST_F(ValidationErrorTest, InvalidDefaults) { " default_value: \"1\" }" "}", - "foo.proto: Foo.foo: DEFAULT_VALUE: Couldn't parse default value.\n" - "foo.proto: Foo.bar: DEFAULT_VALUE: Couldn't parse default value.\n" + "foo.proto: Foo.foo: DEFAULT_VALUE: Couldn't parse default value \"abc\".\n" + "foo.proto: Foo.bar: DEFAULT_VALUE: Couldn't parse default value \"\".\n" "foo.proto: Foo.baz: DEFAULT_VALUE: Boolean default must be true or " "false.\n" "foo.proto: Foo.qux: DEFAULT_VALUE: Messages can't have default values.\n" @@ -2603,6 +3291,38 @@ TEST_F(ValidationErrorTest, NonExtensionWithExtendee) { "non-extension field.\n"); } +TEST_F(ValidationErrorTest, FieldOneofIndexTooLarge) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type:TYPE_INT32 " + " oneof_index: 1 }" + " field { name:\"dummy\" number:2 label:LABEL_OPTIONAL type:TYPE_INT32 " + " oneof_index: 0 }" + " oneof_decl { name:\"bar\" }" + "}", + + "foo.proto: Foo.foo: OTHER: FieldDescriptorProto.oneof_index 1 is out of " + "range for type \"Foo\".\n"); +} + +TEST_F(ValidationErrorTest, FieldOneofIndexNegative) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type:TYPE_INT32 " + " oneof_index: -1 }" + " field { name:\"dummy\" number:2 label:LABEL_OPTIONAL type:TYPE_INT32 " + " oneof_index: 0 }" + " oneof_decl { name:\"bar\" }" + "}", + + "foo.proto: Foo.foo: OTHER: FieldDescriptorProto.oneof_index -1 is out of " + "range for type \"Foo\".\n"); +} + TEST_F(ValidationErrorTest, FieldNumberConflict) { BuildFileWithErrors( "name: \"foo.proto\" " @@ -2763,6 +3483,28 @@ TEST_F(ValidationErrorTest, NotAnExtensionNumber) { "number.\n"); } +TEST_F(ValidationErrorTest, RequiredExtension) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "message_type {" + " name: \"Bar\"" + " extension_range { start: 1000 end: 10000 }" + "}" + "message_type {" + " name: \"Foo\"" + " extension {" + " name:\"foo\"" + " number:1000" + " label:LABEL_REQUIRED" + " type:TYPE_INT32" + " extendee: \"Bar\"" + " }" + "}", + + "foo.proto: Foo.foo: TYPE: Message extensions cannot have required " + "fields.\n"); +} + TEST_F(ValidationErrorTest, UndefinedFieldType) { BuildFileWithErrors( "name: \"foo.proto\" " @@ -2774,6 +3516,36 @@ TEST_F(ValidationErrorTest, UndefinedFieldType) { "foo.proto: Foo.foo: TYPE: \"Bar\" is not defined.\n"); } +TEST_F(ValidationErrorTest, UndefinedFieldTypeWithDefault) { + // See b/12533582. Previously this failed because the default value was not + // accepted by the parser, which assumed an enum type, leading to an unclear + // error message. We want this input to yield a validation error instead, + // since the unknown type is the primary problem. + BuildFileWithErrors( + "name: \"foo.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type_name:\"int\" " + " default_value:\"1\" }" + "}", + + "foo.proto: Foo.foo: TYPE: \"int\" is not defined.\n"); +} + +TEST_F(ValidationErrorTest, UndefinedNestedFieldType) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "message_type {" + " name: \"Foo\"" + " nested_type { name:\"Baz\" }" + " field { name:\"foo\" number:1" + " label:LABEL_OPTIONAL" + " type_name:\"Foo.Baz.Bar\" }" + "}", + + "foo.proto: Foo.foo: TYPE: \"Foo.Baz.Bar\" is not defined.\n"); +} + TEST_F(ValidationErrorTest, FieldTypeDefinedInUndeclaredDependency) { BuildFile( "name: \"bar.proto\" " @@ -2790,8 +3562,167 @@ TEST_F(ValidationErrorTest, FieldTypeDefinedInUndeclaredDependency) { "necessary import.\n"); } +TEST_F(ValidationErrorTest, FieldTypeDefinedInIndirectDependency) { + // Test for hidden dependencies. + // + // // bar.proto + // message Bar{} + // + // // forward.proto + // import "bar.proto" + // + // // foo.proto + // import "forward.proto" + // message Foo { + // optional Bar foo = 1; // Error, needs to import bar.proto explicitly. + // } + // + BuildFile( + "name: \"bar.proto\" " + "message_type { name: \"Bar\" }"); + + BuildFile( + "name: \"forward.proto\"" + "dependency: \"bar.proto\""); + + BuildFileWithErrors( + "name: \"foo.proto\" " + "dependency: \"forward.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type_name:\"Bar\" }" + "}", + "foo.proto: Foo.foo: TYPE: \"Bar\" seems to be defined in \"bar.proto\", " + "which is not imported by \"foo.proto\". To use it here, please add the " + "necessary import.\n"); +} + +TEST_F(ValidationErrorTest, FieldTypeDefinedInPublicDependency) { + // Test for public dependencies. + // + // // bar.proto + // message Bar{} + // + // // forward.proto + // import public "bar.proto" + // + // // foo.proto + // import "forward.proto" + // message Foo { + // optional Bar foo = 1; // Correct. "bar.proto" is public imported into + // // forward.proto, so when "foo.proto" imports + // // "forward.proto", it imports "bar.proto" too. + // } + // + BuildFile( + "name: \"bar.proto\" " + "message_type { name: \"Bar\" }"); + + BuildFile( + "name: \"forward.proto\"" + "dependency: \"bar.proto\" " + "public_dependency: 0"); + + BuildFile( + "name: \"foo.proto\" " + "dependency: \"forward.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type_name:\"Bar\" }" + "}"); +} + +TEST_F(ValidationErrorTest, FieldTypeDefinedInTransitivePublicDependency) { + // Test for public dependencies. + // + // // bar.proto + // message Bar{} + // + // // forward.proto + // import public "bar.proto" + // + // // forward2.proto + // import public "forward.proto" + // + // // foo.proto + // import "forward2.proto" + // message Foo { + // optional Bar foo = 1; // Correct, public imports are transitive. + // } + // + BuildFile( + "name: \"bar.proto\" " + "message_type { name: \"Bar\" }"); + + BuildFile( + "name: \"forward.proto\"" + "dependency: \"bar.proto\" " + "public_dependency: 0"); + + BuildFile( + "name: \"forward2.proto\"" + "dependency: \"forward.proto\" " + "public_dependency: 0"); + + BuildFile( + "name: \"foo.proto\" " + "dependency: \"forward2.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type_name:\"Bar\" }" + "}"); +} + +TEST_F(ValidationErrorTest, + FieldTypeDefinedInPrivateDependencyOfPublicDependency) { + // Test for public dependencies. + // + // // bar.proto + // message Bar{} + // + // // forward.proto + // import "bar.proto" + // + // // forward2.proto + // import public "forward.proto" + // + // // foo.proto + // import "forward2.proto" + // message Foo { + // optional Bar foo = 1; // Error, the "bar.proto" is not public imported + // // into "forward.proto", so will not be imported + // // into either "forward2.proto" or "foo.proto". + // } + // + BuildFile( + "name: \"bar.proto\" " + "message_type { name: \"Bar\" }"); + + BuildFile( + "name: \"forward.proto\"" + "dependency: \"bar.proto\""); + + BuildFile( + "name: \"forward2.proto\"" + "dependency: \"forward.proto\" " + "public_dependency: 0"); + + BuildFileWithErrors( + "name: \"foo.proto\" " + "dependency: \"forward2.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type_name:\"Bar\" }" + "}", + "foo.proto: Foo.foo: TYPE: \"Bar\" seems to be defined in \"bar.proto\", " + "which is not imported by \"foo.proto\". To use it here, please add the " + "necessary import.\n"); +} + + TEST_F(ValidationErrorTest, SearchMostLocalFirst) { - // The following should produce an error that Bar.Baz is not defined: + // The following should produce an error that Bar.Baz is resolved but + // not defined: // message Bar { message Baz {} } // message Foo { // message Bar { @@ -2817,7 +3748,10 @@ TEST_F(ValidationErrorTest, SearchMostLocalFirst) { " type_name:\"Bar.Baz\" }" "}", - "foo.proto: Foo.baz: TYPE: \"Bar.Baz\" is not defined.\n"); + "foo.proto: Foo.baz: TYPE: \"Bar.Baz\" is resolved to \"Foo.Bar.Baz\"," + " which is not defined. The innermost scope is searched first in name " + "resolution. Consider using a leading '.'(i.e., \".Bar.Baz\") to start " + "from the outermost scope.\n"); } TEST_F(ValidationErrorTest, SearchMostLocalFirst2) { @@ -2959,6 +3893,20 @@ TEST_F(ValidationErrorTest, BadEnumDefaultValue) { "\"NO_SUCH_VALUE\".\n"); } +TEST_F(ValidationErrorTest, EnumDefaultValueIsInteger) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "enum_type { name: \"Bar\" value { name:\"DUMMY\" number:0 } } " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_OPTIONAL type_name:\"Bar\"" + " default_value:\"0\" }" + "}", + + "foo.proto: Foo.foo: DEFAULT_VALUE: Default value for an enum field must " + "be an identifier.\n"); +} + TEST_F(ValidationErrorTest, PrimitiveWithTypeName) { BuildFileWithErrors( "name: \"foo.proto\" " @@ -2983,6 +3931,31 @@ TEST_F(ValidationErrorTest, NonPrimitiveWithoutTypeName) { "type_name.\n"); } +TEST_F(ValidationErrorTest, OneofWithNoFields) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "message_type {" + " name: \"Foo\"" + " oneof_decl { name:\"bar\" }" + "}", + + "foo.proto: Foo.bar: NAME: Oneof must have at least one field.\n"); +} + +TEST_F(ValidationErrorTest, OneofLabelMismatch) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "message_type {" + " name: \"Foo\"" + " field { name:\"foo\" number:1 label:LABEL_REPEATED type:TYPE_INT32 " + " oneof_index:0 }" + " oneof_decl { name:\"bar\" }" + "}", + + "foo.proto: Foo.foo: NAME: Fields of oneofs must themselves have label " + "LABEL_OPTIONAL.\n"); +} + TEST_F(ValidationErrorTest, InputTypeNotDefined) { BuildFileWithErrors( "name: \"foo.proto\" " @@ -2992,7 +3965,8 @@ TEST_F(ValidationErrorTest, InputTypeNotDefined) { " method { name: \"A\" input_type: \"Bar\" output_type: \"Foo\" }" "}", - "foo.proto: TestService.A: INPUT_TYPE: \"Bar\" is not defined.\n"); + "foo.proto: TestService.A: INPUT_TYPE: \"Bar\" is not defined.\n" + ); } TEST_F(ValidationErrorTest, InputTypeNotAMessage) { @@ -3005,7 +3979,8 @@ TEST_F(ValidationErrorTest, InputTypeNotAMessage) { " method { name: \"A\" input_type: \"Bar\" output_type: \"Foo\" }" "}", - "foo.proto: TestService.A: INPUT_TYPE: \"Bar\" is not a message type.\n"); + "foo.proto: TestService.A: INPUT_TYPE: \"Bar\" is not a message type.\n" + ); } TEST_F(ValidationErrorTest, OutputTypeNotDefined) { @@ -3017,7 +3992,8 @@ TEST_F(ValidationErrorTest, OutputTypeNotDefined) { " method { name: \"A\" input_type: \"Foo\" output_type: \"Bar\" }" "}", - "foo.proto: TestService.A: OUTPUT_TYPE: \"Bar\" is not defined.\n"); + "foo.proto: TestService.A: OUTPUT_TYPE: \"Bar\" is not defined.\n" + ); } TEST_F(ValidationErrorTest, OutputTypeNotAMessage) { @@ -3030,9 +4006,11 @@ TEST_F(ValidationErrorTest, OutputTypeNotAMessage) { " method { name: \"A\" input_type: \"Foo\" output_type: \"Bar\" }" "}", - "foo.proto: TestService.A: OUTPUT_TYPE: \"Bar\" is not a message type.\n"); + "foo.proto: TestService.A: OUTPUT_TYPE: \"Bar\" is not a message type.\n" + ); } + TEST_F(ValidationErrorTest, IllegalPackedField) { BuildFileWithErrors( "name: \"foo.proto\" " @@ -3139,20 +4117,85 @@ TEST_F(ValidationErrorTest, InvalidOptionName) { "reserved name \"uninterpreted_option\".\n"); } -TEST_F(ValidationErrorTest, RepeatedOption) { +TEST_F(ValidationErrorTest, RepeatedMessageOption) { BuildDescriptorMessagesInTestPool(); BuildFileWithErrors( "name: \"foo.proto\" " "dependency: \"google/protobuf/descriptor.proto\" " - "extension { name: \"foo\" number: 7672757 label: LABEL_REPEATED " - " type: TYPE_FLOAT extendee: \"google.protobuf.FileOptions\" }" - "options { uninterpreted_option { name { name_part: \"foo\" " + "message_type: { name: \"Bar\" field: { " + " name: \"foo\" number: 1 label: LABEL_OPTIONAL type: TYPE_INT32 } " + "} " + "extension { name: \"bar\" number: 7672757 label: LABEL_REPEATED " + " type: TYPE_MESSAGE type_name: \"Bar\" " + " extendee: \"google.protobuf.FileOptions\" }" + "options { uninterpreted_option { name { name_part: \"bar\" " " is_extension: true } " - " double_value: 1.2 } }", + " name { name_part: \"foo\" " + " is_extension: false } " + " positive_int_value: 1 } }", - "foo.proto: foo.proto: OPTION_NAME: Option field \"(foo)\" is repeated. " - "Repeated options are not supported.\n"); + "foo.proto: foo.proto: OPTION_NAME: Option field \"(bar)\" is a " + "repeated message. Repeated message options must be initialized " + "using an aggregate value.\n"); +} + +TEST_F(ValidationErrorTest, ResolveUndefinedOption) { + // The following should produce an eror that baz.bar is resolved but not + // defined. + // foo.proto: + // package baz + // import google/protobuf/descriptor.proto + // message Bar { optional int32 foo = 1; } + // extend FileOptions { optional Bar bar = 7672757; } + // + // qux.proto: + // package qux.baz + // option (baz.bar).foo = 1; + // + // Although "baz.bar" is already defined, the lookup code will try + // "qux.baz.bar", since it's the match from the innermost scope, which will + // cause a symbol not defined error. + BuildDescriptorMessagesInTestPool(); + + BuildFile( + "name: \"foo.proto\" " + "package: \"baz\" " + "dependency: \"google/protobuf/descriptor.proto\" " + "message_type: { name: \"Bar\" field: { " + " name: \"foo\" number: 1 label: LABEL_OPTIONAL type: TYPE_INT32 } " + "} " + "extension { name: \"bar\" number: 7672757 label: LABEL_OPTIONAL " + " type: TYPE_MESSAGE type_name: \"Bar\" " + " extendee: \"google.protobuf.FileOptions\" }"); + + BuildFileWithErrors( + "name: \"qux.proto\" " + "package: \"qux.baz\" " + "options { uninterpreted_option { name { name_part: \"baz.bar\" " + " is_extension: true } " + " name { name_part: \"foo\" " + " is_extension: false } " + " positive_int_value: 1 } }", + + "qux.proto: qux.proto: OPTION_NAME: Option \"(baz.bar)\" is resolved to " + "\"(qux.baz.bar)\"," + " which is not defined. The innermost scope is searched first in name " + "resolution. Consider using a leading '.'(i.e., \"(.baz.bar)\") to start " + "from the outermost scope.\n"); +} + +TEST_F(ValidationErrorTest, UnknownOption) { + BuildFileWithErrors( + "name: \"qux.proto\" " + "package: \"qux.baz\" " + "options { uninterpreted_option { name { name_part: \"baaz.bar\" " + " is_extension: true } " + " name { name_part: \"foo\" " + " is_extension: false } " + " positive_int_value: 1 } }", + + "qux.proto: qux.proto: OPTION_NAME: Option \"(baaz.bar)\" unknown.\n"); } TEST_F(ValidationErrorTest, CustomOptionConflictingFieldNumber) { @@ -3425,25 +4468,69 @@ TEST_F(ValidationErrorTest, StringOptionValueIsNotString) { "string option \"foo\".\n"); } -TEST_F(ValidationErrorTest, TryingToSetMessageValuedOption) { +TEST_F(ValidationErrorTest, DuplicateExtensionFieldNumber) { + BuildDescriptorMessagesInTestPool(); + + BuildFile( + "name: \"foo.proto\" " + "dependency: \"google/protobuf/descriptor.proto\" " + "extension { name: \"option1\" number: 1000 label: LABEL_OPTIONAL " + " type: TYPE_INT32 extendee: \"google.protobuf.FileOptions\" }"); + + BuildFileWithWarnings( + "name: \"bar.proto\" " + "dependency: \"google/protobuf/descriptor.proto\" " + "extension { name: \"option2\" number: 1000 label: LABEL_OPTIONAL " + " type: TYPE_INT32 extendee: \"google.protobuf.FileOptions\" }", + "bar.proto: option2: NUMBER: Extension number 1000 has already been used " + "in \"google.protobuf.FileOptions\" by extension \"option1\" defined in " + "foo.proto.\n"); +} + +// Helper function for tests that check for aggregate value parsing +// errors. The "value" argument is embedded inside the +// "uninterpreted_option" portion of the result. +static string EmbedAggregateValue(const char* value) { + return strings::Substitute( + "name: \"foo.proto\" " + "dependency: \"google/protobuf/descriptor.proto\" " + "message_type { name: \"Foo\" } " + "extension { name: \"foo\" number: 7672757 label: LABEL_OPTIONAL " + " type: TYPE_MESSAGE type_name: \"Foo\" " + " extendee: \"google.protobuf.FileOptions\" }" + "options { uninterpreted_option { name { name_part: \"foo\" " + " is_extension: true } " + " $0 } }", + value); +} + +TEST_F(ValidationErrorTest, AggregateValueNotFound) { + BuildDescriptorMessagesInTestPool(); + + BuildFileWithErrors( + EmbedAggregateValue("string_value: \"\""), + "foo.proto: foo.proto: OPTION_VALUE: Option \"foo\" is a message. " + "To set the entire message, use syntax like " + "\"foo = { <proto text format> }\". To set fields within it, use " + "syntax like \"foo.foo = value\".\n"); +} + +TEST_F(ValidationErrorTest, AggregateValueParseError) { BuildDescriptorMessagesInTestPool(); BuildFileWithErrors( - "name: \"foo.proto\" " - "dependency: \"google/protobuf/descriptor.proto\" " - "message_type { " - " name: \"TestMessage\" " - " field { name:\"baz\" number:1 label:LABEL_OPTIONAL type:TYPE_STRING }" - "}" - "extension { name: \"bar\" number: 7672757 label: LABEL_OPTIONAL " - " type: TYPE_MESSAGE type_name: \"TestMessage\" " - " extendee: \"google.protobuf.FileOptions\" }" - "options { uninterpreted_option { name { name_part: \"bar\" " - " is_extension: true } " - " identifier_value: \"QUUX\" } }", + EmbedAggregateValue("aggregate_value: \"1+2\""), + "foo.proto: foo.proto: OPTION_VALUE: Error while parsing option " + "value for \"foo\": Expected identifier.\n"); +} + +TEST_F(ValidationErrorTest, AggregateValueUnknownFields) { + BuildDescriptorMessagesInTestPool(); - "foo.proto: foo.proto: OPTION_NAME: Option field \"(bar)\" cannot be of " - "message type.\n"); + BuildFileWithErrors( + EmbedAggregateValue("aggregate_value: \"x:100\""), + "foo.proto: foo.proto: OPTION_VALUE: Error while parsing option " + "value for \"foo\": Message type \"Foo\" has no field named \"x\".\n"); } TEST_F(ValidationErrorTest, NotLiteImportsLite) { @@ -3483,11 +4570,25 @@ TEST_F(ValidationErrorTest, LiteExtendsNotLite) { TEST_F(ValidationErrorTest, NoLiteServices) { BuildFileWithErrors( "name: \"foo.proto\" " - "options { optimize_for: LITE_RUNTIME } " + "options {" + " optimize_for: LITE_RUNTIME" + " cc_generic_services: true" + " java_generic_services: true" + "} " "service { name: \"Foo\" }", "foo.proto: Foo: NAME: Files with optimize_for = LITE_RUNTIME cannot " - "define services.\n"); + "define services unless you set both options cc_generic_services and " + "java_generic_sevices to false.\n"); + + BuildFile( + "name: \"bar.proto\" " + "options {" + " optimize_for: LITE_RUNTIME" + " cc_generic_services: false" + " java_generic_services: false" + "} " + "service { name: \"Bar\" }"); } TEST_F(ValidationErrorTest, RollbackAfterError) { @@ -3514,7 +4615,8 @@ TEST_F(ValidationErrorTest, RollbackAfterError) { " }" "}", - "foo.proto: TestService.Baz: INPUT_TYPE: \"NoSuchType\" is not defined.\n"); + "foo.proto: TestService.Baz: INPUT_TYPE: \"NoSuchType\" is not defined.\n" + ); // Make sure that if we build the same file again with the error fixed, // it works. If the above rollback was incomplete, then some symbols will @@ -3563,6 +4665,78 @@ TEST_F(ValidationErrorTest, ErrorsReportedToLogError) { EXPECT_EQ(" Foo: \"Foo\" is already defined.", errors[1]); } +TEST_F(ValidationErrorTest, DisallowEnumAlias) { + BuildFileWithErrors( + "name: \"foo.proto\" " + "enum_type {" + " name: \"Bar\"" + " value { name:\"ENUM_A\" number:0 }" + " value { name:\"ENUM_B\" number:0 }" + "}", + "foo.proto: Bar: NUMBER: " + "\"ENUM_B\" uses the same enum value as \"ENUM_A\". " + "If this is intended, set 'option allow_alias = true;' to the enum " + "definition.\n"); +} + +TEST_F(ValidationErrorTest, AllowEnumAlias) { + BuildFile( + "name: \"foo.proto\" " + "enum_type {" + " name: \"Bar\"" + " value { name:\"ENUM_A\" number:0 }" + " value { name:\"ENUM_B\" number:0 }" + " options { allow_alias: true }" + "}"); +} + +TEST_F(ValidationErrorTest, UnusedImportWarning) { + + pool_.AddUnusedImportTrackFile("bar.proto"); + BuildFile( + "name: \"bar.proto\" " + "message_type { name: \"Bar\" }"); + + pool_.AddUnusedImportTrackFile("base.proto"); + BuildFile( + "name: \"base.proto\" " + "message_type { name: \"Base\" }"); + + pool_.AddUnusedImportTrackFile("baz.proto"); + BuildFile( + "name: \"baz.proto\" " + "message_type { name: \"Baz\" }"); + + pool_.AddUnusedImportTrackFile("public.proto"); + BuildFile( + "name: \"public.proto\" " + "dependency: \"bar.proto\"" + "public_dependency: 0"); + + // // forward.proto + // import "base.proto" // No warning: Base message is used. + // import "bar.proto" // Will log a warning. + // import public "baz.proto" // No warning: Do not track import public. + // import "public.proto" // No warning: public.proto has import public. + // message Forward { + // optional Base base = 1; + // } + // + pool_.AddUnusedImportTrackFile("forward.proto"); + BuildFile( + "name: \"forward.proto\"" + "dependency: \"base.proto\"" + "dependency: \"bar.proto\"" + "dependency: \"baz.proto\"" + "dependency: \"public.proto\"" + "public_dependency: 2 " + "message_type {" + " name: \"Forward\"" + " field { name:\"base\" number:1 label:LABEL_OPTIONAL type_name:\"Base\" }" + "}"); +} + + // =================================================================== // DescriptorDatabase @@ -3581,16 +4755,23 @@ class DatabaseBackedPoolTest : public testing::Test { virtual void SetUp() { AddToDatabase(&database_, - "name: \"foo.proto\" " - "message_type { name:\"Foo\" extension_range { start: 1 end: 100 } } " - "enum_type { name:\"TestEnum\" value { name:\"DUMMY\" number:0 } } " - "service { name:\"TestService\" } "); + "name: 'foo.proto' " + "message_type { name:'Foo' extension_range { start: 1 end: 100 } } " + "enum_type { name:'TestEnum' value { name:'DUMMY' number:0 } } " + "service { name:'TestService' } "); AddToDatabase(&database_, - "name: \"bar.proto\" " - "dependency: \"foo.proto\" " - "message_type { name:\"Bar\" } " - "extension { name:\"foo_ext\" extendee: \".Foo\" number:5 " + "name: 'bar.proto' " + "dependency: 'foo.proto' " + "message_type { name:'Bar' } " + "extension { name:'foo_ext' extendee: '.Foo' number:5 " " label:LABEL_OPTIONAL type:TYPE_INT32 } "); + // Baz has an undeclared dependency on Foo. + AddToDatabase(&database_, + "name: 'baz.proto' " + "message_type { " + " name:'Baz' " + " field { name:'foo' number:1 label:LABEL_OPTIONAL type_name:'Foo' } " + "}"); } // We can't inject a file containing errors into a DescriptorPool, so we @@ -3829,6 +5010,33 @@ TEST_F(DatabaseBackedPoolTest, ErrorWithErrorCollector) { error_collector.text_); } +TEST_F(DatabaseBackedPoolTest, UndeclaredDependencyOnUnbuiltType) { + // Check that we find and report undeclared dependencies on types that exist + // in the descriptor database but that have not not been built yet. + MockErrorCollector error_collector; + DescriptorPool pool(&database_, &error_collector); + EXPECT_TRUE(pool.FindMessageTypeByName("Baz") == NULL); + EXPECT_EQ( + "baz.proto: Baz.foo: TYPE: \"Foo\" seems to be defined in \"foo.proto\", " + "which is not imported by \"baz.proto\". To use it here, please add " + "the necessary import.\n", + error_collector.text_); +} + +TEST_F(DatabaseBackedPoolTest, RollbackAfterError) { + // Make sure that all traces of bad types are removed from the pool. This used + // to be b/4529436, due to the fact that a symbol resolution failure could + // potentially cause another file to be recursively built, which would trigger + // a checkpoint _past_ possibly invalid symbols. + // Baz is defined in the database, but the file is invalid because it is + // missing a necessary import. + DescriptorPool pool(&database_); + EXPECT_TRUE(pool.FindMessageTypeByName("Baz") == NULL); + // Make sure that searching again for the file or the type fails. + EXPECT_TRUE(pool.FindFileByName("baz.proto") == NULL); + EXPECT_TRUE(pool.FindMessageTypeByName("Baz") == NULL); +} + TEST_F(DatabaseBackedPoolTest, UnittestProto) { // Try to load all of unittest.proto from a DescriptorDatabase. This should // thoroughly test all paths through DescriptorBuilder to insure that there @@ -3851,6 +5059,10 @@ TEST_F(DatabaseBackedPoolTest, UnittestProto) { EXPECT_EQ(original_file_proto.DebugString(), file_from_database_proto.DebugString()); + + // Also verify that CopyTo() did not omit any information. + EXPECT_EQ(original_file->DebugString(), + file_from_database->DebugString()); } TEST_F(DatabaseBackedPoolTest, DoesntRetryDbUnnecessarily) { @@ -3885,6 +5097,16 @@ TEST_F(DatabaseBackedPoolTest, DoesntRetryDbUnnecessarily) { EXPECT_TRUE(file->FindEnumValueByName("NO_SUCH_VALUE") == NULL); EXPECT_TRUE(file->FindServiceByName("NO_SUCH_VALUE") == NULL); EXPECT_TRUE(file->FindExtensionByName("no_such_extension") == NULL); + + EXPECT_TRUE(pool.FindFileContainingSymbol("Foo.no.such.field") == NULL); + EXPECT_TRUE(pool.FindFileContainingSymbol("Foo.no_such_field") == NULL); + EXPECT_TRUE(pool.FindMessageTypeByName("Foo.NoSuchMessageType") == NULL); + EXPECT_TRUE(pool.FindFieldByName("Foo.no_such_field") == NULL); + EXPECT_TRUE(pool.FindExtensionByName("Foo.no_such_extension") == NULL); + EXPECT_TRUE(pool.FindEnumTypeByName("Foo.NoSuchEnumType") == NULL); + EXPECT_TRUE(pool.FindEnumValueByName("Foo.NO_SUCH_VALUE") == NULL); + EXPECT_TRUE(pool.FindMethodByName("TestService.NoSuchMethod") == NULL); + EXPECT_EQ(0, call_counter.call_count_); } @@ -3909,15 +5131,93 @@ TEST_F(DatabaseBackedPoolTest, DoesntReloadFilesUncesessarily) { EXPECT_EQ("", error_collector.text_); } +// DescriptorDatabase that attempts to induce exponentially-bad performance +// in DescriptorPool. For every positive N, the database contains a file +// fileN.proto, which defines a message MessageN, which contains fields of +// type MessageK for all K in [0,N). Message0 is not defined anywhere +// (file0.proto exists, but is empty), so every other file and message type +// will fail to build. +// +// If the DescriptorPool is not careful to memoize errors, an attempt to +// build a descriptor for MessageN can require O(2^N) time. +class ExponentialErrorDatabase : public DescriptorDatabase { + public: + ExponentialErrorDatabase() {} + ~ExponentialErrorDatabase() {} + + // implements DescriptorDatabase --------------------------------- + bool FindFileByName(const string& filename, + FileDescriptorProto* output) { + int file_num = -1; + FullMatch(filename, "file", ".proto", &file_num); + if (file_num > -1) { + return PopulateFile(file_num, output); + } else { + return false; + } + } + bool FindFileContainingSymbol(const string& symbol_name, + FileDescriptorProto* output) { + int file_num = -1; + FullMatch(symbol_name, "Message", "", &file_num); + if (file_num > 0) { + return PopulateFile(file_num, output); + } else { + return false; + } + } + bool FindFileContainingExtension(const string& containing_type, + int field_number, + FileDescriptorProto* output) { + return false; + } + + private: + void FullMatch(const string& name, + const string& begin_with, + const string& end_with, + int* file_num) { + int begin_size = begin_with.size(); + int end_size = end_with.size(); + if (name.substr(0, begin_size) != begin_with || + name.substr(name.size()- end_size, end_size) != end_with) { + return; + } + safe_strto32(name.substr(begin_size, name.size() - end_size - begin_size), + file_num); + } + + bool PopulateFile(int file_num, FileDescriptorProto* output) { + using strings::Substitute; + GOOGLE_CHECK_GE(file_num, 0); + output->Clear(); + output->set_name(Substitute("file$0.proto", file_num)); + // file0.proto doesn't define Message0 + if (file_num > 0) { + DescriptorProto* message = output->add_message_type(); + message->set_name(Substitute("Message$0", file_num)); + for (int i = 0; i < file_num; ++i) { + output->add_dependency(Substitute("file$0.proto", i)); + FieldDescriptorProto* field = message->add_field(); + field->set_name(Substitute("field$0", i)); + field->set_number(i); + field->set_label(FieldDescriptorProto::LABEL_OPTIONAL); + field->set_type(FieldDescriptorProto::TYPE_MESSAGE); + field->set_type_name(Substitute("Message$0", i)); + } + } + return true; + } +}; + TEST_F(DatabaseBackedPoolTest, DoesntReloadKnownBadFiles) { - ErrorDescriptorDatabase error_database; - MockErrorCollector error_collector; - DescriptorPool pool(&error_database, &error_collector); + ExponentialErrorDatabase error_database; + DescriptorPool pool(&error_database); - EXPECT_TRUE(pool.FindFileByName("error.proto") == NULL); - error_collector.text_.clear(); - EXPECT_TRUE(pool.FindFileByName("error.proto") == NULL); - EXPECT_EQ("", error_collector.text_); + GOOGLE_LOG(INFO) << "A timeout in this test probably indicates a real bug."; + + EXPECT_TRUE(pool.FindFileByName("file40.proto") == NULL); + EXPECT_TRUE(pool.FindMessageTypeByName("Message40") == NULL); } TEST_F(DatabaseBackedPoolTest, DoesntFallbackOnWrongType) { @@ -3950,6 +5250,251 @@ TEST_F(DatabaseBackedPoolTest, DoesntFallbackOnWrongType) { // =================================================================== +class AbortingErrorCollector : public DescriptorPool::ErrorCollector { + public: + AbortingErrorCollector() {} + + virtual void AddError( + const string &filename, + const string &element_name, + const Message *message, + ErrorLocation location, + const string &error_message) { + GOOGLE_LOG(FATAL) << "AddError() called unexpectedly: " << filename << ": " + << error_message; + } + private: + GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(AbortingErrorCollector); +}; + +// A source tree containing only one file. +class SingletonSourceTree : public compiler::SourceTree { + public: + SingletonSourceTree(const string& filename, const string& contents) + : filename_(filename), contents_(contents) {} + + virtual io::ZeroCopyInputStream* Open(const string& filename) { + return filename == filename_ ? + new io::ArrayInputStream(contents_.data(), contents_.size()) : NULL; + } + + private: + const string filename_; + const string contents_; + + GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(SingletonSourceTree); +}; + +const char *const kSourceLocationTestInput = + "syntax = \"proto2\";\n" + "message A {\n" + " optional int32 a = 1;\n" + " message B {\n" + " required double b = 1;\n" + " }\n" + "}\n" + "enum Indecision {\n" + " YES = 1;\n" + " NO = 2;\n" + " MAYBE = 3;\n" + "}\n" + "service S {\n" + " rpc Method(A) returns (A.B);\n" + // Put an empty line here to make the source location range match. + "\n" + "}\n" + "message MessageWithExtensions {\n" + " extensions 1000 to max;\n" + "}\n" + "extend MessageWithExtensions {\n" + " optional int32 int32_extension = 1001;\n" + "}\n" + "message C {\n" + " extend MessageWithExtensions {\n" + " optional C message_extension = 1002;\n" + " }\n" + "}\n"; + +class SourceLocationTest : public testing::Test { + public: + SourceLocationTest() + : source_tree_("/test/test.proto", kSourceLocationTestInput), + db_(&source_tree_), + pool_(&db_, &collector_) {} + + static string PrintSourceLocation(const SourceLocation &loc) { + return strings::Substitute("$0:$1-$2:$3", + 1 + loc.start_line, + 1 + loc.start_column, + 1 + loc.end_line, + 1 + loc.end_column); + } + + private: + AbortingErrorCollector collector_; + SingletonSourceTree source_tree_; + compiler::SourceTreeDescriptorDatabase db_; + + protected: + DescriptorPool pool_; +}; + +// TODO(adonovan): implement support for option fields and for +// subparts of declarations. + +TEST_F(SourceLocationTest, GetSourceLocation) { + SourceLocation loc; + + const FileDescriptor *file_desc = + GOOGLE_CHECK_NOTNULL(pool_.FindFileByName("/test/test.proto")); + + const Descriptor *a_desc = file_desc->FindMessageTypeByName("A"); + EXPECT_TRUE(a_desc->GetSourceLocation(&loc)); + EXPECT_EQ("2:1-7:2", PrintSourceLocation(loc)); + + const Descriptor *a_b_desc = a_desc->FindNestedTypeByName("B"); + EXPECT_TRUE(a_b_desc->GetSourceLocation(&loc)); + EXPECT_EQ("4:3-6:4", PrintSourceLocation(loc)); + + const EnumDescriptor *e_desc = file_desc->FindEnumTypeByName("Indecision"); + EXPECT_TRUE(e_desc->GetSourceLocation(&loc)); + EXPECT_EQ("8:1-12:2", PrintSourceLocation(loc)); + + const EnumValueDescriptor *yes_desc = e_desc->FindValueByName("YES"); + EXPECT_TRUE(yes_desc->GetSourceLocation(&loc)); + EXPECT_EQ("9:3-9:13", PrintSourceLocation(loc)); + + const ServiceDescriptor *s_desc = file_desc->FindServiceByName("S"); + EXPECT_TRUE(s_desc->GetSourceLocation(&loc)); + EXPECT_EQ("13:1-16:2", PrintSourceLocation(loc)); + + const MethodDescriptor *m_desc = s_desc->FindMethodByName("Method"); + EXPECT_TRUE(m_desc->GetSourceLocation(&loc)); + EXPECT_EQ("14:3-14:31", PrintSourceLocation(loc)); + +} + +TEST_F(SourceLocationTest, ExtensionSourceLocation) { + SourceLocation loc; + + const FileDescriptor *file_desc = + GOOGLE_CHECK_NOTNULL(pool_.FindFileByName("/test/test.proto")); + + const FieldDescriptor *int32_extension_desc = + file_desc->FindExtensionByName("int32_extension"); + EXPECT_TRUE(int32_extension_desc->GetSourceLocation(&loc)); + EXPECT_EQ("21:3-21:41", PrintSourceLocation(loc)); + + const Descriptor *c_desc = file_desc->FindMessageTypeByName("C"); + EXPECT_TRUE(c_desc->GetSourceLocation(&loc)); + EXPECT_EQ("23:1-27:2", PrintSourceLocation(loc)); + + const FieldDescriptor *message_extension_desc = + c_desc->FindExtensionByName("message_extension"); + EXPECT_TRUE(message_extension_desc->GetSourceLocation(&loc)); + EXPECT_EQ("25:5-25:41", PrintSourceLocation(loc)); +} + +// Missing SourceCodeInfo doesn't cause crash: +TEST_F(SourceLocationTest, GetSourceLocation_MissingSourceCodeInfo) { + SourceLocation loc; + + const FileDescriptor *file_desc = + GOOGLE_CHECK_NOTNULL(pool_.FindFileByName("/test/test.proto")); + + FileDescriptorProto proto; + file_desc->CopyTo(&proto); // Note, this discards the SourceCodeInfo. + EXPECT_FALSE(proto.has_source_code_info()); + + DescriptorPool bad1_pool(&pool_); + const FileDescriptor* bad1_file_desc = + GOOGLE_CHECK_NOTNULL(bad1_pool.BuildFile(proto)); + const Descriptor *bad1_a_desc = bad1_file_desc->FindMessageTypeByName("A"); + EXPECT_FALSE(bad1_a_desc->GetSourceLocation(&loc)); +} + +// Corrupt SourceCodeInfo doesn't cause crash: +TEST_F(SourceLocationTest, GetSourceLocation_BogusSourceCodeInfo) { + SourceLocation loc; + + const FileDescriptor *file_desc = + GOOGLE_CHECK_NOTNULL(pool_.FindFileByName("/test/test.proto")); + + FileDescriptorProto proto; + file_desc->CopyTo(&proto); // Note, this discards the SourceCodeInfo. + EXPECT_FALSE(proto.has_source_code_info()); + SourceCodeInfo_Location *loc_msg = + proto.mutable_source_code_info()->add_location(); + loc_msg->add_path(1); + loc_msg->add_path(2); + loc_msg->add_path(3); + loc_msg->add_span(4); + loc_msg->add_span(5); + loc_msg->add_span(6); + + DescriptorPool bad2_pool(&pool_); + const FileDescriptor* bad2_file_desc = + GOOGLE_CHECK_NOTNULL(bad2_pool.BuildFile(proto)); + const Descriptor *bad2_a_desc = bad2_file_desc->FindMessageTypeByName("A"); + EXPECT_FALSE(bad2_a_desc->GetSourceLocation(&loc)); +} + +// =================================================================== + +const char* const kCopySourceCodeInfoToTestInput = + "syntax = \"proto2\";\n" + "message Foo {}\n"; + +// Required since source code information is not preserved by +// FileDescriptorTest. +class CopySourceCodeInfoToTest : public testing::Test { + public: + CopySourceCodeInfoToTest() + : source_tree_("/test/test.proto", kCopySourceCodeInfoToTestInput), + db_(&source_tree_), + pool_(&db_, &collector_) {} + + private: + AbortingErrorCollector collector_; + SingletonSourceTree source_tree_; + compiler::SourceTreeDescriptorDatabase db_; + + protected: + DescriptorPool pool_; +}; + +TEST_F(CopySourceCodeInfoToTest, CopyTo_DoesNotCopySourceCodeInfo) { + const FileDescriptor* file_desc = + GOOGLE_CHECK_NOTNULL(pool_.FindFileByName("/test/test.proto")); + FileDescriptorProto file_desc_proto; + ASSERT_FALSE(file_desc_proto.has_source_code_info()); + + file_desc->CopyTo(&file_desc_proto); + EXPECT_FALSE(file_desc_proto.has_source_code_info()); +} + +TEST_F(CopySourceCodeInfoToTest, CopySourceCodeInfoTo) { + const FileDescriptor* file_desc = + GOOGLE_CHECK_NOTNULL(pool_.FindFileByName("/test/test.proto")); + FileDescriptorProto file_desc_proto; + ASSERT_FALSE(file_desc_proto.has_source_code_info()); + + file_desc->CopySourceCodeInfoTo(&file_desc_proto); + const SourceCodeInfo& info = file_desc_proto.source_code_info(); + ASSERT_EQ(3, info.location_size()); + // Get the Foo message location + const SourceCodeInfo_Location& foo_location = info.location(1); + ASSERT_EQ(2, foo_location.path_size()); + EXPECT_EQ(FileDescriptorProto::kMessageTypeFieldNumber, foo_location.path(0)); + EXPECT_EQ(0, foo_location.path(1)); // Foo is the first message defined + ASSERT_EQ(3, foo_location.span_size()); // Foo spans one line + EXPECT_EQ(1, foo_location.span(0)); // Foo is declared on line 1 + EXPECT_EQ(0, foo_location.span(1)); // Foo starts at column 0 + EXPECT_EQ(14, foo_location.span(2)); // Foo ends on column 14 +} + +// =================================================================== + } // namespace descriptor_unittest } // namespace protobuf |