[CodeCompletion] Don't recommend functions with async alternatives in async contexts

When an function has an async alternative, that should be preferred when we are completing in an async context. Thus, the sync method should be marked as not recommended if the current context can handle async methods.

rdar://88354910

Author: ahoppen

include/swift/AST/DiagnosticsIDE.def

@@ -26,6 +26,10 @@ WARNING(ide_cross_actor_reference_swift5, none,
         "actor-isolated %0 should only be referenced from inside the actor",
         (StringRef))
 
+WARNING(ide_has_async_alternative, none,
+        "%0 has an async alternative that should be preferred in an async "
+        "context", (StringRef))
+
 WARNING(ide_redundant_import, none,
         "module %0 is already imported", (StringRef))
 

include/swift/IDE/CodeCompletionResult.h

@@ -251,16 +251,17 @@ enum class CodeCompletionDiagnosticSeverity : uint8_t {
 /// E.g. \c InvalidAsyncContext depends on whether the usage context is async or
 /// not.
 enum class NotRecommendedReason : uint8_t {
-  None = 0,                    // both contextual and context-free
-  RedundantImport,             // contextual
-  RedundantImportIndirect,     // contextual
-  Deprecated,                  // context-free
-  SoftDeprecated,              // context-free
-  InvalidAsyncContext,         // contextual
-  CrossActorReference,         // contextual
-  VariableUsedInOwnDefinition, // contextual
-
-  MAX_VALUE = VariableUsedInOwnDefinition
+  None = 0,                              // both contextual and context-free
+  RedundantImport,                       // contextual
+  RedundantImportIndirect,               // contextual
+  Deprecated,                            // context-free
+  SoftDeprecated,                        // context-free
+  InvalidAsyncContext,                   // contextual
+  CrossActorReference,                   // contextual
+  VariableUsedInOwnDefinition,           // contextual
+  NonAsyncAlternativeUsedInAsyncContext, // contextual
+
+  MAX_VALUE = NonAsyncAlternativeUsedInAsyncContext
 };
 
 /// TODO: We consider deprecation warnings as context free although they don't
@@ -294,10 +295,14 @@ enum class ContextualNotRecommendedReason : uint8_t {
   None = 0,
   RedundantImport,
   RedundantImportIndirect,
+  /// A method that is async is being used in a non-async context.
   InvalidAsyncContext,
   CrossActorReference,
   VariableUsedInOwnDefinition,
-  MAX_VALUE = VariableUsedInOwnDefinition
+  /// A method that is sync and has an async alternative is used in an async
+  /// context.
+  NonAsyncAlternativeUsedInAsyncContext,
+  MAX_VALUE = NonAsyncAlternativeUsedInAsyncContext
 };
 
 enum class CodeCompletionResultKind : uint8_t {
@@ -330,6 +335,9 @@ class ContextFreeCodeCompletionResult {
 
   bool IsSystem : 1;
   bool IsAsync : 1;
+  /// Whether the result has been annotated as having an async alternative that
+  /// should be prefered in async contexts.
+  bool HasAsyncAlternative : 1;
   CodeCompletionString *CompletionString;
   NullTerminatedStringRef ModuleName;
   NullTerminatedStringRef BriefDocComment;
@@ -361,7 +369,7 @@ class ContextFreeCodeCompletionResult {
   ContextFreeCodeCompletionResult(
       CodeCompletionResultKind Kind, uint8_t AssociatedKind,
       CodeCompletionOperatorKind KnownOperatorKind, bool IsSystem, bool IsAsync,
-      CodeCompletionString *CompletionString,
+      bool HasAsyncAlternative, CodeCompletionString *CompletionString,
       NullTerminatedStringRef ModuleName,
       NullTerminatedStringRef BriefDocComment,
       ArrayRef<NullTerminatedStringRef> AssociatedUSRs,
@@ -372,10 +380,11 @@ class ContextFreeCodeCompletionResult {
       NullTerminatedStringRef FilterName,
       NullTerminatedStringRef NameForDiagnostics)
       : Kind(Kind), KnownOperatorKind(KnownOperatorKind), IsSystem(IsSystem),
-        IsAsync(IsAsync), CompletionString(CompletionString),
-        ModuleName(ModuleName), BriefDocComment(BriefDocComment),
-        AssociatedUSRs(AssociatedUSRs), ResultType(ResultType),
-        NotRecommended(NotRecommended), DiagnosticSeverity(DiagnosticSeverity),
+        IsAsync(IsAsync), HasAsyncAlternative(HasAsyncAlternative),
+        CompletionString(CompletionString), ModuleName(ModuleName),
+        BriefDocComment(BriefDocComment), AssociatedUSRs(AssociatedUSRs),
+        ResultType(ResultType), NotRecommended(NotRecommended),
+        DiagnosticSeverity(DiagnosticSeverity),
         DiagnosticMessage(DiagnosticMessage), FilterName(FilterName),
         NameForDiagnostics(NameForDiagnostics) {
     this->AssociatedKind.Opaque = AssociatedKind;
@@ -387,6 +396,8 @@ class ContextFreeCodeCompletionResult {
            "Completion item should have diagnostic message iff the diagnostics "
            "severity is not none");
     assert(CompletionString && "Result should have a completion string");
+    assert(!(HasAsyncAlternative && IsAsync) &&
+           "A function shouldn't be both async and have an async alternative");
     if (isOperator() && KnownOperatorKind == CodeCompletionOperatorKind::None) {
       this->KnownOperatorKind = getCodeCompletionOperatorKind(CompletionString);
     }
@@ -442,7 +453,7 @@ class ContextFreeCodeCompletionResult {
   createDeclResult(CodeCompletionResultSink &Sink,
                    CodeCompletionString *CompletionString,
                    const Decl *AssociatedDecl, bool IsAsync,
-                   NullTerminatedStringRef ModuleName,
+                   bool HasAsyncAlternative, NullTerminatedStringRef ModuleName,
                    NullTerminatedStringRef BriefDocComment,
                    ArrayRef<NullTerminatedStringRef> AssociatedUSRs,
                    CodeCompletionResultType ResultType,
@@ -480,6 +491,8 @@ class ContextFreeCodeCompletionResult {
 
   bool isAsync() const { return IsAsync; };
 
+  bool hasAsyncAlternative() const { return HasAsyncAlternative; };
+
   CodeCompletionString *getCompletionString() const { return CompletionString; }
 
   NullTerminatedStringRef getModuleName() const { return ModuleName; }
@@ -671,6 +684,8 @@ class CodeCompletionResult {
       return NotRecommendedReason::RedundantImportIndirect;
     case ContextualNotRecommendedReason::InvalidAsyncContext:
       return NotRecommendedReason::InvalidAsyncContext;
+    case ContextualNotRecommendedReason::NonAsyncAlternativeUsedInAsyncContext:
+      return NotRecommendedReason::NonAsyncAlternativeUsedInAsyncContext;
     case ContextualNotRecommendedReason::CrossActorReference:
       return NotRecommendedReason::CrossActorReference;
     case ContextualNotRecommendedReason::VariableUsedInOwnDefinition:

lib/IDE/CodeCompletionCache.cpp

@@ -104,7 +104,7 @@ CodeCompletionCache::~CodeCompletionCache() {}
 /// This should be incremented any time we commit a change to the format of the
 /// cached results. This isn't expected to change very often.
 static constexpr uint32_t onDiskCompletionCacheVersion =
-    9; // Store whether a decl is async
+    10; // Store if decl has an async alternative
 
 /// Deserializes CodeCompletionResults from \p in and stores them in \p V.
 /// \see writeCacheModule.
@@ -236,6 +236,7 @@ static bool readCachedModule(llvm::MemoryBuffer *in,
         static_cast<CodeCompletionDiagnosticSeverity>(*cursor++);
     auto isSystem = static_cast<bool>(*cursor++);
     auto isAsync = static_cast<bool>(*cursor++);
+    auto hasAsyncAlternative = static_cast<bool>(*cursor++);
     auto chunkIndex = read32le(cursor);
     auto moduleIndex = read32le(cursor);
     auto briefDocIndex = read32le(cursor);
@@ -265,8 +266,9 @@ static bool readCachedModule(llvm::MemoryBuffer *in,
 
     ContextFreeCodeCompletionResult *result =
         new (*V.Allocator) ContextFreeCodeCompletionResult(
-            kind, associatedKind, opKind, isSystem, isAsync, string, moduleName,
-            briefDocComment, makeArrayRef(assocUSRs).copy(*V.Allocator),
+            kind, associatedKind, opKind, isSystem, isAsync,
+            hasAsyncAlternative, string, moduleName, briefDocComment,
+            makeArrayRef(assocUSRs).copy(*V.Allocator),
             CodeCompletionResultType(resultTypes), notRecommended, diagSeverity,
             diagMessage, filterName, nameForDiagnostics);
 
@@ -425,6 +427,7 @@ static void writeCachedModule(llvm::raw_ostream &out,
       LE.write(static_cast<uint8_t>(R->getDiagnosticSeverity()));
       LE.write(static_cast<uint8_t>(R->isSystem()));
       LE.write(static_cast<uint8_t>(R->isAsync()));
+      LE.write(static_cast<uint8_t>(R->hasAsyncAlternative()));
       LE.write(
           static_cast<uint32_t>(addCompletionString(R->getCompletionString())));
       LE.write(addString(R->getModuleName()));      // index into strings

lib/IDE/CodeCompletionDiagnostics.cpp

@@ -183,6 +183,9 @@ bool swift::ide::getContextualCompletionDiagnostics(
     return Diag.getDiagnostics(Severity, Out,
                                diag::ide_recursive_accessor_reference,
                                NameForDiagnostics, /*"getter"*/ 0);
+  case ContextualNotRecommendedReason::NonAsyncAlternativeUsedInAsyncContext:
+    return Diag.getDiagnostics(
+        Severity, Out, diag::ide_has_async_alternative, NameForDiagnostics);
   case ContextualNotRecommendedReason::None:
     llvm_unreachable("invalid not recommended reason");
   }

lib/IDE/CodeCompletionResult.cpp

@@ -43,7 +43,7 @@ ContextFreeCodeCompletionResult::createPatternOrBuiltInOperatorResult(
   }
   return new (Sink.getAllocator()) ContextFreeCodeCompletionResult(
       Kind, /*AssociatedKind=*/0, KnownOperatorKind,
-      /*IsSystem=*/false, IsAsync, CompletionString,
+      /*IsSystem=*/false, IsAsync, /*HasAsyncAlternative=*/false, CompletionString,
       /*ModuleName=*/"", BriefDocComment,
       /*AssociatedUSRs=*/{}, ResultType, NotRecommended, DiagnosticSeverity,
       DiagnosticMessage,
@@ -63,7 +63,7 @@ ContextFreeCodeCompletionResult::createKeywordResult(
   return new (Sink.getAllocator()) ContextFreeCodeCompletionResult(
       CodeCompletionResultKind::Keyword, static_cast<uint8_t>(Kind),
       CodeCompletionOperatorKind::None, /*IsSystem=*/false, /*IsAsync=*/false,
-      CompletionString,
+      /*HasAsyncAlternative=*/false, CompletionString,
       /*ModuleName=*/"", BriefDocComment,
       /*AssociatedUSRs=*/{}, ResultType, ContextFreeNotRecommendedReason::None,
       CodeCompletionDiagnosticSeverity::None, /*DiagnosticMessage=*/"",
@@ -82,7 +82,8 @@ ContextFreeCodeCompletionResult::createLiteralResult(
   return new (Sink.getAllocator()) ContextFreeCodeCompletionResult(
       CodeCompletionResultKind::Literal, static_cast<uint8_t>(LiteralKind),
       CodeCompletionOperatorKind::None,
-      /*IsSystem=*/false, /*IsAsync=*/false, CompletionString,
+      /*IsSystem=*/false, /*IsAsync=*/false, /*HasAsyncAlternative=*/false,
+      CompletionString,
       /*ModuleName=*/"",
       /*BriefDocComment=*/"",
       /*AssociatedUSRs=*/{}, ResultType, ContextFreeNotRecommendedReason::None,
@@ -109,7 +110,7 @@ getDeclNameForDiagnostics(const Decl *D, CodeCompletionResultSink &Sink) {
 ContextFreeCodeCompletionResult *
 ContextFreeCodeCompletionResult::createDeclResult(
     CodeCompletionResultSink &Sink, CodeCompletionString *CompletionString,
-    const Decl *AssociatedDecl, bool IsAsync,
+    const Decl *AssociatedDecl, bool IsAsync, bool HasAsyncAlternative,
     NullTerminatedStringRef ModuleName, NullTerminatedStringRef BriefDocComment,
     ArrayRef<NullTerminatedStringRef> AssociatedUSRs,
     CodeCompletionResultType ResultType,
@@ -124,8 +125,9 @@ ContextFreeCodeCompletionResult::createDeclResult(
       CodeCompletionResultKind::Declaration,
       static_cast<uint8_t>(getCodeCompletionDeclKind(AssociatedDecl)),
       CodeCompletionOperatorKind::None, getDeclIsSystem(AssociatedDecl),
-      IsAsync, CompletionString, ModuleName, BriefDocComment, AssociatedUSRs,
-      ResultType, NotRecommended, DiagnosticSeverity, DiagnosticMessage,
+      IsAsync, HasAsyncAlternative, CompletionString, ModuleName,
+      BriefDocComment, AssociatedUSRs, ResultType, NotRecommended,
+      DiagnosticSeverity, DiagnosticMessage,
       getCodeCompletionResultFilterName(CompletionString, Sink.getAllocator()),
       /*NameForDiagnostics=*/getDeclNameForDiagnostics(AssociatedDecl, Sink));
 }
@@ -301,6 +303,10 @@ getNotRecommenedReason(const ContextFreeCodeCompletionResult &ContextFree,
   if (ContextFree.isAsync() && !CanCurrDeclContextHandleAsync) {
     return ContextualNotRecommendedReason::InvalidAsyncContext;
   }
+  if (ContextFree.hasAsyncAlternative() && CanCurrDeclContextHandleAsync) {
+    return ContextualNotRecommendedReason::
+        NonAsyncAlternativeUsedInAsyncContext;
+  }
   return ContextualNotRecommendedReason::None;
 }
 

lib/IDE/CodeCompletionResultBuilder.cpp

@@ -128,7 +128,7 @@ CodeCompletionResult *CodeCompletionResultBuilder::takeResult() {
     }
 
     ContextFreeResult = ContextFreeCodeCompletionResult::createDeclResult(
-        Sink, CCS, AssociatedDecl, IsAsync, ModuleName,
+        Sink, CCS, AssociatedDecl, IsAsync, HasAsyncAlternative, ModuleName,
         NullTerminatedStringRef(BriefDocComment, Allocator),
         copyAssociatedUSRs(Allocator, AssociatedDecl), ResultType,
         ContextFreeNotRecReason, ContextFreeDiagnosticSeverity,

lib/IDE/CodeCompletionResultBuilder.h

@@ -44,6 +44,7 @@ class CodeCompletionResultBuilder {
   unsigned NumBytesToErase = 0;
   const Decl *AssociatedDecl = nullptr;
   bool IsAsync = false;
+  bool HasAsyncAlternative = false;
   Optional<CodeCompletionLiteralKind> LiteralKind;
   CodeCompletionKeywordKind KeywordKind = CodeCompletionKeywordKind::None;
   unsigned CurrentNestingLevel = 0;
@@ -116,6 +117,9 @@ class CodeCompletionResultBuilder {
   void setAssociatedDecl(const Decl *D);
 
   void setIsAsync(bool IsAsync) { this->IsAsync = IsAsync; }
+  void setHasAsyncAlternative(bool HasAsyncAlternative) {
+    this->HasAsyncAlternative = HasAsyncAlternative;
+  }
 
   void setLiteralKind(CodeCompletionLiteralKind kind) { LiteralKind = kind; }
   void setKeywordKind(CodeCompletionKeywordKind kind) { KeywordKind = kind; }

lib/IDE/CompletionLookup.cpp

@@ -1351,6 +1351,9 @@ void CompletionLookup::addMethodCall(const FuncDecl *FD,
         Sink, CodeCompletionResultKind::Declaration,
         getSemanticContext(FD, Reason, dynamicLookupInfo));
     Builder.setIsAsync(implictlyAsync || (AFT->hasExtInfo() && AFT->isAsync()));
+    Builder.setHasAsyncAlternative(
+        FD->getAsyncAlternative() &&
+        !FD->getAsyncAlternative()->shouldHideFromEditor());
     Builder.setCanCurrDeclContextHandleAsync(CanCurrDeclContextHandleAsync);
     Builder.setAssociatedDecl(FD);
 

test/IDE/complete_async_imported.swift

@@ -37,14 +37,14 @@ func test(obj: MyObjCClass) async throws {
 // COMPLETE-NOT: method4()
 // COMPLETE-NOT: method5()
 // COMPLETE-DAG: Keyword[self]/CurrNominal:          self[#MyObjCClass#]; name=self
-// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   method1({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#];
+// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended:   method1({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#]; name=method1(completionHandler:); diagnostics=warning:'method1(completionHandler:)' has an async alternative that should be preferred in an async context
 // COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   method1()[' async'][' throws'][#Void#];
 // COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   named3()[' async'][' throws'][#Void#];
 // COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   method4({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#];
 // COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   method5({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#];
-// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   method6({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#];
+// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended:   method6({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#]; name=method6(completionHandler:); diagnostics=warning:'method6(completionHandler:)' has an async alternative that should be preferred in an async context
 // COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   method6()[' async'][' throws'][#Void#];
-// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   method7({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#];
+// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended:   method7({#completionHandler: (Error?) -> Void##(Error?) -> Void#})[#Void#]; name=method7(completionHandler:); diagnostics=warning:'method7(completionHandler:)' has an async alternative that should be preferred in an async context
 // COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal:   named7()[' async'][' throws'][#Void#];
 // COMPLETE: End completions
 }

tools/SourceKit/lib/SwiftLang/CodeCompletionOrganizer.cpp

@@ -1168,7 +1168,8 @@ Completion *CompletionBuilder::finish() {
             contextFreeBase.getKind(),
             contextFreeBase.getOpaqueAssociatedKind(), opKind,
             contextFreeBase.isSystem(), contextFreeBase.isAsync(),
-            newCompletionString, contextFreeBase.getModuleName(),
+            contextFreeBase.hasAsyncAlternative(), newCompletionString,
+            contextFreeBase.getModuleName(),
             contextFreeBase.getBriefDocComment(),
             contextFreeBase.getAssociatedUSRs(),
             contextFreeBase.getResultType(),