Index: include-fixer/IncludeFixer.h =================================================================== --- include-fixer/IncludeFixer.h +++ include-fixer/IncludeFixer.h @@ -34,7 +34,8 @@ /// \param StyleName Fallback style for reformatting. /// \param MinimizeIncludePaths whether inserted include paths are optimized. IncludeFixerActionFactory(SymbolIndexManager &SymbolIndexMgr, - IncludeFixerContext &Context, StringRef StyleName, + std::vector &Contexts, + StringRef StyleName, bool MinimizeIncludePaths = true); ~IncludeFixerActionFactory() override; @@ -50,7 +51,7 @@ SymbolIndexManager &SymbolIndexMgr; /// The context that contains all information about the symbol being queried. - IncludeFixerContext &Context; + std::vector &Contexts; /// Whether inserted include paths should be optimized. bool MinimizeIncludePaths; @@ -65,7 +66,6 @@ /// first header for insertion. /// /// \param Code The source code. -/// \param FilePath The source file path. /// \param Context The context which contains all information for creating /// include-fixer replacements. /// \param Style clang-format style being used. @@ -76,7 +76,7 @@ /// qualifiers on success; otherwise, an llvm::Error carrying llvm::StringError /// is returned. llvm::Expected createIncludeFixerReplacements( - StringRef Code, StringRef FilePath, const IncludeFixerContext &Context, + StringRef Code, const IncludeFixerContext &Context, const format::FormatStyle &Style = format::getLLVMStyle(), bool AddQualifiers = true); Index: include-fixer/IncludeFixer.cpp =================================================================== --- include-fixer/IncludeFixer.cpp +++ include-fixer/IncludeFixer.cpp @@ -37,6 +37,7 @@ std::unique_ptr CreateASTConsumer(clang::CompilerInstance &Compiler, StringRef InFile) override { + FilePath = InFile; return llvm::make_unique(); } @@ -228,7 +229,7 @@ Symbol.getContexts(), Symbol.getNumOccurrences()); } - return IncludeFixerContext(QuerySymbolInfos, SymbolCandidates); + return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates); } private: @@ -297,6 +298,9 @@ /// recovery. std::vector MatchedSymbols; + /// The absolute path to the file being processed. + std::string FilePath; + /// Whether we should use the smallest possible include path. bool MinimizeIncludePaths = true; }; @@ -304,9 +308,10 @@ } // namespace IncludeFixerActionFactory::IncludeFixerActionFactory( - SymbolIndexManager &SymbolIndexMgr, IncludeFixerContext &Context, - StringRef StyleName, bool MinimizeIncludePaths) - : SymbolIndexMgr(SymbolIndexMgr), Context(Context), + SymbolIndexManager &SymbolIndexMgr, + std::vector &Contexts, StringRef StyleName, + bool MinimizeIncludePaths) + : SymbolIndexMgr(SymbolIndexMgr), Contexts(Contexts), MinimizeIncludePaths(MinimizeIncludePaths) {} IncludeFixerActionFactory::~IncludeFixerActionFactory() = default; @@ -337,9 +342,9 @@ llvm::make_unique(SymbolIndexMgr, MinimizeIncludePaths); Compiler.ExecuteAction(*ScopedToolAction); - Context = ScopedToolAction->getIncludeFixerContext( + Contexts.push_back(ScopedToolAction->getIncludeFixerContext( Compiler.getSourceManager(), - Compiler.getPreprocessor().getHeaderSearchInfo()); + 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 @@ -348,10 +353,11 @@ } llvm::Expected createIncludeFixerReplacements( - StringRef Code, StringRef FilePath, const IncludeFixerContext &Context, + StringRef Code, const IncludeFixerContext &Context, const clang::format::FormatStyle &Style, bool AddQualifiers) { if (Context.getHeaderInfos().empty()) return tooling::Replacements(); + StringRef FilePath = Context.getFilePath(); std::string IncludeName = "#include " + Context.getHeaderInfos().front().Header + "\n"; // Create replacements for the new header. Index: include-fixer/IncludeFixerContext.h =================================================================== --- include-fixer/IncludeFixerContext.h +++ include-fixer/IncludeFixerContext.h @@ -18,7 +18,8 @@ namespace clang { namespace include_fixer { -/// \brief A context for the symbol being queried. +/// \brief A context for the file being processed. It includes all query +/// information, e.g. symbols being queried in database, all header candiates. class IncludeFixerContext { public: struct HeaderInfo { @@ -46,7 +47,8 @@ }; IncludeFixerContext() = default; - IncludeFixerContext(std::vector QuerySymbols, + IncludeFixerContext(StringRef FilePath, + std::vector QuerySymbols, std::vector Symbols); /// \brief Get symbol name. @@ -59,6 +61,9 @@ return QuerySymbolInfos.front().Range; } + /// \brief Get the absolute file path. + StringRef getFilePath() const { return FilePath; } + /// \brief Get header information. const std::vector &getHeaderInfos() const { return HeaderInfos; } @@ -70,6 +75,9 @@ private: friend struct llvm::yaml::MappingTraits; + /// \brief The absolute path to the file being processed. + std::string FilePath; + /// \brief All instances of an unidentified symbol being queried. std::vector QuerySymbolInfos; Index: include-fixer/IncludeFixerContext.cpp =================================================================== --- include-fixer/IncludeFixerContext.cpp +++ include-fixer/IncludeFixerContext.cpp @@ -76,9 +76,9 @@ } // anonymous namespace IncludeFixerContext::IncludeFixerContext( - std::vector QuerySymbols, + StringRef FilePath, std::vector QuerySymbols, std::vector Symbols) - : QuerySymbolInfos(std::move(QuerySymbols)), + : FilePath(FilePath), QuerySymbolInfos(std::move(QuerySymbols)), MatchedSymbols(std::move(Symbols)) { // Remove replicated QuerySymbolInfos with the same range. // Index: include-fixer/tool/ClangIncludeFixer.cpp =================================================================== --- include-fixer/tool/ClangIncludeFixer.cpp +++ include-fixer/tool/ClangIncludeFixer.cpp @@ -73,6 +73,7 @@ static void mapping(IO &IO, IncludeFixerContext &Context) { IO.mapRequired("QuerySymbolInfos", Context.QuerySymbolInfos); IO.mapRequired("HeaderInfos", Context.HeaderInfos); + IO.mapRequired("FilePath", Context.FilePath); } }; } // namespace yaml @@ -203,7 +204,9 @@ void writeToJson(llvm::raw_ostream &OS, const IncludeFixerContext& Context) { OS << "{\n" - " \"QuerySymbolInfos\": [\n"; + << " \"FilePath\": \"" + << llvm::yaml::escape(Context.getFilePath()) << "\",\n" + << " \"QuerySymbolInfos\": [\n"; for (const auto &Info : Context.getQuerySymbolInfos()) { OS << " {\"RawIdentifier\": \"" << Info.RawIdentifier << "\",\n"; OS << " \"Range\":{"; @@ -251,9 +254,6 @@ tool.mapVirtualFile(options.getSourcePathList().front(), Code->getBuffer()); } - StringRef FilePath = options.getSourcePathList().front(); - format::FormatStyle InsertStyle = format::getStyle("file", FilePath, Style); - if (!InsertHeader.empty()) { if (!STDINMode) { errs() << "Should be running in STDIN mode\n"; @@ -289,9 +289,10 @@ const IncludeFixerContext::HeaderInfo &RHS) { return LHS.QualifiedName == RHS.QualifiedName; }); - + format::FormatStyle InsertStyle = + format::getStyle("file", Context.getFilePath(), Style); auto Replacements = clang::include_fixer::createIncludeFixerReplacements( - Code->getBuffer(), FilePath, Context, InsertStyle, + Code->getBuffer(), Context, InsertStyle, /*AddQualifiers=*/IsUniqueQualifiedName); if (!Replacements) { errs() << "Failed to create replacements: " @@ -316,8 +317,8 @@ return 1; // Now run our tool. - include_fixer::IncludeFixerContext Context; - include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Context, + std::vector Contexts; + include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Contexts, Style, MinimizeIncludePaths); if (tool.run(&Factory) != 0) { @@ -326,43 +327,49 @@ return 1; } + assert(!Contexts.empty()); + if (OutputHeaders) { - writeToJson(llvm::outs(), Context); + // FIXME: Print contexts of all processing files instead of the first one. + writeToJson(llvm::outs(), Contexts.front()); return 0; } - if (Context.getHeaderInfos().empty()) - return 0; + std::vector FixerReplacements; + for (const auto &Context : Contexts) { + StringRef FilePath = Context.getFilePath(); + format::FormatStyle InsertStyle = format::getStyle("file", FilePath, Style); + auto Buffer = llvm::MemoryBuffer::getFile(FilePath); + if (!Buffer) { + errs() << "Couldn't open file: " + FilePath.str() + ": " + << Buffer.getError().message() + "\n"; + return 1; + } - auto Buffer = llvm::MemoryBuffer::getFile(FilePath); - if (!Buffer) { - errs() << "Couldn't open file: " << FilePath << ": " - << Buffer.getError().message() << '\n'; - return 1; + auto Replacements = clang::include_fixer::createIncludeFixerReplacements( + Buffer.get()->getBuffer(), Context, InsertStyle); + if (!Replacements) { + errs() << "Failed to create replacement: " + << llvm::toString(Replacements.takeError()) << "\n"; + return 1; + } + FixerReplacements.push_back(*Replacements); } - auto Replacements = clang::include_fixer::createIncludeFixerReplacements( - /*Code=*/Buffer.get()->getBuffer(), FilePath, Context, InsertStyle); - if (!Replacements) { - errs() << "Failed to create header insertion replacement: " - << llvm::toString(Replacements.takeError()) << "\n"; - return 1; + if (!Quiet) { + for (const auto &Context : Contexts) { + if (!Context.getHeaderInfos().empty()) { + llvm::errs() << "Added #include " + << Context.getHeaderInfos().front().Header << " for " + << Context.getFilePath() << "\n"; + } + } } - if (!Quiet) - llvm::errs() << "Added #include " << Context.getHeaderInfos().front().Header - << "\n"; - - // Set up a new source manager for applying the resulting replacements. - IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions); - DiagnosticsEngine Diagnostics(new DiagnosticIDs, &*DiagOpts); - TextDiagnosticPrinter DiagnosticPrinter(outs(), &*DiagOpts); - SourceManager SM(Diagnostics, tool.getFiles()); - Diagnostics.setClient(&DiagnosticPrinter, false); - if (STDINMode) { - auto ChangedCode = - tooling::applyAllReplacements(Code->getBuffer(), *Replacements); + assert(FixerReplacements.size() == 1); + auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), + FixerReplacements.front()); if (!ChangedCode) { llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n"; return 1; @@ -371,9 +378,22 @@ return 0; } + // Set up a new source manager for applying the resulting replacements. + IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions); + DiagnosticsEngine Diagnostics(new DiagnosticIDs, &*DiagOpts); + TextDiagnosticPrinter DiagnosticPrinter(outs(), &*DiagOpts); + SourceManager SM(Diagnostics, tool.getFiles()); + Diagnostics.setClient(&DiagnosticPrinter, false); + // Write replacements to disk. Rewriter Rewrites(SM, LangOptions()); - tooling::applyAllReplacements(*Replacements, Rewrites); + for (const auto Replacement : FixerReplacements) { + if (!tooling::applyAllReplacements(Replacement, Rewrites)) { + llvm::errs() << "Failed to apply replacements for " + << Replacement.begin()->getFilePath() << '\n'; + return 1; + } + } return Rewrites.overwriteChangedFiles(); } Index: include-fixer/tool/clang-include-fixer.py =================================================================== --- include-fixer/tool/clang-include-fixer.py +++ include-fixer/tool/clang-include-fixer.py @@ -149,21 +149,16 @@ return try: - # If there is only one suggested header, insert it directly. - if len(unique_headers) == 1 or maximum_suggested_headers == 1: - InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos, - "HeaderInfos": header_infos}, text) - print "Added #include {0} for {1}.".format(unique_headers[0], symbol) - return - - selected = GetUserSelection("choose a header file for {0}.".format(symbol), - unique_headers, maximum_suggested_headers) - selected_header_infos = [ + inserted_header_infos = header_infos + if len(unique_headers) > 1: + selected = GetUserSelection( + "choose a header file for {0}.".format(symbol), + unique_headers, maximum_suggested_headers) + inserted_header_infos = [ header for header in header_infos if header["Header"] == selected] + include_fixer_context["HeaderInfos"] = inserted_header_infos - # Insert a selected header. - InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos, - "HeaderInfos": selected_header_infos}, text) + InsertHeaderToVimBuffer(include_fixer_context, text) print "Added #include {0} for {1}.".format(selected, symbol) except Exception as error: print >> sys.stderr, error.message Index: test/include-fixer/commandline_options.cpp =================================================================== --- test/include-fixer/commandline_options.cpp +++ test/include-fixer/commandline_options.cpp @@ -1,9 +1,9 @@ // REQUIRES: shell // RUN: echo "foo f;" > %t.cpp // RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE -// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp -// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp +// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE +// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp +// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{FilePath: %t.cpp, QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp // // CHECK: "HeaderInfos": [ // CHECK-NEXT: {"Header": "\"foo.h\"", Index: test/include-fixer/multiple_fixes.cpp =================================================================== --- /dev/null +++ test/include-fixer/multiple_fixes.cpp @@ -0,0 +1,13 @@ +// REQUIRES: shell +// RUN: sed -e 's#//.*$##' %s > %t.cpp +// RUN: mkdir -p %T/include-fixer/multiple-fixes +// RUN: echo 'foo f;' > %T/include-fixer/multiple-fixes/foo.cpp +// RUN: echo 'bar b;' > %T/include-fixer/multiple-fixes/bar.cpp +// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h";bar= "bar.h"' %T/include-fixer/multiple-fixes/*.cpp -- +// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/bar.cpp %s -check-prefix=CHECK-BAR +// RUN: FileCheck -input-file=%T/include-fixer/multiple-fixes/foo.cpp %s -check-prefix=CHECK-FOO +// +// CHECK-FOO: #include "foo.h" +// CHECK-FOO: foo f; +// CHECK-BAR: #include "bar.h" +// CHECK-BAR: bar b; Index: unittests/include-fixer/IncludeFixerTest.cpp =================================================================== --- unittests/include-fixer/IncludeFixerTest.cpp +++ unittests/include-fixer/IncludeFixerTest.cpp @@ -88,15 +88,15 @@ SymbolIndexMgr->addSymbolIndex( llvm::make_unique(Symbols)); - IncludeFixerContext FixerContext; - IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm"); - + std::vector FixerContexts; + IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContexts, "llvm"); std::string FakeFileName = "input.cc"; runOnCode(&Factory, Code, FakeFileName, ExtraArgs); - if (FixerContext.getHeaderInfos().empty()) + assert(FixerContexts.size() == 1); + if (FixerContexts.front().getHeaderInfos().empty()) return Code; auto Replaces = clang::include_fixer::createIncludeFixerReplacements( - Code, FakeFileName, FixerContext); + Code, FixerContexts.front()); EXPECT_TRUE(static_cast(Replaces)) << llvm::toString(Replaces.takeError()) << "\n"; if (!Replaces)