Make tooling::applyAllReplacements return llvm::Expected<string> instead of empty string to indicate potential error.

Summary:
return llvm::Expected<> to carry error status and error information.
This is the first step towards introducing "Error" into tooling::Replacements.

Reviewers: djasper, klimek

Subscribers: ioeric, klimek, cfe-commits

Differential Revision: http://reviews.llvm.org/D21601

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@275062 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Format/Format.h

@@ -770,16 +770,18 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
                                    unsigned *Cursor = nullptr);
 
 /// \brief Returns the replacements corresponding to applying and formatting
-/// \p Replaces.
-tooling::Replacements formatReplacements(StringRef Code,
-                                         const tooling::Replacements &Replaces,
-                                         const FormatStyle &Style);
+/// \p Replaces on success; otheriwse, return an llvm::Error carrying
+/// llvm::StringError.
+llvm::Expected<tooling::Replacements>
+formatReplacements(StringRef Code, const tooling::Replacements &Replaces,
+                   const FormatStyle &Style);
 
 /// \brief Returns the replacements corresponding to applying \p Replaces and
-/// cleaning up the code after that.
+/// cleaning up the code after that on success; otherwise, return an llvm::Error
+/// carrying llvm::StringError.
 /// This also inserts a C++ #include directive into the correct block if the
 /// replacement corresponding to the header insertion has offset UINT_MAX.
-tooling::Replacements
+llvm::Expected<tooling::Replacements>
 cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
                           const FormatStyle &Style);
 

include/clang/Tooling/Core/Replacement.h

@@ -22,6 +22,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include <map>
 #include <set>
 #include <string>
@@ -165,9 +166,13 @@ bool applyAllReplacements(const std::vector<Replacement> &Replaces,
 
 /// \brief Applies all replacements in \p Replaces to \p Code.
 ///
-/// This completely ignores the path stored in each replacement. If one or more
-/// replacements cannot be applied, this returns an empty \c string.
-std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);
+/// This completely ignores the path stored in each replacement. If all
+/// replacements are applied successfully, this returns the code with
+/// replacements applied; otherwise, an llvm::Error carrying llvm::StringError
+/// is returned (the Error message can be converted to string using
+/// `llvm::toString()` and 'std::error_code` in the `Error` should be ignored).
+llvm::Expected<std::string> applyAllReplacements(StringRef Code,
+                                                 const Replacements &Replaces);
 
 /// \brief Calculates how a code \p Position is shifted when \p Replaces are
 /// applied.
@@ -203,29 +208,6 @@ struct TranslationUnitReplacements {
   std::vector<Replacement> Replacements;
 };
 
-/// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite.
-///
-/// Replacement applications happen independently of the success of
-/// other applications.
-///
-/// \returns true if all replacements apply. false otherwise.
-bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite);
-
-/// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite.
-///
-/// Replacement applications happen independently of the success of
-/// other applications.
-///
-/// \returns true if all replacements apply. false otherwise.
-bool applyAllReplacements(const std::vector<Replacement> &Replaces,
-                          Rewriter &Rewrite);
-
-/// \brief Applies all replacements in \p Replaces to \p Code.
-///
-/// This completely ignores the path stored in each replacement. If one or more
-/// replacements cannot be applied, this returns an empty \c string.
-std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);
-
 /// \brief Calculates the ranges in a single file that are affected by the
 /// Replacements. Overlapping ranges will be merged.
 ///

lib/Format/Format.cpp

@@ -1393,27 +1393,29 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code,
 }
 
 template <typename T>
-static tooling::Replacements
+static llvm::Expected<tooling::Replacements>
 processReplacements(T ProcessFunc, StringRef Code,
                     const tooling::Replacements &Replaces,
                     const FormatStyle &Style) {
   if (Replaces.empty())
     return tooling::Replacements();
 
-  std::string NewCode = applyAllReplacements(Code, Replaces);
+  auto NewCode = applyAllReplacements(Code, Replaces);
+  if (!NewCode)
+    return NewCode.takeError();
   std::vector<tooling::Range> ChangedRanges =
       tooling::calculateChangedRanges(Replaces);
   StringRef FileName = Replaces.begin()->getFilePath();
 
   tooling::Replacements FormatReplaces =
-      ProcessFunc(Style, NewCode, ChangedRanges, FileName);
+      ProcessFunc(Style, *NewCode, ChangedRanges, FileName);
 
   return mergeReplacements(Replaces, FormatReplaces);
 }
 
-tooling::Replacements formatReplacements(StringRef Code,
-                                         const tooling::Replacements &Replaces,
-                                         const FormatStyle &Style) {
+llvm::Expected<tooling::Replacements>
+formatReplacements(StringRef Code, const tooling::Replacements &Replaces,
+                   const FormatStyle &Style) {
   // We need to use lambda function here since there are two versions of
   // `sortIncludes`.
   auto SortIncludes = [](const FormatStyle &Style, StringRef Code,
@@ -1421,8 +1423,10 @@ tooling::Replacements formatReplacements(StringRef Code,
                          StringRef FileName) -> tooling::Replacements {
     return sortIncludes(Style, Code, Ranges, FileName);
   };
-  tooling::Replacements SortedReplaces =
+  auto SortedReplaces =
       processReplacements(SortIncludes, Code, Replaces, Style);
+  if (!SortedReplaces)
+    return SortedReplaces.takeError();
 
   // We need to use lambda function here since there are two versions of
   // `reformat`.
@@ -1431,7 +1435,7 @@ tooling::Replacements formatReplacements(StringRef Code,
                      StringRef FileName) -> tooling::Replacements {
     return reformat(Style, Code, Ranges, FileName);
   };
-  return processReplacements(Reformat, Code, SortedReplaces, Style);
+  return processReplacements(Reformat, Code, *SortedReplaces, Style);
 }
 
 namespace {
@@ -1591,7 +1595,7 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
 
 } // anonymous namespace
 
-tooling::Replacements
+llvm::Expected<tooling::Replacements>
 cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
                           const FormatStyle &Style) {
   // We need to use lambda function here since there are two versions of

lib/Tooling/Core/Replacement.cpp

@@ -249,8 +249,10 @@ bool applyAllReplacements(const std::vector<Replacement> &Replaces,
   return Result;
 }
 
-std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) {
-  if (Replaces.empty()) return Code;
+llvm::Expected<std::string> applyAllReplacements(StringRef Code,
+                                                const Replacements &Replaces) {
+  if (Replaces.empty())
+    return Code.str();
 
   IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
       new vfs::InMemoryFileSystem);
@@ -269,7 +271,9 @@ std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) {
     Replacement Replace("<stdin>", I->getOffset(), I->getLength(),
                         I->getReplacementText());
     if (!Replace.apply(Rewrite))
-      return "";
+      return llvm::make_error<llvm::StringError>(
+          "Failed to apply replacement: " + Replace.toString(),
+          llvm::inconvertibleErrorCode());
   }
   std::string Result;
   llvm::raw_string_ostream OS(Result);

lib/Tooling/Refactoring.cpp

@@ -79,9 +79,13 @@ bool formatAndApplyAllReplacements(const Replacements &Replaces,
     StringRef Code = SM.getBufferData(ID);
 
     format::FormatStyle CurStyle = format::getStyle(Style, FilePath, "LLVM");
-    Replacements NewReplacements =
+    auto NewReplacements =
         format::formatReplacements(Code, CurReplaces, CurStyle);
-    Result = applyAllReplacements(NewReplacements, Rewrite) && Result;
+    if (!NewReplacements) {
+      llvm::errs() << llvm::toString(NewReplacements.takeError()) << "\n";
+      return false;
+    }
+    Result = applyAllReplacements(*NewReplacements, Rewrite) && Result;
   }
   return Result;
 }

tools/clang-format/ClangFormat.cpp

@@ -257,13 +257,16 @@ static bool format(StringRef FileName) {
   unsigned CursorPosition = Cursor;
   Replacements Replaces = sortIncludes(FormatStyle, Code->getBuffer(), Ranges,
                                        AssumedFileName, &CursorPosition);
-  std::string ChangedCode =
-      tooling::applyAllReplacements(Code->getBuffer(), Replaces);
+  auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);
+  if (!ChangedCode) {
+    llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
+    return true;
+  }
   for (const auto &R : Replaces)
     Ranges.push_back({R.getOffset(), R.getLength()});
 
   bool IncompleteFormat = false;
-  Replacements FormatChanges = reformat(FormatStyle, ChangedCode, Ranges,
+  Replacements FormatChanges = reformat(FormatStyle, *ChangedCode, Ranges,
                                         AssumedFileName, &IncompleteFormat);
   Replaces = tooling::mergeReplacements(Replaces, FormatChanges);
   if (OutputXML) {

unittests/Format/CleanupTest.cpp

@@ -25,9 +25,9 @@ protected:
                       const FormatStyle &Style = getLLVMStyle()) {
     tooling::Replacements Replaces = format::cleanup(Style, Code, Ranges);
 
-    std::string Result = applyAllReplacements(Code, Replaces);
-    EXPECT_NE("", Result);
-    return Result;
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    return *Result;
   }
 };
 
@@ -254,16 +254,26 @@ protected:
 
   inline std::string apply(StringRef Code,
                            const tooling::Replacements Replaces) {
-    return applyAllReplacements(
-        Code, cleanupAroundReplacements(Code, Replaces, Style));
+    auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+    EXPECT_TRUE(static_cast<bool>(CleanReplaces))
+        << llvm::toString(CleanReplaces.takeError()) << "\n";
+    auto Result = applyAllReplacements(Code, *CleanReplaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    return *Result;
   }
 
   inline std::string formatAndApply(StringRef Code,
                                     const tooling::Replacements Replaces) {
-    return applyAllReplacements(
-        Code,
-        formatReplacements(
-            Code, cleanupAroundReplacements(Code, Replaces, Style), Style));
+
+    auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style);
+    EXPECT_TRUE(static_cast<bool>(CleanReplaces))
+        << llvm::toString(CleanReplaces.takeError()) << "\n";
+    auto FormattedReplaces = formatReplacements(Code, *CleanReplaces, Style);
+    EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
+        << llvm::toString(FormattedReplaces.takeError()) << "\n";
+    auto Result = applyAllReplacements(Code, *FormattedReplaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    return *Result;
   }
 
   int getOffset(StringRef Code, int Line, int Column) {

unittests/Format/FormatTest.cpp

@@ -47,10 +47,10 @@ protected:
       EXPECT_EQ(ExpectedIncompleteFormat, IncompleteFormat) << Code << "\n\n";
     }
     ReplacementCount = Replaces.size();
-    std::string Result = applyAllReplacements(Code, Replaces);
-    EXPECT_NE("", Result);
-    DEBUG(llvm::errs() << "\n" << Result << "\n\n");
-    return Result;
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
   }
 
   FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
@@ -11553,8 +11553,12 @@ TEST_F(ReplacementTest, FormatCodeAfterReplacements) {
 
   format::FormatStyle Style = format::getLLVMStyle();
   Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility.
-  EXPECT_EQ(Expected, applyAllReplacements(
-                          Code, formatReplacements(Code, Replaces, Style)));
+  auto FormattedReplaces = formatReplacements(Code, Replaces, Style);
+  EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
+      << llvm::toString(FormattedReplaces.takeError()) << "\n";
+  auto Result = applyAllReplacements(Code, *FormattedReplaces);
+  EXPECT_TRUE(static_cast<bool>(Result));
+  EXPECT_EQ(Expected, *Result);
 }
 
 TEST_F(ReplacementTest, SortIncludesAfterReplacement) {
@@ -11578,8 +11582,12 @@ TEST_F(ReplacementTest, SortIncludesAfterReplacement) {
 
   format::FormatStyle Style = format::getLLVMStyle();
   Style.SortIncludes = true;
-  auto FinalReplaces = formatReplacements(Code, Replaces, Style);
-  EXPECT_EQ(Expected, applyAllReplacements(Code, FinalReplaces));
+  auto FormattedReplaces = formatReplacements(Code, Replaces, Style);
+  EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
+      << llvm::toString(FormattedReplaces.takeError()) << "\n";
+  auto Result = applyAllReplacements(Code, *FormattedReplaces);
+  EXPECT_TRUE(static_cast<bool>(Result));
+  EXPECT_EQ(Expected, *Result);
 }
 
 } // end namespace

unittests/Format/FormatTestJS.cpp

@@ -28,10 +28,10 @@ protected:
     tooling::Replacements Replaces =
         reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
     EXPECT_FALSE(IncompleteFormat);
-    std::string Result = applyAllReplacements(Code, Replaces);
-    EXPECT_NE("", Result);
-    DEBUG(llvm::errs() << "\n" << Result << "\n\n");
-    return Result;
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
   }
 
   static std::string format(

unittests/Format/FormatTestJava.cpp

@@ -25,10 +25,10 @@ protected:
     DEBUG(llvm::errs() << Code << "\n\n");
     std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
     tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-    std::string Result = applyAllReplacements(Code, Replaces);
-    EXPECT_NE("", Result);
-    DEBUG(llvm::errs() << "\n" << Result << "\n\n");
-    return Result;
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
   }
 
   static std::string

unittests/Format/FormatTestProto.cpp

@@ -25,10 +25,10 @@ protected:
     DEBUG(llvm::errs() << Code << "\n\n");
     std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
     tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-    std::string Result = applyAllReplacements(Code, Replaces);
-    EXPECT_NE("", Result);
-    DEBUG(llvm::errs() << "\n" << Result << "\n\n");
-    return Result;
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
   }
 
   static std::string format(llvm::StringRef Code) {

unittests/Format/FormatTestSelective.cpp

@@ -28,10 +28,10 @@ protected:
     tooling::Replacements Replaces =
         reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
     EXPECT_FALSE(IncompleteFormat) << Code << "\n\n";
-    std::string Result = applyAllReplacements(Code, Replaces);
-    EXPECT_NE("", Result);
-    DEBUG(llvm::errs() << "\n" << Result << "\n\n");
-    return Result;
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
   }
 
   FormatStyle Style = getLLVMStyle();

unittests/Format/SortImportsTestJS.cpp

@@ -25,10 +25,13 @@ protected:
     if (Length == 0U)
       Length = Code.size() - Offset;
     std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
-    std::string Sorted =
+    auto Sorted =
         applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));
-    return applyAllReplacements(Sorted,
-                                reformat(Style, Sorted, Ranges, FileName));
+    EXPECT_TRUE(static_cast<bool>(Sorted));
+    auto Formatted = applyAllReplacements(
+        *Sorted, reformat(Style, *Sorted, Ranges, FileName));
+    EXPECT_TRUE(static_cast<bool>(Formatted));
+    return *Formatted;
   }
 
   void verifySort(llvm::StringRef Expected, llvm::StringRef Code,

unittests/Format/SortIncludesTest.cpp

@@ -26,10 +26,13 @@ protected:
 
   std::string sort(StringRef Code, StringRef FileName = "input.cpp") {
     auto Ranges = GetCodeRange(Code);
-    std::string Sorted =
+    auto Sorted =
         applyAllReplacements(Code, sortIncludes(Style, Code, Ranges, FileName));
-    return applyAllReplacements(Sorted,
-                                reformat(Style, Sorted, Ranges, FileName));
+    EXPECT_TRUE(static_cast<bool>(Sorted));
+    auto Result = applyAllReplacements(
+        *Sorted, reformat(Style, *Sorted, Ranges, FileName));
+    EXPECT_TRUE(static_cast<bool>(Result));
+    return *Result;
   }
 
   unsigned newCursor(llvm::StringRef Code, unsigned Cursor) {

unittests/Tooling/RefactoringTest.cpp

@@ -640,27 +640,32 @@ protected:
                            StringRef Result, const Replacements &First,
                            const Replacements &Second) {
     // These are mainly to verify the test itself and make it easier to read.
-    std::string AfterFirst = applyAllReplacements(Code, First);
-    std::string InSequenceRewrite = applyAllReplacements(AfterFirst, Second);
-    EXPECT_EQ(Intermediate, AfterFirst);
-    EXPECT_EQ(Result, InSequenceRewrite);
+    auto AfterFirst = applyAllReplacements(Code, First);
+    EXPECT_TRUE(static_cast<bool>(AfterFirst));
+    auto InSequenceRewrite = applyAllReplacements(*AfterFirst, Second);
+    EXPECT_TRUE(static_cast<bool>(InSequenceRewrite));
+    EXPECT_EQ(Intermediate, *AfterFirst);
+    EXPECT_EQ(Result, *InSequenceRewrite);
 
     tooling::Replacements Merged = mergeReplacements(First, Second);
-    std::string MergedRewrite = applyAllReplacements(Code, Merged);
-    EXPECT_EQ(InSequenceRewrite, MergedRewrite);
-    if (InSequenceRewrite != MergedRewrite)
+    auto MergedRewrite = applyAllReplacements(Code, Merged);
+    EXPECT_TRUE(static_cast<bool>(MergedRewrite));
+    EXPECT_EQ(*InSequenceRewrite, *MergedRewrite);
+    if (*InSequenceRewrite != *MergedRewrite)
       for (tooling::Replacement M : Merged)
         llvm::errs() << M.getOffset() << " " << M.getLength() << " "
                      << M.getReplacementText() << "\n";
   }
   void mergeAndTestRewrite(StringRef Code, const Replacements &First,
                            const Replacements &Second) {
-    std::string InSequenceRewrite =
-        applyAllReplacements(applyAllReplacements(Code, First), Second);
+    auto AfterFirst = applyAllReplacements(Code, First);
+    EXPECT_TRUE(static_cast<bool>(AfterFirst));
+    auto InSequenceRewrite = applyAllReplacements(*AfterFirst, Second);
     tooling::Replacements Merged = mergeReplacements(First, Second);
-    std::string MergedRewrite = applyAllReplacements(Code, Merged);
-    EXPECT_EQ(InSequenceRewrite, MergedRewrite);
-    if (InSequenceRewrite != MergedRewrite)
+    auto MergedRewrite = applyAllReplacements(Code, Merged);
+    EXPECT_TRUE(static_cast<bool>(MergedRewrite));
+    EXPECT_EQ(*InSequenceRewrite, *MergedRewrite);
+    if (*InSequenceRewrite != *MergedRewrite)
       for (tooling::Replacement M : Merged)
         llvm::errs() << M.getOffset() << " " << M.getLength() << " "
                      << M.getReplacementText() << "\n";

unittests/Tooling/RewriterTest.cpp

@@ -41,8 +41,9 @@ TEST(Rewriter, AdjacentInsertAndDelete) {
   Replacements Replaces;
   Replaces.insert(Replacement("<file>", 6, 6, ""));
   Replaces.insert(Replacement("<file>", 6, 0, "replaced\n"));
-  EXPECT_EQ("line1\nreplaced\nline3\nline4",
-            applyAllReplacements("line1\nline2\nline3\nline4", Replaces));
+  auto Rewritten = applyAllReplacements("line1\nline2\nline3\nline4", Replaces);
+  EXPECT_TRUE(static_cast<bool>(Rewritten));
+  EXPECT_EQ("line1\nreplaced\nline3\nline4", *Rewritten);
 }
 
 } // end namespace