[clangd] Added highlightings for namespace specifiers.

[jvikstrom]

Summary

Added highlightings for namespace specifiers.

[hokein]

clang-tools-extra/clangd/SemanticHighlighting.cpp

41 ↗

(On Diff #208956)

nit: const auto *

42 ↗

(On Diff #208956)

nit: you miss a return

73 ↗

(On Diff #208956)

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).

247 ↗

(On Diff #208956)

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

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

114 ↗

(On Diff #208956)

could you add a testcase for anonymous namespace?

namespace {}

119 ↗

(On Diff #208956)

nit: the static is not needed for a type.

[jvikstrom]

Addressed comments.

[jvikstrom]

clang-tools-extra/clangd/SemanticHighlighting.cpp

73 ↗

(On Diff #208956)

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.

247 ↗

(On Diff #208956)

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

[sammccall]

clang-tools-extra/clangd/SemanticHighlighting.cpp

87 ↗

(On Diff #208998)

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

131 ↗

(On Diff #208998)

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?

73 ↗

(On Diff #208956)

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)

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

86 ↗

(On Diff #208998)

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

87 ↗

(On Diff #208998)

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 ↗

(On Diff #208998)

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

73 ↗

(On Diff #208956)

thanks for the explanation, fair enough.

247 ↗

(On Diff #208956)

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/trunk/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/trunk/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() !=
       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.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:          ],
 # 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.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/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::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 {