Cognitive complexity: Recursion not handled properly

[nathanaelg]

I recently discovered that cognitive complexity was adding an increment for recursion in some places that weren't actually recursion and the more I think about it, the more I am unconvinced by the current strategy for identifying recursion.

The problem is that for something to be actually recursion it needs to be a call to the current function and there doesn't seem to be a very robust or easy way of determining this.

At the moment the current function's name is determined by finding an IDENTIFIER within functionDefinition that is not in the functionDeclSpecifierSeq, parametersAndQualifiers or functionBody. It should also exclude nestedNameSpecifier to not use the class name in something like void ClassA::FunctionB(int param)

Then if that IDENTIFIER is found in the function (in a different place to the declaration), it will get counted as a recursive call.

Even if that issue in bold is fixed, I think this strategy will fail in other cases:
overloaded functions
overridden functions
same function name in a different class

All of those will probably currently get classed as recursion even though they're not.

In some ways I think unless there is a better way of identifying recursion there will be more false positives than true positives - I'd just remove the increment for recursion for now.

[nathanaelg]

This is the change to ignore classes while identifying the current function, but as discussed above - it doesn't fix all the problematic cases.

[guwirth]

Hello @nathanaelg maybe it's the easiest to remove the recursion detection at all? Think it's very hard to find the right context in all cases. We have then a performance penalty in all cases just for rare usage of recursion.

[nathanaelg]

That would be my preference at this stage anyway, I think there's still value in the metric without counting recursion and at least it should be correct the rest of the time instead of incorrectly penalizing things for being recursion when they're not.