Skip to content

fixed #13939 - look for platform file relative to project file (first) with CLI #7612

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.

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

Merged
merged 1 commit into from
Jun 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 39 additions & 30 deletions cli/cmdlineparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,11 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
ImportProject project;
std::string vsConfig;

std::string platform;
char defaultSign = '\0';

std::vector<std::string> lookupPaths{argv[0]};

bool executorAuto = true;

for (int i = 1; i < argc; i++) {
Expand Down Expand Up @@ -847,10 +852,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
mSettings.force = true;

else if (std::strcmp(argv[i], "--fsigned-char") == 0)
mSettings.platform.defaultSign = 's';
defaultSign = 's';

else if (std::strcmp(argv[i], "--funsigned-char") == 0)
mSettings.platform.defaultSign = 'u';
defaultSign = 'u';

// Ignored paths
else if (std::strncmp(argv[i], "-i", 2) == 0) {
Expand Down Expand Up @@ -1098,26 +1103,12 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a

// Specify platform
else if (std::strncmp(argv[i], "--platform=", 11) == 0) {
const std::string platform(11+argv[i]);

std::string errstr;
const std::vector<std::string> paths = {argv[0]};
if (!mSettings.platform.set(platform, errstr, paths, mSettings.debuglookup || mSettings.debuglookupPlatform)) {
mLogger.printError(errstr);
std::string p = 11 + argv[i];
if (p.empty()) {
mLogger.printError("empty platform specified.");
return Result::Fail;
}

// TODO: remove
// these are loaded via external files and thus have Settings::PlatformFile set instead.
// override the type so they behave like the regular platforms.
if (platform == "unix32-unsigned") {
mSettings.platform.type = Platform::Type::Unix32;
mLogger.printMessage("The platform 'unix32-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix32 --funsigned-char' instead");
}
else if (platform == "unix64-unsigned") {
mSettings.platform.type = Platform::Type::Unix64;
mLogger.printMessage("The platform 'unix64-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix64 --funsigned-char' instead");
}
platform = std::move(p);
}

// Write results in results.plist
Expand Down Expand Up @@ -1210,17 +1201,11 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
const auto& excludedPaths = project.guiProject.excludedPaths;
std::copy(excludedPaths.cbegin(), excludedPaths.cend(), std::back_inserter(mIgnoredPaths));

std::string platform(project.guiProject.platform);
if (!project.guiProject.platform.empty())
platform = project.guiProject.platform;

// keep existing platform from command-line intact
if (!platform.empty()) {
std::string errstr;
const std::vector<std::string> paths = {projectFile, argv[0]};
if (!mSettings.platform.set(platform, errstr, paths, mSettings.debuglookup || mSettings.debuglookupPlatform)) {
mLogger.printError(errstr);
return Result::Fail;
}
}
// look for external files relative to project first
lookupPaths.insert(lookupPaths.cbegin(), projectFile);

const auto& projectFileGui = project.guiProject.projectFile;
if (!projectFileGui.empty()) {
Expand Down Expand Up @@ -1641,6 +1626,30 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
project.ignoreOtherConfigs(vsConfig);
}

if (!platform.empty())
{
std::string errstr;
if (!mSettings.platform.set(platform, errstr, lookupPaths, mSettings.debuglookup || mSettings.debuglookupPlatform)) {
mLogger.printError(errstr);
return Result::Fail;
}

// TODO: remove
// these are loaded via external files and thus have Settings::PlatformFile set instead.
// override the type so they behave like the regular platforms.
if (platform == "unix32-unsigned") {
mSettings.platform.type = Platform::Type::Unix32;
mLogger.printMessage("The platform 'unix32-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix32 --funsigned-char' instead");
}
else if (platform == "unix64-unsigned") {
mSettings.platform.type = Platform::Type::Unix64;
mLogger.printMessage("The platform 'unix64-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix64 --funsigned-char' instead");
}
}

if (defaultSign != '\0')
mSettings.platform.defaultSign = defaultSign;
Comment on lines +1650 to +1651
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This does not reflect the left-to-right processing as the imported project might change this(?). Even more reason for #7293.

Also make me wonder if this flag could be set per file and thus needs to life inside FileSettings instead of the platform after all...

Copy link
Owner

@danmar danmar Jun 21, 2025

Choose a reason for hiding this comment

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

Also make me wonder if this flag could be set per file and thus needs to life inside FileSettings instead of the platform after all...

I misunderstood first..

I guess you mean that some files could be compiled with -funsigned-char and other files could be compiled with -fsigned-char? That's weird but not impossible.

I'm not sure if we need to handle this case properly.

The defaultSign do belong in platform in my head.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah. that seems to be a edge case to me. I am not 100% sure if we need to support analysing multi-platform builds in 1 analysis.

That is the misconception. The signedness of char has nothing to do with the platform. It is dependent on the compiler and can be controlled individually with a CLI option. It is like specifying the language or standard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We already set the platform type per file so this would be akin to that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Either way it is out of the scope of this PR.


if (!mSettings.analyzeAllVsConfigs) {
if (projectType != ImportProject::Type::VS_SLN && projectType != ImportProject::Type::VS_VCXPROJ) {
if (mAnalyzeAllVsConfigsSetOnCmdLine) {
Expand Down
20 changes: 13 additions & 7 deletions test/cli/lookup_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_lib_lookup_notfound_project(tmpdir): # #13938
]


def test_lib_lookup_notfound_compdb(tmpdir): # #13938
def test_lib_lookup_notfound_compdb(tmpdir):
compdb_file, _ = __create_compdb(tmpdir)

exitcode, stdout, _, exe = cppcheck_ex(['--debug-lookup=library', '--library=none', '--project={}'.format(compdb_file)])
Expand All @@ -144,7 +144,6 @@ def test_lib_lookup_notfound_compdb(tmpdir): # #13938
assert exitcode == 1, stdout
lines = __remove_std_lookup_log(stdout.splitlines(), exepath)
assert lines == [
# TODO: needs to look relative to the project first
# TODO: specify which folder is actually used for lookup here
"looking for library 'none.cfg'",
"looking for library '{}/none.cfg'".format(exepath),
Expand Down Expand Up @@ -409,18 +408,27 @@ def test_platform_lookup_notfound(tmpdir):

def test_platform_lookup_notfound_project(tmpdir): # #13939
project_file, _ = __create_gui_project(tmpdir)
project_path = os.path.dirname(project_file)

exitcode, stdout, _, exe = cppcheck_ex(['--debug-lookup=platform', '--platform=none', '--project={}'.format(project_file)])
exepath = os.path.dirname(exe)
exepath_bin = os.path.join(exepath, 'cppcheck')
if sys.platform == 'win32':
exepath = exepath.replace('\\', '/')
exepath_bin += '.exe'
project_path = project_path.replace('\\', '/')
assert exitcode == 1, stdout
lines = stdout.splitlines()
assert lines == [
# TODO: needs to look relative to project file first
# TODO: the CWD lookups are duplicated
# TODO: needs to do the relative project lookup first
"looking for platform 'none' relative to '{}'".format(project_file),
"try to load platform file 'none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename=none.xml",
"try to load platform file 'platforms/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename=platforms/none.xml",
"try to load platform file '{}/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/none.xml".format(project_path, project_path),
"try to load platform file '{}/platforms/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/platforms/none.xml".format(project_path, project_path),
"looking for platform 'none' relative to '{}'".format(exepath_bin),
# TODO: should we really check CWD before relative to executable? should we check CWD at all?
"try to load platform file 'none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename=none.xml",
"try to load platform file 'platforms/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename=platforms/none.xml",
"try to load platform file '{}/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename={}/none.xml".format(exepath, exepath),
Expand All @@ -429,7 +437,7 @@ def test_platform_lookup_notfound_project(tmpdir): # #13939
]


def test_platform_lookup_notfound_compdb(tmpdir): # #13939
def test_platform_lookup_notfound_compdb(tmpdir):
compdb_file, _ = __create_compdb(tmpdir)

exitcode, stdout, _, exe = cppcheck_ex(['--debug-lookup=platform', '--platform=none', '--project={}'.format(compdb_file)])
Expand All @@ -441,7 +449,6 @@ def test_platform_lookup_notfound_compdb(tmpdir): # #13939
assert exitcode == 1, stdout
lines = stdout.splitlines()
assert lines == [
# TODO: needs to look relative to project file first
"looking for platform 'none' relative to '{}'".format(exepath_bin),
"try to load platform file 'none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename=none.xml",
"try to load platform file 'platforms/none.xml' ... Error=XML_ERROR_FILE_NOT_FOUND ErrorID=3 (0x3) Line number=0: filename=platforms/none.xml",
Expand Down Expand Up @@ -686,7 +693,7 @@ def test_addon_lookup_notfound_project(tmpdir): # #13940 / #13941
]


def test_addon_lookup_notfound_compdb(tmpdir): # #13940
def test_addon_lookup_notfound_compdb(tmpdir):
compdb_file, _ = __create_compdb(tmpdir)

exitcode, stdout, _, exe = cppcheck_ex(['--debug-lookup=addon', '--addon=none', '--project={}'.format(compdb_file)])
Expand All @@ -695,7 +702,6 @@ def test_addon_lookup_notfound_compdb(tmpdir): # #13940
assert exitcode == 1, stdout
lines = stdout.splitlines()
assert lines == [
# TODO: needs to look relative to the project file first
"looking for addon 'none.py'",
"looking for addon '{}none.py'".format(exepath_sep),
"looking for addon '{}addons/none.py'".format(exepath_sep), # TODO: mixed separators
Expand Down
8 changes: 8 additions & 0 deletions test/testcmdlineparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ class TestCmdlineParser : public TestFixture {
TEST_CASE(platformUnspecified);
TEST_CASE(platformPlatformFile);
TEST_CASE(platformUnknown);
TEST_CASE(platformEmpty);
TEST_CASE(plistEmpty);
TEST_CASE(plistDoesNotExist);
TEST_CASE(suppressionsOld);
Expand Down Expand Up @@ -1727,6 +1728,13 @@ class TestCmdlineParser : public TestFixture {
ASSERT_EQUALS("cppcheck: error: unrecognized platform: 'win128'.\n", logger->str());
}

void platformEmpty() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--platform=", "file.cpp"};
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parseFromArgs(argv));
ASSERT_EQUALS("cppcheck: error: empty platform specified.\n", logger->str());
}

void plistEmpty() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--plist-output=", "file.cpp"};
Expand Down
Loading