Changes related to tooling::applyAllReplacements interface change in D21601.

[ioeric]

Summary

this patch contains changes related to the interface change from
http://reviews.llvm.org/D21601. Only submit this patch after D21601 is
submitted.

[ioeric]

• Changes corresponding to new interfaces: formatReplacements and cleanupAroundReplacements.

[ioeric]

• Addressed reviewer's recomments in the other patch.

[ioeric]

Ping

[ioeric]

• Always handle error.

[klimek]

LG. Nice!

include-fixer/IncludeFixer.cpp

366–367

Just return formatReplacements(...)?

[ioeric]

• Addressed reviewer's comment.

[ioeric]

• Merged with origin/master. Resolved a few conflicts.

include-fixer/IncludeFixer.h

 /// Create replacements, which are generated by clang-format, for the header
 /// insertion.
 ///
 /// \param Code The source code.
 /// \param FilePath The source file path.
 /// \param Header The header being inserted.
 /// \param Style clang-format style being used.
 ///
-/// \return Replacements for inserting and sorting headers.
-tooling::Replacements createInsertHeaderReplacements(
+/// \return Replacements for inserting and sorting headers on success;
+/// otherwise, an llvm::Error carrying llvm::StringError is returned.
+llvm::Expected<tooling::Replacements> createInsertHeaderReplacements(
     StringRef Code, StringRef FilePath, StringRef Header,
     const clang::format::FormatStyle &Style = clang::format::getLLVMStyle());
 
 } // namespace include_fixer
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXER_H

include-fixer/IncludeFixer.cpp

Context = ScopedToolAction->getIncludeFixerContext(
       Compiler.getPreprocessor().getHeaderSearchInfo());
 
   // Technically this should only return true if we're sure that we have a
   // parseable file. We don't know that though. Only inform users of fatal
   // errors.
   return !Compiler.getDiagnostics().hasFatalErrorOccurred();
 }
 
-tooling::Replacements
+llvm::Expected<tooling::Replacements>
 createInsertHeaderReplacements(StringRef Code, StringRef FilePath,
                                StringRef Header,
                                const clang::format::FormatStyle &Style) {
   if (Header.empty())
     return tooling::Replacements();
   std::string IncludeName = "#include " + Header.str() + "\n";
   // Create replacements for the new header.
   clang::tooling::Replacements Insertions = {
       tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)};
 
-  return formatReplacements(
-      Code, cleanupAroundReplacements(Code, Insertions, Style), Style);
+  auto CleanReplaces = cleanupAroundReplacements(Code, Insertions, Style);
+  if (!CleanReplaces)
+    return CleanReplaces;
+  return formatReplacements(Code, *CleanReplaces, Style);
 }
 
 } // namespace include_fixer
 } // namespace clang

include-fixer/tool/ClangIncludeFixer.cpp

if (!InsertHeader.empty()) {
     IncludeFixerContext Context;
     yin >> Context;
 
     if (Context.getHeaders().size() != 1) {
       errs() << "Expect exactly one inserted header.\n";
       return 1;
     }
 
-    tooling::Replacements Replacements =
-        clang::include_fixer::createInsertHeaderReplacements(
-            Code->getBuffer(), FilePath, Context.getHeaders().front(),
-            InsertStyle);
-    std::string ChangedCode =
-        tooling::applyAllReplacements(Code->getBuffer(), Replacements);
-    llvm::outs() << ChangedCode;
+    auto Replacements = clang::include_fixer::createInsertHeaderReplacements(
+        Code->getBuffer(), FilePath, Context.getHeaders().front(), InsertStyle);
+    if (!Replacements) {
+      errs() << "Failed to create header insertion replacement: "
+             << llvm::toString(Replacements.takeError()) << "\n";
+      return 1;
+    }
+    auto ChangedCode =
+        tooling::applyAllReplacements(Code->getBuffer(), *Replacements);
+    if (!ChangedCode) {
+      llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
+      return 1;
+    }
+    llvm::outs() << *ChangedCode;
     return 0;
   }
 
   // Set up data source.
   std::unique_ptr<include_fixer::SymbolIndexManager> SymbolIndexMgr =
       createSymbolIndexManager(options.getSourcePathList().front());
   if (!SymbolIndexMgr)
     return 1;
if (Context.getMatchedSymbols().empty())
     return 0;
 
   auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
   if (!Buffer) {
     errs() << "Couldn't open file: " << FilePath;
     return 1;
   }
 
-  tooling::Replacements Replacements =
-      clang::include_fixer::createInsertHeaderReplacements(
-          /*Code=*/Buffer.get()->getBuffer(), FilePath,
-          Context.getHeaders().front(), InsertStyle);
+  // FIXME: Rank the results and pick the best one instead of the first one.
+  auto Replacements = clang::include_fixer::createInsertHeaderReplacements(
+  if (!Replacements) {
+    errs() << "Failed to create header insertion replacement: "
+           << llvm::toString(Replacements.takeError()) << "\n";
+    return 1;
+  }
 
   if (!Quiet)
     llvm::errs() << "Added #include" << Context.getHeaders().front();
 
   // Add missing namespace qualifiers to the unidentified symbol.
   if (Context.getSymbolRange().getLength() > 0)
-    Replacements.insert(Context.createSymbolReplacement(FilePath, 0));
+    Replacements->insert(Context.createSymbolReplacement(FilePath, 0));
 
   // Set up a new source manager for applying the resulting replacements.
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);
   DiagnosticsEngine Diagnostics(new DiagnosticIDs, &*DiagOpts);
   TextDiagnosticPrinter DiagnosticPrinter(outs(), &*DiagOpts);
   SourceManager SM(Diagnostics, tool.getFiles());
   Diagnostics.setClient(&DiagnosticPrinter, false);
 
   if (STDINMode) {
-    std::string ChangedCode =
-        tooling::applyAllReplacements(Code->getBuffer(), Replacements);
-    llvm::outs() << ChangedCode;
+    auto ChangedCode =
+        tooling::applyAllReplacements(Code->getBuffer(), *Replacements);
+    if (!ChangedCode) {
+      llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
+      return 1;
+    }
+    llvm::outs() << *ChangedCode;
     return 0;
   }
 
   // Write replacements to disk.
   Rewriter Rewrites(SM, LangOptions());
-  tooling::applyAllReplacements(Replacements, Rewrites);
+  tooling::applyAllReplacements(*Replacements, Rewrites);
   return Rewrites.overwriteChangedFiles();
 }
 
 } // namespace
 
 int main(int argc, const char **argv) {
   return includeFixerMain(argc, argv);
 }

unittests/clang-tidy/ClangTidyTest.h

 
 #include "ClangTidy.h"
 #include "ClangTidyDiagnosticConsumer.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include <map>
 #include <memory>
 
 namespace clang {
 namespace tidy {
 namespace test {
 
 class TestClangTidyAction : public ASTFrontendAction {
runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
   }
 
   DiagConsumer.finish();
   tooling::Replacements Fixes;
   for (const ClangTidyError &Error : Context.getErrors())
     Fixes.insert(Error.Fix.begin(), Error.Fix.end());
   if (Errors)
     *Errors = Context.getErrors();
-  return tooling::applyAllReplacements(Code, Fixes);
+  auto Result = tooling::applyAllReplacements(Code, Fixes);
+  if (!Result) {
+    // FIXME: propogate the error.
+    llvm::consumeError(Result.takeError());
+    return "";
+  }
+  return *Result;
 }
 
 #define EXPECT_NO_CHANGES(Check, Code)                                         \
   EXPECT_EQ(Code, runCheckOnCode<Check>(Code))
 
 } // namespace test
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANG_TIDY_CLANGTIDYTEST_H

unittests/include-fixer/IncludeFixerTest.cpp

static std::string runIncludeFixer(
 
   IncludeFixerContext FixerContext;
   IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
 
   std::string FakeFileName = "input.cc";
   runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
   if (FixerContext.getMatchedSymbols().empty())
     return Code;
-  tooling::Replacements Replacements =
-      clang::include_fixer::createInsertHeaderReplacements(
-          Code, FakeFileName, FixerContext.getHeaders().front());
+  auto Replaces = clang::include_fixer::createInsertHeaderReplacements(
+  EXPECT_TRUE(static_cast<bool>(Replaces))
+      << llvm::toString(Replaces.takeError()) << "\n";
+  if (!Replaces)
+    return "";
   clang::RewriterTestContext Context;
   clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
   if (FixerContext.getSymbolRange().getLength() > 0)
-    Replacements.insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
-  clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
+    Replaces->insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
+  clang::tooling::applyAllReplacements(*Replaces, Context.Rewrite);
   return Context.getRewrittenText(ID);
 }
 
 TEST(IncludeFixer, Typo) {
   EXPECT_EQ("#include <string>\nstd::string foo;\n",
             runIncludeFixer("std::string foo;\n"));
 
   // FIXME: the current version of include-fixer does not get this test case