I am getting a cpp:S3471: “override” or “final” should be used instead of “virtual” flag raised where the whole class is defined as final but one of the methods (specifically in this case the destructor) isn’t defined explicitly so. We are running SonarQube Developer Edition Version 9.9.1.
class A {
public:
virtual ~A() = default;
}
class B final : public A {
public:
~B() = default;
}
Let’s first consider a situation when the usual method (not a destructor) is overridden in the derived class that is marked as final:
class Base {
public:
virtual void method(std::string const& s);
};
class Derived final : public Base {
public:
void method(std::string const& s); // Override
};
As mentioned in the core guideline, the Derived::method is already final and cannot be overridden, and indeed, from that perspective, the final specifier is redundant.
However, the override and final have additional meaning, they document that this method should override the virtual method from the Base class in a way that is understandable for the programmer and the compiler. To illustrate, imagine that someone decides to change the parameter from std::string const& to std::string_view in the Derived class:
class DerivedMistake final : public Base {
public:
void method(std::string_view s); // No longer overrides the method in base
};
Such code will compile without any issues (we will raise issue S1242). In contrast if override or final was put on the method declaration, the compiler will emit an error (see Compiler Explorer).
However, this does not apply to the destructor, which does not accept parameters or return a type, so marking it as override or final does not add much value. I have created a ticket CPP-5383 to cover this case.
After your explanation the intend of the exception is clearer though from my understanding this is not reflected in the rule description/name (neither in C.128) which does not state anything about explicitly marking the methods. Your example would be related also to hiding a function of a parent class which while I totally agree it’s error prone it seems like a separate issue.
In any case if this is an intended usecase of the rule it would be nice to have the example in the description/text
