[CodeComplete] Complete enumerators when preferred type is an enum

[kadircet]

LGTM, thanks!

clang/test/CodeCompletion/enum-preferred-type.cpp

14

Could you also add an else if case, with a similar fixme as in the code?
So that when someone fixes it, we can be sure that fix is working.

[ilya-biryukov]

• add a test case with a fixme

[kadircet]

clang/test/CodeCompletion/enum-preferred-type.cpp

20

I suppose line number needs to be 14?

[ilya-biryukov]

• Fix a line number

[ilya-biryukov]

Thanks for a quick review!

clang/test/CodeCompletion/enum-preferred-type.cpp

20

Right. Otherwise tests fail.

clang/lib/Sema/SemaCodeComplete.cpp

struct Sema::CodeCompleteExpressionData {
 
   QualType PreferredType;
   bool IntegralConstantExpression;
   bool ObjCCollection;
   bool IsParenthesized;
   SmallVector<Decl *, 4> IgnoreDecls;
 };
 
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+  llvm::SmallPtrSet<EnumConstantDecl *, 8> Seen;
+  NestedNameSpecifier *SuggestedQualifier = nullptr;
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+                           EnumDecl *Enum, DeclContext *CurContext,
+                           const CoveredEnumerators &Enumerators) {
+  NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
+  if (Context.getLangOpts().CPlusPlus && !Qualifier && Enumerators.Seen.empty()) {
+    // If there are no prior enumerators in C++, check whether we have to
+    // qualify the names of the enumerators that we suggest, because they
+    // may not be visible in this scope.
+    Qualifier = getRequiredQualification(Context, CurContext, Enum);
+  }
+
+  Results.EnterNewScope();
+  for (auto *E : Enum->enumerators()) {
+    if (Enumerators.Seen.count(E))
+      continue;
+
+    CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
+    Results.AddResult(R, CurContext, nullptr, false);
+  }
+  Results.ExitScope();
+}
+
 /// Perform code-completion in an expression context when we know what
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S,
                                   const CodeCompleteExpressionData &Data) {
   ResultBuilder Results(
       *this, CodeCompleter->getAllocator(),
       CodeCompleter->getCodeCompletionTUInfo(),
       CodeCompletionContext(
LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
                      CodeCompleter->includeGlobals(),
                      CodeCompleter->loadExternal());
 
   Results.EnterNewScope();
   AddOrdinaryNameResults(PCC, S, *this, Results);
   Results.ExitScope();
 
   bool PreferredTypeIsPointer = false;
-  if (!Data.PreferredType.isNull())
+  if (!Data.PreferredType.isNull()) {
     PreferredTypeIsPointer = Data.PreferredType->isAnyPointerType() ||
                              Data.PreferredType->isMemberPointerType() ||
                              Data.PreferredType->isBlockPointerType();
+    if (Data.PreferredType->isEnumeralType()) {
+      EnumDecl *Enum = Data.PreferredType->castAs<EnumType>()->getDecl();
+      if (auto *Def = Enum->getDefinition())
+        Enum = Def;
+      // FIXME: collect covered enumerators in cases like:
+      //        if (x == my_enum::one) { ... } else if (x == ^) {}
+      AddEnumerators(Results, Context, Enum, CurContext, CoveredEnumerators());
+    }
+  }
 
   if (S->getFnParent() && !Data.ObjCCollection &&
       !Data.IntegralConstantExpression)
     AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
     AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false,
                     PreferredTypeIsPointer);
void Sema::CodeCompleteCase(Scope *S) {
   EnumDecl *Enum = type->castAs<EnumType>()->getDecl();
   if (EnumDecl *Def = Enum->getDefinition())
     Enum = Def;
 
   // Determine which enumerators we have already seen in the switch statement.
   // FIXME: Ideally, we would also be able to look *past* the code-completion
   // token, in case we are code-completing in the middle of the switch and not
   // at the end. However, we aren't able to do so at the moment.
-  llvm::SmallPtrSet<EnumConstantDecl *, 8> EnumeratorsSeen;
-  NestedNameSpecifier *Qualifier = nullptr;
+  CoveredEnumerators Enumerators;
   for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
        SC = SC->getNextSwitchCase()) {
     CaseStmt *Case = dyn_cast<CaseStmt>(SC);
     if (!Case)
       continue;
 
     Expr *CaseVal = Case->getLHS()->IgnoreParenCasts();
     if (auto *DRE = dyn_cast<DeclRefExpr>(CaseVal))
       if (auto *Enumerator =
               dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
         // We look into the AST of the case statement to determine which
         // enumerator was named. Alternatively, we could compute the value of
         // the integral constant expression, then compare it against the
         // values of each enumerator. However, value-based approach would not
         // work as well with C++ templates where enumerators declared within a
         // template are type- and value-dependent.
-        EnumeratorsSeen.insert(Enumerator);
+        Enumerators.Seen.insert(Enumerator);
 
         // If this is a qualified-id, keep track of the nested-name-specifier
         // so that we can reproduce it as part of code completion, e.g.,
         //
         //   switch (TagD.getKind()) {
         //     case TagDecl::TK_enum:
         //       break;
         //     case XXX
         //
         // At the XXX, our completions are TagDecl::TK_union,
         // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
         // TK_struct, and TK_class.
-        Qualifier = DRE->getQualifier();
+        Enumerators.SuggestedQualifier = DRE->getQualifier();
       }
   }
 
-  if (getLangOpts().CPlusPlus && !Qualifier && EnumeratorsSeen.empty()) {
-    // If there are no prior enumerators in C++, check whether we have to
-    // qualify the names of the enumerators that we suggest, because they
-    // may not be visible in this scope.
-    Qualifier = getRequiredQualification(Context, CurContext, Enum);
-  }
-
   // Add any enumerators that have not yet been mentioned.
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
                         CodeCompleter->getCodeCompletionTUInfo(),
                         CodeCompletionContext::CCC_Expression);
-  Results.EnterNewScope();
-  for (auto *E : Enum->enumerators()) {
-    if (EnumeratorsSeen.count(E))
-      continue;
-
-    CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
-    Results.AddResult(R, CurContext, nullptr, false);
-  }
-  Results.ExitScope();
+  AddEnumerators(Results, Context, Enum, CurContext, Enumerators);
 
   if (CodeCompleter->includeMacros()) {
     AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size());
 }
 

clang/test/CodeCompletion/enum-preferred-type.cpp

+namespace N {
+  enum Color {
+    Red,
+    Blue,
+    Orange,
+  };
+}
+
+void test(N::Color color) {
+  color = N::Color::Red;
+  test(N::Color::Red);
+  if (color == N::Color::Red) {}
+  // FIXME: ideally, we should not show 'Red' on the next line.
+  else if (color == N::Color::Blue) {}
+
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:11 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:11:8 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:21 %s -o - | FileCheck %s
+  // CHECK: Blue : [#N::Color#]N::Blue
+  // CHECK: color : [#N::Color#]color
+  // CHECK: Orange : [#N::Color#]N::Orange
+  // CHECK: Red : [#N::Color#]N::Red
+}