2 files changed, 177 insertions, 7 deletions

diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
index 6bce81bae30..f6966f619ad 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,107 @@ llvm::Expected<std::string> qualifyAllDecls(const FunctionDecl *FD) {
   return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
 }
 
+/// Generates Replacements for changing template and function parameter names in
+/// \p Dest to be the same as in \p Source.
+llvm::Expected<tooling::Replacements>
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+  llvm::DenseMap<const Decl *, std::string> ParamToNewName;
+  llvm::DenseMap<const NamedDecl *, std::vector<SourceLocation>> RefLocs;
+  auto HandleParam = [&](const NamedDecl *DestParam,
+                         const NamedDecl *SourceParam) {
+    // No need to rename if parameters already have the same name.
+    if (DestParam->getName() == SourceParam->getName())
+      return;
+    std::string NewName;
+    // Unnamed parameters won't be visited in findExplicitReferences. So add
+    // them here.
+    if (DestParam->getName().empty()) {
+      RefLocs[DestParam].push_back(DestParam->getLocation());
+      // If decl is unnamed in destination we pad the new name to avoid gluing
+      // with previous token, e.g. foo(int^) shouldn't turn into foo(intx).
+      NewName = " ";
+    }
+    NewName.append(SourceParam->getName());
+    ParamToNewName[DestParam->getCanonicalDecl()] = std::move(NewName);
+  };
+
+  // Populate mapping for template parameters.
+  auto *DestTempl = Dest->getDescribedFunctionTemplate();
+  auto *SourceTempl = Source->getDescribedFunctionTemplate();
+  assert(bool(DestTempl) == bool(SourceTempl));
+  if (DestTempl) {
+    const auto *DestTPL = DestTempl->getTemplateParameters();
+    const auto *SourceTPL = SourceTempl->getTemplateParameters();
+    assert(DestTPL->size() == SourceTPL->size());
+
+    for (size_t I = 0, EP = DestTPL->size(); I != EP; ++I)
+      HandleParam(DestTPL->getParam(I), SourceTPL->getParam(I));
+  }
+
+  // Populate mapping for function params.
+  assert(Dest->param_size() == Source->param_size());
+  for (size_t I = 0, E = Dest->param_size(); I != E; ++I)
+    HandleParam(Dest->getParamDecl(I), Source->getParamDecl(I));
+
+  const SourceManager &SM = Dest->getASTContext().getSourceManager();
+  const LangOptions &LangOpts = Dest->getASTContext().getLangOpts();
+  // Collect other references in function signature, i.e parameter types and
+  // default arguments.
+  findExplicitReferences(
+      // Use function template in case of templated functions to visit template
+      // parameters.
+      DestTempl ? llvm::dyn_cast<Decl>(DestTempl) : llvm::dyn_cast<Decl>(Dest),
+      [&](ReferenceLoc Ref) {
+        if (Ref.Targets.size() != 1)
+          return;
+        const auto *Target =
+            llvm::cast<NamedDecl>(Ref.Targets.front()->getCanonicalDecl());
+        auto It = ParamToNewName.find(Target);
+        if (It == ParamToNewName.end())
+          return;
+        RefLocs[Target].push_back(Ref.NameLoc);
+      });
+
+  // Now try to generate edits for all the refs.
+  tooling::Replacements Replacements;
+  for (auto &Entry : RefLocs) {
+    const auto *OldDecl = Entry.first;
+    llvm::StringRef OldName = OldDecl->getName();
+    llvm::StringRef NewName = ParamToNewName[OldDecl];
+    for (SourceLocation RefLoc : Entry.second) {
+      CharSourceRange ReplaceRange;
+      // In case of unnamed parameters, we have an empty char range, whereas we
+      // have a tokenrange at RefLoc with named parameters.
+      if (OldName.empty())
+        ReplaceRange = CharSourceRange::getCharRange(RefLoc, RefLoc);
+      else
+        ReplaceRange = CharSourceRange::getTokenRange(RefLoc, RefLoc);
+      // If occurence is coming from a macro expansion, try to get back to the
+      // file range.
+      if (RefLoc.isMacroID()) {
+        ReplaceRange = Lexer::makeFileCharRange(ReplaceRange, SM, LangOpts);
+        // Bail out if we need to replace macro bodies.
+        if (ReplaceRange.isInvalid()) {
+          auto Err = llvm::createStringError(
+              llvm::inconvertibleErrorCode(),
+              "Cant rename parameter inside macro body.");
+          elog("define inline: {0}", Err);
+          return std::move(Err);
+        }
+      }
+
+      if (auto Err = Replacements.add(
+              tooling::Replacement(SM, ReplaceRange, NewName))) {
+        elog("define inline: Couldn't replace parameter name for {0} to {1}: "
+             "{2}",
+             OldName, NewName, Err);
+        return std::move(Err);
+      }
+    }
+  }
+  return Replacements;
+}
+
 // Returns the canonical declaration for the given FunctionDecl. This will
 // usually be the first declaration in current translation unit with the
 // exception of template specialization.
@@ -332,6 +433,10 @@ public:
           "Couldn't find semicolon for target declaration.");
     }
 
+    auto ParamReplacements = renameParameters(Target, Source);
+    if (!ParamReplacements)
+      return ParamReplacements.takeError();
+
     auto QualifiedBody = qualifyAllDecls(Source);
     if (!QualifiedBody)
       return QualifiedBody.takeError();
@@ -354,8 +459,9 @@ public:
 
     llvm::SmallVector<std::pair<std::string, Edit>, 2> Edits;
     // Edit for Target.
-    auto FE = Effect::fileEdit(SM, SM.getFileID(*Semicolon),
-                               tooling::Replacements(SemicolonToFuncBody));
+    auto FE = Effect::fileEdit(
+        SM, SM.getFileID(*Semicolon),
+        tooling::Replacements(SemicolonToFuncBody).merge(*ParamReplacements));
     if (!FE)
       return FE.takeError();
     Edits.push_back(std::move(*FE));
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index f14383b2408..126aab61ada 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -40,9 +40,9 @@
 #include <vector>
 
 using ::testing::AllOf;
+using ::testing::ElementsAre;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
-using ::testing::ElementsAre;
 
 namespace clang {
 namespace clangd {
@@ -1386,7 +1386,7 @@ TEST_F(DefineInlineTest, TransformFunctionTempls) {
   // Template body is not parsed until instantiation time on windows, which
   // results in arbitrary failures as function body becomes NULL.
   ExtraArgs.push_back("-fno-delayed-template-parsing");
-  for(const auto &Case : Cases)
+  for (const auto &Case : Cases)
     EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1429,7 +1429,7 @@ TEST_F(DefineInlineTest, TransformTypeLocs) {
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test =R"cpp(
+  auto Test = R"cpp(
     namespace a {
       template <typename T> class Bar {
       public:
@@ -1498,6 +1498,70 @@ TEST_F(DefineInlineTest, StaticMembers) {
   EXPECT_EQ(apply(Test), Expected);
 }
 
+TEST_F(DefineInlineTest, TransformParamNames) {
+  std::pair<llvm::StringRef, llvm::StringRef> Cases[] = {
+      {R"cpp(
+        void foo(int, bool b, int T\
+est);
+        void ^foo(int f, bool x, int z) {})cpp",
+       R"cpp(
+        void foo(int f, bool x, int z){}
+        )cpp"},
+      {R"cpp(
+        #define PARAM int Z
+        void foo(PARAM);
+
+        void ^foo(int X) {})cpp",
+       "fail: Cant rename parameter inside macro body."},
+      {R"cpp(
+        #define TYPE int
+        #define PARAM TYPE Z
+        #define BODY(x) 5 * (x) + 2
+        template <int P>
+        void foo(PARAM, TYPE Q, TYPE, TYPE W = BODY(P));
+        template <int x>
+        void ^foo(int Z, int b, int c, int d) {})cpp",
+       R"cpp(
+        #define TYPE int
+        #define PARAM TYPE Z
+        #define BODY(x) 5 * (x) + 2
+        template <int x>
+        void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){}
+        )cpp"},
+  };
+  for (const auto &Case : Cases)
+    EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
+}
+
+TEST_F(DefineInlineTest, TransformTemplParamNames) {
+  auto Test = R"cpp(
+    struct Foo {
+      struct Bar {
+        template <class, class X,
+                  template<typename> class, template<typename> class Y,
+                  int, int Z>
+        void foo(X, Y<X>, int W = 5 * Z + 2);
+      };
+    };
+
+    template <class T, class U,
+              template<typename> class V, template<typename> class W,
+              int X, int Y>
+    void Foo::Bar::f^oo(U, W<U>, int Q) {})cpp";
+  auto Expected = R"cpp(
+    struct Foo {
+      struct Bar {
+        template <class T, class U,
+                  template<typename> class V, template<typename> class W,
+                  int X, int Y>
+        void foo(U, W<U>, int Q = 5 * Y + 2){}
+      };
+    };
+
+    )cpp";
+  EXPECT_EQ(apply(Test), Expected);
+}
+
 TEST_F(DefineInlineTest, TransformInlineNamespaces) {
   auto Test = R"cpp(
     namespace a { inline namespace b { namespace { struct Foo{}; } } }
@@ -1536,7 +1600,7 @@ TEST_F(DefineInlineTest, TokensBeforeSemicolon) {
           void fo^o() { return ; })cpp",
        "fail: Couldn't find semicolon for target declaration."},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
     EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1594,7 +1658,7 @@ TEST_F(DefineInlineTest, HandleMacros) {
           void TARGET(){ return; }
           )cpp"},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
     EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }