Skip to content

Commit 4addad1

Browse files
authored
moved settings-related code from CppCheckExecutor to CmdLineParser (#5672)
`CppCheckExecutor` contains some code which is not related to the execution but actually to the creation of the settings. This is causing inconsistencies in the error handling/logging as well as interfering with the testability.
1 parent 56c7ac3 commit 4addad1

5 files changed

Lines changed: 331 additions & 319 deletions

File tree

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,10 +647,10 @@ $(libcppdir)/utils.o: lib/utils.cpp lib/config.h lib/utils.h
647647
$(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathlib.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h
648648
$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vfvalue.cpp
649649

650-
cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
650+
cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
651651
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
652652

653-
cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/filelister.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
653+
cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
654654
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp
655655

656656
cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/config.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h

cli/cmdlineparser.cpp

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,20 @@
1818

1919
#include "cmdlineparser.h"
2020

21+
#include "addoninfo.h"
2122
#include "check.h"
23+
#include "color.h"
2224
#include "config.h"
25+
#include "cppcheck.h"
2326
#include "cppcheckexecutor.h"
2427
#include "errorlogger.h"
2528
#include "errortypes.h"
29+
#include "filelister.h"
2630
#include "filesettings.h"
2731
#include "importproject.h"
32+
#include "library.h"
2833
#include "path.h"
34+
#include "pathmatch.h"
2935
#include "platform.h"
3036
#include "settings.h"
3137
#include "standards.h"
@@ -34,6 +40,7 @@
3440
#include "utils.h"
3541

3642
#include <algorithm>
43+
#include <cassert>
3744
#include <climits>
3845
#include <cstdio>
3946
#include <cstdlib> // EXIT_FAILURE
@@ -109,13 +116,186 @@ static bool addPathsToSet(const std::string& fileName, std::set<std::string>& se
109116
return true;
110117
}
111118

119+
namespace {
120+
class XMLErrorMessagesLogger : public ErrorLogger
121+
{
122+
void reportOut(const std::string & outmsg, Color /*c*/ = Color::Reset) override
123+
{
124+
std::cout << outmsg << std::endl;
125+
}
126+
127+
void reportErr(const ErrorMessage &msg) override
128+
{
129+
reportOut(msg.toXML());
130+
}
131+
132+
void reportProgress(const std::string & /*filename*/, const char /*stage*/[], const std::size_t /*value*/) override
133+
{}
134+
};
135+
}
136+
112137
CmdLineParser::CmdLineParser(CmdLineLogger &logger, Settings &settings, Suppressions &suppressions, Suppressions &suppressionsNoFail)
113138
: mLogger(logger)
114139
, mSettings(settings)
115140
, mSuppressions(suppressions)
116141
, mSuppressionsNoFail(suppressionsNoFail)
117142
{}
118143

144+
bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
145+
{
146+
const bool success = parseFromArgs(argc, argv);
147+
148+
if (success) {
149+
if (getShowVersion() && !getShowErrorMessages()) {
150+
if (!mSettings.cppcheckCfgProductName.empty()) {
151+
mLogger.printRaw(mSettings.cppcheckCfgProductName);
152+
} else {
153+
const char * const extraVersion = CppCheck::extraVersion();
154+
if (*extraVersion != 0)
155+
mLogger.printRaw(std::string("Cppcheck ") + CppCheck::version() + " ("+ extraVersion + ')');
156+
else
157+
mLogger.printRaw(std::string("Cppcheck ") + CppCheck::version());
158+
}
159+
}
160+
161+
if (getShowErrorMessages()) {
162+
XMLErrorMessagesLogger xmlLogger;
163+
std::cout << ErrorMessage::getXMLHeader(mSettings.cppcheckCfgProductName);
164+
CppCheck::getErrorMessages(xmlLogger);
165+
std::cout << ErrorMessage::getXMLFooter() << std::endl;
166+
}
167+
168+
if (exitAfterPrinting()) {
169+
Settings::terminate();
170+
return true;
171+
}
172+
} else {
173+
return false;
174+
}
175+
176+
// Libraries must be loaded before FileLister is executed to ensure markup files will be
177+
// listed properly.
178+
if (!loadLibraries(mSettings))
179+
return false;
180+
181+
if (!loadAddons(mSettings))
182+
return false;
183+
184+
// Check that all include paths exist
185+
{
186+
for (std::list<std::string>::iterator iter = mSettings.includePaths.begin();
187+
iter != mSettings.includePaths.end();
188+
) {
189+
const std::string path(Path::toNativeSeparators(*iter));
190+
if (Path::isDirectory(path))
191+
++iter;
192+
else {
193+
// TODO: this bypasses the template format and other settings
194+
// If the include path is not found, warn user and remove the non-existing path from the list.
195+
if (mSettings.severity.isEnabled(Severity::information))
196+
std::cout << "(information) Couldn't find path given by -I '" << path << '\'' << std::endl;
197+
iter = mSettings.includePaths.erase(iter);
198+
}
199+
}
200+
}
201+
202+
// Output a warning for the user if he tries to exclude headers
203+
const std::vector<std::string>& ignored = getIgnoredPaths();
204+
const bool warn = std::any_of(ignored.cbegin(), ignored.cend(), [](const std::string& i) {
205+
return Path::isHeader(i);
206+
});
207+
if (warn) {
208+
mLogger.printMessage("filename exclusion does not apply to header (.h and .hpp) files.");
209+
mLogger.printMessage("Please use --suppress for ignoring results from the header files.");
210+
}
211+
212+
const std::vector<std::string>& pathnamesRef = getPathNames();
213+
const std::list<FileSettings>& fileSettingsRef = getFileSettings();
214+
215+
// the inputs can only be used exclusively - CmdLineParser should already handle this
216+
assert(!(!pathnamesRef.empty() && !fileSettingsRef.empty()));
217+
218+
if (!fileSettingsRef.empty()) {
219+
std::list<FileSettings> fileSettings;
220+
if (!mSettings.fileFilters.empty()) {
221+
// filter only for the selected filenames from all project files
222+
std::copy_if(fileSettingsRef.cbegin(), fileSettingsRef.cend(), std::back_inserter(fileSettings), [&](const FileSettings &fs) {
223+
return matchglobs(mSettings.fileFilters, fs.filename);
224+
});
225+
if (fileSettings.empty()) {
226+
mLogger.printError("could not find any files matching the filter.");
227+
return false;
228+
}
229+
}
230+
else {
231+
fileSettings = fileSettingsRef;
232+
}
233+
234+
mFileSettings.clear();
235+
236+
// sort the markup last
237+
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
238+
return !mSettings.library.markupFile(fs.filename) || !mSettings.library.processMarkupAfterCode(fs.filename);
239+
});
240+
241+
std::copy_if(fileSettings.cbegin(), fileSettings.cend(), std::back_inserter(mFileSettings), [&](const FileSettings &fs) {
242+
return mSettings.library.markupFile(fs.filename) && mSettings.library.processMarkupAfterCode(fs.filename);
243+
});
244+
}
245+
246+
if (!pathnamesRef.empty()) {
247+
std::list<std::pair<std::string, std::size_t>> filesResolved;
248+
// TODO: this needs to be inlined into PathMatch as it depends on the underlying filesystem
249+
#if defined(_WIN32)
250+
// For Windows we want case-insensitive path matching
251+
const bool caseSensitive = false;
252+
#else
253+
const bool caseSensitive = true;
254+
#endif
255+
// Execute recursiveAddFiles() to each given file parameter
256+
const PathMatch matcher(ignored, caseSensitive);
257+
for (const std::string &pathname : pathnamesRef) {
258+
const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), mSettings.library.markupExtensions(), matcher);
259+
if (!err.empty()) {
260+
// TODO: bail out?
261+
mLogger.printMessage(err);
262+
}
263+
}
264+
265+
std::list<std::pair<std::string, std::size_t>> files;
266+
if (!mSettings.fileFilters.empty()) {
267+
std::copy_if(filesResolved.cbegin(), filesResolved.cend(), std::inserter(files, files.end()), [&](const decltype(filesResolved)::value_type& entry) {
268+
return matchglobs(mSettings.fileFilters, entry.first);
269+
});
270+
if (files.empty()) {
271+
mLogger.printError("could not find any files matching the filter.");
272+
return false;
273+
}
274+
}
275+
else {
276+
files = std::move(filesResolved);
277+
}
278+
279+
// sort the markup last
280+
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
281+
return !mSettings.library.markupFile(entry.first) || !mSettings.library.processMarkupAfterCode(entry.first);
282+
});
283+
284+
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
285+
return mSettings.library.markupFile(entry.first) && mSettings.library.processMarkupAfterCode(entry.first);
286+
});
287+
}
288+
289+
if (mFiles.empty() && mFileSettings.empty()) {
290+
mLogger.printError("could not find or open any of the paths given.");
291+
if (!ignored.empty())
292+
mLogger.printMessage("Maybe all paths were ignored?");
293+
return false;
294+
}
295+
296+
return true;
297+
}
298+
119299
// TODO: normalize/simplify/native all path parameters
120300
// TODO: error out on all missing given files/paths
121301
bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
@@ -1444,3 +1624,90 @@ bool CmdLineParser::isCppcheckPremium() const {
14441624
mSettings.loadCppcheckCfg();
14451625
return startsWith(mSettings.cppcheckCfgProductName, "Cppcheck Premium");
14461626
}
1627+
1628+
bool CmdLineParser::tryLoadLibrary(Library& destination, const std::string& basepath, const char* filename)
1629+
{
1630+
const Library::Error err = destination.load(basepath.c_str(), filename);
1631+
1632+
if (err.errorcode == Library::ErrorCode::UNKNOWN_ELEMENT)
1633+
std::cout << "cppcheck: Found unknown elements in configuration file '" << filename << "': " << err.reason << std::endl;
1634+
else if (err.errorcode != Library::ErrorCode::OK) {
1635+
std::cout << "cppcheck: Failed to load library configuration file '" << filename << "'. ";
1636+
switch (err.errorcode) {
1637+
case Library::ErrorCode::OK:
1638+
break;
1639+
case Library::ErrorCode::FILE_NOT_FOUND:
1640+
std::cout << "File not found";
1641+
break;
1642+
case Library::ErrorCode::BAD_XML:
1643+
std::cout << "Bad XML";
1644+
break;
1645+
case Library::ErrorCode::UNKNOWN_ELEMENT:
1646+
std::cout << "Unexpected element";
1647+
break;
1648+
case Library::ErrorCode::MISSING_ATTRIBUTE:
1649+
std::cout << "Missing attribute";
1650+
break;
1651+
case Library::ErrorCode::BAD_ATTRIBUTE_VALUE:
1652+
std::cout << "Bad attribute value";
1653+
break;
1654+
case Library::ErrorCode::UNSUPPORTED_FORMAT:
1655+
std::cout << "File is of unsupported format version";
1656+
break;
1657+
case Library::ErrorCode::DUPLICATE_PLATFORM_TYPE:
1658+
std::cout << "Duplicate platform type";
1659+
break;
1660+
case Library::ErrorCode::PLATFORM_TYPE_REDEFINED:
1661+
std::cout << "Platform type redefined";
1662+
break;
1663+
}
1664+
if (!err.reason.empty())
1665+
std::cout << " '" + err.reason + "'";
1666+
std::cout << std::endl;
1667+
return false;
1668+
}
1669+
return true;
1670+
}
1671+
1672+
bool CmdLineParser::loadLibraries(Settings& settings)
1673+
{
1674+
if (!tryLoadLibrary(settings.library, settings.exename, "std.cfg")) {
1675+
const std::string msg("Failed to load std.cfg. Your Cppcheck installation is broken, please re-install.");
1676+
#ifdef FILESDIR
1677+
const std::string details("The Cppcheck binary was compiled with FILESDIR set to \""
1678+
FILESDIR "\" and will therefore search for "
1679+
"std.cfg in " FILESDIR "/cfg.");
1680+
#else
1681+
const std::string cfgfolder(Path::fromNativeSeparators(Path::getPathFromFilename(settings.exename)) + "cfg");
1682+
const std::string details("The Cppcheck binary was compiled without FILESDIR set. Either the "
1683+
"std.cfg should be available in " + cfgfolder + " or the FILESDIR "
1684+
"should be configured.");
1685+
#endif
1686+
std::cout << msg << " " << details << std::endl;
1687+
return false;
1688+
}
1689+
1690+
bool result = true;
1691+
for (const auto& lib : settings.libraries) {
1692+
if (!tryLoadLibrary(settings.library, settings.exename, lib.c_str())) {
1693+
result = false;
1694+
}
1695+
}
1696+
return result;
1697+
}
1698+
1699+
bool CmdLineParser::loadAddons(Settings& settings)
1700+
{
1701+
bool result = true;
1702+
for (const std::string &addon: settings.addons) {
1703+
AddonInfo addonInfo;
1704+
const std::string failedToGetAddonInfo = addonInfo.getAddonInfo(addon, settings.exename);
1705+
if (!failedToGetAddonInfo.empty()) {
1706+
std::cout << failedToGetAddonInfo << std::endl;
1707+
result = false;
1708+
continue;
1709+
}
1710+
settings.addonInfos.emplace_back(std::move(addonInfo));
1711+
}
1712+
return result;
1713+
}

cli/cmdlineparser.h

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <cstddef>
2323
#include <list>
2424
#include <string>
25+
#include <utility>
2526
#include <vector>
2627

2728
#include "cmdlinelogger.h"
@@ -30,6 +31,7 @@
3031

3132
class Settings;
3233
class Suppressions;
34+
class Library;
3335

3436
/// @addtogroup CLI
3537
/// @{
@@ -55,6 +57,16 @@ class CmdLineParser {
5557
*/
5658
CmdLineParser(CmdLineLogger &logger, Settings &settings, Suppressions &suppressions, Suppressions &suppressionsNoFail);
5759

60+
/**
61+
* @brief Parse command line args and fill settings and file lists
62+
* from there.
63+
*
64+
* @param argc argc from main()
65+
* @param argv argv from main()
66+
* @return false when errors are found in the input
67+
*/
68+
bool fillSettingsFromArgs(int argc, const char* const argv[]);
69+
5870
/**
5971
* Parse given command line.
6072
* @return true if command line was ok, false if there was an error.
@@ -82,6 +94,13 @@ class CmdLineParser {
8294
return mPathNames;
8395
}
8496

97+
/**
98+
* Return the files user gave to command line.
99+
*/
100+
const std::list<std::pair<std::string, std::size_t>>& getFiles() const {
101+
return mFiles;
102+
}
103+
85104
/**
86105
* Return the file settings read from command line.
87106
*/
@@ -130,9 +149,30 @@ class CmdLineParser {
130149
return true;
131150
}
132151

152+
/**
153+
* Tries to load a library and prints warning/error messages
154+
* @return false, if an error occurred (except unknown XML elements)
155+
*/
156+
static bool tryLoadLibrary(Library& destination, const std::string& basepath, const char* filename);
157+
158+
/**
159+
* @brief Load libraries
160+
* @param settings Settings
161+
* @return Returns true if successful
162+
*/
163+
static bool loadLibraries(Settings& settings);
164+
165+
/**
166+
* @brief Load addons
167+
* @param settings Settings
168+
* @return Returns true if successful
169+
*/
170+
static bool loadAddons(Settings& settings);
171+
133172
CmdLineLogger &mLogger;
134173

135174
std::vector<std::string> mPathNames;
175+
std::list<std::pair<std::string, std::size_t>> mFiles;
136176
std::list<FileSettings> mFileSettings;
137177
std::vector<std::string> mIgnoredPaths;
138178
Settings &mSettings;

0 commit comments

Comments
 (0)