[clang-tools-extra] 353a988 - [clangd] DefineOutline: removes static token from static CXXMethodDecl

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-04-07T11:57:43+01:00
New Revision: 353a9883680e9d9666d02516a6692e8319af6d66

URL: https://github.com/llvm/llvm-project/commit/353a9883680e9d9666d02516a6692e8319af6d66
DIFF: https://github.com/llvm/llvm-project/commit/353a9883680e9d9666d02516a6692e8319af6d66.diff

LOG: [clangd] DefineOutline: removes static token from static CXXMethodDecl

Summary: Removes the `static` token when defining a function out of line if the function is a `CXXMethodDecl`

Reviewers: sammccall, kadircet, hokein

Reviewed By: kadircet

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

Tags: #clang, #clang-tools-extra

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
    clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index d9e07a001d23..c2d344a3a46e 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -239,21 +239,22 @@ getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
   DelAttr(FD->getAttr<OverrideAttr>());
   DelAttr(FD->getAttr<FinalAttr>());
 
-  if (FD->isVirtualAsWritten()) {
-    SourceRange SpecRange{FD->getBeginLoc(), FD->getLocation()};
-    bool HasErrors = true;
-
-    // Clang allows duplicating virtual specifiers so check for multiple
-    // occurrences.
-    for (const auto &Tok : TokBuf.expandedTokens(SpecRange)) {
-      if (Tok.kind() != tok::kw_virtual)
+  auto DelKeyword = [&](tok::TokenKind Kind, SourceRange FromRange) {
+    bool FoundAny = false;
+    for (const auto &Tok : TokBuf.expandedTokens(FromRange)) {
+      if (Tok.kind() != Kind)
         continue;
+      FoundAny = true;
       auto Spelling = TokBuf.spelledForExpanded(llvm::makeArrayRef(Tok));
       if (!Spelling) {
-        HasErrors = true;
+        Errors = llvm::joinErrors(
+            std::move(Errors),
+            llvm::createStringError(
+                llvm::inconvertibleErrorCode(),
+                llvm::formatv("define outline: couldn't remove `{0}` keyword.",
+                              tok::getKeywordSpelling(Kind))));
         break;
       }
-      HasErrors = false;
       CharSourceRange DelRange =
           syntax::Token::range(SM, Spelling->front(), Spelling->back())
               .toCharRange(SM);
@@ -261,13 +262,22 @@ getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace,
               DeclarationCleanups.add(tooling::Replacement(SM, DelRange, "")))
         Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
     }
-    if (HasErrors) {
+    if (!FoundAny) {
       Errors = llvm::joinErrors(
           std::move(Errors),
-          llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                  "define outline: Can't move out of line as "
-                                  "function has a macro `virtual` specifier."));
+          llvm::createStringError(
+              llvm::inconvertibleErrorCode(),
+              llvm::formatv(
+                  "define outline: couldn't find `{0}` keyword to remove.",
+                  tok::getKeywordSpelling(Kind))));
     }
+  };
+
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+    if (MD->isVirtualAsWritten())
+      DelKeyword(tok::kw_virtual, {FD->getBeginLoc(), FD->getLocation()});
+    if (MD->isStatic())
+      DelKeyword(tok::kw_static, {FD->getBeginLoc(), FD->getLocation()});
   }
 
   if (Errors)

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index e91ff22d3533..979ee44f4474 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2142,6 +2142,28 @@ TEST_F(DefineOutlineTest, ApplyTest) {
             };)cpp",
           "void B::foo()   {}\n",
       },
+      {
+          R"cpp(
+            struct A {
+              static void fo^o() {}
+            };)cpp",
+          R"cpp(
+            struct A {
+              static void foo() ;
+            };)cpp",
+          " void A::foo() {}\n",
+      },
+      {
+          R"cpp(
+            struct A {
+              static static void fo^o() {}
+            };)cpp",
+          R"cpp(
+            struct A {
+              static static void foo() ;
+            };)cpp",
+          "  void A::foo() {}\n",
+      },
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Test);
@@ -2236,6 +2258,24 @@ TEST_F(DefineOutlineTest, HandleMacros) {
             STUPID_MACRO(sizeof sizeof int) void foo() ;
           };)cpp",
        " void A::foo() {}\n"},
+      {R"cpp(#define STAT static
+          struct A {
+            STAT void f^oo() {}
+          };)cpp",
+       R"cpp(#define STAT static
+          struct A {
+            STAT void foo() ;
+          };)cpp",
+       " void A::foo() {}\n"},
+      {R"cpp(#define STUPID_MACRO(X) static
+          struct A {
+            STUPID_MACRO(sizeof sizeof int) void f^oo() {}
+          };)cpp",
+       R"cpp(#define STUPID_MACRO(X) static
+          struct A {
+            STUPID_MACRO(sizeof sizeof int) void foo() ;
+          };)cpp",
+       " void A::foo() {}\n"},
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Test);
@@ -2360,8 +2400,7 @@ TEST_F(DefineOutlineTest, FailsMacroSpecifier) {
           struct A {
             VIRT fo^o() {}
           };)cpp",
-          "fail: define outline: Can't move out of line as function has a "
-          "macro `virtual` specifier."},
+          "fail: define outline: couldn't remove `virtual` keyword."},
       {
           R"cpp(
           #define OVERFINAL final override