[clangd] Added highlighting for members and methods

[jvikstrom]

Summary

Added highlighting for members and also changed highlighting for methods into their own HighlightingKind.

[sammccall]

LG from my side

clang-tools-extra/clangd/SemanticHighlighting.h

28 ↗

(On Diff #209425)

nit: clang calls these Field which is terser and pretty understandable, I think.

(Formally, I think these are data members and "methods" are member functions, but I like the shorter names better)

[hokein]

clang-tools-extra/clangd/SemanticHighlighting.cpp

44 ↗

(On Diff #209425)

nit: this can be simplified like

if (const auto* D = dyn_cast<CXXDestructorDecl>(ME->getMemberDecl())) {
  // When calling the destructor manually like: AAA::~A(); The ~ is a
  // MemberExpr.  
 return true;
}

53 ↗

(On Diff #209425)

It took me a while to understand how the comment associate with the code here, maybe add and we use the MemberExpr in the comment?

142 ↗

(On Diff #209425)

nit: clang-format.

As it is class-related, could you move it to Line 133 (immediately after the Class case), the same to the FieldDecl.

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

140 ↗

(On Diff #209425)

could you add a test case for static member?

[jvikstrom]

Addressed comments.

[jvikstrom]

Removed addToken for Exprs.

[hokein]

clang-tools-extra/clangd/SemanticHighlighting.h

28 ↗

(On Diff #209510)

nit: put it Field around Class.

clang-tools-extra/trunk/clangd/SemanticHighlighting.h

 #include "Protocol.h"
 
 namespace clang {
 namespace clangd {
 
 enum class HighlightingKind {
   Variable = 0,
   Function,
+  Method,
+  Field,
   Class,
   Enum,
   EnumConstant,
   Namespace,
 
   NumKinds,
 };
 

clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp

public:
   }
 
   bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) {
     // The target namespace of an alias can not be found in any other way.
     addToken(NAD->getTargetNameLoc(), HighlightingKind::Namespace);
     return true;
   }
 
+  bool VisitMemberExpr(MemberExpr *ME) {
+    const auto *MD = ME->getMemberDecl();
+    if (isa<CXXDestructorDecl>(MD))
+      // When calling the destructor manually like: AAA::~A(); The ~ is a
+      // MemberExpr. Other methods should still be highlighted though.
+      return true;
+    addToken(ME->getMemberLoc(), MD);
+    return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *ND) {
     // UsingDirectiveDecl's namespaces do not show up anywhere else in the
     // Visit/Traverse mehods. But they should also be highlighted as a
     // namespace.
     if (const auto *UD = dyn_cast<UsingDirectiveDecl>(ND)) {
       addToken(UD->getIdentLocation(), HighlightingKind::Namespace);
       return true;
     }
void addToken(SourceLocation Loc, const NamedDecl *D) {
     if (isa<RecordDecl>(D)) {
       addToken(Loc, HighlightingKind::Class);
       return;
     }
     if (isa<CXXConstructorDecl>(D)) {
       addToken(Loc, HighlightingKind::Class);
       return;
     }
+    if (isa<CXXMethodDecl>(D)) {
+      addToken(Loc, HighlightingKind::Method);
+      return;
+    }
+    if (isa<FieldDecl>(D)) {
+      addToken(Loc, HighlightingKind::Field);
+      return;
+    }
     if (isa<EnumDecl>(D)) {
       addToken(Loc, HighlightingKind::Enum);
       return;
     }
     if (isa<EnumConstantDecl>(D)) {
       addToken(Loc, HighlightingKind::EnumConstant);
       return;
     }
toSemanticHighlightingInformation(llvm::ArrayRef<HighlightingToken> Tokens) {
   return Lines;
 }
 
 llvm::StringRef toTextMateScope(HighlightingKind Kind) {
   // FIXME: Add scopes for C and Objective C.
   switch (Kind) {
   case HighlightingKind::Function:
     return "entity.name.function.cpp";
+  case HighlightingKind::Method:
+    return "entity.name.function.method.cpp";
   case HighlightingKind::Variable:
-    return "variable.cpp";
+    return "variable.other.cpp";
+  case HighlightingKind::Field:
+    return "variable.other.field.cpp";
   case HighlightingKind::Class:
     return "entity.name.type.class.cpp";
   case HighlightingKind::Enum:
     return "entity.name.type.enum.cpp";
   case HighlightingKind::EnumConstant:
     return "variable.other.enummember.cpp";
   case HighlightingKind::Namespace:
     return "entity.name.namespace.cpp";
   case HighlightingKind::NumKinds:
     llvm_unreachable("must not pass NumKinds to the function");
   }
   llvm_unreachable("unhandled HighlightingKind");
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/test/semantic-highlighting.test

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"semanticHighlightingCapabilities":{"semanticHighlighting":true}}},"trace":"off"}}
 ---
 #      CHECK:  "id": 0,
 # CHECK:      "semanticHighlighting": {
 # CHECK-NEXT:        "scopes": [
 # CHECK-NEXT:          [
-# CHECK-NEXT:            "variable.cpp"
+# CHECK-NEXT:            "variable.other.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.function.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
+# CHECK-NEXT:            "entity.name.function.method.cpp"
+# CHECK-NEXT:          ],
+# CHECK-NEXT:          [
+# CHECK-NEXT:            "variable.other.field.cpp"
+# CHECK-NEXT:          ],
+# CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.type.class.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.type.enum.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "variable.other.enummember.cpp"
 # CHECK-NEXT:          ],

clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

void checkHighlightings(llvm::StringRef Code) {
   Annotations Test(Code);
   auto AST = TestTU::withCode(Test.code()).build();
   static const std::map<HighlightingKind, std::string> KindToString{
       {HighlightingKind::Variable, "Variable"},
       {HighlightingKind::Function, "Function"},
       {HighlightingKind::Class, "Class"},
       {HighlightingKind::Enum, "Enum"},
       {HighlightingKind::Namespace, "Namespace"},
-      {HighlightingKind::EnumConstant, "EnumConstant"}};
+      {HighlightingKind::EnumConstant, "EnumConstant"},
+      {HighlightingKind::Field, "Field"},
+      {HighlightingKind::Method, "Method"}};
   std::vector<HighlightingToken> ExpectedTokens;
   for (const auto &KindString : KindToString) {
     std::vector<HighlightingToken> Toks = makeHighlightingTokens(
         Test.ranges(KindString.second), KindString.first);
     ExpectedTokens.insert(ExpectedTokens.end(), Toks.begin(), Toks.end());
   }
 
   auto ActualTokens = getSemanticHighlightings(AST);
   EXPECT_THAT(ActualTokens, testing::UnorderedElementsAreArray(ExpectedTokens));
 }
 
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
     R"cpp(
       struct $Class[[AS]] {
-        double SomeMember;
+        double $Field[[SomeMember]];
       };
       struct {
       } $Variable[[S]];
       void $Function[[foo]](int $Variable[[A]], $Class[[AS]] $Variable[[As]]) {
         auto $Variable[[VeryLongVariableName]] = 12312;
         $Class[[AS]]     $Variable[[AA]];
-        auto $Variable[[L]] = $Variable[[AA]].SomeMember + $Variable[[A]];
+        auto $Variable[[L]] = $Variable[[AA]].$Field[[SomeMember]] + $Variable[[A]];
         auto $Variable[[FN]] = [ $Variable[[AA]]](int $Variable[[A]]) -> void {};
         $Variable[[FN]](12312);
       }
     )cpp",
     R"cpp(
       void $Function[[foo]](int);
       void $Function[[Gah]]();
       void $Function[[foo]]() {
         auto $Variable[[Bou]] = $Function[[Gah]];
       }
       struct $Class[[A]] {
-        void $Function[[abc]]();
+        void $Method[[abc]]();
       };
     )cpp",
     R"cpp(
       namespace $Namespace[[abc]] {
         template<typename T>
         struct $Class[[A]] {
-          T t;
+          T $Field[[t]];
         };
       }
       template<typename T>
       struct $Class[[C]] : $Namespace[[abc]]::A<T> {
-        typename T::A* D;
+        typename T::A* $Field[[D]];
       };
       $Namespace[[abc]]::$Class[[A]]<int> $Variable[[AA]];
       typedef $Namespace[[abc]]::$Class[[A]]<int> AAA;
       struct $Class[[B]] {
         $Class[[B]]();
         ~$Class[[B]]();
         void operator<<($Class[[B]]);
-        $Class[[AAA]] AA;
+        $Class[[AAA]] $Field[[AA]];
       };
       $Class[[B]]::$Class[[B]]() {}
       $Class[[B]]::~$Class[[B]]() {}
       void $Function[[f]] () {
         $Class[[B]] $Variable[[BB]] = $Class[[B]]();
         $Variable[[BB]].~$Class[[B]]();
         $Class[[B]]();
       }
     )cpp",
     R"cpp(
       enum class $Enum[[E]] {
         $EnumConstant[[A]],
         $EnumConstant[[B]],
       };
       enum $Enum[[EE]] {
         $EnumConstant[[Hi]],
       };
       struct $Class[[A]] {
-        $Enum[[E]] EEE;
-        $Enum[[EE]] EEEE;
+        $Enum[[E]] $Field[[EEE]];
+        $Enum[[EE]] $Field[[EEEE]];
       };
       int $Variable[[I]] = $EnumConstant[[Hi]];
       $Enum[[E]] $Variable[[L]] = $Enum[[E]]::$EnumConstant[[B]];
     )cpp",
     R"cpp(
       namespace $Namespace[[abc]] {
         namespace {}
         namespace $Namespace[[bcd]] {
R"cpp(
       using namespace $Namespace[[abc]]::$Namespace[[bcd]];
       namespace $Namespace[[vwz]] =
             $Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]];
       $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]];
       $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] =
             $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::$EnumConstant[[Hi]];
       ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]];
       ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]];
+    )cpp",
+    R"cpp(
+      struct $Class[[D]] {
+        double $Field[[C]];
+      };
+      struct $Class[[A]] {
+        double $Field[[B]];
+        $Class[[D]] $Field[[E]];
+        static double $Variable[[S]];
+        void $Method[[foo]]() {
+          $Field[[B]] = 123;
+          this->$Field[[B]] = 156;
+          this->$Method[[foo]]();
+          $Method[[foo]]();
+          $Variable[[S]] = 90.1;
+        }
+      };
+      void $Function[[foo]]() {
+        $Class[[A]] $Variable[[AA]];
+        $Variable[[AA]].$Field[[B]] += 2;
+        $Variable[[AA]].$Method[[foo]]();
+        $Variable[[AA]].$Field[[E]].$Field[[C]];
+        $Class[[A]]::$Variable[[S]] = 90;
+      }
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);
   }
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
   class HighlightingsCounterDiagConsumer : public DiagnosticsConsumer {