[include-fixer] Use scope contexts information to improve query.

[bkramer]

This needs more tests (check that using stuff from a different namespace in namespace scope still works). It's also better written as a unit test against the in-memory database, that way we don't have to create a file for every test.

[hokein]

Improvement and use unittest.

[bkramer]

Tests look much better now, thanks!

[hokein]

Rebase.

clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp

clang::TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo,
                                     CorrectionCandidateCallback &CCC,
                                     DeclContext *MemberContext,
                                     bool EnteringContext,
                                     const ObjCObjectPointerType *OPT) override {
     // Ignore spurious callbacks from SFINAE contexts.
     if (getCompilerInstance().getSema().isSFINAEContext())
       return clang::TypoCorrection();
 
+    std::string TypoScopeString;
+    if (S) {
+      // FIXME: Currently we only use namespace contexts. Use other context
+      // types for query.
+      for (const auto *Context = S->getEntity(); Context;
+           Context = Context->getParent()) {
+        if (const auto *ND = dyn_cast<NamespaceDecl>(Context)) {
+          if (!ND->getName().empty())
+            TypoScopeString = ND->getNameAsString() + "::" + TypoScopeString;
+        }
+      }
+    }
+
     /// If we have a scope specification, use that to get more precise results.
     std::string QueryString;
     if (SS && SS->getRange().isValid()) {
       auto Range = CharSourceRange::getTokenRange(SS->getRange().getBegin(),
                                                   Typo.getLoc());
       StringRef Source =
           Lexer::getSourceText(Range, getCompilerInstance().getSourceManager(),
                                getCompilerInstance().getLangOpts());
if (SS && SS->getRange().isValid()) {
       while (isIdentifierBody(*End) || *End == ':')
         ++End;
 
       QueryString = std::string(Source.begin(), End);
     } else {
       QueryString = Typo.getAsString();
     }
 
-    return query(QueryString, 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());
+
+    // 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
+    // identifier to something that's not wanted by the user.
+    return clang::TypoCorrection();
   }
 
   StringRef filename() const { return Filename; }
 
   /// Called for each include file we discover is in the file.
   /// \param SourceManager the active SourceManager
   /// \param canonical_path the canonical path to the include file
   /// \param uttered_path the path as it appeared in the program
public:
   /// \param include_path The include path to try.
   void TryInclude(const std::string &query, const std::string &include_path) {
     if (Untried.insert(include_path).second)
       UntriedList.push_back(include_path);
   }
 
 private:
   /// Query the database for a given identifier.
-  clang::TypoCorrection query(StringRef Query, SourceLocation Loc) {
+  bool query(StringRef Query, SourceLocation Loc) {
     assert(!Query.empty() && "Empty query!");
 
     // Save database lookups by not looking up identifiers multiple times.
     if (!SeenQueries.insert(Query).second)
-      return clang::TypoCorrection();
+      return true;
 
     DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at ");
     DEBUG(Loc.print(llvm::dbgs(), getCompilerInstance().getSourceManager()));
     DEBUG(llvm::dbgs() << " ...");
 
     std::string error_text;
     auto SearchReply = SymbolIndexMgr.search(Query);
     DEBUG(llvm::dbgs() << SearchReply.size() << " replies\n");
     if (SearchReply.empty())
-      return clang::TypoCorrection();
+      return false;
 
     // Add those files to the set of includes to try out.
     // FIXME: Rank the results and pick the best one instead of the first one.
     TryInclude(Query, SearchReply[0]);
 
-    // 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
-    // identifier to something that's not wanted by the user.
-    return clang::TypoCorrection();
+    return true;
   }
 
   /// The client to use to find cross-references.
   SymbolIndexManager &SymbolIndexMgr;
 
   // Remeber things we looked up to avoid querying things twice.
   llvm::StringSet<> SeenQueries;
 

clang-tools-extra/trunk/include-fixer/SymbolIndexManager.cpp

DEBUG(llvm::dbgs() << "Searching " << Names.back() << "... got "
                      << Symbols.size() << " results...\n");
 
   std::vector<std::string> Results;
   for (const auto &Symbol : Symbols) {
     // Match the identifier name without qualifier.
     if (Symbol.getName() == Names.back()) {
       bool IsMatched = true;
       auto SymbolContext = Symbol.getContexts().begin();
+      auto IdentiferContext = Names.rbegin() + 1; // Skip identifier name;
       // Match the remaining context names.
-      for (auto IdentiferContext = Names.rbegin() + 1;
-           IdentiferContext != Names.rend() &&
+      for (; IdentiferContext != Names.rend() &&
              SymbolContext != Symbol.getContexts().end();
            ++IdentiferContext, ++SymbolContext) {
         if (SymbolContext->second != *IdentiferContext) {
           IsMatched = false;
           break;
         }
       }
 
-      if (IsMatched) {
+      // FIXME: Support full match. At this point, we only find symbols in
+      // database which end with the same contexts with the identifier.
+      if (IsMatched && IdentiferContext == Names.rend()) {
         // 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

clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

std::vector<SymbolInfo> Symbols = {
       SymbolInfo("sting", SymbolInfo::SymbolKind::Class, "\"sting\"", 1,
                  {{SymbolInfo::ContextType::Namespace, "std"}}),
       SymbolInfo("size_type", SymbolInfo::SymbolKind::Variable, "<string>", 1,
                  {{SymbolInfo::ContextType::Namespace, "string"},
                   {SymbolInfo::ContextType::Namespace, "std"}}),
       SymbolInfo("foo", SymbolInfo::SymbolKind::Class, "\"dir/otherdir/qux.h\"",
                  1, {{SymbolInfo::ContextType::Namespace, "b"},
                      {SymbolInfo::ContextType::Namespace, "a"}}),
+      SymbolInfo("bar", SymbolInfo::SymbolKind::Class, "\"bar.h\"",
+                 1, {{SymbolInfo::ContextType::Namespace, "b"},
+                     {SymbolInfo::ContextType::Namespace, "a"}}),
   };
   auto SymbolIndexMgr = llvm::make_unique<include_fixer::SymbolIndexManager>();
   SymbolIndexMgr->addSymbolIndex(
       llvm::make_unique<include_fixer::InMemorySymbolIndex>(Symbols));
 
   std::vector<clang::tooling::Replacement> Replacements;
   IncludeFixerActionFactory Factory(*SymbolIndexMgr, Replacements);
   runOnCode(&Factory, Code, "input.cc", ExtraArgs);

 }
 #endif
 
 TEST(IncludeFixer, MultipleMissingSymbols) {
   EXPECT_EQ("#include <string>\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("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"));
+}
 } // namespace
 } // namespace include_fixer
 } // namespace clang