[SourceKit] Add a dedicated request to retrieve the diagnostics of a file

Currently, SourceKit always implicitly sends diagnostics to the client after every edit. This doesn’t match our new cancellation story because diagnostics retrieval is no request and thus can’t be cancelled.

Instead, diagnostics retrieval should be a standalone request, which returns a cancellation token like every other request and which can thus also be cancelled as such.

The indented transition path here is to change all open and edit requests to be syntactic only. When diagnostics are needed, the new diagnostics request should be used.

rdar://83391522
This commit is contained in:
Alex Hoppen
2021-10-05 19:02:08 +02:00
parent 3bd6716d04
commit 2f19d1847f
10 changed files with 169 additions and 2 deletions

View File

@@ -0,0 +1,49 @@
// Use -print-raw-response on the first request so we don't wait for semantic info which will never come.
// RUN: %sourcekitd-test -req=open %s -req-opts=syntactic_only=1 -print-raw-response -- %s == \
// RUN: -req=stats == \
// RUN: -req=diags %s -print-raw-response -- %s == \
// RUN: -req=stats == \
// RUN: -req=diags %s -print-raw-response -- %s == \
// RUN: -req=stats | %FileCheck %s
func foo(y: String) {
foo(y: 1)
}
// We shouldn't build an AST for the syntactic open
// CHECK: 0 {{.*}} source.statistic.num-ast-builds
// Retrieving diagnostics should workd
// CHECK: {
// CHECK: key.diagnostics: [
// CHECK: {
// CHECK: key.line: 10,
// CHECK: key.column: 10,
// CHECK: key.filepath: "{{.*}}",
// CHECK: key.severity: source.diagnostic.severity.error,
// CHECK: key.id: "cannot_convert_argument_value",
// CHECK: key.description: "cannot convert value of type 'Int' to expected argument type 'String'"
// CHECK: }
// CHECK: ]
// CHECK: }
// ... and we should have built an AST for it
// CHECK: 1 {{.*}} source.statistic.num-ast-builds
// Retrieving diagnostics again should workd
// CHECK: {
// CHECK: key.diagnostics: [
// CHECK: {
// CHECK: key.line: 10,
// CHECK: key.column: 10,
// CHECK: key.filepath: "{{.*}}",
// CHECK: key.severity: source.diagnostic.severity.error,
// CHECK: key.id: "cannot_convert_argument_value",
// CHECK: key.description: "cannot convert value of type 'Int' to expected argument type 'String'"
// CHECK: }
// CHECK: ]
// CHECK: }
// ... but we shouldn't rebuild an AST
// CHECK: 1 {{.*}} source.statistic.num-ast-builds
// CHECK: 1 {{.*}} source.statistic.num-ast-cache-hits

View File

@@ -500,6 +500,9 @@ struct CursorInfoData {
llvm::ArrayRef<RefactoringInfo> AvailableActions;
};
/// The result type of `LangSupport::getDiagnostics`
typedef ArrayRef<DiagnosticEntryInfo> DiagnosticsResult;
struct RangeInfo {
UIdent RangeKind;
StringRef ExprType;
@@ -832,6 +835,13 @@ public:
SourceKitCancellationToken CancellationToken,
std::function<void(const RequestResult<CursorInfoData> &)> Receiver) = 0;
virtual void
getDiagnostics(StringRef InputFile, ArrayRef<const char *> Args,
Optional<VFSOptions> VfsOptions,
SourceKitCancellationToken CancellationToken,
std::function<void(const RequestResult<DiagnosticsResult> &)>
Receiver) = 0;
virtual void
getNameInfo(StringRef Filename, unsigned Offset, NameTranslatingInfo &Input,
ArrayRef<const char *> Args,

View File

@@ -13,6 +13,7 @@
#ifndef LLVM_SOURCEKIT_SUPPORT_FILESYSTEMPROVIDER_H
#define LLVM_SOURCEKIT_SUPPORT_FILESYSTEMPROVIDER_H
#include "SourceKit/Core/LangSupport.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"

View File

@@ -569,6 +569,13 @@ public:
std::function<void(const RequestResult<CursorInfoData> &)>
Receiver) override;
void
getDiagnostics(StringRef InputFile, ArrayRef<const char *> Args,
Optional<VFSOptions> VfsOptions,
SourceKitCancellationToken CancellationToken,
std::function<void(const RequestResult<DiagnosticsResult> &)>
Receiver) override;
void getNameInfo(
StringRef Filename, unsigned Offset, NameTranslatingInfo &Input,
ArrayRef<const char *> Args, SourceKitCancellationToken CancellationToken,

View File

@@ -10,12 +10,13 @@
//
//===----------------------------------------------------------------------===//
#include "SwiftASTManager.h"
#include "SwiftLangSupport.h"
#include "SourceKit/Support/FileSystemProvider.h"
#include "SourceKit/Support/ImmutableTextBuffer.h"
#include "SourceKit/Support/Logging.h"
#include "SourceKit/Support/UIdent.h"
#include "SwiftASTManager.h"
#include "SwiftEditorDiagConsumer.h"
#include "SwiftLangSupport.h"
#include "swift/AST/ASTDemangler.h"
#include "swift/AST/ASTPrinter.h"
@@ -1588,6 +1589,36 @@ static void resolveCursor(
CancellationToken, fileSystem);
}
static void computeDiagnostics(
SwiftLangSupport &Lang, StringRef InputFile, SwiftInvocationRef Invok,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
SourceKitCancellationToken CancellationToken,
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver) {
class DiagnosticsConsumer : public SwiftASTConsumer {
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver;
public:
DiagnosticsConsumer(
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver)
: Receiver(Receiver) {}
void handlePrimaryAST(ASTUnitRef AstUnit) override {
unsigned BufferID =
AstUnit->getPrimarySourceFile().getBufferID().getValue();
auto &DiagConsumer = AstUnit->getEditorDiagConsumer();
auto Diagnostics = DiagConsumer.getDiagnosticsForBuffer(BufferID);
Receiver(RequestResult<DiagnosticsResult>::fromResult(Diagnostics));
}
};
auto Consumer = std::make_shared<DiagnosticsConsumer>(std::move(Receiver));
Lang.getASTManager()->processASTAsync(Invok, std::move(Consumer),
/*OncePerASTToken=*/nullptr,
CancellationToken, FileSystem);
}
static void resolveName(
SwiftLangSupport &Lang, StringRef InputFile, unsigned Offset,
SwiftInvocationRef Invok, bool TryExistingAST, NameTranslatingInfo &Input,
@@ -1846,6 +1877,33 @@ void SwiftLangSupport::getCursorInfo(
fileSystem, CancellationToken, Receiver);
}
void SwiftLangSupport::getDiagnostics(
StringRef InputFile, ArrayRef<const char *> Args,
Optional<VFSOptions> VfsOptions,
SourceKitCancellationToken CancellationToken,
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver) {
std::string FileSystemError;
auto FileSystem = getFileSystem(VfsOptions, InputFile, FileSystemError);
if (!FileSystem) {
Receiver(RequestResult<DiagnosticsResult>::fromError(FileSystemError));
return;
}
std::string InvocationError;
SwiftInvocationRef Invok =
ASTMgr->getInvocation(Args, InputFile, FileSystem, InvocationError);
if (!InvocationError.empty()) {
LOG_WARN_FUNC("error creating ASTInvocation: " << InvocationError);
}
if (!Invok) {
Receiver(RequestResult<DiagnosticsResult>::fromError(InvocationError));
return;
}
computeDiagnostics(*this, InputFile, Invok, FileSystem, CancellationToken,
Receiver);
}
void SwiftLangSupport::getRangeInfo(
StringRef InputFile, unsigned Offset, unsigned Length,
bool CancelOnSubsequentRequest, ArrayRef<const char *> Args,

View File

@@ -150,6 +150,7 @@ bool TestOptions::parseArgs(llvm::ArrayRef<const char *> Args) {
.Case("collect-var-type", SourceKitRequest::CollectVariableType)
.Case("global-config", SourceKitRequest::GlobalConfiguration)
.Case("dependency-updated", SourceKitRequest::DependencyUpdated)
.Case("diags", SourceKitRequest::Diagnostics)
#define SEMANTIC_REFACTORING(KIND, NAME, ID) .Case("refactoring." #ID, SourceKitRequest::KIND)
#include "swift/IDE/RefactoringKinds.def"
.Default(SourceKitRequest::None);

View File

@@ -67,6 +67,7 @@ enum class SourceKitRequest {
CollectVariableType,
GlobalConfiguration,
DependencyUpdated,
Diagnostics,
#define SEMANTIC_REFACTORING(KIND, NAME, ID) KIND,
#include "swift/IDE/RefactoringKinds.def"
};

View File

@@ -1075,6 +1075,9 @@ static int handleTestInvocation(TestOptions Opts, TestOptions &InitOpts) {
sourcekitd_request_dictionary_set_uid(Req, KeyRequest,
RequestDependencyUpdated);
break;
case SourceKitRequest::Diagnostics:
sourcekitd_request_dictionary_set_uid(Req, KeyRequest, RequestDiagnostics);
break;
}
if (!SourceFile.empty()) {
@@ -1305,6 +1308,7 @@ static bool handleResponse(sourcekitd_response_t Resp, const TestOptions &Opts,
case SourceKitRequest::TypeContextInfo:
case SourceKitRequest::ConformingMethodList:
case SourceKitRequest::DependencyUpdated:
case SourceKitRequest::Diagnostics:
sourcekitd_response_description_dump_filedesc(Resp, STDOUT_FILENO);
break;
@@ -1465,6 +1469,7 @@ static bool handleResponse(sourcekitd_response_t Resp, const TestOptions &Opts,
break;
case SourceKitRequest::Statistics:
printStatistics(Info, llvm::outs());
break;
}
}

View File

@@ -193,6 +193,9 @@ static sourcekitd_response_t reportDocInfo(llvm::MemoryBuffer *InputBuf,
static void reportCursorInfo(const RequestResult<CursorInfoData> &Result, ResponseReceiver Rec);
static void reportDiagnostics(const RequestResult<DiagnosticsResult> &Result,
ResponseReceiver Rec);
static void reportExpressionTypeInfo(const RequestResult<ExpressionTypesInFile> &Result,
ResponseReceiver Rec);
@@ -1277,6 +1280,16 @@ static void handleSemanticRequest(
Args, CancellationToken, Rec);
}
if (ReqUID == RequestDiagnostics) {
LangSupport &Lang = getGlobalContext().getSwiftLangSupport();
Lang.getDiagnostics(*SourceFile, Args, std::move(vfsOptions),
CancellationToken,
[Rec](const RequestResult<DiagnosticsResult> &Result) {
reportDiagnostics(Result, Rec);
});
return;
}
{
llvm::raw_svector_ostream OSErr(ErrBuf);
OSErr << "unknown request: " << UIdentFromSKDUID(ReqUID).getName();
@@ -1948,6 +1961,27 @@ static void reportCursorInfo(const RequestResult<CursorInfoData> &Result,
return Rec(RespBuilder.createResponse());
}
//===----------------------------------------------------------------------===//
// ReportDiagnostics
//===----------------------------------------------------------------------===//
static void reportDiagnostics(const RequestResult<DiagnosticsResult> &Result,
ResponseReceiver Rec) {
if (Result.isCancelled())
return Rec(createErrorRequestCancelled());
if (Result.isError())
return Rec(createErrorRequestFailed(Result.getError()));
auto &DiagResults = Result.value();
ResponseBuilder RespBuilder;
auto Dict = RespBuilder.getDictionary();
auto DiagArray = Dict.setArray(KeyDiagnostics);
for (const auto &DiagInfo : DiagResults)
fillDictionaryForDiagnosticInfo(DiagArray.appendDictionary(), DiagInfo);
Rec(RespBuilder.createResponse());
}
//===----------------------------------------------------------------------===//
// ReportRangeInfo
//===----------------------------------------------------------------------===//

View File

@@ -262,6 +262,7 @@ UID_REQUESTS = [
REQUEST('CollectVariableType', 'source.request.variable.type'),
REQUEST('GlobalConfiguration', 'source.request.configuration.global'),
REQUEST('DependencyUpdated', 'source.request.dependency_updated'),
REQUEST('Diagnostics', 'source.request.diagnostics'),
]