Skip to content

Commit 7da51b8

Browse files
KiritoYGKiritoYG
committed
Explain configurations skipped by error directives in check-config
1 parent 8bb772d commit 7da51b8

3 files changed

Lines changed: 145 additions & 2 deletions

File tree

‎lib/cppcheck.cpp‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,8 +1106,16 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
11061106
}
11071107

11081108
if (mSettings.checkConfiguration) {
1109-
for (const std::string &config : configurations)
1110-
(void)preprocessor.getcode(config, files, false);
1109+
for (const std::string &config : configurations) {
1110+
simplecpp::OutputList outputList_cfg;
1111+
const simplecpp::TokenList tokensP = preprocessor.preprocess(config, files, outputList_cfg);
1112+
const simplecpp::Output* output = preprocessor.handleErrors(outputList_cfg);
1113+
// Other failures, and #error with explicit defines, are already reported by handleErrors.
1114+
if (output && output->type == simplecpp::Output::ERROR && startsWith(output->msg, "#error") &&
1115+
(mSettings.userDefines.empty() || mSettings.force)) {
1116+
invalidConfigurationMessage(file.spath(), tokensP.file(output->location), config, *output);
1117+
}
1118+
}
11111119

11121120
if (configurations.size() > maxConfigs)
11131121
tooManyConfigsError(Path::toNativeSeparators(file.spath()), configurations.size());
@@ -1752,6 +1760,33 @@ void CppCheck::purgedConfigurationMessage(const std::string &file, const std::st
17521760
mErrorLogger.reportErr(errmsg);
17531761
}
17541762

1763+
void CppCheck::invalidConfigurationMessage(const std::string& file0, const std::string& file,
1764+
const std::string& configuration, const simplecpp::Output& output)
1765+
{
1766+
std::list<ErrorMessage::FileLocation> locations;
1767+
if (!file.empty()) {
1768+
std::string filename = Path::fromNativeSeparators(file);
1769+
if (mSettings.relativePaths)
1770+
filename = Path::getRelativePath(filename, mSettings.basePaths);
1771+
locations.emplace_back(filename, output.location.line, output.location.col);
1772+
}
1773+
1774+
// preprocess() also applies userDefines; include them in the configuration shown to the user.
1775+
std::string effectiveConfig = mSettings.userDefines;
1776+
const std::vector<std::string> userDefines = split(mSettings.userDefines, ";");
1777+
for (const std::string& define : split(configuration, ";")) {
1778+
if (define.empty() || std::find(userDefines.cbegin(), userDefines.cend(), define) != userDefines.cend())
1779+
continue;
1780+
if (!effectiveConfig.empty())
1781+
effectiveConfig += ';';
1782+
effectiveConfig += define;
1783+
}
1784+
1785+
mErrorLogger.reportErr(ErrorMessage(std::move(locations), file0, Severity::information,
1786+
"The configuration '" + effectiveConfig + "' was not checked because of a preprocessor error: " + output.msg,
1787+
"invalidConfiguration", Certainty::normal));
1788+
}
1789+
17551790
//---------------------------------------------------------------------------
17561791

17571792
void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
@@ -1763,6 +1798,7 @@ void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
17631798
CppCheck cppcheck(settings, supprs, errorlogger, nullptr, true, nullptr);
17641799
cppcheck.purgedConfigurationMessage("","");
17651800
cppcheck.tooManyConfigsError("",0U);
1801+
cppcheck.invalidConfigurationMessage("", "", "", simplecpp::Output(simplecpp::Output::ERROR, {}, "#error"));
17661802
// TODO: add functions to get remaining error messages
17671803

17681804
Settings s;

‎lib/cppcheck.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ class CPPCHECKLIB CppCheck {
146146

147147
private:
148148
void purgedConfigurationMessage(const std::string &file, const std::string& configuration);
149+
void invalidConfigurationMessage(const std::string& file0, const std::string& file,
150+
const std::string& configuration, const simplecpp::Output& output);
149151

150152
bool isPremiumCodingStandardId(const std::string& id) const;
151153

‎test/testcppcheck.cpp‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@ class TestCppcheck : public TestFixture {
101101
TEST_CASE(checkPlistOutput);
102102
TEST_CASE(premiumResultsCache);
103103
TEST_CASE(purgedConfiguration);
104+
TEST_CASE(checkConfigurationInvalid);
105+
TEST_CASE(checkConfigurationValid);
106+
TEST_CASE(checkConfigurationSuppression);
107+
TEST_CASE(checkConfigurationExplicitError);
108+
TEST_CASE(checkConfigurationCombinedDefines);
109+
TEST_CASE(checkConfigurationNormalAnalysis);
110+
TEST_CASE(checkConfigurationHeader);
104111
TEST_CASE(recheckInclude);
105112
}
106113

@@ -126,6 +133,7 @@ class TestCppcheck : public TestFixture {
126133
bool foundTooManyConfigs = false;
127134
bool foundMissingInclude = false; // #11984
128135
bool foundMissingIncludeSystem = false; // #11984
136+
bool foundInvalidConfiguration = false;
129137
for (const std::string & it : errorLogger.ids) {
130138
if (it == "purgedConfiguration")
131139
foundPurgedConfiguration = true;
@@ -135,11 +143,14 @@ class TestCppcheck : public TestFixture {
135143
foundMissingInclude = true;
136144
else if (it == "missingIncludeSystem")
137145
foundMissingIncludeSystem = true;
146+
else if (it == "invalidConfiguration")
147+
foundInvalidConfiguration = true;
138148
}
139149
ASSERT(foundPurgedConfiguration);
140150
ASSERT(foundTooManyConfigs);
141151
ASSERT(foundMissingInclude);
142152
ASSERT(foundMissingIncludeSystem);
153+
ASSERT(foundInvalidConfiguration);
143154
}
144155

145156
static std::string exename_(const std::string& exe)
@@ -639,6 +650,100 @@ class TestCppcheck : public TestFixture {
639650
it->toString(false, templateFormat, ""));
640651
}
641652

653+
static const char* invalidConfigurationCode() {
654+
return "#ifndef PLATFORM\n"
655+
"#error Select PLATFORM\n"
656+
"#endif\n"
657+
"int base;\n"
658+
"#ifdef FEATURE\n"
659+
"int feature;\n"
660+
"#endif\n";
661+
}
662+
663+
std::list<ErrorMessage> configurationMessages(const char* code, const Settings& settings, const char* suppression = nullptr) const {
664+
const ScopedFile source("check-config.c", code);
665+
Settings configuredSettings = settings;
666+
configuredSettings.templateFormat = templateFormat;
667+
Suppressions supprs;
668+
if (suppression)
669+
ASSERT_EQUALS("", supprs.nomsg.addSuppressionLine(suppression));
670+
ErrorLogger2 errorLogger;
671+
CppCheck cppcheck(configuredSettings, supprs, errorLogger, nullptr, true, {});
672+
cppcheck.check(FileWithDetails(source.path(), Path::identify(source.path(), false), 0));
673+
errorLogger.errmsgs.remove_if([](const ErrorMessage& msg) {
674+
return msg.id == "logChecker";
675+
});
676+
return errorLogger.errmsgs;
677+
}
678+
679+
void checkConfigurationInvalid() const {
680+
// Trac #6672: FEATURE is considered independently of the required PLATFORM.
681+
const auto settings = dinit(Settings, $.checkConfiguration = true, $.templateFormat = templateFormat);
682+
const auto messages = configurationMessages(invalidConfigurationCode(), settings);
683+
ASSERT_EQUALS(1, messages.size());
684+
const ErrorMessage& msg = messages.front();
685+
ASSERT_EQUALS("invalidConfiguration", msg.id);
686+
ASSERT(msg.severity == Severity::information);
687+
ASSERT_EQUALS("check-config.c", msg.file0);
688+
ASSERT_EQUALS(1, msg.callStack.size());
689+
ASSERT_EQUALS("check-config.c", msg.callStack.back().getfile(false));
690+
ASSERT_EQUALS(2, msg.callStack.back().line);
691+
ASSERT(msg.shortMessage().find("FEATURE") != std::string::npos);
692+
ASSERT(msg.shortMessage().find("#error Select PLATFORM") != std::string::npos);
693+
}
694+
695+
void checkConfigurationValid() const {
696+
const auto settings = dinit(Settings, $.checkConfiguration = true, $.userDefines = "PLATFORM=1");
697+
ASSERT(configurationMessages(invalidConfigurationCode(), settings).empty());
698+
699+
const auto automaticSettings = dinit(Settings, $.checkConfiguration = true);
700+
ASSERT(configurationMessages("#if 0\n#error inactive\n#endif\nint value;\n", automaticSettings).empty());
701+
ASSERT(configurationMessages("#ifdef FEATURE\nint feature;\n#endif\nint value;\n", automaticSettings).empty());
702+
}
703+
704+
void checkConfigurationSuppression() const {
705+
const auto settings = dinit(Settings, $.checkConfiguration = true);
706+
ASSERT(configurationMessages(invalidConfigurationCode(), settings, "invalidConfiguration").empty());
707+
}
708+
709+
void checkConfigurationExplicitError() const {
710+
const auto settings = dinit(Settings, $.checkConfiguration = true, $.userDefines = "FEATURE=1");
711+
const auto messages = configurationMessages(invalidConfigurationCode(), settings);
712+
ASSERT_EQUALS(1, messages.size());
713+
ASSERT_EQUALS("preprocessorErrorDirective", messages.front().id);
714+
ASSERT(messages.front().severity == Severity::error);
715+
}
716+
717+
void checkConfigurationCombinedDefines() const {
718+
const auto settings = dinit(Settings, $.checkConfiguration = true, $.force = true, $.userDefines = "EXTRA=7");
719+
const auto messages = configurationMessages(invalidConfigurationCode(), settings);
720+
ASSERT_EQUALS(1, messages.size());
721+
ASSERT_EQUALS("invalidConfiguration", messages.front().id);
722+
const std::string message = messages.front().shortMessage();
723+
ASSERT(message.find("EXTRA=7;FEATURE") != std::string::npos);
724+
ASSERT(message.find("EXTRA=7", message.find("EXTRA=7") + 1) == std::string::npos);
725+
}
726+
727+
void checkConfigurationNormalAnalysis() const {
728+
const auto settings = dinit(Settings, $.force = true, $.severity.enable(Severity::information));
729+
ASSERT(configurationMessages(invalidConfigurationCode(), settings).empty());
730+
}
731+
732+
void checkConfigurationHeader() const {
733+
const ScopedFile header("config-error.h", "#ifndef PLATFORM\n#error Select PLATFORM\n#endif\n");
734+
const auto settings = dinit(Settings, $.checkConfiguration = true);
735+
const auto messages = configurationMessages("#include \"config-error.h\"\n#ifdef FEATURE\nint feature;\n#endif\n", settings);
736+
// Both the empty and FEATURE configurations fail in the included header.
737+
ASSERT_EQUALS(2, messages.size());
738+
ASSERT(messages.front().shortMessage() != messages.back().shortMessage());
739+
for (const ErrorMessage& message : messages) {
740+
ASSERT_EQUALS("invalidConfiguration", message.id);
741+
ASSERT_EQUALS("check-config.c", message.file0);
742+
ASSERT_EQUALS("config-error.h", message.callStack.back().getfile(false));
743+
ASSERT_EQUALS(2, message.callStack.back().line);
744+
}
745+
}
746+
642747
void recheckInclude() const
643748
{
644749
const auto settings = dinit(Settings,

0 commit comments

Comments
 (0)