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

Reply via email to