New check: Find suspicious case labels like 'case A||B:'

lib/checkother.cpp

@@ -1057,6 +1057,42 @@ void CheckOther::switchCaseFallThrough(const Token *tok)
                 "switchCaseFallThrough", "Switch falls through case without comment. 'break;' missing?");
 }
 
+
+//---------------------------------------------------------------------------
+// Check for statements like case A||B: in switch()
+//---------------------------------------------------------------------------
+void CheckOther::checkSuspiciousCaseInSwitch()
+{
+    if (!_settings->inconclusive || !_settings->isEnabled("style"))
+        return;
+
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+
+    for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
+        if (i->type != Scope::eSwitch)
+            continue;
+
+        for (const Token* tok = i->classStart->next(); tok != i->classEnd; tok = tok->next()) {
+            if (tok->str() == "case") {
+                for (const Token* tok2 = tok->next(); tok2; tok2 = tok2->next()) {
+                    if (Token::Match(tok2, "[:;}{]"))
+                        break;
+                    if (Token::Match(tok2, "&&|%oror%"))
+                        suspiciousCaseInSwitchError(tok, tok2->str());
+                }
+            }
+        }
+    }
+}
+
+void CheckOther::suspiciousCaseInSwitchError(const Token* tok, const std::string& operatorString)
+{
+    reportError(tok, Severity::warning, "suspiciousCase",
+                "Found suspicious case label in switch(). Operator '" + operatorString + "' probably doesn't work as intended.\n"
+                "Using an operator like '" + operatorString + "' in a case label is suspicious. Did you intend to use a bitwise operator, multiple case labels or if/else instead?", true);
+}
+
+
 //---------------------------------------------------------------------------
 //    int x = 1;
 //    x = x;            // <- redundant assignment to self

lib/checkother.h

@@ -62,6 +62,7 @@ public:
         checkOther.suspiciousSizeofCalculation();
         checkOther.checkRedundantAssignment();
         checkOther.checkRedundantAssignmentInSwitch();
+        checkOther.checkSuspiciousCaseInSwitch();
         checkOther.checkAssignmentInAssert();
         checkOther.checkSizeofForArrayParameter();
         checkOther.checkSizeofForPointerSize();
@@ -186,6 +187,9 @@ public:
     /** @brief %Check for assigning to the same variable twice in a switch statement*/
     void checkRedundantAssignmentInSwitch();
 
+    /** @brief %Check for code like 'case A||B:'*/
+    void checkSuspiciousCaseInSwitch();
+
     /** @brief %Check for switch case fall through without comment */
     void checkSwitchCaseFallThrough();
 
@@ -311,6 +315,7 @@ private:
     void redundantCopyInSwitchError(const Token *tok1, const Token* tok2, const std::string &var);
     void redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname);
     void switchCaseFallThrough(const Token *tok);
+    void suspiciousCaseInSwitchError(const Token* tok, const std::string& operatorString);
     void selfAssignmentError(const Token *tok, const std::string &varname);
     void assignmentInAssertError(const Token *tok, const std::string &varname);
     void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always);
@@ -392,6 +397,7 @@ private:
         c.redundantAssignmentInSwitchError(0, 0, "var");
         c.redundantCopyInSwitchError(0, 0, "var");
         c.switchCaseFallThrough(0);
+        c.suspiciousCaseInSwitchError(0, "||");
         c.selfAssignmentError(0, "varname");
         c.assignmentInAssertError(0, "varname");
         c.incorrectLogicOperatorError(0, "foo > 3 && foo < 4", true);
@@ -469,6 +475,7 @@ private:
                "* look for calculations inside sizeof()\n"
                "* look for suspicious calculations with sizeof()\n"
                "* assignment of a variable to itself\n"
+               "* Suspicious case labels in switch()\n"
                "* mutual exclusion over || always evaluating to true\n"
                "* Clarify calculation with parentheses\n"
                "* using increment on boolean\n"

test/testother.cpp

@@ -86,6 +86,8 @@ private:
         TEST_CASE(switchFallThroughCase);
         TEST_CASE(unreachableCode);
 
+        TEST_CASE(suspiciousCase);
+
         TEST_CASE(selfAssignment);
         TEST_CASE(trac1132);
         TEST_CASE(testMisusedScopeObjectDoesNotPickFunction1);
@@ -2726,6 +2728,31 @@ private:
     }
 
 
+    void suspiciousCase() {
+        check("void foo() {\n"
+              "    switch(a) {\n"
+              "        case A&&B:\n"
+              "            foo();\n"
+              "        case (A||B):\n"
+              "            foo();\n"
+              "        case 1||5:\n"
+              "            foo();\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Found suspicious case label in switch(). Operator '&&' probably doesn't work as intended.\n"
+                      "[test.cpp:5]: (warning, inconclusive) Found suspicious case label in switch(). Operator '||' probably doesn't work as intended.\n"
+                      "[test.cpp:7]: (warning, inconclusive) Found suspicious case label in switch(). Operator '||' probably doesn't work as intended.\n", errout.str());
+
+        check("void foo() {\n"
+              "    switch(a) {\n"
+              "        case 1:\n"
+              "            a=A&&B;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+
     void selfAssignment() {
         check("void foo()\n"
               "{\n"