diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp @@ -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 namespace clang { namespace clangd { @@ -40,6 +48,51 @@ return nullptr; } +llvm::Optional 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 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 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 @@ } Expected 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: diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/TweakTesting.cpp --- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp @@ -127,7 +127,7 @@ 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; diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -1868,6 +1868,110 @@ })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 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 void fo^o(T, T x) { return; }", + "template void foo(T, T x) ;", + "template void foo(T, T x) { return; }", + }, + { + "template void fo^o() { return; }", + "template void foo() ;", + "template void foo() { return; }", + }, + // Template specialization. + { + R"cpp( + template void foo(); + template <> void fo^o() { return; })cpp", + R"cpp( + template void foo(); + template <> void foo() ;)cpp", + "template <> void foo() { 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 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