[clang-tools-extra] 5331e12 - [clang][transformer] Fix crash on replacement-less ASTEdit.

[Clement Courbet via cfe-commits]

Author: Clement Courbet
Date: 2022-08-10T09:08:05+02:00
New Revision: 5331e1229aa6d0d33b5ec9fab7c05310187746d9

URL: https://github.com/llvm/llvm-project/commit/5331e1229aa6d0d33b5ec9fab7c05310187746d9
DIFF: https://github.com/llvm/llvm-project/commit/5331e1229aa6d0d33b5ec9fab7c05310187746d9.diff

LOG: [clang][transformer] Fix crash on replacement-less ASTEdit.

Given that we provide an EditGenerator edit(ASTEdit), we can't ever be
sure that the user won't give us an empty replacement.

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
index a3600ab58ff3f..9106d5a77dd7d 100644
--- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -90,6 +90,32 @@ TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) {
   EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
 }
 
+transformer::ASTEdit noReplacementEdit(transformer::RangeSelector Target) {
+  transformer::ASTEdit E;
+  E.TargetRange = std::move(Target);
+  return E;
+}
+
+TEST(TransformerClangTidyCheckTest, EmptyReplacement) {
+  class DiagOnlyCheck : public TransformerClangTidyCheck {
+  public:
+    DiagOnlyCheck(StringRef Name, ClangTidyContext *Context)
+        : TransformerClangTidyCheck(
+              makeRule(returnStmt(), edit(noReplacementEdit(node(RootID))),
+                       cat("message")),
+              Name, Context) {}
+  };
+  std::string Input = "int h() { return 5; }";
+  std::vector<ClangTidyError> Errors;
+  EXPECT_EQ("int h() { }", test::runCheckOnCode<DiagOnlyCheck>(Input, &Errors));
+  EXPECT_EQ(Errors.size(), 1U);
+  EXPECT_EQ(Errors[0].Message.Message, "message");
+  EXPECT_THAT(Errors[0].Message.Ranges, testing::IsEmpty());
+
+  // The diagnostic is anchored to the match, "return 5".
+  EXPECT_EQ(Errors[0].Message.FileOffset, 10U);
+}
+
 TEST(TransformerClangTidyCheckTest, DiagnosticMessageEscaped) {
   class GiveDiagWithPercentSymbol : public TransformerClangTidyCheck {
   public:

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index 3e76489782f34..822d00d33d164 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -50,17 +50,21 @@ translateEdits(const MatchResult &Result, ArrayRef<ASTEdit> ASTEdits) {
     // produces a bad range, whereas the latter will simply ignore A.
     if (!EditRange)
       return SmallVector<Edit, 0>();
-    auto Replacement = E.Replacement->eval(Result);
-    if (!Replacement)
-      return Replacement.takeError();
-    auto Metadata = E.Metadata(Result);
-    if (!Metadata)
-      return Metadata.takeError();
     transformer::Edit T;
     T.Kind = E.Kind;
     T.Range = *EditRange;
-    T.Replacement = std::move(*Replacement);
-    T.Metadata = std::move(*Metadata);
+    if (E.Replacement) {
+      auto Replacement = E.Replacement->eval(Result);
+      if (!Replacement)
+        return Replacement.takeError();
+      T.Replacement = std::move(*Replacement);
+    }
+    if (E.Metadata) {
+      auto Metadata = E.Metadata(Result);
+      if (!Metadata)
+        return Metadata.takeError();
+      T.Metadata = std::move(*Metadata);
+    }
     Edits.push_back(std::move(T));
   }
   return Edits;