diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index 5d45741c6714..bb4951d0606e 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -571,6 +571,26 @@ void Parser::SkipRestOfBlock() { // =================================================================== +bool Parser::ValidateMessage(const DescriptorProto* proto) { + for (int i = 0; i < proto->options().uninterpreted_option_size(); i++) { + const UninterpretedOption& option = + proto->options().uninterpreted_option(i); + if (option.name_size() > 0 && !option.name(0).is_extension() && + option.name(0).name_part() == "map_entry") { + int line = -1, col = 0; // indicates line and column not known + if (source_location_table_ != nullptr) { + source_location_table_->Find( + &option, DescriptorPool::ErrorCollector::OPTION_NAME, &line, &col); + } + RecordError(line, col, + "map_entry should not be set explicitly. " + "Use map instead."); + return false; + } + } + return true; +} + bool Parser::ValidateEnum(const EnumDescriptorProto* proto) { bool has_allow_alias = false; bool allow_alias = false; @@ -661,9 +681,8 @@ bool Parser::Parse(io::Tokenizer* input, FileDescriptorProto* file) { root_location.RecordLegacyLocation(file, DescriptorPool::ErrorCollector::OTHER); - if (require_syntax_identifier_ || LookingAt("syntax") - || LookingAt("edition") - ) { + if (require_syntax_identifier_ || LookingAt("syntax") || + LookingAt("edition")) { if (!ParseSyntaxIdentifier(file, root_location)) { // Don't attempt to parse the file if we didn't recognize the syntax // identifier. @@ -867,6 +886,7 @@ bool IsMessageSetWireFormatMessage(const DescriptorProto& message) { for (int i = 0; i < options.uninterpreted_option_size(); ++i) { const UninterpretedOption& uninterpreted = options.uninterpreted_option(i); if (uninterpreted.name_size() == 1 && + !uninterpreted.name(0).is_extension() && uninterpreted.name(0).name_part() == "message_set_wire_format" && uninterpreted.identifier_value() == "true") { return true; @@ -931,6 +951,9 @@ bool Parser::ParseMessageBlock(DescriptorProto* message, if (message->reserved_range_size() > 0) { AdjustReservedRangesWithMaxEndNumber(message); } + + DO(ValidateMessage(message)); + return true; } diff --git a/src/google/protobuf/compiler/parser.h b/src/google/protobuf/compiler/parser.h index e68db1b9eabc..93367ab1f19a 100644 --- a/src/google/protobuf/compiler/parser.h +++ b/src/google/protobuf/compiler/parser.h @@ -534,6 +534,7 @@ class PROTOBUF_EXPORT Parser { return syntax_identifier_ == "proto3"; } + bool ValidateMessage(const DescriptorProto* proto); bool ValidateEnum(const EnumDescriptorProto* proto); // ================================================================= diff --git a/src/google/protobuf/compiler/parser_unittest.cc b/src/google/protobuf/compiler/parser_unittest.cc index 3c16aad4c8e7..c1bc49086961 100644 --- a/src/google/protobuf/compiler/parser_unittest.cc +++ b/src/google/protobuf/compiler/parser_unittest.cc @@ -171,6 +171,8 @@ class ParserTest : public testing::Test { // input. void ExpectHasEarlyExitErrors(const char* text, const char* expected_errors) { SetupParser(text); + SourceLocationTable source_locations; + parser_->RecordSourceLocationsTo(&source_locations); FileDescriptorProto file; EXPECT_FALSE(parser_->Parse(input_.get(), &file)); EXPECT_EQ(expected_errors, error_collector_.text_); @@ -1746,12 +1748,12 @@ TEST_F(ParseErrorTest, EnumReservedMissingQuotes) { TEST_F(ParseErrorTest, EnumReservedInvalidIdentifier) { ExpectHasWarnings( - R"pb( + R"( enum TestEnum { FOO = 1; reserved "foo bar"; } - )pb", + )", "3:17: Reserved name \"foo bar\" is not a valid identifier.\n"); } @@ -1784,11 +1786,11 @@ TEST_F(ParseErrorTest, ReservedMissingQuotes) { TEST_F(ParseErrorTest, ReservedInvalidIdentifier) { ExpectHasWarnings( - R"pb( + R"( message Foo { reserved "foo bar"; } - )pb", + )", "2:17: Reserved name \"foo bar\" is not a valid identifier.\n"); } @@ -2235,7 +2237,7 @@ TEST_F(ParserValidationErrorTest, EnumValueAliasError) { } TEST_F(ParserValidationErrorTest, ExplicitlyMapEntryError) { - ExpectHasValidationErrors( + ExpectHasErrors( "message Foo {\n" " message ValueEntry {\n" " option map_entry = true;\n" @@ -2243,9 +2245,8 @@ TEST_F(ParserValidationErrorTest, ExplicitlyMapEntryError) { " optional int32 value = 2;\n" " extensions 99 to 999;\n" " }\n" - " repeated ValueEntry value = 1;\n" "}", - "7:11: map_entry should not be set explicitly. Use " + "2:11: map_entry should not be set explicitly. Use " "map instead.\n"); } @@ -2950,7 +2951,7 @@ class SourceInfoTest : public ParserTest { } } - return false; + return false; } private: @@ -4143,7 +4144,6 @@ TEST_F(ParseEditionsTest, FeaturesWithoutEditions) { } - } // anonymous namespace } // namespace compiler