avoid some redundant and unused settings in tests among other cleanup…

…s / added and used `WARN_UNUSED` attribute (danmar#5284)
chrchr-github · Aug 9, 2023 · 2502897 · 2502897
1 parent 8166bfc
commit 2502897
Show file tree

Hide file tree

Showing 18 changed files with 85 additions and 55 deletions.
diff --git 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 <string>

diff --git a/lib/settings.h b/lib/settings.h
@@ -90,7 +90,7 @@ class SimpleEnableGroup {
  * 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
@@ -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
@@ -130,49 +130,66 @@ class TestFixture : public ErrorLogger {
         check.runChecks(tokenizer, settings, errorLogger);
     }
 
-    // TODO: bail out on redundant settings
     class SettingsBuilder
     {
     public:
         explicit SettingsBuilder(const TestFixture &fixture) : fixture(fixture) {}
         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 @@ class TestFixture : public ErrorLogger {
         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 @@ class TestFixture : public ErrorLogger {
     private:
         const TestFixture &fixture;
         Settings settings;
+
+        const bool REDUNDANT_CHECK = false;
     };
 
     SettingsBuilder settingsBuilder() const {

diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
@@ -76,7 +76,7 @@ class TestBufferOverrun : public TestFixture {
         // 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
@@ -39,7 +39,7 @@ class TestClass : public TestFixture {
 
 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
@@ -127,7 +127,7 @@ class TestCondition : public TestFixture {
         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 @@ class TestCondition : public TestFixture {
     }
 
     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 @@ class TestCondition : public TestFixture {
     }
 
     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
@@ -63,7 +63,7 @@ class TestExceptionSafety : public TestFixture {
         // 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 @@ class TestExceptionSafety : public TestFixture {
         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
@@ -1562,18 +1562,21 @@ class TestFunctions : public TestFixture {
 
         {
             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 @@ class TestFunctions : public TestFixture {
     }
 
     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
@@ -474,7 +474,7 @@ class TestLeakAutoVar : public TestFixture {
     }
 
     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 @@ class TestLeakAutoVar : public TestFixture {
     }
 
     void functionCallCastConfig() { // #9652
-        Settings settingsFunctionCall = settings;
-
         const char xmldata[] = "<?xml version=\"1.0\"?>\n"
                                "<def format=\"2\">\n"
                                "  <function name=\"free_func\">\n"
@@ -2778,7 +2776,8 @@ class TestLeakAutoVar : public TestFixture {
                                "    </arg>\n"
                                "  </function>\n"
                                "</def>";
-        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 @@ class TestLeakAutoVar : public TestFixture {
                                "    <arg nr=\"1\" direction=\"in\"/>\n"
                                "  </function>\n"
                                "</def>\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
@@ -77,12 +77,13 @@ class TestProcessExecutor : public TestFixture {
             }
         }
 
-        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<std::unique_ptr<ScopedFile>> scopedfiles;
         scopedfiles.reserve(filemap.size());
         for (std::map<std::string, std::size_t>::const_iterator i = filemap.cbegin(); i != filemap.cend(); ++i)

diff --git a/test/testsimplifytemplate.cpp b/test/testsimplifytemplate.cpp
@@ -38,7 +38,7 @@ class TestSimplifyTemplate : public TestFixture {
 
 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
@@ -34,11 +34,10 @@ class TestSimplifyTokens : public TestFixture {
 
 
 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
@@ -40,10 +40,8 @@ class TestSimplifyTypedef : public TestFixture {
 
 
 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 @@ class TestSimplifyTypedef : public TestFixture {
         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
@@ -39,10 +39,7 @@ class TestSimplifyUsing : public TestFixture {
 
 
 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
@@ -68,9 +68,8 @@ class TestSymbolDatabase : public TestFixture {
 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;