fixup! Report overlapping inner conditions as identicalInnerCondition

Author: francois-berder

lib/checkcondition.cpp

@@ -760,7 +760,7 @@ void CheckCondition::multiCondition2()
                                 } else if (!isReturnVar && isSameExpression(true, firstCondition, cond2, *mSettings, true, true, &errorPath)) {
                                     identicalInnerConditionError(firstCondition, cond2, errorPath);
                                 } else if (!isReturnVar && isOverlappingCond(cond2, firstCondition, true)) {
-                                    identicalInnerConditionError(firstCondition, cond2, errorPath);
+                                    overlappingInnerConditionError(firstCondition, cond2, errorPath);
                                 }
                             }
                             return ChildrenToVisit::none;
@@ -881,6 +881,21 @@ void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token*
     reportError(std::move(errorPath), Severity::warning, "oppositeInnerCondition", msg, CWE398, Certainty::normal);
 }
 
+void CheckCondition::overlappingInnerConditionError(const Token *tok1,  const Token* tok2, ErrorPath errorPath)
+{
+    if (diag(tok1, tok2))
+        return;
+    const std::string s1(tok1 ? tok1->expressionString() : "x");
+    const std::string s2(tok2 ? tok2->expressionString() : "x");
+    const std::string innerSmt = innerSmtString(tok2);
+    errorPath.emplace_back(tok1, "outer condition: " + s1);
+    errorPath.emplace_back(tok2, "overlapping inner condition: " + s2);
+
+    const std::string msg("Overlapping inner '" + innerSmt + "' condition is always true.\n"
+                          "Overlapping inner '" + innerSmt + "' condition is always true (outer condition is '" + s1 + "' and inner condition is '" + s2 + "').");
+    reportError(std::move(errorPath), Severity::warning, "overlappingInnerCondition", msg, CWE398, Certainty::normal);
+}
+
 void CheckCondition::identicalInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath)
 {
     if (diag(tok1, tok2))
@@ -2113,6 +2128,7 @@ void CheckCondition::getErrorMessages(ErrorLogger *errorLogger, const Settings *
     c.comparisonError(nullptr, "&", 6, "==", 1, false);
     c.duplicateConditionError(nullptr, nullptr, ErrorPath{});
     c.overlappingElseIfConditionError(nullptr, 1);
+    c.overlappingInnerConditionError(nullptr, nullptr, ErrorPath());
     c.mismatchingBitAndError(nullptr, 0xf0, nullptr, 1);
     c.oppositeInnerConditionError(nullptr, nullptr, ErrorPath{});
     c.identicalInnerConditionError(nullptr, nullptr, ErrorPath{});

lib/checkcondition.h

@@ -130,6 +130,7 @@ class CPPCHECKLIB CheckCondition : public Check {
                          bool result);
     void duplicateConditionError(const Token *tok1, const Token *tok2, ErrorPath errorPath);
     void overlappingElseIfConditionError(const Token *tok, nonneg int line1);
+    void overlappingInnerConditionError(const Token *tok1, const Token *tok2, ErrorPath errorPath);
     void oppositeElseIfConditionError(const Token *ifCond, const Token *elseIfCond, ErrorPath errorPath);
 
     void oppositeInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath);

test/testcondition.cpp

@@ -94,6 +94,8 @@ class TestCondition : public TestFixture {
         TEST_CASE(identicalConditionAfterEarlyExit);
         TEST_CASE(innerConditionModified);
 
+        TEST_CASE(overlappingInnerCondition);
+
         TEST_CASE(clarifyCondition1);     // if (a = b() < 0)
         TEST_CASE(clarifyCondition2);     // if (a & b == c)
         TEST_CASE(clarifyCondition3);     // if (! a & b)
@@ -2834,20 +2836,6 @@ class TestCondition : public TestFixture {
               "    }\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:2:12] -> [test.cpp:4:13]: (warning) Identical inner 'if' condition is always true. [identicalInnerCondition]\n", errout_str());
-
-        check("void f(int x) {\n"
-              "    if (x == 1) {\n"
-              "        if (x & 7) {}\n"
-              "    }\n"
-              "}");
-        ASSERT_EQUALS("[test.cpp:2:11] -> [test.cpp:3:15]: (warning) Identical inner 'if' condition is always true. [identicalInnerCondition]\n", errout_str());
-
-        check("void f(int x) {\n"
-              "    if (x & 7) {\n"
-              "        if (x == 1) {}\n"
-              "    }\n"
-              "}");
-        ASSERT_EQUALS("", errout_str());
     }
 
     void identicalConditionAfterEarlyExit() {
@@ -3036,6 +3024,22 @@ class TestCondition : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
 
+    void overlappingInnerCondition() {
+        check("void f(int x) {\n"
+              "    if (x == 1) {\n"
+              "        if (x & 7) {}\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2:11] -> [test.cpp:3:15]: (warning) Overlapping inner 'if' condition is always true. [overlappingInnerCondition]\n", errout_str());
+
+        check("void f(int x) {\n"
+              "    if (x & 7) {\n"
+              "        if (x == 1) {}\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     // clarify conditions with = and comparison
     void clarifyCondition1() {
         check("void f() {\n"