[clang-tools-extra] r274832 - [include-fixer] Add missing namespace qualifiers after inserting a missing header.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 8 02:10:29 PDT 2016


Author: hokein
Date: Fri Jul  8 04:10:29 2016
New Revision: 274832

URL: http://llvm.org/viewvc/llvm-project?rev=274832&view=rev
Log:
[include-fixer] Add missing namespace qualifiers after inserting a missing header.

Summary:
This is an initial version of fixing namespace issues by adding missing
namespace qualifiers to an unidentified symbol.

This version only fixes the first discovered unidentified symbol.
In the long run, include-fixer should fix all unidentified symbols
with a same name at one run.

Currently, it works on command-line tool. The vim integration is not
implemented yet.

Reviewers: klimek, bkramer, djasper

Subscribers: bkramer, ioeric, cfe-commits

Differential Revision: http://reviews.llvm.org/D21603

Modified:
    clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
    clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
    clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
    clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h
    clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp
    clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
    clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
    clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

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=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Fri Jul  8 04:10:29 2016
@@ -70,7 +70,11 @@ public:
       return false;
 
     clang::ASTContext &context = getCompilerInstance().getASTContext();
-    query(T.getUnqualifiedType().getAsString(context.getPrintingPolicy()), Loc);
+    std::string QueryString =
+        T.getUnqualifiedType().getAsString(context.getPrintingPolicy());
+    DEBUG(llvm::dbgs() << "Query missing complete type '" << QueryString
+                       << "'");
+    query(QueryString, "", tooling::Range());
     return false;
   }
 
@@ -152,32 +156,31 @@ public:
 
     /// If we have a scope specification, use that to get more precise results.
     std::string QueryString;
+    tooling::Range SymbolRange;
+    const auto &SM = getCompilerInstance().getSourceManager();
+    auto CreateToolingRange = [&QueryString, &SM](SourceLocation BeginLoc) {
+      return tooling::Range(SM.getDecomposedLoc(BeginLoc).second,
+                            QueryString.size());
+    };
     if (SS && SS->getRange().isValid()) {
       auto Range = CharSourceRange::getTokenRange(SS->getRange().getBegin(),
                                                   Typo.getLoc());
 
       QueryString = ExtendNestedNameSpecifier(Range);
+      SymbolRange = CreateToolingRange(Range.getBegin());
     } else if (Typo.getName().isIdentifier() && !Typo.getLoc().isMacroID()) {
       auto Range =
           CharSourceRange::getTokenRange(Typo.getBeginLoc(), Typo.getEndLoc());
 
       QueryString = ExtendNestedNameSpecifier(Range);
+      SymbolRange = CreateToolingRange(Range.getBegin());
     } else {
       QueryString = Typo.getAsString();
+      SymbolRange = CreateToolingRange(Typo.getLoc());
     }
 
-    // Follow C++ Lookup rules. Firstly, lookup the identifier with scoped
-    // namespace contexts. If fails, falls back to identifier.
-    // For example:
-    //
-    // namespace a {
-    // b::foo f;
-    // }
-    //
-    // 1. lookup a::b::foo.
-    // 2. lookup b::foo.
-    if (!query(TypoScopeString + QueryString, Typo.getLoc()))
-      query(QueryString, Typo.getLoc());
+    DEBUG(llvm::dbgs() << "TypoScopeQualifiers: " << TypoScopeString << "\n");
+    query(QueryString, TypoScopeString, SymbolRange);
 
     // FIXME: We should just return the name we got as input here and prevent
     // clang from trying to correct the typo by itself. That may change the
@@ -215,32 +218,63 @@ public:
   IncludeFixerContext
   getIncludeFixerContext(const clang::SourceManager &SourceManager,
                          clang::HeaderSearch &HeaderSearch) {
-    IncludeFixerContext FixerContext;
-    FixerContext.SymbolIdentifier = QuerySymbol;
-    for (const auto &Header : SymbolQueryResults)
-      FixerContext.Headers.push_back(
-          minimizeInclude(Header, SourceManager, HeaderSearch));
-
-    return FixerContext;
+    std::vector<find_all_symbols::SymbolInfo> SymbolCandidates;
+    for (const auto &Symbol : MatchedSymbols) {
+      std::string FilePath = Symbol.getFilePath().str();
+      std::string MinimizedFilePath = minimizeInclude(
+          ((FilePath[0] == '"' || FilePath[0] == '<') ? FilePath
+                                                      : "\"" + FilePath + "\""),
+          SourceManager, HeaderSearch);
+      SymbolCandidates.emplace_back(Symbol.getName(), Symbol.getSymbolKind(),
+                                    MinimizedFilePath, Symbol.getLineNumber(),
+                                    Symbol.getContexts(),
+                                    Symbol.getNumOccurrences());
+    }
+    return IncludeFixerContext(QuerySymbol, SymbolScopedQualifiers,
+                               SymbolCandidates, QuerySymbolRange);
   }
 
 private:
   /// Query the database for a given identifier.
-  bool query(StringRef Query, SourceLocation Loc) {
+  bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range Range) {
     assert(!Query.empty() && "Empty query!");
 
-    // Skip other identifers once we have discovered an identfier successfully.
-    if (!SymbolQueryResults.empty())
+    // Skip other identifiers once we have discovered an identfier successfully.
+    if (!MatchedSymbols.empty())
       return false;
 
     DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
-    DEBUG(Loc.print(llvm::dbgs(), getCompilerInstance().getSourceManager()));
+    DEBUG(getCompilerInstance()
+              .getSourceManager()
+              .getLocForStartOfFile(
+                  getCompilerInstance().getSourceManager().getMainFileID())
+              .getLocWithOffset(Range.getOffset())
+              .print(llvm::dbgs(), getCompilerInstance().getSourceManager()));
     DEBUG(llvm::dbgs() << " ...");
 
     QuerySymbol = Query.str();
-    SymbolQueryResults = SymbolIndexMgr.search(Query);
-    DEBUG(llvm::dbgs() << SymbolQueryResults.size() << " replies\n");
-    return !SymbolQueryResults.empty();
+    QuerySymbolRange = Range;
+    SymbolScopedQualifiers = ScopedQualifiers;
+
+    // Query the symbol based on C++ name Lookup rules.
+    // Firstly, lookup the identifier with scoped namespace contexts;
+    // If that fails, falls back to look up the identifier directly.
+    //
+    // For example:
+    //
+    // namespace a {
+    // b::foo f;
+    // }
+    //
+    // 1. lookup a::b::foo.
+    // 2. lookup b::foo.
+    std::string QueryString = ScopedQualifiers.str() + Query.str();
+    MatchedSymbols = SymbolIndexMgr.search(QueryString);
+    if (MatchedSymbols.empty() && !ScopedQualifiers.empty())
+      MatchedSymbols = SymbolIndexMgr.search(Query);
+    DEBUG(llvm::dbgs() << "Having found " << MatchedSymbols.size()
+                       << " symbols\n");
+    return !MatchedSymbols.empty();
   }
 
   /// The client to use to find cross-references.
@@ -252,9 +286,18 @@ private:
   /// The symbol being queried.
   std::string QuerySymbol;
 
-  /// The query results of an identifier. We only include the first discovered
-  /// identifier to avoid getting caught in results from error recovery.
-  std::vector<std::string> SymbolQueryResults;
+  /// The scoped qualifiers of QuerySymbol. It is represented as a sequence of
+  /// names and scope resolution operatiors ::, ending with a scope resolution
+  /// operator (e.g. a::b::). Empty if the symbol is not in a specific scope.
+  std::string SymbolScopedQualifiers;
+
+  /// The replacement range of the first discovered QuerySymbol.
+  tooling::Range QuerySymbolRange;
+
+  /// All symbol candidates which match QuerySymbol. We only include the first
+  /// discovered identifier to avoid getting caught in results from error
+  /// recovery.
+  std::vector<find_all_symbols::SymbolInfo> MatchedSymbols;
 
   /// Whether we should use the smallest possible include path.
   bool MinimizeIncludePaths = true;

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h?rev=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h Fri Jul  8 04:10:29 2016
@@ -10,6 +10,9 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXERCONTEXT_H
 #define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXERCONTEXT_H
 
+#include "find-all-symbols/SymbolInfo.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include <algorithm>
 #include <string>
 #include <vector>
 
@@ -17,11 +20,89 @@ namespace clang {
 namespace include_fixer {
 
 /// \brief A context for the symbol being queried.
-struct IncludeFixerContext {
+class IncludeFixerContext {
+public:
+  IncludeFixerContext() {}
+  IncludeFixerContext(llvm::StringRef Name, llvm::StringRef ScopeQualifiers,
+                      const std::vector<find_all_symbols::SymbolInfo> Symbols,
+                      tooling::Range Range)
+      : SymbolIdentifier(Name), SymbolScopedQualifiers(ScopeQualifiers),
+        MatchedSymbols(Symbols), SymbolRange(Range) {
+    // Deduplicate headers, so that we don't want to suggest the same header
+    // twice.
+    for (const auto &Symbol : MatchedSymbols)
+      Headers.push_back(Symbol.getFilePath());
+    Headers.erase(std::unique(Headers.begin(), Headers.end(),
+                              [](const std::string &A, const std::string &B) {
+                                return A == B;
+                              }),
+                  Headers.end());
+  }
+
+  /// \brief Create a replacement for adding missing namespace qualifiers to the
+  /// symbol.
+  tooling::Replacement createSymbolReplacement(llvm::StringRef FilePath,
+                                               size_t Idx = 0) {
+    assert(Idx < MatchedSymbols.size());
+    std::string QualifiedName = MatchedSymbols[Idx].getQualifiedName();
+    // For nested classes, the qualified name constructed from database misses
+    // some stripped qualifiers, because when we search a symbol in database,
+    // we strip qualifiers from the end until we find a result. So append the
+    // missing stripped qualifiers here.
+    //
+    // Get stripped qualifiers.
+    llvm::SmallVector<llvm::StringRef, 8> SymbolQualifiers;
+    getSymbolIdentifier().split(SymbolQualifiers, "::");
+    std::string StrippedQualifiers;
+    while (!SymbolQualifiers.empty() &&
+           !llvm::StringRef(QualifiedName).endswith(SymbolQualifiers.back())) {
+      StrippedQualifiers= "::" + SymbolQualifiers.back().str();
+      SymbolQualifiers.pop_back();
+    }
+    // Append the missing stripped qualifiers.
+    std::string FullyQualifiedName = QualifiedName + StrippedQualifiers;
+    auto pos = FullyQualifiedName.find(SymbolScopedQualifiers);
+    return {FilePath, SymbolRange.getOffset(), SymbolRange.getLength(),
+            FullyQualifiedName.substr(
+                pos == std::string::npos ? 0 : SymbolScopedQualifiers.size())};
+  }
+
+  /// \brief Get symbol name.
+  llvm::StringRef getSymbolIdentifier() const { return SymbolIdentifier; }
+
+  /// \brief Get replacement range of the symbol.
+  tooling::Range getSymbolRange() const { return SymbolRange; }
+
+  /// \brief Get all matched symbols.
+  const std::vector<find_all_symbols::SymbolInfo> &getMatchedSymbols() const {
+    return MatchedSymbols;
+  }
+
+  /// \brief Get all headers. The headers are sorted in a descending order based
+  /// on the popularity info in SymbolInfo.
+  const std::vector<std::string> &getHeaders() const { return Headers; }
+
+private:
+  friend struct llvm::yaml::MappingTraits<IncludeFixerContext>;
+
   /// \brief The symbol name.
   std::string SymbolIdentifier;
+
+  /// \brief The qualifiers of the scope in which SymbolIdentifier lookup
+  /// occurs. It is represented as a sequence of names and scope resolution
+  /// operatiors ::, ending with a scope resolution operator (e.g. a::b::).
+  /// Empty if SymbolIdentifier is not in a specific scope.
+  std::string SymbolScopedQualifiers;
+
   /// \brief The headers which have SymbolIdentifier definitions.
   std::vector<std::string> Headers;
+
+  /// \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 replacement range of SymbolIdentifier.
+  tooling::Range SymbolRange;
 };
 
 } // namespace include_fixer

Modified: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp?rev=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp Fri Jul  8 04:10:29 2016
@@ -20,7 +20,7 @@ namespace include_fixer {
 
 using clang::find_all_symbols::SymbolInfo;
 
-/// Sorts and uniques SymbolInfos based on the popularity info in SymbolInfo.
+/// Sorts SymbolInfos based on the popularity info in SymbolInfo.
 static void rankByPopularity(std::vector<SymbolInfo> &Symbols) {
   // First collect occurrences per header file.
   llvm::DenseMap<llvm::StringRef, unsigned> HeaderPopularity;
@@ -39,17 +39,9 @@ static void rankByPopularity(std::vector
                 return APop > BPop;
               return A.getFilePath() < B.getFilePath();
             });
-
-  // Deduplicate based on the file name. They will have the same popularity and
-  // we don't want to suggest the same header twice.
-  Symbols.erase(std::unique(Symbols.begin(), Symbols.end(),
-                            [](const SymbolInfo &A, const SymbolInfo &B) {
-                              return A.getFilePath() == B.getFilePath();
-                            }),
-                Symbols.end());
 }
 
-std::vector<std::string>
+std::vector<find_all_symbols::SymbolInfo>
 SymbolIndexManager::search(llvm::StringRef Identifier) const {
   // The identifier may be fully qualified, so split it and get all the context
   // names.
@@ -127,20 +119,7 @@ SymbolIndexManager::search(llvm::StringR
   }
 
   rankByPopularity(MatchedSymbols);
-  std::vector<std::string> Results;
-  for (const auto &Symbol : MatchedSymbols) {
-    // FIXME: file path should never be in the form of <...> or "...", but
-    // the unit test with fixed database use <...> file path, which might
-    // need to be changed.
-    // FIXME: if the file path is a system header name, we want to use
-    // angle brackets.
-    std::string FilePath = Symbol.getFilePath().str();
-    Results.push_back((FilePath[0] == '"' || FilePath[0] == '<')
-                          ? FilePath
-                          : "\"" + FilePath + "\"");
-  }
-
-  return Results;
+  return MatchedSymbols;
 }
 
 } // namespace include_fixer

Modified: clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h?rev=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h (original)
+++ clang-tools-extra/trunk/include-fixer/SymbolIndexManager.h Fri Jul  8 04:10:29 2016
@@ -11,6 +11,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_SYMBOLINDEXMANAGER_H
 
 #include "SymbolIndex.h"
+#include "find-all-symbols/SymbolInfo.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
@@ -29,11 +30,10 @@ public:
   ///                   fully qualified.
   /// \returns A list of inclusion candidates, in a format ready for being
   ///          pasted after an #include token.
-  // FIXME: Expose the type name so we can also insert using declarations (or
-  // fix the usage)
   // FIXME: Move mapping from SymbolInfo to headers out of
   // SymbolIndexManager::search and return SymbolInfos instead of header paths.
-  std::vector<std::string> search(llvm::StringRef Identifier) const;
+  std::vector<find_all_symbols::SymbolInfo>
+  search(llvm::StringRef Identifier) const;
 
 private:
   std::vector<std::unique_ptr<SymbolIndex>> SymbolIndices;

Modified: clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp?rev=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.cpp Fri Jul  8 04:10:29 2016
@@ -90,6 +90,16 @@ bool SymbolInfo::operator<(const SymbolI
                   Symbol.Contexts);
 }
 
+std::string SymbolInfo::getQualifiedName() const {
+  std::string QualifiedName = Name;
+  for (const auto &Context : Contexts) {
+    if (Context.first == ContextType::EnumDecl)
+      continue;
+    QualifiedName = Context.second + "::" + QualifiedName;
+  }
+  return QualifiedName;
+}
+
 bool WriteSymbolInfosToStream(llvm::raw_ostream &OS,
                               const std::set<SymbolInfo> &Symbols) {
   llvm::yaml::Output yout(OS);

Modified: clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h?rev=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h (original)
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h Fri Jul  8 04:10:29 2016
@@ -57,6 +57,9 @@ public:
   /// \brief Get symbol name.
   llvm::StringRef getName() const { return Name; }
 
+  /// \brief Get the fully-qualified symbol name.
+  std::string getQualifiedName() const;
+
   /// \brief Get symbol type.
   SymbolKind getSymbolKind() const { return Type; }
 

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=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp Fri Jul  8 04:10:29 2016
@@ -16,6 +16,7 @@
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -154,11 +155,11 @@ createSymbolIndexManager(StringRef FileP
 
 void writeToJson(llvm::raw_ostream &OS, const IncludeFixerContext& Context) {
   OS << "{\n"
-        "  \"SymbolIdentifier\": \"" << Context.SymbolIdentifier << "\",\n"
+        "  \"SymbolIdentifier\": \"" << Context.getSymbolIdentifier() << "\",\n"
         "  \"Headers\": [ ";
-  for (const auto &Header : Context.Headers) {
+  for (const auto &Header : Context.getHeaders()) {
     OS << " \"" << llvm::yaml::escape(Header) << "\"";
-    if (Header != Context.Headers.back())
+    if (Header != Context.getHeaders().back())
       OS << ", ";
   }
   OS << " ]\n"
@@ -203,14 +204,15 @@ int includeFixerMain(int argc, const cha
     IncludeFixerContext Context;
     yin >> Context;
 
-    if (Context.Headers.size() != 1) {
+    if (Context.getHeaders().size() != 1) {
       errs() << "Expect exactly one inserted header.\n";
       return 1;
     }
 
     tooling::Replacements Replacements =
         clang::include_fixer::createInsertHeaderReplacements(
-            Code->getBuffer(), FilePath, Context.Headers[0], InsertStyle);
+            Code->getBuffer(), FilePath, Context.getHeaders().front(),
+            InsertStyle);
     std::string ChangedCode =
         tooling::applyAllReplacements(Code->getBuffer(), Replacements);
     llvm::outs() << ChangedCode;
@@ -239,7 +241,7 @@ int includeFixerMain(int argc, const cha
     return 0;
   }
 
-  if (Context.Headers.empty())
+  if (Context.getMatchedSymbols().empty())
     return 0;
 
   auto Buffer = llvm::MemoryBuffer::getFile(FilePath);
@@ -248,14 +250,17 @@ int includeFixerMain(int argc, const cha
     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(), FilePath,
+          Context.getHeaders().front(), InsertStyle);
 
   if (!Quiet)
-    llvm::errs() << "Added #include" << Context.Headers.front();
+    llvm::errs() << "Added #include" << Context.getHeaders().front();
+
+  // Add missing namespace qualifiers to the unidentified symbol.
+  if (Context.getSymbolRange().getLength() > 0)
+    Replacements.insert(Context.createSymbolReplacement(FilePath, 0));
 
   // Set up a new source manager for applying the resulting replacements.
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions);

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=274832&r1=274831&r2=274832&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Fri Jul  8 04:10:29 2016
@@ -82,14 +82,17 @@ static std::string runIncludeFixer(
   IncludeFixerContext FixerContext;
   IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
 
-  runOnCode(&Factory, Code, "input.cc", ExtraArgs);
-  if (FixerContext.Headers.empty())
+  std::string FakeFileName = "input.cc";
+  runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
+  if (FixerContext.getMatchedSymbols().empty())
     return Code;
   tooling::Replacements Replacements =
       clang::include_fixer::createInsertHeaderReplacements(
-          Code, "input.cc", FixerContext.Headers.front());
+          Code, FakeFileName, FixerContext.getHeaders().front());
   clang::RewriterTestContext Context;
-  clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
+  clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
+  if (FixerContext.getSymbolRange().getLength() > 0)
+    Replacements.insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
   clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
   return Context.getRewrittenText(ID);
 }
@@ -113,15 +116,9 @@ TEST(IncludeFixer, Typo) {
       "#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>\nstd::string foo;\n",
             runIncludeFixer("string foo;\n"));
 
-  // Fully qualified name.
-  EXPECT_EQ("#include <string>\n::std::string foo;\n",
-            runIncludeFixer("::std::string foo;\n"));
   // Should not match std::string.
   EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
 }
@@ -129,7 +126,7 @@ TEST(IncludeFixer, Typo) {
 TEST(IncludeFixer, IncompleteType) {
   EXPECT_EQ(
       "#include \"foo.h\"\n#include <string>\n"
-      "namespace std {\nclass string;\n}\nstring foo;\n",
+      "namespace std {\nclass string;\n}\nstd::string foo;\n",
       runIncludeFixer("#include \"foo.h\"\n"
                       "namespace std {\nclass string;\n}\nstring foo;\n"));
 }
@@ -186,7 +183,7 @@ TEST(IncludeFixer, ScopedNamespaceSymbol
             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 {\nb::bar b;\n}",
+  EXPECT_EQ("#include \"bar.h\"\nnamespace A {\na::b::bar b;\n}",
             runIncludeFixer("namespace A {\nb::bar b;\n}"));
 }
 
@@ -221,6 +218,44 @@ TEST(IncludeFixer, DoNotDeleteMatchedSym
             runIncludeFixer("a::Vector v;"));
 }
 
+TEST(IncludeFixer, FixNamespaceQualifiers) {
+  EXPECT_EQ("#include \"bar.h\"\na::b::bar b;\n",
+            runIncludeFixer("b::bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\na::b::bar b;\n",
+            runIncludeFixer("a::b::bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\na::b::bar b;\n",
+            runIncludeFixer("bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar b;\n}\n",
+            runIncludeFixer("namespace a {\nb::bar b;\n}\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar b;\n}\n",
+            runIncludeFixer("namespace a {\nbar b;\n}\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nnamespace b{\nbar b;\n}\n}\n",
+            runIncludeFixer("namespace a {\nnamespace b{\nbar b;\n}\n}\n"));
+  EXPECT_EQ("c::b::bar b;\n",
+            runIncludeFixer("c::b::bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace c {\na::b::bar b;\n}\n",
+            runIncludeFixer("namespace c {\nbar b;\n}\n"));
+
+  // Test nested classes.
+  EXPECT_EQ("#include \"bar.h\"\nnamespace c {\na::b::bar::t b;\n}\n",
+            runIncludeFixer("namespace c {\nbar::t b;\n}\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar::t b;\n}\n",
+            runIncludeFixer("namespace a {\nbar::t b;\n}\n"));
+
+  EXPECT_EQ(
+      "#include \"color.h\"\nint test = a::b::Green;\n",
+      runIncludeFixer("int test = Green;\n"));
+  EXPECT_EQ("#include \"color.h\"\nnamespace d {\nint test = a::b::Green;\n}\n",
+            runIncludeFixer("namespace d {\nint test = Green;\n}\n"));
+  EXPECT_EQ("#include \"color.h\"\nnamespace a {\nint test = b::Green;\n}\n",
+            runIncludeFixer("namespace a {\nint test = Green;\n}\n"));
+
+  // FIXME: Fix-namespace should not fix the global qualified identifier.
+  EXPECT_EQ(
+      "#include \"bar.h\"\na::b::bar b;\n",
+      runIncludeFixer("::a::b::bar b;\n"));
+}
+
 } // namespace
 } // namespace include_fixer
 } // namespace clang




More information about the cfe-commits mailing list