hokein created this revision.
hokein added reviewers: bkramer, ioeric.
hokein added a subscriber: cfe-commits.
http://reviews.llvm.org/D20966
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
test/include-fixer/insert_header_in_header.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===================================================================
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -44,8 +44,6 @@
llvm::MemoryBuffer::getMemBuffer("\n"));
InMemoryFileSystem->addFile("dir/otherdir/qux.h", 0,
llvm::MemoryBuffer::getMemBuffer("\n"));
- InMemoryFileSystem->addFile("header.h", 0,
- llvm::MemoryBuffer::getMemBuffer("bar b;"));
return Invocation.run();
}
@@ -188,11 +186,6 @@
runIncludeFixer("int test = a::b::Green;\n"));
}
-TEST(IncludeFixer, IgnoreSymbolFromHeader) {
- std::string Code = "#include \"header.h\"";
- EXPECT_EQ(Code, runIncludeFixer(Code));
-}
-
// FIXME: add test cases for inserting and sorting multiple headers when
// include-fixer supports multiple headers insertion.
TEST(IncludeFixer, InsertAndSortSingleHeader) {
Index: test/include-fixer/insert_header_in_header.cpp
===================================================================
--- /dev/null
+++ test/include-fixer/insert_header_in_header.cpp
@@ -0,0 +1,10 @@
+// REQUIRES: shell
+// RUN: echo 'foo f;' > %T/main.h
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=yaml -input=%p/Inputs/fake_yaml_db.yaml %t.cpp --
+// RUN: FileCheck %s -input-file=%T/main.h
+
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+#include "main.h"
Index: test/include-fixer/commandline_options.cpp
===================================================================
--- test/include-fixer/commandline_options.cpp
+++ test/include-fixer/commandline_options.cpp
@@ -1,7 +1,7 @@
// REQUIRES: shell
// RUN: sed -e 's#//.*$##' %s > %t.cpp
// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
-// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, Headers: ["\"foo.h\""]}' %t.cpp | FileCheck %s -check-prefix=CHECK
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{SymbolIdentifier: foo, FilePath: %t.cpp, Headers: ["\"foo.h\""]}' %t.cpp | FileCheck %s -check-prefix=CHECK
//
// CHECK-HEADERS: "Headers": [ "\"foo.h\"", "\"bar.h\"" ]
//
Index: include-fixer/tool/clang-include-fixer.py
===================================================================
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -49,11 +49,19 @@
return p.communicate(input=text)
-def InsertHeaderToVimBuffer(header, text):
+def InsertHeaderToVimBuffer(header, code):
command = [binary, "-stdin", "-insert-header="+json.dumps(header),
vim.current.buffer.name]
- stdout, stderr = execute(command, text)
+ stdout, stderr = execute(command, code)
+
if stdout:
+ # If the file being inserted is not the current one in vim buffer, just
+ # overwrite it in filesystem.
+ if header["FilePath"] != vim.current.buffer.name:
+ with open(header["FilePath"], "w") as f:
+ f.write(stdout)
+ return
+
lines = stdout.splitlines()
sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
for op in reversed(sequence.get_opcodes()):
@@ -72,19 +80,24 @@
# Get the current text.
buf = vim.current.buffer
- text = '\n'.join(buf)
+ code = '\n'.join(buf)
# Run command to get all headers.
command = [binary, "-stdin", "-output-headers", "-db="+args.db,
"-input="+args.input, vim.current.buffer.name]
- stdout, stderr = execute(command, text)
+ stdout, stderr = execute(command, code)
if stderr:
print >> sys.stderr, "Error while running clang-include-fixer: " + stderr
return
include_fixer_context = json.loads(stdout)
symbol = include_fixer_context["SymbolIdentifier"]
headers = include_fixer_context["Headers"]
+ file_path = include_fixer_context["FilePath"]
+
+ if file_path != vim.current.buffer.name:
+ with open(file_path, "r") as f:
+ code = f.read();
if not symbol:
print "The file is fine, no need to add a header.\n"
@@ -98,8 +111,11 @@
# If there is only one suggested header, insert it directly.
if len(headers) == 1 or maximum_suggested_headers == 1:
InsertHeaderToVimBuffer({"SymbolIdentifier": symbol,
- "Headers":[headers[0]]}, text)
- print "Added #include {0} for {1}.\n".format(headers[0], symbol)
+ "Headers": [headers[0]],
+ "FilePath": file_path}, code)
+ print "Added #include {0} for {1} in {2}.\n".format(headers[0],
+ symbol,
+ file_path)
return
choices_message = ""
@@ -112,8 +128,11 @@
choices_message)
# Insert a selected header.
InsertHeaderToVimBuffer({"SymbolIdentifier": symbol,
- "Headers":[headers[select-1]]}, text)
- print "Added #include {0} for {1}.\n".format(headers[select-1], symbol)
+ "Headers": [headers[select-1]],
+ "FilePath": file_path}, code)
+ print "Added #include {0} for {1} in {2}.\n".format(headers[select-1],
+ symbol,
+ file_path)
return;
Index: include-fixer/tool/ClangIncludeFixer.cpp
===================================================================
--- include-fixer/tool/ClangIncludeFixer.cpp
+++ include-fixer/tool/ClangIncludeFixer.cpp
@@ -33,6 +33,7 @@
static void mapping(IO &io, IncludeFixerContext &Context) {
io.mapRequired("SymbolIdentifier", Context.SymbolIdentifier);
io.mapRequired("Headers", Context.Headers);
+ io.mapRequired("FilePath", Context.FilePath);
}
};
} // namespace yaml
@@ -80,6 +81,7 @@
"JSON format to stdout:\n"
" {\n"
" \"SymbolIdentifier\": \"foo\",\n"
+ " \"FilePath\": \"path/to/main.cc\", \n"
" \"Headers\": [\"\\\"foo_a.h\\\"\"]\n"
" }"),
cl::init(false), cl::cat(IncludeFixerCategory));
@@ -90,6 +92,7 @@
"The result is written to stdout. It is currently used for\n"
"editor integration. Support YAML/JSON format:\n"
" -insert-header=\"{SymbolIdentifier: foo,\n"
+ " FilePath: path/to/main.cc,\n"
" Headers: ['\\\"foo_a.h\\\"']}\""),
cl::init(""), cl::cat(IncludeFixerCategory));
@@ -155,6 +158,7 @@
void writeToJson(llvm::raw_ostream &OS, const IncludeFixerContext& Context) {
OS << "{\n"
" \"SymbolIdentifier\": \"" << Context.SymbolIdentifier << "\",\n"
+ " \"FilePath\": \"" << Context.FilePath << "\",\n"
" \"Headers\": [ ";
for (const auto &Header : Context.Headers) {
OS << " \"" << llvm::yaml::escape(Header) << "\"";
@@ -190,8 +194,8 @@
tool.mapVirtualFile(options.getSourcePathList().front(), Code->getBuffer());
}
- StringRef FilePath = options.getSourcePathList().front();
- format::FormatStyle InsertStyle = format::getStyle("file", FilePath, Style);
+ format::FormatStyle InsertStyle =
+ format::getStyle("file", options.getSourcePathList().front(), Style);
if (!InsertHeader.empty()) {
if (!STDINMode) {
@@ -210,7 +214,8 @@
tooling::Replacements Replacements =
clang::include_fixer::createInsertHeaderReplacements(
- Code->getBuffer(), FilePath, Context.Headers[0], InsertStyle);
+ Code->getBuffer(), Context.FilePath, Context.Headers[0],
+ InsertStyle);
tooling::Replacements Replaces(Replacements.begin(), Replacements.end());
std::string ChangedCode =
tooling::applyAllReplacements(Code->getBuffer(), Replaces);
@@ -243,17 +248,16 @@
if (Context.Headers.empty())
return 0;
- auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
+ auto Buffer = llvm::MemoryBuffer::getFile(Context.FilePath);
if (!Buffer) {
- errs() << "Couldn't open file: " << FilePath;
+ errs() << "Couldn't open file: " << Context.FilePath;
return 1;
}
- // FIXME: Rank the results and pick the best one instead of the first one.
tooling::Replacements Replacements =
clang::include_fixer::createInsertHeaderReplacements(
- /*Code=*/Buffer.get()->getBuffer(), FilePath, Context.Headers.front(),
- InsertStyle);
+ /*Code=*/Buffer.get()->getBuffer(), Context.FilePath,
+ Context.Headers.front(), InsertStyle);
if (!Quiet)
llvm::errs() << "Added #include" << Context.Headers.front();
Index: include-fixer/IncludeFixerContext.h
===================================================================
--- include-fixer/IncludeFixerContext.h
+++ include-fixer/IncludeFixerContext.h
@@ -20,6 +20,8 @@
struct IncludeFixerContext {
/// \brief The symbol name.
std::string SymbolIdentifier;
+ /// \bridf The absolute path to a file where the SymbolIdentifier comes from.
+ std::string FilePath;
/// \brief The headers which have SymbolIdentifier definitions.
std::vector<std::string> Headers;
};
Index: include-fixer/IncludeFixer.cpp
===================================================================
--- include-fixer/IncludeFixer.cpp
+++ include-fixer/IncludeFixer.cpp
@@ -37,7 +37,6 @@
std::unique_ptr<clang::ASTConsumer>
CreateASTConsumer(clang::CompilerInstance &Compiler,
StringRef InFile) override {
- Filename = InFile;
return llvm::make_unique<clang::ASTConsumer>();
}
@@ -86,29 +85,6 @@
if (getCompilerInstance().getSema().isSFINAEContext())
return clang::TypoCorrection();
- // We currently ignore the unidentified symbol which is not from the
- // main file.
- //
- // However, this is not always true due to templates in a non-self contained
- // header, consider the case:
- //
- // // header.h
- // template <typename T>
- // class Foo {
- // T t;
- // };
- //
- // // test.cc
- // // We need to add <bar.h> in test.cc instead of header.h.
- // class Bar;
- // Foo<Bar> foo;
- //
- // FIXME: Add the missing header to the header file where the symbol comes
- // from.
- if (!getCompilerInstance().getSourceManager().isWrittenInMainFile(
- Typo.getLoc()))
- return clang::TypoCorrection();
-
std::string TypoScopeString;
if (S) {
// FIXME: Currently we only use namespace contexts. Use other context
@@ -185,8 +161,6 @@
return clang::TypoCorrection();
}
- StringRef filename() const { return Filename; }
-
/// Get the minimal include for a given path.
std::string minimizeInclude(StringRef Include,
const clang::SourceManager &SourceManager,
@@ -217,6 +191,9 @@
clang::HeaderSearch &HeaderSearch) {
IncludeFixerContext FixerContext;
FixerContext.SymbolIdentifier = QuerySymbol;
+ SmallString<256> CleanedFilePath = StringRef(Filename);
+ llvm::sys::path::remove_dots(CleanedFilePath, /*remove_dot_dot=*/true);
+ FixerContext.FilePath = CleanedFilePath.str();
for (const auto &Header : SymbolQueryResults)
FixerContext.Headers.push_back(
minimizeInclude(Header, SourceManager, HeaderSearch));
@@ -238,15 +215,16 @@
DEBUG(llvm::dbgs() << " ...");
QuerySymbol = Query.str();
+ Filename = getCompilerInstance().getSourceManager().getFilename(Loc);
SymbolQueryResults = SymbolIndexMgr.search(Query);
DEBUG(llvm::dbgs() << SymbolQueryResults.size() << " replies\n");
return !SymbolQueryResults.empty();
}
/// The client to use to find cross-references.
SymbolIndexManager &SymbolIndexMgr;
- /// The absolute path to the file being processed.
+ /// The absolute path to a file where the QuerySymbol comes from.
std::string Filename;
/// The symbol being queried.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
