[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

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

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

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

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

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

nit: put it Field around Class.

clang-tools-extra/clangd/SemanticHighlighting.h

 #include "ClangdUnit.h"
 #include "Protocol.h"
 
 namespace clang {
 namespace clangd {
 
 enum class HighlightingKind {
   Variable = 0,
+  MemberVariable,
   Function,
+  Method,
   Class,
   Enum,
   Namespace,
 
   NumKinds,
 };
 
 // Contains all information needed for the highlighting a token.

clang-tools-extra/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) {
+    if (const CXXMethodDecl *MD =
+            dyn_cast<CXXMethodDecl>(ME->getMemberDecl())) {
+      // When calling the destructor manually like: AAA::~A(); The ~ is a
+      // MemberExpr.
+      if (!isa<CXXDestructorDecl>(MD))
+        addToken(ME->getMemberLoc(), MD);
+      return true;
+    }
+
+    // The MemberDecl is VarDecl for static members, therefore getMemberDecl
+    // does not work for all member variables.
+    addToken(ME->getMemberLoc(), ME);
+    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<EnumDecl>(D)) {
       addToken(Loc, HighlightingKind::Enum);
       return;
     }
     if (isa<VarDecl>(D)) {
       addToken(Loc, HighlightingKind::Variable);
       return;
     }
+    if(isa<CXXMethodDecl>(D)) {
+      addToken(Loc, HighlightingKind::Method);
+      return;
+    }
     if (isa<FunctionDecl>(D)) {
       addToken(Loc, HighlightingKind::Function);
       return;
     }
     if (isa<NamespaceDecl>(D)) {
       addToken(Loc, HighlightingKind::Namespace);
       return;
     }
     if (isa<NamespaceAliasDecl>(D)) {
       addToken(Loc, HighlightingKind::Namespace);
       return;
     }
+    if (isa<FieldDecl>(D)) {
+      addToken(Loc, HighlightingKind::MemberVariable);
+      return;
+    }
+  }
+
+  void addToken(SourceLocation Loc, const Expr *D) {
+    if (isa<MemberExpr>(D)) {
+      addToken(Loc, HighlightingKind::MemberVariable);
+      return;
+    }
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
     if (Loc.isMacroID())
       // FIXME: skip tokens inside macros for now.
       return;
 
     auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
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::MemberVariable:
+    return "variable.other.member.cpp";
   case HighlightingKind::Class:
     return "entity.name.type.class.cpp";
   case HighlightingKind::Enum:
     return "entity.name.type.enum.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/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:            "variable.other.member.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:            "entity.name.type.class.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.type.enum.cpp"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.namespace.cpp"
 # CHECK-NEXT:          ]

clang-tools-extra/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::Namespace, "Namespace"},
+      {HighlightingKind::MemberVariable, "MemberVariable"},
+      {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 $MemberVariable[[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]].$MemberVariable[[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 $MemberVariable[[t]];
         };
       }
       template<typename T>
       struct $Class[[C]] : $Namespace[[abc]]::A<T> {
-        typename T::A* D;
+        typename T::A* $MemberVariable[[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]] $MemberVariable[[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]] {};
       enum $Enum[[EE]] {};
       struct $Class[[A]] {
-        $Enum[[E]] EEE;
-        $Enum[[EE]] EEEE;
+        $Enum[[E]] $MemberVariable[[EEE]];
+        $Enum[[EE]] $MemberVariable[[EEEE]];
       };
     )cpp",
     R"cpp(
       namespace $Namespace[[abc]] {
         namespace {}
         namespace $Namespace[[bcd]] {
           struct $Class[[A]] {};
           namespace $Namespace[[cde]] {
             struct $Class[[A]] {
               enum class $Enum[[B]] {
                 Hi,
               };
             };
           }
         }
       }
       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]]::Hi;
       ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]];
       ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]];
+    )cpp",
+    R"cpp(
+      struct $Class[[D]] {
+        double $MemberVariable[[C]];
+      };
+      struct $Class[[A]] {
+        double $MemberVariable[[B]];
+        $Class[[D]] $MemberVariable[[E]];
+        void $Method[[foo]]() {
+          $MemberVariable[[B]] = 123;
+          this->$MemberVariable[[B]] = 156;
+          this->$Method[[foo]]();
+          $Method[[foo]]();
+        }
+      };
+      void $Function[[foo]]() {
+        $Class[[A]] $Variable[[AA]];
+        $Variable[[AA]].$MemberVariable[[B]] += 2;
+        $Variable[[AA]].$Method[[foo]]();
+        $Variable[[AA]].$MemberVariable[[E]].$MemberVariable[[C]];
+      }
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);
   }
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
   class HighlightingsCounterDiagConsumer : public DiagnosticsConsumer {
std::vector<HighlightingToken> Tokens{
       {HighlightingKind::Function,
                         Range{CreatePosition(3, 4), CreatePosition(3, 7)}},
       {HighlightingKind::Variable,
                         Range{CreatePosition(1, 1), CreatePosition(1, 5)}}};
   std::vector<SemanticHighlightingInformation> ActualResults =
       toSemanticHighlightingInformation(Tokens);
   std::vector<SemanticHighlightingInformation> ExpectedResults = {
       {1, "AAAAAQAEAAA="},
-      {3, "AAAACAAEAAAAAAAEAAMAAQ=="}};
+      {3, "AAAACAAEAAAAAAAEAAMAAg=="}};
   EXPECT_EQ(ActualResults, ExpectedResults);
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang