Hi, I would like to suggest a new rule:
Conditional return statements should not be gratuitious
A conditional return that returns the same value as the normal code flow would otherwise return, is
unnecessary and can be removed. If returning the same value does not match the programmer’s intent, then it’s a bug, and the return values should be fixed.
Conditional with identical branches
A conditional with identical branches would be detected by squid:S3923 (All branches in a conditional structure should not have exactly the same implementation)
int foo(int b) {
doSomething();
if (b == 0) {
return 42; // detected by squid:S3923
} else {
return 42;
}
}
However, since the branches in the code above all end with return
, the else
-branch is redundant and can be unwrapped, leading to equivalent logic without else
-branch.
This code now contains a redundant return
that is not detected by any existing rule:
Non-compliant Code Example
int foo(int b) {
doSomething();
if (b == 0) {
return 42; // non-compliant (redundant return)
}
return 42;
}
If the programmer’s intent is actually to return the same value, the code can be simplified to:
Compliant Code Example
int foo(int b) {
doSomething();
return 42; // compliant
}
If the programmer’s intent is to return different values based on the condition, the code is wrong and should be corrected:
Compliant Code Example
int foo(int b) {
doSomething();
if (b == 0) {
return 17; // compliant
}
return 42;
}
Another Non-Compliant Code Example:
String foo() {
boolean cond = checkSomething();
if (cond) {
return "very nice"; // non-compliant (redundant return)
}
return "very nice";
}
Taking into account possible side-effects of checkSomething()
, this can be simplified to:
Compliant Code Example
String foo() {
checkSomething(); // need to keep this due to possible side-effects
return "very nice"; // compliant
}
Compliant Code Example
String foo() {
boolean cond = checkSomething();
if (cond) {
return "something different"; // compliant
}
return "very nice";
}
Exception: Redundant return in void
methods
This rule does not apply to void
methods because redundant return in void
methods is already covered by squid:S3626 (Jump statements should not be redundant):
void foo(int b) {
doSomething();
if (b == 0) {
return; // redundant, detected by squid:S3626
}
}