Repository: kythe/kythe
Pull request #4872 (2021-03-30 21:25:50)

fix(proto): only decorate the relative type name in fields
Some UIs have problems decorating the multi-line spans which can result from taking the type locations directly from the proto compiler.  Split qualified names and only decorate the final unscoped type name.  This also matches the behavior from most other languages.


Patch:
diff --git a/kythe/cxx/indexer/proto/file_descriptor_walker.cc b/kythe/cxx/indexer/proto/file_descriptor_walker.cc
index 5b8908264..b3be59d94 100644
--- a/kythe/cxx/indexer/proto/file_descriptor_walker.cc
+++ b/kythe/cxx/indexer/proto/file_descriptor_walker.cc
@@ -82,6 +82,19 @@ class ScopedLookup {
   const int component_;
 };
 
+absl::optional<absl::string_view> TypeName(const FieldDescriptor& field) {
+  if (field.is_map()) {
+    return absl::nullopt;
+  }
+  if (const EnumDescriptor* desc = field.enum_type()) {
+    return desc->name();
+  }
+  if (const Descriptor* desc = field.message_type()) {
+    return desc->name();
+  }
+  return absl::nullopt;
+}
+
 }  // namespace
 
 int FileDescriptorWalker::ComputeByteOffset(int line_number,
@@ -358,6 +371,17 @@ void FileDescriptorWalker::VisitField(const std::string* parent_name,
     }
     const std::vector<int>& type_span = location_map_[lookup_path];
     InitializeLocation(type_span, &type_location);
+
+    // If we're in a message or enum type, decorate only the span
+    // covering the type name itself, not the full package name.
+    if (absl::optional<absl::string_view> type_name = TypeName(*field)) {
+      absl::string_view full_type = absl::string_view(content_).substr(
+          type_location.begin, type_location.end - type_location.begin);
+      if (auto pos = full_type.rfind(*type_name); pos != full_type.npos) {
+        type_location.begin += pos;
+        type_location.end = type_location.begin + type_name->size();
+      }
+    }
   }
   VName type = VNameForFieldType(field);
 


Test patch:
diff --git a/kythe/cxx/indexer/proto/testdata/basic/extend.proto b/kythe/cxx/indexer/proto/testdata/basic/extend.proto
index 201e5700f..70b38c498 100644
--- a/kythe/cxx/indexer/proto/testdata/basic/extend.proto
+++ b/kythe/cxx/indexer/proto/testdata/basic/extend.proto
@@ -23,7 +23,7 @@ extend Extendee {
 
 //- @"itsapackage.Foo" ref FooMessage
 extend itsapackage.Foo {
-  //- @"itsapackage.Bar" ref BarMessage
+  //- @Bar ref BarMessage
   //- @other_package_ext defines/binding OtherExtField
   //- OtherExtField childof MainPackage
   //- OtherExtField completes FooMessage
@@ -43,7 +43,7 @@ message Nested {
 
   //- @"itsapackage.Foo" ref FooMessage
   extend itsapackage.Foo {
-    //- @"itsapackage.Bar" ref BarMessage
+    //- @Bar ref BarMessage
     //- @nested_other_package_ext defines/binding NestedOtherField
     //- NestedOtherField childof NestedMessage
     //- NestedOtherField completes FooMessage
diff --git a/kythe/cxx/indexer/proto/testdata/basic/nested-message-field.proto b/kythe/cxx/indexer/proto/testdata/basic/nested-message-field.proto
index 397a488f8..922feb7b3 100644
--- a/kythe/cxx/indexer/proto/testdata/basic/nested-message-field.proto
+++ b/kythe/cxx/indexer/proto/testdata/basic/nested-message-field.proto
@@ -9,7 +9,7 @@ import "kythe/cxx/indexer/proto/testdata/basic/nested-message.proto";
 //- @TestMessage defines/binding TestMessageNode
 //- TestMessageNode childof ThePackage
 message TestMessage {
-  //- @"ParentMessage.NestedMessage" ref NestedMessageNode
+  //- @NestedMessage ref NestedMessageNode
   //- @test_nested_import defines/binding ImportFieldNode
   //- ImportFieldNode childof TestMessageNode
   optional ParentMessage.NestedMessage test_nested_import = 1;
diff --git a/kythe/cxx/indexer/proto/testdata/corner_cases/import_syntax.proto b/kythe/cxx/indexer/proto/testdata/corner_cases/import_syntax.proto
index 7167669b0..c72ab88ac 100644
--- a/kythe/cxx/indexer/proto/testdata/corner_cases/import_syntax.proto
+++ b/kythe/cxx/indexer/proto/testdata/corner_cases/import_syntax.proto
@@ -16,34 +16,22 @@ import public "kythe/cxx/indexer/proto/testdata/basic/oneof.proto";
 // Message to test for various syntax expressions of referring to a message
 // defined in a namespace.
 message TestMessage {
-  //- @"itsapackage.Foo" ref FooMessage
+  //- @Foo ref FooMessage
   optional itsapackage.Foo test_foo = 1;
-  //- DotLineOne.node/kind anchor
-  //- DotLineOne.loc/start @^"itsapackage."
-  //- DotLineOne.loc/end @$+3"Bar"
-  //- DotLineOne ref BarMessage
+  //- @+2"Bar" ref BarMessage
   optional itsapackage.
   Bar test_bar = 2;
 
-  //- DotLineTwo.node/kind anchor
-  //- DotLineTwo.loc/start @^"itsapackage"
-  //- DotLineTwo.loc/end @$+3".Baz"
-  //- DotLineTwo ref BazMessage
+  //- @+2"Baz" ref BazMessage
   optional itsapackage
   .Baz test_baz = 3;
 
   // Here the base case is already covered in nested-message-field.proto
-  //- NestedDotLineOne.node/kind anchor
-  //- NestedDotLineOne.loc/start @^"ParentMessage."
-  //- NestedDotLineOne.loc/end @$+3"NestedMessage"
-  //- NestedDotLineOne ref NestedMessage
+  //- @+2"NestedMessage" ref NestedMessage
   optional ParentMessage.
   NestedMessage test_nested_import = 4;
 
-  //- NestedDotLineTwo.node/kind anchor
-  //- NestedDotLineTwo.loc/start @^"ParentMessage"
-  //- NestedDotLineTwo.loc/end @$+3".NestedMessage"
-  //- NestedDotLineTwo ref NestedMessage
+  //- @+2"NestedMessage" ref NestedMessage
   optional ParentMessage
   .NestedMessage test_nested_import_again = 5;
 }
diff --git a/kythe/cxx/indexer/proto/testdata/corner_cases/tabs.proto b/kythe/cxx/indexer/proto/testdata/corner_cases/tabs.proto
index 3b33f9b43..837ebfac1 100644
--- a/kythe/cxx/indexer/proto/testdata/corner_cases/tabs.proto
+++ b/kythe/cxx/indexer/proto/testdata/corner_cases/tabs.proto
@@ -22,12 +22,12 @@ message MessageWithTabsInside {
 	//- BarField childof MessageNode
 	optional MessageWithTabsInside bar = 2;
 
-	//- @"itsapackage.Foo" ref FooMessage
+	//- @Foo ref FooMessage
 	//- @foo_data defines/binding AnotherFooField
 	//- AnotherFooField childof MessageNode
 	optional itsapackage.Foo foo_data = 3;
 
-	//- @"itsapackage.Bar" ref BarMessage
+	//- @Bar ref BarMessage
 	//- @bar_data defines/binding AnotherBarField
 	//- AnotherBarField childof MessageNode
 	optional itsapackage.Bar bar_data = 4;