Fix various useful clang-tidy warnings

* bugprone-branch-clone
* bugprone-copy-constructor-init
* bugprone-empty-catch
* bugprone-sizeof-expression
* bugprone-switch-missing-default-case
* bugprone-unused-local-non-trivial-variable
* clang-analyzer-core.uninitialized.Assign
* clang-analyzer-cplusplus.Move
* clang-analyzer-optin.cplusplus.VirtualCall
* modernize-loop-convert
* modernize-raw-string-literal
* modernize-use-equals-default
* modernize-use-override
* modernize-use-using
* performance-avoid-endl
* performance-inefficient-string-concatenation
* performance-inefficient-vector-operation
* performance-noexcept-move-constructor
* performance-unnecessary-value-param (and improve generator example)
* readability-duplicate-include
* readability-inconsistent-declaration-parameter-name
* readability-non-const-parameter
* readability-redundant-casting
* readability-redundant-member-init
* readability-redundant-smartptr-get
* readability-static-accessed-through-instance
* unused variable in amalgamted tests

12 files changed, 35 insertions, 36 deletions

diff --git a/tests/SelfTest/IntrospectiveTests/Details.tests.cpp b/tests/SelfTest/IntrospectiveTests/Details.tests.cpp
index d7175756..5566bb59 100644
--- a/tests/SelfTest/IntrospectiveTests/Details.tests.cpp
+++ b/tests/SelfTest/IntrospectiveTests/Details.tests.cpp
@@ -120,13 +120,13 @@ TEST_CASE( "Optional supports move ops", "[optional][approvals]" ) {
     }
     SECTION( "Move construction from optional" ) {
         Optional<MoveChecker> opt_B( CATCH_MOVE( opt_A ) );
-        REQUIRE( opt_A->has_moved );
+        REQUIRE( opt_A->has_moved ); // NOLINT(clang-analyzer-cplusplus.Move)
     }
     SECTION( "Move assignment from optional" ) {
         Optional<MoveChecker> opt_B( opt_A );
         REQUIRE_FALSE( opt_A->has_moved );
         opt_B = CATCH_MOVE( opt_A );
-        REQUIRE( opt_A->has_moved );
+        REQUIRE( opt_A->has_moved ); // NOLINT(clang-analyzer-cplusplus.Move)
     }
 }
 
diff --git a/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp b/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp
index acfeebed..14c90114 100644
--- a/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp
+++ b/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp
@@ -18,7 +18,6 @@
 #include <catch2/generators/catch_generators_adapters.hpp>
 #include <catch2/generators/catch_generators_random.hpp>
 #include <catch2/generators/catch_generators_range.hpp>
-#include <catch2/generators/catch_generator_exception.hpp>
 
 // Tests of generator implementation details
 TEST_CASE("Generators internals", "[generators][internals]") {
diff --git a/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp b/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp
index e5a65bda..86ba1175 100644
--- a/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp
+++ b/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp
@@ -107,7 +107,7 @@ TEST_CASE( "Reporter's write listings to provided stream", "[reporters]" ) {
     for (auto const& factory : factories) {
         INFO("Tested reporter: " << factory.first);
         auto sstream = Catch::Detail::make_unique<StringIStream>();
-        auto& sstreamRef = *sstream.get();
+        auto& sstreamRef = *sstream;
 
         Catch::ConfigData cfg_data;
         cfg_data.rngSeed = 1234;
diff --git a/tests/SelfTest/IntrospectiveTests/TestSpec.tests.cpp b/tests/SelfTest/IntrospectiveTests/TestSpec.tests.cpp
index 9c4eb03b..11a7a7a8 100644
--- a/tests/SelfTest/IntrospectiveTests/TestSpec.tests.cpp
+++ b/tests/SelfTest/IntrospectiveTests/TestSpec.tests.cpp
@@ -236,7 +236,7 @@ TEST_CASE( "Parse test names and tags", "[command-line][test-spec][approvals]" )
         CHECK( spec.matches( *tcD ) == false );
     }
     SECTION( "two wildcarded names" ) {
-        TestSpec spec = parseTestSpec( "\"longer*\"\"*spaces\"" );
+        TestSpec spec = parseTestSpec( R"("longer*""*spaces")" );
         CHECK( spec.hasFilters() == true );
         CHECK( spec.matches( *tcA ) == false );
         CHECK( spec.matches( *tcB ) == false );
diff --git a/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp b/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp
index 82de5a27..653f65ba 100644
--- a/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp
+++ b/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp
@@ -152,7 +152,7 @@ TEST_CASE( "TextFlow::Column respects indentation for empty lines",
 
     std::string written = as_written(col);
 
-    REQUIRE(as_written(col) == "  \n  \n  third line");
+    REQUIRE(written == "  \n  \n  third line");
 }
 
 TEST_CASE( "TextFlow::Column leading/trailing whitespace",
diff --git a/tests/SelfTest/UsageTests/Benchmark.tests.cpp b/tests/SelfTest/UsageTests/Benchmark.tests.cpp
index 557b2131..c489eaa8 100644
--- a/tests/SelfTest/UsageTests/Benchmark.tests.cpp
+++ b/tests/SelfTest/UsageTests/Benchmark.tests.cpp
@@ -90,14 +90,14 @@ TEST_CASE("Benchmark containers", "[!benchmark]") {
         };
         REQUIRE(v.size() == size);
 
-        int array[size];
+        int array[size] {};
         BENCHMARK("A fixed size array that should require no allocations") {
             for (int i = 0; i < size; ++i)
                 array[i] = i;
         };
         int sum = 0;
-        for (int i = 0; i < size; ++i)
-            sum += array[i];
+        for (int val : array)
+            sum += val;
         REQUIRE(sum > size);
 
         SECTION("XYZ") {
@@ -121,8 +121,8 @@ TEST_CASE("Benchmark containers", "[!benchmark]") {
                 runs = benchmarkIndex;
             };
 
-            for (size_t i = 0; i < v.size(); ++i) {
-                REQUIRE(v[i] == runs);
+            for (int val : v) {
+                REQUIRE(val == runs);
             }
         }
     }
@@ -135,8 +135,8 @@ TEST_CASE("Benchmark containers", "[!benchmark]") {
             for (int i = 0; i < size; ++i)
                 v[i] = generated;
         };
-        for (size_t i = 0; i < v.size(); ++i) {
-            REQUIRE(v[i] == generated);
+        for (int val : v) {
+            REQUIRE(val == generated);
         }
     }
 
diff --git a/tests/SelfTest/UsageTests/Class.tests.cpp b/tests/SelfTest/UsageTests/Class.tests.cpp
index bab7a684..682171da 100644
--- a/tests/SelfTest/UsageTests/Class.tests.cpp
+++ b/tests/SelfTest/UsageTests/Class.tests.cpp
@@ -39,7 +39,7 @@ namespace {
     };
 
     template <typename T> struct Template_Fixture_2 {
-        Template_Fixture_2() {}
+        Template_Fixture_2() = default;
 
         T m_a;
     };
diff --git a/tests/SelfTest/UsageTests/Exception.tests.cpp b/tests/SelfTest/UsageTests/Exception.tests.cpp
index 4f91a30c..7c6b0c86 100644
--- a/tests/SelfTest/UsageTests/Exception.tests.cpp
+++ b/tests/SelfTest/UsageTests/Exception.tests.cpp
@@ -119,7 +119,7 @@ TEST_CASE( "When unchecked exceptions are thrown, but caught, they do not affect
     try {
         throw std::domain_error( "unexpected exception" );
     }
-    catch(...) {}
+    catch(...) {} // NOLINT(bugprone-empty-catch)
 }
 
 
@@ -152,7 +152,7 @@ TEST_CASE( "Custom exceptions can be translated when testing for throwing as som
 }
 
 TEST_CASE( "Unexpected exceptions can be translated", "[.][failing][!throws]"  ) {
-    throw double( 3.14 );
+    throw double( 3.14 ); // NOLINT(readability-redundant-casting): the type is important here, so we want to be explicit
 }
 
 TEST_CASE("Thrown string literals are translated", "[.][failing][!throws]") {
diff --git a/tests/SelfTest/UsageTests/Matchers.tests.cpp b/tests/SelfTest/UsageTests/Matchers.tests.cpp
index 74bedf5e..7c4501c6 100644
--- a/tests/SelfTest/UsageTests/Matchers.tests.cpp
+++ b/tests/SelfTest/UsageTests/Matchers.tests.cpp
@@ -1027,7 +1027,6 @@ TEST_CASE( "Combining MatchNotOfGeneric does not nest",
 }
 
 struct EvilAddressOfOperatorUsed : std::exception {
-    EvilAddressOfOperatorUsed() {}
     const char* what() const noexcept override {
         return "overloaded address-of operator of matcher was used instead of "
                "std::addressof";
@@ -1035,7 +1034,6 @@ struct EvilAddressOfOperatorUsed : std::exception {
 };
 
 struct EvilCommaOperatorUsed : std::exception {
-    EvilCommaOperatorUsed() {}
     const char* what() const noexcept override {
         return "overloaded comma operator of matcher was used";
     }
@@ -1073,7 +1071,6 @@ struct ImmovableMatcher : Catch::Matchers::MatcherGenericBase {
 };
 
 struct MatcherWasMovedOrCopied : std::exception {
-    MatcherWasMovedOrCopied() {}
     const char* what() const noexcept override {
         return "attempted to copy or move a matcher";
     }
@@ -1081,17 +1078,20 @@ struct MatcherWasMovedOrCopied : std::exception {
 
 struct ThrowOnCopyOrMoveMatcher : Catch::Matchers::MatcherGenericBase {
     ThrowOnCopyOrMoveMatcher() = default;
-    [[noreturn]] ThrowOnCopyOrMoveMatcher( ThrowOnCopyOrMoveMatcher const& ):
-        Catch::Matchers::MatcherGenericBase() {
+
+    [[noreturn]] ThrowOnCopyOrMoveMatcher( ThrowOnCopyOrMoveMatcher const& other ):
+        Catch::Matchers::MatcherGenericBase( other ) {
         throw MatcherWasMovedOrCopied();
     }
-    [[noreturn]] ThrowOnCopyOrMoveMatcher( ThrowOnCopyOrMoveMatcher&& ):
-        Catch::Matchers::MatcherGenericBase() {
+    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
+    [[noreturn]] ThrowOnCopyOrMoveMatcher( ThrowOnCopyOrMoveMatcher&& other ):
+        Catch::Matchers::MatcherGenericBase( CATCH_MOVE(other) ) {
         throw MatcherWasMovedOrCopied();
     }
     ThrowOnCopyOrMoveMatcher& operator=( ThrowOnCopyOrMoveMatcher const& ) {
         throw MatcherWasMovedOrCopied();
     }
+    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     ThrowOnCopyOrMoveMatcher& operator=( ThrowOnCopyOrMoveMatcher&& ) {
         throw MatcherWasMovedOrCopied();
     }
diff --git a/tests/SelfTest/UsageTests/Message.tests.cpp b/tests/SelfTest/UsageTests/Message.tests.cpp
index 6367bf59..7626e005 100644
--- a/tests/SelfTest/UsageTests/Message.tests.cpp
+++ b/tests/SelfTest/UsageTests/Message.tests.cpp
@@ -80,20 +80,20 @@ TEST_CASE( "Output from all sections is reported", "[failing][messages][.]" ) {
 
 TEST_CASE( "Standard output from all sections is reported", "[messages][.]" ) {
     SECTION( "one" ) {
-        std::cout << "Message from section one" << std::endl;
+        std::cout << "Message from section one\n";
     }
 
     SECTION( "two" ) {
-        std::cout << "Message from section two" << std::endl;
+        std::cout << "Message from section two\n";
     }
 }
 
 TEST_CASE( "Standard error is reported and redirected", "[messages][.][approvals]" ) {
     SECTION( "std::cerr" ) {
-        std::cerr << "Write to std::cerr" << std::endl;
+        std::cerr << "Write to std::cerr\n";
     }
     SECTION( "std::clog" ) {
-        std::clog << "Write to std::clog" << std::endl;
+        std::clog << "Write to std::clog\n";
     }
     SECTION( "Interleaved writes to cerr and clog" ) {
         std::cerr << "Inter";
@@ -101,7 +101,7 @@ TEST_CASE( "Standard error is reported and redirected", "[messages][.][approvals
         std::cerr << ' ';
         std::clog << "writes";
         std::cerr << " to error";
-        std::clog << " streams" << std::endl;
+        std::clog << " streams\n" << std::flush;
     }
 }
 
diff --git a/tests/SelfTest/UsageTests/Misc.tests.cpp b/tests/SelfTest/UsageTests/Misc.tests.cpp
index 7f06704b..f08c8ef1 100644
--- a/tests/SelfTest/UsageTests/Misc.tests.cpp
+++ b/tests/SelfTest/UsageTests/Misc.tests.cpp
@@ -158,9 +158,9 @@ TEST_CASE( "looped tests", "[.][failing]" ) {
 }
 
 TEST_CASE( "Sends stuff to stdout and stderr", "[.]" ) {
-    std::cout << "A string sent directly to stdout" << std::endl;
-    std::cerr << "A string sent directly to stderr" << std::endl;
-    std::clog << "A string sent to stderr via clog" << std::endl;
+    std::cout << "A string sent directly to stdout\n" << std::flush;
+    std::cerr << "A string sent directly to stderr\n" << std::flush;
+    std::clog << "A string sent to stderr via clog\n" << std::flush;
 }
 
 TEST_CASE( "null strings" ) {
@@ -396,7 +396,7 @@ TEMPLATE_PRODUCT_TEST_CASE("Product with differing arities", "[template][product
 using MyTypes = std::tuple<int, char, float>;
 TEMPLATE_LIST_TEST_CASE("Template test case with test types specified inside std::tuple", "[template][list]", MyTypes)
 {
-    REQUIRE(sizeof(TestType) > 0);
+    REQUIRE(std::is_arithmetic<TestType>::value);
 }
 
 struct NonDefaultConstructibleType {
@@ -406,7 +406,7 @@ struct NonDefaultConstructibleType {
 using MyNonDefaultConstructibleTypes = std::tuple<NonDefaultConstructibleType, float>;
 TEMPLATE_LIST_TEST_CASE("Template test case with test types specified inside non-default-constructible std::tuple", "[template][list]", MyNonDefaultConstructibleTypes)
 {
-    REQUIRE(sizeof(TestType) > 0);
+    REQUIRE(std::is_trivially_copyable<TestType>::value);
 }
 
 struct NonCopyableAndNonMovableType {
@@ -421,7 +421,7 @@ struct NonCopyableAndNonMovableType {
 using NonCopyableAndNonMovableTypes = std::tuple<NonCopyableAndNonMovableType, float>;
 TEMPLATE_LIST_TEST_CASE("Template test case with test types specified inside non-copyable and non-movable std::tuple", "[template][list]", NonCopyableAndNonMovableTypes)
 {
-    REQUIRE(sizeof(TestType) > 0);
+    REQUIRE(std::is_default_constructible<TestType>::value);
 }
 
 // https://github.com/philsquared/Catch/issues/166
diff --git a/tests/SelfTest/UsageTests/Tricky.tests.cpp b/tests/SelfTest/UsageTests/Tricky.tests.cpp
index 25770340..041d7867 100644
--- a/tests/SelfTest/UsageTests/Tricky.tests.cpp
+++ b/tests/SelfTest/UsageTests/Tricky.tests.cpp
@@ -261,7 +261,7 @@ TEST_CASE( "non streamable - with conv. op", "[Tricky]" )
 
 inline void foo() {}
 
-typedef void (*fooptr_t)();
+using fooptr_t = void (*)();
 
 TEST_CASE( "Comparing function pointers", "[Tricky][function pointer]" )
 {
@@ -281,7 +281,7 @@ struct S
 
 TEST_CASE( "Comparing member function pointers", "[Tricky][member function pointer][approvals]" )
 {
-    typedef void (S::*MF)();
+    using MF = void (S::*)();
     MF m = &S::f;
 
     CHECK( m == &S::f );