diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index f220fad8be..2ab823981e 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -408,6 +408,11 @@ Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent) { Init(parent, parent.source_code_info_); } +Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent, + SourceCodeInfo* source_code_info) { + Init(parent, source_code_info); +} + Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent, int path1, SourceCodeInfo* source_code_info) { @@ -1237,13 +1242,22 @@ bool Parser::ParseDefaultAssignment( field->clear_default_value(); } + LocationRecorder location(field_location, + FieldDescriptorProto::kDefaultValueFieldNumber); + DO(Consume("default")); DO(Consume("=")); - LocationRecorder location(field_location, - FieldDescriptorProto::kDefaultValueFieldNumber); - location.RecordLegacyLocation(field, - DescriptorPool::ErrorCollector::DEFAULT_VALUE); + // We don't need to create separate spans in source code info for name and value, + // since there's no way to represent them distinctly in a location path. But we will + // want a separate recorder for the value, just to have more precise location info + // in error messages. So we let it create a location in no_op, so it doesn't add a + // span to the file descriptor. + SourceCodeInfo no_op; + LocationRecorder value_location(location, &no_op); + value_location.RecordLegacyLocation( + field, DescriptorPool::ErrorCollector::DEFAULT_VALUE); + std::string* default_value = field->mutable_default_value(); if (!field->has_type()) { @@ -1377,13 +1391,23 @@ bool Parser::ParseJsonName(FieldDescriptorProto* field, LocationRecorder location(field_location, FieldDescriptorProto::kJsonNameFieldNumber); - location.RecordLegacyLocation(field, - DescriptorPool::ErrorCollector::OPTION_NAME); - DO(Consume("json_name")); + // We don't need to create separate spans in source code info for name and value, + // since there's no way to represent them distinctly in a location path. But we will + // want a separate recorder for them, just to have more precise location info + // in error messages. So we let them create a location in no_op, so they don't + // add a span to the file descriptor. + SourceCodeInfo no_op; + { + LocationRecorder name_location(location, &no_op); + name_location.RecordLegacyLocation( + field, DescriptorPool::ErrorCollector::OPTION_NAME); + + DO(Consume("json_name")); + } DO(Consume("=")); - LocationRecorder value_location(location); + LocationRecorder value_location(location, &no_op); value_location.RecordLegacyLocation( field, DescriptorPool::ErrorCollector::OPTION_VALUE); diff --git a/src/google/protobuf/compiler/parser.h b/src/google/protobuf/compiler/parser.h index d4eb76302c..8b05d0e4ad 100644 --- a/src/google/protobuf/compiler/parser.h +++ b/src/google/protobuf/compiler/parser.h @@ -237,6 +237,10 @@ class PROTOBUF_EXPORT Parser { LocationRecorder(const LocationRecorder& parent, int path1, int path2); // Creates a recorder that generates locations into given source code info. + LocationRecorder(const LocationRecorder& parent, + SourceCodeInfo* source_code_info); + // Creates a recorder that generates locations into given source code info + // and calls AddPath() one time. LocationRecorder(const LocationRecorder& parent, int path1, SourceCodeInfo* source_code_info); diff --git a/src/google/protobuf/compiler/parser_unittest.cc b/src/google/protobuf/compiler/parser_unittest.cc index 55ed7acb6f..a164d7d54a 100644 --- a/src/google/protobuf/compiler/parser_unittest.cc +++ b/src/google/protobuf/compiler/parser_unittest.cc @@ -3415,18 +3415,19 @@ TEST_F(SourceInfoTest, FieldOptions) { EXPECT_TRUE( Parse("message Foo {" " optional int32 bar = 1 " - "$a$[default=$b$123$c$,$d$opt1=123$e$," - "$f$opt2='hi'$g$]$h$;" + "$a$[$b$default=123$c$, $d$opt1=123$e$, " + "$f$opt2='hi'$g$, $h$json_name='barBar'$i$]$j$;" "}\n")); const FieldDescriptorProto& field = file_.message_type(0).field(0); const UninterpretedOption& option1 = field.options().uninterpreted_option(0); const UninterpretedOption& option2 = field.options().uninterpreted_option(1); - EXPECT_TRUE(HasSpan('a', 'h', field.options())); + EXPECT_TRUE(HasSpan('a', 'j', field.options())); EXPECT_TRUE(HasSpan('b', 'c', field, "default_value")); EXPECT_TRUE(HasSpan('d', 'e', option1)); EXPECT_TRUE(HasSpan('f', 'g', option2)); + EXPECT_TRUE(HasSpan('h', 'i', field, "json_name")); // Ignore these. EXPECT_TRUE(HasSpan(file_));