From c4325bbec3c74df61b6971dfaeda30fcb4d79ef8 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Fri, 26 Apr 2019 05:30:41 -0500
Subject: [PATCH] Fix issue 9103: False positive duplicateConditionAssign
 (#1808)

* Fix issue 9103: False positive duplicateConditionAssign

* Update conditional message
---
 lib/checkcondition.cpp | 12 +++++++-----
 test/testcondition.cpp | 15 ++++++++++++---
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
index 26375ca99..a2b4afbfa 100644
--- a/lib/checkcondition.cpp
+++ b/lib/checkcondition.cpp
@@ -1543,10 +1543,6 @@ void CheckCondition::pointerAdditionResultNotNullError(const Token *tok, const T
 
 void CheckCondition::checkDuplicateConditionalAssign()
 {
-    // danmar: this is disabled until #9103 is fixed.
-    // the output should be clarified somehow.. see #9101
-    return;
-
     if (!mSettings->isEnabled(Settings::STYLE))
         return;
 
@@ -1568,6 +1564,8 @@ void CheckCondition::checkDuplicateConditionalAssign()
             const Token *assignTok = blockTok->next()->astTop();
             if (!Token::simpleMatch(assignTok, "="))
                 continue;
+            if (nextAfterAstRightmostLeaf(assignTok) != blockTok->link()->previous())
+                continue;
             if (!isSameExpression(
                     mTokenizer->isCPP(), true, condTok->astOperand1(), assignTok->astOperand1(), mSettings->library, true, true))
                 continue;
@@ -1582,15 +1580,19 @@ void CheckCondition::checkDuplicateConditionalAssign()
 void CheckCondition::duplicateConditionalAssignError(const Token *condTok, const Token* assignTok)
 {
     ErrorPath errors;
+    std::string msg = "Duplicate expression for the condition and assignment.";
     if (condTok && assignTok) {
         if (condTok->str() == "==") {
+            msg = "Assignment '" + assignTok->expressionString() + "' is redundant with condition '" + condTok->expressionString() + "'.";
             errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "'");
             errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "' is redundant");
         } else {
+            msg = "The statement 'if (" + condTok->expressionString() + ") " + assignTok->expressionString() + "' is logically equivalent to '" + assignTok->expressionString() + "'.";
             errors.emplace_back(assignTok, "Assignment '" + assignTok->expressionString() + "'");
             errors.emplace_back(condTok, "Condition '" + condTok->expressionString() + "' is redundant");
         }
     }
+
     reportError(
-        errors, Severity::style, "duplicateConditionalAssign", "Duplicate expression for the condition and assignment.", CWE398, false);
+        errors, Severity::style, "duplicateConditionalAssign", msg, CWE398, false);
 }
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index 072341103..c9fb409f3 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -3084,13 +3084,13 @@ private:
               "    if (x == y)\n"
               "        x = y;\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Duplicate expression for the condition and assignment.\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Assignment 'x=y' is redundant with condition 'x==y'.\n", errout.str());
 
         check("void f(int& x, int y) {\n"
               "    if (x != y)\n"
               "        x = y;\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Duplicate expression for the condition and assignment.\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) The statement 'if (x!=y) x=y' is logically equivalent to 'x=y'.\n", errout.str());
 
         check("void f(int& x, int y) {\n"
               "    if (x == y)\n"
@@ -3098,7 +3098,7 @@ private:
               "    else\n"
               "        x = 1;\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Duplicate expression for the condition and assignment.\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Assignment 'x=y' is redundant with condition 'x==y'.\n", errout.str());
 
         check("void f(int& x, int y) {\n"
               "    if (x != y)\n"
@@ -3113,6 +3113,15 @@ private:
               "        x = y + 1;\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("void g();\n"
+              "void f(int& x, int y) {\n"
+              "    if (x == y) {\n"
+              "        x = y;\n"
+              "        g();\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 };