From 267552779d9a18efb520df6080822e969817a8c7 Mon Sep 17 00:00:00 2001
From: PKEuS <philipp.kloke@web.de>
Date: Sat, 3 Jan 2015 18:01:49 +0100
Subject: [PATCH] Improved check (#6391): Detect identical code in both
 branches of ternary operator

---
 lib/checkother.cpp | 13 ++++++++++++-
 lib/checkother.h   |  2 ++
 test/testother.cpp | 18 ++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 90607b0d2..015f76c98 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -87,7 +87,7 @@ bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::
         tok1 = tok1->astOperand2();
     if (tok2->str() == "." && tok2->astOperand1() && tok2->astOperand1()->str() == "this")
         tok2 = tok2->astOperand2();
-    if (tok1->str() != tok2->str() || tok1->varId() != tok2->varId())
+    if (tok1->varId() != tok2->varId() || tok1->str() != tok2->str())
         return false;
     if (tok1->str() == "." && tok1->originalName() != tok2->originalName())
         return false;
@@ -2303,6 +2303,7 @@ void CheckOther::checkDuplicateExpression()
 
     std::list<Scope>::const_iterator scope;
     std::list<const Function*> constFunctions;
+    const std::set<std::string> temp; // Can be used as dummy for isSameExpression()
     getConstFunctions(symbolDatabase, constFunctions);
 
     for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
@@ -2350,6 +2351,9 @@ void CheckOther::checkDuplicateExpression()
                         }
                     }
                 }
+            } else if (tok->astOperand1() && tok->astOperand2() && tok->str() == ":" && tok->astParent() && tok->astParent()->str() == "?") {
+                if (isSameExpression(tok->astOperand1(), tok->astOperand2(), temp))
+                    duplicateExpressionTernaryError(tok);
             }
         }
     }
@@ -2367,6 +2371,13 @@ void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2,
                 "determine if it is correct.");
 }
 
+void CheckOther::duplicateExpressionTernaryError(const Token *tok)
+{
+    reportError(tok, Severity::style, "duplicateExpressionTernary", "Same expression in both branches of ternary operator.\n"
+                "Finding the same expression in both branches of ternary operator is suspicious as "
+                "the same code is executed regardless of the condition.");
+}
+
 void CheckOther::selfAssignmentError(const Token *tok, const std::string &varname)
 {
     reportError(tok, Severity::warning,
diff --git a/lib/checkother.h b/lib/checkother.h
index d296af233..ff5647ed5 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -271,6 +271,7 @@ private:
     void duplicateIfError(const Token *tok1, const Token *tok2);
     void duplicateBranchError(const Token *tok1, const Token *tok2);
     void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
+    void duplicateExpressionTernaryError(const Token *tok);
     void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2);
     void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
     void duplicateBreakError(const Token *tok, bool inconclusive);
@@ -331,6 +332,7 @@ private:
         c.clarifyStatementError(0);
         c.duplicateBranchError(0, 0);
         c.duplicateExpressionError(0, 0, "&&");
+        c.duplicateExpressionTernaryError(0);
         c.duplicateBreakError(0, false);
         c.unreachableCodeError(0, false);
         c.unsignedLessThanZeroError(0, "varname", false);
diff --git a/test/testother.cpp b/test/testother.cpp
index e386ccdfa..5eb43ade0 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -140,6 +140,7 @@ private:
         TEST_CASE(duplicateExpression4); // ticket #3354 (++)
         TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values)
         TEST_CASE(duplicateExpression6); // ticket #4639
+        TEST_CASE(duplicateExpressionTernary); // #6391
 
         TEST_CASE(checkSignOfUnsignedVariable);
         TEST_CASE(checkSignOfPointer);
@@ -4322,6 +4323,23 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void duplicateExpressionTernary() { // #6391
+        check("void f() {\n"
+              "    return A ? x : x;\n"
+              "}");
+        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"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) {
         // Clear the error buffer..
         errout.str("");