False Positive: S1854 Unused assignments should be removed

SonarQube for IDE
1

Language: C

In case of a switch statement the case of unspecified (default) behavior is not considered

See the following example.

We do not want to add a default clause to the switch statement because then the compiler does not warn anymore that some enumerated values are not considered in the switch clause. This is mainly a maintainability issue, where programmers who add an additional enum value to the ‘colors_t’ enum would be warned about locations in the code where the enum value should be considered. If there is a ‘default’ then this does not occur. Also it forces you to considered every possible value for the enum variable.

Leaving the value uninitialized is wrong too.

#include <stdio.h>

typedef enum {
    BLUE = 0,
    RED = 1
} colors_t;

const char* colors_to_str(colors_t color)
{
    // c:S1854 Unused assignments should be removed
    //   Value stored to 'result' during its initialization is never read
    // Obviously wrong since 'unknown' is in the output of the program
    const char* result = "unknown";

    switch(color) {
    case BLUE:
        result = "blue";
        break;
    case RED:
        result = "red";
        break;
    // no default
    }

    return result;
}

int main(void)
{
    printf("%s, %s and %s\n", colors_to_str(RED), colors_to_str(BLUE),
           colors_to_str(3));
    return 0;
}
5

I’d say, it depends.
Many users would use enums that hold at most 1 alternative at a time. Imagine such a user seeing complaints about the unhandled “default” case in a switch that covers all alternatives.

It’s a good practice not to have such extra default switch-cases because that would mask the switch statements that need to be updated once a new entry is added to the enumeration.

If the switch statement explicitly mentions all alternatives and a new alternative is added to the enumeration, a new compiler warning will appear highlighting the places you should consider patching.

It’s fair to point out that in the rule, we don’t apply any heuristics to determine if an enumeration is actually a bitmask (hence multiple alternatives can be possible at once) or just a regular choice type. That could be detected for enums that only have a power of 2 entries, but this guessing still falls flat for “small” enums, where it’s still ambiguous.

In this particular case, if the code would have been written in a slightly simpler form, then we wouldn’t raise an issue:

const char* colors_to_str(colors_t color) {
    switch(color) {
        case BLUE: return "blue";
        case RED:  return "red";
    }
    return "unknown";
}

Notice that we only have return statements instead of mutating some states and having a common return statement.

By thoroughly investigating the example case, we created 3 tickets:

  • CPP-5627: Improving the engine in case the enumeration value is concrete by following a fall-through branch in case none of the switch-cases apply to the concrete enum value.
  • CPP-5628: Rule idea: Report complete switches that take bitwise manipulated symbolic values
  • CPP-5629: Suppress S1854 reports crossing complete switches
1 Like