[clangd] Define out-of-line availability checks

Summary:
Initial availability checks for performing define out-of-line code
action, which is a refactoring that will help users move function/method
definitions from headers to implementation files.

Proposed implementation only checks whether we have an interesting selection,
namely function name or full function definition/body.

Reviewers: hokein

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69266

clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt

@@ -15,6 +15,7 @@ add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
   DefineInline.cpp
+  DefineOutline.cpp
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp

clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp

@@ -0,0 +1,109 @@
+//===--- 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 "SourceCode.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 (!isHeaderFile(Sel.AST.getSourceManager().getFilename(Sel.Cursor),
+                      Sel.AST.getASTContext().getLangOpts()))
+      return false;
+
+    Source = getSelectedFunction(Sel.ASTSelection.commonAncestor());
+    // Bail out if the selection is not a in-line function definition.
+    if (!Source || !Source->doesThisDeclarationHaveABody() ||
+        Source->isOutOfLine())
+      return false;
+
+    // Bail out in templated classes, as it is hard to spell the class name, i.e
+    // if the template parameter is unnamed.
+    if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source)) {
+      if (MD->getParent()->isTemplated())
+        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

clang-tools-extra/clangd/unittests/TweakTesting.cpp

@@ -63,12 +63,14 @@ std::pair<unsigned, unsigned> rangeOrPoint(const Annotations &A) {
           cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P6(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
+MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, 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.ExtraArgs = ExtraArgs;
@@ -91,6 +93,7 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
   auto Selection = rangeOrPoint(Input);
 
   TestTU TU;
+  TU.Filename = FileName;
   TU.HeaderCode = Header;
   TU.AdditionalFiles = std::move(ExtraFiles);
   TU.Code = Input.code();
@@ -132,7 +135,7 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode,
 
 ::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const {
   return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraArgs,
-                          ExtraFiles, Index.get());
+                          ExtraFiles, Index.get(), FileName);
 }
 
 std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) {

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 @@ protected:
   // testcases.
   std::string Header;
 
+  llvm::StringRef FileName = "TestTU.cpp";
+
   // Extra flags passed to the compilation in apply().
   std::vector<const char *> ExtraArgs;
 

clang-tools-extra/clangd/unittests/TweakTests.cpp

@@ -1807,6 +1807,67 @@ TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
   EXPECT_EQ(apply(Test), Expected) << Test;
 }
 
+TWEAK_TEST(DefineOutline);
+TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
+  FileName = "Test.cpp";
+  // 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");
+
+  // Not available for out-of-line methods.
+  EXPECT_UNAVAILABLE(R"cpp(
+    class Bar {
+      void baz();
+    };
+
+    [[void [[Bar::[[b^a^z]]]]() [[{
+      return;
+    }]]]])cpp");
+
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+    class Bar {
+      [[void [[f^o^o]]() [[{ return; }]]]]
+    };
+
+    void foo();
+    [[void [[f^o^o]]() [[{
+      return;
+    }]]]])cpp");
+
+  // Not available on defaulted/deleted members.
+  EXPECT_UNAVAILABLE(R"cpp(
+    class Foo {
+      Fo^o() = default;
+      F^oo(const Foo&) = delete;
+    };)cpp");
+
+  // Not available within templated classes, as it is hard to spell class name
+  // out-of-line in such cases.
+  EXPECT_UNAVAILABLE(R"cpp(
+    template <typename> struct Foo { void fo^o(){} };
+    })cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang