Skip to content

[4.2] [sourcekitd] Capture diagnostics with invalid locations for compile notifications #15804

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.

Sign up for GitHub

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
merged 2 commits into from
Apr 6, 2018
Merged

[4.2] [sourcekitd] Capture diagnostics with invalid locations for compile notifications #15804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3deb675
[sourcekitd] Remove dead method for checking invalid loc diagnostics
benlangmuir Apr 5, 2018
4ec84c1
[sourcekitd] Capture diagnostics with invalid locations for compile n…
benlangmuir Apr 5, 2018
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
21 changes: 21 additions & 0 deletions test/SourceKit/CompileNotifications/diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,28 @@
// CLANG_IMPORTER-NEXT: key.severity: source.diagnostic.severity.error,
// CLANG_IMPORTER-NEXT: key.description: {{.*}}not found

// RUN: %sourcekitd-test -req=track-compiles == -req=sema %s -- %s -Xcc -ivfsoverlay -Xcc /doesnotexist | %FileCheck %s -check-prefix=CLANG_IMPORTER_UNKNOWN
// CLANG_IMPORTER_UNKNOWN: key.notification: source.notification.compile-did-finish,
// CLANG_IMPORTER_UNKNOWN-NEXT: key.diagnostics: [
// CLANG_IMPORTER_UNKNOWN-NEXT: {
// CLANG_IMPORTER_UNKNOWN-NEXT: key.filepath: "<unknown>"
// CLANG_IMPORTER_UNKNOWN-NEXT: key.severity: source.diagnostic.severity.error,
// CLANG_IMPORTER_UNKNOWN-NEXT: key.offset: 0
// CLANG_IMPORTER_UNKNOWN-NEXT: key.description: "virtual filesystem{{.*}}not found

// Note: we're missing the "compiler is in code completion mode" diagnostic,
// which is probably just as well.
// RUN: %sourcekitd-test -req=track-compiles == -req=complete -offset=0 %s -- %s | %FileCheck %s -check-prefix=NODIAGS
// RUN: %sourcekitd-test -req=track-compiles == -req=complete -pos=2:1 %S/Inputs/sema-error.swift -- %S/Inputs/sema-error.swift | %FileCheck %s -check-prefix=SEMA

// FIXME: invalid arguments cause us to early-exit and not send the notifications
// RUN_DISABLED: %sourcekitd-test -req=track-compiles == -req=sema %s -- %s -invalid-arg | %FileCheck %s -check-prefix=INVALID_ARG

// RUN: %sourcekitd-test -req=track-compiles == -req=sema %s -- %s -Xcc -invalid-arg | %FileCheck %s -check-prefix=INVALID_ARG_CLANG
// INVALID_ARG_CLANG: key.notification: source.notification.compile-did-finish,
// INVALID_ARG_CLANG-NEXT: key.diagnostics: [
// INVALID_ARG_CLANG-NEXT: {
// INVALID_ARG_CLANG-NEXT: key.filepath: "<unknown>"
// INVALID_ARG_CLANG-NEXT: key.severity: source.diagnostic.severity.warning,
// INVALID_ARG_CLANG-NEXT: key.offset: 0
// INVALID_ARG_CLANG-NEXT: key.description: "argument unused
77 changes: 46 additions & 31 deletions tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "swift/AST/ASTWalker.h"
#include "swift/AST/DiagnosticsClangImporter.h"
#include "swift/AST/DiagnosticsParse.h"
#include "swift/AST/DiagnosticsFrontend.h"
#include "swift/Basic/SourceManager.h"
#include "swift/Demangling/ManglingUtils.h"
#include "swift/Frontend/Frontend.h"
Expand Down Expand Up @@ -59,6 +60,9 @@ static std::vector<unsigned> getSortedBufferIDs(

void EditorDiagConsumer::getAllDiagnostics(
SmallVectorImpl<DiagnosticEntryInfo> &Result) {

Result.append(InvalidLocDiagnostics.begin(), InvalidLocDiagnostics.end());

// Note: we cannot reuse InputBufIds because there may be diagnostics outside
// the inputs. Instead, sort the extant buffers.
auto bufferIDs = getSortedBufferIDs(BufferDiagnostics);
Expand All @@ -77,16 +81,11 @@ void EditorDiagConsumer::handleDiagnostic(
HadAnyError = true;
}

// Filter out lexer errors for placeholders.
if (Info.ID == diag::lex_editor_placeholder.ID)
// Filter out benign diagnostics for editing.
if (Info.ID == diag::lex_editor_placeholder.ID ||
Info.ID == diag::error_doing_code_completion.ID)
return;

if (Loc.isInvalid()) {
if (Kind == DiagnosticKind::Error)
HadInvalidLocError = true;
clearLastDiag();
return;
}
bool IsNote = (Kind == DiagnosticKind::Note);

if (IsNote && !haveLastDiag())
Expand All @@ -109,9 +108,12 @@ void EditorDiagConsumer::handleDiagnostic(
}
SKInfo.Description = Text.str();

unsigned BufferID = SM.findBufferContainingLoc(Loc);
Optional<unsigned> BufferIDOpt;
if (Loc.isValid()) {
BufferIDOpt = SM.findBufferContainingLoc(Loc);
}

if (!isInputBufferID(BufferID)) {
if (BufferIDOpt && !isInputBufferID(*BufferIDOpt)) {
if (Info.ID == diag::error_from_clang.ID ||
Info.ID == diag::warning_from_clang.ID ||
Info.ID == diag::note_from_clang.ID ||
Expand All @@ -127,7 +129,7 @@ void EditorDiagConsumer::handleDiagnostic(
// buffer identifier and append it to the diagnostic message.
auto &LastDiag = getLastDiag();
SKInfo.Description += " (";
SKInfo.Description += SM.getIdentifierForBuffer(BufferID);
SKInfo.Description += SM.getIdentifierForBuffer(*BufferIDOpt);
SKInfo.Description += ")";
SKInfo.Offset = LastDiag.Offset;
SKInfo.Line = LastDiag.Line;
Expand All @@ -138,35 +140,39 @@ void EditorDiagConsumer::handleDiagnostic(
}
}

SKInfo.Offset = SM.getLocOffsetInBuffer(Loc, BufferID);
std::tie(SKInfo.Line, SKInfo.Column) = SM.getLineAndColumn(Loc, BufferID);
SKInfo.Filename = SM.getIdentifierForBuffer(BufferID);
if (BufferIDOpt.hasValue()) {
unsigned BufferID = *BufferIDOpt;

for (auto R : Info.Ranges) {
if (R.isInvalid() || SM.findBufferContainingLoc(R.getStart()) != BufferID)
continue;
unsigned Offset = SM.getLocOffsetInBuffer(R.getStart(), BufferID);
unsigned Length = R.getByteLength();
SKInfo.Ranges.push_back({ Offset, Length });
}
SKInfo.Offset = SM.getLocOffsetInBuffer(Loc, BufferID);
std::tie(SKInfo.Line, SKInfo.Column) = SM.getLineAndColumn(Loc, BufferID);
SKInfo.Filename = SM.getIdentifierForBuffer(BufferID);

for (auto F : Info.FixIts) {
if (F.getRange().isInvalid() ||
SM.findBufferContainingLoc(F.getRange().getStart()) != BufferID)
continue;
unsigned Offset = SM.getLocOffsetInBuffer(F.getRange().getStart(),
BufferID);
unsigned Length = F.getRange().getByteLength();
SKInfo.Fixits.push_back({ Offset, Length, F.getText() });
for (auto R : Info.Ranges) {
if (R.isInvalid() || SM.findBufferContainingLoc(R.getStart()) != BufferID)
continue;
unsigned Offset = SM.getLocOffsetInBuffer(R.getStart(), BufferID);
unsigned Length = R.getByteLength();
SKInfo.Ranges.push_back({Offset, Length});
}

for (auto F : Info.FixIts) {
if (F.getRange().isInvalid() ||
SM.findBufferContainingLoc(F.getRange().getStart()) != BufferID)
continue;
unsigned Offset =
SM.getLocOffsetInBuffer(F.getRange().getStart(), BufferID);
unsigned Length = F.getRange().getByteLength();
SKInfo.Fixits.push_back({Offset, Length, F.getText()});
}
} else {
SKInfo.Filename = "<unknown>";
}

if (IsNote) {
getLastDiag().Notes.push_back(std::move(SKInfo));
return;
}

DiagnosticsTy &Diagnostics = BufferDiagnostics[BufferID];

switch (Kind) {
case DiagnosticKind::Error:
SKInfo.Severity = DiagnosticSeverityKind::Error;
Expand All @@ -179,6 +185,15 @@ void EditorDiagConsumer::handleDiagnostic(
llvm_unreachable("already covered");
}

if (!BufferIDOpt) {
InvalidLocDiagnostics.push_back(std::move(SKInfo));
clearLastDiag();
return;
}

unsigned BufferID = *BufferIDOpt;
DiagnosticsTy &Diagnostics = BufferDiagnostics[BufferID];

if (Diagnostics.empty() || Diagnostics.back().Offset <= SKInfo.Offset) {
Diagnostics.push_back(std::move(SKInfo));
LastDiagBufferID = BufferID;
Expand Down
4 changes: 1 addition & 3 deletions tools/SourceKit/lib/SwiftLang/SwiftEditorDiagConsumer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class EditorDiagConsumer : public swift::DiagnosticConsumer {
/// Maps from a BufferID to the diagnostics that were emitted inside that
/// buffer.
llvm::DenseMap<unsigned, DiagnosticsTy> BufferDiagnostics;
DiagnosticsTy InvalidLocDiagnostics;

SmallVector<unsigned, 8> InputBufIDs;
int LastDiagBufferID = -1;
Expand All @@ -40,7 +41,6 @@ class EditorDiagConsumer : public swift::DiagnosticConsumer {
return BufferDiagnostics[LastDiagBufferID][LastDiagIndex];
}

bool HadInvalidLocError = false;
bool HadAnyError = false;

public:
Expand All @@ -64,8 +64,6 @@ class EditorDiagConsumer : public swift::DiagnosticConsumer {

void getAllDiagnostics(SmallVectorImpl<DiagnosticEntryInfo> &Result);

bool hadErrorWithInvalidLoc() const { return HadInvalidLocError; }

bool hadAnyError() const { return HadAnyError; }

void handleDiagnostic(swift::SourceManager &SM, swift::SourceLoc Loc,
Expand Down