From 18a229e849b5448ed84987b35158f353cc0c72d5 Mon Sep 17 00:00:00 2001 From: Sean Robinson Date: Wed, 10 Nov 2021 13:54:25 -0700 Subject: [PATCH 1/7] Add a minimal binary to generate compile_commands.json file clang-tidy needs compile_commands.json to generate a set of source files to parse. tidy-base is not built but is used as a source file in which included headers can be parsed. Without this process, clang-tidy will process many other source files (e.g. doctest.hpp) that we do not want to worry about. Signed-off-by: Sean Robinson --- test/CMakeLists.txt | 7 +++++++ test/tidy-base.cpp | 4 ++++ 2 files changed, 11 insertions(+) create mode 100644 test/tidy-base.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3c502d9..02572a0 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -57,3 +57,10 @@ set_property(TARGET ARGPARSE PROPERTY CXX_STANDARD 17) # Set ${PROJECT_NAME} as the startup project set_property(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY VS_STARTUP_PROJECT ARGPARSE) + +file(GLOB ARGPARSE_LINT_SOURCES + tidy-base.cpp +) +ADD_EXECUTABLE(ARGPARSE_LINT ${ARGPARSE_LINT_SOURCES}) +set_target_properties(ARGPARSE_LINT PROPERTIES OUTPUT_NAME tidy-base) +set_property(TARGET ARGPARSE_LINT PROPERTY CXX_STANDARD 17) diff --git a/test/tidy-base.cpp b/test/tidy-base.cpp new file mode 100644 index 0000000..87b47a1 --- /dev/null +++ b/test/tidy-base.cpp @@ -0,0 +1,4 @@ + +#include "argparse/argparse.hpp" + +int main(int argc, const char* argv[]) {} From 9a9c3042aa63c597751ea8b5754ca7f7e5c9c882 Mon Sep 17 00:00:00 2001 From: Sean Robinson Date: Wed, 10 Nov 2021 13:54:27 -0700 Subject: [PATCH 2/7] Add a clang-tidy configuration file This file matches the current practices of the argparse project. Signed-off-by: Sean Robinson --- .clang-tidy | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..56d7f64 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,24 @@ +Checks: + -*, + readability-*, + -readability-braces-around-statements, + -readability-container-size-empty, + -readability-else-after-return, + -readability-implicit-bool-conversion, + -readability-function-cognitive-complexity, + -readability-magic-numbers, + -readability-named-parameter, + -readability-qualified-auto, + -readability-static-accessed-through-instance, + +CheckOptions: + - { key: readability-identifier-naming.ClassCase, value: CamelCase } + - { key: readability-identifier-naming.ConstexprVariableCase, value: lower_case } + - { key: readability-identifier-naming.FunctionCase, value: lower_case } + - { key: readability-identifier-naming.LocalVariableIgnoredRegexp, value: "^[a-z][a-z_]+" } + - { key: readability-identifier-naming.NamespaceCase, value: lower_case } + - { key: readability-identifier-naming.PrivateMemberPrefix, value: m } + - { key: readability-identifier-naming.StructCase, value: lower_case } + - { key: readability-identifier-naming.VariableCase, value: camelBack } + +HeaderFilterRegex: '.*' From 8d8282bac36cde915bbbdd2272ea1ff437516bb6 Mon Sep 17 00:00:00 2001 From: Sean Robinson Date: Wed, 10 Nov 2021 13:54:29 -0700 Subject: [PATCH 3/7] Add Static Analysis action to run on Pull Request Unit test source files are not currently checked. Hopefully, these can be added so that all source files in a pull request are verified. Signed-off-by: Sean Robinson --- .github/workflows/static_analysis.yml | 42 +++++++++++++++++++++++++++ tools/static_analysis_setup.sh | 4 +++ 2 files changed, 46 insertions(+) create mode 100644 .github/workflows/static_analysis.yml create mode 100755 tools/static_analysis_setup.sh diff --git a/.github/workflows/static_analysis.yml b/.github/workflows/static_analysis.yml new file mode 100644 index 0000000..0d47ee1 --- /dev/null +++ b/.github/workflows/static_analysis.yml @@ -0,0 +1,42 @@ + +name: Static Analysis + +on: pull_request + +jobs: + + static_analysis: + + name: ${{ matrix.toolchain }} + runs-on: ${{ matrix.os }} + + strategy: + + matrix: + + toolchain: + - ubuntu-latest + + include: + - toolchain: ubuntu-latest + os: ubuntu-latest + compiler: clang + + steps: + + - name: Checkout Code + uses: actions/checkout@v2 + + - name: Analyze + uses: JacobDomagala/StaticAnalysis@master + with: + clang_tidy_args: >- + --config-file=$GITHUB_WORKSPACE/.clang-tidy + --extra-arg=-I$GITHUB_WORKSPACE/include --extra-arg=-std=c++17 + cppcheck_args: >- + --enable=all --inconclusive --inline-suppr + -i$GITHUB_WORKSPACE/test/main.cpp + -i$GITHUB_WORKSPACE/test/test_*.cpp + --suppress=missingInclude + --suppress='*:$GITHUB_WORKSPACE/test/doctest.hpp' + init_script: tools/static_analysis_setup.sh diff --git a/tools/static_analysis_setup.sh b/tools/static_analysis_setup.sh new file mode 100755 index 0000000..cd0dc06 --- /dev/null +++ b/tools/static_analysis_setup.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +# Change to the "test" subdir before "build" subdir is made. +cd test From 6530a067470e6f1b1c398d875b9f6b430b567fa3 Mon Sep 17 00:00:00 2001 From: Sean Robinson Date: Wed, 10 Nov 2021 13:54:31 -0700 Subject: [PATCH 4/7] Copy more members in ArgumentParser copy constructor This showed as a cppcheck warning: uninitMemberVar. Signed-off-by: Sean Robinson --- include/argparse/argparse.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/argparse/argparse.hpp b/include/argparse/argparse.hpp index aa5f959..ab3c564 100644 --- a/include/argparse/argparse.hpp +++ b/include/argparse/argparse.hpp @@ -857,6 +857,9 @@ public: ArgumentParser(const ArgumentParser &other) : mProgramName(other.mProgramName), + mVersion(other.mVersion), + mDescription(other.mDescription), + mEpilog(other.mEpilog), mIsParsed(other.mIsParsed), mPositionalArguments(other.mPositionalArguments), mOptionalArguments(other.mOptionalArguments) { From abb220614129fb96fbf53308d2150dbf8016dddf Mon Sep 17 00:00:00 2001 From: Sean Robinson Date: Wed, 10 Nov 2021 13:54:33 -0700 Subject: [PATCH 5/7] Declare lambda parameter and ArgumentParser::print_help as const These were respectively reported as constParameter and functionConst style issues by cppcheck. Signed-off-by: Sean Robinson --- include/argparse/argparse.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/argparse/argparse.hpp b/include/argparse/argparse.hpp index ab3c564..34dab79 100644 --- a/include/argparse/argparse.hpp +++ b/include/argparse/argparse.hpp @@ -453,7 +453,7 @@ public: mUsedName = usedName; if (mNumArgs == 0) { mValues.emplace_back(mImplicitValue); - std::visit([](auto &aAction) { aAction({}); }, mAction); + std::visit([](const auto &aAction) { aAction({}); }, mAction); return start; } else if (mNumArgs <= std::distance(start, end)) { if (auto expected = maybe_nargs()) { @@ -1049,7 +1049,7 @@ public: // Printing the one and only help message // I've stuck with a simple message format, nothing fancy. [[deprecated("Use cout << program; instead. See also help().")]] std::string - print_help() { + print_help() const { auto out = help(); std::cout << out.rdbuf(); return out.str(); From c0bbcf613c0fc6c89391c45b1e44d147a4e91971 Mon Sep 17 00:00:00 2001 From: Sean Robinson Date: Wed, 10 Nov 2021 13:54:35 -0700 Subject: [PATCH 6/7] Remove sentry check in ArgumentParser::operator<< cppcheck reports "Variable 'sen' is assigned a value that is never used. [unreadVariable]" for this line. As far as I understand, std::ostream::sentry is used to prepare access to the stream buffer. But, we are never directly accessing the stream buffer. Stream access in this function uses other operator<< functions. Most noise in this patch is about unindenting after if() removal. Signed-off-by: Sean Robinson --- include/argparse/argparse.hpp | 60 +++++++++++++++++------------------ 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/include/argparse/argparse.hpp b/include/argparse/argparse.hpp index 34dab79..2aa06fd 100644 --- a/include/argparse/argparse.hpp +++ b/include/argparse/argparse.hpp @@ -1002,39 +1002,37 @@ public: // Print help message friend auto operator<<(std::ostream &stream, const ArgumentParser &parser) -> std::ostream & { - if (auto sen = std::ostream::sentry(stream)) { - stream.setf(std::ios_base::left); - stream << "Usage: " << parser.mProgramName << " [options] "; - std::size_t tLongestArgumentLength = parser.get_length_of_longest_argument(); + stream.setf(std::ios_base::left); + stream << "Usage: " << parser.mProgramName << " [options] "; + std::size_t tLongestArgumentLength = parser.get_length_of_longest_argument(); - for (const auto &argument : parser.mPositionalArguments) { - stream << argument.mNames.front() << " "; - } - stream << "\n\n"; - - if (!parser.mDescription.empty()) - stream << parser.mDescription << "\n\n"; - - if (!parser.mPositionalArguments.empty()) - stream << "Positional arguments:\n"; - - for (const auto &mPositionalArgument : parser.mPositionalArguments) { - stream.width(tLongestArgumentLength); - stream << mPositionalArgument; - } - - if (!parser.mOptionalArguments.empty()) - stream << (parser.mPositionalArguments.empty() ? "" : "\n") - << "Optional arguments:\n"; - - for (const auto &mOptionalArgument : parser.mOptionalArguments) { - stream.width(tLongestArgumentLength); - stream << mOptionalArgument; - } - - if (!parser.mEpilog.empty()) - stream << parser.mEpilog << "\n\n"; + for (const auto &argument : parser.mPositionalArguments) { + stream << argument.mNames.front() << " "; } + stream << "\n\n"; + + if (!parser.mDescription.empty()) + stream << parser.mDescription << "\n\n"; + + if (!parser.mPositionalArguments.empty()) + stream << "Positional arguments:\n"; + + for (const auto &mPositionalArgument : parser.mPositionalArguments) { + stream.width(tLongestArgumentLength); + stream << mPositionalArgument; + } + + if (!parser.mOptionalArguments.empty()) + stream << (parser.mPositionalArguments.empty() ? "" : "\n") + << "Optional arguments:\n"; + + for (const auto &mOptionalArgument : parser.mOptionalArguments) { + stream.width(tLongestArgumentLength); + stream << mOptionalArgument; + } + + if (!parser.mEpilog.empty()) + stream << parser.mEpilog << "\n\n"; return stream; } From 6246a9df0e001b9f7c06757f91fb6f96ca9ea5af Mon Sep 17 00:00:00 2001 From: Sean Robinson Date: Tue, 11 Jan 2022 06:48:00 -0700 Subject: [PATCH 7/7] Change Static Analysis trigger event to pull_request_target The GH security model restricts comment posting from PR actions. StaticAnalysis has added support for pull_request_target to mitigate risks while still allowing comments by the bot. Signed-off-by: Sean Robinson --- .github/workflows/static_analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/static_analysis.yml b/.github/workflows/static_analysis.yml index 0d47ee1..d5a2e50 100644 --- a/.github/workflows/static_analysis.yml +++ b/.github/workflows/static_analysis.yml @@ -1,7 +1,7 @@ name: Static Analysis -on: pull_request +on: pull_request_target jobs: