[clang-tools-extra] ec61882 - [clangd] Rename constructors and destructors in cross-file case
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 12 04:13:13 PST 2019
Author: Kirill Bobyrev
Date: 2019-12-12T13:10:59+01:00
New Revision: ec618826dfb91c5413353ebcc54f360e43df10a0
URL: https://github.com/llvm/llvm-project/commit/ec618826dfb91c5413353ebcc54f360e43df10a0
DIFF: https://github.com/llvm/llvm-project/commit/ec618826dfb91c5413353ebcc54f360e43df10a0.diff
LOG: [clangd] Rename constructors and destructors in cross-file case
* Use ad-hoc Decl canonicalization from Clang-Rename to allow renaming
constructors and destructors while using cross-file rename.
* Manually handle the destructor selection
* Add unit tests to prevent regressions and ensure the correct behaviour
Reviewed by: sammccall
Differential Revision: https://reviews.llvm.org/D71247
Added:
Modified:
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 8ca90afba69a..010f7e388b55 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -18,8 +18,10 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
+#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include <algorithm>
@@ -83,21 +85,17 @@ llvm::DenseSet<const Decl *> locateDeclAt(ParsedAST &AST,
if (!SelectedNode)
return {};
- // If the location points to a Decl, we check it is actually on the name
- // range of the Decl. This would avoid allowing rename on unrelated tokens.
- // ^class Foo {} // SelectionTree returns CXXRecordDecl,
- // // we don't attempt to trigger rename on this position.
- // FIXME: Make this work on destructors, e.g. "~F^oo()".
- if (const auto *D = SelectedNode->ASTNode.get<Decl>()) {
- if (D->getLocation() != TokenStartLoc)
- return {};
- }
-
llvm::DenseSet<const Decl *> Result;
for (const auto *D :
targetDecl(SelectedNode->ASTNode,
- DeclRelation::Alias | DeclRelation::TemplatePattern))
- Result.insert(D);
+ DeclRelation::Alias | DeclRelation::TemplatePattern)) {
+ const auto *ND = llvm::dyn_cast<NamedDecl>(D);
+ if (!ND)
+ continue;
+ // Get to CXXRecordDecl from constructor or destructor.
+ ND = tooling::getCanonicalSymbolDeclaration(ND);
+ Result.insert(ND);
+ }
return Result;
}
@@ -214,17 +212,16 @@ llvm::Error makeError(ReasonToReject Reason) {
// Return all rename occurrences in the main file.
std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST,
const NamedDecl &ND) {
- // In theory, locateDeclAt should return the primary template. However, if the
- // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
- // will be the CXXRecordDecl, for this case, we need to get the primary
- // template maunally.
- const auto &RenameDecl =
- ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
+ // If the cursor is at the underlying CXXRecordDecl of the
+ // ClassTemplateDecl, ND will be the CXXRecordDecl. In this case, we need to
+ // get the primary template maunally.
// getUSRsForDeclaration will find other related symbols, e.g. virtual and its
// overriddens, primary template and all explicit specializations.
// FIXME: Get rid of the remaining tooling APIs.
- std::vector<std::string> RenameUSRs = tooling::getUSRsForDeclaration(
- tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext());
+ const auto RenameDecl =
+ ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND;
+ std::vector<std::string> RenameUSRs =
+ tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext());
llvm::DenseSet<SymbolID> TargetIDs;
for (auto &USR : RenameUSRs)
TargetIDs.insert(SymbolID(USR));
@@ -455,14 +452,21 @@ llvm::Expected<FileEdits> rename(const RenameInputs &RInputs) {
return (*Content)->getBuffer().str();
};
- SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
- getBeginningOfIdentifier(RInputs.Pos, SM, AST.getLangOpts()));
+ // Try to find the tokens adjacent to the cursor position.
+ auto Loc = sourceLocationInMainFile(SM, RInputs.Pos);
+ if (!Loc)
+ return Loc.takeError();
+ const syntax::Token *IdentifierToken =
+ spelledIdentifierTouching(*Loc, AST.getTokens());
+ // Renames should only triggered on identifiers.
+ if (!IdentifierToken)
+ return makeError(ReasonToReject::NoSymbolFound);
// FIXME: Renaming macros is not supported yet, the macro-handling code should
// be moved to rename tooling library.
- if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
+ if (locateMacroAt(IdentifierToken->location(), AST.getPreprocessor()))
return makeError(ReasonToReject::UnsupportedSymbol);
- auto DeclsUnderCursor = locateDeclAt(AST, SourceLocationBeg);
+ auto DeclsUnderCursor = locateDeclAt(AST, IdentifierToken->location());
if (DeclsUnderCursor.empty())
return makeError(ReasonToReject::NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index f253625ed032..e98bf78323b9 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -487,11 +487,10 @@ TEST(RenameTest, Renameable) {
"not a supported kind", HeaderFile, Index},
{
-
R"cpp(
struct X { X operator++(int); };
void f(X x) {x+^+;})cpp",
- "not a supported kind", HeaderFile, Index},
+ "no symbol", HeaderFile, Index},
{R"cpp(// foo is declared outside the file.
void fo^o() {}
@@ -720,24 +719,24 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) {
MockCompilationDatabase CDB;
CDB.ExtraClangFlags = {"-xc++"};
class IgnoreDiagnostics : public DiagnosticsConsumer {
- void onDiagnosticsReady(PathRef File,
- std::vector<Diag> Diagnostics) override {}
+ void onDiagnosticsReady(PathRef File,
+ std::vector<Diag> Diagnostics) override {}
} DiagConsumer;
// rename is runnning on the "^" point in FooH, and "[[]]" ranges are the
- // expcted rename occurrences.
+ // expected rename occurrences.
struct Case {
llvm::StringRef FooH;
llvm::StringRef FooCC;
- } Cases [] = {
- {
- // classes.
- R"cpp(
+ } Cases[] = {
+ {
+ // classes.
+ R"cpp(
class [[Fo^o]] {
[[Foo]]();
~[[Foo]]();
};
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
[[Foo]]::[[Foo]]() {}
[[Foo]]::~[[Foo]]() {}
@@ -746,15 +745,15 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) {
[[Foo]] foo;
}
)cpp",
- },
- {
- // class methods.
- R"cpp(
+ },
+ {
+ // class methods.
+ R"cpp(
class Foo {
void [[f^oo]]();
};
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
void Foo::[[foo]]() {}
@@ -762,13 +761,49 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) {
p->[[foo]]();
}
)cpp",
- },
- {
- // functions.
- R"cpp(
+ },
+ {
+ // Constructor.
+ R"cpp(
+ class [[Foo]] {
+ [[^Foo]]();
+ ~[[Foo]]();
+ };
+ )cpp",
+ R"cpp(
+ #include "foo.h"
+ [[Foo]]::[[Foo]]() {}
+ [[Foo]]::~[[Foo]]() {}
+
+ void func() {
+ [[Foo]] foo;
+ }
+ )cpp",
+ },
+ {
+ // Destructor (selecting before the identifier).
+ R"cpp(
+ class [[Foo]] {
+ [[Foo]]();
+ ~[[Foo^]]();
+ };
+ )cpp",
+ R"cpp(
+ #include "foo.h"
+ [[Foo]]::[[Foo]]() {}
+ [[Foo]]::~[[Foo]]() {}
+
+ void func() {
+ [[Foo]] foo;
+ }
+ )cpp",
+ },
+ {
+ // functions.
+ R"cpp(
void [[f^oo]]();
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
void [[foo]]() {}
@@ -776,63 +811,63 @@ TEST(CrossFileRenameTests, WithUpToDateIndex) {
[[foo]]();
}
)cpp",
- },
- {
- // typedefs.
- R"cpp(
+ },
+ {
+ // typedefs.
+ R"cpp(
typedef int [[IN^T]];
[[INT]] foo();
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
[[INT]] foo() {}
)cpp",
- },
- {
- // usings.
- R"cpp(
+ },
+ {
+ // usings.
+ R"cpp(
using [[I^NT]] = int;
[[INT]] foo();
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
[[INT]] foo() {}
)cpp",
- },
- {
- // variables.
- R"cpp(
+ },
+ {
+ // variables.
+ R"cpp(
static const int [[VA^R]] = 123;
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
int s = [[VAR]];
)cpp",
- },
- {
- // scope enums.
- R"cpp(
+ },
+ {
+ // scope enums.
+ R"cpp(
enum class [[K^ind]] { ABC };
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
[[Kind]] ff() {
return [[Kind]]::ABC;
}
)cpp",
- },
- {
- // enum constants.
- R"cpp(
+ },
+ {
+ // enum constants.
+ R"cpp(
enum class Kind { [[A^BC]] };
)cpp",
- R"cpp(
+ R"cpp(
#include "foo.h"
Kind ff() {
return Kind::[[ABC]];
}
)cpp",
- },
+ },
};
for (const auto& T : Cases) {
More information about the cfe-commits
mailing list