Skip to content

Commit dd99f0c

Browse files
autoantwortdanmarLeander Schulten
authored
Fix #14334 (Support more msbuild conditional constructs) (#8258)
Continuation of #8116 --------- Co-authored-by: Daniel Marjamäki <daniel.marjamaki@cppchecksolutions.com> Co-authored-by: Leander Schulten <Leander.Schulten@tetys.de>
1 parent e64454d commit dd99f0c

4 files changed

Lines changed: 143 additions & 36 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:197 --suppress=unusedFunction:lib/importproject.cpp:1584 --suppress=unusedFunction:lib/importproject.cpp:1608"
124+
supprs="--suppress=unusedFunction:lib/errorlogger.h:197 --suppress=unusedFunction:lib/importproject.cpp:1660 --suppress=unusedFunction:lib/importproject.cpp:1684"
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: 101 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ bool ImportProject::importSlnx(const std::string& filename, const std::vector<st
558558

559559
namespace {
560560
struct ProjectConfiguration {
561+
ProjectConfiguration() = default;
561562
explicit ProjectConfiguration(const tinyxml2::XMLElement *cfg) {
562563
const char *a = cfg->Attribute("Include");
563564
if (a)
@@ -603,18 +604,30 @@ namespace {
603604

604605
// see https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-conditions
605606
// properties are .NET String objects and you can call any of its members on them
606-
bool conditionIsTrue(const ProjectConfiguration &p) const {
607+
bool conditionIsTrue(const ProjectConfiguration &p, const std::string &filename, std::vector<std::string> &errors) const {
607608
if (mCondition.empty())
608609
return true;
609-
std::string c = '(' + mCondition + ");";
610+
try {
611+
return evalCondition(mCondition, p);
612+
}
613+
catch (const std::runtime_error& r)
614+
{
615+
errors.emplace_back(filename + ": Can not evaluate condition '" + mCondition + "': " + r.what());
616+
return false;
617+
}
618+
}
619+
620+
static bool evalCondition(const std::string& condition, const ProjectConfiguration &p) {
621+
std::string c = '(' + condition + ")";
610622
replaceAll(c, "$(Configuration)", p.configuration);
611623
replaceAll(c, "$(Platform)", p.platformStr);
612624

613-
// TODO: improve evaluation
614625
const Settings s;
615626
TokenList tokenlist(s, Standards::Language::C);
616-
tokenlist.createTokensFromBuffer(c.data(), c.size()); // TODO: check result
617-
// TODO: put in a helper
627+
if (!tokenlist.createTokensFromBuffer(c.data(), c.size())) {
628+
throw std::runtime_error("Can not tokenize condition");
629+
}
630+
618631
// generate links
619632
{
620633
std::stack<Token*> lpar;
@@ -623,25 +636,88 @@ namespace {
623636
lpar.push(tok2);
624637
else if (tok2->str() == ")") {
625638
if (lpar.empty())
626-
break;
639+
throw std::runtime_error("unmatched ')' in condition " + condition);
627640
Token::createMutualLinks(lpar.top(), tok2);
628641
lpar.pop();
629642
}
630643
}
644+
if (!lpar.empty())
645+
throw std::runtime_error("'(' without closing ')'!");
646+
}
647+
648+
// Replace "And" and "Or" with "&&" and "||"
649+
for (Token *tok = tokenlist.front(); tok; tok = tok->next()) {
650+
if (tok->str() == "And")
651+
tok->str("&&");
652+
else if (tok->str() == "Or")
653+
tok->str("||");
631654
}
655+
632656
tokenlist.createAst();
657+
658+
// Locate ast top and execute the condition
633659
for (const Token *tok = tokenlist.front(); tok; tok = tok->next()) {
634-
if (tok->str() == "(" && tok->astOperand1() && tok->astOperand2()) {
635-
// TODO: this is wrong - it is Contains() not Equals()
636-
if (tok->astOperand1()->expressionString() == "Configuration.Contains")
637-
return ('\'' + p.configuration + '\'') == tok->astOperand2()->str();
660+
if (tok->astParent()) {
661+
return execute(tok->astTop(), p) == "True";
638662
}
639-
if (tok->str() == "==" && tok->astOperand1() && tok->astOperand2() && tok->astOperand1()->str() == tok->astOperand2()->str())
640-
return true;
641663
}
642-
return false;
664+
throw std::runtime_error("Invalid condition: '" + condition + "'");
643665
}
666+
667+
644668
private:
669+
670+
static std::string executeOp1(const Token* tok, const ProjectConfiguration &p) {
671+
return execute(tok->astOperand1(), p);
672+
}
673+
674+
static std::string executeOp2(const Token* tok, const ProjectConfiguration &p) {
675+
return execute(tok->astOperand2(), p);
676+
}
677+
678+
static std::string execute(const Token* tok, const ProjectConfiguration &p) {
679+
if (!tok)
680+
throw std::runtime_error("Missing operator");
681+
auto boolResult = [](bool b) -> std::string {
682+
return b ? "True" : "False";
683+
};
684+
if (tok->isUnaryOp("!"))
685+
return boolResult(executeOp1(tok, p) == "False");
686+
if (tok->str() == "==")
687+
return boolResult(executeOp1(tok, p) == executeOp2(tok, p));
688+
if (tok->str() == "!=")
689+
return boolResult(executeOp1(tok, p) != executeOp2(tok, p));
690+
if (tok->str() == "&&")
691+
return boolResult(executeOp1(tok, p) == "True" && executeOp2(tok, p) == "True");
692+
if (tok->str() == "||")
693+
return boolResult(executeOp1(tok, p) == "True" || executeOp2(tok, p) == "True");
694+
if (tok->str() == "(" && Token::Match(tok->previous(), "$ ( %name% . %name% (")) {
695+
const std::string& propertyName = tok->strAt(1);
696+
std::string propertyValue;
697+
if (propertyName == "Configuration")
698+
propertyValue = p.configuration;
699+
else if (propertyName == "Platform")
700+
propertyValue = p.platformStr;
701+
else
702+
throw std::runtime_error("Unhandled property '" + propertyName + "'");
703+
const std::string& method = tok->strAt(3);
704+
std::string arg = executeOp2(tok->tokAt(4), p);
705+
if (arg.size() >= 2 && arg[0] == '\'')
706+
arg = arg.substr(1, arg.size() - 2);
707+
if (method == "Contains")
708+
return boolResult(propertyValue.find(arg) != std::string::npos);
709+
if (method == "EndsWith")
710+
return boolResult(endsWith(propertyValue,arg.c_str(),arg.size()));
711+
if (method == "StartsWith")
712+
return boolResult(startsWith(propertyValue,arg));
713+
throw std::runtime_error("Unhandled method '" + method + "'");
714+
}
715+
if (tok->str().size() >= 2 && tok->str()[0] == '\'') // String Literal
716+
return tok->str();
717+
718+
throw std::runtime_error("Unknown/unhandled operator/operand '" + tok->str() + "'");
719+
}
720+
645721
std::string mCondition;
646722
};
647723

@@ -947,7 +1023,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
9471023
}
9481024
std::string additionalIncludePaths;
9491025
for (const ItemDefinitionGroup &i : itemDefinitionGroupList) {
950-
if (!i.conditionIsTrue(p))
1026+
if (!i.conditionIsTrue(p, cfilename, errors))
9511027
continue;
9521028
fs.standard = Standards::getCPP(i.cppstd);
9531029
fs.defines += ';' + i.preprocessorDefinitions;
@@ -965,7 +1041,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
9651041
}
9661042
bool useUnicode = false;
9671043
for (const ConfigurationPropertyGroup &c : configurationPropertyGroups) {
968-
if (!c.conditionIsTrue(p))
1044+
if (!c.conditionIsTrue(p, cfilename, errors))
9691045
continue;
9701046
// in msbuild the last definition wins
9711047
useUnicode = c.useUnicode;
@@ -1622,3 +1698,13 @@ void ImportProject::setRelativePaths(const std::string &filename)
16221698
}
16231699
}
16241700

1701+
// only used by tests (testimportproject.cpp::testVcxprojConditions):
1702+
// cppcheck-suppress unusedFunction
1703+
bool cppcheck::testing::evaluateVcxprojCondition(const std::string& condition, const std::string& configuration,
1704+
const std::string& platform)
1705+
{
1706+
ProjectConfiguration p;
1707+
p.configuration = configuration;
1708+
p.platformStr = platform;
1709+
return ConditionalGroup::evalCondition(condition, p);
1710+
}

‎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
/**
@@ -196,6 +201,10 @@ namespace CppcheckXml {
196201
static constexpr char ProjectNameElementName[] = "project-name";
197202
}
198203

204+
namespace testing
205+
{
206+
CPPCHECKLIB bool evaluateVcxprojCondition(const std::string& condition, const std::string& configuration, const std::string& platform);
207+
}
199208
/// @}
200209
//---------------------------------------------------------------------------
201210
#endif // importprojectH

‎test/testimportproject.cpp‎

Lines changed: 32 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>
@@ -85,6 +86,7 @@ class TestImportProject : public TestFixture {
8586
TEST_CASE(testCollectArgs5);
8687
TEST_CASE(testCollectArgs6);
8788
TEST_CASE(testCollectArgs7);
89+
TEST_CASE(testVcxprojConditions);
8890
}
8991

9092
void setDefines() const {
@@ -682,28 +684,38 @@ class TestImportProject : public TestFixture {
682684
ASSERT_EQUALS("Missing closing quote in command string", error);
683685
}
684686

687+
void testVcxprojConditions() const
688+
{
689+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Debug'", "Debug", "Win32"));
690+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Platform)'=='Win32'", "Debug", "Win32"));
691+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Release'", "Debug", "Win32"));
692+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' ", "Debug", "Win32"));
693+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' != 'Debug' ", "Debug", "Win32"));
694+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)|$(Platform)' == 'Debug|Win32' ", "Debug", "Win32"));
695+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition("!('$(Configuration)|$(Platform)' == 'Debug|Win32' )", "Debug", "Win32"));
696+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' And '$(Platform)' == 'Win32'", "Debug", "Win32"));
697+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' Or '$(Platform)' == 'Win32'", "Release", "Win32"));
698+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.StartsWith('Debug'))", "Debug-AddressSanitizer", "Win32"));
699+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.EndsWith('AddressSanitizer'))", "Debug-AddressSanitizer", "Win32"));
700+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address'))", "Debug-AddressSanitizer", "Win32"));
701+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains ( 'Address' ) )", "Debug-AddressSanitizer", "Win32"));
702+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address')) And '$(Platform)' == 'Win32'", "Debug-AddressSanitizer", "Win32"));
703+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" ($(Configuration.Contains('Address')) ) And ( '$(Platform)' == 'Win32')", "Debug-AddressSanitizer", "Win32"));
704+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("And", "", ""), std::runtime_error, "Invalid condition: 'And'");
705+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("Or", "", ""), std::runtime_error, "Invalid condition: 'Or'");
706+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("!", "", ""), std::runtime_error, "Invalid condition: '!'");
707+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("'' == '' And ", "", ""), std::runtime_error, "Missing operator");
708+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("('' == ''", "", ""), std::runtime_error, "'(' without closing ')'!");
709+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("'' == '')", "", ""), std::runtime_error, "unmatched ')' in condition '' == '')");
710+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("''", "", ""), std::runtime_error, "Invalid condition: ''''");
711+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("'' == '", "", ""), std::runtime_error, "Can not tokenize condition");
712+
ASSERT_THROW_EQUALS(cppcheck::testing::evaluateVcxprojCondition("$(Configuration.Lower())", "", ""), std::runtime_error, "Missing operator");
713+
// invalid expression in => no error. We are ok with that as long as we don't crash
714+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition("' ' && ' '", "", ""));
715+
}
716+
685717
// TODO: test fsParseCommand()
686718

687-
// TODO: test vcxproj conditions
688-
/*
689-
<?xml version="1.0" encoding="utf-8"?>
690-
<Project DefaultTargets="Build" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
691-
<ItemGroup Label="ProjectConfigurations">
692-
<ProjectConfiguration Include="Debug|x64">
693-
<Configuration>Debug</Configuration>
694-
<Platform>x64</Platform>
695-
</ProjectConfiguration>
696-
</ItemGroup>
697-
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
698-
<ClCompile>
699-
<PreprocessorDefinitions>CPPCHECKLIB_IMPORT</PreprocessorDefinitions>
700-
</ClCompile>
701-
</ItemDefinitionGroup>
702-
<ItemGroup>
703-
<ClCompile Include="main.c" />
704-
</ItemGroup>
705-
</Project>
706-
*/
707719
};
708720

709721
REGISTER_TEST(TestImportProject)

0 commit comments

Comments
 (0)