[clang-tools-extra] Add support for renaming objc methods, even those with multiple selector pieces (PR #76466)

Alex Hoppen via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 2 09:57:13 PST 2024


================
@@ -508,24 +513,46 @@ static bool mayBeValidIdentifier(llvm::StringRef Ident) {
       !isAsciiIdentifierStart(Ident.front(), AllowDollar))
     return false;
   for (char C : Ident) {
+    if (AllowColon && C == ':')
+      continue;
     if (llvm::isASCII(C) && !isAsciiIdentifierContinue(C, AllowDollar))
       return false;
   }
   return true;
 }
 
+std::string getName(const NamedDecl &RenameDecl) {
+  if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl))
+    return MD->getSelector().getAsString();
+  if (const auto *ID = RenameDecl.getIdentifier())
+    return ID->getName().str();
+  return "";
+}
+
 // Check if we can rename the given RenameDecl into NewName.
 // Return details if the rename would produce a conflict.
-std::optional<InvalidName> checkName(const NamedDecl &RenameDecl,
-                                     llvm::StringRef NewName) {
+std::optional<llvm::Error> checkName(const NamedDecl &RenameDecl,
+                                     llvm::StringRef NewName,
+                                     llvm::StringRef OldName) {
   trace::Span Tracer("CheckName");
   static constexpr trace::Metric InvalidNameMetric(
       "rename_name_invalid", trace::Metric::Counter, "invalid_kind");
+
+  if (OldName == NewName)
+    return makeError(ReasonToReject::SameName);
+
+  if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
+    const auto Sel = MD->getSelector();
+    if (Sel.getNumArgs() != NewName.count(':') &&
+        NewName != "__clangd_rename_placeholder")
+      return makeError(InvalidName{InvalidName::BadIdentifier, NewName.str()});
----------------
ahoppen wrote:

This seems awfully ad-hoc to check for `__clangd_rename_placeholder`. I would prefer to change the prepare rename request to just change one of the selector pieces to `__clangd_rename_placeholder`.

I’ve been doing this here

https://github.com/llvm/llvm-project/pull/78872/files#diff-26ff7c74af8a1d882abbad43625d3cd4bf8d024ef5c7064993f5ede3eec8752eR817-R829

https://github.com/llvm/llvm-project/pull/76466


More information about the cfe-commits mailing list