Skip to content

Commit 2f19d18

Browse files
committed
[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
1 parent 3bd6716 commit 2f19d18

File tree

10 files changed

+169
-2
lines changed

10 files changed

+169
-2
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Use -print-raw-response on the first request so we don't wait for semantic info which will never come.
2+
// RUN: %sourcekitd-test -req=open %s -req-opts=syntactic_only=1 -print-raw-response -- %s == \
3+
// RUN: -req=stats == \
4+
// RUN: -req=diags %s -print-raw-response -- %s == \
5+
// RUN: -req=stats == \
6+
// RUN: -req=diags %s -print-raw-response -- %s == \
7+
// RUN: -req=stats | %FileCheck %s
8+
9+
func foo(y: String) {
10+
foo(y: 1)
11+
}
12+
13+
// We shouldn't build an AST for the syntactic open
14+
// CHECK: 0 {{.*}} source.statistic.num-ast-builds
15+
16+
// Retrieving diagnostics should workd
17+
// CHECK: {
18+
// CHECK: key.diagnostics: [
19+
// CHECK: {
20+
// CHECK: key.line: 10,
21+
// CHECK: key.column: 10,
22+
// CHECK: key.filepath: "{{.*}}",
23+
// CHECK: key.severity: source.diagnostic.severity.error,
24+
// CHECK: key.id: "cannot_convert_argument_value",
25+
// CHECK: key.description: "cannot convert value of type 'Int' to expected argument type 'String'"
26+
// CHECK: }
27+
// CHECK: ]
28+
// CHECK: }
29+
30+
// ... and we should have built an AST for it
31+
// CHECK: 1 {{.*}} source.statistic.num-ast-builds
32+
33+
// Retrieving diagnostics again should workd
34+
// CHECK: {
35+
// CHECK: key.diagnostics: [
36+
// CHECK: {
37+
// CHECK: key.line: 10,
38+
// CHECK: key.column: 10,
39+
// CHECK: key.filepath: "{{.*}}",
40+
// CHECK: key.severity: source.diagnostic.severity.error,
41+
// CHECK: key.id: "cannot_convert_argument_value",
42+
// CHECK: key.description: "cannot convert value of type 'Int' to expected argument type 'String'"
43+
// CHECK: }
44+
// CHECK: ]
45+
// CHECK: }
46+
47+
// ... but we shouldn't rebuild an AST
48+
// CHECK: 1 {{.*}} source.statistic.num-ast-builds
49+
// CHECK: 1 {{.*}} source.statistic.num-ast-cache-hits

tools/SourceKit/include/SourceKit/Core/LangSupport.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,9 @@ struct CursorInfoData {
500500
llvm::ArrayRef<RefactoringInfo> AvailableActions;
501501
};
502502

503+
/// The result type of `LangSupport::getDiagnostics`
504+
typedef ArrayRef<DiagnosticEntryInfo> DiagnosticsResult;
505+
503506
struct RangeInfo {
504507
UIdent RangeKind;
505508
StringRef ExprType;
@@ -832,6 +835,13 @@ class LangSupport {
832835
SourceKitCancellationToken CancellationToken,
833836
std::function<void(const RequestResult<CursorInfoData> &)> Receiver) = 0;
834837

838+
virtual void
839+
getDiagnostics(StringRef InputFile, ArrayRef<const char *> Args,
840+
Optional<VFSOptions> VfsOptions,
841+
SourceKitCancellationToken CancellationToken,
842+
std::function<void(const RequestResult<DiagnosticsResult> &)>
843+
Receiver) = 0;
844+
835845
virtual void
836846
getNameInfo(StringRef Filename, unsigned Offset, NameTranslatingInfo &Input,
837847
ArrayRef<const char *> Args,

tools/SourceKit/include/SourceKit/Support/FileSystemProvider.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef LLVM_SOURCEKIT_SUPPORT_FILESYSTEMPROVIDER_H
1414
#define LLVM_SOURCEKIT_SUPPORT_FILESYSTEMPROVIDER_H
1515

16+
#include "SourceKit/Core/LangSupport.h"
1617
#include "llvm/ADT/IntrusiveRefCntPtr.h"
1718
#include "llvm/ADT/SmallVector.h"
1819
#include "llvm/ADT/StringMap.h"

tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,13 @@ class SwiftLangSupport : public LangSupport {
569569
std::function<void(const RequestResult<CursorInfoData> &)>
570570
Receiver) override;
571571

572+
void
573+
getDiagnostics(StringRef InputFile, ArrayRef<const char *> Args,
574+
Optional<VFSOptions> VfsOptions,
575+
SourceKitCancellationToken CancellationToken,
576+
std::function<void(const RequestResult<DiagnosticsResult> &)>
577+
Receiver) override;
578+
572579
void getNameInfo(
573580
StringRef Filename, unsigned Offset, NameTranslatingInfo &Input,
574581
ArrayRef<const char *> Args, SourceKitCancellationToken CancellationToken,

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
#include "SwiftASTManager.h"
14-
#include "SwiftLangSupport.h"
1513
#include "SourceKit/Support/FileSystemProvider.h"
1614
#include "SourceKit/Support/ImmutableTextBuffer.h"
1715
#include "SourceKit/Support/Logging.h"
1816
#include "SourceKit/Support/UIdent.h"
17+
#include "SwiftASTManager.h"
18+
#include "SwiftEditorDiagConsumer.h"
19+
#include "SwiftLangSupport.h"
1920

2021
#include "swift/AST/ASTDemangler.h"
2122
#include "swift/AST/ASTPrinter.h"
@@ -1588,6 +1589,36 @@ static void resolveCursor(
15881589
CancellationToken, fileSystem);
15891590
}
15901591

1592+
static void computeDiagnostics(
1593+
SwiftLangSupport &Lang, StringRef InputFile, SwiftInvocationRef Invok,
1594+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
1595+
SourceKitCancellationToken CancellationToken,
1596+
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver) {
1597+
1598+
class DiagnosticsConsumer : public SwiftASTConsumer {
1599+
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver;
1600+
1601+
public:
1602+
DiagnosticsConsumer(
1603+
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver)
1604+
: Receiver(Receiver) {}
1605+
1606+
void handlePrimaryAST(ASTUnitRef AstUnit) override {
1607+
unsigned BufferID =
1608+
AstUnit->getPrimarySourceFile().getBufferID().getValue();
1609+
auto &DiagConsumer = AstUnit->getEditorDiagConsumer();
1610+
auto Diagnostics = DiagConsumer.getDiagnosticsForBuffer(BufferID);
1611+
Receiver(RequestResult<DiagnosticsResult>::fromResult(Diagnostics));
1612+
}
1613+
};
1614+
1615+
auto Consumer = std::make_shared<DiagnosticsConsumer>(std::move(Receiver));
1616+
1617+
Lang.getASTManager()->processASTAsync(Invok, std::move(Consumer),
1618+
/*OncePerASTToken=*/nullptr,
1619+
CancellationToken, FileSystem);
1620+
}
1621+
15911622
static void resolveName(
15921623
SwiftLangSupport &Lang, StringRef InputFile, unsigned Offset,
15931624
SwiftInvocationRef Invok, bool TryExistingAST, NameTranslatingInfo &Input,
@@ -1846,6 +1877,33 @@ void SwiftLangSupport::getCursorInfo(
18461877
fileSystem, CancellationToken, Receiver);
18471878
}
18481879

1880+
void SwiftLangSupport::getDiagnostics(
1881+
StringRef InputFile, ArrayRef<const char *> Args,
1882+
Optional<VFSOptions> VfsOptions,
1883+
SourceKitCancellationToken CancellationToken,
1884+
std::function<void(const RequestResult<DiagnosticsResult> &)> Receiver) {
1885+
std::string FileSystemError;
1886+
auto FileSystem = getFileSystem(VfsOptions, InputFile, FileSystemError);
1887+
if (!FileSystem) {
1888+
Receiver(RequestResult<DiagnosticsResult>::fromError(FileSystemError));
1889+
return;
1890+
}
1891+
1892+
std::string InvocationError;
1893+
SwiftInvocationRef Invok =
1894+
ASTMgr->getInvocation(Args, InputFile, FileSystem, InvocationError);
1895+
if (!InvocationError.empty()) {
1896+
LOG_WARN_FUNC("error creating ASTInvocation: " << InvocationError);
1897+
}
1898+
if (!Invok) {
1899+
Receiver(RequestResult<DiagnosticsResult>::fromError(InvocationError));
1900+
return;
1901+
}
1902+
1903+
computeDiagnostics(*this, InputFile, Invok, FileSystem, CancellationToken,
1904+
Receiver);
1905+
}
1906+
18491907
void SwiftLangSupport::getRangeInfo(
18501908
StringRef InputFile, unsigned Offset, unsigned Length,
18511909
bool CancelOnSubsequentRequest, ArrayRef<const char *> Args,

tools/SourceKit/tools/sourcekitd-test/TestOptions.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ bool TestOptions::parseArgs(llvm::ArrayRef<const char *> Args) {
150150
.Case("collect-var-type", SourceKitRequest::CollectVariableType)
151151
.Case("global-config", SourceKitRequest::GlobalConfiguration)
152152
.Case("dependency-updated", SourceKitRequest::DependencyUpdated)
153+
.Case("diags", SourceKitRequest::Diagnostics)
153154
#define SEMANTIC_REFACTORING(KIND, NAME, ID) .Case("refactoring." #ID, SourceKitRequest::KIND)
154155
#include "swift/IDE/RefactoringKinds.def"
155156
.Default(SourceKitRequest::None);

tools/SourceKit/tools/sourcekitd-test/TestOptions.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ enum class SourceKitRequest {
6767
CollectVariableType,
6868
GlobalConfiguration,
6969
DependencyUpdated,
70+
Diagnostics,
7071
#define SEMANTIC_REFACTORING(KIND, NAME, ID) KIND,
7172
#include "swift/IDE/RefactoringKinds.def"
7273
};

tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,6 +1075,9 @@ static int handleTestInvocation(TestOptions Opts, TestOptions &InitOpts) {
10751075
sourcekitd_request_dictionary_set_uid(Req, KeyRequest,
10761076
RequestDependencyUpdated);
10771077
break;
1078+
case SourceKitRequest::Diagnostics:
1079+
sourcekitd_request_dictionary_set_uid(Req, KeyRequest, RequestDiagnostics);
1080+
break;
10781081
}
10791082

10801083
if (!SourceFile.empty()) {
@@ -1305,6 +1308,7 @@ static bool handleResponse(sourcekitd_response_t Resp, const TestOptions &Opts,
13051308
case SourceKitRequest::TypeContextInfo:
13061309
case SourceKitRequest::ConformingMethodList:
13071310
case SourceKitRequest::DependencyUpdated:
1311+
case SourceKitRequest::Diagnostics:
13081312
sourcekitd_response_description_dump_filedesc(Resp, STDOUT_FILENO);
13091313
break;
13101314

@@ -1465,6 +1469,7 @@ static bool handleResponse(sourcekitd_response_t Resp, const TestOptions &Opts,
14651469
break;
14661470
case SourceKitRequest::Statistics:
14671471
printStatistics(Info, llvm::outs());
1472+
break;
14681473
}
14691474
}
14701475

tools/SourceKit/tools/sourcekitd/lib/API/Requests.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,9 @@ static sourcekitd_response_t reportDocInfo(llvm::MemoryBuffer *InputBuf,
193193

194194
static void reportCursorInfo(const RequestResult<CursorInfoData> &Result, ResponseReceiver Rec);
195195

196+
static void reportDiagnostics(const RequestResult<DiagnosticsResult> &Result,
197+
ResponseReceiver Rec);
198+
196199
static void reportExpressionTypeInfo(const RequestResult<ExpressionTypesInFile> &Result,
197200
ResponseReceiver Rec);
198201

@@ -1277,6 +1280,16 @@ static void handleSemanticRequest(
12771280
Args, CancellationToken, Rec);
12781281
}
12791282

1283+
if (ReqUID == RequestDiagnostics) {
1284+
LangSupport &Lang = getGlobalContext().getSwiftLangSupport();
1285+
Lang.getDiagnostics(*SourceFile, Args, std::move(vfsOptions),
1286+
CancellationToken,
1287+
[Rec](const RequestResult<DiagnosticsResult> &Result) {
1288+
reportDiagnostics(Result, Rec);
1289+
});
1290+
return;
1291+
}
1292+
12801293
{
12811294
llvm::raw_svector_ostream OSErr(ErrBuf);
12821295
OSErr << "unknown request: " << UIdentFromSKDUID(ReqUID).getName();
@@ -1948,6 +1961,27 @@ static void reportCursorInfo(const RequestResult<CursorInfoData> &Result,
19481961
return Rec(RespBuilder.createResponse());
19491962
}
19501963

1964+
//===----------------------------------------------------------------------===//
1965+
// ReportDiagnostics
1966+
//===----------------------------------------------------------------------===//
1967+
1968+
static void reportDiagnostics(const RequestResult<DiagnosticsResult> &Result,
1969+
ResponseReceiver Rec) {
1970+
if (Result.isCancelled())
1971+
return Rec(createErrorRequestCancelled());
1972+
if (Result.isError())
1973+
return Rec(createErrorRequestFailed(Result.getError()));
1974+
1975+
auto &DiagResults = Result.value();
1976+
1977+
ResponseBuilder RespBuilder;
1978+
auto Dict = RespBuilder.getDictionary();
1979+
auto DiagArray = Dict.setArray(KeyDiagnostics);
1980+
for (const auto &DiagInfo : DiagResults)
1981+
fillDictionaryForDiagnosticInfo(DiagArray.appendDictionary(), DiagInfo);
1982+
Rec(RespBuilder.createResponse());
1983+
}
1984+
19511985
//===----------------------------------------------------------------------===//
19521986
// ReportRangeInfo
19531987
//===----------------------------------------------------------------------===//

utils/gyb_sourcekit_support/UIDs.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ def __init__(self, internal_name, external_name):
262262
REQUEST('CollectVariableType', 'source.request.variable.type'),
263263
REQUEST('GlobalConfiguration', 'source.request.configuration.global'),
264264
REQUEST('DependencyUpdated', 'source.request.dependency_updated'),
265+
REQUEST('Diagnostics', 'source.request.diagnostics'),
265266
]
266267

267268

0 commit comments

Comments
 (0)