forked from OSchip/llvm-project
[clangd] Define out-of-line initial apply logic
Summary: Initial implementation for apply logic, replaces function body with a semicolon in source location and copies the full function definition into target location. Will handle qualification of return type and function name in following patches to keep the changes small. Reviewers: hokein Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D69298
This commit is contained in:
parent
9f251eece4
commit
ce21892022
|
@ -13,9 +13,17 @@
|
|||
#include "refactor/Tweak.h"
|
||||
#include "clang/AST/ASTTypeTraits.h"
|
||||
#include "clang/AST/Decl.h"
|
||||
#include "clang/AST/DeclTemplate.h"
|
||||
#include "clang/AST/Stmt.h"
|
||||
#include "clang/Basic/SourceLocation.h"
|
||||
#include "clang/Basic/SourceManager.h"
|
||||
#include "clang/Driver/Types.h"
|
||||
#include "clang/Format/Format.h"
|
||||
#include "clang/Tooling/Core/Replacement.h"
|
||||
#include "llvm/ADT/Optional.h"
|
||||
#include "llvm/Support/Path.h"
|
||||
#include "llvm/ADT/StringRef.h"
|
||||
#include "llvm/Support/Error.h"
|
||||
#include <cstddef>
|
||||
|
||||
namespace clang {
|
||||
namespace clangd {
|
||||
|
@ -40,6 +48,51 @@ const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
|
|||
return nullptr;
|
||||
}
|
||||
|
||||
llvm::Optional<Path> getSourceFile(llvm::StringRef FileName,
|
||||
const Tweak::Selection &Sel) {
|
||||
if (auto Source = getCorrespondingHeaderOrSource(
|
||||
FileName,
|
||||
&Sel.AST.getSourceManager().getFileManager().getVirtualFileSystem()))
|
||||
return *Source;
|
||||
return getCorrespondingHeaderOrSource(FileName, Sel.AST, Sel.Index);
|
||||
}
|
||||
|
||||
// Creates a modified version of function definition that can be inserted at a
|
||||
// different location. Contains both function signature and body.
|
||||
llvm::Optional<llvm::StringRef> getFunctionSourceCode(const FunctionDecl *FD) {
|
||||
auto &SM = FD->getASTContext().getSourceManager();
|
||||
auto CharRange = toHalfOpenFileRange(SM, FD->getASTContext().getLangOpts(),
|
||||
FD->getSourceRange());
|
||||
if (!CharRange)
|
||||
return llvm::None;
|
||||
// Include template parameter list.
|
||||
if (auto *FTD = FD->getDescribedFunctionTemplate())
|
||||
CharRange->setBegin(FTD->getBeginLoc());
|
||||
|
||||
// FIXME: Qualify return type.
|
||||
// FIXME: Qualify function name depending on the target context.
|
||||
return toSourceCode(SM, *CharRange);
|
||||
}
|
||||
|
||||
// Returns the most natural insertion point for \p QualifiedName in \p Contents.
|
||||
// This currently cares about only the namespace proximity, but in feature it
|
||||
// should also try to follow ordering of declarations. For example, if decls
|
||||
// come in order `foo, bar, baz` then this function should return some point
|
||||
// between foo and baz for inserting bar.
|
||||
llvm::Expected<size_t> getInsertionOffset(llvm::StringRef Contents,
|
||||
llvm::StringRef QualifiedName,
|
||||
const format::FormatStyle &Style) {
|
||||
auto Region = getEligiblePoints(Contents, QualifiedName, Style);
|
||||
|
||||
assert(!Region.EligiblePoints.empty());
|
||||
// FIXME: This selection can be made smarter by looking at the definition
|
||||
// locations for adjacent decls to Source. Unfortunately psudeo parsing in
|
||||
// getEligibleRegions only knows about namespace begin/end events so we
|
||||
// can't match function start/end positions yet.
|
||||
auto InsertionPoint = Region.EligiblePoints.back();
|
||||
return positionToOffset(Contents, InsertionPoint);
|
||||
}
|
||||
|
||||
/// Moves definition of a function/method to an appropriate implementation file.
|
||||
///
|
||||
/// Before:
|
||||
|
@ -94,8 +147,64 @@ public:
|
|||
}
|
||||
|
||||
Expected<Effect> apply(const Selection &Sel) override {
|
||||
return llvm::createStringError(llvm::inconvertibleErrorCode(),
|
||||
"Not implemented yet");
|
||||
const SourceManager &SM = Sel.AST.getSourceManager();
|
||||
auto MainFileName =
|
||||
getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
|
||||
if (!MainFileName)
|
||||
return llvm::createStringError(
|
||||
llvm::inconvertibleErrorCode(),
|
||||
"Couldn't get absolute path for mainfile.");
|
||||
|
||||
auto CCFile = getSourceFile(*MainFileName, Sel);
|
||||
if (!CCFile)
|
||||
return llvm::createStringError(
|
||||
llvm::inconvertibleErrorCode(),
|
||||
"Couldn't find a suitable implementation file.");
|
||||
|
||||
auto &FS =
|
||||
Sel.AST.getSourceManager().getFileManager().getVirtualFileSystem();
|
||||
auto Buffer = FS.getBufferForFile(*CCFile);
|
||||
// FIXME: Maybe we should consider creating the implementation file if it
|
||||
// doesn't exist?
|
||||
if (!Buffer)
|
||||
return llvm::createStringError(Buffer.getError(),
|
||||
Buffer.getError().message());
|
||||
auto Contents = Buffer->get()->getBuffer();
|
||||
auto InsertionOffset =
|
||||
getInsertionOffset(Contents, Source->getQualifiedNameAsString(),
|
||||
getFormatStyleForFile(*CCFile, Contents, &FS));
|
||||
if (!InsertionOffset)
|
||||
return InsertionOffset.takeError();
|
||||
|
||||
auto FuncDef = getFunctionSourceCode(Source);
|
||||
if (!FuncDef)
|
||||
return llvm::createStringError(
|
||||
llvm::inconvertibleErrorCode(),
|
||||
"Couldn't get full source for function definition.");
|
||||
|
||||
SourceManagerForFile SMFF(*CCFile, Contents);
|
||||
const tooling::Replacement InsertFunctionDef(*CCFile, *InsertionOffset, 0,
|
||||
*FuncDef);
|
||||
auto Effect = Effect::mainFileEdit(
|
||||
SMFF.get(), tooling::Replacements(InsertFunctionDef));
|
||||
if (!Effect)
|
||||
return Effect.takeError();
|
||||
|
||||
// FIXME: We should also get rid of inline qualifier.
|
||||
const tooling::Replacement DeleteFuncBody(
|
||||
Sel.AST.getSourceManager(),
|
||||
CharSourceRange::getTokenRange(
|
||||
*toHalfOpenFileRange(SM, Sel.AST.getASTContext().getLangOpts(),
|
||||
Source->getBody()->getSourceRange())),
|
||||
";");
|
||||
auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(),
|
||||
tooling::Replacements(DeleteFuncBody));
|
||||
if (!HeaderFE)
|
||||
return HeaderFE.takeError();
|
||||
|
||||
Effect->ApplyEdits.try_emplace(HeaderFE->first,
|
||||
std::move(HeaderFE->second));
|
||||
return std::move(*Effect);
|
||||
}
|
||||
|
||||
private:
|
||||
|
|
|
@ -127,7 +127,7 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
|
|||
ADD_FAILURE() << "There were changes to additional files, but client "
|
||||
"provided a nullptr for EditedFiles.";
|
||||
else
|
||||
EditedFiles->try_emplace(It.first(), Unwrapped.str());
|
||||
EditedFiles->insert_or_assign(It.first(), Unwrapped.str());
|
||||
}
|
||||
}
|
||||
return EditedMainFile;
|
||||
|
|
|
@ -1868,6 +1868,110 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
|
|||
})cpp");
|
||||
}
|
||||
|
||||
TEST_F(DefineOutlineTest, FailsWithoutSource) {
|
||||
FileName = "Test.hpp";
|
||||
llvm::StringRef Test = "void fo^o() { return; }";
|
||||
llvm::StringRef Expected =
|
||||
"fail: Couldn't find a suitable implementation file.";
|
||||
EXPECT_EQ(apply(Test), Expected);
|
||||
}
|
||||
|
||||
TEST_F(DefineOutlineTest, ApplyTest) {
|
||||
llvm::StringMap<std::string> EditedFiles;
|
||||
ExtraFiles["Test.cpp"] = "";
|
||||
FileName = "Test.hpp";
|
||||
|
||||
struct {
|
||||
llvm::StringRef Test;
|
||||
llvm::StringRef ExpectedHeader;
|
||||
llvm::StringRef ExpectedSource;
|
||||
} Cases[] = {
|
||||
// Simple check
|
||||
{
|
||||
"void fo^o() { return; }",
|
||||
"void foo() ;",
|
||||
"void foo() { return; }",
|
||||
},
|
||||
// Templated function.
|
||||
{
|
||||
"template <typename T> void fo^o(T, T x) { return; }",
|
||||
"template <typename T> void foo(T, T x) ;",
|
||||
"template <typename T> void foo(T, T x) { return; }",
|
||||
},
|
||||
{
|
||||
"template <typename> void fo^o() { return; }",
|
||||
"template <typename> void foo() ;",
|
||||
"template <typename> void foo() { return; }",
|
||||
},
|
||||
// Template specialization.
|
||||
{
|
||||
R"cpp(
|
||||
template <typename> void foo();
|
||||
template <> void fo^o<int>() { return; })cpp",
|
||||
R"cpp(
|
||||
template <typename> void foo();
|
||||
template <> void foo<int>() ;)cpp",
|
||||
"template <> void foo<int>() { return; }",
|
||||
},
|
||||
};
|
||||
for (const auto &Case : Cases) {
|
||||
SCOPED_TRACE(Case.Test);
|
||||
EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader);
|
||||
EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
|
||||
testPath("Test.cpp"), Case.ExpectedSource)));
|
||||
}
|
||||
}
|
||||
|
||||
TEST_F(DefineOutlineTest, HandleMacros) {
|
||||
llvm::StringMap<std::string> EditedFiles;
|
||||
ExtraFiles["Test.cpp"] = "";
|
||||
FileName = "Test.hpp";
|
||||
|
||||
struct {
|
||||
llvm::StringRef Test;
|
||||
llvm::StringRef ExpectedHeader;
|
||||
llvm::StringRef ExpectedSource;
|
||||
} Cases[] = {
|
||||
{R"cpp(
|
||||
#define BODY { return; }
|
||||
void f^oo()BODY)cpp",
|
||||
R"cpp(
|
||||
#define BODY { return; }
|
||||
void foo();)cpp",
|
||||
"void foo()BODY"},
|
||||
|
||||
{R"cpp(
|
||||
#define BODY return;
|
||||
void f^oo(){BODY})cpp",
|
||||
R"cpp(
|
||||
#define BODY return;
|
||||
void foo();)cpp",
|
||||
"void foo(){BODY}"},
|
||||
|
||||
{R"cpp(
|
||||
#define TARGET void foo()
|
||||
[[TARGET]]{ return; })cpp",
|
||||
R"cpp(
|
||||
#define TARGET void foo()
|
||||
TARGET;)cpp",
|
||||
"TARGET{ return; }"},
|
||||
|
||||
{R"cpp(
|
||||
#define TARGET foo
|
||||
void [[TARGET]](){ return; })cpp",
|
||||
R"cpp(
|
||||
#define TARGET foo
|
||||
void TARGET();)cpp",
|
||||
"void TARGET(){ return; }"},
|
||||
};
|
||||
for (const auto &Case : Cases) {
|
||||
SCOPED_TRACE(Case.Test);
|
||||
EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader);
|
||||
EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
|
||||
testPath("Test.cpp"), Case.ExpectedSource)));
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace clangd
|
||||
} // namespace clang
|
||||
|
|
Loading…
Reference in New Issue