[PATCH] D70656: [clangd] Define out-of-line qualify function name

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 231512.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70656/new/

https://reviews.llvm.org/D70656

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  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
@@ -2006,7 +2006,7 @@
             Bar foo() ;
           }
         })cpp",
-       "a::Foo::Bar foo() { return {}; }\n"},
+       "a::Foo::Bar a::Foo::foo() { return {}; }\n"},
       {R"cpp(
         class Foo;
         Foo fo^o() { return; })cpp",
@@ -2024,6 +2024,58 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyFunctionName) {
+  FileName = "Test.hpp";
+  struct {
+    llvm::StringRef TestHeader;
+    llvm::StringRef TestSource;
+    llvm::StringRef ExpectedHeader;
+    llvm::StringRef ExpectedSource;
+  } Cases[] = {
+      {
+          R"cpp(
+            namespace a {
+              namespace b {
+                class Foo {
+                  void fo^o() {}
+                };
+              }
+            })cpp",
+          "",
+          R"cpp(
+            namespace a {
+              namespace b {
+                class Foo {
+                  void foo() ;
+                };
+              }
+            })cpp",
+          "void a::b::Foo::foo() {}\n",
+      },
+      {
+          "namespace a { namespace b { void f^oo() {} } }",
+          "namespace a{}",
+          "namespace a { namespace b { void foo() ; } }",
+          "namespace a{void b::foo() {} }",
+      },
+      {
+          "namespace a { namespace b { void f^oo() {} } }",
+          "using namespace a;",
+          "namespace a { namespace b { void foo() ; } }",
+          // FIXME: Take using namespace directives in the source file into
+          // account. This can be spelled as b::foo instead.
+          "using namespace a;void a::b::foo() {} ",
+      },
+  };
+  llvm::StringMap<std::string> EditedFiles;
+  for (auto &Case : Cases) {
+    ExtraFiles["Test.cpp"] = Case.TestSource;
+    EXPECT_EQ(apply(Case.TestHeader, &EditedFiles), Case.ExpectedHeader);
+    EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+                                 testPath("Test.cpp"), Case.ExpectedSource)))
+        << Case.TestHeader;
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -128,8 +128,9 @@
     // inside a macro body.
     if (Ref.Qualifier || Ref.Targets.empty() || Ref.NameLoc.isMacroID())
       return;
-    // Qualify return type
-    if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin())
+    // Only qualify return type and function name.
+    if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin() &&
+        Ref.NameLoc != FD->getLocation())
       return;
 
     for (const NamedDecl *ND : Ref.Targets) {
@@ -284,9 +285,7 @@
     auto FuncDef =
         getFunctionSourceCode(Source, InsertionPoint->EnclosingNamespace);
     if (!FuncDef)
-      return llvm::createStringError(
-          llvm::inconvertibleErrorCode(),
-          "Couldn't get full source for function definition.");
+      return FuncDef.takeError();
 
     SourceManagerForFile SMFF(*CCFile, Contents);
     const tooling::Replacement InsertFunctionDef(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70656.231512.patch
Type: text/x-patch
Size: 3393 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191129/f5ff6054/attachment.bin>