Improve duplicate expressions in the ternary op by checking for equal values as well (#1134)

* Improve duplicate expressions in the ternary op by checking for equal values as well

* Use value instead of expression

lib/checkother.cpp

@@ -1973,7 +1973,9 @@ void CheckOther::checkDuplicateExpression()
                     }
                 }
             } else if (styleEnabled && tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
-                if (isSameExpression(_tokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), _settings->library, false))
+                if (!tok->astOperand1()->values().empty() && !tok->astOperand2()->values().empty() && isEqualKnownValue(tok->astOperand1(), tok->astOperand2()))
+                    duplicateValueTernaryError(tok);
+                else if (isSameExpression(_tokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), _settings->library, false))
                     duplicateExpressionTernaryError(tok);
             }
         }
@@ -1997,6 +1999,13 @@ void CheckOther::duplicateExpressionTernaryError(const Token *tok)
                 "the same code is executed regardless of the condition.", CWE398, false);
 }
 
+void CheckOther::duplicateValueTernaryError(const Token *tok)
+{
+    reportError(tok, Severity::style, "duplicateValueTernary", "Same value in both branches of ternary operator.\n"
+                "Finding the same value in both branches of ternary operator is suspicious as "
+                "the same code is executed regardless of the condition.", CWE398, false);
+}
+
 void CheckOther::selfAssignmentError(const Token *tok, const std::string &varname)
 {
     reportError(tok, Severity::warning,

lib/checkother.h

@@ -239,6 +239,7 @@ private:
     void misusedScopeObjectError(const Token *tok, const std::string &varname);
     void duplicateBranchError(const Token *tok1, const Token *tok2);
     void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
+    void duplicateValueTernaryError(const Token *tok);
     void duplicateExpressionTernaryError(const Token *tok);
     void duplicateBreakError(const Token *tok, bool inconclusive);
     void unreachableCodeError(const Token* tok, bool inconclusive);
@@ -297,6 +298,7 @@ private:
         c.clarifyStatementError(nullptr);
         c.duplicateBranchError(nullptr, nullptr);
         c.duplicateExpressionError(nullptr, nullptr, "&&");
+        c.duplicateValueTernaryError(nullptr);
         c.duplicateExpressionTernaryError(nullptr);
         c.duplicateBreakError(nullptr,  false);
         c.unreachableCodeError(nullptr,  false);

test/testother.cpp

@@ -128,6 +128,7 @@ private:
         TEST_CASE(duplicateExpression4); // ticket #3354 (++)
         TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values)
         TEST_CASE(duplicateExpression6); // ticket #4639
+        TEST_CASE(duplicateValueTernary);
         TEST_CASE(duplicateExpressionTernary); // #6391
         TEST_CASE(duplicateExpressionTemplate); // #6930
 
@@ -3855,11 +3856,6 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (style) Same expression in both branches of ternary operator.\n", errout.str());
 
-        check("void f() {\n"
-              "    if( a ? (b ? false:false): false ) ;\n"
-              "}");
-        ASSERT_EQUALS("[test.cpp:2]: (style) Same expression in both branches of ternary operator.\n", errout.str());
-
         check("void f() {\n"
               "    return A ? x : z;\n"
               "}");
@@ -3876,6 +3872,37 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void duplicateValueTernary() {
+        check("void f() {\n"
+              "    if( a ? (b ? false:false): false ) ;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Same value in both branches of ternary operator.\n", errout.str());
+
+        check("int f1(int a) {return (a == 1) ? (int)1 : 1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+
+        check("int f2(int a) {return (a == 1) ? (int)1 : (int)1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+        
+        check("int f3(int a) {return (a == 1) ? 1 : (int)1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+        
+        check("int f4(int a) {return (a == 1) ? 1 : 1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+        
+        check("int f5(int a) {return (a == (int)1) ? (int)1 : 1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+        
+        check("int f6(int a) {return (a == (int)1) ? (int)1 : (int)1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+        
+        check("int f7(int a) {return (a == (int)1) ? 1 : (int)1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+        
+        check("int f8(int a) {return (a == (int)1) ? 1 : 1; }");
+        ASSERT_EQUALS("[test.cpp:1]: (style) Same value in both branches of ternary operator.\n", errout.str());
+    }
+
     void duplicateExpressionTemplate() { // #6930
         check("template <int I> void f() {\n"
               "    if (I >= 0 && I < 3) {}\n"