diff --git a/include/clang/Tooling/Core/Replacement.h b/include/clang/Tooling/Core/Replacement.h
index 5bb1e7f023fac8dbdf1d9d479f8762b94317948b..95dc3cd6e7639eb923cd129e19e27597b1afbd02 100644
--- a/include/clang/Tooling/Core/Replacement.h
+++ b/include/clang/Tooling/Core/Replacement.h
@@ -22,11 +22,14 @@
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
#include <map>
#include <set>
#include <string>
+#include <system_error>
#include <vector>
namespace clang {
@@ -137,6 +140,59 @@ private:
std::string ReplacementText;
};
+enum class replacement_error {
+ fail_to_apply = 0,
+ wrong_file_path,
+ overlap_conflict,
+ insert_conflict,
+};
+
+/// \brief Carries extra error information in replacement-related llvm::Error,
+/// e.g. fail applying replacements and replacements conflict.
+class ReplacementError : public llvm::ErrorInfo<ReplacementError> {
+public:
+ ReplacementError(replacement_error Err) : Err(Err) {}
+
+ /// \brief Constructs an error related to an existing replacement.
+ ReplacementError(replacement_error Err, Replacement Existing)
+ : Err(Err), ExistingReplacement(std::move(Existing)) {}
+
+ /// \brief Constructs an error related to a new replacement and an existing
+ /// replacement in a set of replacements.
+ ReplacementError(replacement_error Err, Replacement New, Replacement Existing)
+ : Err(Err), NewReplacement(std::move(New)),
+ ExistingReplacement(std::move(Existing)) {}
+
+ std::string message() const override;
+
+ void log(raw_ostream &OS) const override { OS << message(); }
+
+ replacement_error get() const { return Err; }
+
+ static char ID;
+
+ const llvm::Optional<Replacement> &getNewReplacement() const {
+ return NewReplacement;
+ }
+
+ const llvm::Optional<Replacement> &getExistingReplacement() const {
+ return ExistingReplacement;
+ }
+
+private:
+ // Users are not expected to use error_code.
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+
+ replacement_error Err;
+ // A new replacement, which is to expected be added into a set of
+ // replacements, that is causing problem.
+ llvm::Optional<Replacement> NewReplacement;
+ // An existing replacement in a replacements set that is causing problem.
+ llvm::Optional<Replacement> ExistingReplacement;
+};
+
/// \brief Less-than operator between two Replacements.
bool operator<(const Replacement &LHS, const Replacement &RHS);
diff --git a/lib/Tooling/Core/Replacement.cpp b/lib/Tooling/Core/Replacement.cpp
index ff12ab7e49fc6c4b5c81fca78a37e7585dfbb0ad..e194b59a6e2b14df3f1fd076dcef342f9950178b 100644
--- a/lib/Tooling/Core/Replacement.cpp
+++ b/lib/Tooling/Core/Replacement.cpp
@@ -30,8 +30,7 @@ namespace tooling {
static const char * const InvalidLocation = "";
-Replacement::Replacement()
- : FilePath(InvalidLocation) {}
+Replacement::Replacement() : FilePath(InvalidLocation) {}
Replacement::Replacement(StringRef FilePath, unsigned Offset, unsigned Length,
StringRef ReplacementText)
@@ -144,14 +143,33 @@ Replacements::getReplacementInChangedCode(const Replacement &R) const {
R.getReplacementText());
}
-static llvm::Error makeConflictReplacementsError(const Replacement &New,
- const Replacement &Existing) {
- return llvm::make_error<llvm::StringError>(
- "New replacement:\n" + New.toString() +
- "\nconflicts with existing replacement:\n" + Existing.toString(),
- llvm::inconvertibleErrorCode());
+static std::string getReplacementErrString(replacement_error Err) {
+ switch (Err) {
+ case replacement_error::fail_to_apply:
+ return "Failed to apply a replacement.";
+ case replacement_error::wrong_file_path:
+ return "The new replacement's file path is different from the file path of "
+ "existing replacements";
+ case replacement_error::overlap_conflict:
+ return "The new replacement overlaps with an existing replacement.";
+ case replacement_error::insert_conflict:
+ return "The new insertion has the same insert location as an existing "
+ "replacement.";
+ }
+ llvm_unreachable("A value of replacement_error has no message.");
+}
+
+std::string ReplacementError::message() const {
+ std::string Message = getReplacementErrString(Err);
+ if (NewReplacement.hasValue())
+ Message += "\nNew replacement: " + NewReplacement->toString();
+ if (ExistingReplacement.hasValue())
+ Message += "\nExisting replacement: " + ExistingReplacement->toString();
+ return Message;
}
+char ReplacementError::ID = 0;
+
Replacements Replacements::getCanonicalReplacements() const {
std::vector<Replacement> NewReplaces;
// Merge adjacent replacements.
@@ -202,17 +220,15 @@ Replacements::mergeIfOrderIndependent(const Replacement &R) const {
if (MergeShiftedRs.getCanonicalReplacements() ==
MergeShiftedReplaces.getCanonicalReplacements())
return MergeShiftedRs;
- return makeConflictReplacementsError(R, *Replaces.begin());
+ return llvm::make_error<ReplacementError>(replacement_error::overlap_conflict,
+ R, *Replaces.begin());
}
llvm::Error Replacements::add(const Replacement &R) {
// Check the file path.
if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
- return llvm::make_error<llvm::StringError>(
- "All replacements must have the same file path. New replacement: " +
- R.getFilePath() + ", existing replacements: " +
- Replaces.begin()->getFilePath() + "\n",
- llvm::inconvertibleErrorCode());
+ return llvm::make_error<ReplacementError>(
+ replacement_error::wrong_file_path, R, *Replaces.begin());
// Special-case header insertions.
if (R.getOffset() == UINT_MAX) {
@@ -239,9 +255,9 @@ llvm::Error Replacements::add(const Replacement &R) {
// Check if two insertions are order-indepedent: if inserting them in
// either order produces the same text, they are order-independent.
if ((R.getReplacementText() + I->getReplacementText()).str() !=
- (I->getReplacementText() + R.getReplacementText()).str()) {
- return makeConflictReplacementsError(R, *I);
- }
+ (I->getReplacementText() + R.getReplacementText()).str())
+ return llvm::make_error<ReplacementError>(
+ replacement_error::insert_conflict, R, *I);
// If insertions are order-independent, we can merge them.
Replacement NewR(
R.getFilePath(), R.getOffset(), 0,
@@ -555,9 +571,8 @@ llvm::Expected<std::string> applyAllReplacements(StringRef Code,
Replacement Replace("<stdin>", I->getOffset(), I->getLength(),
I->getReplacementText());
if (!Replace.apply(Rewrite))
- return llvm::make_error<llvm::StringError>(
- "Failed to apply replacement: " + Replace.toString(),
- llvm::inconvertibleErrorCode());
+ return llvm::make_error<ReplacementError>(
+ replacement_error::fail_to_apply, Replace);
}
std::string Result;
llvm::raw_string_ostream OS(Result);
diff --git a/unittests/Tooling/RefactoringTest.cpp b/unittests/Tooling/RefactoringTest.cpp
index c9149a2669854418870e7b8f6741f04375a18afc..c29f8d7d71230ddb974ed21b99517c3d5e75c14a 100644
--- a/unittests/Tooling/RefactoringTest.cpp
+++ b/unittests/Tooling/RefactoringTest.cpp
@@ -100,21 +100,71 @@ TEST_F(ReplacementTest, ReturnsInvalidPath) {
EXPECT_TRUE(Replace2.getFilePath().empty());
}
+// Checks that an llvm::Error instance contains a ReplacementError with expected
+// error code, expected new replacement, and expected existing replacement.
+static bool checkReplacementError(
+ llvm::Error&& Error, replacement_error ExpectedErr,
+ llvm::Optional<Replacement> ExpectedExisting,
+ llvm::Optional<Replacement> ExpectedNew) {
+ if (!Error) {
+ llvm::errs() << "Error is a success.";
+ return false;
+ }
+ std::string ErrorMessage;
+ llvm::raw_string_ostream OS(ErrorMessage);
+ llvm::handleAllErrors(std::move(Error), [&](const ReplacementError &RE) {
+ llvm::errs() << "Handling error...\n";
+ if (ExpectedErr != RE.get())
+ OS << "Unexpected error code: " << int(RE.get()) << "\n";
+ if (ExpectedExisting != RE.getExistingReplacement()) {
+ OS << "Expected Existing != Actual Existing.\n";
+ if (ExpectedExisting.hasValue())
+ OS << "Expected existing replacement: " << ExpectedExisting->toString()
+ << "\n";
+ if (RE.getExistingReplacement().hasValue())
+ OS << "Actual existing replacement: "
+ << RE.getExistingReplacement()->toString() << "\n";
+ }
+ if (ExpectedNew != RE.getNewReplacement()) {
+ OS << "Expected New != Actual New.\n";
+ if (ExpectedNew.hasValue())
+ OS << "Expected new replacement: " << ExpectedNew->toString() << "\n";
+ if (RE.getNewReplacement().hasValue())
+ OS << "Actual new replacement: " << RE.getNewReplacement()->toString()
+ << "\n";
+ }
+ });
+ OS.flush();
+ if (ErrorMessage.empty()) return true;
+ llvm::errs() << ErrorMessage;
+ return false;
+}
+
TEST_F(ReplacementTest, FailAddReplacements) {
Replacements Replaces;
Replacement Deletion("x.cc", 0, 10, "3");
auto Err = Replaces.add(Deletion);
EXPECT_TRUE(!Err);
llvm::consumeError(std::move(Err));
- Err = Replaces.add(Replacement("x.cc", 0, 2, "a"));
- EXPECT_TRUE((bool)Err);
- llvm::consumeError(std::move(Err));
- Err = Replaces.add(Replacement("x.cc", 2, 2, "a"));
- EXPECT_TRUE((bool)Err);
- llvm::consumeError(std::move(Err));
- Err = Replaces.add(Replacement("y.cc", 20, 2, ""));
- EXPECT_TRUE((bool)Err);
- llvm::consumeError(std::move(Err));
+
+ Replacement OverlappingReplacement("x.cc", 0, 2, "a");
+ Err = Replaces.add(OverlappingReplacement);
+ EXPECT_TRUE(checkReplacementError(std::move(Err),
+ replacement_error::overlap_conflict,
+ Deletion, OverlappingReplacement));
+
+ Replacement ContainedReplacement("x.cc", 2, 2, "a");
+ Err = Replaces.add(Replacement(ContainedReplacement));
+ EXPECT_TRUE(checkReplacementError(std::move(Err),
+ replacement_error::overlap_conflict,
+ Deletion, ContainedReplacement));
+
+ Replacement WrongPathReplacement("y.cc", 20, 2, "");
+ Err = Replaces.add(WrongPathReplacement);
+ EXPECT_TRUE(checkReplacementError(std::move(Err),
+ replacement_error::wrong_file_path,
+ Deletion, WrongPathReplacement));
+
EXPECT_EQ(1u, Replaces.size());
EXPECT_EQ(Deletion, *Replaces.begin());
}
@@ -299,9 +349,11 @@ TEST_F(ReplacementTest, FailAddRegression) {
// Make sure we find the overlap with the first entry when inserting a
// replacement that ends exactly at the seam of the existing replacements.
- Err = Replaces.add(Replacement("x.cc", 5, 5, "fail"));
- EXPECT_TRUE((bool)Err);
- llvm::consumeError(std::move(Err));
+ Replacement OverlappingReplacement("x.cc", 5, 5, "fail");
+ Err = Replaces.add(OverlappingReplacement);
+ EXPECT_TRUE(checkReplacementError(std::move(Err),
+ replacement_error::overlap_conflict,
+ *Replaces.begin(), OverlappingReplacement));
Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
EXPECT_TRUE(!Err);
@@ -333,9 +385,11 @@ TEST_F(ReplacementTest, AddInsertAtOtherInsertWhenOderIndependent) {
auto Err = Replaces.add(Replacement("x.cc", 10, 0, "a"));
EXPECT_TRUE(!Err);
llvm::consumeError(std::move(Err));
- Err = Replaces.add(Replacement("x.cc", 10, 0, "b"));
- EXPECT_TRUE((bool)Err);
- llvm::consumeError(std::move(Err));
+ Replacement ConflictInsertion("x.cc", 10, 0, "b");
+ Err = Replaces.add(ConflictInsertion);
+ EXPECT_TRUE(checkReplacementError(std::move(Err),
+ replacement_error::insert_conflict,
+ *Replaces.begin(), ConflictInsertion));
Replaces.clear();
Err = Replaces.add(Replacement("x.cc", 10, 0, "a"));
@@ -436,10 +490,12 @@ TEST_F(ReplacementTest, FailOrderDependentReplacements) {
auto Replaces = toReplacements({Replacement(
Context.Sources, Context.getLocation(ID, 2, 1), 5, "other")});
- auto Err = Replaces.add(Replacement(
- Context.Sources, Context.getLocation(ID, 2, 1), 5, "rehto"));
- EXPECT_TRUE((bool)Err);
- llvm::consumeError(std::move(Err));
+ Replacement ConflictReplacement(Context.Sources,
+ Context.getLocation(ID, 2, 1), 5, "rehto");
+ auto Err = Replaces.add(ConflictReplacement);
+ EXPECT_TRUE(checkReplacementError(std::move(Err),
+ replacement_error::overlap_conflict,
+ *Replaces.begin(), ConflictReplacement));
EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite));
EXPECT_EQ("line1\nother\nline3\nline4", Context.getRewrittenText(ID));