diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 650862c99bcd1..4e135801f6853 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -811,8 +811,18 @@ renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl, continue; Locs.push_back(RenameLoc); } - if (const auto *MD = dyn_cast(&RenameDecl)) - return renameObjCMethodWithinFile(AST, MD, NewName, std::move(Locs)); + if (const auto *MD = dyn_cast(&RenameDecl)) { + // The custom ObjC selector logic doesn't handle the zero arg selector + // case, as it relies on parsing selectors via the trailing `:`. + // We also choose to use regular rename logic for the single-arg selectors + // as the AST/Index has the right locations in that case. + if (MD->getSelector().getNumArgs() > 1) + return renameObjCMethodWithinFile(AST, MD, NewName, std::move(Locs)); + + // Eat trailing : for single argument methods since they're actually + // considered a separate token during rename. + NewName.consume_back(":"); + } for (const auto &Loc : Locs) { if (auto Err = FilteredChanges.add(tooling::Replacement( SM, CharSourceRange::getTokenRange(Loc), NewName))) @@ -930,10 +940,9 @@ renameOutsideFile(const NamedDecl &RenameDecl, llvm::StringRef MainFilePath, std::optional Selector = std::nullopt; llvm::SmallVector NewNames; if (const auto *MD = dyn_cast(&RenameDecl)) { - if (MD->getSelector().getNumArgs() > 1) { - RenameIdentifier = MD->getSelector().getNameForSlot(0).str(); + RenameIdentifier = MD->getSelector().getNameForSlot(0).str(); + if (MD->getSelector().getNumArgs() > 1) Selector = MD->getSelector(); - } } NewName.split(NewNames, ":"); diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index d91ef85d67271..7d9252110b27d 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1943,6 +1943,144 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) { } } +TEST(CrossFileRenameTests, ObjC) { + MockCompilationDatabase CDB; + CDB.ExtraClangFlags = {"-xobjective-c"}; + // rename is runnning on all "^" points in FooH. + struct Case { + llvm::StringRef FooH; + llvm::StringRef FooM; + llvm::StringRef NewName; + llvm::StringRef ExpectedFooH; + llvm::StringRef ExpectedFooM; + }; + Case Cases[] = {// --- Zero arg selector + { + // Input + R"cpp( + @interface Foo + - (int)performA^ction; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performAction { + [self performAction]; + } + @end + )cpp", + // New name + "performNewAction", + // Expected + R"cpp( + @interface Foo + - (int)performNewAction; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performNewAction { + [self performNewAction]; + } + @end + )cpp", + }, + // --- Single arg selector + { + // Input + R"cpp( + @interface Foo + - (int)performA^ction:(int)action; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performAction:(int)action { + [self performAction:action]; + } + @end + )cpp", + // New name + "performNewAction:", + // Expected + R"cpp( + @interface Foo + - (int)performNewAction:(int)action; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performNewAction:(int)action { + [self performNewAction:action]; + } + @end + )cpp", + }, + // --- Multi arg selector + { + // Input + R"cpp( + @interface Foo + - (int)performA^ction:(int)action with:(int)value; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performAction:(int)action with:(int)value { + [self performAction:action with:value]; + } + @end + )cpp", + // New name + "performNewAction:by:", + // Expected + R"cpp( + @interface Foo + - (int)performNewAction:(int)action by:(int)value; + @end + )cpp", + R"cpp( + @implementation Foo + - (int)performNewAction:(int)action by:(int)value { + [self performNewAction:action by:value]; + } + @end + )cpp", + }}; + + trace::TestTracer Tracer; + for (const auto &T : Cases) { + SCOPED_TRACE(T.FooH); + Annotations FooH(T.FooH); + Annotations FooM(T.FooM); + std::string FooHPath = testPath("foo.h"); + std::string FooMPath = testPath("foo.m"); + + MockFS FS; + FS.Files[FooHPath] = std::string(FooH.code()); + FS.Files[FooMPath] = std::string(FooM.code()); + + auto ServerOpts = ClangdServer::optsForTest(); + ServerOpts.BuildDynamicSymbolIndex = true; + ClangdServer Server(CDB, FS, ServerOpts); + + // Add all files to clangd server to make sure the dynamic index has been + // built. + runAddDocument(Server, FooHPath, FooH.code()); + runAddDocument(Server, FooMPath, FooM.code()); + + for (const auto &RenamePos : FooH.points()) { + EXPECT_THAT(Tracer.takeMetric("rename_files"), SizeIs(0)); + auto FileEditsList = + llvm::cantFail(runRename(Server, FooHPath, RenamePos, T.NewName, {})); + EXPECT_THAT(Tracer.takeMetric("rename_files"), ElementsAre(2)); + EXPECT_THAT(applyEdits(std::move(FileEditsList.GlobalChanges)), + UnorderedElementsAre(Pair(Eq(FooHPath), Eq(T.ExpectedFooH)), + Pair(Eq(FooMPath), Eq(T.ExpectedFooM)))); + } + } +} + TEST(CrossFileRenameTests, CrossFileOnLocalSymbol) { // cross-file rename should work for function-local symbols, even there is no // index provided.