Merge pull request #36961 from benlangmuir/not-recommended-improvement

[completion] Clarify and simplify not-recommended state

Author: benlangmuir

include/swift/IDE/CodeCompletion.h

@@ -599,21 +599,21 @@ class CodeCompletionResult {
     Identical,
   };
 
-  enum NotRecommendedReason {
-    Redundant,
+  enum class NotRecommendedReason {
+    None = 0,
+    RedundantImport,
     Deprecated,
-    InvalidContext,
+    InvalidAsyncContext,
     CrossActorReference,
-    NoReason,
+    VariableUsedInOwnDefinition,
   };
 
 private:
   unsigned Kind : 3;
   unsigned AssociatedKind : 8;
   unsigned KnownOperatorKind : 6;
   unsigned SemanticContext : 3;
-  unsigned NotRecommended : 1;
-  unsigned NotRecReason : 3;
+  unsigned NotRecommended : 4;
   unsigned IsSystem : 1;
 
   /// The number of bytes to the left of the code completion point that
@@ -644,11 +644,10 @@ class CodeCompletionResult {
                            CodeCompletionOperatorKind::None,
                        StringRef BriefDocComment = StringRef())
       : Kind(Kind), KnownOperatorKind(unsigned(KnownOperatorKind)),
-        SemanticContext(unsigned(SemanticContext)), NotRecommended(false),
-        NotRecReason(NotRecommendedReason::NoReason),
+        SemanticContext(unsigned(SemanticContext)),
+        NotRecommended(unsigned(NotRecommendedReason::None)),
         NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
-        BriefDocComment(BriefDocComment),
-        TypeDistance(TypeDistance) {
+        BriefDocComment(BriefDocComment), TypeDistance(TypeDistance) {
     assert(Kind != Declaration && "use the other constructor");
     assert(CompletionString);
     if (isOperator() && KnownOperatorKind == CodeCompletionOperatorKind::None)
@@ -670,8 +669,8 @@ class CodeCompletionResult {
                        ExpectedTypeRelation TypeDistance,
                        StringRef BriefDocComment = StringRef())
       : Kind(Keyword), KnownOperatorKind(0),
-        SemanticContext(unsigned(SemanticContext)), NotRecommended(false),
-        NotRecReason(NotRecommendedReason::NoReason),
+        SemanticContext(unsigned(SemanticContext)),
+        NotRecommended(unsigned(NotRecommendedReason::None)),
         NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
         BriefDocComment(BriefDocComment), TypeDistance(TypeDistance) {
     assert(CompletionString);
@@ -688,8 +687,8 @@ class CodeCompletionResult {
                        CodeCompletionString *CompletionString,
                        ExpectedTypeRelation TypeDistance)
       : Kind(Literal), KnownOperatorKind(0),
-        SemanticContext(unsigned(SemanticContext)), NotRecommended(false),
-        NotRecReason(NotRecommendedReason::NoReason),
+        SemanticContext(unsigned(SemanticContext)),
+        NotRecommended(unsigned(NotRecommendedReason::None)),
         NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
         TypeDistance(TypeDistance) {
     AssociatedKind = static_cast<unsigned>(LiteralKind);
@@ -706,15 +705,14 @@ class CodeCompletionResult {
                        unsigned NumBytesToErase,
                        CodeCompletionString *CompletionString,
                        const Decl *AssociatedDecl, StringRef ModuleName,
-                       bool NotRecommended,
                        CodeCompletionResult::NotRecommendedReason NotRecReason,
                        StringRef BriefDocComment,
                        ArrayRef<StringRef> AssociatedUSRs,
                        ArrayRef<std::pair<StringRef, StringRef>> DocWords,
                        enum ExpectedTypeRelation TypeDistance)
       : Kind(ResultKind::Declaration), KnownOperatorKind(0),
         SemanticContext(unsigned(SemanticContext)),
-        NotRecommended(NotRecommended), NotRecReason(NotRecReason),
+        NotRecommended(unsigned(NotRecReason)),
         NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
         ModuleName(ModuleName), BriefDocComment(BriefDocComment),
         AssociatedUSRs(AssociatedUSRs), DocWords(DocWords),
@@ -735,7 +733,7 @@ class CodeCompletionResult {
                        unsigned NumBytesToErase,
                        CodeCompletionString *CompletionString,
                        CodeCompletionDeclKind DeclKind, bool IsSystem,
-                       StringRef ModuleName, bool NotRecommended,
+                       StringRef ModuleName,
                        CodeCompletionResult::NotRecommendedReason NotRecReason,
                        StringRef BriefDocComment,
                        ArrayRef<StringRef> AssociatedUSRs,
@@ -745,11 +743,11 @@ class CodeCompletionResult {
       : Kind(ResultKind::Declaration),
         KnownOperatorKind(unsigned(KnownOperatorKind)),
         SemanticContext(unsigned(SemanticContext)),
-        NotRecommended(NotRecommended), NotRecReason(NotRecReason),
-        IsSystem(IsSystem), NumBytesToErase(NumBytesToErase),
-        CompletionString(CompletionString), ModuleName(ModuleName),
-        BriefDocComment(BriefDocComment), AssociatedUSRs(AssociatedUSRs),
-        DocWords(DocWords), TypeDistance(TypeDistance) {
+        NotRecommended(unsigned(NotRecReason)), IsSystem(IsSystem),
+        NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
+        ModuleName(ModuleName), BriefDocComment(BriefDocComment),
+        AssociatedUSRs(AssociatedUSRs), DocWords(DocWords),
+        TypeDistance(TypeDistance) {
     AssociatedKind = static_cast<unsigned>(DeclKind);
     assert(CompletionString);
     assert(!isOperator() ||
@@ -800,15 +798,15 @@ class CodeCompletionResult {
   }
 
   NotRecommendedReason getNotRecommendedReason() const {
-    return static_cast<NotRecommendedReason>(NotRecReason);
+    return static_cast<NotRecommendedReason>(NotRecommended);
   }
 
   SemanticContextKind getSemanticContext() const {
     return static_cast<SemanticContextKind>(SemanticContext);
   }
 
   bool isNotRecommended() const {
-    return NotRecommended;
+    return getNotRecommendedReason() != NotRecommendedReason::None;
   }
 
   unsigned getNumBytesToErase() const {

lib/IDE/CodeCompletion.cpp

@@ -59,6 +59,7 @@ using namespace swift;
 using namespace ide;
 
 using CommandWordsPairs = std::vector<std::pair<StringRef, StringRef>>;
+using NotRecommendedReason = CodeCompletionResult::NotRecommendedReason;
 
 enum CodeCompletionCommandKind {
   none,
@@ -819,7 +820,7 @@ void CodeCompletionResultBuilder::setAssociatedDecl(const Decl *D) {
   }
 
   if (D->getAttrs().getDeprecated(D->getASTContext()))
-    setNotRecommended(CodeCompletionResult::Deprecated);
+    setNotRecommended(NotRecommendedReason::Deprecated);
 }
 
 namespace {
@@ -1294,8 +1295,7 @@ CodeCompletionResult *CodeCompletionResultBuilder::takeResult() {
 
     return new (*Sink.Allocator) CodeCompletionResult(
         SemanticContext, NumBytesToErase, CCS, AssociatedDecl, ModuleName,
-        /*NotRecommended=*/IsNotRecommended, NotRecReason,
-        copyString(*Sink.Allocator, BriefComment),
+        NotRecReason, copyString(*Sink.Allocator, BriefComment),
         copyAssociatedUSRs(*Sink.Allocator, AssociatedDecl),
         copyArray(*Sink.Allocator, CommentWords), ExpectedTypeRelation);
   }
@@ -2090,8 +2090,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
       Builder.addBaseName(MD->getNameStr());
       Builder.addTypeAnnotation("Module");
       if (Pair.second)
-        Builder.setNotRecommended(CodeCompletionResult::NotRecommendedReason::
-                                    Redundant);
+        Builder.setNotRecommended(NotRecommendedReason::RedundantImport);
     }
   }
 
@@ -2115,9 +2114,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     }
   }
 
-  void addModuleName(
-      ModuleDecl *MD,
-      Optional<CodeCompletionResult::NotRecommendedReason> R = None) {
+  void addModuleName(ModuleDecl *MD, Optional<NotRecommendedReason> R = None) {
 
     // Don't add underscored cross-import overlay modules.
     if (MD->getDeclaringModuleIfCrossImportOverlay())
@@ -2151,11 +2148,11 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
         continue;
 
       auto MD = ModuleDecl::create(ModuleName, Ctx);
-      Optional<CodeCompletionResult::NotRecommendedReason> Reason = None;
+      Optional<NotRecommendedReason> Reason = None;
 
       // Imported modules are not recommended.
       if (ImportedModules.count(MD->getNameStr()) != 0)
-        Reason = CodeCompletionResult::NotRecommendedReason::Redundant;
+        Reason = NotRecommendedReason::RedundantImport;
 
       addModuleName(MD, Reason);
     }
@@ -2518,9 +2515,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     return Type();
   }
 
-  void analyzeActorIsolation(
-      const ValueDecl *VD, Type T, bool &implicitlyAsync,
-      Optional<CodeCompletionResult::NotRecommendedReason> &NotRecommended) {
+  void analyzeActorIsolation(const ValueDecl *VD, Type T, bool &implicitlyAsync,
+                             Optional<NotRecommendedReason> &NotRecommended) {
     auto isolation = getActorIsolation(const_cast<ValueDecl *>(VD));
 
     switch (isolation.getKind()) {
@@ -2544,19 +2540,19 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     if (implicitlyAsync && T) {
       if (isa<VarDecl>(VD)) {
         if (!isSendableType(CurrDeclContext, T)) {
-          NotRecommended = CodeCompletionResult::CrossActorReference;
+          NotRecommended = NotRecommendedReason::CrossActorReference;
         }
       } else {
         assert(isa<FuncDecl>(VD) || isa<SubscriptDecl>(VD));
         // Check if the result and the param types are all 'Sendable'.
         auto *AFT = T->castTo<AnyFunctionType>();
         if (!isSendableType(CurrDeclContext, AFT->getResult())) {
-          NotRecommended = CodeCompletionResult::CrossActorReference;
+          NotRecommended = NotRecommendedReason::CrossActorReference;
         } else {
           for (auto &param : AFT->getParams()) {
             Type paramType = param.getPlainType();
             if (!isSendableType(CurrDeclContext, paramType)) {
-              NotRecommended = CodeCompletionResult::CrossActorReference;
+              NotRecommended = NotRecommendedReason::CrossActorReference;
               break;
             }
           }
@@ -2577,18 +2573,18 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     if (VD->hasInterfaceType())
       VarType = getTypeOfMember(VD, dynamicLookupInfo);
 
-    Optional<CodeCompletionResult::NotRecommendedReason> NotRecommended;
+    Optional<NotRecommendedReason> NotRecommended;
     // "not recommended" in its own getter.
     if (Kind == LookupKind::ValueInDeclContext) {
       if (auto accessor = dyn_cast<AccessorDecl>(CurrDeclContext)) {
         if (accessor->getStorage() == VD && accessor->isGetter())
-          NotRecommended = CodeCompletionResult::NoReason;
+          NotRecommended = NotRecommendedReason::VariableUsedInOwnDefinition;
       }
     }
     bool implicitlyAsync = false;
     analyzeActorIsolation(VD, VarType, implicitlyAsync, NotRecommended);
     if (!NotRecommended && implicitlyAsync && !CanCurrDeclContextHandleAsync) {
-      NotRecommended = CodeCompletionResult::InvalidContext;
+      NotRecommended = NotRecommendedReason::InvalidAsyncContext;
     }
 
     CommandWordsPairs Pairs;
@@ -2929,8 +2925,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
         addTypeAnnotation(Builder, AFT->getResult(), genericSig);
 
       if (AFT->isAsync() && !CanCurrDeclContextHandleAsync) {
-        Builder.setNotRecommended(
-            CodeCompletionResult::NotRecommendedReason::InvalidContext);
+        Builder.setNotRecommended(NotRecommendedReason::InvalidAsyncContext);
       }
     };
 
@@ -3039,14 +3034,14 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     if (AFT && !IsImplicitlyCurriedInstanceMethod)
       trivialTrailingClosure = hasTrivialTrailingClosure(FD, AFT);
 
-    Optional<CodeCompletionResult::NotRecommendedReason> NotRecommended;
+    Optional<NotRecommendedReason> NotRecommended;
     bool implictlyAsync = false;
     analyzeActorIsolation(FD, AFT, implictlyAsync, NotRecommended);
 
     if (!NotRecommended && !IsImplicitlyCurriedInstanceMethod &&
         ((AFT && AFT->isAsync()) || implictlyAsync) &&
         !CanCurrDeclContextHandleAsync) {
-      NotRecommended = CodeCompletionResult::InvalidContext;
+      NotRecommended = NotRecommendedReason::InvalidAsyncContext;
     }
 
     // Add the method, possibly including any default arguments.
@@ -3244,8 +3239,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
       }
 
       if (ConstructorType->isAsync() && !CanCurrDeclContextHandleAsync) {
-        Builder.setNotRecommended(
-            CodeCompletionResult::NotRecommendedReason::InvalidContext);
+        Builder.setNotRecommended(NotRecommendedReason::InvalidAsyncContext);
       }
     };
 
@@ -3291,12 +3285,12 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     if (!subscriptType)
       return;
 
-    Optional<CodeCompletionResult::NotRecommendedReason> NotRecommended;
+    Optional<NotRecommendedReason> NotRecommended;
     bool implictlyAsync = false;
     analyzeActorIsolation(SD, subscriptType, implictlyAsync, NotRecommended);
 
     if (!NotRecommended && implictlyAsync && !CanCurrDeclContextHandleAsync) {
-      NotRecommended = CodeCompletionResult::InvalidContext;
+      NotRecommended = NotRecommendedReason::InvalidAsyncContext;
     }
 
     CommandWordsPairs Pairs;

lib/IDE/CodeCompletionCache.cpp

@@ -192,7 +192,8 @@ static bool readCachedModule(llvm::MemoryBuffer *in,
     auto declKind = static_cast<CodeCompletionDeclKind>(*cursor++);
     auto opKind = static_cast<CodeCompletionOperatorKind>(*cursor++);
     auto context = static_cast<SemanticContextKind>(*cursor++);
-    auto notRecommended = static_cast<bool>(*cursor++);
+    auto notRecommended =
+        static_cast<CodeCompletionResult::NotRecommendedReason>(*cursor++);
     auto isSystem = static_cast<bool>(*cursor++);
     auto numBytesToErase = static_cast<unsigned>(*cursor++);
     auto oldCursor = cursor;
@@ -228,9 +229,10 @@ static bool readCachedModule(llvm::MemoryBuffer *in,
     if (kind == CodeCompletionResult::Declaration) {
       result = new (*V.Sink.Allocator) CodeCompletionResult(
           context, numBytesToErase, string, declKind, isSystem, moduleName,
-          notRecommended, CodeCompletionResult::NotRecommendedReason::NoReason,
-          briefDocComment, copyArray(*V.Sink.Allocator, ArrayRef<StringRef>(assocUSRs)),
-          copyArray(*V.Sink.Allocator, ArrayRef<std::pair<StringRef, StringRef>>(declKeywords)),
+          notRecommended, briefDocComment,
+          copyArray(*V.Sink.Allocator, ArrayRef<StringRef>(assocUSRs)),
+          copyArray(*V.Sink.Allocator,
+                    ArrayRef<std::pair<StringRef, StringRef>>(declKeywords)),
           CodeCompletionResult::Unknown, opKind);
     } else {
       result = new (*V.Sink.Allocator)
@@ -349,7 +351,7 @@ static void writeCachedModule(llvm::raw_ostream &out,
       else
         LE.write(static_cast<uint8_t>(CodeCompletionOperatorKind::None));
       LE.write(static_cast<uint8_t>(R->getSemanticContext()));
-      LE.write(static_cast<uint8_t>(R->isNotRecommended()));
+      LE.write(static_cast<uint8_t>(R->getNotRecommendedReason()));
       LE.write(static_cast<uint8_t>(R->isSystem()));
       LE.write(static_cast<uint8_t>(R->getNumBytesToErase()));
       LE.write(

lib/IDE/CodeCompletionResultBuilder.h

@@ -89,9 +89,8 @@ class CodeCompletionResultBuilder {
       CodeCompletionResult::Unknown;
   bool Cancelled = false;
   ArrayRef<std::pair<StringRef, StringRef>> CommentWords;
-  bool IsNotRecommended = false;
   CodeCompletionResult::NotRecommendedReason NotRecReason =
-    CodeCompletionResult::NotRecommendedReason::NoReason;
+      CodeCompletionResult::NotRecommendedReason::None;
   StringRef BriefDocComment = StringRef();
 
   void addChunkWithText(CodeCompletionString::Chunk::ChunkKind Kind,
@@ -148,7 +147,6 @@ class CodeCompletionResultBuilder {
   void setLiteralKind(CodeCompletionLiteralKind kind) { LiteralKind = kind; }
   void setKeywordKind(CodeCompletionKeywordKind kind) { KeywordKind = kind; }
   void setNotRecommended(CodeCompletionResult::NotRecommendedReason Reason) {
-    IsNotRecommended = true;
     NotRecReason = Reason;
   }
 

tools/SourceKit/lib/SwiftLang/CodeCompletionOrganizer.cpp

@@ -1169,10 +1169,9 @@ Completion *CompletionBuilder::finish() {
       base = SwiftResult(
           semanticContext, current.getNumBytesToErase(), completionString,
           current.getAssociatedDeclKind(), current.isSystem(),
-          current.getModuleName(), current.isNotRecommended(),
-          current.getNotRecommendedReason(), current.getBriefDocComment(),
-          current.getAssociatedUSRs(), current.getDeclKeywords(),
-          typeRelation, opKind);
+          current.getModuleName(), current.getNotRecommendedReason(),
+          current.getBriefDocComment(), current.getAssociatedUSRs(),
+          current.getDeclKeywords(), typeRelation, opKind);
     } else {
       base = SwiftResult(current.getKind(), semanticContext,
                          current.getNumBytesToErase(), completionString,