[clangd] Use SymbolName to represent Objective-C selectors (#82061)

This is a cleaner design than using identifier and an optional `Selector`. It also allows rename of Objective-C method names if no declaration is at hand and thus no `Selector` instance can be formed. For example, when finding the ranges to rename based on an index that’s not clangd’s built-in index.

Author: ahoppen

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

@@ -18,9 +18,11 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/ParentMapContext.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Basic/CharInfo.h"
@@ -597,11 +599,12 @@ bool isMatchingSelectorName(const syntax::Token &Cur, const syntax::Token &Next,
 // The search will terminate upon seeing Terminator or a ; at the top level.
 std::optional<SymbolRange>
 findAllSelectorPieces(llvm::ArrayRef<syntax::Token> Tokens,
-                      const SourceManager &SM, Selector Sel,
+                      const SourceManager &SM, const RenameSymbolName &Name,
                       tok::TokenKind Terminator) {
   assert(!Tokens.empty());
 
-  unsigned NumArgs = Sel.getNumArgs();
+  ArrayRef<std::string> NamePieces = Name.getNamePieces();
+  unsigned NumArgs = NamePieces.size();
   llvm::SmallVector<tok::TokenKind, 8> Closes;
   std::vector<Range> SelectorPieces;
   for (unsigned Index = 0, Last = Tokens.size(); Index < Last - 1; ++Index) {
@@ -611,12 +614,12 @@ findAllSelectorPieces(llvm::ArrayRef<syntax::Token> Tokens,
       auto PieceCount = SelectorPieces.size();
       if (PieceCount < NumArgs &&
           isMatchingSelectorName(Tok, Tokens[Index + 1], SM,
-                                 Sel.getNameForSlot(PieceCount))) {
+                                 NamePieces[PieceCount])) {
         // If 'foo:' instead of ':' (empty selector), we need to skip the ':'
         // token after the name. We don't currently properly support empty
         // selectors since we may lex them improperly due to ternary statements
         // as well as don't properly support storing their ranges for edits.
-        if (!Sel.getNameForSlot(PieceCount).empty())
+        if (!NamePieces[PieceCount].empty())
           ++Index;
         SelectorPieces.push_back(
             halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
@@ -668,16 +671,17 @@ findAllSelectorPieces(llvm::ArrayRef<syntax::Token> Tokens,
 
 /// Collects all ranges of the given identifier/selector in the source code.
 ///
-/// If a selector is given, this does a full lex of the given source code in
-/// order to identify all selector fragments (e.g. in method exprs/decls) since
-/// they are non-contiguous.
-std::vector<SymbolRange> collectRenameIdentifierRanges(
-    llvm::StringRef Identifier, llvm::StringRef Content,
-    const LangOptions &LangOpts, std::optional<Selector> Selector) {
+/// If `Name` is an Objective-C symbol name, this does a full lex of the given
+/// source code in order to identify all selector fragments (e.g. in method
+/// exprs/decls) since they are non-contiguous.
+std::vector<SymbolRange>
+collectRenameIdentifierRanges(const RenameSymbolName &Name,
+                              llvm::StringRef Content,
+                              const LangOptions &LangOpts) {
   std::vector<SymbolRange> Ranges;
-  if (!Selector) {
+  if (auto SinglePiece = Name.getSinglePiece()) {
     auto IdentifierRanges =
-        collectIdentifierRanges(Identifier, Content, LangOpts);
+        collectIdentifierRanges(*SinglePiece, Content, LangOpts);
     for (const auto &R : IdentifierRanges)
       Ranges.emplace_back(R);
     return Ranges;
@@ -691,7 +695,7 @@ std::vector<SymbolRange> collectRenameIdentifierRanges(
   // parsing a method declaration or definition which isn't at the top level or
   // similar looking expressions (e.g. an @selector() expression).
   llvm::SmallVector<tok::TokenKind, 8> Closes;
-  llvm::StringRef FirstSelPiece = Selector->getNameForSlot(0);
+  llvm::StringRef FirstSelPiece = Name.getNamePieces()[0];
 
   auto Tokens = syntax::tokenize(SM.getMainFileID(), SM, LangOpts);
   unsigned Last = Tokens.size() - 1;
@@ -723,7 +727,7 @@ std::vector<SymbolRange> collectRenameIdentifierRanges(
       // Check if we can find all the relevant selector peices starting from
       // this token
       auto SelectorRanges =
-          findAllSelectorPieces(ArrayRef(Tokens).slice(Index), SM, *Selector,
+          findAllSelectorPieces(ArrayRef(Tokens).slice(Index), SM, Name,
                                 Closes.empty() ? tok::l_brace : Closes.back());
       if (SelectorRanges)
         Ranges.emplace_back(std::move(*SelectorRanges));
@@ -770,7 +774,6 @@ renameObjCMethodWithinFile(ParsedAST &AST, const ObjCMethodDecl *MD,
                            std::vector<SourceLocation> SelectorOccurences) {
   const SourceManager &SM = AST.getSourceManager();
   auto Code = SM.getBufferData(SM.getMainFileID());
-  auto RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
   llvm::SmallVector<llvm::StringRef, 8> NewNames;
   NewName.split(NewNames, ":");
 
@@ -780,7 +783,7 @@ renameObjCMethodWithinFile(ParsedAST &AST, const ObjCMethodDecl *MD,
     Ranges.push_back(tokenRangeForLoc(AST, Loc, SM, LangOpts));
   auto FilePath = AST.tuPath();
   auto RenameRanges = collectRenameIdentifierRanges(
-      RenameIdentifier, Code, LangOpts, MD->getSelector());
+      RenameSymbolName(MD->getDeclName()), Code, LangOpts);
   auto RenameEdit = buildRenameEdit(FilePath, Code, RenameRanges, NewNames);
   if (!RenameEdit)
     return error("failed to rename in file {0}: {1}", FilePath,
@@ -942,21 +945,14 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
            ExpBuffer.getError().message());
       continue;
     }
-    std::string RenameIdentifier = RenameDecl.getNameAsString();
-    std::optional<Selector> Selector = std::nullopt;
+    RenameSymbolName RenameName(RenameDecl.getDeclName());
     llvm::SmallVector<llvm::StringRef, 8> NewNames;
-    if (const auto *MD = dyn_cast<ObjCMethodDecl>(&RenameDecl)) {
-      RenameIdentifier = MD->getSelector().getNameForSlot(0).str();
-      if (MD->getSelector().getNumArgs() > 1)
-        Selector = MD->getSelector();
-    }
     NewName.split(NewNames, ":");
 
     auto AffectedFileCode = (*ExpBuffer)->getBuffer();
-    auto RenameRanges =
-        adjustRenameRanges(AffectedFileCode, RenameIdentifier,
-                           std::move(FileAndOccurrences.second),
-                           RenameDecl.getASTContext().getLangOpts(), Selector);
+    auto RenameRanges = adjustRenameRanges(
+        AffectedFileCode, RenameName, std::move(FileAndOccurrences.second),
+        RenameDecl.getASTContext().getLangOpts());
     if (!RenameRanges) {
       // Our heuristics fails to adjust rename ranges to the current state of
       // the file, it is most likely the index is stale, so we give up the
@@ -1017,6 +1013,50 @@ void findNearMiss(
 
 } // namespace
 
+RenameSymbolName::RenameSymbolName() : NamePieces({}) {}
+
+namespace {
+std::vector<std::string> extractNamePieces(const DeclarationName &DeclName) {
+  if (DeclName.getNameKind() ==
+          DeclarationName::NameKind::ObjCMultiArgSelector ||
+      DeclName.getNameKind() == DeclarationName::NameKind::ObjCOneArgSelector) {
+    const Selector &Sel = DeclName.getObjCSelector();
+    std::vector<std::string> Result;
+    for (unsigned Slot = 0; Slot < Sel.getNumArgs(); ++Slot) {
+      Result.push_back(Sel.getNameForSlot(Slot).str());
+    }
+    return Result;
+  }
+  return {DeclName.getAsString()};
+}
+} // namespace
+
+RenameSymbolName::RenameSymbolName(const DeclarationName &DeclName)
+    : RenameSymbolName(extractNamePieces(DeclName)) {}
+
+RenameSymbolName::RenameSymbolName(ArrayRef<std::string> NamePieces) {
+  for (const auto &Piece : NamePieces)
+    this->NamePieces.push_back(Piece);
+}
+
+std::optional<std::string> RenameSymbolName::getSinglePiece() const {
+  if (getNamePieces().size() == 1) {
+    return NamePieces.front();
+  }
+  return std::nullopt;
+}
+
+std::string RenameSymbolName::getAsString() const {
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  this->print(OS);
+  return Result;
+}
+
+void RenameSymbolName::print(raw_ostream &OS) const {
+  llvm::interleave(NamePieces, OS, ":");
+}
+
 SymbolRange::SymbolRange(Range R) : Ranges({R}) {}
 
 SymbolRange::SymbolRange(std::vector<Range> Ranges)
@@ -1244,14 +1284,13 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
 //          were inserted). If such a "near miss" is found, the rename is still
 //          possible
 std::optional<std::vector<SymbolRange>>
-adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
-                   std::vector<Range> Indexed, const LangOptions &LangOpts,
-                   std::optional<Selector> Selector) {
+adjustRenameRanges(llvm::StringRef DraftCode, const RenameSymbolName &Name,
+                   std::vector<Range> Indexed, const LangOptions &LangOpts) {
   trace::Span Tracer("AdjustRenameRanges");
   assert(!Indexed.empty());
   assert(llvm::is_sorted(Indexed));
   std::vector<SymbolRange> Lexed =
-      collectRenameIdentifierRanges(Identifier, DraftCode, LangOpts, Selector);
+      collectRenameIdentifierRanges(Name, DraftCode, LangOpts);
   llvm::sort(Lexed);
   return getMappedRanges(Indexed, Lexed);
 }

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

@@ -35,6 +35,49 @@ struct RenameOptions {
   bool RenameVirtual = true;
 };
 
+/// A name of a symbol that should be renamed.
+///
+/// Symbol's name can be composed of multiple strings. For example, Objective-C
+/// methods can contain multiple argument labels:
+///
+/// \code
+/// - (void) myMethodNamePiece: (int)x anotherNamePieces:(int)y;
+///          ^~ string 0 ~~~~~         ^~ string 1 ~~~~~
+/// \endcode
+class RenameSymbolName {
+  llvm::SmallVector<std::string, 1> NamePieces;
+
+public:
+  RenameSymbolName();
+
+  /// Create a new \c SymbolName with the specified pieces.
+  explicit RenameSymbolName(ArrayRef<std::string> NamePieces);
+
+  explicit RenameSymbolName(const DeclarationName &Name);
+
+  ArrayRef<std::string> getNamePieces() const { return NamePieces; }
+
+  /// If this symbol consists of a single piece return it, otherwise return
+  /// `None`.
+  ///
+  /// Only symbols in Objective-C can consist of multiple pieces, so this
+  /// function always returns a value for non-Objective-C symbols.
+  std::optional<std::string> getSinglePiece() const;
+
+  /// Returns a human-readable version of this symbol name.
+  ///
+  /// If the symbol consists of multiple pieces (aka. it is an Objective-C
+  /// selector/method name), the pieces are separated by `:`, otherwise just an
+  /// identifier name.
+  std::string getAsString() const;
+
+  void print(raw_ostream &OS) const;
+
+  bool operator==(const RenameSymbolName &Other) const {
+    return NamePieces == Other.NamePieces;
+  }
+};
+
 struct RenameInputs {
   Position Pos; // the position triggering the rename
   llvm::StringRef NewName;
@@ -108,9 +151,8 @@ llvm::Expected<Edit> buildRenameEdit(llvm::StringRef AbsFilePath,
 /// occurrence has the same length).
 /// REQUIRED: Indexed is sorted.
 std::optional<std::vector<SymbolRange>>
-adjustRenameRanges(llvm::StringRef DraftCode, llvm::StringRef Identifier,
-                   std::vector<Range> Indexed, const LangOptions &LangOpts,
-                   std::optional<Selector> Selector);
+adjustRenameRanges(llvm::StringRef DraftCode, const RenameSymbolName &Name,
+                   std::vector<Range> Indexed, const LangOptions &LangOpts);
 
 /// Calculates the lexed occurrences that the given indexed occurrences map to.
 /// Returns std::nullopt if we don't find a mapping.

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

@@ -2223,9 +2223,9 @@ TEST(CrossFileRenameTests, adjustRenameRanges) {
   for (const auto &T : Tests) {
     SCOPED_TRACE(T.DraftCode);
     Annotations Draft(T.DraftCode);
-    auto ActualRanges = adjustRenameRanges(Draft.code(), "x",
-                                           Annotations(T.IndexedCode).ranges(),
-                                           LangOpts, std::nullopt);
+    auto ActualRanges = adjustRenameRanges(
+        Draft.code(), RenameSymbolName(ArrayRef<std::string>{"x"}),
+        Annotations(T.IndexedCode).ranges(), LangOpts);
     if (!ActualRanges)
        EXPECT_THAT(Draft.ranges(), testing::IsEmpty());
     else