Methods should not be empty(S1186) vs virtual functions

[torgeir.skogen (Torgeir Woldstad Skogen)]

So there’s this rule that flags empty functions. It is a generally useful rule, but I’m wondering if it should get an exception n C++ code for functions that are declared as virtual.

Because we have this sort of Listener class with a bunch of different functions that can be overridden.

class ListenerBase
{
    virtual void onEventA(){}
    virtual void onEventB(){}
    virtual void onEventC(){}
    ...
};

The intention is that some deriving listener will implement a subset of the virtual functions, because not all events are useful for every possible implementation. In this situation it is perfectly reasonable for the base class to have empty functions.

So I’m wondering if it would be reasonable to modify the rule to allow virtual functions to be empty, or to add a new version of this rule that on flags empty functions only if they aren’t virtual.

[Amelie (Amélie Renard)]

Hello @torgeir.skogen ,

In my opinion, the case you’re illustrating corresponds to the third point described in the rule:

• The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Adding a comment in each of this function, explaining for example when an event should be implemented, would prevent the rule from flagging these functions.

Does this solution seem suitable for you?

[torgeir.skogen (Torgeir Woldstad Skogen)]

I’m not entirely sure if we would go down that route instead of just disabling the rule. It just seems like it would potentially be useful to find empty non-virtual functions.

[Amelie (Amélie Renard)]

I understand your point of view, thanks for sharing.
I’ve taken note of this and we’ll see what we can do.

Just an additional note to explain the complexity of the problem: it would not be enough to ignore virtual functions so that no issue is raised on functions that will be overloaded because we’d need to also take static polymorphism into account.