Skip to content

Commit 5cd4d50

Browse files
Leander SchultenLeander Schulten
Leander Schulten
authored and
Leander Schulten
committed
Fix #14334: Support more msbuild conditional constructs
1 parent ddce8d5 commit 5cd4d50

4 files changed

Lines changed: 227 additions & 55 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:1530 --suppress=unusedFunction:lib/importproject.cpp:1554"
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: 191 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <iostream>
3535
#include <iterator>
3636
#include <stack>
37+
#include <string>
3738
#include <unordered_set>
3839
#include <utility>
3940
#include <vector>
@@ -504,6 +505,7 @@ bool ImportProject::importSln(std::istream &istr, const std::string &path, const
504505

505506
namespace {
506507
struct ProjectConfiguration {
508+
ProjectConfiguration() = default;
507509
explicit ProjectConfiguration(const tinyxml2::XMLElement *cfg) {
508510
const char *a = cfg->Attribute("Include");
509511
if (a)
@@ -547,45 +549,189 @@ namespace {
547549
}
548550
}
549551

552+
/**
553+
* @brief evalExpression tries to evaluate the expression or throws an exception if it can not
554+
* @param expr The msbuild condition
555+
* @return the result of the expression
556+
* @throws std::runtime_error if the expression can not be evaluated
557+
*/
558+
static bool evalExpression(const std::string& expr /* should be a std::string_view */, const ProjectConfiguration& p)
559+
{
560+
const auto startsHere = [&](size_t& i, const std::string& string)
561+
{
562+
if (expr.compare(i, string.length(), string) == 0)
563+
{
564+
i += string.length();
565+
return true;
566+
}
567+
return false;
568+
};
569+
const auto skipSpaces = [&](size_t& i)
570+
{
571+
i = expr.find_first_not_of(' ', i);
572+
};
573+
const auto expectAfterSpaces = [&](size_t &i, const std::string& expected, const char* error)
574+
{
575+
skipSpaces(i);
576+
if (!startsHere(i, expected))
577+
{
578+
throw std::runtime_error(error);
579+
}
580+
};
581+
const auto parseString = [&](size_t& i)
582+
{
583+
expectAfterSpaces(i, "\'", "Expecting a start of a string!");
584+
const auto start = i;
585+
const auto end = expr.find('\'', i);
586+
if (end == std::string::npos)
587+
{
588+
throw std::runtime_error("Expecting an end of a string!");
589+
}
590+
std::string string = expr.substr(start, end - start);
591+
i = end + 1;
592+
return string;
593+
};
594+
bool currentVal = false; // should be std::optional<bool>
595+
bool initialized = false;
596+
for (std::size_t i = 0; i < expr.length();)
597+
{
598+
if (expr[i] == ' ')
599+
{
600+
++i;
601+
continue;
602+
}
603+
if (expr[i] == '!')
604+
{
605+
return !evalExpression(expr.substr(i + 1), p);
606+
}
607+
if (startsHere(i, "And"))
608+
{
609+
if (!initialized)
610+
{
611+
throw std::runtime_error("'And' without previous expression!");
612+
}
613+
return currentVal && evalExpression(expr.substr(i), p);
614+
}
615+
if (startsHere(i, "Or"))
616+
{
617+
if (!initialized)
618+
{
619+
throw std::runtime_error("'Or' without previous expression!");
620+
}
621+
return currentVal || evalExpression(expr.substr(i), p);
622+
}
623+
if (expr[i] == '(')
624+
{
625+
// find closing )
626+
int count = 1;
627+
bool inString = false;
628+
auto end = std::string::npos;
629+
for (int j = i + 1; j < expr.length(); ++j)
630+
{
631+
if (inString)
632+
{
633+
if (expr[j] == '\'')
634+
inString = false;
635+
}
636+
else if (expr[j] == '\'')
637+
{
638+
inString = true;
639+
}
640+
else if (expr[j] == '(')
641+
{
642+
++count;
643+
}
644+
else if (expr[j] == ')')
645+
{
646+
--count;
647+
if (count == 0)
648+
{
649+
end = j;
650+
break;
651+
}
652+
}
653+
}
654+
if (end == std::string::npos)
655+
{
656+
throw std::runtime_error("'(' without closing ')'!");
657+
}
658+
initialized = true;
659+
currentVal = evalExpression(expr.substr(i + 1, end - i - 1), p);
660+
i = end + 1;
661+
continue;
662+
}
663+
if (expr[i] == '\'')
664+
{
665+
auto left = parseString(i);
666+
skipSpaces(i);
667+
if (i + 4 >= expr.length())
668+
{
669+
throw std::runtime_error("Within a string comparison. We expect at least a =='' or !='' !");
670+
}
671+
bool equal = expr[i] == '=';
672+
i += 2;
673+
// expect a string now
674+
auto right = parseString(i);
675+
replaceAll(left, "$(Configuration)", p.configuration);
676+
replaceAll(left, "$(Platform)", p.platformStr);
677+
replaceAll(right, "$(Configuration)", p.configuration);
678+
replaceAll(right, "$(Platform)", p.platformStr);
679+
initialized = true;
680+
currentVal = equal ? left == right : left != right;
681+
continue;
682+
}
683+
if (startsHere(i, "$(Configuration."))
684+
{
685+
initialized = true;
686+
if (startsHere(i, "Contains"))
687+
{
688+
expectAfterSpaces(i, "(", "Expected start of call");
689+
auto containsParam = parseString(i);
690+
currentVal = p.configuration.find(containsParam) != std::string::npos;
691+
}
692+
else if (startsHere(i, "StartsWith"))
693+
{
694+
expectAfterSpaces(i, "(", "Expected start of call");
695+
auto startsWithParam = parseString(i);
696+
currentVal = p.configuration.find(startsWithParam) == 0;
697+
}
698+
else if (startsHere(i, "EndsWith"))
699+
{
700+
expectAfterSpaces(i, "(", "Expected start of call");
701+
auto endsWithParam = parseString(i);
702+
currentVal = p.configuration.rfind(endsWithParam) == p.configuration.length() - endsWithParam.length();
703+
}
704+
else
705+
{
706+
throw std::runtime_error("Unexpected function call!");
707+
}
708+
expectAfterSpaces(i, ")", "Expecting end of function call!");
709+
expectAfterSpaces(i, ")", "Expecting end of expression!");
710+
continue;
711+
}
712+
throw std::runtime_error("Unhandled expression!");
713+
}
714+
if (!initialized)
715+
{
716+
throw std::runtime_error("Expected expression here!");
717+
}
718+
return currentVal;
719+
}
720+
550721
// see https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-conditions
551722
// properties are .NET String objects and you can call any of its members on them
552-
bool conditionIsTrue(const ProjectConfiguration &p) const {
723+
bool conditionIsTrue(const ProjectConfiguration &p, std::vector<std::string> &errors, const std::string &filename) const {
553724
if (mCondition.empty())
554725
return true;
555-
std::string c = '(' + mCondition + ");";
556-
replaceAll(c, "$(Configuration)", p.configuration);
557-
replaceAll(c, "$(Platform)", p.platformStr);
558-
559-
// TODO: improve evaluation
560-
const Settings s;
561-
TokenList tokenlist(s, Standards::Language::C);
562-
tokenlist.createTokensFromBuffer(c.data(), c.size()); // TODO: check result
563-
// TODO: put in a helper
564-
// generate links
726+
try
565727
{
566-
std::stack<Token*> lpar;
567-
for (Token* tok2 = tokenlist.front(); tok2; tok2 = tok2->next()) {
568-
if (tok2->str() == "(")
569-
lpar.push(tok2);
570-
else if (tok2->str() == ")") {
571-
if (lpar.empty())
572-
break;
573-
Token::createMutualLinks(lpar.top(), tok2);
574-
lpar.pop();
575-
}
576-
}
728+
return evalExpression(mCondition, p);
577729
}
578-
tokenlist.createAst();
579-
for (const Token *tok = tokenlist.front(); tok; tok = tok->next()) {
580-
if (tok->str() == "(" && tok->astOperand1() && tok->astOperand2()) {
581-
// TODO: this is wrong - it is Contains() not Equals()
582-
if (tok->astOperand1()->expressionString() == "Configuration.Contains")
583-
return ('\'' + p.configuration + '\'') == tok->astOperand2()->str();
584-
}
585-
if (tok->str() == "==" && tok->astOperand1() && tok->astOperand2() && tok->astOperand1()->str() == tok->astOperand2()->str())
586-
return true;
730+
catch (const std::runtime_error& r)
731+
{
732+
errors.emplace_back(filename + ": Can not evaluate condition '" + mCondition + "': " + r.what());
733+
return false;
587734
}
588-
return false;
589735
}
590736
private:
591737
std::string mCondition;
@@ -658,6 +804,17 @@ namespace {
658804
};
659805
}
660806

807+
// only used by tests (testimportproject.cpp::testVcxprojConditions):
808+
// cppcheck-suppress unusedFunction
809+
bool cppcheck::testing::evaluateVcxprojCondition(const std::string& condition, const std::string& configuration,
810+
const std::string& platform)
811+
{
812+
ProjectConfiguration p;
813+
p.configuration = configuration;
814+
p.platformStr = platform;
815+
return ConditionalGroup::evalExpression(condition, p);
816+
}
817+
661818
static std::list<std::string> toStringList(const std::string &s)
662819
{
663820
std::list<std::string> ret;
@@ -893,7 +1050,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
8931050
}
8941051
std::string additionalIncludePaths;
8951052
for (const ItemDefinitionGroup &i : itemDefinitionGroupList) {
896-
if (!i.conditionIsTrue(p))
1053+
if (!i.conditionIsTrue(p, errors, cfilename))
8971054
continue;
8981055
fs.standard = Standards::getCPP(i.cppstd);
8991056
fs.defines += ';' + i.preprocessorDefinitions;
@@ -911,7 +1068,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X
9111068
}
9121069
bool useUnicode = false;
9131070
for (const ConfigurationPropertyGroup &c : configurationPropertyGroups) {
914-
if (!c.conditionIsTrue(p))
1071+
if (!c.conditionIsTrue(p, errors, cfilename))
9151072
continue;
9161073
// in msbuild the last definition wins
9171074
useUnicode = c.useUnicode;

‎lib/importproject.h‎

Lines changed: 5 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
/**

‎test/testimportproject.cpp‎

Lines changed: 30 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>
@@ -83,6 +84,7 @@ class TestImportProject : public TestFixture {
8384
TEST_CASE(testCollectArgs5);
8485
TEST_CASE(testCollectArgs6);
8586
TEST_CASE(testCollectArgs7);
87+
TEST_CASE(testVcxprojConditions);
8688
}
8789

8890
void setDefines() const {
@@ -680,28 +682,36 @@ class TestImportProject : public TestFixture {
680682
ASSERT_EQUALS("Missing closing quote in command string", error);
681683
}
682684

685+
void testVcxprojConditions() const
686+
{
687+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Debug'", "Debug", "Win32"));
688+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Platform)'=='Win32'", "Debug", "Win32"));
689+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)'=='Release'", "Debug", "Win32"));
690+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' ", "Debug", "Win32"));
691+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' != 'Debug' ", "Debug", "Win32"));
692+
ASSERT(cppcheck::testing::evaluateVcxprojCondition("'$(Configuration)|$(Platform)' == 'Debug|Win32' ", "Debug", "Win32"));
693+
ASSERT(!cppcheck::testing::evaluateVcxprojCondition("!('$(Configuration)|$(Platform)' == 'Debug|Win32' )", "Debug", "Win32"));
694+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' And '$(Platform)' == 'Win32'", "Debug", "Win32"));
695+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" '$(Configuration)' == 'Debug' Or '$(Platform)' == 'Win32'", "Release", "Win32"));
696+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.StartsWith('Debug'))", "Debug-AddressSanitizer", "Win32"));
697+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.EndsWith('AddressSanitizer'))", "Debug-AddressSanitizer", "Win32"));
698+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address'))", "Debug-AddressSanitizer", "Win32"));
699+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains ( 'Address' ) )", "Debug-AddressSanitizer", "Win32"));
700+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" $(Configuration.Contains('Address')) And '$(Platform)' == 'Win32'", "Debug-AddressSanitizer", "Win32"));
701+
ASSERT(cppcheck::testing::evaluateVcxprojCondition(" ($(Configuration.Contains('Address')) ) And ( '$(Platform)' == 'Win32')", "Debug-AddressSanitizer", "Win32"));
702+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("And", "", ""), std::runtime_error, "'And' without previous expression!");
703+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("Or", "", ""), std::runtime_error, "'Or' without previous expression!");
704+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("!", "", ""), std::runtime_error, "Expected expression here!");
705+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '' And ", "", ""), std::runtime_error, "Expected expression here!");
706+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("('' == ''", "", ""), std::runtime_error, "'(' without closing ')'!");
707+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '')", "", ""), std::runtime_error, "Unhandled expression!");
708+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("''", "", ""), std::runtime_error, "Within a string comparison. We expect at least a =='' or !='' !");
709+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '", "", ""), std::runtime_error, "Within a string comparison. We expect at least a =='' or !='' !");
710+
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("$(Configuration.Lower())", "", ""), std::runtime_error, "Unexpected function call!");
711+
}
712+
683713
// TODO: test fsParseCommand()
684714

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

707717
REGISTER_TEST(TestImportProject)

0 commit comments

Comments
 (0)