Index: include-fixer/IncludeFixer.h =================================================================== --- include-fixer/IncludeFixer.h +++ include-fixer/IncludeFixer.h @@ -68,8 +68,9 @@ /// \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 createInsertHeaderReplacements( StringRef Code, StringRef FilePath, StringRef Header, const clang::format::FormatStyle &Style = clang::format::getLLVMStyle()); Index: include-fixer/IncludeFixer.cpp =================================================================== --- include-fixer/IncludeFixer.cpp +++ include-fixer/IncludeFixer.cpp @@ -349,7 +349,7 @@ return !Compiler.getDiagnostics().hasFatalErrorOccurred(); } -tooling::Replacements +llvm::Expected createInsertHeaderReplacements(StringRef Code, StringRef FilePath, StringRef Header, const clang::format::FormatStyle &Style) { @@ -360,8 +360,10 @@ 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 Index: include-fixer/tool/ClangIncludeFixer.cpp =================================================================== --- include-fixer/tool/ClangIncludeFixer.cpp +++ include-fixer/tool/ClangIncludeFixer.cpp @@ -209,13 +209,20 @@ 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; } @@ -250,17 +257,22 @@ 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( + /*Code=*/Buffer.get()->getBuffer(), FilePath, + Context.getHeaders().front(), InsertStyle); + 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 DiagOpts(new DiagnosticOptions); @@ -270,15 +282,19 @@ 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(); } Index: unittests/clang-tidy/ClangTidyTest.h =================================================================== --- unittests/clang-tidy/ClangTidyTest.h +++ unittests/clang-tidy/ClangTidyTest.h @@ -17,6 +17,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Tooling.h" +#include "llvm/ADT/Optional.h" #include #include @@ -121,7 +122,13 @@ 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) \ Index: unittests/include-fixer/IncludeFixerTest.cpp =================================================================== --- unittests/include-fixer/IncludeFixerTest.cpp +++ unittests/include-fixer/IncludeFixerTest.cpp @@ -86,14 +86,17 @@ 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( + Code, FakeFileName, FixerContext.getHeaders().front()); + EXPECT_TRUE(static_cast(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); }