Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsIDE.def
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
49 changes: 32 additions & 17 deletions include/swift/IDE/CodeCompletionResult.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -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:
Expand Down
9 changes: 6 additions & 3 deletions lib/IDE/CodeCompletionCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions lib/IDE/CodeCompletionDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
18 changes: 12 additions & 6 deletions lib/IDE/CodeCompletionResult.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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=*/"",
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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));
}
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/CodeCompletionResultBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions lib/IDE/CodeCompletionResultBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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; }
Expand Down
3 changes: 3 additions & 0 deletions lib/IDE/CompletionLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions test/IDE/complete_async_imported.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
3 changes: 2 additions & 1 deletion tools/SourceKit/lib/SwiftLang/CodeCompletionOrganizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down