diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "AST.h" +#include "FindTarget.h" #include "Logger.h" #include "Selection.h" #include "SourceCode.h" @@ -59,6 +60,26 @@ namespace clangd { namespace { +// Lexes from end of \p FD until it finds a semicolon. +llvm::Optional getSemiColonForDecl(const FunctionDecl *FD) { + const SourceManager &SM = FD->getASTContext().getSourceManager(); + const LangOptions &LangOpts = FD->getASTContext().getLangOpts(); + + SourceLocation SemiColon = + Lexer::getLocForEndOfToken(FD->getEndLoc(), 0, SM, LangOpts); + llvm::StringRef BufData = SM.getBufferData(SM.getFileID(SemiColon)); + Lexer RawLexer(SM.getLocForStartOfFile(SM.getFileID(SemiColon)), LangOpts, + BufData.begin(), SM.getCharacterData(SemiColon), + BufData.end()); + Token CurToken; + while (!CurToken.is(tok::semi)) { + if (RawLexer.LexFromRawLexer(CurToken)) + return llvm::None; + } + SemiColon = CurToken.getLocation(); + return SemiColon; +} + // Deduces the FunctionDecl from a selection. Requires either the function body // or the function decl to be selected. Returns null if none of the above // criteria is met. @@ -113,6 +134,71 @@ return true; } +// Rewrites body of FD to fully-qualify all of the decls inside. +llvm::Expected qualifyAllDecls(const FunctionDecl *FD) { + // There are three types of spellings that needs to be qualified in a function + // body: + // - Types: Foo -> ns::Foo + // - DeclRefExpr: ns2::foo() -> ns1::ns2::foo(); + // - UsingDecls: + // using ns2::foo -> using ns1::ns2::foo + // using namespace ns2 -> using namespace ns1::ns2 + // using ns3 = ns2 -> using ns3 = ns1::ns2 + // + // Go over all references inside a function body to generate replacements that + // will fully qualify those. So that body can be moved into an arbitrary file. + // We perform the qualification by qualyfying the first type/decl in a + // (un)qualified name. e.g: + // namespace a { namespace b { class Bar{}; void foo(); } } + // b::Bar x; -> a::b::Bar x; + // foo(); -> a::b::foo(); + // FIXME: Instead of fully qualyfying we should try deducing visible scopes at + // target location and generate minimal edits. + + const SourceManager &SM = FD->getASTContext().getSourceManager(); + tooling::Replacements Replacements; + findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) { + // Since we want to qualify only the first qualifier, skip names with a + // qualifier. + if (Ref.Qualifier.hasQualifier()) + return; + // There might be no decl in dependent contexts, there's nothing much we can + // do in such cases. + if (Ref.Targets.empty()) + return; + + // All Targets should be in the same nested name scope, so we can safely + // chose any one of them. + const NamedDecl *ND = Ref.Targets.front(); + // Skip local symbols. + if (index::isFunctionLocalSymbol(ND)) + return; + // Skip members as qualyfying the left side is enough. + if (ND->isCXXInstanceMember()) + return; + + std::string Qualifier = printNamespaceScope(*ND->getDeclContext()); + + if (auto Err = Replacements.add( + tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) + elog("Failed to add qualifier: {0}", std::move(Err)); + }); + + auto QualifiedFunc = tooling::applyAllReplacements( + SM.getBufferData(SM.getFileID(FD->getLocation())), Replacements); + if (!QualifiedFunc) + return QualifiedFunc.takeError(); + + // Get new begin and end positions for the qualified body. + SourceRange OrigFuncRange = FD->getBody()->getSourceRange(); + unsigned BodyBegin = SM.getFileOffset(OrigFuncRange.getBegin()); + unsigned BodyEnd = Replacements.getShiftedCodePosition( + SM.getFileOffset(OrigFuncRange.getEnd())); + + // Trim the result to function body. + return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1); +} + // Returns the canonical declaration for the given FunctionDecl. This will // usually be the first declaration in current translation unit with the // exception of template specialization. @@ -134,6 +220,15 @@ return PrevDecl; } +// Returns the begining location for a FunctionDecl. Returns location of +// template keyword for templated functions. +const SourceLocation getBeginLoc(const FunctionDecl *FD) { + // Include template parameter list. + if (auto *FTD = FD->getDescribedFunctionTemplate()) + return FTD->getBeginLoc(); + return FD->getBeginLoc(); +} + /// Moves definition of a function/method to its declaration location. /// Before: /// a.h: @@ -157,7 +252,6 @@ std::string title() const override { return "Move function body to declaration"; } - bool hidden() const override { return true; } // Returns true when selection is on a function definition that does not // make use of any internal symbols. @@ -189,8 +283,51 @@ } Expected apply(const Selection &Sel) override { - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Not implemented yet"); + const ASTContext &AST = Sel.AST.getASTContext(); + const SourceManager &SM = AST.getSourceManager(); + + auto QualifiedBody = qualifyAllDecls(Source); + if (!QualifiedBody) + return QualifiedBody.takeError(); + + auto SemiColon = getSemiColonForDecl(Target); + if (!SemiColon) { + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Couldn't find semicolon for target declaration"); + } + + tooling::Replacements Replacements; + auto Err = Replacements.add( + tooling::Replacement(SM, *SemiColon, 1, *QualifiedBody)); + if (Err) + return std::move(Err); + Effect E; + + // We need to generate two edits if the Source and Target are in different + // files. + if (!SM.isWrittenInSameFile(Source->getBeginLoc(), Target->getBeginLoc())) { + auto FE = Effect::fileEdit(SM, SM.getFileID(*SemiColon), + std::move(Replacements)); + if (!FE) + return FE.takeError(); + E.ApplyEdits.try_emplace(FE->first, std::move(FE->second)); + Replacements.clear(); + } + + SourceLocation BeginLoc = getBeginLoc(Source); + unsigned int SourceLen = + SM.getFileOffset(Source->getEndLoc()) - SM.getFileOffset(BeginLoc) + 1; + Err = Replacements.add(tooling::Replacement(SM, BeginLoc, SourceLen, "")); + if (Err) + return std::move(Err); + + auto FE = + Effect::fileEdit(SM, SM.getFileID(Sel.Cursor), std::move(Replacements)); + if (!FE) + return FE.takeError(); + E.ApplyEdits.try_emplace(FE->first, std::move(FE->second)); + return E; } private: diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.h b/clang-tools-extra/clangd/unittests/TweakTesting.h --- a/clang-tools-extra/clangd/unittests/TweakTesting.h +++ b/clang-tools-extra/clangd/unittests/TweakTesting.h @@ -51,6 +51,7 @@ // Mapping from file name to contents. llvm::StringMap ExtraFiles; + llvm::StringMap EditedFiles; protected: TweakTest(const char *TweakID) : TweakID(TweakID) {} @@ -65,13 +66,15 @@ // Apply the current tweak to the range (or point) in MarkedCode. // MarkedCode will be wrapped according to the Context. - // - if the tweak produces edits, returns the edited code (without markings). + // - if the tweak produces edits, returns the edited code (without markings) + // for the main file. Populates \p EditedFiles if there were any other + // changes. // The context added to MarkedCode will be stripped away before returning, // unless the tweak edited it. // - if the tweak produces a message, returns "message:\n" // - if prepare() returns false, returns "unavailable" // - if apply() returns an error, returns "fail: " - std::string apply(llvm::StringRef MarkedCode) const; + std::string apply(llvm::StringRef MarkedCode); // Accepts a code snippet with many ranges (or points) marked, and returns a // list of snippets with one range marked each. 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 @@ -10,9 +10,11 @@ #include "Annotations.h" #include "SourceCode.h" +#include "TestFS.h" #include "refactor/Tweak.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/Support/Error.h" +#include namespace clang { namespace clangd { @@ -79,12 +81,13 @@ } // namespace -std::string TweakTest::apply(llvm::StringRef MarkedCode) const { +std::string TweakTest::apply(llvm::StringRef MarkedCode) { std::string WrappedCode = wrap(Context, MarkedCode); Annotations Input(WrappedCode); auto Selection = rangeOrPoint(Input); TestTU TU; TU.HeaderCode = Header; + TU.AdditionalFiles = std::move(ExtraFiles); TU.Code = Input.code(); ParsedAST AST = TU.build(); Tweak::Selection S(AST, Selection.first, Selection.second); @@ -101,14 +104,19 @@ return "message:\n" + *Result->ShowMessage; if (Result->ApplyEdits.empty()) return "no effect"; - if (Result->ApplyEdits.size() > 1) - return "received multi-file edits"; - auto ApplyEdit = Result->ApplyEdits.begin()->second; - if (auto NewText = ApplyEdit.apply()) - return unwrap(Context, *NewText); - else - return "bad edits: " + llvm::toString(NewText.takeError()); + std::string EditedMainFile; + for (auto &It : Result->ApplyEdits) { + auto NewText = It.second.apply(); + if (!NewText) + return "bad edits: " + llvm::toString(NewText.takeError()); + llvm::StringRef Unwrapped = unwrap(Context, *NewText); + if (It.first() == testPath(TU.Filename)) + EditedMainFile = Unwrapped; + else + EditedFiles.try_emplace(It.first(), Unwrapped.str()); + } + return EditedMainFile; } ::testing::Matcher TweakTest::isAvailable() const { 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 @@ -25,6 +25,7 @@ #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" @@ -103,7 +104,7 @@ EXPECT_UNAVAILABLE(R"cpp(R"(multi )" ^"token " u8"str\ning")cpp"); // nonascii EXPECT_UNAVAILABLE(R"cpp(^R^"^(^multi )" "token " "str\ning")cpp"); // raw EXPECT_UNAVAILABLE(R"cpp(^"token\n" __FILE__)cpp"); // chunk is macro - EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char + EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char const char *Input = R"cpp(R"(multi token)" "\nst^ring\n" "literal")cpp"; @@ -772,6 +773,449 @@ })cpp"); } +TEST_F(DefineInlineTest, TransformUsings) { + EXPECT_EQ(apply(R"cpp( + namespace a { + void bar(); + namespace b { + void baz(); + namespace c { + void aux(); + } + } + } + + void foo(); + void f^oo() { + using namespace a; + + using namespace b; + using namespace a::b; + + using namespace c; + using namespace b::c; + using namespace a::b::c; + + using a::bar; + + using b::baz; + using a::b::baz; + + using c::aux; + using b::c::aux; + using a::b::c::aux; + + namespace d = c; + namespace d = b::c; + } + )cpp"), + R"cpp( + namespace a { + void bar(); + namespace b { + void baz(); + namespace c { + void aux(); + } + } + } + + void foo(){ + using namespace a; + + using namespace a::b; + using namespace a::b; + + using namespace a::b::c; + using namespace a::b::c; + using namespace a::b::c; + + using a::bar; + + using a::b::baz; + using a::b::baz; + + using a::b::c::aux; + using a::b::c::aux; + using a::b::c::aux; + + namespace d = a::b::c; + namespace d = a::b::c; + } + + )cpp"); +} + +TEST_F(DefineInlineTest, TransformDecls) { + EXPECT_EQ(apply(R"cpp( + void foo() /*Comment -_-*/ ; + + void f^oo() { + class Foo { + public: + void foo(); + int x; + static int y; + }; + Foo::y = 0; + + enum En { Zero, One }; + En x = Zero; + + enum class EnClass { Zero, One }; + EnClass y = EnClass::Zero; + + template class Bar {}; + Bar z; + } + )cpp"), + R"cpp( + void foo() /*Comment -_-*/ { + class Foo { + public: + void foo(); + int x; + static int y; + }; + Foo::y = 0; + + enum En { Zero, One }; + En x = Zero; + + enum class EnClass { Zero, One }; + EnClass y = EnClass::Zero; + + template class Bar {}; + Bar z; + } + + + )cpp"); +} + +TEST_F(DefineInlineTest, TransformTemplDecls) { + EXPECT_EQ(apply(R"cpp( + namespace a { + template class Bar { + public: + void bar(); + }; + template T bar; + template void aux() {} + } + + void foo() /*Comment -_-*/ ; + + void f^oo() { + using namespace a; + bar>.bar(); + aux>(); + } + )cpp"), + R"cpp( + namespace a { + template class Bar { + public: + void bar(); + }; + template T bar; + template void aux() {} + } + + void foo() /*Comment -_-*/ { + using namespace a; + a::bar>.bar(); + a::aux>(); + } + + + )cpp"); +} + +MATCHER_P2(FileWithContents, FileName, Contents, "") { + return arg.first() == FileName && arg.second == Contents; +} + +TEST_F(DefineInlineTest, TransformMembers) { + EXPECT_EQ(apply(R"cpp( + class Foo { + void foo() /*Comment -_-*/ ; + }; + + void Foo::f^oo() { + return; + } + )cpp"), + R"cpp( + class Foo { + void foo() /*Comment -_-*/ { + return; + } + }; + + + )cpp"); + + ExtraFiles["a.h"] = R"cpp( + class Foo { + void foo() /*Comment -_-*/ ; + };)cpp"; + EXPECT_EQ(apply(R"cpp( + #include "a.h" + void Foo::f^oo() { + return; + })cpp"), + R"cpp( + #include "a.h" + )cpp"); + EXPECT_THAT(EditedFiles, + testing::ElementsAre(FileWithContents(testPath("a.h"), + R"cpp( + class Foo { + void foo() /*Comment -_-*/ { + return; + } + };)cpp"))); +} + +TEST_F(DefineInlineTest, TransformDependentTypes) { + EXPECT_EQ(apply(R"cpp( + namespace a { + template class Bar {}; + } + using namespace a; + + template + void foo() /*Comment -_-*/ ; + + template + void f^oo() { + Bar B; + Bar> q; + } + )cpp"), + R"cpp( + namespace a { + template class Bar {}; + } + using namespace a; + + template + void foo() /*Comment -_-*/ { + a::Bar B; + a::Bar> q; + } + + + )cpp"); +} + +TEST_F(DefineInlineTest, TransformFunctionTempls) { + EXPECT_EQ(apply(R"cpp( + template + void foo(T p); + + template <> + void foo(int p); + + template <> + void foo(char p); + + template <> + void fo^o(int p) { + return; + } + )cpp"), + R"cpp( + template + void foo(T p); + + template <> + void foo(int p){ + return; + } + + template <> + void foo(char p); + + + )cpp"); + + EXPECT_EQ(apply(R"cpp( + template + void foo(T p); + + template <> + void foo(int p); + + template <> + void foo(char p); + + template <> + void fo^o(char p) { + return; + } + )cpp"), + R"cpp( + template + void foo(T p); + + template <> + void foo(int p); + + template <> + void foo(char p){ + return; + } + + + )cpp"); + + EXPECT_EQ(apply(R"cpp( + template + void foo(T p); + + template <> + void foo(int p); + + template + void fo^o(T p) { + return; + } + )cpp"), + R"cpp( + template + void foo(T p){ + return; + } + + template <> + void foo(int p); + + + )cpp"); +} + +TEST_F(DefineInlineTest, TransformTypeLocs) { + EXPECT_EQ(apply(R"cpp( + namespace a { + template class Bar { + public: + template class Baz {}; + }; + namespace b{ + class Foo{}; + }; + } + using namespace a; + using namespace b; + using namespace c; + + void foo() /*Comment -_-*/ ; + + void f^oo() { + Bar B; + b::Foo foo; + a::Bar>::Baz> q; + } + )cpp"), + R"cpp( + namespace a { + template class Bar { + public: + template class Baz {}; + }; + namespace b{ + class Foo{}; + }; + } + using namespace a; + using namespace b; + using namespace c; + + void foo() /*Comment -_-*/ { + a::Bar B; + a::b::Foo foo; + a::Bar>::Baz> q; + } + + + )cpp"); +} + +TEST_F(DefineInlineTest, TransformDeclRefs) { + EXPECT_EQ(apply(R"cpp( + namespace a { + template class Bar { + public: + void foo(); + static void bar(); + int x; + static int y; + }; + void bar(); + namespace b { + class Foo{}; + namespace c { + void test(); + } + } + } + using namespace a; + using namespace b; + using namespace c; + + void foo() /*Comment -_-*/ ; + + void f^oo() { + a::Bar B; + B.foo(); + a::bar(); + Bar>::bar(); + a::Bar::bar(); + B.x = Bar::y; + Bar::y = 3; + bar(); + c::test(); + } + )cpp"), + R"cpp( + namespace a { + template class Bar { + public: + void foo(); + static void bar(); + int x; + static int y; + }; + void bar(); + namespace b { + class Foo{}; + namespace c { + void test(); + } + } + } + using namespace a; + using namespace b; + using namespace c; + + void foo() /*Comment -_-*/ { + a::Bar B; + B.foo(); + a::bar(); + a::Bar>::bar(); + a::Bar::bar(); + B.x = a::Bar::y; + a::Bar::y = 3; + a::bar(); + a::b::c::test(); + } + + + )cpp"); +} + } // namespace } // namespace clangd } // namespace clang