[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
This commit is contained in:
Kirill Bobyrev 2019-12-12 13:10:59 +01:00
parent 8ddcd1dc26
commit ec618826df
No known key found for this signature in database
GPG Key ID: 2307C055C8384FA0
2 changed files with 109 additions and 70 deletions

View File

@ -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)

View File

@ -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) {