Fix FPs in bitwiseOnBoolean (#3455)

Author: pfultz2

lib/checkbool.cpp

@@ -72,6 +72,13 @@ void CheckBool::incrementBooleanError(const Token *tok)
         );
 }
 
+static bool isConvertedToBool(const Token* tok)
+{
+    if (!tok->astParent())
+        return false;
+    return astIsBool(tok->astParent()) || Token::Match(tok->astParent()->previous(), "if|while (");
+}
+
 //---------------------------------------------------------------------------
 // if (bool & bool) -> if (bool && bool)
 // if (bool | bool) -> if (bool || bool)
@@ -90,20 +97,29 @@ void CheckBool::checkBitwiseOnBoolean()
     for (const Scope * scope : symbolDatabase->functionScopes) {
         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
             if (tok->isBinaryOp() && (tok->str() == "&" || tok->str() == "|")) {
-                if (astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2())) {
-                    if (tok->astOperand2()->variable() && tok->astOperand2()->variable()->nameToken() == tok->astOperand2())
-                        continue;
-                    const std::string expression = astIsBool(tok->astOperand1()) ? tok->astOperand1()->expressionString() : tok->astOperand2()->expressionString();
-                    bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||");
-                }
+                if (!(astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2())))
+                    continue;
+                if (tok->str() == "|" && !isConvertedToBool(tok) && !(astIsBool(tok->astOperand1()) && astIsBool(tok->astOperand2())))
+                    continue;
+                if (!isConstExpression(tok->astOperand1(), mSettings->library, true, mTokenizer->isCPP()))
+                    continue;
+                if (!isConstExpression(tok->astOperand2(), mSettings->library, true, mTokenizer->isCPP()))
+                    continue;
+                if (tok->astOperand2()->variable() && tok->astOperand2()->variable()->nameToken() == tok->astOperand2())
+                    continue;
+                const std::string expression = astIsBool(tok->astOperand1()) ? tok->astOperand1()->expressionString()
+                                                                             : tok->astOperand2()->expressionString();
+                bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||");
             }
         }
     }
 }
 
-void CheckBool::bitwiseOnBooleanError(const Token *tok, const std::string &expression, const std::string &op)
+void CheckBool::bitwiseOnBooleanError(const Token* tok, const std::string& expression, const std::string& op)
 {
-    reportError(tok, Severity::style, "bitwiseOnBoolean",
+    reportError(tok,
+                Severity::style,
+                "bitwiseOnBoolean",
                 "Boolean expression '" + expression + "' is used in bitwise operation. Did you mean '" + op + "'?",
                 CWE398,
                 Certainty::inconclusive);

lib/checkbool.h

@@ -104,7 +104,7 @@ class CPPCHECKLIB CheckBool : public Check {
     void comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression);
     void assignBoolToPointerError(const Token *tok);
     void assignBoolToFloatError(const Token *tok);
-    void bitwiseOnBooleanError(const Token *tok, const std::string &expression, const std::string &op);
+    void bitwiseOnBooleanError(const Token* tok, const std::string& expression, const std::string& op);
     void comparisonOfBoolExpressionWithIntError(const Token *tok, bool not0or1);
     void pointerArithBoolError(const Token *tok);
     void returnValueBoolError(const Token *tok);

test/testbool.cpp

@@ -810,6 +810,26 @@ class TestBool : public TestFixture {
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a>0' is used in bitwise operation. Did you mean '&&'?\n", errout.str());
 
+        check("void f(bool a, int b) {\n"
+              "    if(a | b) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
+
+        check("void f(int a, bool b) {\n"
+              "    if(a | b) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'b' is used in bitwise operation. Did you mean '||'?\n", errout.str());
+
+        check("int f(bool a, int b) {\n"
+              "    return a | b;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("bool f(bool a, int b) {\n"
+              "    return a | b;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Boolean expression 'a' is used in bitwise operation. Did you mean '||'?\n", errout.str());
+
         check("void f(int a, int b) {\n"
               "    if(a & b) {}\n"
               "}");
@@ -824,6 +844,12 @@ class TestBool : public TestFixture {
               "    class C { void foo(bool &b) {} };\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("bool f();\n"
+              "bool g() {\n"
+              "  return f() | f();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void incrementBoolean() {