From 2f19d1847f5ff81ee45519ffd472febd35f11704 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Tue, 5 Oct 2021 19:02:08 +0200 Subject: [PATCH] [SourceKit] Add a dedicated request to retrieve the diagnostics of a file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- test/SourceKit/Diagnostics/diags.swift | 49 +++++++++++++++ .../include/SourceKit/Core/LangSupport.h | 10 +++ .../SourceKit/Support/FileSystemProvider.h | 1 + .../lib/SwiftLang/SwiftLangSupport.h | 7 +++ .../lib/SwiftLang/SwiftSourceDocInfo.cpp | 62 ++++++++++++++++++- .../tools/sourcekitd-test/TestOptions.cpp | 1 + .../tools/sourcekitd-test/TestOptions.h | 1 + .../tools/sourcekitd-test/sourcekitd-test.cpp | 5 ++ .../tools/sourcekitd/lib/API/Requests.cpp | 34 ++++++++++ utils/gyb_sourcekit_support/UIDs.py | 1 + 10 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 test/SourceKit/Diagnostics/diags.swift diff --git a/test/SourceKit/Diagnostics/diags.swift b/test/SourceKit/Diagnostics/diags.swift new file mode 100644 index 00000000000..9769138f797 --- /dev/null +++ b/test/SourceKit/Diagnostics/diags.swift @@ -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 diff --git a/tools/SourceKit/include/SourceKit/Core/LangSupport.h b/tools/SourceKit/include/SourceKit/Core/LangSupport.h index 67cb7f3afa9..0a7d777b1e5 100644 --- a/tools/SourceKit/include/SourceKit/Core/LangSupport.h +++ b/tools/SourceKit/include/SourceKit/Core/LangSupport.h @@ -500,6 +500,9 @@ struct CursorInfoData { llvm::ArrayRef AvailableActions; }; +/// The result type of `LangSupport::getDiagnostics` +typedef ArrayRef DiagnosticsResult; + struct RangeInfo { UIdent RangeKind; StringRef ExprType; @@ -832,6 +835,13 @@ public: SourceKitCancellationToken CancellationToken, std::function &)> Receiver) = 0; + virtual void + getDiagnostics(StringRef InputFile, ArrayRef Args, + Optional VfsOptions, + SourceKitCancellationToken CancellationToken, + std::function &)> + Receiver) = 0; + virtual void getNameInfo(StringRef Filename, unsigned Offset, NameTranslatingInfo &Input, ArrayRef Args, diff --git a/tools/SourceKit/include/SourceKit/Support/FileSystemProvider.h b/tools/SourceKit/include/SourceKit/Support/FileSystemProvider.h index e84c19b7e22..91363a84566 100644 --- a/tools/SourceKit/include/SourceKit/Support/FileSystemProvider.h +++ b/tools/SourceKit/include/SourceKit/Support/FileSystemProvider.h @@ -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" diff --git a/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h b/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h index 3f523d44169..8a7b4ebd854 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h +++ b/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h @@ -569,6 +569,13 @@ public: std::function &)> Receiver) override; + void + getDiagnostics(StringRef InputFile, ArrayRef Args, + Optional VfsOptions, + SourceKitCancellationToken CancellationToken, + std::function &)> + Receiver) override; + void getNameInfo( StringRef Filename, unsigned Offset, NameTranslatingInfo &Input, ArrayRef Args, SourceKitCancellationToken CancellationToken, diff --git a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp index 6cf1585d6ea..99f8eceeedd 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp @@ -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 FileSystem, + SourceKitCancellationToken CancellationToken, + std::function &)> Receiver) { + + class DiagnosticsConsumer : public SwiftASTConsumer { + std::function &)> Receiver; + + public: + DiagnosticsConsumer( + std::function &)> 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::fromResult(Diagnostics)); + } + }; + + auto Consumer = std::make_shared(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 Args, + Optional VfsOptions, + SourceKitCancellationToken CancellationToken, + std::function &)> Receiver) { + std::string FileSystemError; + auto FileSystem = getFileSystem(VfsOptions, InputFile, FileSystemError); + if (!FileSystem) { + Receiver(RequestResult::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::fromError(InvocationError)); + return; + } + + computeDiagnostics(*this, InputFile, Invok, FileSystem, CancellationToken, + Receiver); +} + void SwiftLangSupport::getRangeInfo( StringRef InputFile, unsigned Offset, unsigned Length, bool CancelOnSubsequentRequest, ArrayRef Args, diff --git a/tools/SourceKit/tools/sourcekitd-test/TestOptions.cpp b/tools/SourceKit/tools/sourcekitd-test/TestOptions.cpp index 07865a35c49..765c8b9a44b 100644 --- a/tools/SourceKit/tools/sourcekitd-test/TestOptions.cpp +++ b/tools/SourceKit/tools/sourcekitd-test/TestOptions.cpp @@ -150,6 +150,7 @@ bool TestOptions::parseArgs(llvm::ArrayRef 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); diff --git a/tools/SourceKit/tools/sourcekitd-test/TestOptions.h b/tools/SourceKit/tools/sourcekitd-test/TestOptions.h index 9779585eceb..c651d1b7973 100644 --- a/tools/SourceKit/tools/sourcekitd-test/TestOptions.h +++ b/tools/SourceKit/tools/sourcekitd-test/TestOptions.h @@ -67,6 +67,7 @@ enum class SourceKitRequest { CollectVariableType, GlobalConfiguration, DependencyUpdated, + Diagnostics, #define SEMANTIC_REFACTORING(KIND, NAME, ID) KIND, #include "swift/IDE/RefactoringKinds.def" }; diff --git a/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp b/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp index dc2d45e6251..f9a0e556263 100644 --- a/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp +++ b/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp @@ -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; } } diff --git a/tools/SourceKit/tools/sourcekitd/lib/API/Requests.cpp b/tools/SourceKit/tools/sourcekitd/lib/API/Requests.cpp index 75b3212771c..ed412a0e674 100644 --- a/tools/SourceKit/tools/sourcekitd/lib/API/Requests.cpp +++ b/tools/SourceKit/tools/sourcekitd/lib/API/Requests.cpp @@ -193,6 +193,9 @@ static sourcekitd_response_t reportDocInfo(llvm::MemoryBuffer *InputBuf, static void reportCursorInfo(const RequestResult &Result, ResponseReceiver Rec); +static void reportDiagnostics(const RequestResult &Result, + ResponseReceiver Rec); + static void reportExpressionTypeInfo(const RequestResult &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 &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 &Result, return Rec(RespBuilder.createResponse()); } +//===----------------------------------------------------------------------===// +// ReportDiagnostics +//===----------------------------------------------------------------------===// + +static void reportDiagnostics(const RequestResult &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 //===----------------------------------------------------------------------===// diff --git a/utils/gyb_sourcekit_support/UIDs.py b/utils/gyb_sourcekit_support/UIDs.py index 864ad09b40b..01de03b07db 100644 --- a/utils/gyb_sourcekit_support/UIDs.py +++ b/utils/gyb_sourcekit_support/UIDs.py @@ -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'), ]