Skip to content

Fix #14334 (Support more msbuild conditional constructs) #8116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danmar wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix #14334 (Support more msbuild conditional constructs) #8116

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a9867f
Fix #14334 (Support more msbuild conditional constructs)
danmar Jan 13, 2026
c4b0e57
fix
danmar Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/selfcheck.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
104 changes: 92 additions & 12 deletions lib/importproject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@

namespace {
struct ProjectConfiguration {
ProjectConfiguration() = default;
explicit ProjectConfiguration(const tinyxml2::XMLElement *cfg) {
const char *a = cfg->Attribute("Include");
if (a)
Expand Down Expand Up @@ -535,10 +536,14 @@

// 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<std::string> &errors) const {
return conditionIsTrue(mCondition, p, filename, errors);
}

static bool conditionIsTrue(const std::string& condition, const ProjectConfiguration &p, const std::string &filename, std::vector<std::string> &errors) {
Comment thread Dismissed
if (condition.empty())
return true;
std::string c = '(' + mCondition + ");";
std::string c = '(' + condition + ");";
replaceAll(c, "$(Configuration)", p.configuration);
replaceAll(c, "$(Platform)", p.platformStr);

Expand All @@ -561,19 +566,83 @@
}
}
}

// 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("||");
}
Comment on lines +573 to +578
Copy link
Copy Markdown
Contributor

@autoantwort autoantwort Feb 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that your goal is according to false positives: But && and || are not valid MSBuild conditionals.
I am fine with that. Imho wrong input -> wrong output is ok here.


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");
Comment on lines +611 to +620
Copy link
Copy Markdown
Contributor

@autoantwort autoantwort Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows something like ' ' Or ' ' which is not valid msbuild.
I am fine with that because people only use valid msbuild with cppcheck.

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;
};

Expand Down Expand Up @@ -879,7 +948,7 @@
}
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;
Expand All @@ -897,7 +966,7 @@
}
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;
Expand Down Expand Up @@ -1554,3 +1623,14 @@
}
}

// 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<std::string> errors;
return ConditionalGroup::conditionIsTrue(condition, p, "file.vcxproj", errors) && errors.empty();
}
9 changes: 9 additions & 0 deletions lib/importproject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

/**
Expand Down Expand Up @@ -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
51 changes: 31 additions & 20 deletions test/testimportproject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <list>
#include <map>
#include <sstream>
#include <stdexcept>
#include <string>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -75,6 +76,7 @@ class TestImportProject : public TestFixture {
TEST_CASE(importCppcheckGuiProjectPremiumMisra);
TEST_CASE(ignorePaths);
TEST_CASE(testVcxprojUnicode);
TEST_CASE(testVcxprojConditions);
}

void setDefines() const {
Expand Down Expand Up @@ -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, "'And' without previous expression!");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("Or", "", ""), std::runtime_error, "'Or' without previous expression!");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("!", "", ""), std::runtime_error, "Expected expression here!");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '' And ", "", ""), std::runtime_error, "Expected expression here!");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("('' == ''", "", ""), std::runtime_error, "'(' without closing ')'!");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '')", "", ""), std::runtime_error, "Unhandled expression!");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("''", "", ""), std::runtime_error, "Within a string comparison. We expect at least a =='' or !='' !");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("'' == '", "", ""), std::runtime_error, "Within a string comparison. We expect at least a =='' or !='' !");
ASSERT_THROW_EQUALS_2(cppcheck::testing::evaluateVcxprojCondition("$(Configuration.Lower())", "", ""), std::runtime_error, "Unexpected function call!");
}

// TODO: test fsParseCommand()

// TODO: test vcxproj conditions
/*
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<ItemGroup Label="ProjectConfigurations">
<ProjectConfiguration Include="Debug|x64">
<Configuration>Debug</Configuration>
<Platform>x64</Platform>
</ProjectConfiguration>
</ItemGroup>
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
<ClCompile>
<PreprocessorDefinitions>CPPCHECKLIB_IMPORT</PreprocessorDefinitions>
</ClCompile>
</ItemDefinitionGroup>
<ItemGroup>
<ClCompile Include="main.c" />
</ItemGroup>
</Project>
*/
};

REGISTER_TEST(TestImportProject)
Loading