source code info for interpreted options; fix source code info for extension range options (#4342)

* when interpreting options, rewrite file descriptor's source code info
- so that interpreted option paths have correct location information
- so that corresponding uninterpreted option paths are removed
also includes a fix to source code locations for extension range options

2 files changed, 60 insertions, 15 deletions

diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc
index df81e55f..5c7047a6 100644
--- a/src/google/protobuf/compiler/parser.cc
+++ b/src/google/protobuf/compiler/parser.cc
@@ -337,31 +337,42 @@ void Parser::AddError(const string& error) {
 
 Parser::LocationRecorder::LocationRecorder(Parser* parser)
   : parser_(parser),
+    source_code_info_(parser->source_code_info_),
     location_(parser_->source_code_info_->add_location()) {
   location_->add_span(parser_->input_->current().line);
   location_->add_span(parser_->input_->current().column);
 }
 
 Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent) {
-  Init(parent);
+  Init(parent, parent.source_code_info_);
+}
+
+Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent,
+                                           int path1,
+                                           SourceCodeInfo* source_code_info) {
+  Init(parent, source_code_info);
+  AddPath(path1);
 }
 
 Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent,
                                            int path1) {
-  Init(parent);
+  Init(parent, parent.source_code_info_);
   AddPath(path1);
 }
 
 Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent,
                                            int path1, int path2) {
-  Init(parent);
+  Init(parent, parent.source_code_info_);
   AddPath(path1);
   AddPath(path2);
 }
 
-void Parser::LocationRecorder::Init(const LocationRecorder& parent) {
+void Parser::LocationRecorder::Init(const LocationRecorder& parent,
+                                    SourceCodeInfo* source_code_info) {
   parser_ = parent.parser_;
-  location_ = parser_->source_code_info_->add_location();
+  source_code_info_ = source_code_info;
+
+  location_ = source_code_info_->add_location();
   location_->mutable_path()->CopyFrom(parent.location_->path());
 
   location_->add_span(parser_->input_->current().line);
@@ -403,6 +414,10 @@ void Parser::LocationRecorder::RecordLegacyLocation(const Message* descriptor,
   }
 }
 
+int Parser::LocationRecorder::CurrentPathSize() const {
+  return location_->path_size();
+}
+
 void Parser::LocationRecorder::AttachComments(
     string* leading, string* trailing,
     std::vector<string>* detached_comments) const {
@@ -1496,21 +1511,30 @@ bool Parser::ParseExtensions(DescriptorProto* message,
     range->set_end(end);
   } while (TryConsume(","));
 
-  if (LookingAt("[")) {
-    LocationRecorder location(
-        extensions_location,
-        DescriptorProto::ExtensionRange::kOptionsFieldNumber);
 
-    DO(Consume("["));
+  if (LookingAt("[")) {
+    int range_number_index = extensions_location.CurrentPathSize();
+    SourceCodeInfo info;
 
     // Parse extension range options in the first range.
     ExtensionRangeOptions* options =
         message->mutable_extension_range(old_range_size)->mutable_options();
-    do {
-      DO(ParseOption(options, location, containing_file, OPTION_ASSIGNMENT));
-    } while (TryConsume(","));
 
-    DO(Consume("]"));
+    {
+      LocationRecorder index_location(
+          extensions_location, 0 /* we fill this in w/ actual index below */,
+          &info);
+      LocationRecorder location(
+          index_location,
+          DescriptorProto::ExtensionRange::kOptionsFieldNumber);
+      DO(Consume("["));
+
+      do {
+        DO(ParseOption(options, location, containing_file, OPTION_ASSIGNMENT));
+      } while (TryConsume(","));
+
+      DO(Consume("]"));
+    }
 
     // Then copy the extension range options to all of the other ranges we've
     // parsed.
@@ -1518,6 +1542,19 @@ bool Parser::ParseExtensions(DescriptorProto* message,
       message->mutable_extension_range(i)->mutable_options()
           ->CopyFrom(*options);
     }
+    // and copy source locations to the other ranges, too
+    for (int i = old_range_size; i < message->extension_range_size(); i++) {
+      for (int j = 0; j < info.location_size(); j++) {
+        if (info.location(j).path_size() == range_number_index + 1) {
+          // this location's path is up to the extension range index, but doesn't
+          // include options; so it's redundant with location above
+          continue;
+        }
+        SourceCodeInfo_Location* dest = source_code_info_->add_location();
+        dest->CopyFrom(info.location(j));
+        dest->set_path(range_number_index, i);
+      }
+    }
   }
 
   DO(ConsumeEndOfDeclaration(";", &extensions_location));
diff --git a/src/google/protobuf/compiler/parser.h b/src/google/protobuf/compiler/parser.h
index 01d8a66b..5d98e5e1 100644
--- a/src/google/protobuf/compiler/parser.h
+++ b/src/google/protobuf/compiler/parser.h
@@ -224,6 +224,10 @@ class LIBPROTOBUF_EXPORT Parser {
     LocationRecorder(const LocationRecorder& parent, int path1);
     LocationRecorder(const LocationRecorder& parent, int path1, int path2);
 
+    // Creates a recorder that generates locations into given source code info.
+    LocationRecorder(const LocationRecorder& parent, int path1,
+                    SourceCodeInfo* source_code_info);
+
     ~LocationRecorder();
 
     // Add a path component.  See SourceCodeInfo.Location.path in
@@ -250,6 +254,9 @@ class LIBPROTOBUF_EXPORT Parser {
     void RecordLegacyLocation(const Message* descriptor,
         DescriptorPool::ErrorCollector::ErrorLocation location);
 
+    // Returns the number of path components in the recorder's current location.
+    int CurrentPathSize() const;
+
     // Attaches leading and trailing comments to the location.  The two strings
     // will be swapped into place, so after this is called *leading and
     // *trailing will be empty.
@@ -264,9 +271,10 @@ class LIBPROTOBUF_EXPORT Parser {
     // SourceCodeInfo.location repeated field. For top-level elements,
     // parent_index_ is -1.
     Parser* parser_;
+    SourceCodeInfo* source_code_info_;
     SourceCodeInfo::Location* location_;
 
-    void Init(const LocationRecorder& parent);
+    void Init(const LocationRecorder& parent, SourceCodeInfo* source_code_info);
   };
 
   // =================================================================