Merge pull request #15782 from benlangmuir/invalid-diag-loc-note

 [sourcekitd] Capture diagnostics with invalid locations for compile notifications

Author: benlangmuir

test/SourceKit/CompileNotifications/diagnostics.swift

@@ -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

tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp

@@ -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"
@@ -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);
@@ -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())
@@ -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 ||
@@ -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;
@@ -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;
@@ -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;

tools/SourceKit/lib/SwiftLang/SwiftEditorDiagConsumer.h

@@ -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;
@@ -40,7 +41,6 @@ class EditorDiagConsumer : public swift::DiagnosticConsumer {
     return BufferDiagnostics[LastDiagBufferID][LastDiagIndex];
   }
 
-  bool HadInvalidLocError = false;
   bool HadAnyError = false;
 
 public:
@@ -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,