[clangd] Add scoped enum constants to all-scopes-completion

[tom-anders]

Summary

This was originally part of https://reviews.llvm.org/D136925, but we decided to move it to a separate patch.
In case it turns out to be controversial, it can be reverted more easily.

[tom-anders]

Added missing hunk in CodeComplete.cpp

[tom-anders]

Rebase onto previous commit

[nridge]

The test added in the previous patch, CompletionTest.Enums, needs to be updated to reflect this change (Scoped::Clangd3 now appears as a completion).

clang-tools-extra/clangd/CodeComplete.cpp

2136

Why remove the (InTopLevelScope(*EnumDecl) || InClassScope(*EnumDecl)) part?

[tom-anders]

Update CompletionTest.Enums, keep scope condition in isIndexedForCodeCompletion

[tom-anders]

clang-tools-extra/clangd/CodeComplete.cpp

2136

Huh, because I thought this covered all the scopes where you could declare enums anyway, but of course that's wrong - You can e.g. declare a function-local enum, which we of course don't want to index.

2136

d

[tom-anders]

Actually fix test...

[nridge]

Looks good, thanks

clang-tools-extra/clangd/CodeComplete.h

 SignatureHelp signatureHelp(PathRef FileName, Position Pos,
                             const PreambleData &Preamble,
                             const ParseInputs &ParseInput,
                             MarkupKind DocumentationFormat);
 
 // For index-based completion, we only consider:
 //   * symbols in namespaces or translation unit scopes (e.g. no class
 //     members, no locals)
-//   * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
+//   * enum constants (both scoped and unscoped)
 //   * primary templates (no specializations)
 // For the other cases, we let Clang do the completion because it does not
 // need any non-local information and it will be much better at following
 // lookup rules. Other symbols still appear in the index for other purposes,
 // like workspace/symbols or textDocument/definition, but are not used for code
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx);
 

clang-tools-extra/clangd/CodeComplete.cpp

auto InTopLevelScope = [](const NamedDecl &ND) {
     case Decl::Namespace:
     case Decl::LinkageSpec:
       return true;
     default:
       break;
     };
     return false;
   };
-  auto InClassScope = [](const NamedDecl &ND) {
-    return ND.getDeclContext()->getDeclKind() == Decl::CXXRecord;
-  };
   // We only complete symbol's name, which is the same as the name of the
   // *primary* template in case of template specializations.
   if (isExplicitTemplateSpecialization(&ND))
     return false;
 
   // Category decls are not useful on their own outside the interface or
   // implementation blocks. Moreover, sema already provides completion for
   // these, even if it requires preamble deserialization. So by excluding them
   // from the index, we reduce the noise in all the other completion scopes.
   if (llvm::isa<ObjCCategoryDecl>(&ND) || llvm::isa<ObjCCategoryImplDecl>(&ND))
     return false;
 
   if (InTopLevelScope(ND))
     return true;
 
   // Always index enum constants, even if they're not in the top level scope:
   // when
   // --all-scopes-completion is set, we'll want to complete those as well.
-  if (const auto *EnumDecl = dyn_cast<clang::EnumDecl>(ND.getDeclContext()))
-    return (InTopLevelScope(*EnumDecl) || InClassScope(*EnumDecl)) && !EnumDecl->isScoped();
+  if (llvm::isa<clang::EnumDecl>(ND.getDeclContext()))
+    return true;
 
   return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
   CompletionItem LSP;
   const auto *InsertInclude = Includes.empty() ? nullptr : &Includes[0];
   LSP.label = ((InsertInclude && InsertInclude->Insertion)

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

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