[include-fixer] Support processing multiple files in one run.

Summary:
Previously, if we pass multiple files or a file pattern (e.g. /path/to/*.cc) to
include-fixer, include-fixer will apply all replacements to the first argument,
which probably causes crashes.

With this patch, include-fixer can process multiple files now.

Vim and Emacs integration are tested manually.

Reviewers: bkramer

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D23266

llvm-svn: 278102

clang-tools-extra/include-fixer/IncludeFixer.cpp

@@ -37,6 +37,7 @@ class Action : public clang::ASTFrontendAction,
   std::unique_ptr<clang::ASTConsumer>
   CreateASTConsumer(clang::CompilerInstance &Compiler,
                     StringRef InFile) override {
+    FilePath = InFile;
     return llvm::make_unique<clang::ASTConsumer>();
   }
 
@@ -228,7 +229,7 @@ class Action : public clang::ASTFrontendAction,
                                     Symbol.getContexts(),
                                     Symbol.getNumOccurrences());
     }
-    return IncludeFixerContext(QuerySymbolInfos, SymbolCandidates);
+    return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates);
   }
 
 private:
@@ -297,16 +298,20 @@ class Action : public clang::ASTFrontendAction,
   /// recovery.
   std::vector<find_all_symbols::SymbolInfo> MatchedSymbols;
 
+  /// The file path to the file being processed.
+  std::string FilePath;
+
   /// Whether we should use the smallest possible include path.
   bool MinimizeIncludePaths = true;
 };
 
 } // namespace
 
 IncludeFixerActionFactory::IncludeFixerActionFactory(
-    SymbolIndexManager &SymbolIndexMgr, IncludeFixerContext &Context,
-    StringRef StyleName, bool MinimizeIncludePaths)
-    : SymbolIndexMgr(SymbolIndexMgr), Context(Context),
+    SymbolIndexManager &SymbolIndexMgr,
+    std::vector<IncludeFixerContext> &Contexts, StringRef StyleName,
+    bool MinimizeIncludePaths)
+    : SymbolIndexMgr(SymbolIndexMgr), Contexts(Contexts),
       MinimizeIncludePaths(MinimizeIncludePaths) {}
 
 IncludeFixerActionFactory::~IncludeFixerActionFactory() = default;
@@ -337,9 +342,9 @@ bool IncludeFixerActionFactory::runInvocation(
       llvm::make_unique<Action>(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 @@ bool IncludeFixerActionFactory::runInvocation(
 }
 
 llvm::Expected<tooling::Replacements> 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.

clang-tools-extra/include-fixer/IncludeFixer.h

@@ -34,7 +34,8 @@ class IncludeFixerActionFactory : public clang::tooling::ToolAction {
   /// \param StyleName Fallback style for reformatting.
   /// \param MinimizeIncludePaths whether inserted include paths are optimized.
   IncludeFixerActionFactory(SymbolIndexManager &SymbolIndexMgr,
-                            IncludeFixerContext &Context, StringRef StyleName,
+                            std::vector<IncludeFixerContext> &Contexts,
+                            StringRef StyleName,
                             bool MinimizeIncludePaths = true);
 
   ~IncludeFixerActionFactory() override;
@@ -49,8 +50,8 @@ class IncludeFixerActionFactory : public clang::tooling::ToolAction {
   /// The client to use to find cross-references.
   SymbolIndexManager &SymbolIndexMgr;
 
-  /// The context that contains all information about the symbol being queried.
-  IncludeFixerContext &Context;
+  /// Multiple contexts for files being processed.
+  std::vector<IncludeFixerContext> &Contexts;
 
   /// Whether inserted include paths should be optimized.
   bool MinimizeIncludePaths;
@@ -65,7 +66,6 @@ class IncludeFixerActionFactory : public clang::tooling::ToolAction {
 /// 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 @@ class IncludeFixerActionFactory : public clang::tooling::ToolAction {
 /// qualifiers on success; otherwise, an llvm::Error carrying llvm::StringError
 /// is returned.
 llvm::Expected<tooling::Replacements> createIncludeFixerReplacements(
-    StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
+    StringRef Code, const IncludeFixerContext &Context,
     const format::FormatStyle &Style = format::getLLVMStyle(),
     bool AddQualifiers = true);
 

clang-tools-extra/include-fixer/IncludeFixerContext.cpp

@@ -76,9 +76,9 @@ std::string createQualifiedNameForReplacement(
 } // anonymous namespace
 
 IncludeFixerContext::IncludeFixerContext(
-    std::vector<QuerySymbolInfo> QuerySymbols,
+    StringRef FilePath, std::vector<QuerySymbolInfo> QuerySymbols,
     std::vector<find_all_symbols::SymbolInfo> Symbols)
-    : QuerySymbolInfos(std::move(QuerySymbols)),
+    : FilePath(FilePath), QuerySymbolInfos(std::move(QuerySymbols)),
       MatchedSymbols(std::move(Symbols)) {
   // Remove replicated QuerySymbolInfos with the same range.
   //

clang-tools-extra/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 a file being processed. It includes all query
+/// information, e.g. symbols being queried in database, all header candidates.
 class IncludeFixerContext {
 public:
   struct HeaderInfo {
@@ -46,7 +47,8 @@ class IncludeFixerContext {
   };
 
   IncludeFixerContext() = default;
-  IncludeFixerContext(std::vector<QuerySymbolInfo> QuerySymbols,
+  IncludeFixerContext(StringRef FilePath,
+                      std::vector<QuerySymbolInfo> QuerySymbols,
                       std::vector<find_all_symbols::SymbolInfo> Symbols);
 
   /// \brief Get symbol name.
@@ -59,6 +61,9 @@ class IncludeFixerContext {
     return QuerySymbolInfos.front().Range;
   }
 
+  /// \brief Get the file path to the file being processed.
+  StringRef getFilePath() const { return FilePath; }
+
   /// \brief Get header information.
   const std::vector<HeaderInfo> &getHeaderInfos() const { return HeaderInfos; }
 
@@ -70,6 +75,9 @@ class IncludeFixerContext {
 private:
   friend struct llvm::yaml::MappingTraits<IncludeFixerContext>;
 
+  /// \brief The file path to the file being processed.
+  std::string FilePath;
+
   /// \brief All instances of an unidentified symbol being queried.
   std::vector<QuerySymbolInfo> QuerySymbolInfos;
 

clang-tools-extra/include-fixer/tool/ClangIncludeFixer.cpp

@@ -73,6 +73,7 @@ template <> struct MappingTraits<IncludeFixerContext> {
   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 @@ createSymbolIndexManager(StringRef FilePath) {
 
 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 @@ int includeFixerMain(int argc, const char **argv) {
     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 @@ int includeFixerMain(int argc, const char **argv) {
            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 @@ int includeFixerMain(int argc, const char **argv) {
     return 1;
 
   // Now run our tool.
-  include_fixer::IncludeFixerContext Context;
-  include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Context,
+  std::vector<include_fixer::IncludeFixerContext> Contexts;
+  include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Contexts,
                                                    Style, MinimizeIncludePaths);
 
   if (tool.run(&Factory) != 0) {
@@ -326,43 +327,49 @@ int includeFixerMain(int argc, const char **argv) {
     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<tooling::Replacements> 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<DiagnosticOptions> 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,21 @@ int includeFixerMain(int argc, const char **argv) {
     return 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);
+
   // 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.\n";
+      return 1;
+    }
+  }
   return Rewrites.overwriteChangedFiles();
 }
 

clang-tools-extra/include-fixer/tool/clang-include-fixer.py

@@ -149,21 +149,16 @@ def main():
     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 = [
-      header for header in header_infos if header["Header"] == selected]
-
-    # Insert a selected header.
-    InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
-                             "HeaderInfos": selected_header_infos}, text)
+    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
+
+    InsertHeaderToVimBuffer(include_fixer_context, text)
     print "Added #include {0} for {1}.".format(selected, symbol)
   except Exception as error:
     print >> sys.stderr, error.message

clang-tools-extra/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\"",

clang-tools-extra/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;

clang-tools-extra/unittests/include-fixer/IncludeFixerTest.cpp

@@ -88,15 +88,15 @@ static std::string runIncludeFixer(
   SymbolIndexMgr->addSymbolIndex(
       llvm::make_unique<include_fixer::InMemorySymbolIndex>(Symbols));
 
-  IncludeFixerContext FixerContext;
-  IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
-
+  std::vector<IncludeFixerContext> 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<bool>(Replaces))
       << llvm::toString(Replaces.takeError()) << "\n";
   if (!Replaces)