diff options
4 files changed, 95 insertions, 20 deletions
diff --git a/clang-tools-extra/include-fixer/IncludeFixer.cpp b/clang-tools-extra/include-fixer/IncludeFixer.cpp index ea4e948d1c6..5c60499a5aa 100644 --- a/clang-tools-extra/include-fixer/IncludeFixer.cpp +++ b/clang-tools-extra/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. diff --git a/clang-tools-extra/include-fixer/IncludeFixer.h b/clang-tools-extra/include-fixer/IncludeFixer.h index 01478988e0f..504668a82ef 100644 --- a/clang-tools-extra/include-fixer/IncludeFixer.h +++ b/clang-tools-extra/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 diff --git a/clang-tools-extra/include-fixer/tool/ClangIncludeFixer.cpp b/clang-tools-extra/include-fixer/tool/ClangIncludeFixer.cpp index c376907df7a..c698d10ed36 100644 --- a/clang-tools-extra/include-fixer/tool/ClangIncludeFixer.cpp +++ b/clang-tools-extra/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. diff --git a/clang-tools-extra/unittests/include-fixer/IncludeFixerTest.cpp b/clang-tools-extra/unittests/include-fixer/IncludeFixerTest.cpp index 12759761b19..5b500687e4a 100644 --- a/clang-tools-extra/unittests/include-fixer/IncludeFixerTest.cpp +++ b/clang-tools-extra/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 |
