[clang-rename] introduce better symbol finding and add few more tests

[omtcyfz]

Summary

This patch introduces:

• TypeLoc visiting, which helps a lot in renaming types
• NestedNameSpecifierLoc visiting (through getting them via ASTMatcher at the moment, though, because RecursiveASTVisitor<T>::VisitNestedNameSpecifierLoc isn't implemented), which helps to treat nested names correctly
• few refactoring changes

[omtcyfz]

add XFAIL to currently unsupported test

[omtcyfz]

Removed two FIXMEs. Seems like types are now handled correctly. +1 test passing.

[omtcyfz]

applied few stylistic fixes

[omtcyfz]

Can anyone please take a look at this?

My current work is based on this patch and I'd be happy to know I'm doing things right :)

[omtcyfz]

add support for renaming inside NestedNameSpecifier's

[omtcyfz]

ping

[omtcyfz]

few fixes

[omtcyfz]

fix previously unsupported test

[omtcyfz]

add one more XFAIL test

[alexfh]

clang-rename/USRFinder.cpp

79

Declarations of multiple variables can be confusing. Please split this declaration.

94–96

nit: No need for braces around single-line if bodies.

clang-rename/USRFinder.h

49

We should implement it then ;)

test/clang-rename/ComplicatedClassType.cpp

1–2

I guess, this test will need // REQUIRES: shell. You can try to commit without it, but watch windows buildbots closely.

[omtcyfz]

Address comments.

[omtcyfz]

clang-rename/USRFinder.cpp

94–96

Well, I'm really happy with braces everywhere. The codestyle doesn't prohibit it and I think it's more readable than no braces around single statements in logical blocks.

clang-rename/USRFinder.h

49

Yes, it's in my TODO list. But I suppose it's better to fix clang-rename first and get back to it a bit later.

As we discussed, there will probably be no need in RecursiveASTVisitors at all in the end.

Anyway, the patch is already in the state when nobody wants to review it :D I'd be happy just to land it and to continue with all the other bugs present in the tool.

test/clang-rename/ComplicatedClassType.cpp

1–2

This test? There's RUN: cat %s > %t.cpp in every single one of them, I'm not sure what's the issue.

[omtcyfz]

• reduce rate of hardcoding
• simplify symbol location finding
• introduce tests with templates, some of them are PASSing

[omtcyfz]

split declarations

[omtcyfz]

oops... actually split declarations

[omtcyfz]

remove redundant #include and using directives

[alexfh]

LG. Thanks!

clang-rename/USRFinder.h

49

SG. Please change the comment to // FIXME: Implement RecursiveASTVisitor<T>::VisitNestedNameSpecifier instead..

test/clang-rename/ComplicatedClassType.cpp

1–6

Actually, cat a > b is a way to write cp a b in lit tests without requiring a shell. I looked this up and was going to delete the comment, but I've hit "Hide" instead. Sorry for the noise.

[omtcyfz]

@alexfh, awww, sorry, I've uploaded wrong diff last time for whatever reason...

Can you review the changes please?

Now it contains many new tests and I managed to cut off unnecessary blocks of code.

[omtcyfz]

@alexfh, or wait... It's fine, I just forgot to add few tests, sorry.

test/clang-rename/ComplicatedClassType.cpp

1–7

No problem :)

clang-rename/RenamingAction.cpp

for (const auto &USR : USRs) {
       NewCandidates = getLocationsOfUSR(USR, PrevName,
                                         Context.getTranslationUnitDecl());
       RenamingCandidates.insert(RenamingCandidates.end(), NewCandidates.begin(),
                                 NewCandidates.end());
       NewCandidates.clear();
     }
 
     auto PrevNameLen = PrevName.length();
-    if (PrintLocations)
-      for (const auto &Loc : RenamingCandidates) {
+      if (PrintLocations) {
         FullSourceLoc FullLoc(Loc, SourceMgr);
         errs() << "clang-rename: renamed at: " << SourceMgr.getFilename(Loc)
                << ":" << FullLoc.getSpellingLineNumber() << ":"
                << FullLoc.getSpellingColumnNumber() << "\n";
-        Replaces.insert(tooling::Replacement(SourceMgr, Loc, PrevNameLen,
-                                             NewName));
       }
-    else
-      for (const auto &Loc : RenamingCandidates)
-        Replaces.insert(tooling::Replacement(SourceMgr, Loc, PrevNameLen,
-                                             NewName));
     }
+  }
 
 private:
   const std::string &NewName, &PrevName;
   const std::vector<std::string> &USRs;
   tooling::Replacements &Replaces;
   bool PrintLocations;
 };
 
 std::unique_ptr<ASTConsumer> RenamingAction::newASTConsumer() {
   return llvm::make_unique<RenamingASTConsumer>(NewName, PrevName, USRs,
                                                 Replaces, PrintLocations);
 }
 
 } // namespace rename
 } // namespace clang

clang-rename/USRFinder.h

 /// \file
 /// \brief Methods for determining the USR of a symbol at a location in source
 /// code.
 ///
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_FINDER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_FINDER_H
 
+#include "clang/AST/AST.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include <string>
 
+using namespace llvm;
+using namespace clang::ast_matchers;
+
 namespace clang {
 class ASTContext;
 class Decl;
 class SourceLocation;
 class NamedDecl;
 
 namespace rename {
 
 // Given an AST context and a point, returns a NamedDecl identifying the symbol
 // at the point. Returns null if nothing is found at the point.
 const NamedDecl *getNamedDeclAt(const ASTContext &Context,
                                 const SourceLocation Point);
 
 // Given an AST context and a fully qualified name, returns a NamedDecl
 // identifying the symbol with a matching name. Returns null if nothing is
 // found for the name.
 const NamedDecl *getNamedDeclFor(const ASTContext &Context,
                                  const std::string &Name);
 
 // Converts a Decl into a USR.
 std::string getUSRForDecl(const Decl *Decl);
 
+// FIXME: This wouldn't be needed if
+// RecursiveASTVisitor<T>::VisitNestedNameSpecifier would have been implemented.
+class NestedNameSpecifierLocFinder : public MatchFinder::MatchCallback {
+public:
+  explicit NestedNameSpecifierLocFinder(ASTContext &Context)
+    : Context(Context) {}
+
+  std::vector<NestedNameSpecifierLoc> getNestedNameSpecifierLocations() {
+    addMatchers();
+    Finder.matchAST(Context);
+    return Locations;
+  }
+
+private:
+  void addMatchers() {
+    const auto NestedNameSpecifierLocMatcher =
+        nestedNameSpecifierLoc().bind("nestedNameSpecifierLoc");
+    Finder.addMatcher(NestedNameSpecifierLocMatcher, this);
+  }
+
+  virtual void run(const MatchFinder::MatchResult &Result) {
+    const auto *NNS =
+        Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameSpecifierLoc");
+    Locations.push_back(*NNS);
+  }
+
+  ASTContext &Context;
+  std::vector<NestedNameSpecifierLoc> Locations;
+  MatchFinder Finder;
+};
+
 }
 }
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_FINDER_H

clang-rename/USRFinder.cpp

 /// point.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "USRFinder.h"
 #include "clang/AST/AST.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/SmallVector.h"
 
 using namespace llvm;
+using namespace clang::ast_matchers;
 
 namespace clang {
 namespace rename {
 
 // NamedDeclFindingASTVisitor recursively visits each AST node to find the
 // symbol underneath the cursor.
 // FIXME: move to seperate .h/.cc file if this gets too large.
 namespace {
 class NamedDeclFindingASTVisitor
     : public clang::RecursiveASTVisitor<NamedDeclFindingASTVisitor> {
 public:
   // \brief Finds the NamedDecl at a point in the source.
   // \param Point the location in the source to search for the NamedDecl.
   explicit NamedDeclFindingASTVisitor(const SourceManager &SourceMgr,
-                                      const SourceLocation Point)
-      : Result(nullptr), SourceMgr(SourceMgr),
-        Point(Point) {
-  }
+                                      const SourceLocation Point,
+                                      const ASTContext *Context)
+      : Result(nullptr), SourceMgr(SourceMgr), Point(Point), Context(Context) {}
 
   // \brief Finds the NamedDecl for a name in the source.
   // \param Name the fully qualified name.
   explicit NamedDeclFindingASTVisitor(const SourceManager &SourceMgr,
                                       const std::string &Name)
-      : Result(nullptr), SourceMgr(SourceMgr),
-        Name(Name) {
-  }
+      : Result(nullptr), SourceMgr(SourceMgr), Name(Name) {}
 
   // Declaration visitors:
 
   // \brief Checks if the point falls within the NameDecl. This covers every
   // declaration of a named entity that we may come across. Usually, just
   // checking if the point lies within the length of the name of the declaration
   // and the start location is sufficient.
   bool VisitNamedDecl(const NamedDecl *Decl) {
     return setResult(Decl, Decl->getLocation(),
                      Decl->getNameAsString().length());
   }
 
   // Expression visitors:
 
   bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
-    // Check the namespace specifier first.
-    if (!checkNestedNameSpecifierLoc(Expr->getQualifierLoc()))
-      return false;
-
     const auto *Decl = Expr->getFoundDecl();
     return setResult(Decl, Expr->getLocation(),
                      Decl->getNameAsString().length());
   }
 
   bool VisitMemberExpr(const MemberExpr *Expr) {
     const auto *Decl = Expr->getFoundDecl().getDecl();
     return setResult(Decl, Expr->getMemberLoc(),
                      Decl->getNameAsString().length());
   }
 
-  // Other:
+  // Other visitors:
 
-  const NamedDecl *getNamedDecl() {
-    return Result;
+  bool VisitTypeLoc(const TypeLoc Loc) {
+    const auto TypeBeginLoc = Loc.getBeginLoc(),
+               TypeEndLoc = Lexer::getLocForEndOfToken(
+                   TypeBeginLoc, 0, SourceMgr, Context->getLangOpts());
+    return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc,
+                     TypeEndLoc);
   }
 
-private:
+  // Other:
+
+  const NamedDecl *getNamedDecl() { return Result; }
+
   // \brief Determines if a namespace qualifier contains the point.
   // \returns false on success and sets Result.
-  bool checkNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
+  void handleNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
     while (NameLoc) {
       const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
-      if (Decl && !setResult(Decl, NameLoc.getLocalBeginLoc(),
-                             Decl->getNameAsString().length()))
-        return false;
+      if (Decl) {
+        setResult(Decl, NameLoc.getLocalBeginLoc(), NameLoc.getLocalEndLoc());
+      }
       NameLoc = NameLoc.getPrefix();
     }
-    return true;
   }
 
+private:
   // \brief Sets Result to Decl if the Point is within Start and End.
   // \returns false on success.
   bool setResult(const NamedDecl *Decl, SourceLocation Start,
                  SourceLocation End) {
     if (Name.empty()) {
       // Offset is used to find the declaration.
       if (!Start.isValid() || !Start.isFileID() || !End.isValid() ||
           !End.isFileID() || !isPointWithin(Start, End)) {
         return true;
       }
     } else {
       // Fully qualified name is used to find the declaration.
       if (Name != Decl->getQualifiedNameAsString()) {
         return true;
       }
     }
     Result = Decl;
     return false;
   }
 
   // \brief Sets Result to Decl if Point is within Loc and Loc + Offset.
   // \returns false on success.
-  bool setResult(const NamedDecl *Decl, SourceLocation Loc,
-                 unsigned Offset) {
+  bool setResult(const NamedDecl *Decl, SourceLocation Loc, unsigned Offset) {
     // FIXME: Add test for Offset == 0. Add test for Offset - 1 (vs -2 etc).
     return Offset == 0 ||
            setResult(Decl, Loc, Loc.getLocWithOffset(Offset - 1));
   }
 
   // \brief Determines if the Point is within Start and End.
   bool isPointWithin(const SourceLocation Start, const SourceLocation End) {
     // FIXME: Add tests for Point == End.
     return Point == Start || Point == End ||
            (SourceMgr.isBeforeInTranslationUnit(Start, Point) &&
             SourceMgr.isBeforeInTranslationUnit(Point, End));
   }
 
   const NamedDecl *Result;
   const SourceManager &SourceMgr;
   const SourceLocation Point; // The location to find the NamedDecl.
   const std::string Name;
+  const ASTContext *Context;
 };
 } // namespace
 
 const NamedDecl *getNamedDeclAt(const ASTContext &Context,
                                 const SourceLocation Point) {
   const auto &SourceMgr = Context.getSourceManager();
   const auto SearchFile = SourceMgr.getFilename(Point);
 
-  NamedDeclFindingASTVisitor Visitor(SourceMgr, Point);
+  NamedDeclFindingASTVisitor Visitor(SourceMgr, Point, &Context);
 
   // We only want to search the decls that exist in the same file as the point.
   auto Decls = Context.getTranslationUnitDecl()->decls();
   for (auto &CurrDecl : Decls) {
     const auto FileLoc = CurrDecl->getLocStart();
     const auto FileName = SourceMgr.getFilename(FileLoc);
     // FIXME: Add test.
     if (FileName == SearchFile) {
       Visitor.TraverseDecl(CurrDecl);
-      if (const NamedDecl *Result = Visitor.getNamedDecl()) {
-        return Result;
-      }
-    }
+
+  NestedNameSpecifierLocFinder Finder(const_cast<ASTContext &>(Context));
+  for (const auto &Location : Finder.getNestedNameSpecifierLocations()) {
+    Visitor.handleNestedNameSpecifierLoc(Location);
   }
 
-  return nullptr;
+  return Visitor.getNamedDecl();
 }
 
 const NamedDecl *getNamedDeclFor(const ASTContext &Context,
                                  const std::string &Name) {
   const auto &SourceMgr = Context.getSourceManager();
   NamedDeclFindingASTVisitor Visitor(SourceMgr, Name);
-  auto Decls = Context.getTranslationUnitDecl()->decls();
-
-  for (auto &CurrDecl : Decls) {
-    Visitor.TraverseDecl(CurrDecl);
-    if (const NamedDecl *Result = Visitor.getNamedDecl()) {
-      return Result;
-    }
-  }
+  Visitor.TraverseDecl(Context.getTranslationUnitDecl());
 
-  return nullptr;
+  return Visitor.getNamedDecl();
 }
 
 std::string getUSRForDecl(const Decl *Decl) {
   llvm::SmallVector<char, 128> Buff;
 
   // FIXME: Add test for the nullptr case.
   if (Decl == nullptr || index::generateUSRForDecl(Decl, Buff))
     return "";
 
   return std::string(Buff.data(), Buff.size());
 }
 
 } // namespace rename
 } // namespace clang

clang-rename/USRFindingAction.cpp

 namespace clang {
 namespace rename {
 
 namespace {
 // \brief NamedDeclFindingConsumer should delegate finding USRs of given Decl to
 // AdditionalUSRFinder. AdditionalUSRFinder adds USRs of ctor and dtor if given
 // Decl refers to class and adds USRs of all overridden methods if Decl refers
 // to virtual method.
-//
-// FIXME: It's better to match ctors/dtors via typeLoc's instead of adding
-// their USRs to the storage, because we can also match CXXConversionDecl's by
-// typeLoc and we won't have to "manually" handle them here.
 class AdditionalUSRFinder : public MatchFinder::MatchCallback {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
                              std::vector<std::string> *USRs)
     : FoundDecl(FoundDecl), Context(Context), USRs(USRs), USRSet(), Finder() {}
 
   void Find() {
     USRSet.insert(getUSRForDecl(FoundDecl));
virtual void run(const MatchFinder::MatchResult &Result) {
     if (Found) {
       USRSet.insert(getUSRForDecl(VirtualMethod));
     }
   }
 
   void addUSRsFromOverrideSetsAndCtorDtors() {
     // If D is CXXRecordDecl we should add all USRs of its constructors.
     if (const auto *RecordDecl = dyn_cast<CXXRecordDecl>(FoundDecl)) {
-      // Ignore destructors. Find the declaration of and explicit calls to a
-      // destructor through TagTypeLoc (and it is better for the purpose of
-      // renaming).
-      //
-      // For example, in the following code segment,
-      //  1  class C {
-      //  2    ~C();
-      //  3  };
-      //
-      // At line 3, there is a NamedDecl starting from '~' and a TagTypeLoc
-      // starting from 'C'.
       RecordDecl = RecordDecl->getDefinition();
-
-      // Iterate over all the constructors and add their USRs.
       for (const auto *CtorDecl : RecordDecl->ctors()) {
         USRSet.insert(getUSRForDecl(CtorDecl));
       }
+      USRSet.insert(getUSRForDecl(RecordDecl->getDestructor()));
     }
     // If D is CXXMethodDecl we should add all USRs of its overriden methods.
     if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FoundDecl)) {
       for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
         USRSet.insert(getUSRForDecl(OverriddenMethod));
       }
     }
   }

clang-rename/USRLocFinder.cpp

 namespace rename {
 
 namespace {
 // \brief This visitor recursively searches for all instances of a USR in a
 // translation unit and stores them for later usage.
 class USRLocFindingASTVisitor
     : public clang::RecursiveASTVisitor<USRLocFindingASTVisitor> {
 public:
-  explicit USRLocFindingASTVisitor(StringRef USR, StringRef PrevName)
-      : USR(USR), PrevName(PrevName) {}
+  explicit USRLocFindingASTVisitor(StringRef USR, StringRef PrevName,
+                                   const ASTContext &Context)
+      : USR(USR), PrevName(PrevName), Context(Context) {}
 
   // Declaration visitors:
 
-  bool VisitNamedDecl(const NamedDecl *Decl) {
-    if (getUSRForDecl(Decl) == USR) {
-      LocationsFound.push_back(Decl->getLocation());
-    }
-    return true;
-  }
-
-  bool VisitVarDecl(clang::VarDecl *Decl) {
-    clang::QualType Type = Decl->getType();
-    const clang::RecordDecl *RecordDecl = Type->getPointeeCXXRecordDecl();
-    if (RecordDecl) {
-      if (getUSRForDecl(RecordDecl) == USR) {
-        // The declaration refers to a type that is to be renamed.
-        LocationsFound.push_back(Decl->getTypeSpecStartLoc());
-      }
-    }
-    return true;
-  }
-
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
-    const ASTContext &Context = ConstructorDecl->getASTContext();
     for (auto &Initializer : ConstructorDecl->inits()) {
       if (Initializer->getSourceOrder() == -1) {
         // Ignore implicit initializers.
         continue;
       }
-
       if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
         if (getUSRForDecl(FieldDecl) == USR) {
           // The initializer refers to a field that is to be renamed.
           SourceLocation Location = Initializer->getSourceLocation();
           StringRef TokenName = Lexer::getSourceText(
               CharSourceRange::getTokenRange(Location),
               Context.getSourceManager(), Context.getLangOpts());
           if (TokenName == PrevName) {
             // The token of the source location we find actually has the old
             // name.
             LocationsFound.push_back(Initializer->getSourceLocation());
           }
         }
       }
     }
-
-    if (getUSRForDecl(ConstructorDecl) == USR) {
-      // This takes care of the class name part of a non-inline ctor definition.
-      LocationsFound.push_back(ConstructorDecl->getLocStart());
-    }
     return true;
   }
 
-  bool VisitCXXDestructorDecl(clang::CXXDestructorDecl *DestructorDecl) {
-    if (getUSRForDecl(DestructorDecl->getParent()) == USR) {
-      // Handles "~Foo" from "Foo::~Foo".
-      SourceLocation Location = DestructorDecl->getLocation();
-      const ASTContext &Context = DestructorDecl->getASTContext();
-      StringRef LLVM_ATTRIBUTE_UNUSED TokenName = Lexer::getSourceText(
-          CharSourceRange::getTokenRange(Location), Context.getSourceManager(),
-          Context.getLangOpts());
-      // 1 is the length of the "~" string that is not to be touched by the
-      // rename.
-      assert(TokenName.startswith("~"));
-      LocationsFound.push_back(Location.getLocWithOffset(1));
-
-      if (DestructorDecl->isThisDeclarationADefinition()) {
-        // Handles "Foo" from "Foo::~Foo".
-        LocationsFound.push_back(DestructorDecl->getLocStart());
-      }
+  bool VisitNamedDecl(const NamedDecl *Decl) {
+    if (getUSRForDecl(Decl) == USR) {
+      checkAndAddLocation(Decl->getLocation());
     }
-
     return true;
   }
 
   // Expression visitors:
 
   bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
     const auto *Decl = Expr->getFoundDecl();
 
-    checkNestedNameSpecifierLoc(Expr->getQualifierLoc());
     if (getUSRForDecl(Decl) == USR) {
       const SourceManager &Manager = Decl->getASTContext().getSourceManager();
       SourceLocation Location = Manager.getSpellingLoc(Expr->getLocation());
-      LocationsFound.push_back(Location);
+      checkAndAddLocation(Location);
     }
 
     return true;
   }
 
   bool VisitMemberExpr(const MemberExpr *Expr) {
     const auto *Decl = Expr->getFoundDecl().getDecl();
     if (getUSRForDecl(Decl) == USR) {
       const SourceManager &Manager = Decl->getASTContext().getSourceManager();
       SourceLocation Location = Manager.getSpellingLoc(Expr->getMemberLoc());
-      LocationsFound.push_back(Location);
-    }
-    return true;
-  }
-
-  bool VisitCXXConstructExpr(const CXXConstructExpr *Expr) {
-    CXXConstructorDecl *Decl = Expr->getConstructor();
-
-    if (getUSRForDecl(Decl) == USR) {
-      // This takes care of 'new <name>' expressions.
-      LocationsFound.push_back(Expr->getLocation());
+      checkAndAddLocation(Location);
     }
-
     return true;
   }
 
   bool VisitCXXStaticCastExpr(clang::CXXStaticCastExpr *Expr) {
     return handleCXXNamedCastExpr(Expr);
   }
 
   bool VisitCXXDynamicCastExpr(clang::CXXDynamicCastExpr *Expr) {
     return handleCXXNamedCastExpr(Expr);
   }
 
   bool VisitCXXReinterpretCastExpr(clang::CXXReinterpretCastExpr *Expr) {
     return handleCXXNamedCastExpr(Expr);
   }
 
   bool VisitCXXConstCastExpr(clang::CXXConstCastExpr *Expr) {
     return handleCXXNamedCastExpr(Expr);
   }
 
+  // Other visitors:
+
+  bool VisitTypeLoc(const TypeLoc Loc) {
+    if (getUSRForDecl(Loc.getType()->getAsCXXRecordDecl()) == USR) {
+      checkAndAddLocation(Loc.getBeginLoc());
+    }
+    return true;
+  }
+
   // Non-visitors:
 
   // \brief Returns a list of unique locations. Duplicate or overlapping
   // locations are erroneous and should be reported!
   const std::vector<clang::SourceLocation> &getLocationsFound() const {
     return LocationsFound;
   }
 
-private:
   // Namespace traversal:
-  void checkNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
+  void handleNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
     while (NameLoc) {
       const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
-      if (Decl && getUSRForDecl(Decl) == USR)
-        LocationsFound.push_back(NameLoc.getLocalBeginLoc());
+      if (Decl && getUSRForDecl(Decl) == USR) {
+        checkAndAddLocation(NameLoc.getLocalBeginLoc());
+      }
       NameLoc = NameLoc.getPrefix();
     }
   }
 
   bool handleCXXNamedCastExpr(clang::CXXNamedCastExpr *Expr) {
     clang::QualType Type = Expr->getType();
     // See if this a cast of a pointer.
     const RecordDecl *Decl = Type->getPointeeCXXRecordDecl();
     if (!Decl) {
       // See if this is a cast of a reference.
       Decl = Type->getAsCXXRecordDecl();
     }
 
     if (Decl && getUSRForDecl(Decl) == USR) {
       SourceLocation Location =
           Expr->getTypeInfoAsWritten()->getTypeLoc().getBeginLoc();
-      LocationsFound.push_back(Location);
+      checkAndAddLocation(Location);
     }
 
     return true;
   }
 
+private:
+  void checkAndAddLocation(SourceLocation Loc) {
+    const auto BeginLoc = Loc, EndLoc = Lexer::getLocForEndOfToken(
+                                   BeginLoc, 0, Context.getSourceManager(),
+                                   Context.getLangOpts());
+    StringRef TokenName =
+        Lexer::getSourceText(CharSourceRange::getTokenRange(BeginLoc, EndLoc),
+                             Context.getSourceManager(), Context.getLangOpts());
+    size_t Offset = TokenName.find(PrevName);
+    if (Offset != StringRef::npos) {
+      // The token of the source location we find actually has the old
+      // name.
+      LocationsFound.push_back(BeginLoc.getLocWithOffset(Offset));
+    }
+  }
+
   // All the locations of the USR were found.
   const std::string USR;
   // Old name that is renamed.
   const std::string PrevName;
   std::vector<clang::SourceLocation> LocationsFound;
+  const ASTContext &Context;
 };
 } // namespace
 
 std::vector<SourceLocation> getLocationsOfUSR(StringRef USR, StringRef PrevName,
                                               Decl *Decl) {
-  USRLocFindingASTVisitor Visitor(USR, PrevName);
-
+  USRLocFindingASTVisitor Visitor(USR, PrevName, Decl->getASTContext());
   Visitor.TraverseDecl(Decl);
+  NestedNameSpecifierLocFinder Finder(Decl->getASTContext());
+  for (const auto &Location : Finder.getNestedNameSpecifierLocations()) {
+    Visitor.handleNestedNameSpecifierLoc(Location);
+  }
   return Visitor.getLocationsFound();
 }
 
 } // namespace rename
 } // namespace clang

test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+class Foo {};   // CHECK: class Bar {};
+
+template <typename T>
+void func() {}
+
+template <typename T>
+class Baz {};
+
+int main() {
+  func<Foo>();  // CHECK: func<Bar>();
+  Baz<Foo> obj; // CHECK: Baz<Bar> obj;
+  return 0;
+}
+
+// Use grep -FUbo 'Foo' <file> to get the correct offset of Foo when changing
+// this file.

test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=304 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+class Foo {};   // CHECK: class Bar {};
+
+template <typename T>
+void func() {}
+
+template <typename T>
+class Baz {};
+
+int main() {
+  func<Foo>();  // CHECK: func<Bar>();
+  Baz<Foo> obj; // CHECK: Baz<Bar> obj;
+  return 0;
+}
+
+// Use grep -FUbo 'Foo' <file> to get the correct offset of Cla when changing
+// this file.

test/clang-rename/ClassNameInFunctionDefenition.cpp

-// Currently unsupported test.
 // RUN: cat %s > %t.cpp
-// FIXME: clang-rename doesn't recognize symbol in class function definition.
+// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
-class Foo {
+class Foo {             // CHECK: class Bar {
 public:
   void foo(int x);
 };
 
-void Foo::foo(int x) {}
-//   ^ this one
-
-int main() {
-  Foo obj;
-  obj.foo(0);
-  return 0;
-}
+void Foo::foo(int x) {} // CHECK: void Bar::foo(int x) {}

test/clang-rename/ComplicatedClassType.cpp

-// Unsupported test.
 // RUN: cat %s > %t.cpp
-// FIXME: This test contains very simple constructions likely to be seen in any
-// project and therefore passing this test is a slight sign of success.
-// Currently, the test fails badly.
+// RUN: clang-rename -offset=220 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+// Forward declaration.
+class Foo;                            // CHECK: class Bar;
 
 class Foo {                           // CHECK: class Bar {
- public:
-  Foo(int value = 0) : x(value) {}    // Bar(int value=0) : x(value) {}
+  Foo(int value = 0) : x(value) {}    // CHECK: Bar(int value = 0) : x(value) {}
 
-  Foo& operator++(int) {              // Bar& operator++(int) {
+  Foo &operator++(int) {              // CHECK: Bar &operator++(int) {
     x++;
     return *this;
   }
 
-  bool operator<(Foo const& rhs) {    // bool operator<(Bar const &rhs) {
+  bool operator<(Foo const &rhs) {    // CHECK: bool operator<(Bar const &rhs) {
     return this->x < rhs.x;
   }
 
- private:
   int x;
 };
 
 int main() {
-  Foo* Pointer = 0;                   // CHECK: Bar *Pointer = 0;
+  Foo *Pointer = 0;                   // CHECK: Bar *Pointer = 0;
   Foo Variable = Foo(10);             // CHECK: Bar Variable = Bar(10);
-  for (Foo it; it < Variable; it++) { // for (Bar it; it < Variable; it++) {}
+  for (Foo it; it < Variable; it++) { // CHECK: for (Bar it; it < Variable; it++) {
   }
   return 0;
 }

test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=287 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+// Currently unsupported test.
+// FIXME: clang-rename should be able to rename classes with templates
+// correctly.
+// XFAIL: *
+
+template <typename T>
+class Foo {               // CHECK: class Bar;
+public:
+  T foo(T arg, T& ref, T* ptr) {
+    T value;
+    int number = 42;
+    value = (T)number;
+    value = static_cast<T>(number);
+    return value;
+  }
+  static void foo(T value) {}
+  T member;
+};
+
+template <typename T>
+void func() {
+  Foo<T> obj;             // CHECK: Bar<T> obj;
+  obj.member = T();
+  Foo<T>::foo();          // CHECK: Bar<T>::foo();
+}
+
+int main() {
+  Foo<int> i;             // CHECK: Bar<int> i;
+  i.member = 0;
+  Foo<int>::foo(0);       // CHECK: Bar<int>::foo(0);
+
+  Foo<bool> b;            // CHECK: Bar<bool> b;
+  b.member = false;
+  Foo<bool>::foo(false);  // CHECK: Bar<bool>::foo(false);
+
+  return 0;
+}
+
+// Use grep -FUbo 'C' <file> to get the correct offset of foo when changing
+// this file.

test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=703 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+// Currently unsupported test.
+// FIXME: clang-rename should be able to rename classes with templates
+// correctly.
+// XFAIL: *
+
+template <typename T>
+class Foo {               // CHECK: class Bar;
+public:
+  T foo(T arg, T& ref, T* ptr) {
+    T value;
+    int number = 42;
+    value = (T)number;
+    value = static_cast<T>(number);
+    return value;
+  }
+  static void foo(T value) {}
+  T member;
+};
+
+template <typename T>
+void func() {
+  Foo<T> obj;             // CHECK: Bar<T> obj;
+  obj.member = T();
+  Foo<T>::foo();          // CHECK: Bar<T>::foo();
+}
+
+int main() {
+  Foo<int> i;             // CHECK: Bar<int> i;
+  i.member = 0;
+  Foo<int>::foo(0);       // CHECK: Bar<int>::foo(0);
+
+  Foo<bool> b;            // CHECK: Bar<bool> b;
+  b.member = false;
+  Foo<bool>::foo(false);  // CHECK: Bar<bool>::foo(false);
+
+  return 0;
+}
+
+// Use grep -FUbo 'Foo' <file> to get the correct offset of foo when changing
+// this file.

test/clang-rename/TemplateFunctionFindByDeclaration.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=241 -new-name=bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+// FIXME: clang-rename should be able to rename functions with templates.
+// XFAIL: *
+
+template <typename T>
+T foo(T value) {    // CHECK: T boo(T value) {
+  return value;
+}
+
+int main() {
+  foo<bool>(false); // CHECK: bar<bool>(false);
+  foo<int>(0);      // CHECK: bar<int>(0);
+  return 0;
+}
+
+// Use grep -FUbo 'foo' <file> to get the correct offset of foo when changing
+// this file.

test/clang-rename/TemplateFunctionFindByUse.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=290 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+// FIXME: clang-rename should be able to rename functions with templates.
+// XFAIL: *
+
+template <typename T>
+T foo(T value) {
+  return value;
+}
+
+int main() {
+  foo<bool>(false);
+  foo<int>(0);
+  return 0;
+}

test/clang-rename/TemplateTypename.cpp

-// Currently unsupported test.
-// RUN: cat %s > %t.cpp
-// FIXME: clang-rename should be able to rename template parameters correctly.
-
-template <typename T>
-T foo(T arg, T& ref, T* ptr) {
-  T value;
-  int number = 42;
-  value = (T)number;
-  value = static_cast<T>(number);
-  return value;
-}

test/clang-rename/TemplateTypenameFindByTemplateParam.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=270 -new-name=U %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+// Currently unsupported test.
+// FIXME: clang-rename should be able to rename template parameters correctly.
+// XFAIL: *
+
+template <typename T>             // CHECK: template <typename U>
+class Foo {
+T foo(T arg, T& ref, T* ptr) {    // CHECK: U foo(U arg, U& ref, U* ptr) {
+  T value;                        // CHECK: U value;
+  int number = 42;
+  value = (T)number;              // CHECK: value = (U)number;
+  value = static_cast<T>(number); // CHECK: value = static_cast<U>(number);
+  return value;
+}
+
+static void foo(T value) {}       // CHECK: static void foo(U value) {}
+
+T member;                         // CHECK: U member;
+};

test/clang-rename/TemplateTypenameFindByTypeInside.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=350 -new-name=U %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+// Currently unsupported test.
+// FIXME: clang-rename should be able to rename template parameters correctly.
+// XFAIL: *
+
+template <typename T>             // CHECK: template <typename U>
+class Foo {
+T foo(T arg, T& ref, T* ptr) {    // CHECK: U foo(U arg, U& ref, U* ptr) {
+  T value;                        // CHECK: U value;
+  int number = 42;
+  value = (T)number;              // CHECK: value = (U)number;
+  value = static_cast<T>(number); // CHECK: value = static_cast<U>(number);
+  return value;
+}
+
+static void foo(T value) {}       // CHECK: static void foo(U value) {}
+
+T member;                         // CHECK: U member;
+};

test/clang-rename/UserDefinedConversion.cpp

-// Currently unsupported test.
-// RUN: cat %s > %t.cpp
-// FIXME: clang-rename should handle conversions from a class type to another
-// type.
-
-class Foo {};             // CHECK: class Bar {};
-
-class Baz {               // CHECK: class Bar {
-  operator Foo() const {  // CHECK: operator Bar() const {
-    Foo foo;              // CHECK: Bar foo;
-    return foo;
-  }
-};

test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp

-// Currently unsupported test.
 // RUN: cat %s > %t.cpp
-// FIXME: while renaming class/struct clang-rename should be able to change
-// this type name corresponding user-defined conversions, too.
+// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 class Foo {                         // CHECK: class Bar {
 //    ^ offset must be here
 public:
   Foo() {}                          // CHECK: Bar() {}
 };
 
 class Baz {
 public:
   operator Foo() const {            // CHECK: operator Bar() const {
     Foo foo;                        // CHECK: Bar foo;
     return foo;
   }
 };
 
 int main() {
   Baz boo;
   Foo foo = static_cast<Foo>(boo);  // CHECK: Bar foo = static_cast<Bar>(boo);
   return 0;
 }
+
+// Use grep -FUbo 'Foo' <file> to get the correct offset of Cla when changing
+// this file.