[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

2012–2013

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

2012–2013

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

2012–2013

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> AccessibleScopes,
                         const IncludeInserter &Includes,
                         llvm::StringRef FileName,
                         CodeCompletionContext::Kind ContextKind,
                         const CodeCompleteOptions &Opts,
                         bool IsUsingDeclaration, tok::TokenKind NextTokenKind)
       : ASTCtx(ASTCtx),
if (C.IndexResult) {
         Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind);
       if (Completion.Name.empty())
         Completion.Name = std::string(C.IndexResult->Name);
       if (Completion.FilterText.empty())
         Completion.FilterText = Completion.Name;
       // If the completion was visible to Sema, no qualifier is needed. This
       // avoids unneeded qualifiers in cases like with `using ns::X`.
       if (Completion.RequiredQualifier.empty() && !C.SemaResult) {
+        // 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.
+        if (C.IndexResult->SymInfo.Kind == index::SymbolKind::EnumConstant &&
+            SemaDeclCtx && ASTCtx) {
+          for (auto *Ctx = SemaDeclCtx; Ctx; Ctx = Ctx->getParent()) {
+            auto MatchingUsingDecl = llvm::find_if(Ctx->decls(), [&C](Decl *D) {
+              if (const auto *UD = dyn_cast<UsingShadowDecl>(D))
+                if (const auto *ED = dyn_cast<EnumDecl>(UD->getTargetDecl()))
+                  if (ED->isScoped() &&
+                      printQualifiedName(*ED) + "::" == C.IndexResult->Scope)
+                    return true;
+              return false;
+            });
+            if (MatchingUsingDecl != Ctx->decls_end()) {
+              assert(MatchingUsingDecl->isNamed());
+              Completion.RequiredQualifier =
+                  dyn_cast<NamedDecl>(*MatchingUsingDecl)->getName().str() +
+                  "::";
+              break;
+            }
+          }
+        } else {
           llvm::StringRef ShortestQualifier = C.IndexResult->Scope;
           for (llvm::StringRef Scope : AccessibleScopes) {
             llvm::StringRef Qualifier = C.IndexResult->Scope;
             if (Qualifier.consume_front(Scope) &&
                 Qualifier.size() < ShortestQualifier.size())
               ShortestQualifier = Qualifier;
           }
           Completion.RequiredQualifier = std::string(ShortestQualifier);
         }
       }
+    }
     if (C.IdentifierResult) {
       Completion.Origin |= SymbolOrigin::Identifier;
       Completion.Kind = CompletionItemKind::Text;
       Completion.Name = std::string(C.IdentifierResult->Name);
       Completion.FilterText = Completion.Name;
     }
 
     // Turn absolute path into a literal string that can be #included.
private:
   CodeCompletion toCodeCompletion(const CompletionCandidate::Bundle &Bundle) {
     llvm::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, AccessibleScopes, *Inserter, FileName,
+                        Recorder ? Recorder->CCSema->CurContext : nullptr, Item,
+                        SemaCCS, AccessibleScopes, *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^ }