Index: include-fixer/IncludeFixer.h =================================================================== --- include-fixer/IncludeFixer.h +++ include-fixer/IncludeFixer.h @@ -29,11 +29,13 @@ public: /// \param SymbolIndexMgr A source for matching symbols to header files. /// \param Replacements Storage for the output of the fixer. + /// \param StyleName Fallback style for reformatting. /// \param MinimizeIncludePaths whether inserted include paths are optimized. IncludeFixerActionFactory( SymbolIndexManager &SymbolIndexMgr, std::vector &Replacements, - bool MinimizeIncludePaths = true); + StringRef StyleName, bool MinimizeIncludePaths = true); + ~IncludeFixerActionFactory() override; bool @@ -51,6 +53,10 @@ /// Whether inserted include paths should be optimized. bool MinimizeIncludePaths; + + /// The fallback format style for formatting after insertion if there is no + /// clang-format config file found. + std::string FallbackStyle; }; } // namespace include_fixer Index: include-fixer/IncludeFixer.cpp =================================================================== --- include-fixer/IncludeFixer.cpp +++ include-fixer/IncludeFixer.cpp @@ -8,7 +8,9 @@ //===----------------------------------------------------------------------===// #include "IncludeFixer.h" +#include "clang/Format/Format.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" @@ -58,8 +60,9 @@ class Action : public clang::ASTFrontendAction, public clang::ExternalSemaSource { public: - explicit Action(SymbolIndexManager &SymbolIndexMgr, bool MinimizeIncludePaths) - : SymbolIndexMgr(SymbolIndexMgr), + explicit Action(SymbolIndexManager &SymbolIndexMgr, StringRef StyleName, + bool MinimizeIncludePaths) + : SymbolIndexMgr(SymbolIndexMgr), FallbackStyle(StyleName), MinimizeIncludePaths(MinimizeIncludePaths) {} std::unique_ptr @@ -234,26 +237,65 @@ return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"'; } + /// Insert all headers after the last include in \p Code and run clang-format + /// to sort the headers. + /// \return Replacements for inserting and sorting headers. + std::vector + CreateReplacementsForHeaders(StringRef Code, + const std::set &Headers) { + std::set ExistingHeaders; + + // Create replacements for new headers. + // Keep inserting new headers before the first header. + clang::tooling::Replacements Insertions; + if (FirstIncludeOffset == -1U) { + // FIXME: we might want to skip header guards. + FirstIncludeOffset = 0; + // If there is no existing #include, then insert an empty line after new + // header block. + if (Code.front() != '\n') + Insertions.insert( + clang::tooling::Replacement(Filename, FirstIncludeOffset, 0, "\n")); + } + for (StringRef Header : Headers) { + std::string Text = "#include " + Header.str() + "\n"; + + Insertions.insert( + clang::tooling::Replacement(Filename, FirstIncludeOffset, 0, Text)); + } + + DEBUG(llvm::dbgs() << "Header insertions:\n"); + for (const auto &R : Insertions) { + DEBUG(llvm::dbgs() << R.toString() << "\n"); + } + + clang::format::FormatStyle Style = + clang::format::getStyle("file", Filename, FallbackStyle); + clang::tooling::Replacements Replaces = + formatReplacements(Code, Insertions, Style); + // FIXME: remove this when `clang::tooling::Replacements` is implemented as + // `std::vector`. + std::vector Results; + std::copy(Replaces.begin(), Replaces.end(), std::back_inserter(Results)); + return Results; + } + /// 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 &replacements) { + std::vector &Replacements) { if (Untried.empty()) return false; const auto &ToTry = UntriedList.front(); - std::string ToAdd = "#include " + - minimizeInclude(ToTry, SourceManager, HeaderSearch) + - "\n"; - DEBUG(llvm::dbgs() << "Adding " << ToAdd << "\n"); + std::string ToAdd = minimizeInclude(ToTry, SourceManager, HeaderSearch); - if (FirstIncludeOffset == -1U) - FirstIncludeOffset = 0; + std::set Headers; + Headers.insert(ToAdd); - replacements.push_back(clang::tooling::Replacement( - SourceManager, FileBegin.getLocWithOffset(FirstIncludeOffset), 0, - ToAdd)); + StringRef Code = SourceManager.getBufferData(SourceManager.getMainFileID()); + Replacements = CreateReplacementsForHeaders(Code, Headers); // We currently abort after the first inserted include. The more // includes we have the less safe this becomes due to error recovery @@ -314,6 +356,10 @@ /// be used as the insertion point for new include directives. unsigned FirstIncludeOffset = -1U; + /// The fallback format style for formatting after insertion if there is no + /// clang-format config file found. + std::string FallbackStyle; + /// Includes we have left to try. A set to unique them and a list to keep /// track of the order. We prefer includes that were discovered early to avoid /// getting caught in results from error recovery. @@ -366,10 +412,10 @@ IncludeFixerActionFactory::IncludeFixerActionFactory( SymbolIndexManager &SymbolIndexMgr, - std::vector &Replacements, + std::vector &Replacements, StringRef StyleName, bool MinimizeIncludePaths) : SymbolIndexMgr(SymbolIndexMgr), Replacements(Replacements), - MinimizeIncludePaths(MinimizeIncludePaths) {} + MinimizeIncludePaths(MinimizeIncludePaths), FallbackStyle(StyleName) {} IncludeFixerActionFactory::~IncludeFixerActionFactory() = default; @@ -395,8 +441,8 @@ Compiler.getDiagnostics().setErrorLimit(0); // Run the parser, gather missing includes. - auto ScopedToolAction = - llvm::make_unique(SymbolIndexMgr, MinimizeIncludePaths); + auto ScopedToolAction = llvm::make_unique( + SymbolIndexMgr, FallbackStyle, MinimizeIncludePaths); Compiler.ExecuteAction(*ScopedToolAction); // Generate replacements. Index: include-fixer/tool/ClangIncludeFixer.cpp =================================================================== --- include-fixer/tool/ClangIncludeFixer.cpp +++ include-fixer/tool/ClangIncludeFixer.cpp @@ -56,6 +56,12 @@ "used for editor integration."), cl::init(false), cl::cat(IncludeFixerCategory)); +cl::opt + Style("style", + cl::desc("Fallback style for reformatting after inserting new " + "headers if there is no clang-format config file found."), + cl::init("llvm"), cl::cat(IncludeFixerCategory)); + int includeFixerMain(int argc, const char **argv) { tooling::CommonOptionsParser options(argc, argv, IncludeFixerCategory); tooling::ClangTool tool(options.getCompilations(), @@ -135,7 +141,7 @@ // Now run our tool. std::vector Replacements; include_fixer::IncludeFixerActionFactory Factory( - *SymbolIndexMgr, Replacements, MinimizeIncludePaths); + *SymbolIndexMgr, Replacements, Style, MinimizeIncludePaths); if (tool.run(&Factory) != 0) { llvm::errs() @@ -155,11 +161,10 @@ Diagnostics.setClient(&DiagnosticPrinter, false); if (STDINMode) { - for (const tooling::Replacement &Replacement : Replacements) { - FileID ID = SM.getMainFileID(); - unsigned LineNum = SM.getLineNumber(ID, Replacement.getOffset()); - llvm::outs() << LineNum << "," << Replacement.getReplacementText(); - } + tooling::Replacements Replaces(Replacements.begin(), Replacements.end()); + std::string ChangedCode = + tooling::applyAllReplacements(Code->getBuffer(), Replaces); + llvm::outs() << ChangedCode; return 0; } Index: include-fixer/tool/clang-include-fixer.py =================================================================== --- include-fixer/tool/clang-include-fixer.py +++ include-fixer/tool/clang-include-fixer.py @@ -16,6 +16,7 @@ # or save any files. To revert a fix, just undo. import argparse +import difflib import subprocess import sys import vim @@ -54,12 +55,10 @@ if stdout: lines = stdout.splitlines() - for line in lines: - line_num, text = line.split(",") - # clang-include-fixer provides 1-based line number - line_num = int(line_num) - 1 - print 'Inserting "{0}" at line {1}'.format(text, line_num) - vim.current.buffer[line_num:line_num] = [text] + sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines) + for op in reversed(sequence.get_opcodes()): + if op[0] is not 'equal': + vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]] if __name__ == '__main__': main() Index: unittests/include-fixer/IncludeFixerTest.cpp =================================================================== --- unittests/include-fixer/IncludeFixerTest.cpp +++ unittests/include-fixer/IncludeFixerTest.cpp @@ -74,7 +74,7 @@ llvm::make_unique(Symbols)); std::vector Replacements; - IncludeFixerActionFactory Factory(*SymbolIndexMgr, Replacements); + IncludeFixerActionFactory Factory(*SymbolIndexMgr, Replacements, "llvm"); runOnCode(&Factory, Code, "input.cc", ExtraArgs); clang::RewriterTestContext Context; clang::FileID ID = Context.createInMemoryFile("input.cc", Code); @@ -83,32 +83,32 @@ } TEST(IncludeFixer, Typo) { - EXPECT_EQ("#include \nstd::string foo;\n", + EXPECT_EQ("#include \n\nstd::string foo;\n", runIncludeFixer("std::string foo;\n")); EXPECT_EQ( - "// comment\n#include \n#include \"foo.h\"\nstd::string foo;\n" + "// comment\n#include \"foo.h\"\n#include \nstd::string foo;\n" "#include \"dir/bar.h\"\n", runIncludeFixer("// comment\n#include \"foo.h\"\nstd::string foo;\n" "#include \"dir/bar.h\"\n")); - EXPECT_EQ("#include \n#include \"foo.h\"\nstd::string foo;\n", + EXPECT_EQ("#include \"foo.h\"\n#include \nstd::string foo;\n", runIncludeFixer("#include \"foo.h\"\nstd::string foo;\n")); EXPECT_EQ( - "#include \n#include \"foo.h\"\nstd::string::size_type foo;\n", + "#include \"foo.h\"\n#include \nstd::string::size_type foo;\n", runIncludeFixer("#include \"foo.h\"\nstd::string::size_type foo;\n")); // string without "std::" can also be fixed since fixed db results go through // SymbolIndexManager, and SymbolIndexManager matches unqualified identifiers // too. - EXPECT_EQ("#include \nstring foo;\n", + EXPECT_EQ("#include \n\nstring foo;\n", runIncludeFixer("string foo;\n")); } TEST(IncludeFixer, IncompleteType) { EXPECT_EQ( - "#include \n#include \"foo.h\"\n" + "#include \"foo.h\"\n#include \n" "namespace std {\nclass string;\n}\nstring foo;\n", runIncludeFixer("#include \"foo.h\"\n" "namespace std {\nclass string;\n}\nstring foo;\n")); @@ -116,26 +116,26 @@ TEST(IncludeFixer, MinimizeInclude) { std::vector IncludePath = {"-Idir/"}; - EXPECT_EQ("#include \"otherdir/qux.h\"\na::b::foo bar;\n", + EXPECT_EQ("#include \"otherdir/qux.h\"\n\na::b::foo bar;\n", runIncludeFixer("a::b::foo bar;\n", IncludePath)); IncludePath = {"-isystemdir"}; - EXPECT_EQ("#include \na::b::foo bar;\n", + EXPECT_EQ("#include \n\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", + EXPECT_EQ("#include \"otherdir/qux.h\"\n\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", + EXPECT_EQ("#include \"qux.h\"\n\na::b::foo bar;\n", runIncludeFixer("a::b::foo bar;\n", IncludePath)); } TEST(IncludeFixer, NestedName) { // Some tests don't pass for target *-win32. std::vector args = {"-target", "x86_64-unknown-unknown"}; - EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n" + EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n\n" "int x = a::b::foo(0);\n", runIncludeFixer("int x = a::b::foo(0);\n", args)); @@ -145,36 +145,52 @@ EXPECT_EQ("#define FOO(x) a::##x\nint x = FOO(b::foo);\n", runIncludeFixer("#define FOO(x) a::##x\nint x = FOO(b::foo);\n")); - EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n" + EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n\n" "namespace a {}\nint a = a::b::foo(0);\n", runIncludeFixer("namespace a {}\nint a = a::b::foo(0);\n", args)); } TEST(IncludeFixer, MultipleMissingSymbols) { - EXPECT_EQ("#include \nstd::string bar;\nstd::sting foo;\n", + EXPECT_EQ("#include \n\nstd::string bar;\nstd::sting foo;\n", runIncludeFixer("std::string bar;\nstd::sting foo;\n")); } TEST(IncludeFixer, ScopedNamespaceSymbols) { - EXPECT_EQ("#include \"bar.h\"\nnamespace a { b::bar b; }\n", - runIncludeFixer("namespace a { b::bar b; }\n")); - EXPECT_EQ("#include \"bar.h\"\nnamespace A { a::b::bar b; }\n", - runIncludeFixer("namespace A { a::b::bar b; }\n")); - EXPECT_EQ("#include \"bar.h\"\nnamespace a { void func() { b::bar b; } }\n", - runIncludeFixer("namespace a { void func() { b::bar b; } }\n")); + EXPECT_EQ("#include \"bar.h\"\n\nnamespace a {\nb::bar b;\n}", + runIncludeFixer("namespace a {\nb::bar b;\n}")); + EXPECT_EQ("#include \"bar.h\"\n\nnamespace A {\na::b::bar b;\n}", + runIncludeFixer("namespace A {\na::b::bar b;\n}")); + EXPECT_EQ("#include \"bar.h\"\n\nnamespace a {\nvoid func() { b::bar b; }\n}", + runIncludeFixer("namespace a {\nvoid func() { b::bar b; }\n}")); EXPECT_EQ("namespace A { c::b::bar b; }\n", runIncludeFixer("namespace A { c::b::bar b; }\n")); // FIXME: The header should not be added here. Remove this after we support // full match. - EXPECT_EQ("#include \"bar.h\"\nnamespace A { b::bar b; }\n", - runIncludeFixer("namespace A { b::bar b; }\n")); + EXPECT_EQ("#include \"bar.h\"\n\nnamespace A {\nb::bar b;\n}", + runIncludeFixer("namespace A {\nb::bar b;\n}")); } TEST(IncludeFixer, EnumConstantSymbols) { - EXPECT_EQ("#include \"color.h\"\nint test = a::b::Green;\n", + EXPECT_EQ("#include \"color.h\"\n\nint test = a::b::Green;\n", runIncludeFixer("int test = a::b::Green;\n")); } +// FIXME: add test cases for inserting and sorting multiple headers when +// include-fixer supports multiple headers insertion. +TEST(IncludeFixer, InsertAndSortSingleHeader) { + // Insert one header. + std::string Code = "#include \"a.h\"\n" + "#include \"foo.h\"\n" + "\n" + "namespace a { b::bar b; }"; + std::string Expected = "#include \"a.h\"\n" + "#include \"bar.h\"\n" + "#include \"foo.h\"\n" + "\n" + "namespace a { b::bar b; }"; + EXPECT_EQ(Expected, runIncludeFixer(Code)); +} + } // namespace } // namespace include_fixer } // namespace clang