[clangd] Put additional qualifiers to enum constants not declared as "enum class"

[kbobyrev]

Summary

This if a fix of a problem related to
https://github.com/clangd/clangd/issues/39. Currently, for this code:

c++
namespace ns {
enum Color { Green };
enum class Vehicle { Car };
}

The following is true:

• Vehicle::Car scope is ns::Vehicle:: - it can't be accessed without spelling full enum class name
• Color::Green scope is ns:: because it can be accessed as ns::Car

However, this causes index FuzzyFind to show empty results when querying
ns::Color:: - Color::Green will only show up for ns::.

This patch changes the behavior and makes SymbolCollector treat plain enum
the same way it does handle enum class. Ideally, the index would point to the
same symbol for both ns::Green and ns::Color::Green but that increases
coupling since the enum information has to be propagated to the index builder
which is logically quite far from the SymbolCollector.

[kbobyrev]

Actually, I'm not sure this change is worth it. Maybe the intent of not having enum class is actually making the constants discoverable from the outer scope.

clang-tools-extra/clangd/index/SymbolCollector.cpp

const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID,
   auto &SM = Ctx.getSourceManager();
 
   Symbol S;
   S.ID = std::move(ID);
   std::string QName = printQualifiedName(ND);
   // FIXME: this returns foo:bar: for objective-C methods, we prefer only foo:
   // for consistency with CodeCompletionString and a clean name/signature split.
   std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
+  std::string Scope = S.Scope.str();
+  // Put additional qualifiers for enum constants: technically, they can be
+  // accessed without using the enum name prefix (unless declared as enum
+  // class) but it is more convenient to enhance the scope with the enum type
+  // name.
+  // FIXME: It would be better to just have the SymbolIndex look up the same
+  // enum constant both with and without enum type name but at the time the
+  // index is built the information about ND is already lost.
+  if (const auto *EnumConstant = llvm::dyn_cast<EnumConstantDecl>(&ND)) {
+    if (const auto *II = EnumConstant->getType().getBaseTypeIdentifier()) {
+      std::string EnumScope = II->getName().str() + "::";
+      // For "enum class" the qualified name already has EnumScope.
+      if (!S.Scope.endswith(EnumScope))
+        Scope += EnumScope;
+    }
+  }
+  S.Scope = Scope;
   std::string TemplateSpecializationArgs = printTemplateSpecializationArgs(ND);
   S.TemplateSpecializationArgs = TemplateSpecializationArgs;
 
   // We collect main-file symbols, but do not use them for code completion.
   if (!IsMainFileOnly && isIndexedForCodeCompletion(ND, Ctx))
     S.Flags |= Symbol::IndexedForCodeCompletion;
   if (isImplementationDetail(&ND))
     S.Flags |= Symbol::ImplementationDetail;

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

namespace ns {
       };
     }
       )cpp";
   TU.Code = R"cpp(
       #include "foo.h"
       )cpp";
   EXPECT_THAT(getSymbols(TU, "Red"), ElementsAre(QName("Red")));
   EXPECT_THAT(getSymbols(TU, "::Red"), ElementsAre(QName("Red")));
-  EXPECT_THAT(getSymbols(TU, "Green"), ElementsAre(QName("Green")));
-  EXPECT_THAT(getSymbols(TU, "Green"), ElementsAre(QName("Green")));
+  EXPECT_THAT(getSymbols(TU, "Color::Green"),
+              ElementsAre(QName("Color::Green")));
   EXPECT_THAT(getSymbols(TU, "Color2::Yellow"),
               ElementsAre(QName("Color2::Yellow")));
   EXPECT_THAT(getSymbols(TU, "Yellow"), ElementsAre(QName("Color2::Yellow")));
 
   EXPECT_THAT(getSymbols(TU, "ns::Black"), ElementsAre(QName("ns::Black")));
-  EXPECT_THAT(getSymbols(TU, "ns::Blue"), ElementsAre(QName("ns::Blue")));
+  EXPECT_THAT(getSymbols(TU, "ns::Color3::Blue"),
+              ElementsAre(QName("ns::Color3::Blue")));
   EXPECT_THAT(getSymbols(TU, "ns::Color4::White"),
               ElementsAre(QName("ns::Color4::White")));
 }
 
 TEST(WorkspaceSymbols, Ranking) {
   TestTU TU;
   TU.AdditionalFiles["foo.h"] = R"cpp(
       namespace ns{}

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

const std::string Header = R"(
     };
     }
   )";
   runSymbolCollector(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(
                   AllOf(QName("Red"), ForCodeCompletion(true)),
                   AllOf(QName("Color"), ForCodeCompletion(true)),
-                  AllOf(QName("Green"), ForCodeCompletion(true)),
+                  AllOf(QName("Color::Green"), ForCodeCompletion(true)),
                   AllOf(QName("Color2"), ForCodeCompletion(true)),
                   AllOf(QName("Color2::Yellow"), ForCodeCompletion(false)),
                   AllOf(QName("ns"), ForCodeCompletion(true)),
                   AllOf(QName("ns::Black"), ForCodeCompletion(true))));
 }
 
 TEST_F(SymbolCollectorTest, NamelessSymbols) {
   const std::string Header = R"(