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

[Clement Courbet via Phabricator via cfe-commits]

courbet created this revision.
courbet added a reviewer: ymandel.
Herald added a project: All.
courbet requested review of this revision.
Herald added projects: clang, clang-tools-extra.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128887

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


Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===================================================================
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -50,17 +50,21 @@
     // 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;
Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -90,6 +90,32 @@
   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:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128887.441287.patch
Type: text/x-patch
Size: 2678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220630/30079e37/attachment.bin>