Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate argument of (DYNAMIC_)SECTION, TEST_CASE in static-analysis mode #2817

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
---
# Note: Alas, `Checks` is a string, not an array.
# Comments in the block string are not parsed and are passed in the value.
# They must thus be delimited by ',' from either side - then they are
# harmless. It's terrible, but it works.
Checks: >-
clang-diagnostic-*,
clang-analyzer-*,
-clang-analyzer-optin.core.EnumCastOutOfRange,

bugprone-*,
-bugprone-unchecked-optional-access,
,# This is ridiculous, as it triggers on constants,
-bugprone-implicit-widening-of-multiplication-result,
-bugprone-easily-swappable-parameters,
,# Is not really useful, has false positives, triggers for no-noexcept move constructors ...,
-bugprone-exception-escape,
-bugprone-narrowing-conversions,
-bugprone-chained-comparison,# RIP decomposers,

modernize-*,
-modernize-avoid-c-arrays,
-modernize-use-auto,
-modernize-use-emplace,
-modernize-use-nullptr,# it went crazy with three-way comparison operators,
-modernize-use-trailing-return-type,
-modernize-return-braced-init-list,
-modernize-concat-nested-namespaces,
-modernize-use-nodiscard,
-modernize-use-default-member-init,
-modernize-type-traits,# we need to support C++14,
-modernize-deprecated-headers,
,# There's a lot of these and most of them are probably not useful,
-modernize-pass-by-value,

performance-*,
-performance-enum-size,

portability-*,

readability-*,
-readability-braces-around-statements,
-readability-container-size-empty,
-readability-convert-member-functions-to-static,
-readability-else-after-return,
-readability-function-cognitive-complexity,
-readability-function-size,
-readability-identifier-length,
-readability-implicit-bool-conversion,
-readability-isolate-declaration,
-readability-magic-numbers,
-readability-named-parameter,
-readability-qualified-auto,
-readability-redundant-access-specifiers,
-readability-simplify-boolean-expr,
-readability-static-definition-in-anonymous-namespace,
-readability-uppercase-literal-suffix,
-readability-use-anyofallof,
-readability-avoid-return-with-void-value,

,# time hogs,
-bugprone-throw-keyword-missing,
-modernize-replace-auto-ptr,
-readability-identifier-naming,

,# We cannot use this until clang-tidy supports custom unique_ptr,
-bugprone-use-after-move,
,# Doesn't recognize unevaluated context in CATCH_MOVE and CATCH_FORWARD,
-bugprone-macro-repeated-side-effects,
WarningsAsErrors: >-
clang-analyzer-core.*,
clang-analyzer-cplusplus.*,
clang-analyzer-security.*,
clang-analyzer-unix.*,
performance-move-const-arg,
performance-unnecessary-value-param,
readability-duplicate-include,
HeaderFilterRegex: '.*\.(c|cxx|cpp)$'
FormatStyle: none
CheckOptions: {}
...
49 changes: 49 additions & 0 deletions .github/workflows/linux-other-builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,52 @@ jobs:
CTEST_OUTPUT_ON_FAILURE: 1
working-directory: ${{runner.workspace}}/build
run: ctest -C ${{matrix.build_type}} -j `nproc` ${{matrix.other_ctest_args}}
clang-tidy:
name: clang-tidy ${{matrix.version}}, ${{matrix.build_description}}, C++${{matrix.std}} ${{matrix.build_type}}
runs-on: ubuntu-22.04
strategy:
matrix:
include:
- version: "15"
build_description: all
build_type: Debug
std: 17
other_pkgs: ''
cmake_configurations: -DCATCH_BUILD_EXAMPLES=ON -DCATCH_ENABLE_CMAKE_HELPER_TESTS=ON
steps:
- uses: actions/checkout@v4

- name: Prepare environment
run: |
sudo apt-get update
sudo apt-get install -y ninja-build clang-${{matrix.version}} clang-tidy-${{matrix.version}} ${{matrix.other_pkgs}}

- name: Configure build
working-directory: ${{runner.workspace}}
env:
CXX: clang++-${{matrix.version}}
CXXFLAGS: ${{matrix.cxxflags}}
# Note: $GITHUB_WORKSPACE is distinct from ${{runner.workspace}}.
# This is important
run: |
clangtidy="clang-tidy-${{matrix.version}};-use-color"
# Use a dummy compiler/linker/ar/ranlib to effectively disable the
# compilation and only run clang-tidy.
cmake -Bbuild -H$GITHUB_WORKSPACE \
-DCMAKE_BUILD_TYPE=${{matrix.build_type}} \
-DCMAKE_CXX_STANDARD=${{matrix.std}} \
-DCMAKE_CXX_STANDARD_REQUIRED=ON \
-DCMAKE_CXX_EXTENSIONS=OFF \
-DCATCH_DEVELOPMENT_BUILD=ON \
-DCMAKE_CXX_CLANG_TIDY="$clangtidy" \
-DCMAKE_CXX_COMPILER_LAUNCHER=/usr/bin/true \
-DCMAKE_AR=/usr/bin/true \
-DCMAKE_CXX_COMPILER_AR=/usr/bin/true \
-DCMAKE_RANLIB=/usr/bin/true \
-DCMAKE_CXX_LINK_EXECUTABLE=/usr/bin/true \
${{matrix.cmake_configurations}} \
-G Ninja

- name: Run clang-tidy
working-directory: ${{runner.workspace}}/build
run: ninja
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's supposed to be duplicated here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else branch waaay down.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, that's dumb

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very 😄

return {};
} else if ( key[0] == 'X' ) {
// This is a reporter-specific option, we don't check these
Expand Down
Loading
Loading