[clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

[tom-anders]

Summary

Relevant issue: https://github.com/clangd/clangd/issues/1361

The problem here was that writing something like using ns::ScopedEnum
does not cause Sema to visit this scope, to it won't be added into
CodeCompletionBuilder's AccessibleScopes, leading to the qualifier not
being dropped.

To detect this, walk up the DeclContext to check if we have a matching
using declaration. If so, drop the qualifiers.

[tom-anders]

Factor out logic into helper function

[kadircet]

clang-tools-extra/clangd/CodeComplete.cpp

1993

instead of passing it here, can we just do the traversal as part of getQueryScopes and let these be handled uniformly by the shortest qualifier logic ?

[tom-anders]

clang-tools-extra/clangd/CodeComplete.cpp

1993

Yeah that was my initial idea, however the problem is that it looses the information about whether the enum is scoped or not:

1. QueryScopes only stores strings
2. The index has no flag for indicating whether an EnumConstant belongs to a scoped or unscoped enum

Thus, we get a small problem, consider the following code:

namespace ns {
enum class Scoped { Foo };
enum class Unscoped { Bar };
}

int main() {
    using ns::Scoped;
    using ns::Unscoped;

    Foo^
    Bar^
}

Here we want to complete Foo^ to ns::Scoped::Foo, but for Bar^ we want only ns::Bar. To resolve this, the scoped/unscoped information needs to be available when building the completion candidate.

Hope this was clear, maybe I missed something.

[tom-anders]

clang-tools-extra/clangd/CodeComplete.cpp

1993

Sorry, I meant the completion for Foo^ should be Scoped::Foo here, not ns::Scoped::Foo.

[tom-anders]

(friendly ping)

clang-tools-extra/clangd/CodeComplete.cpp

 // Assembles a code completion out of a bundle of >=1 completion candidates.
 // Many of the expensive strings are only computed at this point, once we know
 // the candidate bundle is going to be returned.
 //
 // Many fields are the same for all candidates in a bundle (e.g. name), and are
 // computed from the first candidate, in the constructor.
 // Others vary per candidate, so add() must be called for remaining candidates.
 struct CodeCompletionBuilder {
-  CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C,
+  CodeCompletionBuilder(ASTContext *ASTCtx, DeclContext *SemaDeclCtx,
+                        const CompletionCandidate &C,
                         CodeCompletionString *SemaCCS,
                         llvm::ArrayRef<std::string> QueryScopes,
                         const IncludeInserter &Includes,
                         llvm::StringRef FileName,
                         CodeCompletionContext::Kind ContextKind,
                         const CodeCompleteOptions &Opts,
                         bool IsUsingDeclaration, tok::TokenKind NextTokenKind)
       : ASTCtx(ASTCtx),
if (C.IndexResult) {
         llvm::StringRef ShortestQualifier = C.IndexResult->Scope;
         for (llvm::StringRef Scope : QueryScopes) {
           llvm::StringRef Qualifier = C.IndexResult->Scope;
           if (Qualifier.consume_front(Scope) &&
               Qualifier.size() < ShortestQualifier.size())
             ShortestQualifier = Qualifier;
         }
         Completion.RequiredQualifier = std::string(ShortestQualifier);
+
+        stripNamespaceForEnumConstantIfUsingDecl(*C.IndexResult, SemaDeclCtx);
       }
     }
     if (C.IdentifierResult) {
       Completion.Origin |= SymbolOrigin::Identifier;
       Completion.Kind = CompletionItemKind::Text;
       Completion.Name = std::string(C.IdentifierResult->Name);
       Completion.FilterText = Completion.Name;
     }
CodeCompletionBuilder(ASTContext *ASTCtx, DeclContext *SemaDeclCtx,
     // Prefer includes that do not need edits (i.e. already exist).
     std::stable_partition(Completion.Includes.begin(),
                           Completion.Includes.end(),
                           [](const CodeCompletion::IncludeCandidate &I) {
                             return !I.Insertion.has_value();
                           });
   }
 
+  // With all-scopes-completion, we can complete enum constants of scoped
+  // enums, in which case the completion might not be visible to Sema.
+  // So, if there's a using declaration for the enum class, manually
+  // drop the qualifiers.
+  void stripNamespaceForEnumConstantIfUsingDecl(const Symbol &IndexResult,
+                                                DeclContext *SemaDeclCtx) {
+    if (IndexResult.SymInfo.Kind != index::SymbolKind::EnumConstant)
+      return;
+    for (auto *Ctx = SemaDeclCtx; Ctx; Ctx = Ctx->getParent())
+      for (auto *D : Ctx->decls()) {
+        const auto *UD = dyn_cast<UsingShadowDecl>(D);
+        if (!UD)
+          continue;
+        const auto *ED = dyn_cast<EnumDecl>(UD->getTargetDecl());
+        if (!ED || !ED->isScoped())
+          continue;
+        auto EnumName = printQualifiedName(*ED) + "::";
+        if (EnumName == IndexResult.Scope) {
+          Completion.RequiredQualifier = ED->getName().str() + "::";
+          return;
+        }
+      }
+  }
+
   void add(const CompletionCandidate &C, CodeCompletionString *SemaCCS) {
     assert(bool(C.SemaResult) == bool(SemaCCS));
     Bundled.emplace_back();
     BundledEntry &S = Bundled.back();
     if (C.SemaResult) {
       bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern;
       getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix,
                    &Completion.RequiredQualifier, IsPattern);
private:
   CodeCompletion toCodeCompletion(const CompletionCandidate::Bundle &Bundle) {
     std::optional<CodeCompletionBuilder> Builder;
     for (const auto &Item : Bundle) {
       CodeCompletionString *SemaCCS =
           Item.SemaResult ? Recorder->codeCompletionString(*Item.SemaResult)
                           : nullptr;
       if (!Builder)
         Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr,
-                        Item, SemaCCS, QueryScopes, *Inserter, FileName,
+                        Recorder ? Recorder->CCSema->CurContext : nullptr, Item,
+                        SemaCCS, QueryScopes, *Inserter, FileName,
                         CCContextKind, Opts, IsUsingDeclaration, NextTokenKind);
       else
         Builder->add(Item, SemaCCS);
     }
     return Builder->build();
   }
 };
 

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

auto Results = completions(
     void f() { na::Clangd^ }
   )cpp",
       {cls("na::ClangdA"), cls("nx::ClangdX"), cls("Clangd3")}, Opts);
   EXPECT_THAT(Results.Completions,
               UnorderedElementsAre(
                   AllOf(qualifier(""), scope("na::"), named("ClangdA"))));
 }
 
+// https://github.com/clangd/clangd/issues/1361
+TEST(CompletionTest, ScopedEnumUsingDecl) {
+  clangd::CodeCompleteOptions Opts = {};
+  Opts.AllScopes = true;
+
+  auto Results = completions(
+      R"cpp(
+    namespace ns { enum class Scoped { FooBar }; }
+    using ns::Scoped;
+    void f() { 
+        Foo^ 
+    }
+  )cpp",
+      {enmConstant("ns::Scoped::FooBar")}, Opts);
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
+                                       qualifier("Scoped::"), named("FooBar"),
+                                       kind(CompletionItemKind::EnumMember))));
+}
+
 TEST(CompletionTest, AllScopesCompletion) {
   clangd::CodeCompleteOptions Opts = {};
   Opts.AllScopes = true;
 
   auto Results = completions(
       R"cpp(
     namespace na {
     void f() { Clangd^ }