[clangd] Added highlightings for namespace specifiers.

[jvikstrom]

Summary

Added highlightings for namespace specifiers.

[hokein]

clang-tools-extra/clangd/SemanticHighlighting.cpp

47

nit: const auto *

48

nit: you miss a return

79–80

this comment is stale with this patch, now we are highlighting namespace, it seems more natural to do it here, we can get a NestedNameSpecifierLoc from an ElaboratedTypeLoc (so that we don't need the TraverseNestedNameSpecifierLoc).

253

I think here should be entity.name.namespace.cpp, vscode uses this TX scope for namespace.

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

114

could you add a testcase for anonymous namespace?

namespace {}

119

nit: the static is not needed for a type.

[jvikstrom]

Addressed comments.

[jvikstrom]

clang-tools-extra/clangd/SemanticHighlighting.cpp

79–80

Doing it in VisitTypeLoc means that we fail on this testcase:

namespace $Namespace[[aa]] {
  namespace $Namespace[[bb]] {
    struct $Class[[A]] {};
  }
}
$Namespace[[aa]]::$Namespace[[bb]]::$Class[[A]] $Variable[[a]];

It can't detect the bb namespace qualifier in aa::bb::A a;.

Maybe there is some way of solving this without TraverseNestedNameSpecifierLoc that I am not aware of?
Also don't know how I'd get a NestedNameSpecifierLoc from an ElaboratedTypeLoc.

253

Really? Because I see entity.name.type.namespace.cpp when I inspect the TM scopes for namespaces.

[sammccall]

clang-tools-extra/clangd/SemanticHighlighting.cpp

79–80

TraverseNestedNameSpecifierLoc is the right way to deal with namespaces-as-prefixes, I think it's best to follow that consistently.

(For namespaces not-as-prefixes, you've got namespacealiasdecl, namespacedecl, using-directives... I think that's everything)

93

if you're just doing something and then calling base, can you make this Visit instead of Traverse?

137

this doesn't fit with the contract of this addToken() function, which should only highlight the provided token.

Instead, can you call this from VisitNamespaceAliasDecl?

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

86

can you add a case where we spell with leading colons e.g. ::abc::A

[jvikstrom]

Moved alias target namespace add token to another function and added testcase for global namespace specifier.

[jvikstrom]

clang-tools-extra/clangd/SemanticHighlighting.cpp

93

NestedNameSpecifierLoc does not have a visit method for some reason. It only has a traverse method and this seems to be the way people get nested namespaces from what I can tell. (Maybe a Visit method is something that should be added to the RecursiveASTVisitor, but I wouldn't do that in this patch).

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

86

Added it in the case that focuses on namespaces further down.

[hokein]

looks good from my side, just a comment for the TM scope name.

clang-tools-extra/clangd/SemanticHighlighting.cpp

79–80

thanks for the explanation, fair enough.

253

that's weird, TM inspector told me it was entity.name.namespace.cpp, I can also find it in https://github.com/microsoft/vscode/blob/master/extensions/cpp/syntaxes/cpp.tmLanguage.json#L1709

clang-tools-extra/clangd/SemanticHighlighting.h

 namespace clang {
 namespace clangd {
 
 enum class HighlightingKind {
   Variable = 0,
   Function,
   Class,
   Enum,
+  Namespace,
 
   NumKinds,
 };
 
 // Contains all information needed for the highlighting a token.
 struct HighlightingToken {
   HighlightingKind Kind;
   Range R;

clang-tools-extra/clangd/SemanticHighlighting.cpp

 //===--- SemanticHighlighting.cpp - ------------------------- ---*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "SemanticHighlighting.h"
 #include "Logger.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/Decl.h"
-#include "clang/AST/DeclarationName.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 // Collects all semantic tokens in an ASTContext.
 class HighlightingTokenCollector
     : public RecursiveASTVisitor<HighlightingTokenCollector> {
   std::vector<HighlightingToken> Tokens;
   ASTContext &Ctx;
   const SourceManager &SM;
 
 public:
   HighlightingTokenCollector(ParsedAST &AST)
       : Ctx(AST.getASTContext()), SM(AST.getSourceManager()) {}
 
   std::vector<HighlightingToken> collectTokens() {
     Tokens.clear();
     TraverseAST(Ctx);
     return Tokens;
   }
 
+  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 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;
+    }
+
     // Constructors' TypeLoc has a TypePtr that is a FunctionProtoType. It has
     // no tag decl and therefore constructors must be gotten as NamedDecls
     // instead.
     if (ND->getDeclName().getNameKind() ==
         DeclarationName::CXXConstructorName) {
       addToken(ND->getLocation(), ND);
       return true;
     }
if (Ref->getNameInfo().getName().getNameKind() !=
       // Only want to highlight identifiers.
       return true;
 
     addToken(Ref->getLocation(), Ref->getDecl());
     return true;
   }
 
   bool VisitTypeLoc(TypeLoc &TL) {
     // This check is for not getting two entries when there are anonymous
-    // structs. It also makes us not highlight namespace qualifiers. For
-    // elaborated types the actual type is highlighted as an inner TypeLoc.
+    // structs. It also makes us not highlight certain namespace qualifiers
+    // twice. For elaborated types the actual type is highlighted as an inner
+    // TypeLoc.
     if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated)
       return true;
 
     if (const Type *TP = TL.getTypePtr())
       if (const TagDecl *TD = TP->getAsTagDecl())
-          addToken(TL.getBeginLoc(), TD);
     return true;
   }
 
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) {
+    if (NestedNameSpecifier *NNS = NNSLoc.getNestedNameSpecifier())
+      if (NNS->getKind() == NestedNameSpecifier::Namespace ||
+          NNS->getKind() == NestedNameSpecifier::NamespaceAlias)
+        addToken(NNSLoc.getLocalBeginLoc(), HighlightingKind::Namespace);
+
+    return RecursiveASTVisitor<
+        HighlightingTokenCollector>::TraverseNestedNameSpecifierLoc(NNSLoc);
+  }
+
 private:
   void addToken(SourceLocation Loc, const NamedDecl *D) {
     if (D->getDeclName().isIdentifier() && D->getName().empty())
       // Don't add symbols that don't have any length.
       return;
     // We highlight class decls, constructor decls and destructor decls as
     // `Class` type. The destructor decls are handled in `VisitTypeLoc` (we will
     // visit a TypeLoc where the underlying Type is a CXXRecordDecl).
void addToken(SourceLocation Loc, const NamedDecl *D) {
     if (isa<VarDecl>(D)) {
       addToken(Loc, HighlightingKind::Variable);
       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;
+    }
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
     if (Loc.isMacroID())
       // FIXME: skip tokens inside macros for now.
       return;
 
     auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
llvm::StringRef toTextMateScope(HighlightingKind Kind) {
   case HighlightingKind::Function:
     return "entity.name.function.cpp";
   case HighlightingKind::Variable:
     return "variable.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.type.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:          ],
 # CHECK-NEXT:          [
 # CHECK-NEXT:            "entity.name.function.cpp"
-# CHECK-NEXT:          ]
+# 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.type.namespace.cpp"
 # CHECK-NEXT:          ]
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","version":1,"text":"int x = 2;"}}}
 #      CHECK:  "method": "textDocument/semanticHighlighting",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "lines": [

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::Enum, "Enum"},
+      {HighlightingKind::Namespace, "Namespace"}};
   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);
R"cpp(
       void $Function[[foo]]() {
         auto $Variable[[Bou]] = $Function[[Gah]];
       }
       struct $Class[[A]] {
         void $Function[[abc]]();
       };
     )cpp",
     R"cpp(
-      namespace abc {
+      namespace $Namespace[[abc]] {
         template<typename T>
         struct $Class[[A]] {
           T t;
         };
       }
       template<typename T>
-      struct $Class[[C]] : abc::A<T> {
+      struct $Class[[C]] : $Namespace[[abc]]::A<T> {
         typename T::A* D;
       };
-      abc::$Class[[A]]<int> $Variable[[AA]];
-      typedef abc::$Class[[A]]<int> AAA;
+      $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[[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;
       };
+    )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"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);
   }
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
   class HighlightingsCounterDiagConsumer : public DiagnosticsConsumer {