Skip to content

Commit

Permalink
more FileWithDetails usage (#6500)
Browse files Browse the repository at this point in the history
  • Loading branch information
firewave authored Jun 11, 2024
1 parent d33d29c commit 06889a3
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 48 deletions.
28 changes: 14 additions & 14 deletions Makefile

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion cli/processexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ unsigned int ProcessExecutor::check()
fileChecker.analyseClangTidy(*iFileSettings);
} else {
// Read file from a file
resultOfCheck = fileChecker.check(iFile->path());
resultOfCheck = fileChecker.check(*iFile);
// TODO: call analyseClangTidy()?
}

Expand Down
2 changes: 1 addition & 1 deletion cli/singleexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ unsigned int SingleExecutor::check()
unsigned int c = 0;

for (std::list<FileWithDetails>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
result += mCppcheck.check(i->path());
result += mCppcheck.check(*i);
processedsize += i->size();
++c;
if (!mSettings.quiet)
Expand Down
8 changes: 4 additions & 4 deletions cli/threadexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ class ThreadData
});
}

bool next(const std::string *&file, const FileSettings *&fs, std::size_t &fileSize) {
bool next(const FileWithDetails *&file, const FileSettings *&fs, std::size_t &fileSize) {
std::lock_guard<std::mutex> l(mFileSync);
if (mItNextFile != mFiles.end()) {
file = &mItNextFile->path();
file = &(*mItNextFile);
fs = nullptr;
fileSize = mItNextFile->size();
++mItNextFile;
Expand All @@ -114,7 +114,7 @@ class ThreadData
return false;
}

unsigned int check(ErrorLogger &errorLogger, const std::string *file, const FileSettings *fs) const {
unsigned int check(ErrorLogger &errorLogger, const FileWithDetails *file, const FileSettings *fs) const {
CppCheck fileChecker(errorLogger, false, mExecuteCommand);
fileChecker.settings() = mSettings; // this is a copy

Expand Down Expand Up @@ -163,7 +163,7 @@ static unsigned int STDCALL threadProc(ThreadData *data)
{
unsigned int result = 0;

const std::string *file;
const FileWithDetails *file;
const FileSettings *fs;
std::size_t fileSize;

Expand Down
3 changes: 2 additions & 1 deletion democlient/democlient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <iostream>

#include "cppcheck.h"
#include "filesettings.h"
#include "version.h"

static void unencode(const char *src, char *dest)
Expand Down Expand Up @@ -59,7 +60,7 @@ class CppcheckExecutor : public ErrorLogger {
}

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

void reportOut(const std::string & /*outmsg*/, Color /*c*/) override {}
Expand Down
2 changes: 1 addition & 1 deletion gui/checkthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void CheckThread::run()
QString file = mResult.getNextFile();
while (!file.isEmpty() && mState == Running) {
qDebug() << "Checking file" << file;
mCppcheck.check(file.toStdString());
mCppcheck.check(FileWithDetails(file.toStdString()));
runAddonsAndTools(nullptr, file);
emit fileChecked(file);

Expand Down
2 changes: 1 addition & 1 deletion gui/mainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ void MainWindow::analyzeCode(const QString& code, const QString& filename)
checkLockDownUI();
clearResults();
mUI->mResults->checkingStarted(1);
cppcheck.check(filename.toStdString(), code.toStdString());
cppcheck.check(FileWithDetails(filename.toStdString()), code.toStdString());
analysisDone();

// Expand results
Expand Down
22 changes: 10 additions & 12 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -539,18 +539,18 @@ unsigned int CppCheck::checkClang(const std::string &path)
return mExitCode;
}

unsigned int CppCheck::check(const std::string &path)
unsigned int CppCheck::check(const FileWithDetails &file)
{
if (mSettings.clang)
return checkClang(Path::simplifyPath(path));
return checkClang(file.spath());

return checkFile(Path::simplifyPath(path), emptyString);
return checkFile(file.spath(), emptyString);
}

unsigned int CppCheck::check(const std::string &path, const std::string &content)
unsigned int CppCheck::check(const FileWithDetails &file, const std::string &content)
{
std::istringstream iss(content);
return checkFile(Path::simplifyPath(path), emptyString, &iss);
return checkFile(file.spath(), emptyString, &iss);
}

unsigned int CppCheck::check(const FileSettings &fs)
Expand Down Expand Up @@ -578,12 +578,12 @@ unsigned int CppCheck::check(const FileSettings &fs)
if (mSettings.clang) {
temp.mSettings.includePaths.insert(temp.mSettings.includePaths.end(), fs.systemIncludePaths.cbegin(), fs.systemIncludePaths.cend());
// TODO: propagate back suppressions
const unsigned int returnValue = temp.check(Path::simplifyPath(fs.filename()));
const unsigned int returnValue = temp.check(fs.file);
if (mUnusedFunctionsCheck)
mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
return returnValue;
}
const unsigned int returnValue = temp.checkFile(Path::simplifyPath(fs.filename()), fs.cfg);
const unsigned int returnValue = temp.checkFile(fs.sfilename(), fs.cfg);
mSettings.supprs.nomsg.addSuppressions(temp.mSettings.supprs.nomsg.getSuppressions());
if (mUnusedFunctionsCheck)
mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
Expand Down Expand Up @@ -612,8 +612,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
const Timer fileTotalTimer(mSettings.showtime == SHOWTIME_MODES::SHOWTIME_FILE_TOTAL, filename);

if (!mSettings.quiet) {
std::string fixedpath = Path::simplifyPath(filename);
fixedpath = Path::toNativeSeparators(std::move(fixedpath));
std::string fixedpath = Path::toNativeSeparators(filename);
mErrorLogger.reportOut(std::string("Checking ") + fixedpath + ' ' + cfgname + std::string("..."), Color::FgGreen);

if (mSettings.verbose) {
Expand Down Expand Up @@ -867,8 +866,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string

// If only errors are printed, print filename after the check
if (!mSettings.quiet && (!mCurrentConfig.empty() || checkCount > 1)) {
std::string fixedpath = Path::simplifyPath(filename);
fixedpath = Path::toNativeSeparators(std::move(fixedpath));
std::string fixedpath = Path::toNativeSeparators(filename);
mErrorLogger.reportOut("Checking " + fixedpath + ": " + mCurrentConfig + "...", Color::FgGreen);
}

Expand Down Expand Up @@ -973,7 +971,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
fdump.close();
}

executeAddons(dumpFile, Path::simplifyPath(filename));
executeAddons(dumpFile, filename);

} catch (const TerminateException &) {
// Analysis is terminated
Expand Down
8 changes: 4 additions & 4 deletions lib/cppcheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,26 +83,26 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
/**
* @brief Check the file.
* This function checks one given file for errors.
* @param path Path to the file to check.
* @param file The file to check.
* @return amount of errors found or 0 if none were found.
* @note You must set settings before calling this function (by calling
* settings()).
*/
unsigned int check(const std::string &path);
unsigned int check(const FileWithDetails &file);
unsigned int check(const FileSettings &fs);

/**
* @brief Check the file.
* This function checks one "virtual" file. The file is not read from
* the disk but the content is given in @p content. In errors the @p path
* is used as a filename.
* @param path Path to the file to check.
* @param file The file to check.
* @param content File content as a string.
* @return amount of errors found or 0 if none were found.
* @note You must set settings before calling this function (by calling
* settings()).
*/
unsigned int check(const std::string &path, const std::string &content);
unsigned int check(const FileWithDetails &file, const std::string &content);

/**
* @brief Get reference to current settings.
Expand Down
10 changes: 10 additions & 0 deletions lib/filesettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define fileSettingsH

#include "config.h"
#include "path.h"
#include "platform.h"

#include <list>
Expand All @@ -44,6 +45,11 @@ class FileWithDetails
return mPath;
}

std::string spath() const
{
return Path::simplifyPath(mPath);
}

std::size_t size() const
{
return mSize;
Expand All @@ -69,6 +75,10 @@ struct CPPCHECKLIB FileSettings {
{
return file.path();
}
std::string sfilename() const
{
return file.spath();
}
std::string defines;
// TODO: handle differently
std::string cppcheckDefines() const {
Expand Down
4 changes: 3 additions & 1 deletion oss-fuzz/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

#include "cppcheck.h"
#include "filesettings.h"
#include "type2.h"

#ifdef NO_FUZZ
Expand All @@ -37,13 +38,14 @@ class DummyErrorLogger : public ErrorLogger {
};

static DummyErrorLogger s_errorLogger;
static const FileWithDetails s_file("test.cpp");

static void doCheck(const std::string& code)
{
CppCheck cppcheck(s_errorLogger, false, nullptr);
cppcheck.settings().addEnabled("all");
cppcheck.settings().certainty.setEnabled(Certainty::inconclusive, true);
cppcheck.check("test.cpp", code);
cppcheck.check(s_file, code);
}

#ifndef NO_FUZZ
Expand Down
8 changes: 4 additions & 4 deletions test/testcppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class TestCppcheck : public TestFixture {

ErrorLogger2 errorLogger;
CppCheck cppcheck(errorLogger, false, {});
ASSERT_EQUALS(1, cppcheck.check(file.path()));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(file.path())));
// 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 Down Expand Up @@ -146,7 +146,7 @@ class TestCppcheck : public TestFixture {
const char xmldata[] = R"(<def format="2"><markup ext=".cpp" reporterrors="false"/></def>)";
const Settings s = settingsBuilder().libraryxml(xmldata, sizeof(xmldata)).build();
cppcheck.settings() = s;
ASSERT_EQUALS(0, cppcheck.check(file.path()));
ASSERT_EQUALS(0, cppcheck.check(FileWithDetails(file.path())));
// 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 @@ -169,8 +169,8 @@ class TestCppcheck : public TestFixture {

ErrorLogger2 errorLogger;
CppCheck cppcheck(errorLogger, false, {});
ASSERT_EQUALS(1, cppcheck.check(test_file_a.path()));
ASSERT_EQUALS(1, cppcheck.check(test_file_b.path()));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_a.path())));
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file_b.path())));
// 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
10 changes: 8 additions & 2 deletions test/testfilesettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,38 +25,44 @@ class TestFileSettings : public TestFixture {

private:
void run() override {
TEST_CASE(path);
TEST_CASE(test);
}

void path() const {
void test() const {
{
const FileWithDetails p{"file.cpp"};
ASSERT_EQUALS("file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS(0, p.size());
}
{
const FileWithDetails p{"file.cpp", 123};
ASSERT_EQUALS("file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS(123, p.size());
}
{
const FileWithDetails p{"in/file.cpp"};
ASSERT_EQUALS("in/file.cpp", p.path());
ASSERT_EQUALS("in/file.cpp", p.spath());
ASSERT_EQUALS(0, p.size());
}
{
const FileWithDetails p{"in\\file.cpp"};
ASSERT_EQUALS("in\\file.cpp", p.path());
ASSERT_EQUALS("in/file.cpp", p.spath());
ASSERT_EQUALS(0, p.size());
}
{
const FileWithDetails p{"in/../file.cpp"};
ASSERT_EQUALS("in/../file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS(0, p.size());
}
{
const FileWithDetails p{"in\\..\\file.cpp"};
ASSERT_EQUALS("in\\..\\file.cpp", p.path());
ASSERT_EQUALS("file.cpp", p.spath());
ASSERT_EQUALS(0, p.size());
}
}
Expand Down
44 changes: 44 additions & 0 deletions test/testpath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class TestPath : public TestFixture {
TEST_CASE(identify);
TEST_CASE(identifyWithCppProbe);
TEST_CASE(is_header);
TEST_CASE(simplifyPath);
}

void removeQuotationMarks() const {
Expand Down Expand Up @@ -376,6 +377,49 @@ class TestPath : public TestFixture {
ASSERT(Path::isHeader("index.htm")==false);
ASSERT(Path::isHeader("index.html")==false);
}

void simplifyPath() const {
ASSERT_EQUALS("file.cpp", Path::simplifyPath("file.cpp"));
ASSERT_EQUALS("../file.cpp", Path::simplifyPath("../file.cpp"));
ASSERT_EQUALS("file.cpp", Path::simplifyPath("test/../file.cpp"));
ASSERT_EQUALS("../file.cpp", Path::simplifyPath("../test/../file.cpp"));

ASSERT_EQUALS("file.cpp", Path::simplifyPath("./file.cpp"));
ASSERT_EQUALS("../file.cpp", Path::simplifyPath("./../file.cpp"));
ASSERT_EQUALS("file.cpp", Path::simplifyPath("./test/../file.cpp"));
ASSERT_EQUALS("../file.cpp", Path::simplifyPath("./../test/../file.cpp"));

ASSERT_EQUALS("test/", Path::simplifyPath("test/"));
ASSERT_EQUALS("../test/", Path::simplifyPath("../test/"));
ASSERT_EQUALS("../", Path::simplifyPath("../test/.."));
ASSERT_EQUALS("../", Path::simplifyPath("../test/../"));

ASSERT_EQUALS("/home/file.cpp", Path::simplifyPath("/home/test/../file.cpp"));
ASSERT_EQUALS("/file.cpp", Path::simplifyPath("/home/../test/../file.cpp"));

ASSERT_EQUALS("C:/home/file.cpp", Path::simplifyPath("C:/home/test/../file.cpp"));
ASSERT_EQUALS("C:/file.cpp", Path::simplifyPath("C:/home/../test/../file.cpp"));

ASSERT_EQUALS("../file.cpp", Path::simplifyPath("..\\file.cpp"));
ASSERT_EQUALS("file.cpp", Path::simplifyPath("test\\..\\file.cpp"));
ASSERT_EQUALS("../file.cpp", Path::simplifyPath("..\\test\\..\\file.cpp"));

ASSERT_EQUALS("file.cpp", Path::simplifyPath(".\\file.cpp"));
ASSERT_EQUALS("../file.cpp", Path::simplifyPath(".\\..\\file.cpp"));
ASSERT_EQUALS("file.cpp", Path::simplifyPath(".\\test\\..\\file.cpp"));
ASSERT_EQUALS("../file.cpp", Path::simplifyPath(".\\..\\test\\..\\file.cpp"));

ASSERT_EQUALS("test/", Path::simplifyPath("test\\"));
ASSERT_EQUALS("../test/", Path::simplifyPath("..\\test\\"));
ASSERT_EQUALS("../", Path::simplifyPath("..\\test\\.."));
ASSERT_EQUALS("../", Path::simplifyPath("..\\test\\..\\"));

ASSERT_EQUALS("C:/home/file.cpp", Path::simplifyPath("C:\\home\\test\\..\\file.cpp"));
ASSERT_EQUALS("C:/file.cpp", Path::simplifyPath("C:\\home\\..\\test\\..\\file.cpp"));

ASSERT_EQUALS("//home/file.cpp", Path::simplifyPath("\\\\home\\test\\..\\file.cpp"));
ASSERT_EQUALS("//file.cpp", Path::simplifyPath("\\\\home\\..\\test\\..\\file.cpp"));
}
};

REGISTER_TEST(TestPath)
4 changes: 2 additions & 2 deletions test/testsuppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ class TestSuppressions : public TestFixture {
settings.exitCode = 1;

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

Expand Down Expand Up @@ -1225,7 +1225,7 @@ class TestSuppressions : public TestFixture {
" // cppcheck-suppress unusedStructMember\n"
" int y;\n"
"};";
ASSERT_EQUALS(0, cppCheck.check("/somewhere/test.cpp", code));
ASSERT_EQUALS(0, cppCheck.check(FileWithDetails("/somewhere/test.cpp"), code));
ASSERT_EQUALS("",errout_str());
}

Expand Down

0 comments on commit 06889a3

Please sign in to comment.