Adds tweak to add declarations for pure virtuals

[robot]

Summary

This tweak searches the current or a selected base class for functions
which are still pure virtual in the current class, and adds function
declarations (overriders) for them.

[robot]

This change is not done, I need some advice:

• I don't understand why there are two levels of data structures in an entry of the final overriders map, hence I can't properly name the variables nor do I fully understand what I'm doing there.
• It's not clear if the tweak should be available if there are no pure virtual functions.
• It's not clear if we need to handle the case that there are multiple final overriders, or what we should do in that case.
• There are some test cases at the end of the file which I don't know how to implement.
• It's not clear if this tweak can apply inside a header.
• It's not clear how to prepare the tweak when the cursor is inside the class body (see tests).

[robot]

Remove left-over file of other patch from CMakeLists.txt

[robot]

Remove left-over test file of other ptch from CMakeLists.txt

[sammccall]

Thanks for working on this!

FWIW I had an old draft of this feature that may have interesting ideas: https://reviews.llvm.org/D122827

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

45

see the long comment on CXXFinalOverriderMap
TL;DR: a derived class can inherit multiple times from the same base, each such subobject could potentially have a separate override

59

copying the tokens directly seems brittle, for example we shouldn't be emitting "virtual" but its's hard to avoid doing so.
(Also, surprising that getEndLoc() doesn't cover the = 0!)

D122827 has a reprint() function that seems to work

76

this probably needs to be restructured somewhat as need to enumerate these in prepare() to be sure there are any, but don't want to pay to produce the strings

87

FWIW, I'm not actually sure that the case of multiple bases classes each providing pure-virtual-methods of which you want to override only one set is common enough to be worth this extra complexity.

140

please use ^^ markers underneath the characters where you expect the tweak to trigger

144

no "virtual"

150

what do you think about listing the functions we'll define?

Especially in the case where there's only one.

154

I don't think this is a quick-fix, which should address a diagnostic or so.

Really there doesn't seem to be a standard kind that fits here. Maybe we should add a new constant "generate"?

157

it's important that we only offer the code action (i.e. prepare returns true) if there are actually methods to override.

This means actually enumerating the functions needs to happen in prepare rather than apply. (Which means it needs to be fast...)

164

this should only trigger on the class definition, not forward declarations

171

this seems confusing: if you're going to walk up looking for a CXXBaseSpecifier, why not just do that in the first place?

197

this doesn't seem like a condition we should be surfacing to the user.
How do we get here? if it's e.g. because the base is dependent, then we should be returning false from prepare() in the appropriate dependent-code cases.

213

inserting at the end of the class isn't the most appropriate place.

we have InsertionPoint.h for this, see usage in D122827

clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp

69

it should be unavailable
(this is important, availability triggers the "lightbulb" icon in vscode and is interpreted by users as "something to do here")

592

EXPECT_UNAVAILABLE, we shouldn't be offering the tweak

[robot]

I'm working on the comments, but it will take a while.

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

59

My original idea was to take the AST node, clone it, remove the pure-specifier, and print that. To me, that seemed like the obvious (object-oriented) way how a tweak would manipulate or generate text.
Alas, I was rather stumped that there didn't seem to be support for this already, and it was rather hard to do. I gave up when default arguments came into play.

I've got the idea of manipulating the tokens from the "extract function" tweak (DefineOutline.cpp / getFunctionSourceCode).

I've tried the reprint function and it does not copy noexcept. That is simple to fix but my guess is there are more issues. The idea of token manipulation was to rather copy too much than too little. I have no intuition how many issues could arise from that :)

When I try to naively remove the virtual I currently print (virtual void foo() override;) in the token manipulation approach by skipping the first tokens, I end up skipping any attributes that precede the virtual as well. So I wrote a test for that, and reprint also doesn't copy attributes at the moment. It is not obvious to me if attributes should be copied, e.g. [[noreturn]].

I have a slight preference for my original approach but it's probably too complicated if this is the only use case. Therefore I'll just implement whatever looks most promising to you.

154

For reference:
rust-analyser shows a "Quick Fix" in VSCode for implementing the functions in a trait.
The go extension shows a "Rewrite" (which is an entirely different action?) in VSCode for filling a structure object (inserting code which explicitly initializes each member with its null value).

[sammccall]

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

59

Yeah, I think extract function copies tokens because there could be anything in the selected code, and there are certain constructs clang doesn't print well, it's unclear what to do in the presence of macros etc. Those concerns don't apply here I think.

My original idea was to take the AST node, clone it, remove the pure-specifier, and print that. To me, that seemed like the obvious (object-oriented) way how a tweak would manipulate or generate text.

Yes, unfortunately this approach doesn't work because the AST isn't designed to be mutated, and carries not just syntactic information but also semantic invariants.
(There was an effort to produce a separate representation of the syntax, but it was never finished)

I've tried the reprint function and it does not copy noexcept

True. It looks like the noexcept is part of the functionprototype, so you could replace the logic that prints the return type + arg list with printType(functype, class, methodname) (from AST.h). That should pick up noexcept etc.

It is not obvious to me if attributes should be copied, e.g. [[noreturn]].

No, I don't think attributes should be copied. They're not required to override, and whether they semantically belong there depends on the particular attribute - we can leave this up to the user.

[robot]

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

59

Yeah, I think extract function copies tokens because there could be anything in the selected code, and there are certain constructs clang doesn't print well, it's unclear what to do in the presence of macros etc. Those concerns don't apply here I think.

Sorry, wrong tweak name (but right file/function). I meant "define outline". It also needs to massage the declaration since it needs to remove those parts which are not allowed in an outlined function definition, like virtual and static.

Anyway, I'll try adjusting reprint() next week.

[robot]

so you could replace the logic that prints the return type + arg list with printType(functype, class, methodname) (from AST.h). That should pick up noexcept etc.

I've tried it and it's not obvious to me how to do it properly: Since printType wants to print the _function type_, it will parenthesize the placeholder (methodname):

void (foo)()

I've tried adding a flag to PrintingPolicy to avoid parenthesizing the name, but I wonder if adjusting a "type printer" to print a function declaration is a clean design. Next problem I ran into is parameters: A type printer does not need to concern itself with parameter names nor default arguments:

void foo(int x = 0) = 0;
void foo(int) override;

Shall I continue to pursue that path? Shall I adjust the type printer to be able to print parameter names and default arguments?

[sammccall]

In D153152#4617391, @robot wrote:

so you could replace the logic that prints the return type + arg list with printType(functype, class, methodname) (from AST.h). That should pick up noexcept etc.

I've tried it and it's not obvious to me how to do it properly: Since printType wants to print the _function type_, it will parenthesize the placeholder (methodname):

void (foo)()

I've tried adding a flag to PrintingPolicy to avoid parenthesizing the name, but I wonder if adjusting a "type printer" to print a function declaration is a clean design. Next problem I ran into is parameters: A type printer does not need to concern itself with parameter names nor default arguments:

void foo(int x = 0) = 0;
void foo(int) override;

Shall I continue to pursue that path? Shall I adjust the type printer to be able to print parameter names and default arguments?

Sorry, it does sound like I sent you down a bad path.
You *could* (no need for an option for parens, here we can detect that the placeholder isValidAsciiIdentifier), but entirely possible more things are broken still.

If you prefer the approach with copying the tokens, adding heuristics to patch them up as needed is maybe no worse than the alternatives.

(BTW, I don't think we want to copy default arguments - it's certainly not necessary, and not helpful in common cases where the function is only called through the base)

[robot]

WIP - several improvements

I thought it might be a good idea to give an update even though this is
still work in progress.

• Don't print virtual for added declarations
• Some rework of the declaration generation
• Add title with preview
• Make tweak only available if it adds anything
• Mostly fix multiple inheritance
• Disable tweak on forward declarations

[robot]

I've pushed some WIP change to inform that I'm still working on this topic :) Not sure if I also need to submit a comment to publish what I've marked as "done" via checkboxes.

clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt

add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
   DefineInline.cpp
   DefineOutline.cpp
   ExpandDeducedType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp
   ExtractVariable.cpp
+  FillStructDesignatedInit.cpp
   MemberwiseConstructor.cpp
   ObjCLocalizeStringLiteral.cpp
   ObjCMemberwiseInitializer.cpp
   PopulateSwitch.cpp
   RawStringLiteral.cpp
   RemoveUsingNamespace.cpp
   SpecialMembers.cpp
   SwapIfBranches.cpp
+  DeclarePureVirtuals.cpp
 
   LINK_LIBS
   clangAST
   clangBasic
   clangDaemon
   clangdSupport
   clangFormat
   clangLex
   clangSema
   clangToolingCore
   clangToolingRefactoring
   clangToolingSyntax
 
   DEPENDS
   omp_gen
   )

clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp

+//===--- DeclarePureVirtuals.cpp ---------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Selection.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContextAllocate.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/AttributeCommonInfo.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Refactoring/ASTSelection.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Determines if the class which provided the final overrider map has a non-pure
+// virtual override of Method.
+bool hasNonPureVirtOverrideFor(const CXXFinalOverriderMap &Map,
+                               const CXXMethodDecl *Method) {
+  auto const It = Map.find(Method);
+  if (It == Map.end()) {
+    return false;
+  }
+
+  for (const auto &Overrider : It->second) {
+    for (const auto &WhatIsThis :
+         Overrider.second) { // TODO: why is there a second level????
+      return !WhatIsThis.Method->isPure();
+    }
+  }
+
+  assert(false && "override was found but overrider list was empty");
+}
+
+// Appends a declaration to To which overrides Method.
+void appendDeclForOverride(std::string &To, const CXXMethodDecl *Method,
+                           const SourceManager &SM,
+                           const syntax::TokenBuffer &TokBuf) {
+  const SourceRange MethodDeclRange{Method->getBeginLoc(), Method->getEndLoc()};
+
+  const llvm::ArrayRef<syntax::Token> Tokens =
+      TokBuf.expandedTokens(MethodDeclRange);
+  const auto EqTok =
+      llvm::find_if(llvm::reverse(Tokens), [](const syntax::Token &Tok) {
+        return Tok.kind() == tok::equal;
+      });
+  assert(EqTok != Tokens.rend());
+
+  To.append(SM.getCharacterData(MethodDeclRange.getBegin()),
+            SM.getCharacterData(EqTok->location()));
+  To.append("override;\n");
+}
+
+// Returns a string with override declarations for all virtual functions in the
+// inheritance hierarchy of Start (including Start itself) which are still pure
+// virtual in Target. Start can be the same class as Target. Target must be the
+// same as or derived from Start.
+std::string collectPureVirtualFuncOverrideDecls(
+    const CXXRecordDecl &Target, const CXXRecordDecl &Start,
+    const SourceManager &SM, const syntax::TokenBuffer &TokBuf) {
+  std::string Additions;
+
+  assert(&Target == &Start || Target.isDerivedFrom(&Start));
+
+  CXXFinalOverriderMap FinalOverriderMapOfStart;
+  Start.getFinalOverriders(FinalOverriderMapOfStart);
+
+  CXXFinalOverriderMap FinalOverriderMapOfTarget;
+  // If &Target == &Start then Target doesn't already have any
+  // overrides for functions that are pure in Start. The map remains empty,
+  // which means hasOverrideFor will return false below, which is OK since the
+  // function is then counted as pure virtual.
+  if (&Target != &Start) {
+    Target.getFinalOverriders(FinalOverriderMapOfTarget);
+  }
+
+  for (const auto &bar : FinalOverriderMapOfStart) { // TODO: see line 44
+    // const CXXMethodDecl *Method = bar.first;
+    const OverridingMethods &Overrides = bar.second;
+
+    for (const std::pair<unsigned int,
+                         llvm::SmallVector<UniqueVirtualMethod, 4>>
+             &meow : // TODO
+         Overrides) {
+      const auto &IdontKnowWhatThisIs = meow.second; // TODO
+      for (const UniqueVirtualMethod &Override : IdontKnowWhatThisIs) {
+        // TODO: conjunction of all overrides? or rely on there's only one?
+        if (Override.Method->isPure() &&
+            !hasNonPureVirtOverrideFor(FinalOverriderMapOfTarget,
+                                       Override.Method)) {
+          appendDeclForOverride(Additions, Override.Method, SM, TokBuf);
+        }
+      }
+    }
+  }
+
+  return Additions;
+}
+
+// Finds the CXXBaseSpecifier in/under the selection, if any.
+const CXXBaseSpecifier *findBaseSpecifier(const SelectionTree::Node *Node) {
+  if (!Node)
+    return nullptr;
+
+  const DynTypedNode &ASTNode = Node->ASTNode;
+  const CXXBaseSpecifier *BaseSpec = ASTNode.get<CXXBaseSpecifier>();
+  if (BaseSpec)
+    return BaseSpec;
+
+  const SelectionTree::Node *const Parent = Node->Parent;
+  if (Parent)
+    return Parent->ASTNode.get<CXXBaseSpecifier>();
+
+  return nullptr;
+}
+
+/// Declares overrides for all pure virtual function in a class hierarchy,
+/// starting with a base class specifier.
+///
+/// Before:
+///   class Base    { virtual void foo() = 0; };
+///   class Derived {};
+///
+/// After:
+///   class Base    { virtual void foo() = 0; };
+///   class Derived { virtual void foo() override; };
+class DeclarePureVirtuals : public Tweak {
+public:
+  const char *id() const override;
+
+  std::string title() const override {
+    return "Override pure virtual functions";
+  }
+
+  llvm::StringLiteral kind() const override {
+    return CodeAction::QUICKFIX_KIND;
+  }
+
+  bool prepare(const Selection &Sel) override {
+    const SelectionTree::Node *const CommonAncestor =
+        Sel.ASTSelection.commonAncestor();
+    if (!CommonAncestor)
+      return false;
+
+    // maybe selected a class, in which case override functions of all bases
+    SelectedDerivedClass = CommonAncestor->ASTNode.get<CXXRecordDecl>();
+    if (SelectedDerivedClass) {
+      return !SelectedDerivedClass->bases().empty();
+    }
+
+    // maybe selected a base class specifier, in which case only override those
+    // bases's functions
+    const DeclContext &DC = CommonAncestor->getDeclContext();
+    SelectedDerivedClass = dyn_cast<CXXRecordDecl>(&DC);
+    if (SelectedDerivedClass) {
+      SelectedBaseSpecifier =
+          findBaseSpecifier(Sel.ASTSelection.commonAncestor());
+      return true;
+    }
+
+    return false;
+  }
+
+  const CXXRecordDecl *SelectedDerivedClass = nullptr;
+  const CXXBaseSpecifier *SelectedBaseSpecifier = nullptr;
+
+  Expected<Effect> apply(const Selection &Sel) override {
+    assert(SelectedDerivedClass); // prepare must have been called and returned
+                                  // true
+
+    std::string Additions;
+    SourceManager &SM = Sel.AST->getSourceManager();
+
+    if (SelectedBaseSpecifier) {
+      // TODO: can getType return null?
+      auto const *Start =
+          SelectedBaseSpecifier->getType()->getAsCXXRecordDecl();
+      if (!Start) {
+        return llvm::createStringError(
+            llvm::inconvertibleErrorCode(),
+            "selected base class specifier does not refer to a C++ class");
+      }
+      Additions = collectPureVirtualFuncOverrideDecls(
+          *SelectedDerivedClass, *Start, SM, Sel.AST->getTokens());
+    } else {
+      Additions += collectPureVirtualFuncOverrideDecls(
+          *SelectedDerivedClass, *SelectedDerivedClass, SM,
+          Sel.AST->getTokens());
+    }
+
+    // TODO: can we apply this tweak in a header if we apply the effect to the
+    // "main file"?
+    return Effect::mainFileEdit(
+        SM, tooling::Replacements(tooling::Replacement(
+                SM, SelectedDerivedClass->getEndLoc(), 0, Additions)));
+  }
+};
+
+REGISTER_TWEAK(DeclarePureVirtuals)
+
+} // namespace
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/CMakeLists.txt

add_unittest(ClangdUnitTests ClangdTests
   support/TestTracer.cpp
   support/ThreadingTests.cpp
   support/TraceTests.cpp
 
   tweaks/AddUsingTests.cpp
   tweaks/AnnotateHighlightingsTests.cpp
   tweaks/DefineInlineTests.cpp
   tweaks/DefineOutlineTests.cpp
+  tweaks/DeclarePureVirtualsTests.cpp
   tweaks/DumpASTTests.cpp
   tweaks/DumpRecordLayoutTests.cpp
   tweaks/DumpSymbolTests.cpp
   tweaks/ExpandDeducedTypeTests.cpp
   tweaks/ExpandMacroTests.cpp
   tweaks/ExtractFunctionTests.cpp
   tweaks/ExtractVariableTests.cpp
+  tweaks/FillStructDesignatedInitTests.cpp
   tweaks/MemberwiseConstructorTests.cpp
   tweaks/ObjCLocalizeStringLiteralTests.cpp
   tweaks/ObjCMemberwiseInitializerTests.cpp
   tweaks/PopulateSwitchTests.cpp
   tweaks/RawStringLiteralTests.cpp
   tweaks/RemoveUsingNamespaceTests.cpp
   tweaks/ShowSelectionTreeTests.cpp
   tweaks/SpecialMembersTests.cpp

clang-tools-extra/clangd/unittests/tweaks/DeclarePureVirtualsTests.cpp

+//===-- DeclarePureVirtualsTests.cpp ----------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestTU.h"
+#include "TweakTesting.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TWEAK_TEST(DeclarePureVirtuals);
+
+TEST_F(DeclarePureVirtualsTest, AvailabilityTriggerOnBaseSpecifier) {
+  EXPECT_AVAILABLE(R"cpp(
+    class MyBase {
+      virtual void myFunction() = 0;
+    };
+
+    class MyDerived : ^p^u^b^l^i^c^ ^M^y^B^a^s^e {
+    };
+
+    class MyDerived2 : ^M^y^B^a^s^e {
+    };
+  )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, AvailabilityTriggerOnClass) {
+  EXPECT_AVAILABLE(R"cpp(
+    class MyBase {
+      virtual void myFunction() = 0;
+    };
+
+    class ^M^y^D^e^r^i^v^e^d: public MyBase {^
+    // but not here, see AvailabilityTriggerInsideClass
+    ^};
+  )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, AvailabilityTriggerInsideClass) {
+  // TODO: this should actually be available but I don't know how to implement
+  // it: the common node of the selection returns the TU, so I get no
+  // information about which class we're in.
+  EXPECT_UNAVAILABLE(R"cpp(
+    class MyBase {
+      virtual void myFunction() = 0;
+    };
+
+    class MyDerived : public MyBase {
+    ^
+    };
+  )cpp");
+}
+
+TEST_F(DeclarePureVirtualsTest, UnavailabilityNoBases) {
+  EXPECT_UNAVAILABLE(R"cpp(
+    class ^N^o^D^e^r^i^v^e^d^ ^{^
+    ^};
+  )cpp");
+}
+
+// TODO: should the tweak available if there are no pure virtual functions and
+// do nothing? or should it be unavailable?
+
+TEST_F(DeclarePureVirtualsTest, SinglePureVirtualFunction) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction() = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction() = 0;
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, MultipleInheritanceFirstClass) {
+  const char *Test = R"cpp(
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : pub^lic MyBase1, public MyBase2 {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase1, public MyBase2 {
+virtual void myFunction1() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, MultipleInheritanceSecondClass) {
+  const char *Test = R"cpp(
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase1, pub^lic MyBase2 {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class MyBase2 {
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase1, public MyBase2 {
+virtual void myFunction2() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, SingleInheritanceMultiplePureVirtualFunctions) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction1() override;
+virtual void myFunction2() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, SingleInheritanceMixedVirtualFunctions) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+  virtual void myFunction2();
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+  virtual void myFunction2();
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction1() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest,
+       TwoLevelsInheritanceOnePureVirtualFunctionInTopBase) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+};
+
+class MyDerived : pub^lic MyIntermediate {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+};
+
+class MyDerived : public MyIntermediate {
+virtual void myFunction1() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest,
+       TwoLevelsInheritancePureVirtualFunctionsInBothBases) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : pub^lic MyIntermediate {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+  virtual void myFunction2() = 0;
+};
+
+class MyDerived : public MyIntermediate {
+virtual void myFunction1() override;
+virtual void myFunction2() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest,
+       TwoLevelInheritanceFunctionNoLongerPureVirtual) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+  virtual void myFunction1() override {}
+};
+
+class MyDerived : pub^lic MyIntermediate {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+class MyIntermediate : public MyBase {
+  virtual void myFunction1() override {}
+};
+
+class MyDerived : public MyIntermediate {
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, VirtualFunctionWithDefaultParameters) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction(int x, int y = 42) = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction(int x, int y = 42) = 0;
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction(int x, int y = 42) override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, FunctionOverloading) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction(int x) = 0;
+  virtual void myFunction(float x) = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction(int x) = 0;
+  virtual void myFunction(float x) = 0;
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction(int x) override;
+virtual void myFunction(float x) override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, OverrideAlreadyExisting) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction() = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+  virtual void myFunction() override;
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction() = 0;
+};
+
+class MyDerived : public MyBase {
+  virtual void myFunction() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, TwoOverloadsOnlyOnePureVirtual) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction(int x) = 0;
+  virtual void myFunction(float x);
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction(int x) = 0;
+  virtual void myFunction(float x);
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction(int x) override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, DerivedClassHasNonOverridingFunction) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction(int x) = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+  void myFunction(float x);
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction(int x) = 0;
+};
+
+class MyDerived : public MyBase {
+  void myFunction(float x);
+virtual void myFunction(int x) override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, PureVirtualFunctionWithNoexcept) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction() noexcept = 0;
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction() noexcept = 0;
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction() noexcept override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, MixOfVirtualAndNonVirtualMemberFunctions) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+    void myFunction2();
+};
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+    void myFunction2();
+};
+
+class MyDerived : public MyBase {
+virtual void myFunction1() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, PureVirtualFunctionWithBody) {
+  const char *Test = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+void MyBase::myFunction1() {
+}
+
+class MyDerived : pub^lic MyBase {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase {
+  virtual void myFunction1() = 0;
+};
+
+void MyBase::myFunction1() {
+}
+
+class MyDerived : public MyBase {
+virtual void myFunction1() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, TraversalOrder) {
+  const char *Test = R"cpp(
+class MyBase0 {
+  virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class MyBase2 : MyBase0, MyBase1 {
+  virtual void myFunction2() = 0;
+};
+
+class MyBase3 {
+  virtual void myFunction3() = 0;
+};
+
+class MyBase4 : MyBase2, MyBase3 {
+  virtual void myFunction4() = 0;
+};
+
+class MyDerived : pub^lic MyBase4 {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase0 {
+  virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class MyBase2 : MyBase0, MyBase1 {
+  virtual void myFunction2() = 0;
+};
+
+class MyBase3 {
+  virtual void myFunction3() = 0;
+};
+
+class MyBase4 : MyBase2, MyBase3 {
+  virtual void myFunction4() = 0;
+};
+
+class MyDerived : public MyBase4 {
+virtual void myFunction0() override;
+virtual void myFunction1() override;
+virtual void myFunction2() override;
+virtual void myFunction3() override;
+virtual void myFunction4() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DeclarePureVirtualsTest, AllBaseClassSpecifiers) {
+  const char *Test = R"cpp(
+class MyBase0 {
+  virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class My^Derived : MyBase0, MyBase1 {
+};
+  )cpp";
+
+  const char *Expected = R"cpp(
+class MyBase0 {
+  virtual void myFunction0() = 0;
+};
+
+class MyBase1 {
+  virtual void myFunction1() = 0;
+};
+
+class MyDerived : MyBase0, MyBase1 {
+virtual void myFunction0() override;
+virtual void myFunction1() override;
+};
+  )cpp";
+
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+// This test should fail since MyBase is incomplete. No idea how to test that.
+// TEST_F(DeclarePureVirtualsTest, IncompleteBassClass) {
+//   const char *Test = R"cpp(
+// class MyBase;
+//
+// class MyDerived : My^Base {
+// };
+//   )cpp";
+//
+//   const char *Expected = R"cpp(
+// class MyBase;
+//
+// class MyDerived : MyBase {
+// };
+//   )cpp";
+//
+//   EXPECT_EQ(apply(Test), Expected);
+// }
+
+// This test should fail since MyBase is incomplete. No idea how to test that.
+// This is a different failure mode since it probably fails to apply but not to
+// prepare.
+// TEST_F(DeclarePureVirtualsTest, IncompleteBassClass) {
+//   const char *Test = R"cpp(
+// class MyBase;
+//
+// class My^Derived : MyBase {
+// };
+//   )cpp";
+//
+//   const char *Expected = R"cpp(
+// class MyBase;
+//
+// class MyDerived : MyBase {
+// };
+//   )cpp";
+//
+//   EXPECT_EQ(apply(Test), Expected);
+// }
+
+// This test should fail since MyBase is unknown. No idea how to test that.
+// TEST_F(DeclarePureVirtualsTest, UnknownBaseSpecifier) {
+//   const char *Test = R"cpp(
+// class MyDerived : My^Base {
+// };
+//   )cpp";
+//
+//   const char *Expected = R"cpp(
+// class MyDerived : MyBase {
+// };
+//   )cpp";
+//
+//   EXPECT_EQ(apply(Test), Expected);
+// }
+
+// This test should fail since MyBase is unknown. No idea how to test that.
+// It's a different failure mode since it will probably fail to apply but not to
+// prepare.
+// TEST_F(DeclarePureVirtualsTest, UnknownBaseSpecifier) {
+//   const char *Test = R"cpp(
+// class My^Derived : MyBase {
+// };
+//   )cpp";
+//
+//   const char *Expected = R"cpp(
+// class MyDerived : MyBase {
+// };
+//   )cpp";
+//
+//   EXPECT_EQ(apply(Test), Expected);
+// }
+
+} // namespace
+} // namespace clangd
+} // namespace clang