From 2502897265d4b0ee533e082d77d83f745142bc76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Wed, 9 Aug 2023 12:43:55 +0200 Subject: [PATCH] avoid some redundant and unused settings in tests among other cleanups / added and used `WARN_UNUSED` attribute (#5284) --- lib/config.h | 7 +++++++ lib/settings.h | 2 +- test/fixture.cpp | 7 +++++++ test/fixture.h | 25 ++++++++++++++++++++++++- test/testbufferoverrun.cpp | 2 +- test/testclass.cpp | 2 +- test/testcondition.cpp | 6 +++--- test/testexceptionsafety.cpp | 4 ++-- test/testfunctions.cpp | 24 +++++++++++++----------- test/testleakautovar.cpp | 9 ++++----- test/testprocessexecutor.cpp | 9 +++++---- test/testsimplifytemplate.cpp | 2 +- test/testsimplifytokens.cpp | 9 ++++----- test/testsimplifytypedef.cpp | 8 +++----- test/testsimplifyusing.cpp | 5 +---- test/testsymboldatabase.cpp | 5 ++--- test/testtokenize.cpp | 12 +++++------- test/testvarid.cpp | 2 +- 18 files changed, 85 insertions(+), 55 deletions(-) diff --git a/lib/config.h b/lib/config.h index a0cbee516..2f45debe0 100644 --- a/lib/config.h +++ b/lib/config.h @@ -79,6 +79,13 @@ # define UNUSED #endif +// warn_unused +#if (defined(__clang__) && (__clang_major__ >= 15)) +# define WARN_UNUSED [[gnu::warn_unused]] +#else +# define WARN_UNUSED +#endif + #define REQUIRES(msg, ...) class=typename std::enable_if<__VA_ARGS__::value>::type #include diff --git a/lib/settings.h b/lib/settings.h index 38c7111fb..ab27b1dbd 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -90,7 +90,7 @@ public: * to pass individual values to functions or constructors now or in the * future when we might have even more detailed settings. */ -class CPPCHECKLIB Settings { +class CPPCHECKLIB WARN_UNUSED Settings { private: /** @brief terminate checking */ diff --git a/test/fixture.cpp b/test/fixture.cpp index 64b1471c4..e332fbf7a 100644 --- a/test/fixture.cpp +++ b/test/fixture.cpp @@ -399,6 +399,8 @@ void TestFixture::setTemplateFormat(const std::string &templateFormat) } TestFixture::SettingsBuilder& TestFixture::SettingsBuilder::library(const char lib[]) { + if (REDUNDANT_CHECK && std::find(settings.libraries.cbegin(), settings.libraries.cend(), lib) != settings.libraries.cend()) + throw std::runtime_error("redundant setting: libraries (" + std::string(lib) + ")"); // TODO: exename is not yet set LOAD_LIB_2_EXE(settings.library, lib, fixture.exename.c_str()); // strip extension @@ -414,6 +416,11 @@ TestFixture::SettingsBuilder& TestFixture::SettingsBuilder::library(const char l TestFixture::SettingsBuilder& TestFixture::SettingsBuilder::platform(cppcheck::Platform::Type type) { const std::string platformStr = cppcheck::Platform::toString(type); + + // TODO: the default platform differs between Windows and Linux + //if (REDUNDANT_CHECK && settings.platform.type == type) + // throw std::runtime_error("redundant setting: platform (" + platformStr + ")"); + std::string errstr; // TODO: exename is not yet set if (!settings.platform.set(platformStr, errstr, {fixture.exename})) diff --git a/test/fixture.h b/test/fixture.h index 2a3c26eca..a09a193f0 100644 --- a/test/fixture.h +++ b/test/fixture.h @@ -130,7 +130,6 @@ protected: check.runChecks(tokenizer, settings, errorLogger); } - // TODO: bail out on redundant settings class SettingsBuilder { public: @@ -138,41 +137,59 @@ protected: SettingsBuilder(const TestFixture &fixture, Settings settings) : fixture(fixture), settings(std::move(settings)) {} SettingsBuilder& severity(Severity::SeverityType sev, bool b = true) { + if (REDUNDANT_CHECK && settings.severity.isEnabled(sev) == b) + throw std::runtime_error("redundant setting: severity"); settings.severity.setEnabled(sev, b); return *this; } SettingsBuilder& certainty(Certainty cert, bool b = true) { + if (REDUNDANT_CHECK && settings.certainty.isEnabled(cert) == b) + throw std::runtime_error("redundant setting: certainty"); settings.certainty.setEnabled(cert, b); return *this; } SettingsBuilder& clang() { + if (REDUNDANT_CHECK && settings.clang) + throw std::runtime_error("redundant setting: clang"); settings.clang = true; return *this; } SettingsBuilder& checkLibrary() { + if (REDUNDANT_CHECK && settings.checkLibrary) + throw std::runtime_error("redundant setting: checkLibrary"); settings.checkLibrary = true; return *this; } SettingsBuilder& checkUnusedTemplates(bool b = true) { + if (REDUNDANT_CHECK && settings.checkUnusedTemplates == b) + throw std::runtime_error("redundant setting: checkUnusedTemplates"); settings.checkUnusedTemplates = b; return *this; } SettingsBuilder& debugwarnings(bool b = true) { + if (REDUNDANT_CHECK && settings.debugwarnings == b) + throw std::runtime_error("redundant setting: debugwarnings"); settings.debugwarnings = b; return *this; } SettingsBuilder& c(Standards::cstd_t std) { + // TODO: CLatest and C11 are the same - handle differently + //if (REDUNDANT_CHECK && settings.standards.c == std) + // throw std::runtime_error("redundant setting: standards.c"); settings.standards.c = std; return *this; } SettingsBuilder& cpp(Standards::cppstd_t std) { + // TODO: CPPLatest and CPP20 are the same - handle differently + //if (REDUNDANT_CHECK && settings.standards.cpp == std) + // throw std::runtime_error("redundant setting: standards.cpp"); settings.standards.cpp = std; return *this; } @@ -184,11 +201,15 @@ protected: SettingsBuilder& platform(cppcheck::Platform::Type type); SettingsBuilder& checkConfiguration() { + if (REDUNDANT_CHECK && settings.checkConfiguration) + throw std::runtime_error("redundant setting: checkConfiguration"); settings.checkConfiguration = true; return *this; } SettingsBuilder& checkHeaders(bool b = true) { + if (REDUNDANT_CHECK && settings.checkHeaders == b) + throw std::runtime_error("redundant setting: checkHeaders"); settings.checkHeaders = b; return *this; } @@ -199,6 +220,8 @@ protected: private: const TestFixture &fixture; Settings settings; + + const bool REDUNDANT_CHECK = false; }; SettingsBuilder settingsBuilder() const { diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 97841f45a..862add419 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -76,7 +76,7 @@ private: // Clear the error buffer.. errout.str(""); - const Settings settings = settingsBuilder(settings0).severity(Severity::style).severity(Severity::warning).severity(Severity::portability).severity(Severity::performance) + const Settings settings = settingsBuilder(settings0).severity(Severity::performance) .c(Standards::CLatest).cpp(Standards::CPPLatest).certainty(Certainty::inconclusive).build(); // Raw tokens.. diff --git a/test/testclass.cpp b/test/testclass.cpp index 430fb6c09..b2a343a44 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -39,7 +39,7 @@ public: private: Settings settings0 = settingsBuilder().severity(Severity::style).library("std.cfg").build(); - Settings settings1 = settingsBuilder().severity(Severity::warning).library("std.cfg").build(); + const Settings settings1 = settingsBuilder().severity(Severity::warning).library("std.cfg").build(); void run() override { TEST_CASE(virtualDestructor1); // Base class not found => no error diff --git a/test/testcondition.cpp b/test/testcondition.cpp index b9a7d6e5c..96212fef6 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -127,7 +127,7 @@ private: TEST_CASE(knownConditionIncrementLoop); // #9808 } - void check(const char code[], Settings &settings, const char* filename = "test.cpp") { + void check(const char code[], const Settings &settings, const char* filename = "test.cpp") { // Clear the error buffer.. errout.str(""); @@ -154,7 +154,7 @@ private: } void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) { - Settings settings = settingsBuilder(settings0).certainty(Certainty::inconclusive, inconclusive).build(); + const Settings settings = settingsBuilder(settings0).certainty(Certainty::inconclusive, inconclusive).build(); check(code, settings, filename); } @@ -5645,7 +5645,7 @@ private: } void compareOutOfTypeRange() { - Settings settingsUnix64 = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Unix64).build(); + const Settings settingsUnix64 = settingsBuilder().severity(Severity::style).platform(cppcheck::Platform::Type::Unix64).build(); check("void f(unsigned char c) {\n" " if (c == 256) {}\n" diff --git a/test/testexceptionsafety.cpp b/test/testexceptionsafety.cpp index 274a895a6..97df82275 100644 --- a/test/testexceptionsafety.cpp +++ b/test/testexceptionsafety.cpp @@ -63,7 +63,7 @@ private: // Clear the error buffer.. errout.str(""); - Settings settings1 = settingsBuilder(s ? *s : settings).certainty(Certainty::inconclusive, inconclusive).build(); + const Settings settings1 = settingsBuilder(s ? *s : settings).certainty(Certainty::inconclusive, inconclusive).build(); // Tokenize.. Tokenizer tokenizer(&settings1, this); @@ -397,7 +397,7 @@ private: ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:1]: (style, inconclusive) Unhandled exception specification when calling function f().\n" "[test.cpp:6] -> [test.cpp:1]: (style, inconclusive) Unhandled exception specification when calling function f().\n", errout.str()); - const Settings s = settingsBuilder(settings).library("gnu.cfg").build(); + const Settings s = settingsBuilder().library("gnu.cfg").build(); check(code, true, &s); ASSERT_EQUALS("", errout.str()); } diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp index 418f36130..9d895dcec 100644 --- a/test/testfunctions.cpp +++ b/test/testfunctions.cpp @@ -1562,18 +1562,21 @@ private: { const char code[] = "int main(void) {}"; - Settings s; + { + const Settings s = settingsBuilder().c(Standards::C89).build(); - s.standards.c = Standards::C89; - check(code, "test.c", &s); // c code (c89) - ASSERT_EQUALS("[test.c:1]: (error) Found an exit path from function with non-void return type that has missing return statement\n", errout.str()); + check(code, "test.c", &s); // c code (c89) + ASSERT_EQUALS("[test.c:1]: (error) Found an exit path from function with non-void return type that has missing return statement\n", errout.str()); + } - s.standards.c = Standards::C99; - check(code, "test.c", &s); // c code (c99) - ASSERT_EQUALS("", errout.str()); + { + const Settings s = settingsBuilder().c(Standards::C99).build(); + check(code, "test.c", &s); // c code (c99) + ASSERT_EQUALS("", errout.str()); - check(code, "test.cpp", &s); // c++ code - ASSERT_EQUALS("", errout.str()); + check(code, "test.cpp", &s); // c++ code + ASSERT_EQUALS("", errout.str()); + } } check("F(A,B) { x=1; }"); @@ -1825,9 +1828,8 @@ private: } void checkLibraryMatchFunctions() { - Settings s = settingsBuilder(settings).checkLibrary().build(); + Settings s = settingsBuilder(settings).checkLibrary().debugwarnings().build(); s.daca = true; - s.debugwarnings = true; check("void f() {\n" " lib_func();" diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp index 11eca62d3..c5e2dca4f 100644 --- a/test/testleakautovar.cpp +++ b/test/testleakautovar.cpp @@ -474,7 +474,7 @@ private: } void assign23() { - const Settings s = settingsBuilder(settings).library("posix.cfg").build(); + const Settings s = settingsBuilder().library("posix.cfg").build(); check("void f() {\n" " int n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14;\n" " *&n1 = open(\"xx.log\", O_RDONLY);\n" @@ -2764,8 +2764,6 @@ private: } void functionCallCastConfig() { // #9652 - Settings settingsFunctionCall = settings; - const char xmldata[] = "\n" "\n" " \n" @@ -2778,7 +2776,8 @@ private: " \n" " \n" ""; - ASSERT(settingsFunctionCall.library.loadxmldata(xmldata, sizeof(xmldata))); + const Settings settingsFunctionCall = settingsBuilder(settings).libraryxml(xmldata, sizeof(xmldata)).build(); + check("void test_func()\n" "{\n" " char * buf = malloc(4);\n" @@ -2810,7 +2809,7 @@ private: " \n" " \n" "\n"; - const Settings settingsLeakIgnore = settingsBuilder(settings).libraryxml(xmldata, sizeof(xmldata)).build(); + const Settings settingsLeakIgnore = settingsBuilder().libraryxml(xmldata, sizeof(xmldata)).build(); check("void f() {\n" " double* a = new double[1024];\n" " SomeClass::someMethod(a);\n" diff --git a/test/testprocessexecutor.cpp b/test/testprocessexecutor.cpp index bda5f0a3c..5bd701eab 100644 --- a/test/testprocessexecutor.cpp +++ b/test/testprocessexecutor.cpp @@ -77,12 +77,13 @@ private: } } - settings.jobs = jobs; - settings.showtime = opt.showtime; + Settings s = settings; + s.jobs = jobs; + s.showtime = opt.showtime; if (opt.plistOutput) - settings.plistOutput = opt.plistOutput; + s.plistOutput = opt.plistOutput; // TODO: test with settings.project.fileSettings; - ProcessExecutor executor(filemap, settings, settings.nomsg, *this); + ProcessExecutor executor(filemap, s, s.nomsg, *this); std::vector> scopedfiles; scopedfiles.reserve(filemap.size()); for (std::map::const_iterator i = filemap.cbegin(); i != filemap.cend(); ++i) diff --git a/test/testsimplifytemplate.cpp b/test/testsimplifytemplate.cpp index 92e66e36f..73b851991 100644 --- a/test/testsimplifytemplate.cpp +++ b/test/testsimplifytemplate.cpp @@ -38,7 +38,7 @@ public: private: // If there are unused templates, keep those - const Settings settings = settingsBuilder().severity(Severity::portability).checkUnusedTemplates().build(); + const Settings settings = settingsBuilder().severity(Severity::portability).build(); void run() override { TEST_CASE(template1); diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index 574e364e6..1e633a0f0 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -34,11 +34,10 @@ public: private: - // If there are unused templates, keep those - const Settings settings0 = settingsBuilder().severity(Severity::portability).checkUnusedTemplates().build(); - const Settings settings1 = settingsBuilder().severity(Severity::style).checkUnusedTemplates().build(); - const Settings settings_std = settingsBuilder().library("std.cfg").checkUnusedTemplates().build(); - const Settings settings_windows = settingsBuilder().library("windows.cfg").severity(Severity::portability).checkUnusedTemplates().build(); + const Settings settings0 = settingsBuilder().severity(Severity::portability).build(); + const Settings settings1 = settingsBuilder().severity(Severity::style).build(); + const Settings settings_std = settingsBuilder().library("std.cfg").build(); + const Settings settings_windows = settingsBuilder().library("windows.cfg").severity(Severity::portability).build(); void run() override { TEST_CASE(combine_strings); diff --git a/test/testsimplifytypedef.cpp b/test/testsimplifytypedef.cpp index 55b3c085c..a87595e3e 100644 --- a/test/testsimplifytypedef.cpp +++ b/test/testsimplifytypedef.cpp @@ -40,10 +40,8 @@ public: private: - // If there are unused templates, keep those - const Settings settings0 = settingsBuilder().severity(Severity::style).checkUnusedTemplates().build(); - const Settings settings1 = settingsBuilder().severity(Severity::portability).checkUnusedTemplates().build(); - const Settings settings2 = settingsBuilder().severity(Severity::style).checkUnusedTemplates().build(); + const Settings settings0 = settingsBuilder().severity(Severity::style).build(); + const Settings settings1 = settingsBuilder().severity(Severity::portability).build(); void run() override { TEST_CASE(c1); @@ -285,7 +283,7 @@ private: errout.str(""); // Tokenize.. // show warnings about unhandled typedef - const Settings settings = settingsBuilder(settings2).certainty(Certainty::inconclusive).debugwarnings().build(); + const Settings settings = settingsBuilder(settings0).certainty(Certainty::inconclusive).debugwarnings().build(); Tokenizer tokenizer(&settings, this); std::istringstream istr(code); ASSERT_LOC(tokenizer.tokenize(istr, "test.cpp"), file, line); diff --git a/test/testsimplifyusing.cpp b/test/testsimplifyusing.cpp index fb6c92bb9..32287681a 100644 --- a/test/testsimplifyusing.cpp +++ b/test/testsimplifyusing.cpp @@ -39,10 +39,7 @@ public: private: - // If there are unused templates, keep those - const Settings settings0 = settingsBuilder().severity(Severity::style).checkUnusedTemplates().build(); - const Settings settings1 = settingsBuilder().checkUnusedTemplates().build(); - const Settings settings2 = settingsBuilder().severity(Severity::style).checkUnusedTemplates().build(); + const Settings settings0 = settingsBuilder().severity(Severity::style).build(); void run() override { TEST_CASE(simplifyUsing1); diff --git a/test/testsymboldatabase.cpp b/test/testsymboldatabase.cpp index 3ca632831..13e719e46 100644 --- a/test/testsymboldatabase.cpp +++ b/test/testsymboldatabase.cpp @@ -68,9 +68,8 @@ public: private: const Token* vartok{nullptr}; const Token* typetok{nullptr}; - // If there are unused templates, keep those - Settings settings1 = settingsBuilder().library("std.cfg").checkUnusedTemplates().build(); - const Settings settings2 = settingsBuilder().checkUnusedTemplates().platform(cppcheck::Platform::Type::Unspecified).build(); + Settings settings1 = settingsBuilder().library("std.cfg").build(); + const Settings settings2 = settingsBuilder().platform(cppcheck::Platform::Type::Unspecified).build(); void reset() { vartok = nullptr; diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 2e0972d07..e5814f6e5 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -44,11 +44,9 @@ public: TestTokenizer() : TestFixture("TestTokenizer") {} private: - // If there are unused templates, keep those - const Settings settings0 = settingsBuilder().library("qt.cfg").checkUnusedTemplates().build(); - const Settings settings1 = settingsBuilder().library("qt.cfg").library("std.cfg").checkUnusedTemplates().build(); - const Settings settings2 = settingsBuilder().library("qt.cfg").checkUnusedTemplates().build(); - const Settings settings_windows = settingsBuilder().library("windows.cfg").checkUnusedTemplates().build(); + const Settings settings0 = settingsBuilder().library("qt.cfg").build(); + const Settings settings1 = settingsBuilder().library("qt.cfg").library("std.cfg").build(); + const Settings settings_windows = settingsBuilder().library("windows.cfg").build(); void run() override { TEST_CASE(tokenize1); @@ -522,7 +520,7 @@ private: std::string tokenizeDebugListing_(const char* file, int line, const char code[], const char filename[] = "test.cpp") { errout.str(""); - const Settings settings = settingsBuilder(settings2).c(Standards::C89).cpp(Standards::CPP03).build(); + const Settings settings = settingsBuilder(settings0).c(Standards::C89).cpp(Standards::CPP03).build(); Tokenizer tokenizer(&settings, this); std::istringstream istr(code); @@ -946,7 +944,7 @@ private: } { - const Settings s = settingsBuilder().checkUnusedTemplates().build(); + const Settings s; ASSERT_EQUALS("; template < typename T , u_int uBAR = 0 >\n" "class Foo {\n" "public:\n" diff --git a/test/testvarid.cpp b/test/testvarid.cpp index 90dfeafe6..341c24c2d 100644 --- a/test/testvarid.cpp +++ b/test/testvarid.cpp @@ -34,7 +34,7 @@ public: TestVarID() : TestFixture("TestVarID") {} private: - Settings settings = settingsBuilder().c(Standards::C89).cpp(Standards::CPPLatest).checkUnusedTemplates().platform(cppcheck::Platform::Type::Unix64).build(); + const Settings settings = settingsBuilder().c(Standards::C89).cpp(Standards::CPPLatest).platform(cppcheck::Platform::Type::Unix64).build(); void run() override { TEST_CASE(varid1); TEST_CASE(varid2);