Fix #14334: Support more msbuild conditional constructs

.github/workflows/selfcheck.yml

@@ -121,7 +121,7 @@ jobs:
 
       - name: Self check (unusedFunction / no test / no gui)
         run: |
-          supprs="--suppress=unusedFunction:lib/errorlogger.h:196 --suppress=unusedFunction:lib/importproject.cpp:1516 --suppress=unusedFunction:lib/importproject.cpp:1540"
+          supprs="--suppress=unusedFunction:lib/errorlogger.h:196 --suppress=unusedFunction:lib/importproject.cpp:1665 --suppress=unusedFunction:lib/importproject.cpp:1689"
           ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib -D__CPPCHECK__ -D__GNUC__ --enable=unusedFunction,information --exception-handling -rp=. --project=cmake.output.notest_nogui/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr $supprs
         env:
           DISABLE_VALUEFLOW: 1

lib/importproject.cpp

@@ -490,6 +490,7 @@ bool ImportProject::importSln(std::istream &istr, const std::string &path, const
 
 namespace {
     struct ProjectConfiguration {
+        ProjectConfiguration() = default;
         explicit ProjectConfiguration(const tinyxml2::XMLElement *cfg) {
             const char *a = cfg->Attribute("Include");
             if (a)
@@ -533,45 +534,183 @@ namespace {
             }
         }
 
+        static bool evalExpression(std::string expr /* should be a std::string_view */, const ProjectConfiguration& p)
+        {
+            const auto startsHere = [&](size_t& i, std::string string)
+            {
+                if (i + string.length() > expr.length())
+                {
+                    return false;
+                }
+                if (std::equal(string.begin(), string.end(), expr.begin() + i))
+                {
+                    i += string.length();
+                    return true;
+                }
+                return false;
+            };
+            const auto skipSpaces = [&](size_t& i)
+            {
+                while (i < expr.length() && expr[i] == ' ') ++i;
+            };
+            const auto parseString = [&](size_t& i)
+            {
+                skipSpaces(i);
+                if (i >= expr.length() || expr[i] != '\'')
+                {
+                    throw std::runtime_error("Expecting a start of a string!");
+                }
+                auto start = ++i;
+                while (i < expr.length() && expr[i] != '\'') ++i;
+                std::string string = expr.substr(start, i - start);
+                if (i >= expr.length() || expr[i] != '\'')
+                {
+                    throw std::runtime_error("Expecting an end of a string!");
+                }
+                ++i;
+                return string;
+            };
+            bool currentVal = false; // should be std::optional<bool>
+            bool initialized = false;
+            for (std::size_t i = 0; i < expr.length();)
+            {
+                if (expr[i] == ' ')
+                {
+                    ++i;
+                    continue;
+                }
+                if (expr[i] == '!')
+                {
+                    return !evalExpression(expr.substr(i + 1), p);
+                }
+                if (startsHere(i, "And"))
+                {
+                    if (!initialized)
+                    {
+                        throw std::runtime_error("'And' without previous expression!");
+                    }
+                    return currentVal && evalExpression(expr.substr(i), p);
+                }
+                if (startsHere(i, "Or"))
+                {
+                    if (!initialized)
+                    {
+                        throw std::runtime_error("'Or' without previous expression!");
+                    }
+                    return currentVal || evalExpression(expr.substr(i), p);
+                }
+                if (expr[i] == '(')
+                {
+                    // find closing )
+                    int count = 1;
+                    bool inString = false;
+                    auto end = std::string::npos;
+                    for (int j = i + 1; j < expr.length(); ++j)
+                    {
+                        if (inString)
+                        {
+                            if (expr[j] == '\'')
+                                inString = false;
+                        }
+                        else if (expr[j] == '\'')
+                        {
+                            inString = true;
+                        }
+                        else if (expr[j] == '(')
+                        {
+                            ++count;
+                        }
+                        else if (expr[j] == ')')
+                        {
+                            --count;
+                            if (count == 0)
+                            {
+                                end = j;
+                                break;
+                            }
+                        }
+                    }
+                    if (end == std::string::npos)
+                    {
+                        throw std::runtime_error("'(' without closing ')'!");
+                    }
+                    initialized = true;
+                    currentVal = evalExpression(expr.substr(i + 1, end - i - 1), p);
+                    i = end + 1;
+                    continue;
+                }
+                if (expr[i] == '\'')
+                {
+                    auto left = parseString(i);
+                    skipSpaces(i);
+                    if (i + 4 >= expr.length())
+                    {
+                        throw std::runtime_error("Within a string comparison. We expect at least a =='' or !='' !");
+                    }
+                    bool equal = expr[i] == '=';
+                    i += 2;
+                    // expect a string now
+                    auto right = parseString(i);
+                    replaceAll(left, "$(Configuration)", p.configuration);
+                    replaceAll(left, "$(Platform)", p.platformStr);
+                    replaceAll(right, "$(Configuration)", p.configuration);
+                    replaceAll(right, "$(Platform)", p.platformStr);
+                    initialized = true;
+                    currentVal = equal ? left == right : left != right;
+                    continue;
+                }
+                if (startsHere(i, "$(Configuration."))
+                {
+                    initialized = true;
+                    if (startsHere(i, "Contains("))
+                    {
+                        auto containsParam = parseString(i);
+                        currentVal = p.configuration.find(containsParam) != std::string::npos;
+                    }
+                    else if (startsHere(i, "StartsWith("))
+                    {
+                        auto startsWithParam = parseString(i);
+                        currentVal = p.configuration.find(startsWithParam) == 0;
+                    }
+                    else if (startsHere(i, "EndsWith("))
+                    {
+                        auto endsWithParam = parseString(i);
+                        currentVal = p.configuration.rfind(endsWithParam) == p.configuration.length() - endsWithParam.length();
+                    }
+                    else
+                    {
+                        throw std::runtime_error("Unexpected function call!");
+                    }
+                    skipSpaces(i);
+                    if (!startsHere(i, "))"))
+                    {
+                        throw std::runtime_error("Expecting end of expression!");
+                    }
+                    continue;
+                }
+                throw std::runtime_error("Unhandled expression!");
+            }
+            if (!initialized)
+            {
+                throw std::runtime_error("Expected expression here!");
+            }
+            return currentVal;
+        }
+
         // see https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-conditions
         // properties are .NET String objects and you can call any of its members on them
-        bool conditionIsTrue(const ProjectConfiguration &p) const {
+        bool conditionIsTrue(const ProjectConfiguration &p, std::vector<std::string> &errors, const std::string &filename) const {
             if (mCondition.empty())
                 return true;
-            std::string c = '(' + mCondition + ");";
-            replaceAll(c, "$(Configuration)", p.configuration);
-            replaceAll(c, "$(Platform)", p.platformStr);
-
-            // TODO: improve evaluation
-            const Settings s;
-            TokenList tokenlist(s, Standards::Language::C);
-            tokenlist.createTokensFromBuffer(c.data(), c.size()); // TODO: check result
-            // TODO: put in a helper
-            // generate links
+            try
             {
-                std::stack<Token*> lpar;
-                for (Token* tok2 = tokenlist.front(); tok2; tok2 = tok2->next()) {
-                    if (tok2->str() == "(")
-                        lpar.push(tok2);
-                    else if (tok2->str() == ")") {
-                        if (lpar.empty())
-                            break;
-                        Token::createMutualLinks(lpar.top(), tok2);
-                        lpar.pop();
-                    }
-                }
+                return evalExpression(mCondition, p);
             }
-            tokenlist.createAst();
-            for (const Token *tok = tokenlist.front(); tok; tok = tok->next()) {
-                if (tok->str() == "(" && tok->astOperand1() && tok->astOperand2()) {
-                    // TODO: this is wrong - it is Contains() not Equals()
-                    if (tok->astOperand1()->expressionString() == "Configuration.Contains")
-                        return ('\'' + p.configuration + '\'') == tok->astOperand2()->str();
-                }
-                if (tok->str() == "==" && tok->astOperand1() && tok->astOperand2() && tok->astOperand1()->str() == tok->astOperand2()->str())
-                    return true;
+            catch (const std::runtime_error& r)
+            {
+                errors.emplace_back(filename + ": Can not evaluate condition '" + mCondition + "': " + r.what());
+                return false;
             }
-            return false;
         }
     private:
         std::string mCondition;
@@ -644,6 +783,16 @@ namespace {
     };
 }
 
+// cppcheck-suppress unusedFunction
+bool cppcheck::testing::evaluateVcxprojCondition(const std::string& condition, const std::string& configuration,
+                                                 const std::string& platform)
+{
+    ProjectConfiguration p;
+    p.configuration = configuration;
+    p.platformStr = platform;
+    return ConditionalGroup::evalExpression(condition, p);
+}
+
 static std::list<std::string> toStringList(const std::string &s)
 {
     std::list<std::string> ret;
@@ -879,7 +1028,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
             }
             std::string additionalIncludePaths;
             for (const ItemDefinitionGroup &i : itemDefinitionGroupList) {
-                if (!i.conditionIsTrue(p))
+                if (!i.conditionIsTrue(p, errors, cfilename))
                     continue;
                 fs.standard = Standards::getCPP(i.cppstd);
                 fs.defines += ';' + i.preprocessorDefinitions;
@@ -897,7 +1046,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
             }
             bool useUnicode = false;
             for (const ConfigurationPropertyGroup &c : configurationPropertyGroups) {
-                if (!c.conditionIsTrue(p))
+                if (!c.conditionIsTrue(p, errors, cfilename))
                     continue;
                 // in msbuild the last definition wins
                 useUnicode = c.useUnicode;

lib/importproject.h

@@ -49,6 +49,11 @@ namespace cppcheck {
             return caseInsensitiveStringCompare(lhs,rhs) < 0;
         }
     };
+
+    namespace testing
+    {
+        CPPCHECKLIB bool evaluateVcxprojCondition(const std::string& condition, const std::string& configuration, const std::string& platform);
+    }
 }
 
 /**

test/testimportproject.cpp

@@ -75,6 +75,7 @@ class TestImportProject : public TestFixture {
         TEST_CASE(importCppcheckGuiProjectPremiumMisra);
         TEST_CASE(ignorePaths);
         TEST_CASE(testVcxprojUnicode);
+        TEST_CASE(testVcxprojConditions);
     }
 
     void setDefines() const {
@@ -579,6 +580,25 @@ class TestImportProject : public TestFixture {
         ASSERT_EQUALS(project.fileSettings.back().useMfc, true);
     }
 
+
+    void testVcxprojConditions() const
+    {
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Debug'", "Debug", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Platform)'=='Win32'", "Debug", "Win32"));
+        ASSERT(!cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Release'", "Debug", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' ", "Debug", "Win32"));
+        ASSERT(!cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' != 'Debug' ", "Debug", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)|$(Platform)' == 'Debug|Win32' ", "Debug", "Win32"));
+        ASSERT(!cppcheck::testing::evaluateVcxprojCondition("!('$(Configuration)|$(Platform)' == 'Debug|Win32' )", "Debug", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' And '$(Platform)' == 'Win32'", "Debug", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' Or '$(Platform)' == 'Win32'", "Release", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.StartsWith('Debug'))", "Debug-AddressSanitizer", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.EndsWith('AddressSanitizer'))", "Debug-AddressSanitizer", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address'))", "Debug-AddressSanitizer", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address')) And '$(Platform)' == 'Win32'", "Debug-AddressSanitizer", "Win32"));
+        ASSERT(cppcheck::testing::evaluateVcxprojCondition(" ($(Configuration.Contains('Address')) ) And ( '$(Platform)' == 'Win32')", "Debug-AddressSanitizer", "Win32"));
+    }
+
     // TODO: test fsParseCommand()
 
     // TODO: test vcxproj conditions