[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

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

175

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.

368

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

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

175

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

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

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

175

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

185

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

225

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

Done.

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

225

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

225

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

225

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

158

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

234

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

365

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

365

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/ClangdServer.cpp

auto Action = [Pos, File = File.str(), CB = std::move(CB),
     // have to revisit this when we support cross-file rename.
     auto Changes = renameWithinFile(AST, File, Pos, "dummy", Index);
     if (!Changes) {
       // LSP says to return null on failure, but that will result in a generic
       // failure message. If we send an LSP error response, clients can surface
       // the message to users (VSCode does).
       return CB(Changes.takeError());
     }
+    // FIXME: this is not always correct with SelectionTree behavior, e.g.
+    //     t^emplate <typename T>
+    //     class Foo {};
+    // When we trigger rename on the above cursor, we will rename the template
+    // class Foo, but the token under the cursor is not corresponding to the
+    // "Foo" range, though the final result is correct.
     SourceLocation Loc = getBeginningOfIdentifier(
         Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
     if (auto Range = getTokenRange(AST.getSourceManager(),
                                    AST.getASTContext().getLangOpts(), Loc))
       return CB(*Range);
     // Return null if the "rename" is not valid at the position.
     CB(llvm::None);
   };

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/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)
Index.refs(Req, [&](const Ref &R) {
     if (auto RefFilePath = filePath(R.Location, /*HintFilePath=*/MainFile)) {
       if (*RefFilePath != MainFile)
         OtherFile = *RefFilePath;
     }
   });
   return OtherFile;
 }
 
+llvm::DenseSet<const Decl *> locateDeclAt(ParsedAST &AST, Position P) {
+  const auto &SM = AST.getSourceManager();
+  llvm::StringRef SourceCode = SM.getBufferData(SM.getMainFileID());
+  auto Offset = positionToOffset(SourceCode, P);
+  if (!Offset) {
+    elog("Fail to convert position to offset: {0}", Offset.takeError());
+    return {};
+  }
+  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), *Offset);
+  llvm::DenseSet<const Decl *> Result;
+  if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+    for (const auto *D : targetDecl(
+             N->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());
 }
 
+const NamedDecl &getPrimaryTemplateOrThis(const NamedDecl &ND) {
+  if (const auto *T = ND.getDescribedTemplate())
+    return *T;
+  return ND;
+}
+
 // 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 = getPrimaryTemplateOrThis(ND);
+  // 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);
+        assert(ID);
+        if (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, Pos);
+  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)
-      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.
+  for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) {
+    // We shouldn't have conflicting replacements or replacements from different
+    // files. If there are conflicts, it means that we have bugs either in
+    // clangd or in findExplicitReferences, 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(Loc), 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.
       R"cpp(
         void [[foo]]() {
           [[fo^o]]();
         }
       )cpp",
 
       // Rename type.
       R"cpp(
         struct [[foo]]{};
         [[foo]] test() {
            [[f^oo]] x;
            return x;
         }
       )cpp",
 
       // Rename variable.
       R"cpp(
         void bar() {
           if (auto [[^foo]] = 5) {
             [[foo]] = 3;
           }
         }
       )cpp",
+
+      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",
+
+      // class simple rename
+      R"cpp(
+        class [[F^oo]] {
+          void foo(int x);
+        };
+        void [[Foo]]::foo(int x) {}
+      )cpp",
+
+      // class constructor/destructor.
+      R"cpp(
+        class [[Foo^]] {
+          [[F^oo]]();
+          ~[[F^oo]]();
+        };
+      )cpp",
+
+      // class 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",
+
+      // 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",
+
+      // class constructors
+      R"cpp(
+        class [[^Foo]] {
+        public:
+          [[Foo]]();
+        };
+
+        [[Foo]]::[[Fo^o]]() {}
+      )cpp",
+
+      // constructor initializer list
+      R"cpp(
+        class Baz {};
+        class Qux {
+          Baz [[F^oo]];
+        public:
+          Qux();
+        };
+        // FIXME: selection tree on initializer list Foo below will returns
+        // CXXConstructExpr node, which ends up with renaming class "Baze"
+        // instead of "Foo".
+        Qux::Qux() : [[Foo]]() {}
+      )cpp",
+
+      // DeclRefExpr
+      R"cpp(
+        class C {
+        public:
+          static int [[F^oo]];
+        };
+
+        int foo(int x) { return 0; }
+        #define MACRO(a) foo(a)
+
+        void func() {
+          C::[[F^oo]] = 1;
+          MACRO(C::[[Foo]]);
+          int y = C::[[F^oo]];
+        }
+      )cpp",
+
+      // Forward declaration
+      R"cpp(
+        class [[F^oo]];
+        [[Foo]] *f();
+      )cpp",
+
+      // function marco
+      R"cpp(
+        #define moo [[foo]]
+        int [[fo^o]]() { return 42; }
+        void boo(int value) {}
+
+        void qoo() {
+          [[foo]]();
+          boo([[foo]]());
+          moo();
+          boo(moo());
+        }
+      )cpp",
+
+      // MemberExpr in macro
+      R"cpp(
+        class Baz {
+        public:
+          int [[F^oo]];
+        };
+        int qux(int x) { return 0; }
+        #define MACRO(a) qux(a)
+
+        int main() {
+          Baz baz;
+          baz.[[Foo]] = 1;
+          MACRO(baz.[[Foo]]);
+          int y = baz.[[Foo]];
+        }
+      )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();
+        }
+
+        int main() {
+          [[F^oo]]<int> i;
+          i.member = 0;
+          [[F^oo]]<int>::foo(0);
+
+          [[F^oo]]<bool> b;
+          b.member = false;
+          [[Foo]]<bool>::foo(false);
+
+          return 0;
+        }
+      )cpp",
+
+      // template arguments
+      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",
+
+      // template class methods
+      R"cpp(
+        template <typename T>
+        class A {
+        public:
+          void [[f^oo]]() {}
+        };
+
+        void func() {
+          A<int> a;
+          A<double> b;
+          A<float> c;
+          a.[[f^oo]]();
+          b.[[f^oo]]();
+          c.[[f^oo]]();
+        }
+      )cpp",
+
+      // typedef
+      R"cpp(
+        namespace std {
+        class basic_string {};
+        typedef basic_string [[s^tring]];
+        } // namespace std
+
+        std::[[s^tring]] foo();
+      )cpp",
+
+      // variable
+      R"cpp(
+        #define NAMESPACE namespace A
+        NAMESPACE {
+        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",
+
+      // references in template argument lists
+      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();
     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*/},
   };
 
   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.