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:1673 --suppress=unusedFunction:lib/importproject.cpp:1697"
           ./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 @@
 
 namespace {
     struct ProjectConfiguration {
+        ProjectConfiguration() = default;
         explicit ProjectConfiguration(const tinyxml2::XMLElement *cfg) {
             const char *a = cfg->Attribute("Include");
             if (a)
@@ -535,10 +536,14 @@
 
         // 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 {
-            if (mCondition.empty())
+        bool conditionIsTrue(const ProjectConfiguration &p, const std::string &filename, std::vector<std::string> &errors) const {
+            return conditionIsTrue(mCondition, p, filename, errors);
+        }
+
+        static bool conditionIsTrue(const std::string& condition, const ProjectConfiguration &p, const std::string &filename, std::vector<std::string> &errors) {
+            if (condition.empty())
                 return true;
-            std::string c = '(' + mCondition + ");";
+            std::string c = '(' + condition + ");";
             replaceAll(c, "$(Configuration)", p.configuration);
             replaceAll(c, "$(Platform)", p.platformStr);
 
@@ -555,25 +560,91 @@
                         lpar.push(tok2);
                     else if (tok2->str() == ")") {
                         if (lpar.empty())
-                            break;
+                            throw std::runtime_error("unmatched ')' in condition " + condition);
                         Token::createMutualLinks(lpar.top(), tok2);
                         lpar.pop();
                     }
                 }
+                if (!lpar.empty())
+                    throw std::runtime_error("'(' without closing ')'!");
             }
+
+            // Replace "And" and "Or" with "&&" and "||"
+            for (Token *tok = tokenlist.front(); tok; tok = tok->next()) {
+                if (tok->str() == "And")
+                    tok->str("&&");
+                else if (tok->str() == "Or")
+                    tok->str("||");
+            }
+
             tokenlist.createAst();
+
+            // Locate ast top and execute the condition
             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->astParent()) {
+                    return execute(tok->astTop(), p) == "True";
                 }
-                if (tok->str() == "==" && tok->astOperand1() && tok->astOperand2() && tok->astOperand1()->str() == tok->astOperand2()->str())
-                    return true;
             }
-            return false;
+
+            throw std::runtime_error("Invalid condition: '" + condition + "'");
         }
     private:
+
+        static std::string executeOp1(const Token* tok, const ProjectConfiguration &p, bool b=false) {
+            const std::string result = execute(tok->astOperand1(), p);
+            if (b)
+                return (result != "False" && !result.empty()) ? "True" : "False";
+            return result;
+        }
+
+        static std::string executeOp2(const Token* tok, const ProjectConfiguration &p, bool b=false) {
+            const std::string result = execute(tok->astOperand2(), p);
+            if (b)
+                return (result != "False" && !result.empty()) ? "True" : "False";
+            return result;
+        }
+
+        static std::string execute(const Token* tok, const ProjectConfiguration &p) {
+            if (!tok)
+                throw std::runtime_error("Missing operator");
+            auto boolResult = [](bool b) -> std::string { return b ? "True" : "False"; };
+            if (tok->isUnaryOp("!"))
+                return boolResult(executeOp1(tok, p, true) == "False");
+            if (tok->str() == "==")
+                return boolResult(executeOp1(tok, p) == executeOp2(tok, p));
+            if (tok->str() == "!=")
+                return boolResult(executeOp1(tok, p) != executeOp2(tok, p));
+            if (tok->str() == "&&")
+                return boolResult(executeOp1(tok, p, true) == "True" && executeOp2(tok, p, true) == "True");
+            if (tok->str() == "||")
+                return boolResult(executeOp1(tok, p, true) == "True" || executeOp2(tok, p, true) == "True");
+            if (tok->str() == "(" && Token::Match(tok->previous(), "$ ( %name% . %name% (")) {
+                const std::string propertyName = tok->next()->str();
+                std::string propertyValue;
+                if (propertyName == "Configuration")
+                    propertyValue = p.configuration;
+                else if (propertyName == "Platform")
+                    propertyValue = p.platform;
+                else
+                    throw std::runtime_error("Unhandled property '" + propertyName + "'");
+                const std::string method = tok->strAt(3);
+                std::string arg = executeOp2(tok->tokAt(4), p);
+                if (arg.size() >= 2 && arg[0] == '\'')
+                    arg = arg.substr(1, arg.size() - 2);
+                if (method == "Contains")
+                    return boolResult(propertyValue.find(arg) != std::string::npos);
+                if (method == "EndsWith")
+                    return boolResult(endsWith(propertyValue,arg.c_str(),arg.size()));
+                if (method == "StartsWith")
+                    return boolResult(startsWith(propertyValue,arg));
+                throw std::runtime_error("Unhandled method '" + method + "'");
+            }
+            if (tok->str().size() >= 2 && tok->str()[0] == '\'')
+                return tok->str();
+
+            throw std::runtime_error("Unknown/unhandled operator/operand '" + tok->str() + "'");
+        }
+
         std::string mCondition;
     };
 
@@ -879,7 +950,7 @@
             }
             std::string additionalIncludePaths;
             for (const ItemDefinitionGroup &i : itemDefinitionGroupList) {
-                if (!i.conditionIsTrue(p))
+                if (!i.conditionIsTrue(p, cfilename, errors))
                     continue;
                 fs.standard = Standards::getCPP(i.cppstd);
                 fs.defines += ';' + i.preprocessorDefinitions;
@@ -897,7 +968,7 @@
             }
             bool useUnicode = false;
             for (const ConfigurationPropertyGroup &c : configurationPropertyGroups) {
-                if (!c.conditionIsTrue(p))
+                if (!c.conditionIsTrue(p, cfilename, errors))
                     continue;
                 // in msbuild the last definition wins
                 useUnicode = c.useUnicode;
@@ -1554,3 +1625,14 @@
     }
 }
 
+// only used by tests (testimportproject.cpp::testVcxprojConditions):
+// 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;
+    std::vector<std::string> errors;
+    return ConditionalGroup::conditionIsTrue(condition, p, "file.vcxproj", errors) && errors.empty();
+}

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);
+    }
 }
 
 /**
@@ -191,6 +196,10 @@ namespace CppcheckXml {
     static constexpr char ProjectNameElementName[] = "project-name";
 }
 
+namespace testing
+{
+    CPPCHECKLIB bool evaluateVcxprojCondition(const std::string& condition, const std::string& configuration, const std::string& platform);
+}
 /// @}
 //---------------------------------------------------------------------------
 #endif // importprojectH

test/testimportproject.cpp

@@ -28,6 +28,7 @@
 #include <list>
 #include <map>
 #include <sstream>
+#include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
@@ -75,6 +76,7 @@ class TestImportProject : public TestFixture {
         TEST_CASE(importCppcheckGuiProjectPremiumMisra);
         TEST_CASE(ignorePaths);
         TEST_CASE(testVcxprojUnicode);
+        TEST_CASE(testVcxprojConditions);
     }
 
     void setDefines() const {
@@ -579,28 +581,37 @@ 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'  ) )", "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"));
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("And", "", ""), std::runtime_error, "Invalid condition: 'And'");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("Or", "", ""), std::runtime_error, "Invalid condition: 'Or'");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("!", "", ""), std::runtime_error, "Invalid condition: '!'");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '' And ", "", ""), std::runtime_error, "Missing operator");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("('' == ''", "", ""), std::runtime_error, "'(' without closing ')'!");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '')", "", ""), std::runtime_error, "unmatched ')' in condition '' == '')");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("''", "", ""), std::runtime_error, "Invalid condition: ''''");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '", "", ""), std::runtime_error, "Invalid condition: ''' == ''");
+        ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("$(Configuration.Lower())", "", ""), std::runtime_error, "Missing operator");
+    }
+
     // TODO: test fsParseCommand()
 
-    // TODO: test vcxproj conditions
-    /*
-        <?xml version="1.0" encoding="utf-8"?>
-        <Project DefaultTargets="Build" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
-        <ItemGroup Label="ProjectConfigurations">
-        <ProjectConfiguration Include="Debug|x64">
-        <Configuration>Debug</Configuration>
-        <Platform>x64</Platform>
-        </ProjectConfiguration>
-        </ItemGroup>
-        <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
-        <ClCompile>
-        <PreprocessorDefinitions>CPPCHECKLIB_IMPORT</PreprocessorDefinitions>
-        </ClCompile>
-        </ItemDefinitionGroup>
-        <ItemGroup>
-        <ClCompile Include="main.c" />
-        </ItemGroup>
-        </Project>
-     */
 };
 
 REGISTER_TEST(TestImportProject)