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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.
kadircet updated this revision to Diff 230843.
kadircet added a comment.

- Get rid of debug output


When moving function definitions to a different context, the function
name might need a different spelling, for example in the header it might be:

  namespace a {
    void foo() {}
  }

And we might want to move it into a context which doesn't have `namespace a` as
a parent, then we must re-spell the function name, i.e:

  void a::foo() {}

This patch implements a version of this which ignores using namespace
declarations in the source file.


Repository:
  rG LLVM Github Monorepo

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
@@ -1807,7 +1807,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",
@@ -1825,6 +1825,50 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyFunctionName) {
+  FileName = "Test.hpp";
+  struct {
+    llvm::StringRef Test;
+    llvm::StringRef SourceFile;
+    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.SourceFile;
+    EXPECT_EQ(apply(Case.Test, &EditedFiles), Case.ExpectedHeader);
+    EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+                                 testPath("Test.cpp"), Case.ExpectedSource)))
+        << Case.Test;
+  }
+}
 } // 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
@@ -132,8 +132,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) {
@@ -178,8 +179,6 @@
   if (!QualifiedFunc)
     return QualifiedFunc.takeError();
   return QualifiedFunc->substr(FuncBegin, FuncEnd - FuncBegin + 1);
-
-  // FIXME: Qualify function name depending on the target context.
 }
 
 struct InsertionPoint {
@@ -285,9 +284,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.230843.patch
Type: text/x-patch
Size: 3458 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191125/4c24798c/attachment.bin>