[DiagnosticVerifier] Add support for asserting presence of edu notes

owenv · owenv · commit cfbedd81c42b · 2020-04-09T18:18:25.000-07:00
diff --git a/include/swift/Frontend/DiagnosticVerifier.h b/include/swift/Frontend/DiagnosticVerifier.h
@@ -46,14 +46,19 @@ struct CapturedDiagnosticInfo {
   unsigned Line;
   unsigned Column;
   SmallVector<DiagnosticInfo::FixIt, 2> FixIts;
+  SmallVector<std::string, 1> EducationalNotes;
 
   CapturedDiagnosticInfo(llvm::SmallString<128> Message,
                          llvm::SmallString<32> FileName,
                          DiagnosticKind Classification, SourceLoc Loc,
                          unsigned Line, unsigned Column,
-                         SmallVector<DiagnosticInfo::FixIt, 2> FixIts)
+                         SmallVector<DiagnosticInfo::FixIt, 2> FixIts,
+                         SmallVector<std::string, 1> EducationalNotes)
       : Message(Message), FileName(FileName), Classification(Classification),
-        Loc(Loc), Line(Line), Column(Column), FixIts(FixIts) {}
+        Loc(Loc), Line(Line), Column(Column), FixIts(FixIts),
+        EducationalNotes(EducationalNotes) {
+    std::sort(EducationalNotes.begin(), EducationalNotes.end());
+  }
 };
 /// This class implements support for -verify mode in the compiler.  It
 /// buffers up diagnostics produced during compilation, then checks them
diff --git a/lib/Frontend/DiagnosticVerifier.cpp b/lib/Frontend/DiagnosticVerifier.cpp
@@ -17,9 +17,11 @@
 #include "swift/Frontend/DiagnosticVerifier.h"
 #include "swift/Basic/SourceManager.h"
 #include "swift/Parse/Lexer.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace swift;
@@ -36,6 +38,7 @@ struct ExpectedFixIt {
 namespace {
 
 static constexpr StringLiteral fixitExpectationNoneString("none");
+static constexpr StringLiteral educationalNotesSpecifier("educational-notes=");
 
 struct ExpectedDiagnosticInfo {
   // This specifies the full range of the "expected-foo {{}}" specifier.
@@ -65,6 +68,17 @@ struct ExpectedDiagnosticInfo {
   // Loc of {{none}}
   const char *noneMarkerStartLoc = nullptr;
 
+  /// Represents a specifier of the form '{{educational-notes=note1,note2}}'.
+  struct ExpectedEducationalNotes {
+    const char *StartLoc, *EndLoc; // The loc of the {{ and }}'s.
+    llvm::SmallVector<StringRef, 1> Names; // Names of expected notes.
+
+    ExpectedEducationalNotes(const char *StartLoc, const char *EndLoc,
+                             llvm::SmallVector<StringRef, 1> Names)
+        : StartLoc(StartLoc), EndLoc(EndLoc), Names(Names) {}
+  };
+  Optional<ExpectedEducationalNotes> EducationalNotes;
+
   ExpectedDiagnosticInfo(const char *ExpectedStart,
                          DiagnosticKind Classification)
       : ExpectedStart(ExpectedStart), Classification(Classification) {}
@@ -85,6 +99,18 @@ static std::string getDiagKindString(DiagnosticKind Kind) {
   llvm_unreachable("Unhandled DiagKind in switch.");
 }
 
+/// Render the verifier syntax for a given set of educational notes.
+static std::string
+renderEducationalNotes(llvm::SmallVectorImpl<std::string> &EducationalNotes) {
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  OS << "{{" << educationalNotesSpecifier;
+  interleave(EducationalNotes, [&](const auto &Note) { OS << Note; },
+             [&] { OS << ','; });
+  OS << "}}";
+  return OS.str();
+}
+
 /// If we find the specified diagnostic in the list, return it.
 /// Otherwise return CapturedDiagnostics.end().
 static std::vector<CapturedDiagnosticInfo>::iterator
@@ -439,38 +465,67 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
     StringRef ExtraChecks = MatchStart.substr(End+2).ltrim(" \t");
     while (ExtraChecks.startswith("{{")) {
       // First make sure we have a closing "}}".
-      size_t EndLoc = ExtraChecks.find("}}");
-      if (EndLoc == StringRef::npos) {
+      size_t EndIndex = ExtraChecks.find("}}");
+      if (EndIndex == StringRef::npos) {
         addError(ExtraChecks.data(),
-                 "didn't find '}}' to match '{{' in fix-it verification");
+                 "didn't find '}}' to match '{{' in diagnostic verification");
         break;
       }
-      
+
       // Allow for close braces to appear in the replacement text.
-      while (EndLoc+2 < ExtraChecks.size() && ExtraChecks[EndLoc+2] == '}')
-        ++EndLoc;
-      
-      StringRef FixItStr = ExtraChecks.slice(2, EndLoc);
+      while (EndIndex + 2 < ExtraChecks.size() &&
+             ExtraChecks[EndIndex + 2] == '}')
+        ++EndIndex;
+
+      const char *OpenLoc = ExtraChecks.data(); // Beginning of opening '{{'.
+      const char *CloseLoc =
+          ExtraChecks.data() + EndIndex + 2; // End of closing '}}'.
+
+      StringRef CheckStr = ExtraChecks.slice(2, EndIndex);
       // Check for matching a later "}}" on a different line.
-      if (FixItStr.find_first_of("\r\n") != StringRef::npos) {
+      if (CheckStr.find_first_of("\r\n") != StringRef::npos) {
         addError(ExtraChecks.data(), "didn't find '}}' to match '{{' in "
-                 "fix-it verification");
+                                     "diagnostic verification");
         break;
       }
-      
+
       // Prepare for the next round of checks.
-      ExtraChecks = ExtraChecks.substr(EndLoc+2).ltrim();
-      
+      ExtraChecks = ExtraChecks.substr(EndIndex + 2).ltrim();
+
+      // If this check starts with 'educational-notes=', check for one or more
+      // educational notes instead of a fix-it.
+      if (CheckStr.startswith(educationalNotesSpecifier)) {
+        if (Expected.EducationalNotes.hasValue()) {
+          addError(CheckStr.data(),
+                   "each verified diagnostic may only have one "
+                   "{{educational-notes=<#notes#>}} declaration");
+          continue;
+        }
+        StringRef NotesStr = CheckStr.substr(
+            educationalNotesSpecifier.size()); // Trim 'educational-notes='.
+        llvm::SmallVector<StringRef, 1> names;
+        // Note names are comma-separated.
+        std::pair<StringRef, StringRef> split;
+        do {
+          split = NotesStr.split(',');
+          names.push_back(split.first);
+          NotesStr = split.second;
+        } while (!NotesStr.empty());
+        Expected.EducationalNotes.emplace(OpenLoc, CloseLoc, names);
+        continue;
+      }
+
+      // This wasn't an educational notes specifier, so it must be a fix-it.
       // Special case for specifying no fixits should appear.
-      if (FixItStr == fixitExpectationNoneString) {
+      if (CheckStr == fixitExpectationNoneString) {
         if (Expected.noneMarkerStartLoc) {
-          addError(FixItStr.data() - 2,
+          addError(CheckStr.data() - 2,
                    Twine("A second {{") + fixitExpectationNoneString +
                        "}} was found. It may only appear once in an expectation.");
           break;
         }
 
-        Expected.noneMarkerStartLoc = FixItStr.data() - 2;
+        Expected.noneMarkerStartLoc = CheckStr.data() - 2;
         continue;
       }
 
@@ -482,14 +537,14 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
       }
 
       // Parse the pieces of the fix-it.
-      size_t MinusLoc = FixItStr.find('-');
+      size_t MinusLoc = CheckStr.find('-');
       if (MinusLoc == StringRef::npos) {
-        addError(FixItStr.data(), "expected '-' in fix-it verification");
+        addError(CheckStr.data(), "expected '-' in fix-it verification");
         continue;
       }
-      StringRef StartColStr = FixItStr.slice(0, MinusLoc);
-      StringRef AfterMinus = FixItStr.substr(MinusLoc+1);
-      
+      StringRef StartColStr = CheckStr.slice(0, MinusLoc);
+      StringRef AfterMinus = CheckStr.substr(MinusLoc + 1);
+
       size_t EqualLoc = AfterMinus.find('=');
       if (EqualLoc == StringRef::npos) {
         addError(AfterMinus.data(),
@@ -500,8 +555,8 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
       StringRef AfterEqual = AfterMinus.substr(EqualLoc+1);
       
       ExpectedFixIt FixIt;
-      FixIt.StartLoc = StartColStr.data()-2;
-      FixIt.EndLoc = FixItStr.data()+EndLoc;
+      FixIt.StartLoc = OpenLoc;
+      FixIt.EndLoc = CloseLoc;
       if (StartColStr.getAsInteger(10, FixIt.StartCol)) {
         addError(StartColStr.data(),
                  "invalid column number in fix-it verification");
@@ -514,7 +569,7 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
       }
       
       // Translate literal "\\n" into '\n', inefficiently.
-      StringRef fixItText = AfterEqual.slice(0, EndLoc);
+      StringRef fixItText = AfterEqual.slice(0, EndIndex);
       for (const char *current = fixItText.begin(), *end = fixItText.end();
            current != end; /* in loop */) {
         if (*current == '\\' && current + 1 < end) {
@@ -681,6 +736,33 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
                       replEndLoc, actualFixits);
     }
 
+    if (auto expectedNotes = expected.EducationalNotes) {
+      // Verify educational notes
+      for (auto &foundName : FoundDiagnostic.EducationalNotes) {
+        llvm::erase_if(expectedNotes->Names,
+                       [&](std::string item) { return item == foundName; });
+      }
+
+      if (!expectedNotes->Names.empty()) {
+        if (FoundDiagnostic.EducationalNotes.empty()) {
+          addError(expectedNotes->StartLoc,
+                   "expected educational note(s) not seen");
+        } else {
+          // If we had an incorrect expected note, render it and produce a fixit
+          // of our own.
+          auto actual =
+              renderEducationalNotes(FoundDiagnostic.EducationalNotes);
+          auto replStartLoc = SMLoc::getFromPointer(expectedNotes->StartLoc);
+          auto replEndLoc = SMLoc::getFromPointer(expectedNotes->EndLoc);
+
+          llvm::SMFixIt fix(llvm::SMRange(replStartLoc, replEndLoc), actual);
+          addError(expectedNotes->StartLoc,
+                   "expected educational note(s) not seen; actual educational "
+                   "note(s): " + actual, fix);
+        }
+      }
+    }
+
     // Actually remove the diagnostic from the list, so we don't match it
     // again. We do have to do this after checking fix-its, though, because
     // the diagnostic owns its fix-its.
@@ -866,6 +948,11 @@ void DiagnosticVerifier::handleDiagnostic(SourceManager &SM,
   SmallVector<DiagnosticInfo::FixIt, 2> fixIts;
   std::copy(Info.FixIts.begin(), Info.FixIts.end(), std::back_inserter(fixIts));
 
+  llvm::SmallVector<std::string, 1> eduNotes;
+  for (auto &notePath : Info.EducationalNotePaths) {
+    eduNotes.push_back(llvm::sys::path::stem(notePath));
+  }
+
   llvm::SmallString<128> message;
   {
     llvm::raw_svector_ostream Out(message);
@@ -878,10 +965,10 @@ void DiagnosticVerifier::handleDiagnostic(SourceManager &SM,
     const auto fileName = SM.getDisplayNameForLoc(Info.Loc);
     CapturedDiagnostics.emplace_back(message, fileName, Info.Kind, Info.Loc,
                                      lineAndColumn.first, lineAndColumn.second,
-                                     fixIts);
+                                     fixIts, eduNotes);
   } else {
     CapturedDiagnostics.emplace_back(message, StringRef(), Info.Kind, Info.Loc,
-                                     0, 0, fixIts);
+                                     0, 0, fixIts, eduNotes);
   }
 }
 
diff --git a/test/diagnostics/verifier.swift b/test/diagnostics/verifier.swift
@@ -33,6 +33,15 @@ func b() {
 // CHECK: unexpected warning produced: initialization of immutable value 'c' was never used
 // CHECK-WARNINGS-AS-ERRORS: unexpected error produced: initialization of immutable value 'c' was never used
 
+extension (Int, Int) {} // expected-error {{non-nominal type '(Int, Int)' cannot be extended}} {{educational-notes=foo-bar-baz}}
+// CHECK: error: expected educational note(s) not seen; actual educational note(s): {{[{][{]}}educational-notes=nominal-types{{[}][}]}}
+
+extension (Bool, Int) {} // expected-error {{non-nominal type '(Bool, Int)' cannot be extended}} {{educational-notes=nominal-types}} {{educational-notes=nominal-types}}
+// CHECK: error: each verified diagnostic may only have one {{[{][{]}}educational-notes=<#notes#>{{[}][}]}} declaration
+
+extension (Bool, Bool) {} // expected-error {{non-nominal type '(Bool, Bool)' cannot be extended}} {{educational-notes=nominal-types,foo-bar-baz}}
+// CHECK: error: expected educational note(s) not seen; actual educational note(s): {{[{][{]}}educational-notes=nominal-types{{[}][}]}}
+
 // Verify the serialized diags have the right magic at the top.
 // CHECK-SERIALIZED: DIA
 
