[include-fixer] Add an option to minimize include paths.

This will always pick the shortest possible path based on -I options. Based
on the #include suggestion code for modules.

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@267868 91177308-0d34-0410-b5e6-96231b3b80d8

include-fixer/IncludeFixer.cpp

@@ -10,6 +10,7 @@
 #include "IncludeFixer.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Parse/ParseAST.h"
 #include "clang/Rewrite/Core/Rewriter.h"
@@ -59,7 +60,8 @@ private:
 class Action : public clang::ASTFrontendAction,
                public clang::ExternalSemaSource {
 public:
-  explicit Action(XrefsDB &Xrefs) : Xrefs(Xrefs) {}
+  explicit Action(XrefsDB &Xrefs, bool MinimizeIncludePaths)
+      : Xrefs(Xrefs), MinimizeIncludePaths(MinimizeIncludePaths) {}
 
   std::unique_ptr<clang::ASTConsumer>
   CreateASTConsumer(clang::CompilerInstance &Compiler,
@@ -147,13 +149,40 @@ public:
       FirstIncludeOffset = Offset;
   }
 
+  /// Get the minimal include for a given path.
+  std::string minimizeInclude(StringRef Include,
+                              clang::SourceManager &SourceManager,
+                              clang::HeaderSearch &HeaderSearch) {
+    if (!MinimizeIncludePaths)
+      return Include;
+
+    // Get the FileEntry for the include.
+    StringRef StrippedInclude = Include.trim("\"<>");
+    const FileEntry *Entry =
+        SourceManager.getFileManager().getFile(StrippedInclude);
+
+    // If the file doesn't exist return the path from the database.
+    // FIXME: This should never happen.
+    if (!Entry)
+      return Include;
+
+    bool IsSystem;
+    std::string Suggestion =
+        HeaderSearch.suggestPathToFileForDiagnostics(Entry, &IsSystem);
+
+    return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"';
+  }
+
   /// Generate replacements for the suggested includes.
   /// \return true if changes will be made, false otherwise.
   bool Rewrite(clang::SourceManager &SourceManager,
+               clang::HeaderSearch &HeaderSearch,
                std::vector<clang::tooling::Replacement> &replacements) {
     for (const auto &ToTry : Untried) {
-      DEBUG(llvm::dbgs() << "Adding include " << ToTry << "\n");
-      std::string ToAdd = "#include " + ToTry + "\n";
+      std::string ToAdd = "#include " +
+                          minimizeInclude(ToTry, SourceManager, HeaderSearch) +
+                          "\n";
+      DEBUG(llvm::dbgs() << "Adding " << ToAdd << "\n");
 
       if (FirstIncludeOffset == -1U)
         FirstIncludeOffset = 0;
@@ -228,6 +257,9 @@ private:
 
   /// Includes we have left to try.
   std::set<std::string> Untried;
+
+  /// Whether we should use the smallest possible include path.
+  bool MinimizeIncludePaths = true;
 };
 
 void PreprocessorHooks::FileChanged(clang::SourceLocation Loc,
@@ -271,8 +303,10 @@ void PreprocessorHooks::InclusionDirective(
 } // namespace
 
 IncludeFixerActionFactory::IncludeFixerActionFactory(
-    XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements)
-    : Xrefs(Xrefs), Replacements(Replacements) {}
+    XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements,
+    bool MinimizeIncludePaths)
+    : Xrefs(Xrefs), Replacements(Replacements),
+      MinimizeIncludePaths(MinimizeIncludePaths) {}
 
 IncludeFixerActionFactory::~IncludeFixerActionFactory() = default;
 
@@ -294,11 +328,14 @@ bool IncludeFixerActionFactory::runInvocation(
   Compiler.createSourceManager(*Files);
 
   // Run the parser, gather missing includes.
-  auto ScopedToolAction = llvm::make_unique<Action>(Xrefs);
+  auto ScopedToolAction =
+      llvm::make_unique<Action>(Xrefs, MinimizeIncludePaths);
   Compiler.ExecuteAction(*ScopedToolAction);
 
   // Generate replacements.
-  ScopedToolAction->Rewrite(Compiler.getSourceManager(), Replacements);
+  ScopedToolAction->Rewrite(Compiler.getSourceManager(),
+                            Compiler.getPreprocessor().getHeaderSearchInfo(),
+                            Replacements);
 
   // Technically this should only return true if we're sure that we have a
   // parseable file. We don't know that though.

include-fixer/IncludeFixer.h

@@ -22,8 +22,10 @@ class IncludeFixerActionFactory : public clang::tooling::ToolAction {
 public:
   /// \param Xrefs A source for matching symbols to header files.
   /// \param Replacements Storage for the output of the fixer.
+  /// \param MinimizeIncludePaths whether inserted include paths are optimized.
   IncludeFixerActionFactory(
-      XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements);
+      XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements,
+      bool MinimizeIncludePaths = true);
   ~IncludeFixerActionFactory();
 
   bool
@@ -38,6 +40,9 @@ private:
 
   /// Replacements are written here.
   std::vector<clang::tooling::Replacement> &Replacements;
+
+  /// Whether inserted include paths should be optimized.
+  bool MinimizeIncludePaths;
 };
 
 } // namespace include_fixer

include-fixer/tool/ClangIncludeFixer.cpp

@@ -32,6 +32,11 @@ cl::opt<DatabaseFormatTy> DatabaseFormat(
 cl::opt<std::string> Input("input",
                            cl::desc("String to initialize the database"),
                            cl::cat(IncludeFixerCategory));
+
+cl::opt<bool>
+    MinimizeIncludePaths("minimize-paths",
+                         cl::desc("Whether to minimize added include paths"),
+                         cl::init(true), cl::cat(IncludeFixerCategory));
 } // namespace
 
 int main(int argc, const char **argv) {
@@ -66,7 +71,8 @@ int main(int argc, const char **argv) {
 
   // Now run our tool.
   std::vector<tooling::Replacement> Replacements;
-  include_fixer::IncludeFixerActionFactory Factory(*XrefsDB, Replacements);
+  include_fixer::IncludeFixerActionFactory Factory(*XrefsDB, Replacements,
+                                                   MinimizeIncludePaths);
 
   tool.run(&Factory); // Always succeeds.
 

unittests/include-fixer/IncludeFixerTest.cpp

@@ -7,9 +7,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "unittests/Tooling/RewriterTestContext.h"
 #include "InMemoryXrefsDB.h"
 #include "IncludeFixer.h"
+#include "unittests/Tooling/RewriterTestContext.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 using namespace clang;
@@ -19,34 +19,43 @@ namespace include_fixer {
 namespace {
 
 static bool runOnCode(tooling::ToolAction *ToolAction, StringRef Code,
-                      StringRef FileName) {
+                      StringRef FileName,
+                      const std::vector<std::string> &ExtraArgs) {
   llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
       new vfs::InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr<FileManager> Files(
       new FileManager(FileSystemOptions(), InMemoryFileSystem));
+  std::vector<std::string> Args = {"include_fixer", "-fsyntax-only", FileName};
+  Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
   tooling::ToolInvocation Invocation(
-      {std::string("include_fixer"), std::string("-fsyntax-only"),
-       FileName.str()},
-      ToolAction, Files.get(), std::make_shared<PCHContainerOperations>());
+      Args, ToolAction, Files.get(),
+      std::make_shared<PCHContainerOperations>());
 
   InMemoryFileSystem->addFile(FileName, 0,
                               llvm::MemoryBuffer::getMemBuffer(Code));
 
   InMemoryFileSystem->addFile("foo.h", 0,
                               llvm::MemoryBuffer::getMemBuffer("\n"));
-  InMemoryFileSystem->addFile("bar.h", 0,
+  InMemoryFileSystem->addFile("dir/bar.h", 0,
+                              llvm::MemoryBuffer::getMemBuffer("\n"));
+  InMemoryFileSystem->addFile("dir/otherdir/qux.h", 0,
                               llvm::MemoryBuffer::getMemBuffer("\n"));
   return Invocation.run();
 }
 
-static std::string runIncludeFixer(StringRef Code) {
+static std::string runIncludeFixer(
+    StringRef Code,
+    const std::vector<std::string> &ExtraArgs = std::vector<std::string>()) {
   std::map<std::string, std::vector<std::string>> XrefsMap = {
-      {"std::string", {"<string>"}}, {"std::string::size_type", {"<string>"}}};
+      {"std::string", {"<string>"}},
+      {"std::string::size_type", {"<string>"}},
+      {"a::b::foo", {"dir/otherdir/qux.h"}},
+  };
   auto XrefsDB =
       llvm::make_unique<include_fixer::InMemoryXrefsDB>(std::move(XrefsMap));
   std::vector<clang::tooling::Replacement> Replacements;
   IncludeFixerActionFactory Factory(*XrefsDB, Replacements);
-  runOnCode(&Factory, Code, "input.cc");
+  runOnCode(&Factory, Code, "input.cc", ExtraArgs);
   clang::RewriterTestContext Context;
   clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
   clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
@@ -59,9 +68,9 @@ TEST(IncludeFixer, Typo) {
 
   EXPECT_EQ(
       "// comment\n#include <string>\n#include \"foo.h\"\nstd::string foo;\n"
-      "#include \"bar.h\"\n",
+      "#include \"dir/bar.h\"\n",
       runIncludeFixer("// comment\n#include \"foo.h\"\nstd::string foo;\n"
-                      "#include \"bar.h\"\n"));
+                      "#include \"dir/bar.h\"\n"));
 
   EXPECT_EQ("#include <string>\n#include \"foo.h\"\nstd::string foo;\n",
             runIncludeFixer("#include \"foo.h\"\nstd::string foo;\n"));
@@ -82,6 +91,24 @@ TEST(IncludeFixer, IncompleteType) {
                       "namespace std {\nclass string;\n}\nstring foo;\n"));
 }
 
+TEST(IncludeFixer, MinimizeInclude) {
+  std::vector<std::string> IncludePath = {"-Idir/"};
+  EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n",
+            runIncludeFixer("a::b::foo bar;\n", IncludePath));
+
+  IncludePath = {"-isystemdir"};
+  EXPECT_EQ("#include <otherdir/qux.h>\na::b::foo bar;\n",
+            runIncludeFixer("a::b::foo bar;\n", IncludePath));
+
+  IncludePath = {"-iquotedir"};
+  EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n",
+            runIncludeFixer("a::b::foo bar;\n", IncludePath));
+
+  IncludePath = {"-Idir", "-Idir/otherdir"};
+  EXPECT_EQ("#include \"qux.h\"\na::b::foo bar;\n",
+            runIncludeFixer("a::b::foo bar;\n", IncludePath));
+}
+
 } // namespace
 } // namespace include_fixer
 } // namespace clang