False negative for duplicated implementation in conditional branches

I just found a bug in our code base that was recently introduced (by me :upside_down_face: ) which looks like this

if (left.firstThing == right.firstThing)
{
    return left.secondThing < right.secondThing;
}
return left.secondThing < right.secondThing;

The last line should be comparing firstThing instead.

This looks like it should have triggered a code smell, because this is two sides of a branch having the same implementation. This rule seems relevant https://rules.sonarsource.com/cpp/RSPEC-1871 and also C++ static code analysis: All branches in a conditional structure should not have exactly the same implementation, which looks like a more specific version of the same rule.

I guess the false negative is caused by the lack of an else clause around the second return statement. But more generally, if the expression in the conditional is repeated after the if then if control flow jumps from inside the branch that gets repeated, then it’s possible to have these repeats, weather it’s by return, break, continue, etc.

This is extra relevant when following this rule from clang tidy clang-tidy - readability-else-after-return — Extra Clang Tools 16.0.0git documentation I could have sworn there was a corresponding rule in Sonar, but I can’t seem to find it.

Hi @torgeir.skogen,

You are right that is a false negative. It should be trivial to fix, I created a ticket [CPP-3817] - Jira
Thanks for the detailed valuable report and hopefully we will catch the bug that you will introduce next time :laughing:

Thanks,

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.