[clang] 156c075 - [clang][transformer] Finish plumbing `Note` all the way to the output.

[Clement Courbet via cfe-commits]

Author: Clement Courbet
Date: 2022-08-11T07:54:44+02:00
New Revision: 156c0754bc2ee0b7d198cfeff230374a1961ac04

URL: https://github.com/llvm/llvm-project/commit/156c0754bc2ee0b7d198cfeff230374a1961ac04
DIFF: https://github.com/llvm/llvm-project/commit/156c0754bc2ee0b7d198cfeff230374a1961ac04.diff

LOG: [clang][transformer] Finish plumbing `Note` all the way to the output.

Right now we can only add a single warning, notes are not possible.

Apparently some provisions were made to allow notes, but they were never
propagated all the way to the diagnostics.

Differential Revision: https://reviews.llvm.org/D128807

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
    clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
    clang/include/clang/Tooling/Transformer/RewriteRule.h
    clang/lib/Tooling/Transformer/RewriteRule.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
index bd76e67f12c85..136b616836f1e 100644
--- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "TransformerClangTidyCheck.h"
+#include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/STLExtras.h"
 
@@ -126,18 +127,28 @@ void TransformerClangTidyCheck::check(
   }
 
   // Associate the diagnostic with the location of the first change.
-  DiagnosticBuilder Diag =
-      diag((*Edits)[0].Range.getBegin(), escapeForDiagnostic(*Explanation));
-  for (const auto &T : *Edits)
-    switch (T.Kind) {
-    case transformer::EditKind::Range:
-      Diag << FixItHint::CreateReplacement(T.Range, T.Replacement);
-      break;
-    case transformer::EditKind::AddInclude:
-      Diag << Inserter.createIncludeInsertion(
-          Result.SourceManager->getFileID(T.Range.getBegin()), T.Replacement);
-      break;
+  {
+    DiagnosticBuilder Diag =
+        diag((*Edits)[0].Range.getBegin(), escapeForDiagnostic(*Explanation));
+    for (const auto &T : *Edits) {
+      switch (T.Kind) {
+      case transformer::EditKind::Range:
+        Diag << FixItHint::CreateReplacement(T.Range, T.Replacement);
+        break;
+      case transformer::EditKind::AddInclude:
+        Diag << Inserter.createIncludeInsertion(
+            Result.SourceManager->getFileID(T.Range.getBegin()), T.Replacement);
+        break;
+      }
     }
+  }
+  // Emit potential notes.
+  for (const auto &T : *Edits) {
+    if (!T.Note.empty()) {
+      diag(T.Range.getBegin(), escapeForDiagnostic(T.Note),
+           DiagnosticIDs::Note);
+    }
+  }
 }
 
 void TransformerClangTidyCheck::storeOptions(

diff  --git a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
index 9106d5a77dd7d..5c5a0943c6cf8 100644
--- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -28,6 +28,7 @@ using transformer::IncludeFormat;
 using transformer::makeRule;
 using transformer::node;
 using transformer::noopEdit;
+using transformer::note;
 using transformer::RewriteRuleWith;
 using transformer::RootID;
 using transformer::statement;
@@ -85,6 +86,7 @@ TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) {
   EXPECT_EQ(Errors.size(), 1U);
   EXPECT_EQ(Errors[0].Message.Message, "message");
   EXPECT_THAT(Errors[0].Message.Ranges, testing::IsEmpty());
+  EXPECT_THAT(Errors[0].Notes, testing::IsEmpty());
 
   // The diagnostic is anchored to the match, "return 5".
   EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
@@ -116,6 +118,27 @@ TEST(TransformerClangTidyCheckTest, EmptyReplacement) {
   EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
 }
 
+TEST(TransformerClangTidyCheckTest, NotesCorrectlyGenerated) {
+  class DiagAndNoteCheck : public TransformerClangTidyCheck {
+  public:
+    DiagAndNoteCheck(StringRef Name, ClangTidyContext *Context)
+        : TransformerClangTidyCheck(
+              makeRule(returnStmt(),
+                       note(node(RootID), cat("some note")),
+                       cat("message")),
+              Name, Context) {}
+  };
+  std::string Input = "int h() { return 5; }";
+  std::vector<ClangTidyError> Errors;
+  EXPECT_EQ(Input, test::runCheckOnCode<DiagAndNoteCheck>(Input, &Errors));
+  EXPECT_EQ(Errors.size(), 1U);
+  EXPECT_EQ(Errors[0].Notes.size(), 1U);
+  EXPECT_EQ(Errors[0].Notes[0].Message, "some note");
+
+  // The note is anchored to the match, "return 5".
+  EXPECT_EQ(Errors[0].Notes[0].FileOffset, 10U);
+}
+
 TEST(TransformerClangTidyCheckTest, DiagnosticMessageEscaped) {
   class GiveDiagWithPercentSymbol : public TransformerClangTidyCheck {
   public:

diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index b4609099bf804..4e8537c6c801d 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -46,6 +46,7 @@ struct Edit {
   EditKind Kind = EditKind::Range;
   CharSourceRange Range;
   std::string Replacement;
+  std::string Note;
   llvm::Any Metadata;
 };
 
@@ -138,6 +139,10 @@ inline EditGenerator noEdits() { return editList({}); }
 /// diagnostic `Explanation` with the rule.
 EditGenerator noopEdit(RangeSelector Anchor);
 
+/// Generates a single, no-op edit with the associated note anchored at the
+/// start location of the specified range.
+ASTEdit note(RangeSelector Anchor, TextGenerator Note);
+
 /// Version of `ifBound` specialized to `ASTEdit`.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
                              ASTEdit FalseEdit) {

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index 822d00d33d164..b8acab5327a41 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -59,6 +59,12 @@ translateEdits(const MatchResult &Result, ArrayRef<ASTEdit> ASTEdits) {
         return Replacement.takeError();
       T.Replacement = std::move(*Replacement);
     }
+    if (E.Note) {
+      auto Note = E.Note->eval(Result);
+      if (!Note)
+        return Note.takeError();
+      T.Note = std::move(*Note);
+    }
     if (E.Metadata) {
       auto Metadata = E.Metadata(Result);
       if (!Metadata)
@@ -125,6 +131,13 @@ ASTEdit transformer::changeTo(RangeSelector Target, TextGenerator Replacement) {
   return E;
 }
 
+ASTEdit transformer::note(RangeSelector Anchor, TextGenerator Note) {
+  ASTEdit E;
+  E.TargetRange = transformer::before(Anchor);
+  E.Note = std::move(Note);
+  return E;
+}
+
 namespace {
 /// A \c TextGenerator that always returns a fixed string.
 class SimpleTextGenerator : public MatchComputation<std::string> {