exclude macro expansions from complexity metrics/checks

[ivangalkin]

• partial solution for Complexity metric / checks must be performed on original source code #1547

MOTIVATION:

• US: Add explanation for cognitive/cyclomatic complexity checks #1494 made visible the fact, that preprocessor affects the complexity
  metric. E.g. if function uses macros, the underlying (hidden) complexity
  of theses macros increases the complexity of the function. This is wrong
  because of many reasons:

1. Cognitive complexity:
   Macros do not increase the cognitive complexity of the source code. To
   the reader they are similar to the function invocations.
   Example ASSERT_XX/EXPECT_XX macros of GTest.

2. Cyclomatic complexity:
   Similar logic might be applied to the cyclomatic complexity: macro expansions
   are similar to the function calls (which become inlined). In this case
   the cyclomatic complexity of the caller should not be increased.
   [Cyclomatic] complexity of the callee must be calculated separately.

3. Preprocessor doesn't distinguish between the macros, which
   were declared inside of the analyzed project or were included from the
   external header. Obviously the external source code (e.g. again very
   complex GTest macros) should not affect the complexity of someone's
   custom SonarQube project.

SOLUTION:

In general it's impossible to build a valid C/C++ AST without
performing of preprocessor step. The original source code might
be syntactically incorrect (e.g. if macros are used to "invent" some
domain specific language out of C/C++). All metrics of sonar-cxx
are however AST-based. (This is a very questionably decision
e.g. because #if/#ifdef etc might affect the resulting complexity or [NC]LOC
in dramatic way). So we cannot skip preprocessor step entirely and/or
perform metrics calculation/checks on the non-preprocessed code.

Sonar SSLR has a possibility to mark some tokens as generated.
This property (Token::isGeneratedCode()) is for example used
in order to ignore generated while

• [NC]LOC counting (see AbstractLineLengthCheck<>)
• CDP analysis (see CxxCpdVisitor) or
• Highlighting (see CxxHighlighterVisitor)

This patch ensures, that

• all AstNodes, which were created while macro expansion are marked as
  generated
• all complexity related visitors ignore generated AstNodes

RESULT:

PRO:

1. The complexity of "callers" is not affected by macro anymore.
   This fact makes the complexity check look logical again.

CON:

1. the complexity of "callees" is not calculated at all. This
   is correct in case of external macros, but wrong w.r.t. to the
   project intern macros. Possible solution: preprocessor must try
   to extract the function-like macro declaration into some special AST-subtree.
   Afterwards complexity metrics/check will be able to calculate/check
   macros as separate language construct.

By the way: it must be analyzed if and how macros affect other metrics.
For example LOC etc.

2. This patch might change the complexity related metrics. Depending
   on how many macros were used in the analyzed project, the reduction
   can be radical.

---

This change is 

[guwirth]

the complexity of "callees" is not calculated at all. This is correct in case of external macros, but wrong w.r.t. to the project intern macros.

@ivangalkin not sure if this is really a problem. 'Complexity' is normally 'Readability' (for sure Cognitive complexity). Not sure how other tools are doing this for 'Cyclomatic complexity' in case of C/C++ (with / without prepossessing it). Think it's quite intuitive to calculate only the 'visible' code. Imaging the macro would be a function/method call it's also not counting. So why counting a macro?

[guwirth]

@ivangalkin also like to make a sample:

// currently we don't calculate the complexity of macros
// we expand it and calculate the complexity inside user of the macro
// ==> new: handle this like a function?
#define MACRO(a, b) \
   if (a)  { \
      //... \
   else { \
     //... \
   }

// complexity of func1 is calculated
void func1(int a, int b)
{
   if (a)  { 
      //...
   else {
     //...
   }
}

// in this sample only the complexity of func2 is calculated
// complexity of called functions / methods do not count inside of this function
void func2()
{
    int a=0, b=0;
    if ( /*..*/ ) {
       func1(a, b);
    }
}

// in the past we did a macro expansion and were calculating the resulting complexity
// but I think there is (should be) no difference to func2?
void func3 ()
{
    int a=0, b=0;
    if ( /*..*/ ) {
       MACRO(a, b);
    }
}

[ivangalkin]

@guwirth I agree with your example. Complexity of func2() and func3() should be the same (and it will be the same as result of this PR). I believe, that complexity of MACRO should be somehow added to the overall complexity of the project e.g. by treating them as function-definition-like nodes. The advantage of such one-time calculation will be that complexity of MACRO is added only once. Currently it might be misleadingly multiplied. Example:

void func2()
{
    int a=0, b=0;
    if ( /*..*/ ) {
       func1(a, b); // function-call is not considered to be a complexity source
       func1(a, a); // so 3 functions calls still add nothing to the function complexity
       func1(b, b);
    }
}

void func3 ()
{
    int a=0, b=0;
    if ( /*..*/ ) {
       MACRO(a, b); // we "incline" the complexity of MACRO, but not just once but 3 times (!)
       MACRO(a, a); // this might increase the overall project complexity and cause complexity warnings
       MACRO(b, b); // it also contradicts the func2 example 
    }
}

[guwirth]

@ALL someone a different opinion?

...complexity of MACRO should be somehow added to the overall complexity of the project

@ivangalkin do you see a chance to do this?

[guwirth]

@ALL like to merge this this evening. No feedback means ok 😄.

[ivangalkin]

@guwirth sorry for the delay and thank you for giving this PR a chance

@ivangalkin do you see a chance to do this?

There is a little chance, that token-stringopt (in terms of MSDN) is a parseable AST subtree. My naive idea was to instrument the preprocessor in the way, that...

#define identifier( identifieropt, ... , identifieropt ) token-stringopt 

... becomes ...

void identifier( void* identifieropt, ... , void* identifieropt ) { token-stringopt; }

... so that our parser could try to parse it. If it succeeds, we have a function declaration, which complexity might be analyzed in the standard way. If not - we just ignore it.

I've played around with the preprocessor, but could not manage to get some results yet. We can create an user story / issue as a reminder. Maybe somebody has a better idea.