replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[include-fixer] Add mising qualifiers to all instances of an unidentified symbol. 

Reviewers: bkramer

Subscribers: ioeric, cfe-commits

Differential Revision: https://reviews.llvm.org/D22567

llvm-svn: 276280
This commit is contained in:
Haojian Wu 2016-07-21 13:47:09 +00:00
parent a9a89ae77f
commit 62aee528f6
8 changed files with 192 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
clang-tools-extra/include-fixer/IncludeFixer.cpp
View File

@ -227,17 +227,28 @@ public:
Symbol.getContexts(),
Symbol.getNumOccurrences());
}
return IncludeFixerContext(QuerySymbolInfo, SymbolCandidates);
return IncludeFixerContext(QuerySymbolInfos, SymbolCandidates);
}
private:
/// Query the database for a given identifier.
bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range Range) {
bool query(StringRef Query, StringRef ScopedQualifiers,
tooling::Range Range) {
assert(!Query.empty() && "Empty query!");
// Skip other identifiers once we have discovered an identfier successfully.
if (!MatchedSymbols.empty())
// Save all instances of an unidentified symbol.
//
// We use conservative behavior for detecting the same unidentified symbol
// here. The symbols which have the same ScopedQualifier and RawIdentifier
// are considered equal. So that include-fixer avoids false positives, and
// always adds missing qualifiers to correct symbols.
if (!QuerySymbolInfos.empty()) {
if (ScopedQualifiers == QuerySymbolInfos.front().ScopedQualifiers &&
Query == QuerySymbolInfos.front().RawIdentifier) {
QuerySymbolInfos.push_back({Query.str(), ScopedQualifiers, Range});
}
return false;
}
DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
DEBUG(getCompilerInstance()
@ -248,7 +259,7 @@ private:
.print(llvm::dbgs(), getCompilerInstance().getSourceManager()));
DEBUG(llvm::dbgs() << " ...");
QuerySymbolInfo = {Query.str(), ScopedQualifiers, Range};
QuerySymbolInfos.push_back({Query.str(), ScopedQualifiers, Range});
// Query the symbol based on C++ name Lookup rules.
// Firstly, lookup the identifier with scoped namespace contexts;
@ -274,8 +285,8 @@ private:
/// The client to use to find cross-references.
SymbolIndexManager &SymbolIndexMgr;
/// The symbol information.
IncludeFixerContext::QuerySymbolInfo QuerySymbolInfo;
/// The information of the symbols being queried.
std::vector<IncludeFixerContext::QuerySymbolInfo> QuerySymbolInfos;
/// All symbol candidates which match QuerySymbol. We only include the first
/// discovered identifier to avoid getting caught in results from error
@ -332,13 +343,13 @@ bool IncludeFixerActionFactory::runInvocation(
return !Compiler.getDiagnostics().hasFatalErrorOccurred();
}
llvm::Expected<tooling::Replacements>
createInsertHeaderReplacements(StringRef Code, StringRef FilePath,
StringRef Header,
const clang::format::FormatStyle &Style) {
if (Header.empty())
llvm::Expected<tooling::Replacements> createIncludeFixerReplacements(
StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
const clang::format::FormatStyle &Style, bool AddQualifiers) {
if (Context.getHeaderInfos().empty())
return tooling::Replacements();
std::string IncludeName = "#include " + Header.str() + "\n";
std::string IncludeName =
"#include " + Context.getHeaderInfos().front().Header + "\n";
// Create replacements for the new header.
clang::tooling::Replacements Insertions = {
tooling::Replacement(FilePath, UINT_MAX, 0, IncludeName)};
@ -346,6 +357,14 @@ createInsertHeaderReplacements(StringRef Code, StringRef FilePath,
auto CleanReplaces = cleanupAroundReplacements(Code, Insertions, Style);
if (!CleanReplaces)
return CleanReplaces;
if (AddQualifiers) {
for (const auto &Info : Context.getQuerySymbolInfos()) {
CleanReplaces->insert({FilePath, Info.Range.getOffset(),
Info.Range.getLength(),
Context.getHeaderInfos().front().QualifiedName});
}
}
return formatReplacements(Code, *CleanReplaces, Style);
}

22
clang-tools-extra/include-fixer/IncludeFixer.h
View File

@ -60,19 +60,25 @@ private:
std::string FallbackStyle;
};
/// Create replacements, which are generated by clang-format, for the header
/// insertion.
/// Create replacements, which are generated by clang-format, for the
/// missing header and mising qualifiers insertions. The function uses the
/// first header for insertion.
///
/// \param Code The source code.
/// \param FilePath The source file path.
/// \param Header The header being inserted.
/// \param Context The context which contains all information for creating
/// include-fixer replacements.
/// \param Style clang-format style being used.
/// \param AddQualifiers Whether we should add qualifiers to all instances of
/// an unidentified symbol.
///
/// \return Replacements for inserting and sorting headers on success;
/// otherwise, an llvm::Error carrying llvm::StringError is returned.
llvm::Expected<tooling::Replacements> createInsertHeaderReplacements(
StringRef Code, StringRef FilePath, StringRef Header,
const clang::format::FormatStyle &Style = clang::format::getLLVMStyle());
/// \return Formatted replacements for inserting, sorting headers and adding
/// qualifiers on success; otherwise, an llvm::Error carrying llvm::StringError
/// is returned.
llvm::Expected<tooling::Replacements> createIncludeFixerReplacements(
StringRef Code, StringRef FilePath, const IncludeFixerContext &Context,
const format::FormatStyle &Style = format::getLLVMStyle(),
bool AddQualifiers = true);
} // namespace include_fixer
} // namespace clang

25
clang-tools-extra/include-fixer/IncludeFixerContext.cpp
View File

@ -75,14 +75,33 @@ std::string createQualifiedNameForReplacement(
} // anonymous namespace
IncludeFixerContext::IncludeFixerContext(
const QuerySymbolInfo &QuerySymbol,
std::vector<QuerySymbolInfo> QuerySymbols,
std::vector<find_all_symbols::SymbolInfo> Symbols)
: MatchedSymbols(std::move(Symbols)), QuerySymbol(QuerySymbol) {
: QuerySymbolInfos(std::move(QuerySymbols)),
MatchedSymbols(std::move(Symbols)) {
// Remove replicated QuerySymbolInfos with the same range.
//
// QuerySymbolInfos may contain replicated elements. Because CorrectTypo
// callback doesn't always work as we expected. In somecases, it will be
// triggered at the same position or unidentified symbol multiple times.
std::sort(QuerySymbolInfos.begin(), QuerySymbolInfos.end(),
[&](const QuerySymbolInfo &A, const QuerySymbolInfo &B) {
if (A.Range.getOffset() != B.Range.getOffset())
return A.Range.getOffset() < B.Range.getOffset();
return A.Range.getLength() == B.Range.getLength();
});
QuerySymbolInfos.erase(
std::unique(QuerySymbolInfos.begin(), QuerySymbolInfos.end(),
[](const QuerySymbolInfo &A, const QuerySymbolInfo &B) {
return A.Range == B.Range;
}),
QuerySymbolInfos.end());
for (const auto &Symbol : MatchedSymbols) {
HeaderInfos.push_back(
{Symbol.getFilePath().str(),
createQualifiedNameForReplacement(
QuerySymbol.RawIdentifier, QuerySymbol.ScopedQualifiers, Symbol)});
QuerySymbolInfos.front().RawIdentifier,
QuerySymbolInfos.front().ScopedQualifiers, Symbol)});
}
// Deduplicate header infos.
HeaderInfos.erase(std::unique(HeaderInfos.begin(), HeaderInfos.end(),

20
clang-tools-extra/include-fixer/IncludeFixerContext.h
View File

@ -46,31 +46,39 @@ public:
};
IncludeFixerContext() = default;
IncludeFixerContext(const QuerySymbolInfo &QuerySymbol,
IncludeFixerContext(std::vector<QuerySymbolInfo> QuerySymbols,
std::vector<find_all_symbols::SymbolInfo> Symbols);
/// \brief Get symbol name.
llvm::StringRef getSymbolIdentifier() const {
return QuerySymbol.RawIdentifier;
return QuerySymbolInfos.front().RawIdentifier;
}
/// \brief Get replacement range of the symbol.
tooling::Range getSymbolRange() const { return QuerySymbol.Range; }
tooling::Range getSymbolRange() const {
return QuerySymbolInfos.front().Range;
}
/// \brief Get header information.
const std::vector<HeaderInfo> &getHeaderInfos() const { return HeaderInfos; }
/// \brief Get information of symbols being querid.
const std::vector<QuerySymbolInfo> &getQuerySymbolInfos() const {
return QuerySymbolInfos;
}
private:
friend struct llvm::yaml::MappingTraits<IncludeFixerContext>;
/// \brief All instances of an unidentified symbol being queried.
std::vector<QuerySymbolInfo> QuerySymbolInfos;
/// \brief The symbol candidates which match SymbolIdentifier. The symbols are
/// sorted in a descending order based on the popularity info in SymbolInfo.
std::vector<find_all_symbols::SymbolInfo> MatchedSymbols;
/// \brief The header information.
std::vector<HeaderInfo> HeaderInfos;
/// \brief The information of the symbol being queried.
QuerySymbolInfo QuerySymbol;
};
} // namespace include_fixer

71
clang-tools-extra/include-fixer/tool/ClangIncludeFixer.cpp
View File

@ -29,6 +29,7 @@ using clang::include_fixer::IncludeFixerContext;
LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(IncludeFixerContext)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(std::string)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(IncludeFixerContext::HeaderInfo)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(IncludeFixerContext::QuerySymbolInfo)
namespace llvm {
namespace yaml {
@ -70,7 +71,7 @@ template <> struct MappingTraits<IncludeFixerContext::QuerySymbolInfo> {
template <> struct MappingTraits<IncludeFixerContext> {
static void mapping(IO &IO, IncludeFixerContext &Context) {
IO.mapRequired("QuerySymbolInfo", Context.QuerySymbol);
IO.mapRequired("QuerySymbolInfos", Context.QuerySymbolInfos);
IO.mapRequired("HeaderInfos", Context.HeaderInfos);
}
};
@ -118,10 +119,10 @@ cl::opt<bool> OutputHeaders(
cl::desc("Print the symbol being queried and all its relevant headers in\n"
"JSON format to stdout:\n"
" {\n"
" \"QuerySymbolInfo\": {\n"
" \"RawIdentifier\": \"foo\",\n"
" \"Range\": {\"Offset\": 0, \"Length\": 3}\n"
" },\n"
" \"QuerySymbolInfos\": [\n"
" {\"RawIdentifier\": \"foo\",\n"
" \"Range\": {\"Offset\": 0, \"Length\": 3}}\n"
" ],\n"
" \"HeaderInfos\": [ {\"Header\": \"\\\"foo_a.h\\\"\",\n"
" \"QualifiedName\": \"a::foo\"} ]\n"
" }"),
@ -133,10 +134,10 @@ cl::opt<std::string> InsertHeader(
"The result is written to stdout. It is currently used for\n"
"editor integration. Support YAML/JSON format:\n"
" -insert-header=\"{\n"
" QuerySymbolInfo: {\n"
" RawIdentifier: foo,\n"
" Range: {Offset: 0, Length: 3}\n"
" },\n"
" QuerySymbolInfos: [\n"
" {RawIdentifier: foo,\n"
" Range: {Offset: 0, Length: 3}}\n"
" ],\n"
" HeaderInfos: [ {Headers: \"\\\"foo_a.h\\\"\",\n"
" QualifiedName: \"a::foo\"} ]}\""),
cl::init(""), cl::cat(IncludeFixerCategory));
@ -202,13 +203,16 @@ createSymbolIndexManager(StringRef FilePath) {
void writeToJson(llvm::raw_ostream &OS, const IncludeFixerContext& Context) {
OS << "{\n"
" \"QuerySymbolInfo\": {\n"
" \"RawIdentifier\": \""
<< Context.getSymbolIdentifier() << "\",\n";
OS << " \"Range\": {";
OS << " \"Offset\":" << Context.getSymbolRange().getOffset() << ",";
OS << " \"Length\":" << Context.getSymbolRange().getLength() << " }\n";
OS << " },\n";
" \"QuerySymbolInfos\": [\n";
for (const auto &Info : Context.getQuerySymbolInfos()) {
OS << " {\"RawIdentifier\": \"" << Info.RawIdentifier << "\",\n";
OS << " \"Range\":{";
OS << "\"Offset\":" << Info.Range.getOffset() << ",";
OS << "\"Length\":" << Info.Range.getLength() << "}}";
if (&Info != &Context.getQuerySymbolInfos().back())
OS << ",\n";
}
OS << "\n ],\n";
OS << " \"HeaderInfos\": [\n";
const auto &HeaderInfos = Context.getHeaderInfos();
for (const auto &Info : HeaderInfos) {
@ -275,16 +279,7 @@ int includeFixerMain(int argc, const char **argv) {
return 1;
}
auto Replacements = clang::include_fixer::createInsertHeaderReplacements(
Code->getBuffer(), FilePath, Context.getHeaderInfos().front().Header,
InsertStyle);
if (!Replacements) {
errs() << "Failed to create header insertion replacement: "
<< llvm::toString(Replacements.takeError()) << "\n";
return 1;
}
// If a header have multiple symbols, we won't add the missing namespace
// If a header has multiple symbols, we won't add the missing namespace
// qualifiers because we don't know which one is exactly used.
//
// Check whether all elements in HeaderInfos have the same qualified name.
@ -294,10 +289,16 @@ int includeFixerMain(int argc, const char **argv) {
const IncludeFixerContext::HeaderInfo &RHS) {
return LHS.QualifiedName == RHS.QualifiedName;
});
if (IsUniqueQualifiedName)
Replacements->insert({FilePath, Context.getSymbolRange().getOffset(),
Context.getSymbolRange().getLength(),
Context.getHeaderInfos().front().QualifiedName});
auto Replacements = clang::include_fixer::createIncludeFixerReplacements(
Code->getBuffer(), FilePath, Context, InsertStyle,
/*AddQualifiers=*/IsUniqueQualifiedName);
if (!Replacements) {
errs() << "Failed to create replacements: "
<< llvm::toString(Replacements.takeError()) << "\n";
return 1;
}
auto ChangedCode =
tooling::applyAllReplacements(Code->getBuffer(), *Replacements);
if (!ChangedCode) {
@ -340,9 +341,8 @@ int includeFixerMain(int argc, const char **argv) {
return 1;
}
auto Replacements = clang::include_fixer::createInsertHeaderReplacements(
/*Code=*/Buffer.get()->getBuffer(), FilePath,
Context.getHeaderInfos().front().Header, InsertStyle);
auto Replacements = clang::include_fixer::createIncludeFixerReplacements(
/*Code=*/Buffer.get()->getBuffer(), FilePath, Context, InsertStyle);
if (!Replacements) {
errs() << "Failed to create header insertion replacement: "
<< llvm::toString(Replacements.takeError()) << "\n";
@ -353,11 +353,6 @@ int includeFixerMain(int argc, const char **argv) {
errs() << "Added #include " << Context.getHeaderInfos().front().Header
<< '\n';
// Add missing namespace qualifiers to the unidentified symbol.
Replacements->insert({FilePath, Context.getSymbolRange().getOffset(),
Context.getSymbolRange().getLength(),
Context.getHeaderInfos().front().QualifiedName});
// Set up a new source manager for applying the resulting replacements.
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);
DiagnosticsEngine Diagnostics(new DiagnosticIDs, &*DiagOpts);

15
clang-tools-extra/include-fixer/tool/clang-include-fixer.py
View File

@ -127,8 +127,11 @@ def main():
return
include_fixer_context = json.loads(stdout)
query_symbol_info = include_fixer_context["QuerySymbolInfo"]
symbol = query_symbol_info["RawIdentifier"]
query_symbol_infos = include_fixer_context["QuerySymbolInfos"]
if not query_symbol_infos:
print "The file is fine, no need to add a header."
return
symbol = query_symbol_infos[0]["RawIdentifier"]
# The header_infos is already sorted by include-fixer.
header_infos = include_fixer_context["HeaderInfos"]
# Deduplicate headers while keeping the order, so that the same header would
@ -141,10 +144,6 @@ def main():
seen.add(header)
unique_headers.append(header)
if not symbol:
print "The file is fine, no need to add a header."
return
if not unique_headers:
print "Couldn't find a header for {0}.".format(symbol)
return
@ -152,7 +151,7 @@ def main():
try:
# If there is only one suggested header, insert it directly.
if len(unique_headers) == 1 or maximum_suggested_headers == 1:
InsertHeaderToVimBuffer({"QuerySymbolInfo": query_symbol_info,
InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
"HeaderInfos": header_infos}, text)
print "Added #include {0} for {1}.".format(unique_headers[0], symbol)
return
@ -163,7 +162,7 @@ def main():
header for header in header_infos if header["Header"] == selected]
# Insert a selected header.
InsertHeaderToVimBuffer({"QuerySymbolInfo": query_symbol_info,
InsertHeaderToVimBuffer({"QuerySymbolInfos": query_symbol_infos,
"HeaderInfos": selected_header_infos}, text)
print "Added #include {0} for {1}.".format(selected, symbol)
except Exception as error:

6
clang-tools-extra/test/include-fixer/commandline_options.cpp
View File

@ -1,9 +1,9 @@
// REQUIRES: shell
// RUN: echo "foo f;" > %t.cpp
// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s
// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfo: {RawIdentifier: foo, Range: {Offset: 0, Length: 3}}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfo: {RawIdentifier: foo, Range: {Offset: 0, Length: 3}}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfo: {RawIdentifier: foo, Range: {Offset: 0, Length: 3}}, HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"}]}' %t.cpp | FileCheck %s -check-prefix=CHECK-CODE
// RUN: cat %t.cpp | not clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "foo"},{Header: "\"foo2.h\"", QualifiedName: "foo"}]}' %t.cpp
// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='{QuerySymbolInfos: [{RawIdentifier: foo, Range: {Offset: 0, Length: 3}}], HeaderInfos: [{Header: "\"foo.h\"", QualifiedName: "a:foo"},{Header: "\"foo.h\"", QualifiedName: "b:foo"}]}' %t.cpp
//
// CHECK: "HeaderInfos": [
// CHECK-NEXT: {"Header": "\"foo.h\"",

74
clang-tools-extra/unittests/include-fixer/IncludeFixerTest.cpp
View File

@ -89,17 +89,14 @@ static std::string runIncludeFixer(
runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
if (FixerContext.getHeaderInfos().empty())
return Code;
auto Replaces = clang::include_fixer::createInsertHeaderReplacements(
Code, FakeFileName, FixerContext.getHeaderInfos().front().Header);
auto Replaces = clang::include_fixer::createIncludeFixerReplacements(
Code, FakeFileName, FixerContext);
EXPECT_TRUE(static_cast<bool>(Replaces))
<< llvm::toString(Replaces.takeError()) << "\n";
if (!Replaces)
return "";
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
Replaces->insert({FakeFileName, FixerContext.getSymbolRange().getOffset(),
FixerContext.getSymbolRange().getLength(),
FixerContext.getHeaderInfos().front().QualifiedName});
clang::tooling::applyAllReplacements(*Replaces, Context.Rewrite);
return Context.getRewrittenText(ID);
}
@ -211,12 +208,12 @@ TEST(IncludeFixer, InsertAndSortSingleHeader) {
std::string Code = "#include \"a.h\"\n"
"#include \"foo.h\"\n"
"\n"
"namespace a { b::bar b; }";
"namespace a {\nb::bar b;\n}\n";
std::string Expected = "#include \"a.h\"\n"
"#include \"bar.h\"\n"
"#include \"foo.h\"\n"
"\n"
"namespace a { b::bar b; }";
"namespace a {\nb::bar b;\n}\n";
EXPECT_EQ(Expected, runIncludeFixer(Code));
}
@ -275,6 +272,69 @@ TEST(IncludeFixer, FixNamespaceQualifiers) {
runIncludeFixer("namespace a {\n::a::b::bar b;\n}\n"));
}
TEST(IncludeFixer, FixNamespaceQualifiersForAllInstances) {
const char TestCode[] = R"(
namespace a {
bar b;
int func1() {
bar a;
bar *p = new bar();
return 0;
}
} // namespace a
namespace a {
bar func2() {
bar f;
return f;
}
} // namespace a
// Non-fixed cases:
void f() {
bar b;
}
namespace a {
namespace c {
bar b;
} // namespace c
} // namespace a
)";
const char ExpectedCode[] = R"(
#include "bar.h"
namespace a {
b::bar b;
int func1() {
b::bar a;
b::bar *p = new b::bar();
return 0;
}
} // namespace a
namespace a {
b::bar func2() {
b::bar f;
return f;
}
} // namespace a
// Non-fixed cases:
void f() {
bar b;
}
namespace a {
namespace c {
bar b;
} // namespace c
} // namespace a
)";
EXPECT_EQ(ExpectedCode, runIncludeFixer(TestCode));
}
} // namespace
} // namespace include_fixer
} // namespace clang