[clangd] Renaming: Treat member functions like other functions

[ckandeler]

Summary

... by skipping the conflict check. The same considerations apply.

[hokein]

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

519

The change looks good. It would be nice to have a testcase, can you add one in the RenameTest, Renameable unit test?

[ckandeler]

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

519

For a function, the rename result ends up in LocalChanges (because there is no index), whereas the test assumes they are in GlobalChanges. Should I adapt the test code accordingly or is it not the right one for this case?

[hokein]

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

519

I see, you're right, sorry for missing this bit.

I think it makes sense to change the RenameTest.Renamable to use LocalChanges (comparing ranges should be enough), as all testcases there are a single file.

[ckandeler]

Added test cases.

[ckandeler]

Formatting glitch.

[hokein]

nice, thank you!

[ckandeler]

Fixed clang-format complaints.

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

std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
   std::optional<InvalidName> Result;
   if (isKeyword(NewName, ASTCtx.getLangOpts()))
     Result = InvalidName{InvalidName::Keywords, NewName.str()};
   else if (!mayBeValidIdentifier(NewName))
     Result = InvalidName{InvalidName::BadIdentifier, NewName.str()};
   else {
     // Name conflict detection.
     // Function conflicts are subtle (overloading), so ignore them.
-    if (RenameDecl.getKind() != Decl::Function) {
+    if (RenameDecl.getKind() != Decl::Function &&
+        RenameDecl.getKind() != Decl::CXXMethod) {
       if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
         Result = InvalidName{
             InvalidName::Conflict,
             Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
     }
   }
   if (Result)
     InvalidNameMetric.record(1, toString(Result->K));

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

Case Cases[] = {
 
       {R"cpp(
         void func(int V^ar, int Conflict) {
         }
       )cpp",
        "conflict", !HeaderFile, "Conflict"},
 
       {R"cpp(
+        void func(int);
+        void [[o^therFunc]](double);
+      )cpp",
+       nullptr, !HeaderFile, "func"},
+      {R"cpp(
+        struct S {
+          void func(int);
+          void [[o^therFunc]](double);
+        };
+      )cpp",
+       nullptr, !HeaderFile, "func"},
+
+      {R"cpp(
         int V^ar;
       )cpp",
        "\"const\" is a keyword", !HeaderFile, "const"},
 
       {R"cpp(// Trying to rename into the same name, SameName == SameName.
         void func() {
           int S^ameName;
         }
if (!WantRename) {
       assert(Case.ErrorMessage && "Error message must be set!");
       EXPECT_FALSE(Results)
           << "expected rename returned an error: " << T.code();
       auto ActualMessage = llvm::toString(Results.takeError());
       EXPECT_THAT(ActualMessage, testing::HasSubstr(Case.ErrorMessage));
     } else {
       EXPECT_TRUE(bool(Results)) << "rename returned an error: "
                                  << llvm::toString(Results.takeError());
-      ASSERT_EQ(1u, Results->GlobalChanges.size());
-      EXPECT_EQ(applyEdits(std::move(Results->GlobalChanges)).front().second,
-                expectedResult(T, NewName));
+      EXPECT_EQ(Results->LocalChanges, T.ranges());
     }
   }
 }
 
 MATCHER_P(newText, T, "") { return arg.newText == T; }
 
 TEST(RenameTest, IndexMergeMainFile) {
   Annotations Code("int ^x();");