C++ Cognitive complexity rule counts macro contents

SonarQube Server / Community Build
1

Env:

  • SonarQube 8.9.6
  • CFamily 6.20.2.38358

Our C++ codebase uses logging macros similar to this:

#define LOG(level)                                         \
    if (!::logging::levelEnabled(level)) {}                \
    else ::logging::LogLineHelper(__FILE__, __LINE__, level)
    

LOG(INFO) << "Foo" << 42;

The advantage of this is the parameters of the log are not evaluated at all, if the level is not enabled.

Unfortunately the Cognitive Complexity rule (cpp:S3776) treats all LOG invocations as an ‘if’ statement and gives 1-4 points depending on the nesting. The rule triggers in practically every method that’s not trivial.

Is it possible to configure the rule or annotate the logging macro to be considered as a simple statement and not increase cognitive complexity?

4

Hi @hoditohod and welcome!

Unfortunately, our implementation of cpp:S3776 does not support configuration other than the threshold value.

The rule does treat macros as opaque function calls for the macros that substitute for a complete syntactic construct. For example, #define ASSERT(x) if (x) {} else exit(1);. These macros reduce complexity by hiding the mechanics behind an abstraction.

However, the situation is not as clear for macros that substitute for partial syntactic constructs. For example:

#define IF_BETWEEN(a, x, b) if (a <= x && x <= b)
...
if (x % 2 == 0)
  IF_BETWEEN(0, 10, x) do_a();
else
  do_b();

Here it is not obvious that the else clause is related to the hidden if in IF_BETWEEN than to the outermost if. Another example:

#define FOR_EACH_FLATTEN(iter, list_of_lists) \
  for (auto unique_identifier_iter = list_of_lists.begin(); unique_identifier_iter = list_of_lists.end(); ++unique_identifier_iter) \
    for (auto iter = unique_identifier_iter.begin(); iter != unique_identifier_iter.end(); ++iter)

// (1)
vector<vector<string>> channels = ...
FOR_EACH_FLATTEN(msg, channels) print(msg);

// (2)
FOR_EACH_FLATTEN(msg, channels) {
  if (msg == "quit") break; // This is not what you think it is
  print(msg);
}

Here the FOR_EACH_FLATTEN raises the abstraction of the code, and helps readability in the first use (1), but the same macro leads to surprising behavior in the second use (2).

Your LOG macro looks benign and, indeed, helps readability. I cannot think of a situation where it would bring about a surprise.

In the end, it might be worth changing the criteria for cognitive complexity w.r.t. macros that expand to an incomplete syntactic construct. I will discuss it internally and give an update in this thread.

In the meantime, if you are controlling the definition of LOG, you can change it to be a complete syntactic construct, for example:

#define LOG(level, message)                                \
    if (!::logging::levelEnabled(level)) {}                \
    else ::logging::LogLineHelper(__FILE__, __LINE__, level) << message;

LOG(INFO, "Foo" << 42);

Admittedly, it does not look as nice.

6

Thank you for the detailed reply. Changing the LOG macro isn’t really viable, as the proposed solution won’t work with some additional functionality I’ve not included in the simplified version above.

For the time being (as a workaround) we’ll probably tweak the threshold in the rule to trigger only for methods that are indeed considered too complex by developers.

7

After a discussion, we decided that cognitive complexity should ignore macro invocations, and plan to adjust the rule accordingly: CPP-3743.
Thank you for the report!