[clangd] Implement rename by using SelectionTree and findExplicitReferences.

[hokein]

Summary

With the new implemenation, we will have better coverage of various AST
nodes, and fix some known/potential bugs.

Also added the existing clang-renamae tests. Known changed behavior:

1. "~Fo^o()" will not trigger the rename, will fix afterwards
2. references in macro bodies would not rename now

This fixes:

• https://github.com/clangd/clangd/issues/167
• https://github.com/clangd/clangd/issues/169
• https://github.com/clangd/clangd/issues/171

[ilya-biryukov]

It's a bit unclear how we manage to still rename overrides. Is this handled by the USR-generating functions?
Could we factor out the part that collects NamedDecls and use those instead of the USRs instead?

clang-tools-extra/clangd/ClangdServer.cpp

326 ↗

(On Diff #228188)

I would argue rename should not work in that case.
Could we check that the cursor is actually on the name token of the NamedDecl and not rename if it isn't?

clang-tools-extra/clangd/refactor/Rename.cpp

191

Why USRs? Could we just check whether the ND.getCanonicalDecl() is there instead?

clang-tools-extra/clangd/unittests/RenameTests.cpp

97

Could we stop at B?
I don't think C->D cover any more code paths, they merely add some noise to the test, making it harder to read.

109

NIT: alternatively, just do A().foo() to make the test smaller

131

Could you also check that destructors are renamed properly?

131

NIT: maybe remove the bodies of the functions that don't have references to Foo?
They seem to merely add noise, don't think they improve coverage.

185

The FIXME is a bit unclear. Could you explain in more detail?

214

I would argue we should fail in that case and not rename.
If we change the macro body, there's a chance other usages that we didn't want to update will also be updated.

However, if the current rename does that, also happy to keep as is. Up to you.

369

NIT: also mention in the documentation comment above that rename is run on all points.

[hokein]

simplify the testcases.

[hokein]

In D69934#1736729, @ilya-biryukov wrote:

It's a bit unclear how we manage to still rename overrides. Is this handled by the USR-generating functions?
Could we factor out the part that collects NamedDecls and use those instead of the USRs instead?

Yes, it is handled by the tooling USR-generating function. I would not touch that part, I plan to get rid of that API in a followup.

clang-tools-extra/clangd/ClangdServer.cpp

326 ↗

(On Diff #228188)

you are probably right, we only allow rename which is triggered on the name token. Will update the patch.

clang-tools-extra/clangd/refactor/Rename.cpp

191

checking ND.getCanonicalDecl() is not enough, thinking about virtual methods.

tooling::getUSRsForDeclaration does all the black-magic things here, it returns all rename-related decls.

clang-tools-extra/clangd/unittests/RenameTests.cpp

97

Stopped C.

131

Done. Add a new test for constructor/destructor case.

185

sorry, I forgot this.

This is a tricky case, if the cursor is on cxx initializer list, SelectionTree will return the CXXConsturctorExpr node of Baz, it actually rename the class Baz instead of renaming the Foo field.

214

I was also surprised with this testcase, but it aligns with what the current rename does.

[ilya-biryukov]

clang-tools-extra/clangd/ClangdServer.cpp

326 ↗

(On Diff #228188)

Definitely think we should do it before landing the patch.
Otherwise we'll introduce regressions.

clang-tools-extra/clangd/refactor/Rename.cpp

191

Could you add a comment that we need this to handle virtual methods?

204

Are we sure that USRs will always work here?
I would be protective here, surely there are unhandled cases.

260–261

This seems to assume all occurrences are outside macros.
Won't it break in presence of macros?
Do we have tests when the renamed token is:

• inside macro body
• inside macro arguments

?

[ilya-biryukov]

NIT: a typo in the description:
s/Aslo/Also

[hokein]

• make rename only work when the position is on the name range of the decl
• add more testsj

[hokein]

clang-tools-extra/clangd/ClangdServer.cpp

326 ↗

(On Diff #228188)

Done.

clang-tools-extra/clangd/refactor/Rename.cpp

260–261

if Loc is a macro location, tooling::Replacement will use the spelling location, so if the spelling location is in the main file, the code is correct, we have test cases for it.

but we will fail on cases like below, we should fix this, note that this issue exists even before this patch. Updated the comment here.

// foo.h
#define MACRO foo

// foo.cc
void f() {
  int fo^o = 2;
  MACRO;
}

[merge_guards_bot]

Build result: pass - 59990 tests passed, 0 failed and 763 were skipped.
Log files: console-log.txt, CMakeCache.txt

[ilya-biryukov]

clang-tools-extra/clangd/refactor/Rename.cpp

260–261

Renaming the spelling location seems like a bad idea in this case, I would argue we shouldn't rename in macro bodies (see the relevant comment on the test-case, let's move discussion there maybe?)

It would also be good to handle rename in macros outside tooling::Replacement, it's an important decision and I we should not blindly stick to the default behavior of tooling::Replacement without explicitly explaining why it's the best thing for rename.

clang-tools-extra/clangd/unittests/RenameTests.cpp

202

I would argue we should never rename in macro bodies.
That might cause problems in other uses of the macro as the token might refer to a completely different thing in some other place.
It's also a pretty rare case, so failing here instead of breaking code seems like the proper trade-off.

Are we keen on supporting this? If yes, why?

234

Could we also check partial and full specializations work?

template <>
class Foo<bool> {};

template <class T>
class Foo<T*> {};

Foo<int> x;
Foo<bool> y;
Foo<int*> z;

313

Why does this have to be a macro? This test does not seem to test macros in any way?

Maybe inline the macro?

[hokein]

address review comments:

• don't rename inside macro body
• fixed a corner case where the rename occurrence may come from a non-preamble #include
• add more tests, and simplify the testcases.

[hokein]

clang-tools-extra/clangd/refactor/Rename.cpp

260–261

Yes, I agree. There was a FIXME about this, I was planning to fix this in a follow-up, but now it is fixed.

clang-tools-extra/clangd/unittests/RenameTests.cpp

202

+1 on your suggestion.. The new code should handle this case as well.

234

added.

313

this was from the original clang-rename test, didn't know the intention about it.

[merge_guards_bot]

Build result: pass - 60093 tests passed, 0 failed and 729 were skipped.
Log files: console-log.txt, CMakeCache.txt

[hokein]

rebase: renaming the filed on cxx initializer list e.g. "Foo() : Fie^ld()" is fixed in SelectionTree.

[merge_guards_bot]

Build result: fail - 60148 tests passed, 1 failed and 729 were skipped.

failed: LLVM.Bindings/Go/go.test

Log files: console-log.txt, CMakeCache.txt

[ilya-biryukov]

Thanks, LG. The only important is about testing the foo.inc case separately from the rest of rename tests.

clang-tools-extra/clangd/refactor/Rename.cpp

180

NIT: inline this into the single caller.
Should simplify the code.

253

NIT: could probably shorten the whole sentence to We traverse only main file decls, but locations could come from an #include file, e.g.

clang-tools-extra/clangd/unittests/RenameTests.cpp

45

NIT: Keep periods at the end

363

Maybe test this separately? It's only used in one test.
Having this added for all tests cases is somewhat confusing.

[hokein]

Address comments.

clang-tools-extra/clangd/unittests/RenameTests.cpp

363

moved to a separate test.

[ilya-biryukov]

LGTM

[thakis]

The test fails on Windows:

http://45.33.8.238/win/2544/step_7.txt

Failing Tests (2):
    Clangd Unit Tests :: ./ClangdTests.exe/RenameTest.Renameable
    Clangd Unit Tests :: ./ClangdTests.exe/RenameTest.WithinFileRename

I'm guessing this needs the -fno-delayed-template-parsing treatment (look for that in other files in clangd/unittests)

clang-tools-extra/clangd/refactor/Rename.cpp

 //===--- Rename.cpp - Symbol-rename refactorings -----------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "refactor/Rename.h"
 #include "AST.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
-#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
-#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
-#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 
 namespace clang {
 namespace clangd {
 namespace {
 
 llvm::Optional<std::string> filePath(const SymbolLocation &Loc,
                                      llvm::StringRef HintFilePath) {
   if (!Loc)
     return None;
   auto Path = URI::resolve(Loc.FileURI, HintFilePath);
   if (!Path) {
     elog("Could not resolve URI {0}: {1}", Loc.FileURI, Path.takeError());
     return None;
   }
 
   return *Path;
 }
 
+// Returns true if the given location is expanded from any macro body.
+bool isInMacroBody(const SourceManager &SM, SourceLocation Loc) {
+  while (Loc.isMacroID()) {
+    if (SM.isMacroBodyExpansion(Loc))
+      return true;
+    Loc = SM.getImmediateMacroCallerLoc(Loc);
+  }
+
+  return false;
+}
+
 // Query the index to find some other files where the Decl is referenced.
 llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
                                             const SymbolIndex &Index) {
   RefsRequest Req;
   // We limit the number of results, this is a correctness/performance
   // tradeoff. We expect the number of symbol references in the current file
   // is smaller than the limit.
   Req.Limit = 100;
   if (auto ID = getSymbolID(&D))
     Req.IDs.insert(*ID);
   llvm::Optional<std::string> OtherFile;
   Index.refs(Req, [&](const Ref &R) {
     if (OtherFile)
       return;
     if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
       if (*RefFilePath != MainFile)
         OtherFile = *RefFilePath;
     }
   });
   return OtherFile;
 }
 
+llvm::DenseSet<const Decl *> locateDeclAt(ParsedAST &AST,
+                                          SourceLocation TokenStartLoc) {
+  unsigned Offset =
+      AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
+
+  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+  const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
+  if (!SelectedNode)
+    return {};
+
+  // If the location points to a Decl, we check it is actually on the name
+  // range of the Decl. This would avoid allowing rename on unrelated tokens.
+  //   ^class Foo {} // SelectionTree returns CXXRecordDecl,
+  //                 // we don't attempt to trigger rename on this position.
+  // FIXME: make this work on destructors, e.g. "~F^oo()".
+  if (const auto *D = SelectedNode->ASTNode.get<Decl>()) {
+    if (D->getLocation() != TokenStartLoc)
+      return {};
+  }
+
+  llvm::DenseSet<const Decl *> Result;
+  for (const auto *D :
+       targetDecl(SelectedNode->ASTNode,
+                  DeclRelation::Alias | DeclRelation::TemplatePattern))
+    Result.insert(D);
+  return Result;
+}
+
 enum ReasonToReject {
   NoSymbolFound,
   NoIndexProvided,
   NonIndexable,
   UsedOutsideFile,
   UnsupportedSymbol,
+  AmbiguousSymbol,
 };
 
 // Check the symbol Decl is renameable (per the index) within the file.
 llvm::Optional<ReasonToReject> renamableWithinFile(const Decl &RenameDecl,
                                                    StringRef MainFile,
                                                    const SymbolIndex *Index) {
   if (llvm::isa<NamespaceDecl>(&RenameDecl))
     return ReasonToReject::UnsupportedSymbol;
auto Message = [](ReasonToReject Reason) {
     case NoIndexProvided:
       return "symbol may be used in other files (no index available)";
     case UsedOutsideFile:
       return "the symbol is used outside main file";
     case NonIndexable:
       return "symbol may be used in other files (not eligible for indexing)";
     case UnsupportedSymbol:
       return "symbol is not a supported kind (e.g. namespace, macro)";
+    case AmbiguousSymbol:
+      return "there are multiple symbols at the given location";
     }
     llvm_unreachable("unhandled reason kind");
   };
   return llvm::make_error<llvm::StringError>(
       llvm::formatv("Cannot rename symbol: {0}", Message(Reason)),
       llvm::inconvertibleErrorCode());
 }
 
 // Return all rename occurrences in the main file.
-tooling::SymbolOccurrences
-findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) {
-  const NamedDecl *CanonicalRenameDecl =
-      tooling::getCanonicalSymbolDeclaration(RenameDecl);
-  assert(CanonicalRenameDecl && "RenameDecl must be not null");
-  std::vector<std::string> RenameUSRs =
-      tooling::getUSRsForDeclaration(CanonicalRenameDecl, AST.getASTContext());
-  std::string OldName = CanonicalRenameDecl->getNameAsString();
-  tooling::SymbolOccurrences Result;
+std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST,
+                                                      const NamedDecl &ND) {
+  // In theory, locateDeclAt should return the primary template. However, if the
+  // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
+  // will be the CXXRecordDecl, for this case, we need to get the primary
+  // template maunally.
+  const auto &RenameDecl =
+      ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
+  // getUSRsForDeclaration will find other related symbols, e.g. virtual and its
+  // overriddens, primary template and all explicit specializations.
+  // FIXME: get rid of the remaining tooling APIs.
+  std::vector<std::string> RenameUSRs = tooling::getUSRsForDeclaration(
+      tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext());
+  llvm::DenseSet<SymbolID> TargetIDs;
+  for (auto &USR : RenameUSRs)
+    TargetIDs.insert(SymbolID(USR));
+
+  std::vector<SourceLocation> Results;
   for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
-    tooling::SymbolOccurrences RenameInDecl =
-        tooling::getOccurrencesOfUSRs(RenameUSRs, OldName, TopLevelDecl);
-    Result.insert(Result.end(), std::make_move_iterator(RenameInDecl.begin()),
-                  std::make_move_iterator(RenameInDecl.end()));
+    findExplicitReferences(TopLevelDecl, [&](ReferenceLoc Ref) {
+      if (Ref.Targets.empty())
+        return;
+      for (const auto *Target : Ref.Targets) {
+        auto ID = getSymbolID(Target);
+        if (!ID || TargetIDs.find(*ID) == TargetIDs.end())
+          return;
       }
-  return Result;
+      Results.push_back(Ref.NameLoc);
+    });
+  }
+
+  return Results;
 }
 
 } // namespace
 
 llvm::Expected<tooling::Replacements>
 renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
                  llvm::StringRef NewName, const SymbolIndex *Index) {
   const SourceManager &SM = AST.getSourceManager();
   SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
       getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
   // FIXME: renaming macros is not supported yet, the macro-handling code should
   // be moved to rename tooling library.
   if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
     return makeError(UnsupportedSymbol);
 
-  const auto *RenameDecl =
-      tooling::getNamedDeclAt(AST.getASTContext(), SourceLocationBeg);
-  if (!RenameDecl)
+  auto DeclsUnderCursor = locateDeclAt(AST, SourceLocationBeg);
+  if (DeclsUnderCursor.empty())
     return makeError(NoSymbolFound);
+  if (DeclsUnderCursor.size() > 1)
+    return makeError(AmbiguousSymbol);
+
+  const auto *RenameDecl = llvm::dyn_cast<NamedDecl>(*DeclsUnderCursor.begin());
+  if (!RenameDecl)
+    return makeError(UnsupportedSymbol);
 
   if (auto Reject =
           renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index))
     return makeError(*Reject);
 
-  // Rename sometimes returns duplicate edits (which is a bug). A side-effect of
-  // adding them to a single Replacements object is these are deduplicated.
   tooling::Replacements FilteredChanges;
-  for (const tooling::SymbolOccurrence &Rename :
-       findOccurrencesWithinFile(AST, RenameDecl)) {
-    // Currently, we only support normal rename (one range) for C/C++.
-    // FIXME: support multiple-range rename for objective-c methods.
-    if (Rename.getNameRanges().size() > 1)
+  for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) {
+    SourceLocation RenameLoc = Loc;
+    // We don't rename in any macro bodies, but we allow rename the symbol
+    // spelled in a top-level macro argument in the main file.
+    if (RenameLoc.isMacroID()) {
+      if (isInMacroBody(SM, RenameLoc))
+        continue;
+      RenameLoc = SM.getSpellingLoc(Loc);
+    }
+    // Filter out locations not from main file.
+    // We traverse only main file decls, but locations could come from an
+    // non-preamble #include file e.g.
+    //   void test() {
+    //     int f^oo;
+    //     #include "use_foo.inc"
+    //   }
+    if (!isInsideMainFile(RenameLoc, SM))
       continue;
-    // We shouldn't have conflicting replacements. If there are conflicts, it
-    // means that we have bugs either in clangd or in Rename library, therefore
-    // we refuse to perform the rename.
     if (auto Err = FilteredChanges.add(tooling::Replacement(
-            AST.getASTContext().getSourceManager(),
-            CharSourceRange::getCharRange(Rename.getNameRanges()[0]), NewName)))
+            SM, CharSourceRange::getTokenRange(RenameLoc), NewName)))
       return std::move(Err);
   }
   return FilteredChanges;
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/RenameTests.cpp

for (const auto &R : Test.llvm::Annotations::ranges()) {
     Result += Code.substr(NextChar, R.Begin - NextChar);
     Result += NewName;
     NextChar = R.End;
   }
   Result += Code.substr(NextChar);
   return Result;
 }
 
-TEST(RenameTest, SingleFile) {
-  // "^" points to the position of the rename, and "[[]]" ranges point to the
+TEST(RenameTest, WithinFileRename) {
+  // rename is runnning on all "^" points, and "[[]]" ranges point to the
   // identifier that is being renamed.
   llvm::StringRef Tests[] = {
-      // Rename function.
+      // Function.
       R"cpp(
-        void [[foo]]() {
+        void [[foo^]]() {
           [[fo^o]]();
         }
       )cpp",
 
-      // Rename type.
+      // Type.
       R"cpp(
-        struct [[foo]]{};
+        struct [[foo^]] {};
         [[foo]] test() {
            [[f^oo]] x;
            return x;
         }
       )cpp",
 
-      // Rename variable.
+      // Local variable.
       R"cpp(
         void bar() {
           if (auto [[^foo]] = 5) {
             [[foo]] = 3;
           }
         }
       )cpp",
+
+      // Rename class, including constructor/destructor.
+      R"cpp(
+        class [[F^oo]] {
+          [[F^oo]]();
+          ~[[Foo]]();
+          void foo(int x);
+        };
+        [[Foo]]::[[Fo^o]]() {}
+        void [[Foo]]::foo(int x) {}
+      )cpp",
+
+      // Class in template argument.
+      R"cpp(
+        class [[F^oo]] {};
+        template <typename T> void func();
+        template <typename T> class Baz {};
+        int main() {
+          func<[[F^oo]]>();
+          Baz<[[F^oo]]> obj;
+          return 0;
+        }
+      )cpp",
+
+      // Forward class declaration without definition.
+      R"cpp(
+        class [[F^oo]];
+        [[Foo]] *f();
+      )cpp",
+
+      // Class methods overrides.
+      R"cpp(
+        struct A {
+         virtual void [[f^oo]]() {}
+        };
+        struct B : A {
+          void [[f^oo]]() override {}
+        };
+        struct C : B {
+          void [[f^oo]]() override {}
+        };
+
+        void func() {
+          A().[[f^oo]]();
+          B().[[f^oo]]();
+          C().[[f^oo]]();
+        }
+      )cpp",
+
+      // Template class (partial) specializations.
+      R"cpp(
+        template <typename T>
+        class [[F^oo]] {};
+
+        template<>
+        class [[F^oo]]<bool> {};
+        template <typename T>
+        class [[F^oo]]<T*> {};
+
+        void test() {
+          [[Foo]]<int> x;
+          [[Foo]]<bool> y;
+          [[Foo]]<int*> z;
+        }
+      )cpp",
+
+      // Template class instantiations.
+      R"cpp(
+        template <typename T>
+        class [[F^oo]] {
+        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() {
+          [[F^oo]]<T> obj;
+          obj.member = T();
+          [[Foo]]<T>::foo();
+        }
+
+        void test() {
+          [[F^oo]]<int> i;
+          i.member = 0;
+          [[F^oo]]<int>::foo(0);
+
+          [[F^oo]]<bool> b;
+          b.member = false;
+          [[Foo]]<bool>::foo(false);
+        }
+      )cpp",
+
+      // Template class methods.
+      R"cpp(
+        template <typename T>
+        class A {
+        public:
+          void [[f^oo]]() {}
+        };
+
+        void func() {
+          A<int>().[[f^oo]]();
+          A<double>().[[f^oo]]();
+          A<float>().[[f^oo]]();
+        }
+      )cpp",
+
+      // Complicated class type.
+      R"cpp(
+         // Forward declaration.
+        class [[Fo^o]];
+        class Baz {
+          virtual int getValue() const = 0;
+        };
+
+        class [[F^oo]] : public Baz  {
+        public:
+          [[Foo]](int value = 0) : x(value) {}
+
+          [[Foo]] &operator++(int);
+
+          bool operator<([[Foo]] const &rhs);
+          int getValue() const;
+        private:
+          int x;
+        };
+
+        void func() {
+          [[Foo]] *Pointer = 0;
+          [[Foo]] Variable = [[Foo]](10);
+          for ([[Foo]] it; it < Variable; it++);
+          const [[Foo]] *C = new [[Foo]]();
+          const_cast<[[Foo]] *>(C)->getValue();
+          [[Foo]] foo;
+          const Baz &BazReference = foo;
+          const Baz *BazPointer = &foo;
+          dynamic_cast<const [[^Foo]] &>(BazReference).getValue();
+          dynamic_cast<const [[^Foo]] *>(BazPointer)->getValue();
+          reinterpret_cast<const [[^Foo]] *>(BazPointer)->getValue();
+          static_cast<const [[^Foo]] &>(BazReference).getValue();
+          static_cast<const [[^Foo]] *>(BazPointer)->getValue();
+        }
+      )cpp",
+
+      // CXXConstructor initializer list.
+      R"cpp(
+        class Baz {};
+        class Qux {
+          Baz [[F^oo]];
+        public:
+          Qux();
+        };
+        Qux::Qux() : [[F^oo]]() {}
+      )cpp",
+
+      // DeclRefExpr.
+      R"cpp(
+        class C {
+        public:
+          static int [[F^oo]];
+        };
+
+        int foo(int x);
+        #define MACRO(a) foo(a)
+
+        void func() {
+          C::[[F^oo]] = 1;
+          MACRO(C::[[Foo]]);
+          int y = C::[[F^oo]];
+        }
+      )cpp",
+
+      // Macros.
+      R"cpp(
+        // no rename inside macro body.
+        #define M1 foo
+        #define M2(x) x
+        int [[fo^o]]();
+        void boo(int);
+
+        void qoo() {
+          [[foo]]();
+          boo([[foo]]());
+          M1();
+          boo(M1());
+          M2([[foo]]());
+          M2(M1()); // foo is inside the nested macro body.
+        }
+      )cpp",
+
+      // MemberExpr in macros
+      R"cpp(
+        class Baz {
+        public:
+          int [[F^oo]];
+        };
+        int qux(int x);
+        #define MACRO(a) qux(a)
+
+        int main() {
+          Baz baz;
+          baz.[[Foo]] = 1;
+          MACRO(baz.[[Foo]]);
+          int y = baz.[[Foo]];
+        }
+      )cpp",
+
+      // Template parameters.
+      R"cpp(
+        template <typename [[^T]]>
+        class Foo {
+          [[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;
+        };
+      )cpp",
+
+      // Typedef.
+      R"cpp(
+        namespace std {
+        class basic_string {};
+        typedef basic_string [[s^tring]];
+        } // namespace std
+
+        std::[[s^tring]] foo();
+      )cpp",
+
+      // Variable.
+      R"cpp(
+        namespace A {
+        int [[F^oo]];
+        }
+        int Foo;
+        int Qux = Foo;
+        int Baz = A::[[^Foo]];
+        void fun() {
+          struct {
+            int Foo;
+          } b = {100};
+          int Foo = 100;
+          Baz = Foo;
+          {
+            extern int Foo;
+            Baz = Foo;
+            Foo = A::[[F^oo]] + Baz;
+            A::[[Fo^o]] = b.Foo;
+          }
+          Foo = b.Foo;
+        }
+      )cpp",
+
+      // Namespace alias.
+      R"cpp(
+        namespace a { namespace b { void foo(); } }
+        namespace [[^x]] = a::b;
+        void bar() {
+          [[x]]::foo();
+        }
+      )cpp",
+
+      // Scope enums.
+      R"cpp(
+        enum class [[K^ind]] { ABC };
+        void ff() {
+          [[K^ind]] s;
+          s = [[Kind]]::ABC;
+        }
+      )cpp",
+
+      // template class in template argument list.
+      R"cpp(
+        template<typename T>
+        class [[Fo^o]] {};
+        template <template<typename> class Z> struct Bar { };
+        template <> struct Bar<[[Foo]]> {};
+      )cpp",
   };
   for (const auto T : Tests) {
     Annotations Code(T);
     auto TU = TestTU::withCode(Code.code());
     auto AST = TU.build();
+    EXPECT_TRUE(AST.getDiagnostics().empty())
+        << AST.getDiagnostics().front() << Code.code();
+
     llvm::StringRef NewName = "abcde";
+    for (const auto &RenamePos : Code.points()) {
       auto RenameResult =
-        renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName);
-    ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+          renameWithinFile(AST, testPath(TU.Filename), RenamePos, NewName);
+      ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << T;
       auto ApplyResult = llvm::cantFail(
           tooling::applyAllReplacements(Code.code(), *RenameResult));
       EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
     }
   }
+}
 
 TEST(RenameTest, Renameable) {
   struct Case {
     const char *Code;
     const char* ErrorMessage; // null if no error
     bool IsHeaderFile;
     const SymbolIndex *Index;
   };
Case Cases[] = {
 
           R"cpp(
         struct X { X operator++(int); };
         void f(X x) {x+^+;})cpp",
           "not a supported kind", HeaderFile, Index},
 
       {R"cpp(// foo is declared outside the file.
         void fo^o() {}
-      )cpp", "used outside main file", !HeaderFile /*cc file*/, Index},
+      )cpp",
+       "used outside main file", !HeaderFile /*cc file*/, Index},
 
       {R"cpp(
          // We should detect the symbol is used outside the file from the AST.
          void fo^o() {})cpp",
        "used outside main file", !HeaderFile, nullptr /*no index*/},
+
+      {R"cpp(
+         void foo(int);
+         void foo(char);
+         template <typename T> void f(T t) {
+           fo^o(t);
+         })cpp",
+       "multiple symbols", !HeaderFile, nullptr /*no index*/},
+
+      {R"cpp(// disallow rename on unrelated token.
+         cl^ass Foo {};
+       )cpp",
+       "no symbol", !HeaderFile, nullptr},
+
+      {R"cpp(// disallow rename on unrelated token.
+         temp^late<typename T>
+         class Foo {};
+       )cpp",
+       "no symbol", !HeaderFile, nullptr},
   };
 
   for (const auto& Case : Cases) {
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
     TU.HeaderCode = CommonHeader;
     if (Case.IsHeaderFile) {
       // We open the .h file as the main file.
if (!WantRename) {
                                  << llvm::toString(Results.takeError());
       auto ApplyResult =
           llvm::cantFail(tooling::applyAllReplacements(T.code(), *Results));
       EXPECT_EQ(expectedResult(T, NewName), ApplyResult);
     }
   }
 }
 
+TEST(RenameTest, MainFileReferencesOnly) {
+  // filter out references not from main file.
+  llvm::StringRef Test =
+      R"cpp(
+        void test() {
+          int [[fo^o]] = 1;
+          // rename references not from main file are not included.
+          #include "foo.inc"
+        })cpp";
+
+  Annotations Code(Test);
+  auto TU = TestTU::withCode(Code.code());
+  TU.AdditionalFiles["foo.inc"] = R"cpp(
+      #define Macro(X) X
+      &Macro(foo);
+      &foo;
+    )cpp";
+  auto AST = TU.build();
+  EXPECT_TRUE(AST.getDiagnostics().empty())
+      << AST.getDiagnostics().front() << Code.code();
+  llvm::StringRef NewName = "abcde";
+
+  auto RenameResult =
+      renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName);
+  ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << Code.point();
+  auto ApplyResult =
+      llvm::cantFail(tooling::applyAllReplacements(Code.code(), *RenameResult));
+  EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang