Merge pull request #59000 from ahoppen/pr/dont-recommend-func-with-async-alternative

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(),