[clang-tools-extra] [clangd] Let DefineOutline tweak create a definition from scratch (PR #71950)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: None (ckandeler)

<details>
<summary>Changes</summary>



---
Full diff: https://github.com/llvm/llvm-project/pull/71950.diff


2 Files Affected:

- (modified) clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp (+50-11) 
- (modified) clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp (+26-6) 


``````````diff
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index b84ae04072f2c19..9e715b7bf21175a 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -128,7 +128,16 @@ getFunctionSourceAfterReplacements(const FunctionDecl *FD,
       SM.getBufferData(SM.getMainFileID()), Replacements);
   if (!QualifiedFunc)
     return QualifiedFunc.takeError();
-  return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+  auto QualifiedFuncString =
+      QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
+  if (!FD->hasBody() && !FD->isExplicitlyDefaulted()) {
+    QualifiedFuncString.pop_back(); // The semicolon finishing the declaration.
+    QualifiedFuncString += " {";
+    if (!FD->getReturnType()->isVoidType())
+      QualifiedFuncString += " return {}; ";
+    QualifiedFuncString += "}";
+  }
+  return QualifiedFuncString;
 }
 
 // Returns replacements to delete tokens with kind `Kind` in the range
@@ -348,6 +357,30 @@ llvm::Expected<InsertionPoint> getInsertionPoint(llvm::StringRef Contents,
 // initializers.
 SourceRange getDeletionRange(const FunctionDecl *FD,
                              const syntax::TokenBuffer &TokBuf) {
+  if (!FD->hasBody()) {
+    if (!FD->isExplicitlyDefaulted())
+      return {};
+
+    auto tokens = TokBuf.expandedTokens(FD->getSourceRange());
+    for (auto It = std::rbegin(tokens); It != std::rend(tokens); ++It) {
+      if (It->kind() == tok::kw_default) {
+        const auto nextIt = std::next(It);
+        if (nextIt == std::rend(tokens))
+          return {};
+        const auto &ExpandedEquals = *nextIt;
+        if (ExpandedEquals.kind() != tok::equal)
+          return {};
+        auto SpelledEquals =
+            TokBuf.spelledForExpanded(llvm::ArrayRef(ExpandedEquals));
+        if (!SpelledEquals)
+          return {};
+        return {SpelledEquals->front().location(),
+                FD->getDefaultLoc().getLocWithOffset(1)};
+      }
+    }
+    return {};
+  }
+
   auto DeletionRange = FD->getBody()->getSourceRange();
   if (auto *CD = llvm::dyn_cast<CXXConstructorDecl>(FD)) {
     // AST doesn't contain the location for ":" in ctor initializers. Therefore
@@ -405,7 +438,9 @@ class DefineOutline : public Tweak {
     return CodeAction::REFACTOR_KIND;
   }
   std::string title() const override {
-    return "Move function body to out-of-line";
+    if (Source->doesThisDeclarationHaveABody())
+      return "Move function body to out-of-line";
+    return "Create function body out-of-line";
   }
 
   bool prepare(const Selection &Sel) override {
@@ -417,9 +452,8 @@ class DefineOutline : public Tweak {
       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())
+    // Bail out if the selection is not a function declaration.
+    if (!Source || Source->isDeleted() || Source->isOutOfLine())
       return false;
 
     // Bail out if this is a function template or specialization, as their
@@ -483,12 +517,17 @@ class DefineOutline : public Tweak {
     if (!Effect)
       return Effect.takeError();
 
-    tooling::Replacements HeaderUpdates(tooling::Replacement(
-        Sel.AST->getSourceManager(),
-        CharSourceRange::getTokenRange(*toHalfOpenFileRange(
-            SM, Sel.AST->getLangOpts(),
-            getDeletionRange(Source, Sel.AST->getTokens()))),
-        ";"));
+    tooling::Replacements HeaderUpdates;
+    auto DeletionRange = getDeletionRange(Source, Sel.AST->getTokens());
+    if (DeletionRange.isValid()) {
+      if (auto Error = HeaderUpdates.add(tooling::Replacement(
+              Sel.AST->getSourceManager(),
+              CharSourceRange::getTokenRange(*toHalfOpenFileRange(
+                  SM, Sel.AST->getLangOpts(), DeletionRange)),
+              ";"))) {
+        return Error;
+      }
+    }
 
     if (Source->isInlineSpecified()) {
       auto DelInline =
diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
index d1e60b070f20e95..dede5f7c3c25e8b 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -28,9 +28,6 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
   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;]]
@@ -63,10 +60,9 @@ TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
       return;
     }]]]])cpp");
 
-  // Not available on defaulted/deleted members.
+  // Not available on deleted members.
   EXPECT_UNAVAILABLE(R"cpp(
     class Foo {
-      Fo^o() = default;
       F^oo(const Foo&) = delete;
     };)cpp");
 
@@ -128,12 +124,24 @@ TEST_F(DefineOutlineTest, ApplyTest) {
     llvm::StringRef ExpectedHeader;
     llvm::StringRef ExpectedSource;
   } Cases[] = {
-      // Simple check
+      // Simple check: Move
       {
           "void fo^o() { return; }",
           "void foo() ;",
           "void foo() { return; }",
       },
+      // Simple check: Create
+      {
+          "void fo^o(int i = 5);",
+          "void foo(int i = 5);",
+          "void foo(int i ) {}",
+      },
+      // Simple check: Create with return value
+      {
+          "int fo^o();",
+          "int foo();",
+          "int foo() { return {}; }",
+      },
       // Inline specifier.
       {
           "inline void fo^o() { return; }",
@@ -205,6 +213,18 @@ TEST_F(DefineOutlineTest, ApplyTest) {
               };)cpp",
           "Foo::Foo(int z) __attribute__((weak)) : bar(2){}\n",
       },
+      // Default ctor
+      {
+          R"cpp(
+              class Foo {
+                F^oo() = default;
+              };)cpp",
+          R"cpp(
+              class Foo {
+                Foo() ;
+              };)cpp",
+          "Foo::Foo() = default;",
+      },
       // Virt specifiers.
       {
           R"cpp(

``````````

</details>


https://github.com/llvm/llvm-project/pull/71950