[clangd] Preserve Qualified Names in Override pure virtual methods tweak

[marcogmaia]

Prevents the tweak from splitting qualified names (e.g., foo::Type) by incorrectly inserting a space around the scope resolution (::).

Before:

// input:
virtual foo::Type::func() = 0 
// output:
foo :: Type :: func()

After:

// input:
virtual foo::Type::func() = 0 
// output:
foo::Type::func()

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clangd

Author: Marco Maia (marcogmaia)

Changes

Prevents the tweak from splitting qualified names (e.g., foo::Type) by incorrectly inserting a space around the scope resolution (::).

Before:

// input:
virtual foo::Type::func() = 0 
// output:
foo :: Type :: func()

After:

// input:
virtual foo::Type::func() = 0 
// output:
foo::Type::func()

---

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

2 Files Affected:

• (modified) clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp (+3-2)
• (modified) clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp (+39)

diff --git a/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
index 16febeca70809..b557066d979f5 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/OverridePureVirtuals.cpp
@@ -79,7 +79,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
-#include "clang/AST/Type.h"
+#include "clang/AST/TypeBase.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
@@ -116,7 +116,8 @@ std::string removePureVirtualSyntax(const std::string &MethodDecl,
 
     DeclString += Tk.text();
     if (Tk.Kind != tok::l_paren && Next.Kind != tok::comma &&
-        Next.Kind != tok::r_paren && Next.Kind != tok::l_paren)
+        Next.Kind != tok::r_paren && Next.Kind != tok::l_paren &&
+        Tk.Kind != tok::coloncolon && Next.Kind != tok::coloncolon)
       DeclString += ' ';
   }
   // Trim the last whitespace.
diff --git a/clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp
index b7dcbee1650ec..72095ab2f5982 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/OverridePureVirtualsTests.cpp
@@ -715,6 +715,45 @@ class D : public B {
   EXPECT_EQ(Expected, Applied) << "Applied result:\n" << Applied;
 }
 
+TEST_F(OverridePureVirtualsTests, QualifiedNames) {
+  constexpr auto Before = R"cpp(
+namespace foo { struct S{}; namespace bar { struct S2{}; } }
+
+class B {
+public:
+  virtual foo::S foo(int var = 0) = 0;
+  virtual foo::bar::S2 bar(int var = 0) = 0;
+};
+
+class ^D : public B {};
+)cpp";
+
+  constexpr auto Expected = R"cpp(
+namespace foo { struct S{}; namespace bar { struct S2{}; } }
+
+class B {
+public:
+  virtual foo::S foo(int var = 0) = 0;
+  virtual foo::bar::S2 bar(int var = 0) = 0;
+};
+
+class D : public B {
+public:
+  foo::S foo(int var = 0) override {
+    // TODO: Implement this pure virtual method.
+    static_assert(false, "Method `foo` is not implemented.");
+  }
+
+  foo::bar::S2 bar(int var = 0) override {
+    // TODO: Implement this pure virtual method.
+    static_assert(false, "Method `bar` is not implemented.");
+  }
+};
+)cpp";
+  auto Applied = apply(Before);
+  EXPECT_EQ(Expected, Applied) << "Applied result:\n" << Applied;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

[marcogmaia]

@kadircet, could you please take a look?

[marcogmaia]

Ping.

[kadircet]

thanks, LGTM!

FWIW we normally run clang-format on these edits, looks like clang-format is mishandling void foo :: bar() {} and formatting that to void foo ::bar() {}. it might be worthwhile to file a bug (if there isn't one already)

[marcogmaia]

Thanks @kadircet, this bug is mentioned in #150858.