Conditions: Better handling of function calls

Author: danmar

lib/astutils.cpp

@@ -164,10 +164,23 @@ bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2
     if (tok1->isSigned() != tok2->isSigned())
         return false;
     if (pure && tok1->isName() && tok1->next()->str() == "(" && tok1->str() != "sizeof") {
-        if (!tok1->function() && !Token::Match(tok1->previous(), ".|::") && !library.isFunctionConst(tok1->str(), true) && !tok1->isAttributeConst() && !tok1->isAttributePure())
-            return false;
-        else if (tok1->function() && !tok1->function()->isConst() && !tok1->function()->isAttributeConst() && !tok1->function()->isAttributePure())
-            return false;
+        if (!tok1->function()) {
+            if (!Token::Match(tok1->previous(), ".|::") && !library.isFunctionConst(tok1) && !tok1->isAttributeConst() && !tok1->isAttributePure())
+                return false;
+            if (Token::simpleMatch(tok1->previous(), ".")) {
+                const Token *lhs = tok1->previous();
+                while (Token::Match(lhs, "(|.|["))
+                    lhs = lhs->astOperand1();
+                bool lhsIsConst = (lhs->variable() && lhs->variable()->isConst()) ||
+                                  (lhs->valueType() && lhs->valueType()->constness > 0) ||
+                                  (Token::Match(lhs, "%var% . %name% (") && library.isFunctionConst(lhs->tokAt(2)));
+                if (!lhsIsConst)
+                    return false;
+            }
+        } else {
+            if (tok1->function() && !tok1->function()->isConst() && !tok1->function()->isAttributeConst() && !tok1->function()->isAttributePure())
+                return false;
+        }
     }
     // templates/casts
     if ((Token::Match(tok1, "%name% <") && tok1->next()->link()) ||

test/testcondition.cpp

@@ -546,6 +546,20 @@ class TestCondition : public TestFixture {
               "}");
         ASSERT_EQUALS("", errout.str());
 
+        {
+        check("void f(Class &c) {\n"
+              "    if (c.dostuff() == 3) {}\n"
+              "    else { if (c.dostuff() == 3) {} }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(const Class &c) {\n"
+              "    if (c.dostuff() == 3) {}\n"
+              "    else { if (c.dostuff() == 3) {} }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
+        }
+
         check("void f(int a, int &b) {\n"
               "   x = x / 2;\n"
               "   if (x < 100) { b = 1; }\n"
@@ -2091,7 +2105,7 @@ class TestCondition : public TestFixture {
         ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: a. '!a || (a && b)' is equivalent to '!a || b'\n", errout.str());
 
 
-        check("void f() {\n"
+        check("void f(const Token *tok) {\n"
               "    if (!tok->next()->function() || \n"
               "        (tok->next()->function() && tok->next()->function()->isConstructor()));\n"
               "}");
@@ -2109,7 +2123,7 @@ class TestCondition : public TestFixture {
               "}");
         ASSERT_EQUALS("", errout.str());
 
-        check("void f() {\n"
+        check("void f(const Token *tok) {\n"
               "    if (!tok->next(1)->function(1) || \n"
               "        (tok->next(1)->function(1) && tok->next(1)->function(1)->isConstructor()));\n"
               "}");

test/testother.cpp

@@ -3645,7 +3645,7 @@ class TestOther : public TestFixture {
               "}");
         ASSERT_EQUALS("", errout.str());
 
-        check("void f() {\n"
+        check("void f(const Bar &bar) {\n"
               "    bool a = bar.isSet() && bar->isSet();\n"
               "    bool b = bar.isSet() && bar.isSet();\n"
               "}");
@@ -6124,7 +6124,7 @@ class TestOther : public TestFixture {
         check("bool isInUnoIncludeFile(StringRef name) {"
               "   return  name.startswith(SRCDIR \"/com/\") || name.startswith(SRCDIR \"/uno/\");\n"
               "};", "test.cpp", false, false);
-        TODO_ASSERT_EQUALS("", "[test.cpp:1] -> [test.cpp:1]: (style) Same expression on both sides of '||'.\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
     }
 
     void raceAfterInterlockedDecrement() {