diff --git a/.github/workflows/selfcheck.yml b/.github/workflows/selfcheck.yml index 3a2fd2fffc0..c8baa29f621 100644 --- a/.github/workflows/selfcheck.yml +++ b/.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 diff --git a/lib/importproject.cpp b/lib/importproject.cpp index acd7842224f..e2784237de2 100644 --- a/lib/importproject.cpp +++ b/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) @@ -535,10 +536,14 @@ namespace { // 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 &errors) const { + return conditionIsTrue(mCondition, p, filename, errors); + } + + static bool conditionIsTrue(const std::string& condition, const ProjectConfiguration &p, const std::string &filename, std::vector &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 @@ namespace { 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 @@ 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, cfilename, errors)) continue; fs.standard = Standards::getCPP(i.cppstd); fs.defines += ';' + i.preprocessorDefinitions; @@ -897,7 +968,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, cfilename, errors)) continue; // in msbuild the last definition wins useUnicode = c.useUnicode; @@ -1554,3 +1625,14 @@ void ImportProject::setRelativePaths(const std::string &filename) } } +// 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 errors; + return ConditionalGroup::conditionIsTrue(condition, p, "file.vcxproj", errors) && errors.empty(); +} \ No newline at end of file diff --git a/lib/importproject.h b/lib/importproject.h index 45d17a819f7..68d175df6c3 100644 --- a/lib/importproject.h +++ b/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 diff --git a/test/testimportproject.cpp b/test/testimportproject.cpp index 5e6db0163c8..664e15b57b3 100644 --- a/test/testimportproject.cpp +++ b/test/testimportproject.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -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 - /* - - - - - Debug - x64 - - - - - CPPCHECKLIB_IMPORT - - - - - - - */ }; REGISTER_TEST(TestImportProject)