kadircet updated this revision to Diff 221913.
kadircet added a comment.
- Mark tweak as visible
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66647/new/
https://reviews.llvm.org/D66647
Files:
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h
clang-tools-extra/clangd/unittests/TweakTests.cpp
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ 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 <typename T> class Bar {};
+ Bar<int> 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 <typename T> class Bar {};
+ Bar<int> z;
+ }
+
+
+ )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplDecls) {
+ EXPECT_EQ(apply(R"cpp(
+ namespace a {
+ template <typename T> class Bar {
+ public:
+ void bar();
+ };
+ template <typename T> T bar;
+ template <typename T> void aux() {}
+ }
+
+ void foo() /*Comment -_-*/ ;
+
+ void f^oo() {
+ using namespace a;
+ bar<Bar<int>>.bar();
+ aux<Bar<int>>();
+ }
+ )cpp"),
+ R"cpp(
+ namespace a {
+ template <typename T> class Bar {
+ public:
+ void bar();
+ };
+ template <typename T> T bar;
+ template <typename T> void aux() {}
+ }
+
+ void foo() /*Comment -_-*/ {
+ using namespace a;
+ a::bar<a::Bar<int>>.bar();
+ a::aux<a::Bar<int>>();
+ }
+
+
+ )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 <typename T> class Bar {};
+ }
+ using namespace a;
+
+ template <typename T>
+ void foo() /*Comment -_-*/ ;
+
+ template <typename T>
+ void f^oo() {
+ Bar<T> B;
+ Bar<Bar<T>> q;
+ }
+ )cpp"),
+ R"cpp(
+ namespace a {
+ template <typename T> class Bar {};
+ }
+ using namespace a;
+
+ template <typename T>
+ void foo() /*Comment -_-*/ {
+ a::Bar<T> B;
+ a::Bar<a::Bar<T>> q;
+ }
+
+
+ )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformFunctionTempls) {
+ EXPECT_EQ(apply(R"cpp(
+ template <typename T>
+ void foo(T p);
+
+ template <>
+ void foo<int>(int p);
+
+ template <>
+ void foo<char>(char p);
+
+ template <>
+ void fo^o<int>(int p) {
+ return;
+ }
+ )cpp"),
+ R"cpp(
+ template <typename T>
+ void foo(T p);
+
+ template <>
+ void foo<int>(int p){
+ return;
+ }
+
+ template <>
+ void foo<char>(char p);
+
+
+ )cpp");
+
+ EXPECT_EQ(apply(R"cpp(
+ template <typename T>
+ void foo(T p);
+
+ template <>
+ void foo<int>(int p);
+
+ template <>
+ void foo<char>(char p);
+
+ template <>
+ void fo^o<char>(char p) {
+ return;
+ }
+ )cpp"),
+ R"cpp(
+ template <typename T>
+ void foo(T p);
+
+ template <>
+ void foo<int>(int p);
+
+ template <>
+ void foo<char>(char p){
+ return;
+ }
+
+
+ )cpp");
+
+ EXPECT_EQ(apply(R"cpp(
+ template <typename T>
+ void foo(T p);
+
+ template <>
+ void foo<int>(int p);
+
+ template <typename T>
+ void fo^o(T p) {
+ return;
+ }
+ )cpp"),
+ R"cpp(
+ template <typename T>
+ void foo(T p){
+ return;
+ }
+
+ template <>
+ void foo<int>(int p);
+
+
+ )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTypeLocs) {
+ EXPECT_EQ(apply(R"cpp(
+ namespace a {
+ template <typename T> class Bar {
+ public:
+ template <typename Q> class Baz {};
+ };
+ namespace b{
+ class Foo{};
+ };
+ }
+ using namespace a;
+ using namespace b;
+ using namespace c;
+
+ void foo() /*Comment -_-*/ ;
+
+ void f^oo() {
+ Bar<int> B;
+ b::Foo foo;
+ a::Bar<Bar<int>>::Baz<Bar<int>> q;
+ }
+ )cpp"),
+ R"cpp(
+ namespace a {
+ template <typename T> class Bar {
+ public:
+ template <typename Q> class Baz {};
+ };
+ namespace b{
+ class Foo{};
+ };
+ }
+ using namespace a;
+ using namespace b;
+ using namespace c;
+
+ void foo() /*Comment -_-*/ {
+ a::Bar<int> B;
+ a::b::Foo foo;
+ a::Bar<a::Bar<int>>::Baz<a::Bar<int>> q;
+ }
+
+
+ )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformDeclRefs) {
+ EXPECT_EQ(apply(R"cpp(
+ namespace a {
+ template <typename T> 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<int> B;
+ B.foo();
+ a::bar();
+ Bar<Bar<int>>::bar();
+ a::Bar<int>::bar();
+ B.x = Bar<int>::y;
+ Bar<int>::y = 3;
+ bar();
+ c::test();
+ }
+ )cpp"),
+ R"cpp(
+ namespace a {
+ template <typename T> 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<int> B;
+ B.foo();
+ a::bar();
+ a::Bar<a::Bar<int>>::bar();
+ a::Bar<int>::bar();
+ B.x = a::Bar<int>::y;
+ a::Bar<int>::y = 3;
+ a::bar();
+ a::b::c::test();
+ }
+
+
+ )cpp");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.h
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -51,6 +51,7 @@
// Mapping from file name to contents.
llvm::StringMap<std::string> ExtraFiles;
+ llvm::StringMap<std::string> 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<message>"
// - if prepare() returns false, returns "unavailable"
// - if apply() returns an error, returns "fail: <message>"
- 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.
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ 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 <string>
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<llvm::StringRef> TweakTest::isAvailable() const {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ 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<SourceLocation> 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<std::string> 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<Effect> 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:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
