[clang-tools-extra] r270031 - [include-fixer] Sort headers after inserting new headers.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Thu May 19 01:21:12 PDT 2016
Author: ioeric
Date: Thu May 19 03:21:09 2016
New Revision: 270031
URL: http://llvm.org/viewvc/llvm-project?rev=270031&view=rev
Log:
[include-fixer] Sort headers after inserting new headers.
Summary: [include-fixer] Sort headers after inserting new headers.
Reviewers: bkramer
Subscribers: klimek, djasper, hokein, cfe-commits
Differential Revision: http://reviews.llvm.org/D20370
Modified:
clang-tools-extra/trunk/include-fixer/CMakeLists.txt
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
Modified: clang-tools-extra/trunk/include-fixer/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/CMakeLists.txt?rev=270031&r1=270030&r2=270031&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/include-fixer/CMakeLists.txt Thu May 19 03:21:09 2016
@@ -11,6 +11,7 @@ add_clang_library(clangIncludeFixer
LINK_LIBS
clangAST
clangBasic
+ clangFormat
clangFrontend
clangLex
clangParse
Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=270031&r1=270030&r2=270031&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Thu May 19 03:21:09 2016
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "IncludeFixer.h"
+#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Preprocessor.h"
@@ -58,8 +59,9 @@ private:
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<clang::ASTConsumer>
@@ -234,26 +236,61 @@ public:
return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"';
}
+ /// Insert all headers before the first #include in \p Code and run
+ /// clang-format to sort all headers.
+ /// \return Replacements for inserting and sorting headers.
+ std::vector<clang::tooling::Replacement>
+ CreateReplacementsForHeaders(StringRef Code,
+ const std::set<std::string> &Headers) {
+ std::set<std::string> ExistingHeaders;
+
+ // Create replacements for new headers.
+ clang::tooling::Replacements Insertions;
+ if (FirstIncludeOffset == -1U) {
+ // FIXME: 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"));
+ }
+ // Keep inserting new headers before the first header.
+ 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<clang::tooling::Replacement>`.
+ std::vector<clang::tooling::Replacement> 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<clang::tooling::Replacement> &replacements) {
+ std::set<std::string> &Headers,
+ std::vector<clang::tooling::Replacement> &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");
+ Headers.insert(minimizeInclude(ToTry, SourceManager, HeaderSearch));
- if (FirstIncludeOffset == -1U)
- FirstIncludeOffset = 0;
-
- 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 +351,10 @@ private:
/// 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.
@@ -365,11 +406,12 @@ void PreprocessorHooks::InclusionDirecti
} // namespace
IncludeFixerActionFactory::IncludeFixerActionFactory(
- SymbolIndexManager &SymbolIndexMgr,
- std::vector<clang::tooling::Replacement> &Replacements,
+ SymbolIndexManager &SymbolIndexMgr, std::set<std::string> &Headers,
+ std::vector<clang::tooling::Replacement> &Replacements, StringRef StyleName,
bool MinimizeIncludePaths)
- : SymbolIndexMgr(SymbolIndexMgr), Replacements(Replacements),
- MinimizeIncludePaths(MinimizeIncludePaths) {}
+ : SymbolIndexMgr(SymbolIndexMgr), Headers(Headers),
+ Replacements(Replacements), MinimizeIncludePaths(MinimizeIncludePaths),
+ FallbackStyle(StyleName) {}
IncludeFixerActionFactory::~IncludeFixerActionFactory() = default;
@@ -395,14 +437,14 @@ bool IncludeFixerActionFactory::runInvoc
Compiler.getDiagnostics().setErrorLimit(0);
// Run the parser, gather missing includes.
- auto ScopedToolAction =
- llvm::make_unique<Action>(SymbolIndexMgr, MinimizeIncludePaths);
+ auto ScopedToolAction = llvm::make_unique<Action>(
+ SymbolIndexMgr, FallbackStyle, MinimizeIncludePaths);
Compiler.ExecuteAction(*ScopedToolAction);
// Generate replacements.
ScopedToolAction->Rewrite(Compiler.getSourceManager(),
Compiler.getPreprocessor().getHeaderSearchInfo(),
- Replacements);
+ Headers, Replacements);
// 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
Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.h?rev=270031&r1=270030&r2=270031&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.h (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h Thu May 19 03:21:09 2016
@@ -29,11 +29,13 @@ class IncludeFixerActionFactory : public
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,
+ SymbolIndexManager &SymbolIndexMgr, std::set<std::string> &Headers,
std::vector<clang::tooling::Replacement> &Replacements,
- bool MinimizeIncludePaths = true);
+ StringRef StyleName, bool MinimizeIncludePaths = true);
+
~IncludeFixerActionFactory() override;
bool
@@ -46,11 +48,18 @@ private:
/// The client to use to find cross-references.
SymbolIndexManager &SymbolIndexMgr;
+ /// Headers to be added.
+ std::set<std::string> &Headers;
+
/// Replacements are written here.
std::vector<clang::tooling::Replacement> &Replacements;
/// Whether inserted include paths should be optimized.
bool MinimizeIncludePaths;
+
+ /// The fallback format style for formatting after insertion if no
+ /// clang-format config file was found.
+ std::string FallbackStyle;
};
} // namespace include_fixer
Modified: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp?rev=270031&r1=270030&r2=270031&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp Thu May 19 03:21:09 2016
@@ -56,6 +56,12 @@ cl::opt<bool>
"used for editor integration."),
cl::init(false), cl::cat(IncludeFixerCategory));
+cl::opt<std::string>
+ 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(),
@@ -133,9 +139,10 @@ int includeFixerMain(int argc, const cha
}
// Now run our tool.
+ std::set<std::string> Headers; // Headers to be added.
std::vector<tooling::Replacement> Replacements;
include_fixer::IncludeFixerActionFactory Factory(
- *SymbolIndexMgr, Replacements, MinimizeIncludePaths);
+ *SymbolIndexMgr, Headers, Replacements, Style, MinimizeIncludePaths);
if (tool.run(&Factory) != 0) {
llvm::errs()
@@ -144,8 +151,8 @@ int includeFixerMain(int argc, const cha
}
if (!Quiet)
- for (const tooling::Replacement &Replacement : Replacements)
- llvm::errs() << "Added " << Replacement.getReplacementText();
+ for (const auto &Header : Headers)
+ llvm::errs() << "Added #include " << Header;
// Set up a new source manager for applying the resulting replacements.
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);
@@ -155,11 +162,10 @@ int includeFixerMain(int argc, const cha
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;
}
Modified: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py?rev=270031&r1=270030&r2=270031&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py (original)
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py Thu May 19 03:21:09 2016
@@ -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 @@ def main():
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()
Modified: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp?rev=270031&r1=270030&r2=270031&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Thu May 19 03:21:09 2016
@@ -70,8 +70,10 @@ static std::string runIncludeFixer(
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique<include_fixer::InMemorySymbolIndex>(Symbols));
+ std::set<std::string> Headers;
std::vector<clang::tooling::Replacement> Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Replacements);
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
+ "llvm");
runOnCode(&Factory, Code, "input.cc", ExtraArgs);
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
@@ -80,32 +82,32 @@ static std::string runIncludeFixer(
}
TEST(IncludeFixer, Typo) {
- EXPECT_EQ("#include <string>\nstd::string foo;\n",
+ EXPECT_EQ("#include <string>\n\nstd::string foo;\n",
runIncludeFixer("std::string foo;\n"));
EXPECT_EQ(
- "// comment\n#include <string>\n#include \"foo.h\"\nstd::string foo;\n"
+ "// comment\n#include \"foo.h\"\n#include <string>\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 <string>\n#include \"foo.h\"\nstd::string foo;\n",
+ EXPECT_EQ("#include \"foo.h\"\n#include <string>\nstd::string foo;\n",
runIncludeFixer("#include \"foo.h\"\nstd::string foo;\n"));
EXPECT_EQ(
- "#include <string>\n#include \"foo.h\"\nstd::string::size_type foo;\n",
+ "#include \"foo.h\"\n#include <string>\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 <string>\nstring foo;\n",
+ EXPECT_EQ("#include <string>\n\nstring foo;\n",
runIncludeFixer("string foo;\n"));
}
TEST(IncludeFixer, IncompleteType) {
EXPECT_EQ(
- "#include <string>\n#include \"foo.h\"\n"
+ "#include \"foo.h\"\n#include <string>\n"
"namespace std {\nclass string;\n}\nstring foo;\n",
runIncludeFixer("#include \"foo.h\"\n"
"namespace std {\nclass string;\n}\nstring foo;\n"));
@@ -113,26 +115,26 @@ TEST(IncludeFixer, IncompleteType) {
TEST(IncludeFixer, MinimizeInclude) {
std::vector<std::string> 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 <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 = {"-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<std::string> 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));
@@ -142,36 +144,52 @@ TEST(IncludeFixer, NestedName) {
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 <string>\nstd::string bar;\nstd::sting foo;\n",
+ EXPECT_EQ("#include <string>\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
More information about the cfe-commits
mailing list