Skip to content

filesettings.h: removed constructors without language #7493

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

Merged
firewave merged 1 commit into from
May 6, 2025
Merged

filesettings.h: removed constructors without language #7493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 0 deletions cli/cmdlineparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
{
if (mSettings.library.markupFile(fs.filename()))
continue;
assert(fs.file.lang() == Standards::Language::None);
bool header = false;
fs.file.setLang(Path::identify(fs.filename(), mSettings.cppHeaderProbe, &header));
// unknown extensions default to C++
Expand All @@ -244,6 +245,7 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
for (auto& fs : fileSettings)
{
if (mSettings.library.markupFile(fs.filename())) {
assert(fs.file.lang() == Standards::Language::None);
fs.file.setLang(Standards::Language::C);
}
}
Expand Down
2 changes: 1 addition & 1 deletion democlient/democlient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class CppcheckExecutor : public ErrorLogger {
{}

void run(const char code[]) {
cppcheck.check(FileWithDetails("test.cpp"), code);
cppcheck.check(FileWithDetails("test.cpp", Standards::Language::CPP, 0), code);
}

void reportOut(const std::string & /*outmsg*/, Color /*c*/) override {}
Expand Down
3 changes: 2 additions & 1 deletion gui/mainwindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "helpdialog.h"
#include "importproject.h"
#include "librarydialog.h"
#include "path.h"
#include "platform.h"
#include "projectfile.h"
#include "projectfiledialog.h"
Expand Down Expand Up @@ -702,7 +703,7 @@ void MainWindow::analyzeCode(const QString& code, const QString& filename)
checkLockDownUI();
clearResults();
mUI->mResults->checkingStarted(1);
cppcheck.check(FileWithDetails(filename.toStdString()), code.toStdString());
cppcheck.check(FileWithDetails(filename.toStdString(), Path::identify(filename.toStdString(), false), 0), code.toStdString());
analysisDone();

// Expand results
Expand Down
8 changes: 0 additions & 8 deletions lib/filesettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@
class FileWithDetails
{
public:
explicit FileWithDetails(std::string path)
: FileWithDetails(std::move(path), Standards::Language::None, 0)
{}

FileWithDetails(std::string path, Standards::Language lang, std::size_t size)
Copy link
Copy Markdown
Collaborator

@chrchr-github chrchr-github Apr 29, 2025

Choose a reason for hiding this comment

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

Maybe size = 0 should be the default?

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 29, 2025

Choose a reason for hiding this comment

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

I would like to have it mandatory since the application code should not omit it without a good reason.

Copy link
Copy Markdown
Collaborator

@chrchr-github chrchr-github Apr 30, 2025

Choose a reason for hiding this comment

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

It seems to be always 0 though, except in the new tests.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 30, 2025

Choose a reason for hiding this comment

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

That is because it was not accidentally omitted in the application code.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave May 6, 2025

Choose a reason for hiding this comment

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

Maybe this can be cleaned up with the draft changes to the size handling.

: mPath(std::move(path))
, mPathSimplified(Path::simplifyPath(mPath))
Expand Down Expand Up @@ -80,10 +76,6 @@ class FileWithDetails

/** File settings. Multiple configurations for a file is allowed. */
struct CPPCHECKLIB FileSettings {
explicit FileSettings(std::string path)
: file(std::move(path))
{}

FileSettings(std::string path, Standards::Language lang, std::size_t size)
: file(std::move(path), lang, size)
{}
Expand Down
9 changes: 5 additions & 4 deletions lib/importproject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ bool ImportProject::importCompileCommands(std::istream &istr)
printError("'" + path + "' from compilation database does not exist");
return false;
}
FileSettings fs{std::move(path)};
FileSettings fs{std::move(path), Standards::Language::None, 0}; // file will be identified later on
fsParseCommand(fs, command); // read settings; -D, -I, -U, -std, -m*, -f*
std::map<std::string, std::string, cppcheck::stricmp> variables;
fsSetIncludePaths(fs, directory, fs.includePaths, variables);
Expand Down Expand Up @@ -833,7 +833,7 @@ bool ImportProject::importVcxproj(const std::string &filename, std::map<std::str
continue;
}

FileSettings fs{cfilename};
FileSettings fs{cfilename, Standards::Language::None, 0}; // file will be identified later on
fs.cfg = p.name;
// TODO: detect actual MSC version
fs.msc = true;
Expand Down Expand Up @@ -1195,7 +1195,8 @@ bool ImportProject::importBcb6Prj(const std::string &projectFilename)
//
// We can also force C++ compilation for all files using the -P command line switch.
const bool cppMode = forceCppMode || Path::getFilenameExtensionInLowerCase(c) == ".cpp";
FileSettings fs{Path::simplifyPath(Path::isAbsolute(c) ? c : projectDir + c)};
// TODO: needs to set language and ignore later identification and language enforcement
FileSettings fs{Path::simplifyPath(Path::isAbsolute(c) ? c : projectDir + c), Standards::Language::None, 0}; // file will be identified later on
fsSetIncludePaths(fs, projectDir, toStringList(includePath), variables);
fsSetDefines(fs, cppMode ? cppDefines : defines);
fileSettings.push_back(std::move(fs));
Expand Down Expand Up @@ -1484,7 +1485,7 @@ void ImportProject::setRelativePaths(const std::string &filename)
return;
const std::vector<std::string> basePaths{Path::fromNativeSeparators(Path::getCurrentPath())};
for (auto &fs: fileSettings) {
fs.file = FileWithDetails{Path::getRelativePath(fs.filename(), basePaths)};
fs.file = FileWithDetails{Path::getRelativePath(fs.filename(), basePaths), Standards::Language::None, 0}; // file will be identified later on
for (auto &includePath: fs.includePaths)
includePath = Path::getRelativePath(includePath, basePaths);
}
Expand Down
2 changes: 1 addition & 1 deletion oss-fuzz/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ static Settings create_settings()
}
static const Settings s_settings = create_settings();
static DummyErrorLogger s_errorLogger;
static const FileWithDetails s_file("test.cpp");
static const FileWithDetails s_file("test.cpp", Standards::Language::CPP, 0);

static void doCheck(const std::string& code)
{
Expand Down
13 changes: 7 additions & 6 deletions test/testcppcheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "filesettings.h"
#include "fixture.h"
#include "helpers.h"
#include "path.h"
#include "settings.h"
#include "suppressions.h"

Expand Down Expand Up @@ -116,7 +117,7 @@ class TestCppcheck : public TestFixture {
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(file.path())));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(file.path(), Path::identify(file.path(), false), 0)));
// TODO: how to properly disable these warnings?
errorLogger.ids.erase(std::remove_if(errorLogger.ids.begin(), errorLogger.ids.end(), [](const std::string& id) {
return id == "logChecker";
Expand All @@ -138,7 +139,7 @@ class TestCppcheck : public TestFixture {
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
FileSettings fs{file.path()};
FileSettings fs{file.path(), Path::identify(file.path(), false), 0};
ASSERT_EQUALS(1, cppcheck.check(fs));
// TODO: how to properly disable these warnings?
errorLogger.ids.erase(std::remove_if(errorLogger.ids.begin(), errorLogger.ids.end(), [](const std::string& id) {
Expand All @@ -162,7 +163,7 @@ class TestCppcheck : public TestFixture {
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
ASSERT_EQUALS(0, cppcheck.check(FileWithDetails(file.path())));
ASSERT_EQUALS(0, cppcheck.check(FileWithDetails(file.path(), Path::identify(file.path(), false), 0)));
// TODO: how to properly disable these warnings?
errorLogger.ids.erase(std::remove_if(errorLogger.ids.begin(), errorLogger.ids.end(), [](const std::string& id) {
return id == "logChecker";
Expand All @@ -188,8 +189,8 @@ class TestCppcheck : public TestFixture {
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_a.path())));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_b.path())));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_a.path(), Path::identify(test_file_a.path(), false), 0)));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_b.path(), Path::identify(test_file_b.path(), false), 0)));
// TODO: how to properly disable these warnings?
errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
return msg.id == "logChecker";
Expand Down Expand Up @@ -220,7 +221,7 @@ class TestCppcheck : public TestFixture {
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file.path())));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file.path(), Path::identify(test_file.path(), false), 0)));
// TODO: how to properly disable these warnings?
errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
return msg.id == "logChecker";
Expand Down
2 changes: 1 addition & 1 deletion test/testexecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class TestExecutor : public TestFixture {
}

void hasToLogSimple() {
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
const std::list<FileWithDetails> files{FileWithDetails{"test.c", Standards::Language::C, 0}};
const std::list<FileSettings> fileSettings;
// this is the "simple" format
const auto settings = dinit(Settings,
Expand Down
38 changes: 19 additions & 19 deletions test/testfilesettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,46 +31,46 @@ class TestFileSettings : public TestFixture {

void test() const {
{
const FileWithDetails p{"file.cpp"};
ASSERT_EQUALS("file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS_ENUM(Standards::Language::None, p.lang());
ASSERT_EQUALS(0, p.size());
const FileWithDetails p{"file.c", Standards::Language::C, 456};
ASSERT_EQUALS("file.c", p.path());
ASSERT_EQUALS("file.c", p.spath());
ASSERT_EQUALS_ENUM(Standards::Language::C, p.lang());
ASSERT_EQUALS(456, p.size());
}
{
const FileWithDetails p{"file.cpp", Standards::Language::C, 123};
const FileWithDetails p{"file.cpp", Standards::Language::CPP, 123};
ASSERT_EQUALS("file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS_ENUM(Standards::Language::C, p.lang());
ASSERT_EQUALS_ENUM(Standards::Language::CPP, p.lang());
ASSERT_EQUALS(123, p.size());
}
{
const FileWithDetails p{"in/file.cpp"};
const FileWithDetails p{"in/file.cpp", Standards::Language::CPP, 123};
ASSERT_EQUALS("in/file.cpp", p.path());
ASSERT_EQUALS("in/file.cpp", p.spath());
ASSERT_EQUALS_ENUM(Standards::Language::None, p.lang());
ASSERT_EQUALS(0, p.size());
ASSERT_EQUALS_ENUM(Standards::Language::CPP, p.lang());
ASSERT_EQUALS(123, p.size());
}
{
const FileWithDetails p{"in\\file.cpp"};
const FileWithDetails p{"in\\file.cpp", Standards::Language::CPP, 123};
ASSERT_EQUALS("in\\file.cpp", p.path());
ASSERT_EQUALS("in/file.cpp", p.spath());
ASSERT_EQUALS_ENUM(Standards::Language::None, p.lang());
ASSERT_EQUALS(0, p.size());
ASSERT_EQUALS_ENUM(Standards::Language::CPP, p.lang());
ASSERT_EQUALS(123, p.size());
}
{
const FileWithDetails p{"in/../file.cpp"};
const FileWithDetails p{"in/../file.cpp", Standards::Language::CPP, 123};
ASSERT_EQUALS("in/../file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS_ENUM(Standards::Language::None, p.lang());
ASSERT_EQUALS(0, p.size());
ASSERT_EQUALS_ENUM(Standards::Language::CPP, p.lang());
ASSERT_EQUALS(123, p.size());
}
{
const FileWithDetails p{"in\\..\\file.cpp"};
const FileWithDetails p{"in\\..\\file.cpp", Standards::Language::CPP, 123};
ASSERT_EQUALS("in\\..\\file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS_ENUM(Standards::Language::None, p.lang());
ASSERT_EQUALS(0, p.size());
ASSERT_EQUALS_ENUM(Standards::Language::CPP, p.lang());
ASSERT_EQUALS(123, p.size());
}
}
};
Expand Down
12 changes: 6 additions & 6 deletions test/testimportproject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class TestImportProject : public TestFixture {
}

void setDefines() const {
FileSettings fs{"test.cpp"};
FileSettings fs{"test.cpp", Standards::Language::CPP, 0};

ImportProject::fsSetDefines(fs, "A");
ASSERT_EQUALS("A=1", fs.defines);
Expand All @@ -87,7 +87,7 @@ class TestImportProject : public TestFixture {
}

void setIncludePaths1() const {
FileSettings fs{"test.cpp"};
FileSettings fs{"test.cpp", Standards::Language::CPP, 0};
std::list<std::string> in(1, "../include");
std::map<std::string, std::string, cppcheck::stricmp> variables;
ImportProject::fsSetIncludePaths(fs, "abc/def/", in, variables);
Expand All @@ -96,7 +96,7 @@ class TestImportProject : public TestFixture {
}

void setIncludePaths2() const {
FileSettings fs{"test.cpp"};
FileSettings fs{"test.cpp", Standards::Language::CPP, 0};
std::list<std::string> in(1, "$(SolutionDir)other");
std::map<std::string, std::string, cppcheck::stricmp> variables;
variables["SolutionDir"] = "c:/abc/";
Expand All @@ -106,7 +106,7 @@ class TestImportProject : public TestFixture {
}

void setIncludePaths3() const { // macro names are case insensitive
FileSettings fs{"test.cpp"};
FileSettings fs{"test.cpp", Standards::Language::CPP, 0};
std::list<std::string> in(1, "$(SOLUTIONDIR)other");
std::map<std::string, std::string, cppcheck::stricmp> variables;
variables["SolutionDir"] = "c:/abc/";
Expand Down Expand Up @@ -393,8 +393,8 @@ class TestImportProject : public TestFixture {
}

void ignorePaths() const {
FileSettings fs1{"foo/bar"};
FileSettings fs2{"qwe/rty"};
FileSettings fs1{"foo/bar", Standards::Language::CPP, 0};
FileSettings fs2{"qwe/rty", Standards::Language::CPP, 0};
TestImporter project;
project.fileSettings = {std::move(fs1), std::move(fs2)};

Expand Down
12 changes: 6 additions & 6 deletions test/testsuppressions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1232,7 +1232,7 @@ class TestSuppressions : public TestFixture {
CppCheck cppCheck(settings, supprs, *this, false, nullptr); // <- do not "use global suppressions". pretend this is a thread that just checks a file.

const char code[] = "int f() { int a; return a; }";
ASSERT_EQUALS(0, cppCheck.check(FileWithDetails("test.c"), code)); // <- no unsuppressed error is seen
ASSERT_EQUALS(0, cppCheck.check(FileWithDetails("test.c", Standards::Language::C, 0), code)); // <- no unsuppressed error is seen
ASSERT_EQUALS("[test.c:1:25]: (error) Uninitialized variable: a [uninitvar]\n", errout_str()); // <- report error so ThreadExecutor can suppress it and make sure the global suppression is matched.
}

Expand All @@ -1241,19 +1241,19 @@ class TestSuppressions : public TestFixture {
SuppressionList::Suppression suppression("unusedFunction", "test.c", 3);
suppression.checked = true; // have to do this because fixes for #5704
ASSERT_EQUALS("", suppressions.addSuppression(std::move(suppression)));
ASSERT_EQUALS(true, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c"), true).empty());
ASSERT_EQUALS(true, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c", Standards::Language::C, 0), true).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(true).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c"), false).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c", Standards::Language::C, 0), false).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(false).empty());
}

void globalsuppress_unusedFunction() const { // #4946 - wrong report of "unmatchedSuppression" for "unusedFunction"
SuppressionList suppressions;
ASSERT_EQUALS("", suppressions.addSuppressionLine("unusedFunction:*"));
ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid")));
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c"), true).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c", Standards::Language::C, 0), true).empty());
ASSERT_EQUALS(true, !suppressions.getUnmatchedGlobalSuppressions(true).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c"), false).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions(FileWithDetails("test.c", Standards::Language::C, 0), false).empty());
ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(false).empty());
}

Expand All @@ -1276,7 +1276,7 @@ class TestSuppressions : public TestFixture {
" int y;\n"
"};";
CppCheck cppCheck(settings, supprs, *this, true, nullptr);
ASSERT_EQUALS(0, cppCheck.check(FileWithDetails("/somewhere/test.cpp"), code));
ASSERT_EQUALS(0, cppCheck.check(FileWithDetails("/somewhere/test.cpp", Standards::Language::CPP, 0), code));
ASSERT_EQUALS("",errout_str());
}

Expand Down
Loading