Skip to content

Commit 4a9867f

Browse files
danmardanmar
committed
Fix #14334 (Support more msbuild conditional constructs)
1 parent 34b9c45 commit 4a9867f

4 files changed

Lines changed: 133 additions & 33 deletions

File tree

‎.github/workflows/selfcheck.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ jobs:
121121
122122
- name: Self check (unusedFunction / no test / no gui)
123123
run: |
124-
supprs="--suppress=unusedFunction:lib/errorlogger.h:196 --suppress=unusedFunction:lib/importproject.cpp:1516 --suppress=unusedFunction:lib/importproject.cpp:1540"
124+
supprs="--suppress=unusedFunction:lib/errorlogger.h:196 --suppress=unusedFunction:lib/importproject.cpp:1673 --suppress=unusedFunction:lib/importproject.cpp:1697"
125125
./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
126126
env:
127127
DISABLE_VALUEFLOW: 1

‎lib/importproject.cpp‎

Lines changed: 92 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ bool ImportProject::importSln(std::istream &istr, const std::string &path, const
490490

491491
namespace {
492492
struct ProjectConfiguration {
493+
ProjectConfiguration() = default;
493494
explicit ProjectConfiguration(const tinyxml2::XMLElement *cfg) {
494495
const char *a = cfg->Attribute("Include");
495496
if (a)
@@ -535,10 +536,14 @@ namespace {
535536

536537
// see https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-conditions
537538
// properties are .NET String objects and you can call any of its members on them
538-
bool conditionIsTrue(const ProjectConfiguration &p) const {
539-
if (mCondition.empty())
539+
bool conditionIsTrue(const ProjectConfiguration &p, const std::string &filename, std::vector<std::string> &errors) const {
540+
return conditionIsTrue(mCondition, p, filename, errors);
541+
}
542+
543+
static bool conditionIsTrue(const std::string& condition, const ProjectConfiguration &p, const std::string &filename, std::vector<std::string> &errors) {
544+
if (condition.empty())
540545
return true;
541-
std::string c = '(' + mCondition + ");";
546+
std::string c = '(' + condition + ");";
542547
replaceAll(c, "$(Configuration)", p.configuration);
543548
replaceAll(c, "$(Platform)", p.platformStr);
544549

@@ -561,19 +566,83 @@ namespace {
561566
}
562567
}
563568
}
569+
570+
// Replace "And" and "Or" with "&&" and "||"
571+
for (Token *tok = tokenlist.front(); tok; tok = tok->next()) {
572+
if (tok->str() == "And")
573+
tok->str("&&");
574+
else if (tok->str() == "Or")
575+
tok->str("||");
576+
}
577+
564578
tokenlist.createAst();
579+
580+
// Locate ast top and execute the condition
565581
for (const Token *tok = tokenlist.front(); tok; tok = tok->next()) {
566-
if (tok->str() == "(" && tok->astOperand1() && tok->astOperand2()) {
567-
// TODO: this is wrong - it is Contains() not Equals()
568-
if (tok->astOperand1()->expressionString() == "Configuration.Contains")
569-
return ('\'' + p.configuration + '\'') == tok->astOperand2()->str();
582+
if (tok->astParent()) {
583+
return execute(tok->astTop(), p) == "True";
570584
}
571-
if (tok->str() == "==" && tok->astOperand1() && tok->astOperand2() && tok->astOperand1()->str() == tok->astOperand2()->str())
572-
return true;
573585
}
574-
return false;
586+
587+
throw std::runtime_error("Invalid condition: '" + condition + "'");
575588
}
576589
private:
590+
591+
static std::string executeOp1(const Token* tok, const ProjectConfiguration &p, bool b=false) {
592+
const std::string result = execute(tok->astOperand1(), p);
593+
if (b)
594+
return (result != "False" && !result.empty()) ? "True" : "False";
595+
return result;
596+
}
597+
598+
static std::string executeOp2(const Token* tok, const ProjectConfiguration &p, bool b=false) {
599+
const std::string result = execute(tok->astOperand2(), p);
600+
if (b)
601+
return (result != "False" && !result.empty()) ? "True" : "False";
602+
return result;
603+
}
604+
605+
static std::string execute(const Token* tok, const ProjectConfiguration &p) {
606+
if (!tok)
607+
throw std::runtime_error("Missing operator");
608+
auto boolResult = [](bool b) -> std::string { return b ? "True" : "False"; };
609+
if (tok->isUnaryOp("!"))
610+
return boolResult(executeOp1(tok, p, true) == "False");
611+
if (tok->str() == "==")
612+
return boolResult(executeOp1(tok, p) == executeOp2(tok, p));
613+
if (tok->str() == "!=")
614+
return boolResult(executeOp1(tok, p) != executeOp2(tok, p));
615+
if (tok->str() == "&&")
616+
return boolResult(executeOp1(tok, p, true) == "True" && executeOp2(tok, p, true) == "True");
617+
if (tok->str() == "||")
618+
return boolResult(executeOp1(tok, p, true) == "True" || executeOp2(tok, p, true) == "True");
619+
if (tok->str() == "(" && Token::Match(tok->previous(), "$ ( %name% . %name% (")) {
620+
const std::string propertyName = tok->next()->str();
621+
std::string propertyValue;
622+
if (propertyName == "Configuration")
623+
propertyValue = p.configuration;
624+
else if (propertyName == "Platform")
625+
propertyValue = p.platform;
626+
else
627+
throw std::runtime_error("Unhandled property '" + propertyName + "'");
628+
const std::string method = tok->strAt(3);
629+
std::string arg = executeOp2(tok->tokAt(4), p);
630+
if (arg.size() >= 2 && arg[0] == '\'')
631+
arg = arg.substr(1, arg.size() - 2);
632+
if (method == "Contains")
633+
return boolResult(propertyValue.find(arg) != std::string::npos);
634+
if (method == "EndsWith")
635+
return boolResult(endsWith(propertyValue,arg.c_str(),arg.size()));
636+
if (method == "StartsWith")
637+
return boolResult(startsWith(propertyValue,arg));
638+
throw std::runtime_error("Unhandled method '" + method + "'");
639+
}
640+
if (tok->str().size() >= 2 && tok->str()[0] == '\'')
641+
return tok->str();
642+
643+
throw std::runtime_error("Unknown/unhandled operator/operand '" + tok->str() + "'");
644+
}
645+
577646
std::string mCondition;
578647
};
579648

@@ -879,7 +948,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
879948
}
880949
std::string additionalIncludePaths;
881950
for (const ItemDefinitionGroup &i : itemDefinitionGroupList) {
882-
if (!i.conditionIsTrue(p))
951+
if (!i.conditionIsTrue(p, cfilename, errors))
883952
continue;
884953
fs.standard = Standards::getCPP(i.cppstd);
885954
fs.defines += ';' + i.preprocessorDefinitions;
@@ -897,7 +966,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
897966
}
898967
bool useUnicode = false;
899968
for (const ConfigurationPropertyGroup &c : configurationPropertyGroups) {
900-
if (!c.conditionIsTrue(p))
969+
if (!c.conditionIsTrue(p, cfilename, errors))
901970
continue;
902971
// in msbuild the last definition wins
903972
useUnicode = c.useUnicode;
@@ -1554,3 +1623,14 @@ void ImportProject::setRelativePaths(const std::string &filename)
15541623
}
15551624
}
15561625

1626+
// only used by tests (testimportproject.cpp::testVcxprojConditions):
1627+
// cppcheck-suppress unusedFunction
1628+
bool cppcheck::testing::evaluateVcxprojCondition(const std::string& condition, const std::string& configuration,
1629+
const std::string& platform)
1630+
{
1631+
ProjectConfiguration p;
1632+
p.configuration = configuration;
1633+
p.platformStr = platform;
1634+
std::vector<std::string> errors;
1635+
return ConditionalGroup::conditionIsTrue(condition, p, "file.vcxproj", errors) && errors.empty();
1636+
}

‎lib/importproject.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ namespace cppcheck {
4949
return caseInsensitiveStringCompare(lhs,rhs) < 0;
5050
}
5151
};
52+
53+
namespace testing
54+
{
55+
CPPCHECKLIB bool evaluateVcxprojCondition(const std::string& condition, const std::string& configuration, const std::string& platform);
56+
}
5257
}
5358

5459
/**
@@ -191,6 +196,10 @@ namespace CppcheckXml {
191196
static constexpr char ProjectNameElementName[] = "project-name";
192197
}
193198

199+
namespace testing
200+
{
201+
CPPCHECKLIB bool evaluateVcxprojCondition(const std::string& condition, const std::string& configuration, const std::string& platform);
202+
}
194203
/// @}
195204
//---------------------------------------------------------------------------
196205
#endif // importprojectH

‎test/testimportproject.cpp‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <list>
2929
#include <map>
3030
#include <sstream>
31+
#include <stdexcept>
3132
#include <string>
3233
#include <utility>
3334
#include <vector>
@@ -75,6 +76,7 @@ class TestImportProject : public TestFixture {
7576
TEST_CASE(importCppcheckGuiProjectPremiumMisra);
7677
TEST_CASE(ignorePaths);
7778
TEST_CASE(testVcxprojUnicode);
79+
TEST_CASE(testVcxprojConditions);
7880
}
7981

8082
void setDefines() const {
@@ -579,28 +581,37 @@ class TestImportProject : public TestFixture {
579581
ASSERT_EQUALS(project.fileSettings.back().useMfc, true);
580582
}
581583

584+
585+
void testVcxprojConditions() const
586+
{
587+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Debug'", "Debug", "Win32"));
588+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Platform)'=='Win32'", "Debug", "Win32"));
589+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Release'", "Debug", "Win32"));
590+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' ", "Debug", "Win32"));
591+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' != 'Debug' ", "Debug", "Win32"));
592+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)|$(Platform)' == 'Debug|Win32' ", "Debug", "Win32"));
593+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition("!('$(Configuration)|$(Platform)' == 'Debug|Win32' )", "Debug", "Win32"));
594+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' And '$(Platform)' == 'Win32'", "Debug", "Win32"));
595+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' Or '$(Platform)' == 'Win32'", "Release", "Win32"));
596+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.StartsWith('Debug'))", "Debug-AddressSanitizer", "Win32"));
597+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.EndsWith('AddressSanitizer'))", "Debug-AddressSanitizer", "Win32"));
598+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address'))", "Debug-AddressSanitizer", "Win32"));
599+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains ( 'Address' ) )", "Debug-AddressSanitizer", "Win32"));
600+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address')) And '$(Platform)' == 'Win32'", "Debug-AddressSanitizer", "Win32"));
601+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" ($(Configuration.Contains('Address')) ) And ( '$(Platform)' == 'Win32')", "Debug-AddressSanitizer", "Win32"));
602+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("And", "", ""), std::runtime_error, "'And' without previous expression!");
603+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("Or", "", ""), std::runtime_error, "'Or' without previous expression!");
604+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("!", "", ""), std::runtime_error, "Expected expression here!");
605+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '' And ", "", ""), std::runtime_error, "Expected expression here!");
606+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("('' == ''", "", ""), std::runtime_error, "'(' without closing ')'!");
607+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '')", "", ""), std::runtime_error, "Unhandled expression!");
608+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("''", "", ""), std::runtime_error, "Within a string comparison. We expect at least a =='' or !='' !");
609+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '", "", ""), std::runtime_error, "Within a string comparison. We expect at least a =='' or !='' !");
610+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("$(Configuration.Lower())", "", ""), std::runtime_error, "Unexpected function call!");
611+
}
612+
582613
// TODO: test fsParseCommand()
583614

584-
// TODO: test vcxproj conditions
585-
/*
586-
<?xml version="1.0" encoding="utf-8"?>
587-
<Project DefaultTargets="Build" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
588-
<ItemGroup Label="ProjectConfigurations">
589-
<ProjectConfiguration Include="Debug|x64">
590-
<Configuration>Debug</Configuration>
591-
<Platform>x64</Platform>
592-
</ProjectConfiguration>
593-
</ItemGroup>
594-
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
595-
<ClCompile>
596-
<PreprocessorDefinitions>CPPCHECKLIB_IMPORT</PreprocessorDefinitions>
597-
</ClCompile>
598-
</ItemDefinitionGroup>
599-
<ItemGroup>
600-
<ClCompile Include="main.c" />
601-
</ItemGroup>
602-
</Project>
603-
*/
604615
};
605616

606617
REGISTER_TEST(TestImportProject)

0 commit comments

Comments
 (0)