Skip to content

Commit

Permalink
Fix various useful clang-tidy warnings
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mjerabek authored and horenmar committed Mar 1, 2024
1 parent 7677c16 commit cde3509
Show file tree
Hide file tree
Showing 39 changed files with 86 additions and 87 deletions.
3 changes: 1 addition & 2 deletions examples/210-Evt-EventListeners.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,7 @@ struct MyListener : Catch::EventListenerBase {
CATCH_REGISTER_LISTENER( MyListener )

// Get rid of Wweak-tables
MyListener::~MyListener() {}

MyListener::~MyListener() = default;

// -----------------------------------------------------------------------
// 3. Test cases:
Expand Down
2 changes: 1 addition & 1 deletion examples/231-Cfg-OutputStreams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class out_buff : public std::stringbuf {
std::FILE* m_stream;
public:
out_buff(std::FILE* stream):m_stream(stream) {}
~out_buff();
~out_buff() override;
int sync() override {
int ret = 0;
for (unsigned char c : str()) {
Expand Down
2 changes: 1 addition & 1 deletion examples/232-Cfg-CustomMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ int main(int argc, char** argv) {
return returnCode;

// if set on the command line then 'height' is now set at this point
std::cout << "height: " << height << std::endl;
std::cout << "height: " << height << '\n';

return session.run();
}
2 changes: 1 addition & 1 deletion examples/300-Gen-OwnGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
namespace {

// This class shows how to implement a simple generator for Catch tests
class RandomIntGenerator : public Catch::Generators::IGenerator<int> {
class RandomIntGenerator final : public Catch::Generators::IGenerator<int> {
std::minstd_rand m_rand;
std::uniform_int_distribution<> m_dist;
int current_number;
Expand Down
17 changes: 9 additions & 8 deletions examples/301-Gen-MapTypeConversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ namespace {
// Returns a line from a stream. You could have it e.g. read lines from
// a file, but to avoid problems with paths in examples, we will use
// a fixed stringstream.
class LineGenerator : public Catch::Generators::IGenerator<std::string> {
class LineGenerator final : public Catch::Generators::IGenerator<std::string> {
std::string m_line;
std::stringstream m_stream;
public:
LineGenerator() {
m_stream.str("1\n2\n3\n4\n");
explicit LineGenerator( std::string const& lines ) {
m_stream.str( lines );
if (!next()) {
Catch::Generators::Detail::throw_generator_exception("Couldn't read a single line");
}
Expand All @@ -49,18 +49,19 @@ std::string const& LineGenerator::get() const {
// This helper function provides a nicer UX when instantiating the generator
// Notice that it returns an instance of GeneratorWrapper<std::string>, which
// is a value-wrapper around std::unique_ptr<IGenerator<std::string>>.
Catch::Generators::GeneratorWrapper<std::string> lines(std::string /* ignored for example */) {
Catch::Generators::GeneratorWrapper<std::string>
lines( std::string const& lines ) {
return Catch::Generators::GeneratorWrapper<std::string>(
new LineGenerator()
);
new LineGenerator( lines ) );
}

} // end anonymous namespace


TEST_CASE("filter can convert types inside the generator expression", "[example][generator]") {
auto num = GENERATE(map<int>([](std::string const& line) { return std::stoi(line); },
lines("fake-file")));
auto num = GENERATE(
map<int>( []( std::string const& line ) { return std::stoi( line ); },
lines( "1\n2\n3\n4\n" ) ) );

REQUIRE(num > 0);
}
Expand Down
1 change: 1 addition & 0 deletions src/catch2/catch_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ namespace Catch {
m_messages.back().message += " := ";
start = pos;
}
default:; // noop
}
}
assert(openings.empty() && "Mismatched openings");
Expand Down
1 change: 0 additions & 1 deletion src/catch2/catch_registry_hub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <catch2/internal/catch_noncopyable.hpp>
#include <catch2/interfaces/catch_interfaces_reporter_factory.hpp>
#include <catch2/internal/catch_move_and_forward.hpp>
#include <catch2/internal/catch_reporter_registry.hpp>

#include <exception>

Expand Down
2 changes: 1 addition & 1 deletion src/catch2/catch_test_case_info.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ namespace Catch {
struct TestCaseInfo : Detail::NonCopyable {

TestCaseInfo(StringRef _className,
NameAndTags const& _tags,
NameAndTags const& _nameAndTags,
SourceLineInfo const& _lineInfo);

bool isHidden() const;
Expand Down
10 changes: 5 additions & 5 deletions src/catch2/catch_tostring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ namespace Detail {
}
} // end unnamed namespace

std::string convertIntoString(StringRef string, bool escape_invisibles) {
std::string convertIntoString(StringRef string, bool escapeInvisibles) {
std::string ret;
// This is enough for the "don't escape invisibles" case, and a good
// lower bound on the "escape invisibles" case.
ret.reserve(string.size() + 2);

if (!escape_invisibles) {
if (!escapeInvisibles) {
ret += '"';
ret += string;
ret += '"';
Expand Down Expand Up @@ -138,7 +138,7 @@ std::string StringMaker<char const*>::convert(char const* str) {
return{ "{null string}" };
}
}
std::string StringMaker<char*>::convert(char* str) {
std::string StringMaker<char*>::convert(char* str) { // NOLINT(readability-non-const-parameter)
if (str) {
return Detail::convertIntoString( str );
} else {
Expand Down Expand Up @@ -235,8 +235,8 @@ std::string StringMaker<signed char>::convert(signed char value) {
std::string StringMaker<char>::convert(char c) {
return ::Catch::Detail::stringify(static_cast<signed char>(c));
}
std::string StringMaker<unsigned char>::convert(unsigned char c) {
return ::Catch::Detail::stringify(static_cast<char>(c));
std::string StringMaker<unsigned char>::convert(unsigned char value) {
return ::Catch::Detail::stringify(static_cast<char>(value));
}

int StringMaker<float>::precision = 5;
Expand Down
4 changes: 2 additions & 2 deletions src/catch2/catch_tostring.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,11 @@ namespace Catch {
};
template<>
struct StringMaker<signed char> {
static std::string convert(signed char c);
static std::string convert(signed char value);
};
template<>
struct StringMaker<unsigned char> {
static std::string convert(unsigned char c);
static std::string convert(unsigned char value);
};

template<>
Expand Down
2 changes: 1 addition & 1 deletion src/catch2/internal/catch_commandline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace Catch {
line = trim(line);
if( !line.empty() && !startsWith( line, '#' ) ) {
if( !startsWith( line, '"' ) )
line = '"' + line + '"';
line = '"' + CATCH_MOVE(line) + '"';
config.testsOrTags.push_back( line );
config.testsOrTags.emplace_back( "," );
}
Expand Down
12 changes: 6 additions & 6 deletions src/catch2/internal/catch_console_colour.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,21 +230,21 @@ namespace {

namespace Catch {

Detail::unique_ptr<ColourImpl> makeColourImpl( ColourMode implSelection,
Detail::unique_ptr<ColourImpl> makeColourImpl( ColourMode colourSelection,
IStream* stream ) {
#if defined( CATCH_CONFIG_COLOUR_WIN32 )
if ( implSelection == ColourMode::Win32 ) {
if ( colourSelection == ColourMode::Win32 ) {
return Detail::make_unique<Win32ColourImpl>( stream );
}
#endif
if ( implSelection == ColourMode::ANSI ) {
if ( colourSelection == ColourMode::ANSI ) {
return Detail::make_unique<ANSIColourImpl>( stream );
}
if ( implSelection == ColourMode::None ) {
if ( colourSelection == ColourMode::None ) {
return Detail::make_unique<NoColourImpl>( stream );
}

if ( implSelection == ColourMode::PlatformDefault) {
if ( colourSelection == ColourMode::PlatformDefault) {
#if defined( CATCH_CONFIG_COLOUR_WIN32 )
if ( Win32ColourImpl::useImplementationForStream( *stream ) ) {
return Detail::make_unique<Win32ColourImpl>( stream );
Expand All @@ -256,7 +256,7 @@ namespace Catch {
return Detail::make_unique<NoColourImpl>( stream );
}

CATCH_ERROR( "Could not create colour impl for selection " << static_cast<int>(implSelection) );
CATCH_ERROR( "Could not create colour impl for selection " << static_cast<int>(colourSelection) );
}

bool isColourImplAvailable( ColourMode colourSelection ) {
Expand Down
2 changes: 1 addition & 1 deletion src/catch2/internal/catch_enum_values_registry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace Catch {

std::vector<Catch::Detail::unique_ptr<EnumInfo>> m_enumInfos;

EnumInfo const& registerEnum( StringRef enumName, StringRef allEnums, std::vector<int> const& values) override;
EnumInfo const& registerEnum( StringRef enumName, StringRef allValueNames, std::vector<int> const& values) override;
};

std::vector<StringRef> parseEnums( StringRef enums );
Expand Down
4 changes: 2 additions & 2 deletions src/catch2/internal/catch_jsonwriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace Catch {
m_os{ os }, m_indent_level{ indent_level } {
m_os << '{';
}
JsonObjectWriter::JsonObjectWriter( JsonObjectWriter&& source ):
JsonObjectWriter::JsonObjectWriter( JsonObjectWriter&& source ) noexcept:
m_os{ source.m_os },
m_indent_level{ source.m_indent_level },
m_should_comma{ source.m_should_comma },
Expand Down Expand Up @@ -62,7 +62,7 @@ namespace Catch {
m_os{ os }, m_indent_level{ indent_level } {
m_os << '[';
}
JsonArrayWriter::JsonArrayWriter( JsonArrayWriter&& source ):
JsonArrayWriter::JsonArrayWriter( JsonArrayWriter&& source ) noexcept:
m_os{ source.m_os },
m_indent_level{ source.m_indent_level },
m_should_comma{ source.m_should_comma },
Expand Down
4 changes: 2 additions & 2 deletions src/catch2/internal/catch_jsonwriter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ namespace Catch {
JsonObjectWriter( std::ostream& os );
JsonObjectWriter( std::ostream& os, std::uint64_t indent_level );

JsonObjectWriter( JsonObjectWriter&& source );
JsonObjectWriter( JsonObjectWriter&& source ) noexcept;
JsonObjectWriter& operator=( JsonObjectWriter&& source ) = delete;

~JsonObjectWriter();
Expand All @@ -84,7 +84,7 @@ namespace Catch {
JsonArrayWriter( std::ostream& os );
JsonArrayWriter( std::ostream& os, std::uint64_t indent_level );

JsonArrayWriter( JsonArrayWriter&& source );
JsonArrayWriter( JsonArrayWriter&& source ) noexcept;
JsonArrayWriter& operator=( JsonArrayWriter&& source ) = delete;

~JsonArrayWriter();
Expand Down
2 changes: 1 addition & 1 deletion src/catch2/internal/catch_reporter_spec_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ namespace Catch {
auto kv = splitKVPair( parts[i] );
auto key = kv.key, value = kv.value;

if ( key.empty() || value.empty() ) {
if ( key.empty() || value.empty() ) { // NOLINT(bugprone-branch-clone)
return {};
} else if ( key[0] == 'X' ) {
// This is a reporter-specific option, we don't check these
Expand Down
4 changes: 2 additions & 2 deletions src/catch2/internal/catch_stringref.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ namespace Catch {
constexpr const_iterator end() const { return m_start + m_size; }


friend std::string& operator += (std::string& lhs, StringRef sr);
friend std::ostream& operator << (std::ostream& os, StringRef sr);
friend std::string& operator += (std::string& lhs, StringRef rhs);
friend std::ostream& operator << (std::ostream& os, StringRef str);
friend std::string operator+(StringRef lhs, StringRef rhs);

/**
Expand Down
4 changes: 2 additions & 2 deletions src/catch2/reporters/catch_reporter_console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -520,8 +520,8 @@ void ConsoleReporter::testRunEnded(TestRunStats const& _testRunStats) {
m_stream << '\n' << std::flush;
StreamingReporterBase::testRunEnded(_testRunStats);
}
void ConsoleReporter::testRunStarting(TestRunInfo const& _testInfo) {
StreamingReporterBase::testRunStarting(_testInfo);
void ConsoleReporter::testRunStarting(TestRunInfo const& _testRunInfo) {
StreamingReporterBase::testRunStarting(_testRunInfo);
if ( m_config->testSpec().hasFilters() ) {
m_stream << m_colour->guardColour( Colour::BrightYellow ) << "Filters: "
<< m_config->testSpec() << '\n';
Expand Down
3 changes: 1 addition & 2 deletions src/catch2/reporters/catch_reporter_cumulative_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ namespace Catch {
namespace {
struct BySectionInfo {
BySectionInfo( SectionInfo const& other ): m_other( other ) {}
BySectionInfo( BySectionInfo const& other ):
m_other( other.m_other ) {}
BySectionInfo( BySectionInfo const& other ) = default;
bool operator()(
Detail::unique_ptr<CumulativeReporterBase::SectionNode> const&
node ) const {
Expand Down
4 changes: 2 additions & 2 deletions src/catch2/reporters/catch_reporter_json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ namespace Catch {
return "Outputs listings as JSON. Test listing is Work-in-Progress!";
}

void JsonReporter::testRunStarting( TestRunInfo const& testInfo ) {
StreamingReporterBase::testRunStarting( testInfo );
void JsonReporter::testRunStarting( TestRunInfo const& runInfo ) {
StreamingReporterBase::testRunStarting( runInfo );
endListing();

assert( isInside( Writer::Object ) );
Expand Down
2 changes: 1 addition & 1 deletion src/catch2/reporters/catch_reporter_junit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ namespace Catch {

static void normalizeNamespaceMarkers(std::string& str) {
std::size_t pos = str.find( "::" );
while ( pos != str.npos ) {
while ( pos != std::string::npos ) {
str.replace( pos, 2, "." );
pos += 1;
pos = str.find( "::", pos );
Expand Down
2 changes: 1 addition & 1 deletion src/catch2/reporters/catch_reporter_multi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace Catch {

void assertionEnded( AssertionStats const& assertionStats ) override;
void sectionEnded( SectionStats const& sectionStats ) override;
void testCasePartialEnded(TestCaseStats const& testInfo, uint64_t partNumber) override;
void testCasePartialEnded(TestCaseStats const& testStats, uint64_t partNumber) override;
void testCaseEnded( TestCaseStats const& testCaseStats ) override;
void testRunEnded( TestRunStats const& testRunStats ) override;

Expand Down
2 changes: 1 addition & 1 deletion src/catch2/reporters/catch_reporter_sonarqube.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace Catch {
xml.endElement();
}

void writeRun( TestRunNode const& groupNode );
void writeRun( TestRunNode const& runNode );

void writeTestFile(StringRef filename, std::vector<TestCaseNode const*> const& testCaseNodes);

Expand Down
4 changes: 2 additions & 2 deletions src/catch2/reporters/catch_reporter_teamcity.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ namespace Catch {
return "Reports test results as TeamCity service messages"s;
}

void testRunStarting( TestRunInfo const& groupInfo ) override;
void testRunEnded( TestRunStats const& testGroupStats ) override;
void testRunStarting( TestRunInfo const& runInfo ) override;
void testRunEnded( TestRunStats const& runStats ) override;


void assertionEnded(AssertionStats const& assertionStats) override;
Expand Down
2 changes: 1 addition & 1 deletion tests/ExtraTests/X22-BenchmarksInCumulativeReporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class CumulativeBenchmarkReporter final : public Catch::CumulativeReporterBase {
return "Custom reporter for testing cumulative reporter base";
}

virtual void testRunEndedCumulative() override;
void testRunEndedCumulative() override;
};

CATCH_REGISTER_REPORTER("testReporter", CumulativeBenchmarkReporter)
Expand Down
1 change: 1 addition & 0 deletions tests/ExtraTests/X29-CustomArgumentsForReporters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class TestReporter : public Catch::StreamingReporterBase {

void testRunStarting( Catch::TestRunInfo const& ) override {
std::vector<std::pair<std::string, std::string>> options;
options.reserve( m_customOptions.size() );
for ( auto const& kv : m_customOptions ) {
options.push_back( kv );
}
Expand Down
4 changes: 2 additions & 2 deletions tests/ExtraTests/X91-AmalgamatedCatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
TEST_CASE("Just a dummy test") {
auto i = GENERATE(1, 2, 3);
SECTION("a") {
REQUIRE(1 != 4);
REQUIRE(i != 4);
}
SECTION("b") {
CHECK(1 != 5);
CHECK(i != 5);
}
REQUIRE_THAT(1,
Catch::Matchers::Predicate<int>([](int i) {
Expand Down
4 changes: 2 additions & 2 deletions tests/SelfTest/IntrospectiveTests/Details.tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
1 change: 0 additions & 1 deletion tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]") {
Expand Down
2 changes: 1 addition & 1 deletion tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit cde3509

Please sign in to comment.