kadircet updated this revision to Diff 226045. kadircet marked 8 inline comments as done. kadircet added a comment.
- Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69266/new/ https://reviews.llvm.org/D69266 Files: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt clang-tools-extra/clangd/refactor/tweaks/DefineOutline.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 @@ -1519,6 +1519,49 @@ using namespace a; )cpp"); } + +TWEAK_TEST(DefineOutline); +TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) { + // Not available unless in a header file. + EXPECT_UNAVAILABLE(R"cpp( + [[void [[f^o^o]]() [[{ + return; + }]]]])cpp"); + + FileName = "Test.hpp"; + // Not available unless function name or fully body is selected. + EXPECT_UNAVAILABLE(R"cpp( + // Not a definition + vo^i[[d^ ^f]]^oo(); + + [[vo^id ]]foo[[()]] {[[ + [[(void)(5+3); + return;]] + }]])cpp"); + + // Available even if there are no implementation files. + EXPECT_AVAILABLE(R"cpp( + [[void [[f^o^o]]() [[{ + return; + }]]]])cpp"); + + ExtraFiles["Test.cpp"] = ""; + // Basic check for function body and signature. + EXPECT_AVAILABLE(R"cpp( + class Bar { + void baz(); + [[void [[f^o^o]]() [[{ return; }]]]] + }; + + [[void [[Bar::[[b^a^z]]]]() [[{ + return; + }]]]] + + [[void [[f^o^o]]() [[{ + return; + }]]]])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 @@ -12,6 +12,7 @@ #include "TestTU.h" #include "index/Index.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <memory> @@ -62,6 +63,8 @@ // testcases. std::string Header; + llvm::StringRef FileName = "TestTU.cpp"; + // Extra flags passed to the compilation in apply(). std::vector<const char *> ExtraArgs; Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp =================================================================== --- clang-tools-extra/clangd/unittests/TweakTesting.cpp +++ clang-tools-extra/clangd/unittests/TweakTesting.cpp @@ -62,12 +62,13 @@ cantFail(positionToOffset(A.code(), SelectionRng.end))}; } -MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index, +MATCHER_P6(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles, Index, FileName, (TweakID + (negation ? " is unavailable" : " is available")).str()) { std::string WrappedCode = wrap(Ctx, arg); Annotations Input(WrappedCode); auto Selection = rangeOrPoint(Input); TestTU TU; + TU.Filename = FileName; TU.HeaderCode = Header; TU.Code = Input.code(); TU.AdditionalFiles = std::move(ExtraFiles); @@ -89,6 +90,7 @@ auto Selection = rangeOrPoint(Input); TestTU TU; + TU.Filename = FileName; TU.HeaderCode = Header; TU.AdditionalFiles = std::move(ExtraFiles); TU.Code = Input.code(); @@ -125,7 +127,7 @@ ::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const { return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraFiles, - Index.get()); + Index.get(), FileName); } std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) { Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp @@ -0,0 +1,99 @@ +//===--- DefineOutline.cpp ---------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "HeaderSourceSwitch.h" +#include "Path.h" +#include "Selection.h" +#include "refactor/Tweak.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Decl.h" +#include "clang/AST/Stmt.h" +#include "llvm/ADT/Optional.h" +#include "llvm/Support/Path.h" + +namespace clang { +namespace clangd { +namespace { + +// 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. +// FIXME: This is shared with define inline, move them to a common header once +// we have a place for such. +const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) { + if (!SelNode) + return nullptr; + const ast_type_traits::DynTypedNode &AstNode = SelNode->ASTNode; + if (const FunctionDecl *FD = AstNode.get<FunctionDecl>()) + return FD; + if (AstNode.get<CompoundStmt>() && + SelNode->Selected == SelectionTree::Complete) { + if (const SelectionTree::Node *P = SelNode->Parent) + return P->ASTNode.get<FunctionDecl>(); + } + return nullptr; +} + +/// Moves definition of a function/method to an appropriate implementation file. +/// +/// Before: +/// a.h +/// void foo() { return; } +/// a.cc +/// #include "a.h" +/// +/// ---------------- +/// +/// After: +/// a.h +/// void foo(); +/// a.cc +/// #include "a.h" +/// void foo() { return; } +class DefineOutline : public Tweak { +public: + const char *id() const override; + + bool hidden() const override { return true; } + Intent intent() const override { return Intent::Refactor; } + std::string title() const override { + return "Move function body to out-of-line."; + } + + bool prepare(const Selection &Sel) override { + // Bail out if we are not in a header file. + // FIXME: We might want to consider moving method definitions below class + // definition even if we are inside a source file. + if (!Sel.AST.getASTContext().getLangOpts().IsHeaderFile) + return false; + + // Bail out if the selection is not a function definition. + Source = getSelectedFunction(Sel.ASTSelection.commonAncestor()); + if (!Source || !Source->isThisDeclarationADefinition()) + return false; + + // Note that we don't check whether an implementation file exists or not in + // the prepare, since performing disk IO on each prepare request might be + // expensive. + return true; + } + + Expected<Effect> apply(const Selection &Sel) override { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Not implemented yet"); + } + +private: + const FunctionDecl *Source = nullptr; +}; + +REGISTER_TWEAK(DefineOutline); + +} // namespace +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -15,6 +15,7 @@ AnnotateHighlightings.cpp DumpAST.cpp DefineInline.cpp + DefineOutline.cpp ExpandAutoType.cpp ExpandMacro.cpp ExtractFunction.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits