diff --git a/.github/workflows/selfcheck.yml b/.github/workflows/selfcheck.yml index 804de21a0b4..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:197 --suppress=unusedFunction:lib/importproject.cpp:1531 --suppress=unusedFunction:lib/importproject.cpp:1555" + 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 9706cd5c6b2..9a7eefc3808 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -505,6 +505,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) @@ -550,18 +551,30 @@ 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 { + bool conditionIsTrue(const ProjectConfiguration &p, const std::string &filename, std::vector &errors) const { if (mCondition.empty()) return true; - std::string c = '(' + mCondition + ");"; + try { + return evalCondition(mCondition, p); + } + catch (const std::runtime_error& r) + { + errors.emplace_back(filename + ": Can not evaluate condition '" + mCondition + "': " + r.what()); + return false; + } + } + + static bool evalCondition(const std::string& condition, const ProjectConfiguration &p) { + std::string c = '(' + condition + ")"; 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 + if (!tokenlist.createTokensFromBuffer(c.data(), c.size())) { + throw std::runtime_error("Can not tokenize condition"); + } + // generate links { std::stack lpar; @@ -570,25 +583,86 @@ 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) { + return execute(tok->astOperand1(), p); + } + + static std::string executeOp2(const Token* tok, const ProjectConfiguration &p) { + return execute(tok->astOperand2(), p); + } + + 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) == "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" && executeOp2(tok, p) == "True"); + if (tok->str() == "||") + return boolResult(executeOp1(tok, p) == "True" || executeOp2(tok, p) == "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.platformStr; + 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] == '\'') // String Literal + return tok->str(); + + throw std::runtime_error("Unknown/unhandled operator/operand '" + tok->str() + "'"); + } + std::string mCondition; }; @@ -894,7 +968,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; @@ -912,7 +986,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; @@ -1569,3 +1643,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::evalCondition(condition, p); +} diff --git a/lib/importproject.h b/lib/importproject.h index 45a8dbac8b1..9e36568e2bf 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); + } } /** @@ -194,6 +199,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 c3bee4dcfa6..689e39f228f 100644 --- a/test/testimportproject.cpp +++ b/test/testimportproject.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -85,6 +86,7 @@ class TestImportProject : public TestFixture { TEST_CASE(testCollectArgs5); TEST_CASE(testCollectArgs6); TEST_CASE(testCollectArgs7); + TEST_CASE(testVcxprojConditions); } void setDefines() const { @@ -682,28 +684,37 @@ class TestImportProject : public TestFixture { ASSERT_EQUALS("Missing closing quote in command string", error); } + 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(cppcheck::testing::evaluateVcxprojCondition("And", "", ""), std::runtime_error, "Invalid condition: 'And'"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("Or", "", ""), std::runtime_error, "Invalid condition: 'Or'"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("!", "", ""), std::runtime_error, "Invalid condition: '!'"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("'' == '' And ", "", ""), std::runtime_error, "Missing operator"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("('' == ''", "", ""), std::runtime_error, "'(' without closing ')'!"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("'' == '')", "", ""), std::runtime_error, "unmatched ')' in condition '' == '')"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("''", "", ""), std::runtime_error, "Invalid condition: ''''"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("'' == '", "", ""), std::runtime_error, "Can not tokenize condition"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("$(Configuration.Lower())", "", ""), std::runtime_error, "Missing operator"); + ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("' ' && ' '", "", ""), std::runtime_error, "Missing operator"); + } + // TODO: test fsParseCommand() - // TODO: test vcxproj conditions - /* - - - - - Debug - x64 - - - - - CPPCHECKLIB_IMPORT - - - - - - - */ }; REGISTER_TEST(TestImportProject)