mitigated most bugprone-unused-return-value clang-tidy warnings in …

…test code when we enable it for *all* functions (#6673) We should not have any unchecked returns within the test code to make sure everything is working as expected (there might be a case that should also apply to the production code). Unfortunately enabling all functions is abusing this check and it will produce a lot of "false positives" (e.g. `erase()`/`insert()` of containers or usage of stream insertion operators) so this cannot be enabled by default. Since `llvm:Regex` does not support negative lookahead we cannot configure the check to exclude functions.
danmar · Aug 17, 2024 · d89e241 · d89e241
1 parent cd20dce
commit d89e241
Show file tree

Hide file tree

Showing 27 changed files with 565 additions and 553 deletions.
diff --git a/test/.clang-tidy b/test/.clang-tidy
@@ -0,0 +1,7 @@
+---
+InheritParentConfig: true
+CheckOptions:
+  #- key:             bugprone-unused-return-value.CheckedFunctions
+  #  value:           '::.*'
+  - key:             bugprone-unused-return-value.AllowCastToVoid
+    value:           'true'
diff --git a/test/fixture.cpp b/test/fixture.cpp
@@ -216,9 +216,9 @@ std::string TestFixture::deleteLineNumber(const std::string &message)
     return result;
 }
 
-void TestFixture::assertEqualsWithoutLineNumbers(const char * const filename, const unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg) const
+bool TestFixture::assertEqualsWithoutLineNumbers(const char * const filename, const unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg) const
 {
-    assertEquals(filename, linenr, deleteLineNumber(expected), deleteLineNumber(actual), msg);
+    return assertEquals(filename, linenr, deleteLineNumber(expected), deleteLineNumber(actual), msg);
 }
 
 bool TestFixture::assertEquals(const char * const filename, const unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg) const
@@ -237,20 +237,21 @@ bool TestFixture::assertEquals(const char * const filename, const unsigned int l
 bool TestFixture::assertEquals(const char * const filename, const unsigned int linenr, const long long expected, const long long actual, const std::string &msg) const
 {
     if (expected != actual) {
-        assertEquals(filename, linenr, std::to_string(expected), std::to_string(actual), msg);
+        return assertEquals(filename, linenr, std::to_string(expected), std::to_string(actual), msg);
     }
-    return expected == actual;
+    return true;
 }
 
-void TestFixture::assertEqualsDouble(const char * const filename, const unsigned int linenr, const double expected, const double actual, const double tolerance, const std::string &msg) const
+bool TestFixture::assertEqualsDouble(const char * const filename, const unsigned int linenr, const double expected, const double actual, const double tolerance, const std::string &msg) const
 {
     if (expected < (actual - tolerance) || expected > (actual + tolerance)) {
         std::ostringstream ostr1;
         ostr1 << expected;
         std::ostringstream ostr2;
         ostr2 << actual;
-        assertEquals(filename, linenr, ostr1.str(), ostr2.str(), msg);
+        return assertEquals(filename, linenr, ostr1.str(), ostr2.str(), msg);
     }
+    return false;
 }
 
 void TestFixture::todoAssertEquals(const char * const filename, const unsigned int linenr,
@@ -264,7 +265,7 @@ void TestFixture::todoAssertEquals(const char * const filename, const unsigned i
 
         ++succeeded_todos_counter;
     } else {
-        assertEquals(filename, linenr, current, actual);
+        (void)assertEquals(filename, linenr, current, actual);
         ++todos_counter;
     }
 }

diff --git a/test/fixture.h b/test/fixture.h
@@ -95,12 +95,12 @@ class TestFixture : public ErrorLogger {
     void assertEqualsFailed(const char* filename, unsigned int linenr, const std::string& expected, const std::string& actual, const std::string& msg) const;
 
     bool assertEquals(const char * filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = emptyString) const;
-    void assertEqualsWithoutLineNumbers(const char * filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = emptyString) const;
+    bool assertEqualsWithoutLineNumbers(const char * filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = emptyString) const;
     bool assertEquals(const char * filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg = emptyString) const;
     bool assertEquals(const char * filename, unsigned int linenr, const char expected[], const char actual[], const std::string &msg = emptyString) const;
     bool assertEquals(const char * filename, unsigned int linenr, const std::string& expected, const char actual[], const std::string &msg = emptyString) const;
     bool assertEquals(const char * filename, unsigned int linenr, long long expected, long long actual, const std::string &msg = emptyString) const;
-    void assertEqualsDouble(const char * filename, unsigned int linenr, double expected, double actual, double tolerance, const std::string &msg = emptyString) const;
+    bool assertEqualsDouble(const char * filename, unsigned int linenr, double expected, double actual, double tolerance, const std::string &msg = emptyString) const;
 
     void todoAssertEquals(const char * filename, unsigned int linenr, const std::string &wanted,
                           const std::string &current, const std::string &actual) const;
@@ -296,30 +296,30 @@ class TestInstance {
 };
 
 // TODO: most asserts do not actually assert i.e. do not return
-#define TEST_CASE( NAME )  do { if (prepareTest(#NAME)) { setVerbose(false); try { NAME(); teardownTest(); } catch (...) { assertNoThrowFail(__FILE__, __LINE__); } } } while (false)
-#define ASSERT( CONDITION )  if (!assert_(__FILE__, __LINE__, (CONDITION))) return
-#define ASSERT_LOC( CONDITION, FILE_, LINE_ )  assert_(FILE_, LINE_, (CONDITION))
-#define CHECK_EQUALS( EXPECTED, ACTUAL )  assertEquals(__FILE__, __LINE__, (EXPECTED), (ACTUAL))
+#define TEST_CASE( NAME ) do { if (prepareTest(#NAME)) { setVerbose(false); try { NAME(); teardownTest(); } catch (...) { assertNoThrowFail(__FILE__, __LINE__); } } } while (false)
+#define ASSERT( CONDITION ) if (!assert_(__FILE__, __LINE__, (CONDITION))) return
+#define ASSERT_LOC( CONDITION, FILE_, LINE_ ) (void)assert_(FILE_, LINE_, (CONDITION))
+#define CHECK_EQUALS( EXPECTED, ACTUAL ) (void)assertEquals(__FILE__, __LINE__, (EXPECTED), (ACTUAL))
 // *INDENT-OFF*
 #define ASSERT_EQUALS( EXPECTED, ACTUAL ) do { try { if (!assertEquals(__FILE__, __LINE__, (EXPECTED), (ACTUAL))) return; } catch (...) { assertNoThrowFail(__FILE__, __LINE__); } } while (false)
 // *INDENT-ON*
-#define ASSERT_EQUALS_WITHOUT_LINENUMBERS( EXPECTED, ACTUAL )  assertEqualsWithoutLineNumbers(__FILE__, __LINE__, EXPECTED, ACTUAL)
-#define ASSERT_EQUALS_DOUBLE( EXPECTED, ACTUAL, TOLERANCE )  assertEqualsDouble(__FILE__, __LINE__, EXPECTED, ACTUAL, TOLERANCE)
-#define ASSERT_EQUALS_LOC_MSG( EXPECTED, ACTUAL, MSG, FILE_, LINE_ )  assertEquals(FILE_, LINE_, EXPECTED, ACTUAL, MSG)
-#define ASSERT_EQUALS_MSG( EXPECTED, ACTUAL, MSG )  assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL, MSG)
-#define ASSERT_EQUALS_ENUM( EXPECTED, ACTUAL )  if (!assertEqualsEnum(__FILE__, __LINE__, (EXPECTED), (ACTUAL))) return
-#define ASSERT_THROW( CMD, EXCEPTION ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&) {} catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
-#define ASSERT_THROW_EQUALS( CMD, EXCEPTION, EXPECTED ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { assertEquals(__FILE__, __LINE__, EXPECTED, e.errorMessage); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
-#define ASSERT_THROW_EQUALS_2( CMD, EXCEPTION, EXPECTED ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { assertEquals(__FILE__, __LINE__, EXPECTED, e.what()); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
-#define ASSERT_THROW_INTERNAL( CMD, TYPE ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const InternalError& e) { assertEqualsEnum(__FILE__, __LINE__, InternalError::TYPE, e.type); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
-#define ASSERT_THROW_INTERNAL_EQUALS( CMD, TYPE, EXPECTED ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const InternalError& e) { assertEqualsEnum(__FILE__, __LINE__, InternalError::TYPE, e.type); assertEquals(__FILE__, __LINE__, EXPECTED, e.errorMessage); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
-#define ASSERT_NO_THROW( CMD ) do { try { CMD; } catch (...) { assertNoThrowFail(__FILE__, __LINE__); } } while (false)
-#define TODO_ASSERT_THROW( CMD, EXCEPTION ) do { try { CMD; } catch (const EXCEPTION&) {} catch (...) { assertThrow(__FILE__, __LINE__); } } while (false)
+#define ASSERT_EQUALS_WITHOUT_LINENUMBERS( EXPECTED, ACTUAL ) (void)assertEqualsWithoutLineNumbers(__FILE__, __LINE__, EXPECTED, ACTUAL)
+#define ASSERT_EQUALS_DOUBLE( EXPECTED, ACTUAL, TOLERANCE ) (void)assertEqualsDouble(__FILE__, __LINE__, EXPECTED, ACTUAL, TOLERANCE)
+#define ASSERT_EQUALS_LOC_MSG( EXPECTED, ACTUAL, MSG, FILE_, LINE_ ) (void)assertEquals(FILE_, LINE_, EXPECTED, ACTUAL, MSG)
+#define ASSERT_EQUALS_MSG( EXPECTED, ACTUAL, MSG ) (void)assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL, MSG)
+#define ASSERT_EQUALS_ENUM( EXPECTED, ACTUAL ) if (!assertEqualsEnum(__FILE__, __LINE__, (EXPECTED), (ACTUAL))) return
+#define ASSERT_THROW( CMD, EXCEPTION ) do { try { (void)(CMD); assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&) {} catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
+#define ASSERT_THROW_EQUALS( CMD, EXCEPTION, EXPECTED ) do { try { (void)(CMD); assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { (void)assertEquals(__FILE__, __LINE__, EXPECTED, e.errorMessage); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
+#define ASSERT_THROW_EQUALS_2( CMD, EXCEPTION, EXPECTED ) do { try { (void)(CMD); assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { (void)assertEquals(__FILE__, __LINE__, EXPECTED, e.what()); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
+#define ASSERT_THROW_INTERNAL( CMD, TYPE ) do { try { (void)(CMD); assertThrowFail(__FILE__, __LINE__); } catch (const InternalError& e) { (void)assertEqualsEnum(__FILE__, __LINE__, InternalError::TYPE, e.type); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
+#define ASSERT_THROW_INTERNAL_EQUALS( CMD, TYPE, EXPECTED ) do { try { (void)(CMD); assertThrowFail(__FILE__, __LINE__); } catch (const InternalError& e) { (void)assertEqualsEnum(__FILE__, __LINE__, InternalError::TYPE, e.type); (void)assertEquals(__FILE__, __LINE__, EXPECTED, e.errorMessage); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false)
+#define ASSERT_NO_THROW( CMD ) do { try { (void)(CMD); } catch (...) { assertNoThrowFail(__FILE__, __LINE__); } } while (false)
+#define TODO_ASSERT_THROW( CMD, EXCEPTION ) do { try { (void)(CMD); } catch (const EXCEPTION&) {} catch (...) { assertThrow(__FILE__, __LINE__); } } while (false)
 #define TODO_ASSERT( CONDITION ) do { const bool condition=(CONDITION); todoAssertEquals(__FILE__, __LINE__, true, false, condition); } while (false)
 #define TODO_ASSERT_EQUALS( WANTED, CURRENT, ACTUAL ) todoAssertEquals(__FILE__, __LINE__, WANTED, CURRENT, ACTUAL)
-#define EXPECT_EQ( EXPECTED, ACTUAL ) assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL)
+#define EXPECT_EQ( EXPECTED, ACTUAL ) (void)assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL)
 #define REGISTER_TEST( CLASSNAME ) namespace { class CLASSNAME ## Instance : public TestInstance { public: CLASSNAME ## Instance() : TestInstance(#CLASSNAME) {} TestFixture* create() override { impl.reset(new CLASSNAME); return impl.get(); } }; CLASSNAME ## Instance instance_ ## CLASSNAME; }
 
-#define PLATFORM( P, T ) do { std::string errstr; assertEquals(__FILE__, __LINE__, true, P.set(Platform::toString(T), errstr, {exename}), errstr); } while (false)
+#define PLATFORM( P, T ) do { std::string errstr; (void)assertEquals(__FILE__, __LINE__, true, P.set(Platform::toString(T), errstr, {exename}), errstr); } while (false)
 
 #endif // fixtureH
diff --git a/test/testastutils.cpp b/test/testastutils.cpp
@@ -231,10 +231,10 @@ class TestAstUtils : public TestFixture {
 
     void isVariableChangedTest() {
         // #8211 - no lhs for >> , do not crash
-        isVariableChanged("void f() {\n"
-                          "  int b;\n"
-                          "  if (b) { (int)((INTOF(8))result >> b); }\n"
-                          "}", "if", "}");
+        (void)isVariableChanged("void f() {\n"
+                                "  int b;\n"
+                                "  if (b) { (int)((INTOF(8))result >> b); }\n"
+                                "}", "if", "}");
         // #9235
         ASSERT_EQUALS(false,
                       isVariableChanged("void f() {\n"

diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
@@ -5204,7 +5204,7 @@ class TestBufferOverrun : public TestFixture {
         std::list<Check::FileInfo*> fileInfo;
         Check& c = getCheck<CheckBufferOverrun>();
         fileInfo.push_back(c.getFileInfo(tokenizer, settings0));
-        c.analyseWholeProgram(ctu, fileInfo, settings0, *this);
+        c.analyseWholeProgram(ctu, fileInfo, settings0, *this); // TODO: check result
         while (!fileInfo.empty()) {
             delete fileInfo.back();
             fileInfo.pop_back();

diff --git a/test/testclangimport.cpp b/test/testclangimport.cpp
@@ -1363,7 +1363,7 @@ class TestClangImport : public TestFixture {
                             "`-CXXConstructorDecl 0x5603791b5600 parent 0x560379197110 prev 0x5603791974b8 <line:3:1, col:55> col:47 b 'void (A<type-parameter-0-0, type-parameter-0-1> &)'\n"
                             "  |-ParmVarDecl 0x5603791b5570 <col:49, col:51> col:52 'A<type-parameter-0-0, type-parameter-0-1> &'\n"
                             "  `-CompoundStmt 0x5603791b5700 <col:54, col:55>\n";
-        parse(clang); // don't crash
+        (void)parse(clang); // don't crash
     }
 };
 

diff --git a/test/testclass.cpp b/test/testclass.cpp
@@ -8933,7 +8933,7 @@ class TestClass : public TestFixture {
         }
 
         // Check code..
-        check.analyseWholeProgram(nullptr, fileInfo, settingsDefault, *this);
+        check.analyseWholeProgram(nullptr, fileInfo, settingsDefault, *this); // TODO: check result
 
         while (!fileInfo.empty()) {
             delete fileInfo.back();