SonarSource
Merge S3923 into S1871
S3923 is a more strict version of S1871, which was generated by deprecating S2758. They are both SonarWay rules.
At first glance, there are multiple problems with this split:
- not clear why all branches is more problematic than two branches, or more useful, to justify a separate rule.
- theoretically, S1871 should raise every time S3923 is raised, but on peach this does not seem the case.
- taxonomy classification consistency: one is classified as Intentional (reliability), the other one Adaptable (maintainability)
- severity consistency: one is a bug while the other one is a code smell.
When merging, consider that today's implementation S1871 is not efficient as if in if .. else if chain are analyzed twice.
One suggestion would be only to analyze the top-level if statement, by not analyzing any if statement that has at least one preceding if statement (!GetPrecedingIfsInConditionChain.Any())
More context here.
Here is how to merge the rules: An issue should be raised when a branch is identical to another branch in the if-chain. Exception: The branch has only a single statement (even better: would be a single line). Exception to the Exception: There is an else/default branch and all branches are identical.
Example 1:
if (condition1)
{
DoThing1();
DoThing2();
}
else if (condition2)
{
DoThing3();
DoThing4();
}
else
{ // Noncompliant
DoThing1();
DoThing2();
}
Example 2:
if (condition1)
{
DoThing1();
}
else if (condition2)
{
DoThing3();
}
else
{ // Compliant, exception
DoThing1();
}
Example 3:
if (condition1)
{
DoThing1();
}
// ... possibly more else if with the same body
else if (condition2)
{ // Compliant, exception
DoThing1();
}
// implicit:
// else { }
// This is why it is important to check for the default branch.
Example 4:
if (condition1)
{
DoThing1();
}
// ... possibly more else if with the same body
else
{ // Noncompliant, exception to the exception
DoThing1();
}
