diff --git a/.github/workflows/compiling_and_unittests_on_ubuntu_2404.yml b/.github/workflows/compiling_and_unittests_on_ubuntu_2404.yml index 6495087f..43f5fa4b 100644 --- a/.github/workflows/compiling_and_unittests_on_ubuntu_2404.yml +++ b/.github/workflows/compiling_and_unittests_on_ubuntu_2404.yml @@ -31,7 +31,10 @@ jobs: echo "timestamp=${timestamp}" >> ${GITHUB_OUTPUT} - name: Install Dependencies - run: sudo apt-get install -y libboost-all-dev libgit2-dev ninja-build ccache + run: | + set -euo pipefail + sudo apt-get install -y libboost-all-dev libgit2-dev ninja-build ccache + bash scripts/linux/ubuntu/install_llvm_tools.sh 22 clang-format-22 clang-tidy-22 - name: Setup C/C++ Compiler and ccache run: | diff --git a/.github/workflows/release_artifacts.yml b/.github/workflows/release_artifacts.yml index 110fde47..825151ec 100644 --- a/.github/workflows/release_artifacts.yml +++ b/.github/workflows/release_artifacts.yml @@ -30,6 +30,9 @@ jobs: run: | set -euo pipefail mv "${{ inputs.artifact-name }}" "${{ inputs.distribution-name }}" + mv "${{ inputs.artifact-name }}.sha256" "${{ inputs.distribution-name }}.sha256" + sed -i "s/${{ inputs.artifact-name }}/${{ inputs.distribution-name }}/" \ + "${{ inputs.distribution-name }}.sha256" - name: Release Artifacts to Github Release Page uses: softprops/action-gh-release@v2 @@ -38,3 +41,4 @@ jobs: tag_name: v${{ inputs.release-version }} files: | ${{ inputs.distribution-name }} + ${{ inputs.distribution-name }}.sha256 diff --git a/.github/workflows/release_new_version_on_ubuntu_2404.yml b/.github/workflows/release_new_version_on_ubuntu_2404.yml index be5f5759..8a92265c 100644 --- a/.github/workflows/release_new_version_on_ubuntu_2404.yml +++ b/.github/workflows/release_new_version_on_ubuntu_2404.yml @@ -86,12 +86,18 @@ jobs: # rename mv build/${package_name}.tar.gz build/${artifact_name}.tar.gz + ( + cd build + sha256sum "${artifact_name}.tar.gz" > "${artifact_name}.tar.gz.sha256" + ) - name: Upload Artifacts uses: actions/upload-artifact@v4 with: name: "${{ steps.artifact.outputs.artifact_name }}.tar.gz" - path: build/${{ steps.artifact.outputs.artifact_name }}.tar.gz + path: | + build/${{ steps.artifact.outputs.artifact_name }}.tar.gz + build/${{ steps.artifact.outputs.artifact_name }}.tar.gz.sha256 if-no-files-found: error run-regression-tests: @@ -108,4 +114,3 @@ jobs: artifact-name: "${{ needs.build-unit-tests-package.outputs.artifact_name }}.tar.gz" distribution-name: "${{ needs.build-unit-tests-package.outputs.distribution_name }}.tar.gz" release-version: "${{ needs.build-unit-tests-package.outputs.release_version }}" - diff --git a/.github/workflows/run_regression_tests_on_ubuntu.yml b/.github/workflows/run_regression_tests_on_ubuntu.yml index aaea2d64..3a9d0aa4 100644 --- a/.github/workflows/run_regression_tests_on_ubuntu.yml +++ b/.github/workflows/run_regression_tests_on_ubuntu.yml @@ -17,6 +17,7 @@ jobs: fail-fast: true matrix: include: + - { platform: "Ubuntu-24.04", clang_tool_version: "22" } - { platform: "Ubuntu-24.04", clang_tool_version: "18" } - { platform: "Ubuntu-24.04", clang_tool_version: "17" } - { platform: "Ubuntu-24.04", clang_tool_version: "16" } @@ -33,6 +34,12 @@ jobs: - name: Make Test Repository run: bash ./.github/workflows/scripts/basic_regression_test.sh + - name: Install LLVM 22 Tools + if: matrix.clang_tool_version == '22' + run: | + set -euo pipefail + bash scripts/linux/ubuntu/install_llvm_tools.sh 22 clang-format-22 clang-tidy-22 + - name: Download Artifacts uses: actions/download-artifact@v4 with: @@ -84,4 +91,3 @@ jobs: [[ "${format_failed}" -ne 1 ]] && exit 1 [[ "${tidy_failed}" -ne 1 ]] && exit 1 exit 0 - diff --git a/CMakeLists.txt b/CMakeLists.txt index 9d52e337..35022c30 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,14 +8,14 @@ string(STRIP "${CPP_LINT_ACTION_VERSION}" CPP_LINT_ACTION_VERSION) project(cpp_lint_action VERSION ${CPP_LINT_ACTION_VERSION}) -set(CMAKE_CXX_STANDARD 20) -set(CXX_STANDARD_REQUIRED true) -set(CMAKE_EXPORT_COMPILE_COMMANDS on) -set(CMAKE_BUILD_TYPE Debug) +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) set(cpp_lint_action_root ${CMAKE_CURRENT_SOURCE_DIR}) -set(config_dir ${cpp_lint_action_root}/src/configs) -set(src_dir ${cpp_lint_action_root}/src) +set(config_dir ${cpp_lint_action_root}/src/configs) +set(src_dir ${cpp_lint_action_root}/src) +set(generated_config_dir ${CMAKE_CURRENT_BINARY_DIR}/generated/configs) include(${cpp_lint_action_root}/cmake/FetchSpdlog.cmake) include(${cpp_lint_action_root}/cmake/FetchRangeV3.cmake) @@ -36,22 +36,30 @@ IF(ENABLE_CODE_COVERAGE) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} --coverage -fprofile-abs-path -g -O0") ENDIF() -find_package(Boost REQUIRED CONFIG COMPONENTS filesystem system regex program_options) +find_package(Boost REQUIRED CONFIG COMPONENTS filesystem process regex program_options) +find_package(Threads REQUIRED) +find_package(PkgConfig QUIET) -configure_file(${config_dir}/version.h.in ${config_dir}/version.h) +set(cpp_lint_action_libgit2_target "") +if(PkgConfig_FOUND) + pkg_check_modules(LIBGIT2 QUIET IMPORTED_TARGET libgit2) +endif() +if(TARGET PkgConfig::LIBGIT2) + set(cpp_lint_action_libgit2_target PkgConfig::LIBGIT2) +else() + find_package(libgit2 CONFIG QUIET) + if(TARGET libgit2::libgit2package) + set(cpp_lint_action_libgit2_target libgit2::libgit2package) + else() + find_path(LIBGIT2_INCLUDE_DIR git2.h) + find_library(LIBGIT2_LIBRARY NAMES git2) + if(NOT LIBGIT2_INCLUDE_DIR OR NOT LIBGIT2_LIBRARY) + message(FATAL_ERROR "Could not find libgit2. Install libgit2-dev or provide libgit2.pc.") + endif() + endif() +endif() -include_directories(${Boost_INCLUDE_DIRS} - ${magic_enum_SOURCE_DIR}/include - ${src_dir}) - -link_libraries(${Boost_LIBRARIES} - spdlog - range-v3 - httplib - nlohmann_json - tinyxml2 - magic_enum - git2) +configure_file(${config_dir}/version.h.in ${generated_config_dir}/version.h) FILE(GLOB_RECURSE dep_files "${src_dir}/github/*.cpp" "${src_dir}/tools/*.cpp" @@ -61,7 +69,38 @@ FILE(GLOB_RECURSE dep_files "${src_dir}/github/*.cpp" ) add_library(dep_obj OBJECT ${dep_files}) -add_executable(cpp-lint-action $ "${src_dir}/main.cpp") +target_include_directories(dep_obj + PUBLIC + ${Boost_INCLUDE_DIRS} + ${magic_enum_SOURCE_DIR}/include + ${CMAKE_CURRENT_BINARY_DIR}/generated + ${src_dir} +) +if(LIBGIT2_INCLUDE_DIR) + target_include_directories(dep_obj PUBLIC ${LIBGIT2_INCLUDE_DIR}) +endif() +target_link_libraries(dep_obj + PUBLIC + Boost::filesystem + Boost::process + Boost::regex + Boost::program_options + spdlog + range-v3 + httplib + nlohmann_json + tinyxml2 + magic_enum + Threads::Threads +) +if(cpp_lint_action_libgit2_target) + target_link_libraries(dep_obj PUBLIC ${cpp_lint_action_libgit2_target}) +else() + target_link_libraries(dep_obj PUBLIC ${LIBGIT2_LIBRARY}) +endif() + +add_executable(cpp-lint-action "${src_dir}/main.cpp") +target_link_libraries(cpp-lint-action PRIVATE dep_obj) IF (BUILD_TESTING) add_subdirectory(tests) diff --git a/README.md b/README.md index fc9366e4..95016667 100644 --- a/README.md +++ b/README.md @@ -19,51 +19,78 @@ Use cpp-lint-action on Github workflow is a easy thing. Just add follow lines to your `.github/workflows/your_workflow_name.yml` ```yaml -- uses: emmett2020/cpp-lint-action@v1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} +on: + pull_request: + +permissions: + contents: read + issues: write + pull-requests: write + +jobs: + cpp-lint: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + - uses: emmett2020/cpp-lint-action@v1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + jobs: "2" + max-review-comments: "50" ``` After the workflow is complete, you can see the following results displayed in the GitHub pull request review page. ![output](./docs/cpp-lint-action.png) -You can find more comprehensive configurations in [there](action.yml) and usage examples in the `examples/` directory. +By default, cpp-lint-action uses `clang-format-22` and `clang-tidy-22`. Pull request API reporting is only attempted for `pull_request` and `pull_request_target` events. On push, schedule, and other non-PR events, issue comments and pull request reviews are skipped automatically and `GITHUB_TOKEN` is not required for that reporting path. + +You can find all inputs in [action.yml](action.yml). The most commonly adjusted options are: + +| input | default | description | +| ----- | ------- | ----------- | +| `jobs` | `1` | Maximum file checks each enabled tool can run in parallel. | +| `max-review-comments` | `50` | Maximum inline pull request review comments to send. | +| `enable-comment-on-issue` | `true` | Create or update the summary issue comment on PRs. | +| `enable-pull-request-review` | `false` | Create inline pull request review comments on PRs. | +| `clang-format-version` | `22` | clang-format major version to find from `PATH`. | +| `clang-tidy-version` | `22` | clang-tidy major version to find from `PATH`. | # Develop cpp-lint-action 1. cpp-lint-action needs c++20 standard. Ensure your local compiler version supports it. 2. cpp-lint-action now supports Ubuntu only. -3. cpp-lint-action has two dependencies. You should install it before compilation. +3. cpp-lint-action uses Catch2 for unit tests. Run the Catch2 test binary directly. +4. cpp-lint-action has two system dependencies. You should install them before compilation. ```bash -sudo apt install -y libboost-all-dev libgit2-dev +sudo apt install -y libboost-all-dev libgit2-dev ninja-build ``` -4. Download this repository and build. +5. Download this repository and build. ```bash git clone https://github.com/emmett2020/cpp-lint-action.git -mkdir build && cd build -cmake .. -make -j`nproc` - +cmake -S cpp-lint-action -B cpp-lint-action/build -GNinja +cmake --build cpp-lint-action/build -j`nproc` ``` -5. Run unit tests +6. Run unit tests. ```bash -./build/tests/test +./cpp-lint-action/build/tests/test ``` # How to use cpp-lint-action locally Since cpp-lint-action is used for Github, you must set some environment variables to simulate Github CI environment. ```bash export GITHUB_REPOSITORY="owner/repository" -export GITHUB_TOKEN="your_private_github_token" export GITHUB_EVENT_NAME=pull_request export GITHUB_WORKSPACE="/path/to/your/local/repo" -export GITHUB_REF="ref/pull/NUMBER/merge" +export GITHUB_REF="refs/pull/NUMBER/merge" export GITHUB_SHA="test_branch_name_in_your_local_repository" +export GITHUB_EVENT_PATH="/path/to/event.json" +export GITHUB_TOKEN="your_private_github_token" # only required for PR API reporting ``` Then, just run cpp-lint-action ```bash -./build/cpp-lint-action +./build/cpp-lint-action --target-revision=main ``` diff --git a/action.yml b/action.yml index 7f119a31..50260493 100644 --- a/action.yml +++ b/action.yml @@ -38,6 +38,14 @@ inputs: You can use this action's output to customize later behavior. type: boolean default: false + jobs: + description: How many files each enabled tool can check in parallel + type: string + default: "1" + max-review-comments: + description: Maximum pull request review comments to create + type: string + default: "50" enable-clang-format: description: Enable clang-format check @@ -57,6 +65,7 @@ inputs: to avoid ambigous. And the clang-format-${version} must exist in your $PATH type: string + default: "22" clang-format-binary: description: | Set the full path of clang-format executable binary. @@ -85,6 +94,7 @@ inputs: to avoid ambigous. And the clang-tidy-${version} must exist in your $PATH type: string + default: "22" clang-tidy-binary: description: | Set the full path of clang-tidy executable binary. @@ -135,15 +145,50 @@ runs: - name: Install dependencies on Linux if: runner.os == 'Linux' shell: bash + env: + INPUT_CLANG_FORMAT_VERSION: ${{ inputs.clang-format-version }} + INPUT_CLANG_TIDY_VERSION: ${{ inputs.clang-tidy-version }} + INPUT_CLANG_FORMAT_BINARY: ${{ inputs.clang-format-binary }} + INPUT_CLANG_TIDY_BINARY: ${{ inputs.clang-tidy-binary }} run: | set -euo pipefail + default_clang_tool_version="22" + + clang_format_version="${INPUT_CLANG_FORMAT_VERSION}" + clang_tidy_version="${INPUT_CLANG_TIDY_VERSION}" + : "${clang_format_version:=${default_clang_tool_version}}" + : "${clang_tidy_version:=${default_clang_tool_version}}" + + clang_tool_packages=() + if [[ -z "${INPUT_CLANG_FORMAT_BINARY}" \ + && "${clang_format_version}" == "${default_clang_tool_version}" ]]; then + if ! command -v "clang-format-${default_clang_tool_version}" >/dev/null 2>&1; then + clang_tool_packages+=("clang-format-${default_clang_tool_version}") + fi + fi + if [[ -z "${INPUT_CLANG_TIDY_BINARY}" \ + && "${clang_tidy_version}" == "${default_clang_tool_version}" ]]; then + if ! command -v "clang-tidy-${default_clang_tool_version}" >/dev/null 2>&1; then + clang_tool_packages+=("clang-tidy-${default_clang_tool_version}") + fi + fi + + if (( ${#clang_tool_packages[@]} > 0 )); then + "${GITHUB_ACTION_PATH}/scripts/linux/ubuntu/install_llvm_tools.sh" \ + "${default_clang_tool_version}" \ + "${clang_tool_packages[@]}" + fi + version_file="${GITHUB_ACTION_PATH}/VERSION" - release_version=$(cat ${version_file} | sed '/^$/d' | sed 's/[[:space:]]*$//') - release_version_str=$(echo ${release_version} | tr '.' '_') - echo "Current path: `pwd`, current version: ${release_version}" - wget -q "https://github.com/emmett2020/cpp-lint-action/releases/download/v${release_version}/cpp_lint_action_${release_version_str}_ubuntu_24_04_${{ runner.arch }}.tar.gz" \ - -O cpp-lint-action.tar.gz - tar -xf cpp-lint-action.tar.gz + release_version=$(sed '/^$/d' "${version_file}" | sed 's/[[:space:]]*$//') + release_version_str=$(echo "${release_version}" | tr '.' '_') + archive_name="cpp_lint_action_${release_version_str}_ubuntu_24_04_${{ runner.arch }}.tar.gz" + release_url="https://github.com/emmett2020/cpp-lint-action/releases/download/v${release_version}" + echo "Current path: $(pwd), current version: ${release_version}" + wget -q "${release_url}/${archive_name}" -O "${archive_name}" + wget -q "${release_url}/${archive_name}.sha256" -O "${archive_name}.sha256" + sha256sum -c "${archive_name}.sha256" + tar -xf "${archive_name}" cd cpp_lint_action && ls -lrth && ./install.sh @@ -151,16 +196,44 @@ runs: id: on_linux if: runner.os == 'Linux' shell: bash + env: + INPUT_LOG_LEVEL: ${{ inputs.log-level }} + INPUT_TARGET_REVISION: ${{ inputs.target-revision }} + INPUT_ENABLE_COMMENT_ON_ISSUE: ${{ inputs.enable-comment-on-issue }} + INPUT_ENABLE_PULL_REQUEST_REVIEW: ${{ inputs.enable-pull-request-review }} + INPUT_ENABLE_STEP_SUMMARY: ${{ inputs.enable-step-summary }} + INPUT_ENABLE_ACTION_OUTPUT: ${{ inputs.enable-action-output }} + INPUT_DISABLE_ERRORS: ${{ inputs.disable-errors }} + INPUT_JOBS: ${{ inputs.jobs }} + INPUT_MAX_REVIEW_COMMENTS: ${{ inputs.max-review-comments }} + INPUT_ENABLE_CLANG_FORMAT: ${{ inputs.enable-clang-format }} + INPUT_ENABLE_CLANG_FORMAT_FASTLY_EXIT: ${{ inputs.enable-clang-format-fastly-exit }} + INPUT_CLANG_FORMAT_VERSION: ${{ inputs.clang-format-version }} + INPUT_CLANG_FORMAT_BINARY: ${{ inputs.clang-format-binary }} + INPUT_CLANG_FORMAT_FILE_IREGEX: ${{ inputs.clang-format-file-iregex }} + INPUT_ENABLE_CLANG_TIDY: ${{ inputs.enable-clang-tidy }} + INPUT_ENABLE_CLANG_TIDY_FASTLY_EXIT: ${{ inputs.enable-clang-tidy-fastly-exit }} + INPUT_CLANG_TIDY_VERSION: ${{ inputs.clang-tidy-version }} + INPUT_CLANG_TIDY_BINARY: ${{ inputs.clang-tidy-binary }} + INPUT_CLANG_TIDY_FILE_IREGEX: ${{ inputs.clang-tidy-file-iregex }} + INPUT_CLANG_TIDY_DATABASE: ${{ inputs.clang-tidy-database }} + INPUT_CLANG_TIDY_ALLOW_NO_CHECKS: ${{ inputs.clang-tidy-allow-no-checks }} + INPUT_CLANG_TIDY_ENABLE_CHECK_PROFILE: ${{ inputs.clang-tidy-enable-check-profile }} + INPUT_CLANG_TIDY_CHECKS: ${{ inputs.clang-tidy-checks }} + INPUT_CLANG_TIDY_CONFIG: ${{ inputs.clang-tidy-config }} + INPUT_CLANG_TIDY_CONFIG_FILE: ${{ inputs.clang-tidy-config-file }} + INPUT_CLANG_TIDY_HEADER_FILTER: ${{ inputs.clang-tidy-header-filter }} + INPUT_CLANG_TIDY_LINE_FILTER: ${{ inputs.clang-tidy-line-filter }} run: | echo "::group::Adjust repository" set -euo pipefail - if [[ "${{ env.DEBUG_CPP_LINT_ACTION }}" = "true" ]]; then + if [[ "${DEBUG_CPP_LINT_ACTION:-}" = "true" ]]; then # Debugging mode may not have remote repository. echo "Enabled debugging mode" - default_branch="${{ inputs.target-revision }}" + default_branch="${INPUT_TARGET_REVISION}" else - repo_full_path="${{ env.GITHUB_WORKSPACE }}" + repo_full_path="${GITHUB_WORKSPACE}" echo "Full path of testing repository: ${repo_full_path}" pushd "${repo_full_path}" @@ -168,8 +241,8 @@ runs: echo "Current branch of testing repository: ${current_branch}" default_branch="" - if [ -n "${{ inputs.target-revision }}" ]; then - default_branch="${{ inputs.target-revision }}" + if [[ -n "${INPUT_TARGET_REVISION}" ]]; then + default_branch="${INPUT_TARGET_REVISION}" else git fetch origin default_branch=$(git remote show origin | awk '/HEAD branch/ {print $NF}') @@ -185,61 +258,63 @@ runs: echo "::endgroup::" - options=" " - options="${options} --target-revision=${default_branch}" + default_clang_tool_version="22" + clang_format_version="${INPUT_CLANG_FORMAT_VERSION}" + clang_tidy_version="${INPUT_CLANG_TIDY_VERSION}" + : "${clang_format_version:=${default_clang_tool_version}}" + : "${clang_tidy_version:=${default_clang_tool_version}}" - if [ -n "${{ inputs.clang-format-version }}" ]; then - options="${options} --clang-format-version=${{ inputs.clang-format-version }}" - fi - if [ -n "${{ inputs.clang-format-binary }}" ]; then - options="${options} --clang-format-binary=${{ inputs.clang-format-binary }}" - fi - if [ -n "${{ inputs.clang-format-file-iregex }}" ]; then - options="${options} --clang-format-file-iregex=${{ inputs.clang-format-file-iregex }}" + args=( + "--log-level=${INPUT_LOG_LEVEL}" + "--target-revision=${default_branch}" + "--enable-comment-on-issue=${INPUT_ENABLE_COMMENT_ON_ISSUE}" + "--enable-pull-request-review=${INPUT_ENABLE_PULL_REQUEST_REVIEW}" + "--enable-step-summary=${INPUT_ENABLE_STEP_SUMMARY}" + "--enable-action-output=${INPUT_ENABLE_ACTION_OUTPUT}" + "--disable-errors=${INPUT_DISABLE_ERRORS}" + "--jobs=${INPUT_JOBS}" + "--max-review-comments=${INPUT_MAX_REVIEW_COMMENTS}" + "--enable-clang-format=${INPUT_ENABLE_CLANG_FORMAT}" + "--enable-clang-format-fastly-exit=${INPUT_ENABLE_CLANG_FORMAT_FASTLY_EXIT}" + "--enable-clang-tidy=${INPUT_ENABLE_CLANG_TIDY}" + "--enable-clang-tidy-fastly-exit=${INPUT_ENABLE_CLANG_TIDY_FASTLY_EXIT}" + "--clang-tidy-enable-check-profile=${INPUT_CLANG_TIDY_ENABLE_CHECK_PROFILE}" + "--clang-tidy-allow-no-checks=${INPUT_CLANG_TIDY_ALLOW_NO_CHECKS}" + ) + + if [[ -n "${INPUT_CLANG_FORMAT_BINARY}" ]]; then + args+=("--clang-format-binary=${INPUT_CLANG_FORMAT_BINARY}") + elif [[ -n "${clang_format_version}" ]]; then + args+=("--clang-format-version=${clang_format_version}") fi - if [ -n "${{ inputs.clang-tidy-version }}" ]; then - options="${options} --clang-tidy-version=${{ inputs.clang-tidy-version }}" + if [[ -n "${INPUT_CLANG_FORMAT_FILE_IREGEX}" ]]; then + args+=("--clang-format-file-iregex=${INPUT_CLANG_FORMAT_FILE_IREGEX}") fi - if [ -n "${{ inputs.clang-tidy-binary }}" ]; then - options="${options} --clang-tidy-binary=${{ inputs.clang-tidy-binary }}" + if [[ -n "${INPUT_CLANG_TIDY_BINARY}" ]]; then + args+=("--clang-tidy-binary=${INPUT_CLANG_TIDY_BINARY}") + elif [[ -n "${clang_tidy_version}" ]]; then + args+=("--clang-tidy-version=${clang_tidy_version}") fi - if [ -n "${{ inputs.clang-tidy-file-iregex }}" ]; then - options="${options} --clang-tidy-file-iregex=${{ inputs.clang-tidy-file-iregex }}" + if [[ -n "${INPUT_CLANG_TIDY_FILE_IREGEX}" ]]; then + args+=("--clang-tidy-file-iregex=${INPUT_CLANG_TIDY_FILE_IREGEX}") fi - if [ -n "${{ inputs.clang-tidy-database }}" ]; then - options="${options} --clang-tidy-database=${{ inputs.clang-tidy-database }}" + if [[ -n "${INPUT_CLANG_TIDY_DATABASE}" ]]; then + args+=("--clang-tidy-database=${INPUT_CLANG_TIDY_DATABASE}") fi - if [ -n "${{ inputs.clang-tidy-checks }}" ]; then - options="${options} --clang-tidy-checks=${{ inputs.clang-tidy-checks }}" + if [[ -n "${INPUT_CLANG_TIDY_CHECKS}" ]]; then + args+=("--clang-tidy-checks=${INPUT_CLANG_TIDY_CHECKS}") fi - if [ -n "${{ inputs.clang-tidy-config }}" ]; then - options="${options} --clang-tidy-config=${{ inputs.clang-tidy-config }}" + if [[ -n "${INPUT_CLANG_TIDY_CONFIG}" ]]; then + args+=("--clang-tidy-config=${INPUT_CLANG_TIDY_CONFIG}") fi - if [ -n "${{ inputs.clang-tidy-config-file }}" ]; then - options="${options} --clang-tidy-config-file=${{ inputs.clang-tidy-config-file }}" + if [[ -n "${INPUT_CLANG_TIDY_CONFIG_FILE}" ]]; then + args+=("--clang-tidy-config-file=${INPUT_CLANG_TIDY_CONFIG_FILE}") fi - if [ -n "${{ inputs.clang-tidy-header-filter }}" ]; then - options="${options} --clang-tidy-header-filter=${{ inputs.clang-tidy-header-filter }}" + if [[ -n "${INPUT_CLANG_TIDY_HEADER_FILTER}" ]]; then + args+=("--clang-tidy-header-filter=${INPUT_CLANG_TIDY_HEADER_FILTER}") fi - if [ -n "${{ inputs.clang-tidy-line-filter }}" ]; then - options="${options} --clang-tidy-line-filter=${{ inputs.clang-tidy-line-filter }}" + if [[ -n "${INPUT_CLANG_TIDY_LINE_FILTER}" ]]; then + args+=("--clang-tidy-line-filter=${INPUT_CLANG_TIDY_LINE_FILTER}") fi - /usr/local/bin/cpp-lint-action \ - --log-level="${{ inputs.log-level }}" \ - --enable-comment-on-issue="${{ inputs.enable-comment-on-issue }}" \ - --enable-pull-request-review="${{ inputs.enable-pull-request-review }}" \ - --enable-step-summary="${{ inputs.enable-step-summary }}" \ - --enable-action-output="${{ inputs.enable-action-output }}" \ - --disable-errors="${{ inputs.disable-errors }}" \ - --enable-clang-format="${{ inputs.enable-clang-format }}" \ - --enable-clang-format-fastly-exit="${{ inputs.enable-clang-format-fastly-exit }}" \ - --enable-clang-tidy="${{ inputs.enable-clang-tidy }}" \ - --enable-clang-tidy-fastly-exit="${{ inputs.enable-clang-tidy-fastly-exit }}" \ - --clang-tidy-enable-check-profile="${{ inputs.clang-tidy-enable-check-profile }}" \ - --clang-tidy-allow-no-checks="${{ inputs.clang-tidy-allow-no-checks }}" \ - ${options} - - exit $? - - + /usr/local/bin/cpp-lint-action "${args[@]}" diff --git a/cmake/FetchCatch2.cmake b/cmake/FetchCatch2.cmake index 73ee370b..732ad579 100644 --- a/cmake/FetchCatch2.cmake +++ b/cmake/FetchCatch2.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( Catch2 GIT_REPOSITORY https://github.com/catchorg/Catch2.git - GIT_TAG v3.6.0 + GIT_TAG v3.15.0 GIT_PROGRESS TRUE ) message(STATUS "Downloading Catch2") diff --git a/cmake/FetchHttpLib.cmake b/cmake/FetchHttpLib.cmake index c527beb8..1bb9a9bf 100644 --- a/cmake/FetchHttpLib.cmake +++ b/cmake/FetchHttpLib.cmake @@ -5,7 +5,7 @@ include(FetchContent) FetchContent_Declare( httplib GIT_REPOSITORY https://github.com/yhirose/cpp-httplib.git - GIT_TAG 924f214303b860b78350e1e2dfb0521a8724464f # 4/11/2024 + GIT_TAG v0.46.1 GIT_PROGRESS TRUE ) diff --git a/cmake/FetchLibGit2.cmake b/cmake/FetchLibGit2.cmake index 845fe854..d54ba95e 100644 --- a/cmake/FetchLibGit2.cmake +++ b/cmake/FetchLibGit2.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( libgit2 GIT_REPOSITORY https://github.com/libgit2/libgit2.git - GIT_TAG c6111ec # 5/11/2024 + GIT_TAG v1.9.4 GIT_PROGRESS TRUE ) diff --git a/cmake/FetchMagicEnum.cmake b/cmake/FetchMagicEnum.cmake index 398373b0..59a0a28d 100644 --- a/cmake/FetchMagicEnum.cmake +++ b/cmake/FetchMagicEnum.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( magic_enum GIT_REPOSITORY https://github.com/Neargye/magic_enum.git - GIT_TAG v0.9.6 + GIT_TAG v0.9.8 GIT_PROGRESS TRUE ) diff --git a/cmake/FetchNlohmannJson.cmake b/cmake/FetchNlohmannJson.cmake index 61e1ca90..ffb3abf6 100644 --- a/cmake/FetchNlohmannJson.cmake +++ b/cmake/FetchNlohmannJson.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( nlohmann-json GIT_REPOSITORY https://github.com/nlohmann/json.git - GIT_TAG v3.11.3 + GIT_TAG v3.12.0 GIT_PROGRESS TRUE ) diff --git a/cmake/FetchRangeV3.cmake b/cmake/FetchRangeV3.cmake index d581b052..f6549897 100644 --- a/cmake/FetchRangeV3.cmake +++ b/cmake/FetchRangeV3.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( range-v3 GIT_REPOSITORY https://github.com/ericniebler/range-v3.git - GIT_TAG 7e6f34b1e820fb8321346888ef0558a0ec842b8e # 2/11/2024 + GIT_TAG 0.12.0 GIT_PROGRESS TRUE ) diff --git a/cmake/FetchSpdlog.cmake b/cmake/FetchSpdlog.cmake index 75a0e4a7..7fa6a0dc 100644 --- a/cmake/FetchSpdlog.cmake +++ b/cmake/FetchSpdlog.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( spdlog GIT_REPOSITORY https://github.com/gabime/spdlog.git - GIT_TAG 6192537d08e47c34cbf78d8363d74f8c74b23d69 # 2/11/2024 + GIT_TAG v1.17.0 GIT_PROGRESS TRUE ) diff --git a/cmake/FetchTinyXML2.cmake b/cmake/FetchTinyXML2.cmake index a6de0990..42357fbd 100644 --- a/cmake/FetchTinyXML2.cmake +++ b/cmake/FetchTinyXML2.cmake @@ -3,7 +3,7 @@ include(FetchContent) FetchContent_Declare( tinyxml2 GIT_REPOSITORY https://github.com/leethomason/tinyxml2.git - GIT_TAG d418ac22f204b663880a37ebbb82996dc020f603 # 8/12/2024 + GIT_TAG 11.0.0 GIT_PROGRESS TRUE ) diff --git a/docs/result_report.md b/docs/result_report.md index 1333ed77..2d93cf8d 100644 --- a/docs/result_report.md +++ b/docs/result_report.md @@ -1 +1,14 @@ -TODO +# Result Reporting + +cpp-lint-action can report results in three places: + +| report | default | notes | +| ------ | ------- | ----- | +| GitHub Action output | enabled | Writes `clang_format_failed_number` and `clang_tidy_failed_number`. | +| GitHub step summary | enabled | Writes the markdown summary and reproduction commands. | +| Pull request issue comment | enabled on PRs | Creates a new summary comment or updates the latest existing cpp-lint-action comment. | +| Pull request review | disabled | Sends inline comments when enabled and when comments exist. | + +GitHub API reporting is only active for `pull_request` and `pull_request_target` events. For non-PR events, issue comments and pull request reviews are skipped automatically. + +Inline pull request review comments are capped by `max-review-comments` and empty reviews are not posted. The summary includes notes when comments are truncated or PR API reporting is skipped. diff --git a/docs/supports.md b/docs/supports.md index 50663047..6e812dd9 100644 --- a/docs/supports.md +++ b/docs/supports.md @@ -1,7 +1,10 @@ # clang-tidy & clang-format +The default clang tool major version is `22`. + | version | Ubuntu | MacOS | Windows | | ------- | ------ | ----- | ------- | +| 22.x | ✅ | ❌ | ❌ | | 18.1.3 | ✅ | ❌ | ❌ | | 17.0.6 | ✅ | ❌ | ❌ | | 16.0.6 | ✅ | ❌ | ❌ | @@ -15,4 +18,3 @@ ✅ : pass ❌ : unsupported ❓ : unvalidated - diff --git a/scripts/linux/ubuntu/install_llvm_tools.sh b/scripts/linux/ubuntu/install_llvm_tools.sh new file mode 100755 index 00000000..d3e00e73 --- /dev/null +++ b/scripts/linux/ubuntu/install_llvm_tools.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash + +set -euo pipefail + +if (($# < 1)); then + echo "usage: $0 [apt-package ...]" >&2 + exit 2 +fi + +llvm_version="$1" +shift + +if (($# == 0)); then + exit 0 +fi + +if [[ ! -r /etc/os-release ]]; then + echo "Cannot detect Ubuntu codename: /etc/os-release is missing" >&2 + exit 1 +fi + +# shellcheck disable=SC1091 +. /etc/os-release + +if [[ "${ID:-}" != "ubuntu" ]]; then + echo "LLVM apt bootstrap currently supports Ubuntu only, got: ${ID:-unknown}" >&2 + exit 1 +fi + +codename="${VERSION_CODENAME:-}" +if [[ -z "${codename}" ]]; then + echo "Cannot detect Ubuntu VERSION_CODENAME from /etc/os-release" >&2 + exit 1 +fi + +as_root=() +if ((EUID != 0)); then + as_root=(sudo) +fi + +keyring="/etc/apt/keyrings/apt.llvm.org.gpg" +source_file="/etc/apt/sources.list.d/llvm-toolchain-${llvm_version}.list" +repo_url="https://apt.llvm.org/${codename}/" +repo_suite="llvm-toolchain-${codename}-${llvm_version}" + +"${as_root[@]}" apt-get update +"${as_root[@]}" apt-get install -y ca-certificates gnupg wget +"${as_root[@]}" install -d -m 0755 /etc/apt/keyrings + +tmp_key="$(mktemp)" +trap 'rm -f "${tmp_key}"' EXIT +wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | gpg --dearmor > "${tmp_key}" +"${as_root[@]}" install -m 0644 "${tmp_key}" "${keyring}" + +echo "deb [signed-by=${keyring}] ${repo_url} ${repo_suite} main" \ + | "${as_root[@]}" tee "${source_file}" >/dev/null + +"${as_root[@]}" apt-get update +"${as_root[@]}" apt-get install -y "$@" diff --git a/scripts/linux/ubuntu/x64/manual_distribution/with_compiler/package.sh b/scripts/linux/ubuntu/x64/manual_distribution/with_compiler/package.sh index e142a29b..7ccc758b 100755 --- a/scripts/linux/ubuntu/x64/manual_distribution/with_compiler/package.sh +++ b/scripts/linux/ubuntu/x64/manual_distribution/with_compiler/package.sh @@ -66,7 +66,7 @@ echo "4. Start to compress" # ------- lib/ cp ${CUR_SCRIPT_DIR}/install.sh ${PACKAGE_NAME} cp ${CUR_SCRIPT_DIR}/uninstall.sh ${PACKAGE_NAME} -tar -cvf ${PACKAGE_NAME}.tar.gz ${PACKAGE_NAME} +tar -czf "${PACKAGE_NAME}.tar.gz" "${PACKAGE_NAME}" popd &> /dev/null echo "Successfully packaged ${BINARY_NAME} and it's dependencies into ${PACKAGE_NAME}" diff --git a/scripts/linux/ubuntu/x64/manual_distribution/without_compiler/package.sh b/scripts/linux/ubuntu/x64/manual_distribution/without_compiler/package.sh index e24a1882..34b24dc2 100755 --- a/scripts/linux/ubuntu/x64/manual_distribution/without_compiler/package.sh +++ b/scripts/linux/ubuntu/x64/manual_distribution/without_compiler/package.sh @@ -68,7 +68,7 @@ echo "3. Start to compress" # ------- lib/ cp ${CUR_SCRIPT_DIR}/install.sh ${PACKAGE_NAME} cp ${CUR_SCRIPT_DIR}/uninstall.sh ${PACKAGE_NAME} -tar -cvf ${PACKAGE_NAME}.tar.gz ${PACKAGE_NAME} +tar -czf "${PACKAGE_NAME}.tar.gz" "${PACKAGE_NAME}" popd &> /dev/null echo "Successfully packaged ${BINARY_NAME} and it's dependencies into ${PACKAGE_NAME}" diff --git a/src/context.cpp b/src/context.cpp index e0637cda..356f66bc 100644 --- a/src/context.cpp +++ b/src/context.cpp @@ -50,6 +50,8 @@ namespace lint { spdlog::debug("enable pull request review: {}", ctx.enable_pull_request_review); spdlog::debug("enable action output: {}", ctx.enable_action_output); spdlog::debug("disable errors: {}", ctx.disable_errors); + spdlog::debug("jobs: {}", ctx.jobs); + spdlog::debug("max review comments: {}", ctx.max_review_comments); spdlog::debug("repository path: {}", ctx.repo_path); spdlog::debug("repository: {}", ctx.repo_pair); spdlog::debug("repository token: {}", ctx.token.empty() ? "" : "***"); @@ -63,6 +65,9 @@ namespace lint { for (const auto &file: ctx.changed_files) { spdlog::debug("{}", file); } + for (const auto ¬e: ctx.report_notes) { + spdlog::debug("report note: {}", note); + } spdlog::debug(""); } } // namespace lint diff --git a/src/context.h b/src/context.h index 9f4a078b..7981d88f 100644 --- a/src/context.h +++ b/src/context.h @@ -19,6 +19,7 @@ #include #include #include +#include #include "utils/git_utils.h" @@ -31,6 +32,9 @@ namespace lint { bool enable_pull_request_review = false; bool enable_action_output = false; bool disable_errors = false; + std::size_t jobs = 1; + std::size_t max_review_comments = 50; + std::vector report_notes; // Theses will be filled by [ github::fill_context() ] std::string repo_path; diff --git a/src/github/client.h b/src/github/client.h index f6403d8e..cda728f0 100644 --- a/src/github/client.h +++ b/src/github/client.h @@ -15,9 +15,15 @@ */ #pragma once +#include +#include +#include #include #include +#include #include +#include +#include #include #include @@ -25,20 +31,94 @@ #include "common.h" #include "context.h" +#include "utils/common.h" #include "utils/error.h" namespace lint::github { using namespace std::string_literals; class client { + static constexpr auto github_accept_json = "application/vnd.github+json"; + static constexpr auto http_content_json = "application/json"; + static constexpr auto github_user_agent = "cpp-lint-action"; + static constexpr auto github_api_version = "2022-11-28"; + static constexpr auto max_retry_attempts = 3; + + static auto make_headers(const runtime_context &ctx) -> httplib::Headers { + return { + {"Accept", github_accept_json}, + {"Authorization", fmt::format("Bearer {}", ctx.token)}, + {"User-Agent", github_user_agent}, + {"X-GitHub-Api-Version", github_api_version}, + }; + } + + void configure_client() { + http_client_.set_connection_timeout(10); + http_client_.set_read_timeout(30); + http_client_.set_write_timeout(30); + } + + static auto rate_limit_info(const httplib::Response &response) -> std::string { + auto remaining = response.get_header_value("X-RateLimit-Remaining"); + auto reset = response.get_header_value("X-RateLimit-Reset"); + auto retry_after = response.get_header_value("Retry-After"); + + auto parts = std::vector{}; + if (!remaining.empty()) { + parts.emplace_back(fmt::format("remaining={}", remaining)); + } + if (!reset.empty()) { + parts.emplace_back(fmt::format("reset={}", reset)); + } + if (!retry_after.empty()) { + parts.emplace_back(fmt::format("retry-after={}", retry_after)); + } + if (parts.empty()) { + return ""; + } + return fmt::format(", rate-limit: {}", concat(parts, ',')); + } + + static auto is_retryable(const httplib::Result &response) -> bool { + if (!response) { + return true; + } + return response->status == 429 || response->status / 100 == 5; + } + + template + static auto request_with_retry(Fn &&fn) -> httplib::Result { + auto response = fn(); + for (auto attempt = 1; attempt < max_retry_attempts && is_retryable(response); ++attempt) { + std::this_thread::sleep_for(std::chrono::milliseconds{10 * (1 << (attempt - 1))}); + response = fn(); + } + return response; + } + public: + client() + : http_client_{github_api} { + configure_client(); + } + + explicit client(const std::string &api_url) + : http_client_{api_url} { + configure_client(); + } + static void check_http_response(const httplib::Result &response) { + throw_unless(static_cast(response), "github http request failed"); + auto code = response->status / 100; const auto &reason = response->reason; throw_unless(code == 1 || code == 2, - fmt::format("http response error: http status code: {}, reason: {}", + fmt::format("http response error: http status code: {}, reason: {}{}, body: {}", response->status, - reason)); + reason, + rate_limit_info(*response), + response->body)); } static auto is_our_comment(const nlohmann::json &comment) -> bool { @@ -46,8 +126,7 @@ namespace lint::github { return false; } - auto body = std::string_view{}; - comment["body"].get_to(body); + auto body = comment["body"].get(); return body.starts_with(github_comment_identifier); } @@ -56,36 +135,57 @@ namespace lint::github { assert(ranges::contains(github_pull_request_events, ctx.event_name)); auto path = fmt::format("/repos/{}/issues/{}/comments", ctx.repo_pair, ctx.pr_number); - auto headers = httplib::Headers{ - {"Accept", "application/vnd.github+json"}, - {"Authorization", fmt::format("token {}", ctx.token)} - }; + auto headers = make_headers(ctx); spdlog::debug("Http request path: {}", path); - auto response = client.Get(path, headers); + auto best_comment_id = std::int64_t{-1}; + auto matched_comments = std::size_t{0}; + constexpr auto per_page = 100; + for (auto page = 1;; ++page) { + auto page_path = fmt::format("{}?per_page={}&page={}", path, per_page, page); + auto response = request_with_retry([&] { return http_client_.Get(page_path, headers); }); - check_http_response(response); - spdlog::trace("Get github response body: {}", response->body); + check_http_response(response); + spdlog::trace("Get github response body: {}", response->body); - // data type check - auto comments = nlohmann::json::parse(response->body); - if (comments.is_null()) { - spdlog::debug("The pull request number {} doesn't have any comments yet", ctx.pr_number); - return; - } - throw_unless(comments.is_array(), "issue comments are not an array"); - if (comments.empty()) { - spdlog::debug("The pull request number {} doesn't have any comments yet", ctx.pr_number); - return; + // data type check + auto comments = nlohmann::json::parse(response->body); + if (comments.is_null()) { + spdlog::debug("The pull request number {} doesn't have any comments yet", ctx.pr_number); + break; + } + throw_unless(comments.is_array(), "issue comments are not an array"); + if (comments.empty()) { + spdlog::debug("The pull request number {} doesn't have any comments yet", ctx.pr_number); + break; + } + + for (const auto &comment: comments) { + if (!is_our_comment(comment)) { + continue; + } + auto id = std::int64_t{}; + comment["id"].get_to(id); + best_comment_id = std::max(best_comment_id, id); + ++matched_comments; + } + + if (comments.size() < per_page) { + break; + } } - auto comment = ranges::find_if(comments, is_our_comment); - if (comment == comments.end()) { + if (best_comment_id == -1) { spdlog::debug("The cpp-lint doesn't comments on pull request number {} yet", ctx.pr_number); return; } - (*comment)["id"].get_to(comment_id_); + comment_id_ = best_comment_id; + if (matched_comments > 1) { + spdlog::debug("Found {} cpp-lint-action comments; updating newest id {}", + matched_comments, + comment_id_); + } spdlog::debug("Successfully got comment id {} of pr {}", comment_id_, ctx.pr_number); } @@ -93,19 +193,17 @@ namespace lint::github { spdlog::debug("Start to add issue comment for pr {}", ctx.pr_number); const auto path = fmt::format("/repos/{}/issues/{}/comments", ctx.repo_pair, ctx.pr_number); - const auto headers = httplib::Headers{ - {"Accept", "application/vnd.github.use_diff"}, - {"Authorization", fmt::format("token {}", ctx.token)} - }; + const auto headers = make_headers(ctx); spdlog::debug("Http request path: {}", path); auto json_body = nlohmann::json{}; json_body["body"] = github_comment_identifier + body; spdlog::trace("Http request body:\n{}", json_body.dump()); - auto response = client.Post(path, headers, json_body.dump(), "text/plain"); - spdlog::trace("Get github response body: {}", response->body); + auto response = request_with_retry( + [&] { return http_client_.Post(path, headers, json_body.dump(), http_content_json); }); check_http_response(response); + spdlog::trace("Get github response body: {}", response->body); auto comment = nlohmann::json::parse(response->body); throw_unless(comment.is_object(), "comment isn't object"); @@ -123,19 +221,17 @@ namespace lint::github { spdlog::debug("Start to update issue comment"); const auto path = fmt::format("/repos/{}/issues/comments/{}", ctx.repo_pair, comment_id_); - const auto headers = httplib::Headers{ - {"Accept", "application/vnd.github.use_diff"}, - {"Authorization", fmt::format("token {}", ctx.token)} - }; + const auto headers = make_headers(ctx); spdlog::debug("Http request path: {}", path); auto json_body = nlohmann::json{}; json_body["body"] = github_comment_identifier + body; spdlog::trace("Http request body:\n{}", json_body.dump()); - auto response = client.Post(path, headers, json_body.dump(), "text/plain"); - spdlog::trace("Get github response body: {}", response->body); + auto response = request_with_retry( + [&] { return http_client_.Patch(path, headers, json_body.dump(), http_content_json); }); check_http_response(response); + spdlog::trace("Get github response body: {}", response->body); spdlog::info("Successfully updated comment {} of pr {}", comment_id_, ctx.pr_number); } @@ -151,22 +247,20 @@ namespace lint::github { spdlog::debug("Start to post pull request review for pr number {}", ctx.pr_number); const auto path = fmt::format("/repos/{}/pulls/{}/reviews", ctx.repo_pair, ctx.pr_number); - const auto headers = httplib::Headers{ - {"Accept", "application/vnd.github.use_diff"}, - {"Authorization", fmt::format("token {}", ctx.token)} - }; + const auto headers = make_headers(ctx); spdlog::debug("Http request path: {}", path); spdlog::trace("Http request body:\n{}", body); - auto response = client.Post(path, headers, body, "text/plain"); - spdlog::trace("Get github response body: {}", response->body); + auto response = request_with_retry( + [&] { return http_client_.Post(path, headers, body, http_content_json); }); check_http_response(response); + spdlog::trace("Get github response body: {}", response->body); spdlog::info("Successfully post pull_request_review for pull-request {}", ctx.pr_number); } private: - std::uint32_t comment_id_ = -1; - httplib::Client client{github_api}; + std::int64_t comment_id_ = -1; + httplib::Client http_client_; }; } // namespace lint::github diff --git a/src/github/common.cpp b/src/github/common.cpp index 0129ab79..69bbfdf0 100644 --- a/src/github/common.cpp +++ b/src/github/common.cpp @@ -15,31 +15,70 @@ */ #include "common.h" +#include +#include +#include +#include + +#include + #include "utils/env_manager.h" #include "utils/error.h" namespace lint::github { namespace { + auto is_debug_enabled() -> bool { + auto value = boost::algorithm::to_lower_copy(env::get(debug_cpp_lint_action)); + return value == "1" || value == "true" || value == "yes"; + } + // PR merge branch refs/pull/PULL_REQUEST_NUMBER/merge - auto parse_pr_number(const std::string &ref_name) -> std::int32_t { - spdlog::trace("Enter parse_pr_number()"); + auto parse_pr_number_from_ref(const std::string &ref_name) -> std::optional { + spdlog::trace("Enter parse_pr_number_from_ref()"); auto parts = ranges::views::split(ref_name, '/') | ranges::to>(); - throw_if(parts.size() != 4, fmt::format("ref_name format error: {}", ref_name)); - auto res = std::stoi(parts[2]); + if (parts.size() != 4) { + return std::nullopt; + } + auto res = std::int32_t{}; + try { + res = std::stoi(parts[2]); + } catch (const std::exception &) { + return std::nullopt; + } spdlog::trace("parse pr number uses: {} and got pull-request number: {}", ref_name, res); return res; } - void check_env(const github_env &env) { + auto parse_pr_number_from_event_path(const std::string &event_path) + -> std::optional { + spdlog::trace("Enter parse_pr_number_from_event_path()"); + if (event_path.empty()) { + return std::nullopt; + } + + auto file = std::ifstream{event_path}; + if (!file.is_open()) { + spdlog::debug("Cannot open GitHub event payload: {}", event_path); + return std::nullopt; + } + + try { + auto payload = nlohmann::json::parse(file); + if (payload.contains("number") && payload["number"].is_number_integer()) { + return payload["number"].get(); + } + } catch (const std::exception &err) { + spdlog::debug("Cannot parse GitHub event payload {}: {}", event_path, err.what()); + } + return std::nullopt; + } + + void check_base_env(const github_env &env) { spdlog::debug("Start to check github environment variables"); - throw_if(env.repository.empty(), "empty git repository, check env: GITHUB_REPOSITORY"); - throw_if(env.token.empty(), "empty token, check env: GITHUB_TOKEN"); throw_if(env.event_name.empty(), "empty git event, check env: GITHUB_EVENT_NAME"); - throw_if(env.github_ref.empty(), "empty github ref, check env: GITHUB_REF"); throw_if(env.github_sha.empty(), "empty github sha, check env: GITHUB_SHA"); - throw_if(env.github_ref_type.empty(), "empty git ref type, check env: GITHUB_REF_TYPE"); throw_if(env.workspace.empty(), "empty git repository workspace, check env: GITHUB_WORKSPACE"); } @@ -48,7 +87,7 @@ namespace lint::github { spdlog::debug("Github Envionment Variables:"); spdlog::debug("--------------------------------------------------"); spdlog::debug("git repository:{}", env.repository); - spdlog::debug("git token:{}", env.token); + spdlog::debug("git token:{}", env.token.empty() ? "" : "***"); spdlog::debug("git event name:{}", env.event_name); spdlog::debug("git base ref:{}", env.base_ref); spdlog::debug("git head ref:{}", env.head_ref); @@ -56,6 +95,7 @@ namespace lint::github { spdlog::debug("git sha:{}", env.github_sha); spdlog::debug("git ref type:{}", env.github_ref_type); spdlog::debug("git workspace:{}", env.workspace); + spdlog::debug("git event path:{}", env.event_path); spdlog::debug(""); } @@ -66,7 +106,7 @@ namespace lint::github { auto env = github_env{}; - if (env::get(debug_cpp_lint_action).empty()) { + if (!is_debug_enabled()) { // On real user's workflow env.repository = env::get(github_repository); env.token = env::get(github_token); @@ -77,8 +117,7 @@ namespace lint::github { env.github_sha = env::get(github_sha); env.github_ref_type = env::get(github_ref_type); env.workspace = env::get(github_workspace); - print_env(env); - check_env(env); + env.event_path = env::get(github_event_path); } else { // Only our test workflow. env.repository = env::get("DEBUG_GITHUB_REPOSITORY"); @@ -90,11 +129,19 @@ namespace lint::github { env.github_sha = env::get("DEBUG_GITHUB_SHA"); env.github_ref_type = env::get("DEBUG_GITHUB_REF_TYPE"); env.workspace = env::get("DEBUG_GITHUB_WORKSPACE"); + env.event_path = env::get("DEBUG_GITHUB_EVENT_PATH"); } + print_env(env); + check_base_env(env); return env; } + auto is_pull_request_event(std::string_view event_name) -> bool { + return event_name == github_event_pull_request + || event_name == github_event_pull_request_target; + } + void fill_context(const github_env &env, runtime_context &ctx) { spdlog::trace("Enter fill_context()"); @@ -106,11 +153,41 @@ namespace lint::github { // For reporter ctx.token = env.token; ctx.repo_pair = env.repository; - bool needs_parse_pr_number = ranges::contains(github_pull_request_events, ctx.event_name) - && (ctx.enable_comment_on_issue || ctx.enable_pull_request_review); - if (needs_parse_pr_number) { - ctx.pr_number = parse_pr_number(env.github_ref); + if (is_pull_request_event(ctx.event_name)) { + auto pr_number = parse_pr_number_from_event_path(env.event_path); + if (!pr_number) { + pr_number = parse_pr_number_from_ref(env.github_ref); + } + if (pr_number) { + ctx.pr_number = *pr_number; + } + } + } + + void prepare_reporting_context(runtime_context &ctx) { + if (!is_pull_request_event(ctx.event_name)) { + if (ctx.enable_comment_on_issue || ctx.enable_pull_request_review) { + auto note = + fmt::format("GitHub API reporting skipped because event '{}' is not a pull request", + ctx.event_name); + spdlog::info("{}", note); + ctx.report_notes.push_back(note); + } + ctx.enable_comment_on_issue = false; + ctx.enable_pull_request_review = false; + return; + } + + const auto needs_github_api = ctx.enable_comment_on_issue || ctx.enable_pull_request_review; + if (!needs_github_api) { + return; } + + throw_if(ctx.repo_pair.empty(), + "empty git repository, check env: GITHUB_REPOSITORY"); + throw_if(ctx.token.empty(), "empty token, check env: GITHUB_TOKEN"); + throw_if(ctx.pr_number == -1, + "cannot determine pull request number from GITHUB_EVENT_PATH or GITHUB_REF"); } } // namespace lint::github diff --git a/src/github/common.h b/src/github/common.h index f8b39740..81300dee 100644 --- a/src/github/common.h +++ b/src/github/common.h @@ -16,6 +16,7 @@ #pragma once #include +#include #include #include @@ -104,11 +105,17 @@ namespace lint::github { std::string head_ref; std::string github_ref; std::string github_ref_type; + std::string event_path; }; + [[nodiscard]] auto is_pull_request_event(std::string_view event_name) -> bool; + /// Read Github environment variables. auto read_env() -> github_env; /// Fill runtime context by Github environment variables. void fill_context(const github_env &env, runtime_context &ctx); + + /// Disable impossible GitHub report outputs and validate required API data. + void prepare_reporting_context(runtime_context &ctx); } // namespace lint::github diff --git a/src/main.cpp b/src/main.cpp index 01121aa6..d0884b7b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ #include +#include #include #include #include @@ -105,68 +106,94 @@ namespace { } } + struct git_runtime_guard { + git_runtime_guard() { + git::setup(); + } -} // namespace + ~git_runtime_guard() { + git::shutdown(); + } -auto main(int argc, char **argv) -> int { - auto tool_creators = collect_tool_creators(); - - // Handle program options. - auto desc = program_options::create_desc(); - tool::register_tool_options(tool_creators, desc); - auto user_options = program_options::parse(argc, argv, desc); - if (user_options.contains("help")) { - std::cout << desc << "\n"; - return 0; - } - if (user_options.contains("version")) { - print_version(); - return 0; - } - set_log(user_options); + git_runtime_guard(const git_runtime_guard &) = delete; + git_runtime_guard &operator=(const git_runtime_guard &) = delete; + }; - auto tools = tool::create_enabled_tools(tool_creators, user_options); - print_tools_info(tools); + auto run_application(int argc, char **argv) -> int { + auto tool_creators = collect_tool_creators(); - // Create runtime context. - auto context = runtime_context{}; + // Handle program options. + auto desc = program_options::create_desc(); + tool::register_tool_options(tool_creators, desc); + auto user_options = program_options::parse(argc, argv, desc); + if (user_options.contains("help")) { + std::cout << desc << "\n"; + return 0; + } + if (user_options.contains("version")) { + print_version(); + return 0; + } + set_log(user_options); - // Fill runtime context by program options. - program_options::fill_context(user_options, context); + auto tools = tool::create_enabled_tools(tool_creators, user_options); + print_tools_info(tools); - // Fill runtime context by environment variables. - auto env = github::read_env(); - github::fill_context(env, context); + // Create runtime context. + auto context = runtime_context{}; - // Fill runtime context by git repositofy informations. - git::setup(); - fill_git_info(context); + // Fill runtime context by program options. + program_options::fill_context(user_options, context); - print_context(context); - check_repo_is_on_source(context); + // Fill runtime context by environment variables. + auto env = github::read_env(); + github::fill_context(env, context); + github::prepare_reporting_context(context); - // Run tools within the given context and get reporters. - auto reporters = tool::run_tools(tools, context); - print_brief_result(reporters, context.changed_files.size()); + // Fill runtime context by git repositofy informations. + auto git_guard = git_runtime_guard{}; + fill_git_info(context); - if (context.enable_action_output) { - write_to_github_action_output(context, reporters); - } - if (context.enable_step_summary) { - write_to_github_step_summary(context, reporters); - } - if (context.enable_comment_on_issue) { - comment_on_github_issue(context, reporters); - } - if (context.enable_pull_request_review) { - comment_on_github_pull_request_review(context, reporters); + print_context(context); + check_repo_is_on_source(context); + + // Run tools within the given context and get reporters. + auto reporters = tool::run_tools(tools, context); + print_brief_result(reporters, context.changed_files.size()); + + auto review_comments = github::review_comments{}; + if (context.enable_pull_request_review) { + review_comments = tool::collect_pull_request_review_comments(context, reporters); + } + + if (context.enable_action_output) { + write_to_github_action_output(context, reporters); + } + if (context.enable_step_summary) { + write_to_github_step_summary(context, reporters); + } + if (context.enable_comment_on_issue) { + comment_on_github_issue(context, reporters); + } + if (context.enable_pull_request_review) { + tool::comment_on_github_pull_request_review(context, review_comments); + } + + if (context.disable_errors) { + return 0; + } + return all_passed(reporters) ? 0 : 1; } - git::shutdown(); +} // namespace - if (context.disable_errors) { - return 0; +auto main(int argc, char **argv) -> int { + try { + return run_application(argc, argv); + } catch (const std::exception &err) { + spdlog::error("cpp-lint-action failed: {}", err.what()); + } catch (...) { + spdlog::error("cpp-lint-action failed with an unknown error"); } - return all_passed(reporters) ? 0 : 1; + return 1; } - diff --git a/src/program_options.cpp b/src/program_options.cpp index 6f11723b..2b8a7ae7 100644 --- a/src/program_options.cpp +++ b/src/program_options.cpp @@ -35,6 +35,8 @@ namespace lint::program_options { constexpr auto enable_pull_request_review = "enable-pull-request-review"; constexpr auto enable_action_output = "enable-action-output"; constexpr auto disable_errors = "disable-errors"; + constexpr auto jobs = "jobs"; + constexpr auto max_review_comments = "max-review-comments"; } // namespace using std::string; @@ -45,6 +47,10 @@ namespace lint::program_options { const auto *level = value()->value_name("level")->default_value("info"); const auto *revision = value()->value_name("revision"); + const auto *jobs_value = + value()->value_name("number")->default_value(1); + const auto *max_review_comments_value = + value()->value_name("number")->default_value(50); auto boolean = [](bool def) { return value()->value_name("bool")->default_value(def); @@ -63,6 +69,9 @@ namespace lint::program_options { (enable_step_summary, boolean(true), "Whether enable write step summary to Github action") (enable_action_output, boolean(true), "Whether enable write output to Github action") (disable_errors, boolean(false), "Whether disable errors.") + (jobs, jobs_value, "How many files each tool can check in parallel") + (max_review_comments, max_review_comments_value, + "Maximum pull request review comments to create") ; // clang-format on @@ -131,6 +140,13 @@ namespace lint::program_options { if (variables.contains(disable_errors)) { ctx.disable_errors = variables[disable_errors].as(); } + if (variables.contains(jobs)) { + ctx.jobs = variables[jobs].as(); + throw_if(ctx.jobs == 0, "jobs must be greater than 0"); + } + if (variables.contains(max_review_comments)) { + ctx.max_review_comments = variables[max_review_comments].as(); + } } } // namespace lint::program_options diff --git a/src/tools/base_reporter.cpp b/src/tools/base_reporter.cpp index 4b283ef4..00dce606 100644 --- a/src/tools/base_reporter.cpp +++ b/src/tools/base_reporter.cpp @@ -16,6 +16,7 @@ #include "tools/base_reporter.h" +#include #include #include #include @@ -78,11 +79,18 @@ namespace lint::tool { } } + auto notes = ""s; + if (!context.report_notes.empty()) { + notes = "\n## Notes\n"; + for (const auto ¬e: context.report_notes) { + notes += fmt::format("- {}\n", note); + } + } static const auto reproduce_summary = " :mag_right: Steps to reproduce this result in your local environment\n\n"s; details += fmt::format(details_fmt, reproduce_summary + make_reproduce_spec(reporters)); - return fmt::format("{}{}{}{}{}", header, table_header, table_sep_line, table_rows, details); + return fmt::format("{}{}{}{}{}{}", header, table_header, table_sep_line, table_rows, notes, details); } } // namespace @@ -119,15 +127,36 @@ namespace lint::tool { github_client.add_or_update_issue_comment(context, make_detail_reporter(context, reporters)); } - void comment_on_github_pull_request_review(const runtime_context &context, - const std::vector &reporters) { - auto github_client = github::client{}; - auto comments = github::review_comments{}; + auto collect_pull_request_review_comments(runtime_context &context, + const std::vector &reporters) + -> github::review_comments { + auto comments = github::review_comments{}; for (const auto &reporter: reporters) { auto ret = reporter->make_review_comment(context); comments.insert(comments.end(), ret.begin(), ret.end()); } - auto body = github::make_review_str(comments); + + const auto original_size = comments.size(); + if (original_size > context.max_review_comments) { + comments.resize(context.max_review_comments); + context.report_notes.push_back( + fmt::format("Pull request review comments were truncated from {} to {}", + original_size, + context.max_review_comments)); + } + + return comments; + } + + void comment_on_github_pull_request_review(const runtime_context &context, + const github::review_comments &comments) { + if (comments.empty()) { + spdlog::info("Skip pull request review because there are no inline comments"); + return; + } + + auto github_client = github::client{}; + auto body = github::make_review_str(comments); github_client.post_pull_request_review(context, body); } } // namespace lint::tool diff --git a/src/tools/base_reporter.h b/src/tools/base_reporter.h index ad1b2bf4..137f7b8b 100644 --- a/src/tools/base_reporter.h +++ b/src/tools/base_reporter.h @@ -56,7 +56,11 @@ namespace lint::tool { void comment_on_github_issue(const runtime_context &context, const std::vector &reporters); + auto collect_pull_request_review_comments(runtime_context &context, + const std::vector &reporters) + -> github::review_comments; + void comment_on_github_pull_request_review(const runtime_context &context, - const std::vector &reporters); + const github::review_comments &comments); } // namespace lint::tool diff --git a/src/tools/clang_format/clang_format.cpp b/src/tools/clang_format/clang_format.cpp index 183d7c2b..774d31b1 100644 --- a/src/tools/clang_format/clang_format.cpp +++ b/src/tools/clang_format/clang_format.cpp @@ -108,9 +108,7 @@ namespace lint::tool::clang_format { auto [ec, std_out, std_err] = shell::which(option.binary); throw_unless(ec == 0, fmt::format("Can't find given clang-format binary: {}", option.binary)); } else { - auto [ec, std_out, std_err] = shell::which("clang-format"); - throw_unless(ec == 0, "can't find clang-format"); - option.binary = std_out; + option.binary = find_clang_tool("clang-format", default_clang_tool_version); } option.version = get_version(option.binary); diff --git a/src/tools/clang_format/general/impl.cpp b/src/tools/clang_format/general/impl.cpp index 9083fe7c..8fec56d6 100644 --- a/src/tools/clang_format/general/impl.cpp +++ b/src/tools/clang_format/general/impl.cpp @@ -15,11 +15,17 @@ */ #include "tools/clang_format/general/impl.h" +#include +#include #include #include +#include #include +#include +#include #include #include +#include #include #include @@ -173,8 +179,12 @@ namespace lint::tool::clang_format { assert(!option.binary.empty() && "clang-format binary is empty"); assert(!context.repo_path.empty() && "the repo_path of context is empty"); + result = result_t{}; const auto root_dir = context.repo_path; - const auto files = context.changed_files; + auto files = context.changed_files; + std::sort(files.begin(), files.end()); + + auto files_to_check = std::vector{}; for (const auto &file: files) { const auto &delta = context.deltas.at(file); if (delta.status == GIT_DELTA_DELETED) { @@ -186,24 +196,86 @@ namespace lint::tool::clang_format { continue; } - auto per_file_result = check_single_file(context, root_dir, file); + files_to_check.push_back(file); + } + + auto merge_result = [&](const std::string &file, per_file_result per_file_result) { if (per_file_result.passed) { spdlog::info("file: {} passes {} check.", file, option.binary); result.passes[file] = std::move(per_file_result); - continue; + return true; } spdlog::error("file: {} doesn't pass {} check.", file, option.binary); result.failed_commands.emplace_back( fmt::format("clang-format {}", per_file_result.file_option)); result.fails[file] = std::move(per_file_result); + return false; + }; + + if (option.enabled_fastly_exit || context.jobs == 1) { + for (const auto &file: files_to_check) { + auto per_file_result = check_single_file(context, root_dir, file); + if (merge_result(file, std::move(per_file_result))) { + continue; + } - if (option.enabled_fastly_exit) { - spdlog::info("{} fastly exit since check failed", option.binary); - result.final_passed = false; - result.fastly_exited = true; - return; + if (option.enabled_fastly_exit) { + spdlog::info("{} fastly exit since check failed", option.binary); + result.final_passed = false; + result.fastly_exited = true; + return; + } } + + result.final_passed = result.fails.empty(); + return; + } + + if (files_to_check.empty()) { + result.final_passed = true; + return; + } + + auto next_index = std::atomic_size_t{0}; + auto first_error = std::exception_ptr{}; + auto error_mutex = std::mutex{}; + auto file_results = std::vector>(files_to_check.size()); + auto worker_count = std::min(context.jobs, files_to_check.size()); + auto workers = std::vector{}; + workers.reserve(worker_count); + + for (std::size_t worker_idx = 0; worker_idx < worker_count; ++worker_idx) { + workers.emplace_back([&] { + while (true) { + const auto index = next_index.fetch_add(1); + if (index >= files_to_check.size()) { + break; + } + + try { + file_results[index] = check_single_file(context, root_dir, files_to_check[index]); + } catch (...) { + auto lock = std::lock_guard{error_mutex}; + if (!first_error) { + first_error = std::current_exception(); + } + } + } + }); + } + + for (auto &worker: workers) { + worker.join(); + } + + if (first_error) { + std::rethrow_exception(first_error); + } + + for (std::size_t index = 0; index < files_to_check.size(); ++index) { + auto per_file_result = std::move(*file_results[index]); + merge_result(files_to_check[index], std::move(per_file_result)); } result.final_passed = result.fails.empty(); diff --git a/src/tools/clang_format/general/reporter.h b/src/tools/clang_format/general/reporter.h index cff2bc0d..fdce33b8 100644 --- a/src/tools/clang_format/general/reporter.h +++ b/src/tools/clang_format/general/reporter.h @@ -15,6 +15,7 @@ */ #pragma once +#include #include #include @@ -47,7 +48,12 @@ namespace lint::tool::clang_format { auto get_detail_result([[maybe_unused]] const runtime_context &context) -> std::string override { auto content = ""s; + auto files = std::vector{}; for (const auto &[name, failed]: result.fails) { + files.push_back(name); + } + std::sort(files.begin(), files.end()); + for (const auto &name: files) { auto one = fmt::format("- {}\n", name); content += one; } @@ -58,9 +64,48 @@ namespace lint::tool::clang_format { return result.failed_commands; } - auto make_review_comment([[maybe_unused]] const runtime_context &context) - -> github::review_comments override { - return {}; + auto make_review_comment(const runtime_context &context) -> github::review_comments override { + auto comments = github::review_comments{}; + auto files = std::vector{}; + for (const auto &[file, per_file_result]: result.fails) { + files.push_back(file); + } + std::sort(files.begin(), files.end()); + + for (const auto &file: files) { + if (!context.patches.contains(file)) { + continue; + } + const auto &per_file_result = result.fails.at(file); + const auto &patch = context.patches.at(file); + const auto num_hunk = git::patch::num_hunks(*patch); + + auto rows = std::vector{}; + for (const auto &[row, replacements]: per_file_result.replacements) { + rows.push_back(row); + } + std::sort(rows.begin(), rows.end()); + + for (const auto row: rows) { + const auto row_number = static_cast(row); + auto pos = std::size_t{0}; + for (int hunk_idx = 0; hunk_idx < num_hunk; ++hunk_idx) { + auto [hunk, num_lines] = git::patch::get_hunk(*patch, hunk_idx); + if (!git::hunk::is_row_in_hunk(hunk, row_number)) { + pos += num_lines; + continue; + } + + auto comment = github::review_comment{}; + comment.path = file; + comment.position = pos + static_cast(row_number - hunk.new_start) + 1; + comment.body = "clang-format would reformat this line."; + comments.emplace_back(std::move(comment)); + break; + } + } + } + return comments; } void write_to_action_output([[maybe_unused]] const runtime_context &ctx) override { diff --git a/src/tools/clang_tidy/clang_tidy.cpp b/src/tools/clang_tidy/clang_tidy.cpp index b1a83e1b..c3cea716 100644 --- a/src/tools/clang_tidy/clang_tidy.cpp +++ b/src/tools/clang_tidy/clang_tidy.cpp @@ -124,9 +124,7 @@ namespace lint::tool::clang_tidy { auto [ec, std_out, std_err] = shell::which(option.binary); throw_unless(ec == 0, fmt::format("Can't find given clang-tidy binary: {}", option.binary)); } else { - auto [ec, std_out, std_err] = shell::which("clang-tidy"); - throw_unless(ec == 0, "can't find clang-tidy"); - option.binary = std_out; + option.binary = find_clang_tool("clang-tidy", default_clang_tool_version); } option.version = get_version(option.binary); diff --git a/src/tools/clang_tidy/general/impl.cpp b/src/tools/clang_tidy/general/impl.cpp index f9bde9b7..75dd6f35 100644 --- a/src/tools/clang_tidy/general/impl.cpp +++ b/src/tools/clang_tidy/general/impl.cpp @@ -15,11 +15,16 @@ */ #include "tools/clang_tidy/general/impl.h" +#include +#include #include +#include #include +#include #include #include #include +#include #include #include @@ -253,8 +258,12 @@ namespace lint::tool::clang_tidy { assert(!option.binary.empty() && "clang-tidy binary is empty"); assert(!context.repo_path.empty() && "the repo_path of context is empty"); + result = result_t{}; const auto root_dir = context.repo_path; - const auto files = context.changed_files; + auto files = context.changed_files; + std::sort(files.begin(), files.end()); + + auto files_to_check = std::vector{}; for (const auto &file: files) { const auto &delta = context.deltas.at(file); if (delta.status == GIT_DELTA_DELETED) { @@ -266,24 +275,85 @@ namespace lint::tool::clang_tidy { continue; } - auto per_file_result = check_single_file(context, root_dir, file); + files_to_check.push_back(file); + } + + auto merge_result = [&](const std::string &file, per_file_result per_file_result) { if (per_file_result.passed) { spdlog::info("file: {} passes {} check.", file, option.binary); result.passes[file] = std::move(per_file_result); - continue; + return true; } spdlog::error("file: {} doesn't pass {} check.", file, option.binary); - result.failed_commands.emplace_back( - fmt::format("clang-tidy {}", per_file_result.file_option)); + result.failed_commands.emplace_back(fmt::format("clang-tidy {}", per_file_result.file_option)); result.fails[file] = std::move(per_file_result); + return false; + }; - if (option.enabled_fastly_exit) { - spdlog::info("{} fastly exit since check failed", option.binary); - result.final_passed = false; - result.fastly_exited = true; - return; + if (option.enabled_fastly_exit || context.jobs == 1) { + for (const auto &file: files_to_check) { + auto per_file_result = check_single_file(context, root_dir, file); + if (merge_result(file, std::move(per_file_result))) { + continue; + } + + if (option.enabled_fastly_exit) { + spdlog::info("{} fastly exit since check failed", option.binary); + result.final_passed = false; + result.fastly_exited = true; + return; + } } + + result.final_passed = result.fails.empty(); + return; + } + + if (files_to_check.empty()) { + result.final_passed = true; + return; + } + + auto next_index = std::atomic_size_t{0}; + auto first_error = std::exception_ptr{}; + auto error_mutex = std::mutex{}; + auto file_results = std::vector>(files_to_check.size()); + auto worker_count = std::min(context.jobs, files_to_check.size()); + auto workers = std::vector{}; + workers.reserve(worker_count); + + for (std::size_t worker_idx = 0; worker_idx < worker_count; ++worker_idx) { + workers.emplace_back([&] { + while (true) { + const auto index = next_index.fetch_add(1); + if (index >= files_to_check.size()) { + break; + } + + try { + file_results[index] = check_single_file(context, root_dir, files_to_check[index]); + } catch (...) { + auto lock = std::lock_guard{error_mutex}; + if (!first_error) { + first_error = std::current_exception(); + } + } + } + }); + } + + for (auto &worker: workers) { + worker.join(); + } + + if (first_error) { + std::rethrow_exception(first_error); + } + + for (std::size_t index = 0; index < files_to_check.size(); ++index) { + auto per_file_result = std::move(*file_results[index]); + merge_result(files_to_check[index], std::move(per_file_result)); } result.final_passed = result.fails.empty(); diff --git a/src/tools/clang_tidy/general/reporter.h b/src/tools/clang_tidy/general/reporter.h index b3f2e855..4c73b3e0 100644 --- a/src/tools/clang_tidy/general/reporter.h +++ b/src/tools/clang_tidy/general/reporter.h @@ -17,6 +17,8 @@ #include "context.h" +#include + #include #include "github/common.h" @@ -71,12 +73,18 @@ namespace lint::tool::clang_tidy { result.ignored.size()}; } - auto get_detail_result([[maybe_unused]] const runtime_context &context) - -> std::string override { + auto get_detail_result([[maybe_unused]] const runtime_context &context) -> std::string override { spdlog::trace("Enter clang_tidy::reporter_t::get_detail_result()"); - auto ret = ""s; + auto ret = ""s; + auto files = std::vector{}; for (const auto &[name, failed]: result.fails) { + files.push_back(name); + } + std::sort(files.begin(), files.end()); + + for (const auto &name: files) { + const auto &failed = result.fails.at(name); for (const auto &diag: failed.diags) { // use relative file name rather than diag.header.file_name which is // absolute name @@ -99,9 +107,17 @@ namespace lint::tool::clang_tidy { auto comments = github::review_comments{}; // For each failed file: + auto files = std::vector{}; for (const auto &[file, per_file_result]: result.fails) { - assert(per_file_result.file_path == file); - assert(context.patches.contains(file)); + files.push_back(file); + } + std::sort(files.begin(), files.end()); + + for (const auto &file: files) { + if (!context.patches.contains(file)) { + continue; + } + const auto &per_file_result = result.fails.at(file); const auto &patch = context.patches.at(file); const auto num_hunk = git::patch::num_hunks(*patch); @@ -109,7 +125,6 @@ namespace lint::tool::clang_tidy { // For each clang-tidy diagnostic result in current file: for (const auto &diag: per_file_result.diags) { auto row = std::stoi(diag.header.row_idx); - auto col = std::stoi(diag.header.col_idx); // Check current diagnostic is in diff hunk. auto pos = std::size_t{0}; @@ -120,9 +135,13 @@ namespace lint::tool::clang_tidy { } else { auto comment = github::review_comment{}; comment.path = file; - comment.position = pos + row - hunk.new_start + 1; - comment.body = diag.header.brief + diag.header.checks; + comment.position = pos + static_cast(row - hunk.new_start) + 1; + comment.body = fmt::format("clang-tidy {}: {} [{}]", + diag.header.serverity, + diag.header.brief, + diag.header.checks); comments.emplace_back(std::move(comment)); + break; } } } diff --git a/src/tools/util.h b/src/tools/util.h index 80cab53a..811f1bed 100644 --- a/src/tools/util.h +++ b/src/tools/util.h @@ -17,12 +17,15 @@ #include #include +#include #include "utils/common.h" #include "utils/error.h" #include "utils/shell.h" namespace lint::tool { + inline constexpr auto default_clang_tool_version = std::string_view{"22"}; + // Find the full executable path of clang tools with specific version. inline auto find_clang_tool(std::string_view tool, std::string_view version) -> std::string { auto command = fmt::format("{}-{}", tool, version); diff --git a/src/utils/common.h b/src/utils/common.h index f405b888..514ec3ca 100644 --- a/src/utils/common.h +++ b/src/utils/common.h @@ -22,6 +22,7 @@ #include #include "utils/error.h" +#include "utils/std.h" namespace lint { constexpr auto trim_left(std::string_view str) -> std::string_view { diff --git a/src/utils/git_utils.cpp b/src/utils/git_utils.cpp index 9b7b2c86..dc645d55 100644 --- a/src/utils/git_utils.cpp +++ b/src/utils/git_utils.cpp @@ -974,7 +974,7 @@ namespace lint::git { } bool is_row_in_hunk(const git_diff_hunk &hunk, int row_number) noexcept { - return row_number >= hunk.new_start && row_number <= hunk.new_start + hunk.new_lines; + return row_number >= hunk.new_start && row_number < hunk.new_start + hunk.new_lines; } } // namespace hunk diff --git a/src/utils/shell.cpp b/src/utils/shell.cpp index 5767fb93..b5ec9cbf 100644 --- a/src/utils/shell.cpp +++ b/src/utils/shell.cpp @@ -19,13 +19,13 @@ #include #include -#define BOOST_PROCESS_V2_SEPARATE_COMPILATION #include #include #include -#include -#include +#include +#include #include +#include #include "utils/error.h" #include "utils/common.h" @@ -33,6 +33,25 @@ namespace lint::shell { namespace bp = boost::process::v2; + namespace { + void read_pipe(boost::asio::readable_pipe &pipe, + std::string &output, + std::string_view stream_name, + std::string_view command) { + auto ec = boost::system::error_code{}; + do { + ec.clear(); + boost::asio::read(pipe, boost::asio::dynamic_buffer(output), ec); + } while (ec == boost::asio::error::interrupted); + + throw_if(ec && ec != boost::asio::error::eof, + fmt::format("Read {} message of {} failed since {}", + stream_name, + command, + ec.message())); + } + } // namespace + auto execute(std::string_view command, const options &opts) -> result { auto context = boost::asio::io_context{}; auto rp_out = boost::asio::readable_pipe{context}; @@ -43,16 +62,9 @@ namespace lint::shell { command, opts, bp::process_stdio{.in = {}, .out = rp_out, .err = rp_err}); - auto ec = boost::system::error_code{}; auto res = result{}; - boost::asio::read(rp_out, boost::asio::dynamic_buffer(res.std_out), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stdout message of {} faild since {}", command, ec.message())); - ec.clear(); - - boost::asio::read(rp_err, boost::asio::dynamic_buffer(res.std_err), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stderr message of {} faild since {}", command, ec.message())); + read_pipe(rp_out, res.std_out, "stdout", command); + read_pipe(rp_err, res.std_err, "stderr", command); res.exit_code = proc.wait(); return res; @@ -70,16 +82,9 @@ namespace lint::shell { opts, bp::process_stdio{.in = {}, .out = rp_out, .err = rp_err}, bp::process_start_dir{start_dir}); - auto ec = boost::system::error_code{}; auto res = result{}; - boost::asio::read(rp_out, boost::asio::dynamic_buffer(res.std_out), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stdout message of {} faild since {}", command, ec.message())); - ec.clear(); - - boost::asio::read(rp_err, boost::asio::dynamic_buffer(res.std_err), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stderr message of {} faild since {}", command, ec.message())); + read_pipe(rp_out, res.std_out, "stdout", command); + read_pipe(rp_err, res.std_err, "stderr", command); res.exit_code = proc.wait(); return res; @@ -96,16 +101,9 @@ namespace lint::shell { opts, bp::process_stdio{.in = {}, .out = rp_out, .err = rp_err}, bp::process_environment{env}); - auto ec = boost::system::error_code{}; auto res = result{}; - boost::asio::read(rp_out, boost::asio::dynamic_buffer(res.std_out), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stdout message of {} faild since {}", command, ec.message())); - ec.clear(); - - boost::asio::read(rp_err, boost::asio::dynamic_buffer(res.std_err), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stderr message of {} faild since {}", command, ec.message())); + read_pipe(rp_out, res.std_out, "stdout", command); + read_pipe(rp_err, res.std_err, "stderr", command); res.exit_code = proc.wait(); return res; @@ -126,16 +124,9 @@ namespace lint::shell { bp::process_stdio{.in = {}, .out = rp_out, .err = rp_err}, bp::process_environment{env}, bp::process_start_dir(start_dir)); - auto ec = boost::system::error_code{}; auto res = result{}; - boost::asio::read(rp_out, boost::asio::dynamic_buffer(res.std_out), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stdout message of {} faild since {}", command, ec.message())); - ec.clear(); - - boost::asio::read(rp_err, boost::asio::dynamic_buffer(res.std_err), ec); - throw_if(ec && ec != boost::asio::error::eof, - fmt::format("Read stderr message of {} faild since {}", command, ec.message())); + read_pipe(rp_out, res.std_out, "stdout", command); + read_pipe(rp_err, res.std_err, "stderr", command); res.exit_code = proc.wait(); return res; diff --git a/src/utils/std.h b/src/utils/std.h index 3b36a2c1..23f2849b 100644 --- a/src/utils/std.h +++ b/src/utils/std.h @@ -15,6 +15,9 @@ */ #pragma once +#include +#include + #include /// Implement some lower standard version APIs which have adopted into higher cpp-standards. @@ -36,4 +39,3 @@ inline auto concat(const std::vector &strs, char delim = '\n') -> s return ranges::views::join(strs, delim) | ranges::to(); } - diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 769fb2c2..cb03cbe0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,4 +1,5 @@ FILE(GLOB_RECURSE test_files "${cpp_lint_action_root}/tests/*.cpp") -add_executable(test ${test_files} $) -target_link_libraries(test PRIVATE Catch2::Catch2) - +add_executable(test ${test_files}) +add_dependencies(test cpp-lint-action) +target_compile_definitions(test PRIVATE CPP_LINT_ACTION_BINARY=\"$\") +target_link_libraries(test PRIVATE dep_obj Catch2::Catch2) diff --git a/tests/test_base_reporter.cpp b/tests/test_base_reporter.cpp index 92421102..754f8c91 100644 --- a/tests/test_base_reporter.cpp +++ b/tests/test_base_reporter.cpp @@ -16,7 +16,13 @@ #include "tools/clang_tidy/general/reporter.h" #include "tools/clang_format/general/reporter.h" +#include "test_common.h" +#include "utils/env_manager.h" +#include "github/common.h" +#include +#include +#include #include #include @@ -24,7 +30,59 @@ using namespace lint; -namespace { } // namespace +namespace { + class fake_review_reporter : public tool::reporter_base { + public: + explicit fake_review_reporter(std::size_t comment_count) + : comment_count_{comment_count} { + } + + auto get_brief_result() -> std::tuple override { + return {false, 0, comment_count_, 0}; + } + + auto get_detail_result(const runtime_context &) -> std::string override { + return {}; + } + + auto make_review_comment(const runtime_context &) -> github::review_comments override { + auto comments = github::review_comments{}; + for (std::size_t index = 0; index < comment_count_; ++index) { + comments.push_back(github::review_comment{ + .path = fmt::format("file_{}.cpp", index), + .position = index + 1, + .body = fmt::format("comment {}", index), + }); + } + return comments; + } + + void write_to_action_output(const runtime_context &) override { + } + + auto get_failed_commands() -> std::vector override { + return {}; + } + + auto tool_name() -> std::string override { + return "fake-reviewer"; + } + + private: + std::size_t comment_count_; + }; + + auto read_text_file(const std::filesystem::path &path) -> std::string { + auto file = std::ifstream{path}; + REQUIRE(file.is_open()); + return {std::istreambuf_iterator{file}, std::istreambuf_iterator{}}; + } + + void write_empty_file(const std::filesystem::path &path) { + auto file = std::ofstream{path}; + REQUIRE(file.is_open()); + } +} // namespace TEST_CASE("Test all_passes function", "[cpp-lint-action][tools]") { auto tidy_opt = tool::clang_tidy::option_t{}; @@ -56,4 +114,90 @@ TEST_CASE("Test all_passes function", "[cpp-lint-action][tools]") { } } +TEST_CASE("Reporters write action output and stable summary markdown", + "[cpp-lint-action][tools][reporter]") { + auto temp_dir = std::filesystem::temp_directory_path() / "cpp_lint_action_reporter_test"; + std::filesystem::remove_all(temp_dir); + std::filesystem::create_directories(temp_dir); + auto guard = scope_guard{[&] { std::filesystem::remove_all(temp_dir); }}; + + auto output_file = temp_dir / "github-output.txt"; + auto summary_file = temp_dir / "github-summary.md"; + write_empty_file(output_file); + write_empty_file(summary_file); + + env::set_cache({ + {github::github_output, output_file.string()}, + {github::github_step_summary, summary_file.string()}, + }); + auto env_guard = scope_guard{[] { env::set_cache({}); }}; + + auto format_opt = tool::clang_format::option_t{}; + format_opt.binary = "/tmp/clang-format-fake"; + auto format_res = tool::clang_format::result_t{}; + format_res.final_passed = false; + format_res.failed_commands.push_back("clang-format --output-replacements-xml format_fail.cpp"); + auto format_file = tool::clang_format::per_file_result{}; + format_file.file_path = "format_fail.cpp"; + format_file.passed = false; + format_res.fails["format_fail.cpp"] = format_file; + + auto tidy_opt = tool::clang_tidy::option_t{}; + tidy_opt.binary = "/tmp/clang-tidy-fake"; + auto tidy_res = tool::clang_tidy::result_t{}; + tidy_res.final_passed = false; + tidy_res.failed_commands.push_back("clang-tidy -p=build tidy_fail.cpp"); + auto tidy_file = tool::clang_tidy::per_file_result{}; + tidy_file.file_path = "tidy_fail.cpp"; + tidy_file.passed = false; + auto diag = tool::clang_tidy::diagnostic{}; + diag.header.row_idx = "7"; + diag.header.col_idx = "3"; + diag.header.serverity = "warning"; + diag.header.brief = "fake tidy failure"; + diag.header.checks = "cppcoreguidelines-avoid-magic-numbers"; + tidy_file.diags.push_back(diag); + tidy_res.fails["tidy_fail.cpp"] = tidy_file; + + auto reporters = std::vector{}; + reporters.emplace_back( + std::make_unique(format_opt, format_res)); + reporters.emplace_back(std::make_unique(tidy_opt, tidy_res)); + + auto context = runtime_context{}; + tool::write_to_github_action_output(context, reporters); + tool::write_to_github_step_summary(context, reporters); + + auto output = read_text_file(output_file); + REQUIRE(output.find("clang_format_failed_number=1") != std::string::npos); + REQUIRE(output.find("clang_tidy_failed_number=1") != std::string::npos); + + auto summary = read_text_file(summary_file); + REQUIRE(summary.find("Analysis Report Generated") != std::string::npos); + REQUIRE(summary.find("| **clang-format-fake** |") != std::string::npos); + REQUIRE(summary.find("| **clang-tidy-fake** |") != std::string::npos); + REQUIRE(summary.find("format_fail.cpp") != std::string::npos); + REQUIRE(summary.find("tidy_fail.cpp") != std::string::npos); + REQUIRE(summary.find("fake tidy failure") != std::string::npos); + REQUIRE(summary.find("clang-format --output-replacements-xml format_fail.cpp") + != std::string::npos); + REQUIRE(summary.find("clang-tidy -p=build tidy_fail.cpp") != std::string::npos); + REQUIRE(summary.find("Steps to reproduce") != std::string::npos); +} + +TEST_CASE("Pull request review comments are capped and empty reviews are skipped", + "[cpp-lint-action][tools][reporter]") { + auto context = runtime_context{}; + context.max_review_comments = 4; + auto reporters = std::vector{}; + reporters.emplace_back(std::make_unique(3)); + reporters.emplace_back(std::make_unique(2)); + + auto comments = tool::collect_pull_request_review_comments(context, reporters); + REQUIRE(comments.size() == 4); + REQUIRE(context.report_notes.size() == 1); + REQUIRE(context.report_notes.front().find("truncated from 5 to 4") != std::string::npos); + + REQUIRE_NOTHROW(tool::comment_on_github_pull_request_review(context, github::review_comments{})); +} diff --git a/tests/test_clang_format.cpp b/tests/test_clang_format.cpp index 700f669a..190ca0cb 100644 --- a/tests/test_clang_format.cpp +++ b/tests/test_clang_format.cpp @@ -103,7 +103,7 @@ namespace { TEST_CASE("Test register and create clang-format option", "[cpp-lint-action][program_options][tool][clang_format][creator]") { - SKIP_IF_NO_CLANG_FORMAT + SKIP_IF_NOT_HAS_CLANG_FORMAT_VERSION("22") auto creator = std::make_unique(); auto desc = create_then_register_tool_desc(*creator); @@ -126,7 +126,7 @@ TEST_CASE("Test register and create clang-format option", } SECTION("Receive an invalid clang-format version should throw exception") { - auto opts = parse_opt(desc, "--target-revision=main", "--clang-format-version=18.x.1"); + auto opts = parse_opt(desc, "--target-revision=main", "--clang-format-version=22.x.1"); REQUIRE_THROWS(creator->create_option(opts)); } @@ -153,11 +153,11 @@ TEST_CASE("Test register and create clang-format option", TEST_CASE("Test clang-format should get full version even though user input a " "simplified version", "[cpp-lint-action][tool][clang_format][creator]") { - SKIP_IF_NOT_HAS_CLANG_FORMAT_VERSION("18") + SKIP_IF_NOT_HAS_CLANG_FORMAT_VERSION("22") auto creator = std::make_unique(); auto desc = create_then_register_tool_desc(*creator); - auto vars = parse_opt(desc, "--target-revision=main", "--clang-format-version=18"); + auto vars = parse_opt(desc, "--target-revision=main", "--clang-format-version=22"); auto context = runtime_context{}; program_options::fill_context(vars, context); @@ -165,7 +165,7 @@ TEST_CASE("Test clang-format should get full version even though user input a " auto version = clang_format->version(); auto parts = ranges::views::split(version, '.') | ranges::to>(); REQUIRE(parts.size() == 3); - REQUIRE(parts[0] == "18"); + REQUIRE(parts[0] == "22"); REQUIRE(ranges::all_of(parts[1], isdigit)); REQUIRE(ranges::all_of(parts[2], isdigit)); } diff --git a/tests/test_clang_tidy.cpp b/tests/test_clang_tidy.cpp index ac304c25..53f76436 100644 --- a/tests/test_clang_tidy.cpp +++ b/tests/test_clang_tidy.cpp @@ -103,7 +103,7 @@ namespace { TEST_CASE("Test register and create clang-tidy option", "[cpp-lint-action][program_options][tool][clang_tidy][creator]") { - SKIP_IF_NO_CLANG_TIDY + SKIP_IF_NOT_HAS_CLANG_TIDY_VERSION("22") auto creator = std::make_unique(); auto desc = create_then_register_tool_desc(*creator); @@ -126,7 +126,7 @@ TEST_CASE("Test register and create clang-tidy option", } SECTION("Receive an invalid clang-tidy version should throw exception") { - auto opts = parse_opt(desc, "--target-revision=main", "--clang-tidy-version=18.x.1"); + auto opts = parse_opt(desc, "--target-revision=main", "--clang-tidy-version=22.x.1"); REQUIRE_THROWS(creator->create_option(opts)); } @@ -152,11 +152,11 @@ TEST_CASE("Test register and create clang-tidy option", TEST_CASE("Test clang-tidy should get full version even though user input a " "simplified version", "[cpp-lint-action][tool][clang_tidy][creator]") { - SKIP_IF_NOT_HAS_CLANG_TIDY_VERSION("18") + SKIP_IF_NOT_HAS_CLANG_TIDY_VERSION("22") auto creator = std::make_unique(); auto desc = create_then_register_tool_desc(*creator); - auto vars = parse_opt(desc, "--target-revision=main", "--clang-tidy-version=18"); + auto vars = parse_opt(desc, "--target-revision=main", "--clang-tidy-version=22"); auto context = runtime_context{}; program_options::fill_context(vars, context); @@ -164,7 +164,7 @@ TEST_CASE("Test clang-tidy should get full version even though user input a " auto version = clang_tidy->version(); auto parts = ranges::views::split(version, '.') | ranges::to>(); REQUIRE(parts.size() == 3); - REQUIRE(parts[0] == "18"); + REQUIRE(parts[0] == "22"); REQUIRE(ranges::all_of(parts[1], isdigit)); REQUIRE(ranges::all_of(parts[2], isdigit)); } diff --git a/tests/test_git.cpp b/tests/test_git.cpp index 11458e89..f11e38d4 100644 --- a/tests/test_git.cpp +++ b/tests/test_git.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -23,6 +24,7 @@ #include #include +#include "context.h" #include "test_common.h" #include "utils/git_utils.h" @@ -32,6 +34,17 @@ using namespace std::string_view_literals; const auto default_branch = "master"s; +namespace { + auto clean_commit_id(std::string id) -> std::string { + id.erase(std::find(id.begin(), id.end(), '\0'), id.end()); + return id; + } + + auto commit_changes(repo_t &repo) -> std::string { + return clean_commit_id(repo.commit_changes()); + } +} // namespace + TEST_CASE("Create repo should work", "[cpp-lint-action][git2][repo]") { create_temp_repo_dir(); auto guard = scope_guard{remove_temp_repo_dir}; @@ -39,7 +52,8 @@ TEST_CASE("Create repo should work", "[cpp-lint-action][git2][repo]") { auto repo = git::repo::init(get_temp_repo_dir(), false); REQUIRE(git::repo::is_empty(*repo)); auto temp_repo_dir_with_git = get_temp_repo_dir() / ".git/"; - REQUIRE(git::repo::path(*repo) == temp_repo_dir_with_git); + REQUIRE(std::filesystem::weakly_canonical(git::repo::path(*repo)) + == std::filesystem::weakly_canonical(temp_repo_dir_with_git)); } TEST_CASE("Set config should work", "[cpp-lint-action][git2][config]") { @@ -215,6 +229,41 @@ TEST_CASE("Push two commits and get diff files", "[cpp-lint-action][git2][diff]" REQUIRE(changed_files.size() == 1); } +TEST_CASE("Runtime git context tracks added modified deleted and renamed paths", + "[cpp-lint-action][git2][diff][context]") { + auto repo = repo_t{}; + + repo.add_file("modified.cpp", "int old_value = 0;\n"); + repo.add_file("deleted.cpp", "int removed_value = 0;\n"); + repo.add_file("renamed_old.cpp", "int renamed_value = 0;\n"); + auto target = commit_changes(repo); + + repo.rewrite_file("modified.cpp", "int new_value = 1;\n"); + repo.remove_file("deleted.cpp"); + repo.remove_file("renamed_old.cpp"); + repo.add_file("renamed_new.cpp", "int renamed_value = 0;\n"); + repo.add_file("added.cpp", "int added_value = 0;\n"); + auto source = commit_changes(repo); + + auto context = runtime_context{}; + context.repo_path = repo.get_path(); + context.target = target; + context.source = source; + fill_git_info(context); + + REQUIRE(ranges::contains(context.changed_files, "modified.cpp")); + REQUIRE(ranges::contains(context.changed_files, "deleted.cpp")); + REQUIRE(ranges::contains(context.changed_files, "renamed_old.cpp")); + REQUIRE(ranges::contains(context.changed_files, "renamed_new.cpp")); + REQUIRE(ranges::contains(context.changed_files, "added.cpp")); + + REQUIRE(context.deltas.at("modified.cpp").status == GIT_DELTA_MODIFIED); + REQUIRE(context.deltas.at("deleted.cpp").status == GIT_DELTA_DELETED); + REQUIRE(context.deltas.at("renamed_old.cpp").status == GIT_DELTA_DELETED); + REQUIRE(context.deltas.at("renamed_new.cpp").status == GIT_DELTA_ADDED); + REQUIRE(context.deltas.at("added.cpp").status == GIT_DELTA_ADDED); +} + TEST_CASE("Simple use of patch ", "[cpp-lint-action][git2][patch]") { create_temp_repo_dir(); auto guard = scope_guard{remove_temp_repo_dir}; @@ -283,6 +332,13 @@ TEST_CASE("Get lines in a hunk", "[cpp-lint-action][git2][patch]") { auto diff = git::diff::commit_to_commit(*repo, *commit1, *commit2); auto patch = git::patch::create_from_diff(*diff, 0); + auto [hunk, num_lines] = git::patch::get_hunk(*patch, 0); + REQUIRE(num_lines > 0); + REQUIRE(hunk.new_lines > 0); + REQUIRE(git::hunk::is_row_in_hunk(hunk, hunk.new_start)); + REQUIRE(git::hunk::is_row_in_hunk(hunk, hunk.new_start + hunk.new_lines - 1)); + REQUIRE_FALSE(git::hunk::is_row_in_hunk(hunk, hunk.new_start + hunk.new_lines)); + auto contents = git::patch::get_lines_in_hunk(*patch, 0); REQUIRE(contents[0] == "hello world\n"); REQUIRE(contents[1] == "hello world2\n"); diff --git a/tests/test_github_client.cpp b/tests/test_github_client.cpp new file mode 100644 index 00000000..c45a08cc --- /dev/null +++ b/tests/test_github_client.cpp @@ -0,0 +1,371 @@ +/* + * Copyright (c) 2024 Emmett Zhang + * + * Licensed under the Apache License Version 2.0 with LLVM Exceptions + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://llvm.org/LICENSE.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "github/client.h" +#include "github/common.h" +#include "github/review_comment.h" + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +using namespace lint; + +namespace { + constexpr auto comment_id = 5000000000LL; + + struct request_record { + bool called = false; + + std::string accept; + std::string authorization; + std::string content_type; + std::string user_agent; + std::string api_version; + std::string path; + std::string body; + }; + + class server_runner { + public: + explicit server_runner(httplib::Server &server) + : server_{server} { + port_ = server_.bind_to_any_port("127.0.0.1"); + REQUIRE(port_ > 0); + + thread_ = std::thread{[this] { server_.listen_after_bind(); }}; + server_.wait_until_ready(); + } + + ~server_runner() { + server_.stop(); + if (thread_.joinable()) { + thread_.join(); + } + } + + auto api_url() const -> std::string { + return "http://127.0.0.1:" + std::to_string(port_); + } + + private: + httplib::Server &server_; + int port_ = -1; + std::thread thread_; + }; + + auto make_context() -> runtime_context { + auto ctx = runtime_context{}; + ctx.repo_pair = "owner/repo"; + ctx.token = "github-token"; + ctx.event_name = github::github_event_pull_request; + ctx.pr_number = 42; + return ctx; + } + + void record_request(const httplib::Request &req, + request_record &record, + std::mutex &record_mutex) { + auto lock = std::lock_guard{record_mutex}; + record.called = true; + record.accept = req.get_header_value("Accept"); + record.authorization = req.get_header_value("Authorization"); + record.content_type = req.get_header_value("Content-Type"); + record.user_agent = req.get_header_value("User-Agent"); + record.api_version = req.get_header_value("X-GitHub-Api-Version"); + record.path = req.path; + record.body = req.body; + } + + void check_github_json_request(const request_record &record, const std::string &body) { + REQUIRE(record.called); + REQUIRE(record.accept == "application/vnd.github+json"); + REQUIRE(record.authorization == "Bearer github-token"); + REQUIRE(record.user_agent == "cpp-lint-action"); + REQUIRE(record.api_version == "2022-11-28"); + REQUIRE(record.content_type.starts_with("application/json")); + + auto payload = nlohmann::json::parse(record.body); + REQUIRE(payload["body"].get() + == std::string{github::github_comment_identifier} + body); + } + + template + auto capture_runtime_error(Fn &&fn) -> std::string { + try { + std::forward(fn)(); + } catch (const std::runtime_error &err) { + return err.what(); + } + + FAIL("expected std::runtime_error"); + return {}; + } + + auto contains_text(const std::string &text, const std::string &needle) -> bool { + return text.find(needle) != std::string::npos; + } +} // namespace + +TEST_CASE("GitHub client creates issue comments with the REST JSON API", + "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + auto mutex = std::mutex{}; + auto record = request_record{}; + + server.Post("/repos/owner/repo/issues/42/comments", + [&](const httplib::Request &req, httplib::Response &res) { + record_request(req, record, mutex); + + auto response = nlohmann::json{}; + response["id"] = comment_id; + res.set_content(response.dump(), "application/json"); + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + github_client.add_or_update_issue_comment(make_context(), "new comment"); + + auto lock = std::lock_guard{mutex}; + check_github_json_request(record, "new comment"); +} + +TEST_CASE("GitHub client updates existing issue comments with PATCH", + "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + auto mutex = std::mutex{}; + auto record = request_record{}; + + server.Get("/repos/owner/repo/issues/42/comments", + [](const httplib::Request &, httplib::Response &res) { + auto comments = nlohmann::json::array( + {{{"id", comment_id}, + {"body", std::string{github::github_comment_identifier} + "old"}}}); + res.set_content(comments.dump(), "application/json"); + }); + server.Patch("/repos/owner/repo/issues/comments/5000000000", + [&](const httplib::Request &req, httplib::Response &res) { + record_request(req, record, mutex); + + auto response = nlohmann::json{}; + response["id"] = comment_id; + res.set_content(response.dump(), "application/json"); + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + auto ctx = make_context(); + github_client.get_issue_comment_id(ctx); + github_client.add_or_update_issue_comment(ctx, "updated comment"); + + auto lock = std::lock_guard{mutex}; + check_github_json_request(record, "updated comment"); +} + +TEST_CASE("GitHub client creates a comment after reading an empty comment list", + "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + auto mutex = std::mutex{}; + auto record = request_record{}; + + server.Get("/repos/owner/repo/issues/42/comments", + [](const httplib::Request &, httplib::Response &res) { + res.set_content(nlohmann::json::array().dump(), "application/json"); + }); + server.Post("/repos/owner/repo/issues/42/comments", + [&](const httplib::Request &req, httplib::Response &res) { + record_request(req, record, mutex); + + auto response = nlohmann::json{}; + response["id"] = comment_id; + res.set_content(response.dump(), "application/json"); + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + auto ctx = make_context(); + github_client.get_issue_comment_id(ctx); + github_client.add_or_update_issue_comment(ctx, "first comment"); + + auto lock = std::lock_guard{mutex}; + check_github_json_request(record, "first comment"); +} + +TEST_CASE("GitHub client posts pull request reviews with JSON API headers", + "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + auto mutex = std::mutex{}; + auto record = request_record{}; + + server.Post("/repos/owner/repo/pulls/42/reviews", + [&](const httplib::Request &req, httplib::Response &res) { + record_request(req, record, mutex); + res.set_content(R"({"id":100})", "application/json"); + }); + + auto comments = github::review_comments{}; + comments.push_back(github::review_comment{ + .path = "src/main.cpp", + .position = 3, + .body = "diagnostic body", + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + github_client.post_pull_request_review(make_context(), github::make_review_str(comments)); + + auto lock = std::lock_guard{mutex}; + REQUIRE(record.called); + REQUIRE(record.accept == "application/vnd.github+json"); + REQUIRE(record.authorization == "Bearer github-token"); + REQUIRE(record.user_agent == "cpp-lint-action"); + REQUIRE(record.api_version == "2022-11-28"); + REQUIRE(record.content_type.starts_with("application/json")); + + auto payload = nlohmann::json::parse(record.body); + REQUIRE(payload["event"].get() == "COMMENT"); + REQUIRE(payload["comments"].size() == 1); + REQUIRE(payload["comments"][0]["path"].get() == "src/main.cpp"); + REQUIRE(payload["comments"][0]["position"].get() == 3); + REQUIRE(payload["comments"][0]["body"].get() == "diagnostic body"); +} + +TEST_CASE("GitHub client includes response body when REST API returns an error", + "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + server.Get("/repos/owner/repo/issues/42/comments", + [](const httplib::Request &, httplib::Response &res) { + res.status = 404; + res.reason = "Not Found"; + res.set_content(R"({"message":"missing issue"})", "application/json"); + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + + const auto message = + capture_runtime_error([&github_client] { github_client.get_issue_comment_id(make_context()); }); + REQUIRE(contains_text(message, "404")); + REQUIRE(contains_text(message, "missing issue")); +} + +TEST_CASE("GitHub client paginates issue comments before updating the newest match", + "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + auto mutex = std::mutex{}; + auto record = request_record{}; + + server.Get("/repos/owner/repo/issues/42/comments", + [](const httplib::Request &req, httplib::Response &res) { + auto comments = nlohmann::json::array(); + if (req.get_param_value("page") == "1") { + for (auto index = 0; index < 100; ++index) { + comments.push_back({{"id", index + 1}, {"body", "other comment"}}); + } + } else { + comments.push_back( + {{"id", comment_id - 1}, + {"body", std::string{github::github_comment_identifier} + "older"}}); + comments.push_back( + {{"id", comment_id}, + {"body", std::string{github::github_comment_identifier} + "newer"}}); + } + res.set_content(comments.dump(), "application/json"); + }); + server.Patch("/repos/owner/repo/issues/comments/5000000000", + [&](const httplib::Request &req, httplib::Response &res) { + record_request(req, record, mutex); + res.set_content(R"({"id":5000000000})", "application/json"); + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + auto ctx = make_context(); + github_client.get_issue_comment_id(ctx); + github_client.add_or_update_issue_comment(ctx, "updated newest comment"); + + auto lock = std::lock_guard{mutex}; + check_github_json_request(record, "updated newest comment"); + REQUIRE(record.path == "/repos/owner/repo/issues/comments/5000000000"); +} + +TEST_CASE("GitHub client retries retryable REST API failures", "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + auto calls = std::atomic_int{0}; + auto mutex = std::mutex{}; + auto record = request_record{}; + + server.Post("/repos/owner/repo/issues/42/comments", + [&](const httplib::Request &req, httplib::Response &res) { + ++calls; + record_request(req, record, mutex); + if (calls.load() == 1) { + res.status = 500; + res.reason = "Internal Server Error"; + res.set_content(R"({"message":"temporary"})", "application/json"); + return; + } + + res.set_content(R"({"id":5000000000})", "application/json"); + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + github_client.add_issue_comment(make_context(), "retry comment"); + + REQUIRE(calls.load() == 2); + auto lock = std::lock_guard{mutex}; + check_github_json_request(record, "retry comment"); +} + +TEST_CASE("GitHub client reports rate-limit headers on REST API errors", + "[cpp-lint-action][github]") { + auto server = httplib::Server{}; + server.Get("/repos/owner/repo/issues/42/comments", + [](const httplib::Request &, httplib::Response &res) { + res.status = 403; + res.reason = "Forbidden"; + res.set_header("X-RateLimit-Remaining", "0"); + res.set_header("X-RateLimit-Reset", "1234567890"); + res.set_content(R"({"message":"rate limit"})", "application/json"); + }); + + auto runner = server_runner{server}; + auto github_client = github::client{runner.api_url()}; + + const auto message = + capture_runtime_error([&github_client] { github_client.get_issue_comment_id(make_context()); }); + REQUIRE(contains_text(message, "403")); + REQUIRE(contains_text(message, "rate-limit")); + REQUIRE(contains_text(message, "remaining=0")); + REQUIRE(contains_text(message, "reset=1234567890")); +} + +TEST_CASE("GitHub client reports empty HTTP responses without dereferencing them", + "[cpp-lint-action][github]") { + auto github_client = github::client{"http://127.0.0.1:1"}; + + const auto message = capture_runtime_error( + [&github_client] { github_client.add_issue_comment(make_context(), "body"); }); + REQUIRE(contains_text(message, "github http request failed")); +} diff --git a/tests/test_github_common.cpp b/tests/test_github_common.cpp new file mode 100644 index 00000000..f8f595ce --- /dev/null +++ b/tests/test_github_common.cpp @@ -0,0 +1,124 @@ +/* + * Copyright (c) 2024 Emmett Zhang + * + * Licensed under the Apache License Version 2.0 with LLVM Exceptions + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://llvm.org/LICENSE.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "github/common.h" +#include "test_common.h" +#include "utils/env_manager.h" + +#include +#include +#include +#include + +#include + +using namespace lint; + +namespace { + auto write_event_payload(const std::string &name, const std::string &content) + -> std::filesystem::path { + auto path = std::filesystem::temp_directory_path() / name; + auto file = std::ofstream{path}; + REQUIRE(file.is_open()); + file << content; + return path; + } + + void set_debug_github_env(std::unordered_map extra = {}) { + auto values = std::unordered_map{ + {github::debug_cpp_lint_action, "true"}, + {"DEBUG_GITHUB_REPOSITORY", "owner/repo"}, + {"DEBUG_GITHUB_TOKEN", "github-token"}, + {"DEBUG_GITHUB_EVENT_NAME", github::github_event_pull_request}, + {"DEBUG_GITHUB_BASE_REF", "main"}, + {"DEBUG_GITHUB_HEAD_REF", "feature"}, + {"DEBUG_GITHUB_REF", "refs/pull/9/merge"}, + {"DEBUG_GITHUB_SHA", "1234567890abcdef"}, + {"DEBUG_GITHUB_REF_TYPE", "branch"}, + {"DEBUG_GITHUB_WORKSPACE", "/tmp/repo"}, + {"DEBUG_GITHUB_EVENT_PATH", ""}, + }; + for (auto &[key, value]: extra) { + values[key] = std::move(value); + } + env::set_cache(std::move(values)); + } +} // namespace + +TEST_CASE("GitHub environment reads pull request number from event payload first", + "[cpp-lint-action][github]") { + auto event_path = write_event_payload("cpp_lint_action_pull_request_event.json", R"({"number":42})"); + auto guard = scope_guard{[&] { + std::filesystem::remove(event_path); + env::set_cache({}); + }}; + + set_debug_github_env({{"DEBUG_GITHUB_EVENT_PATH", event_path.string()}, + {"DEBUG_GITHUB_REF", "refs/pull/9/merge"}}); + + auto ctx = runtime_context{}; + auto env = github::read_env(); + github::fill_context(env, ctx); + + REQUIRE(ctx.event_name == github::github_event_pull_request); + REQUIRE(ctx.pr_number == 42); +} + +TEST_CASE("GitHub environment falls back to pull request number from ref", + "[cpp-lint-action][github]") { + auto guard = scope_guard{[] { env::set_cache({}); }}; + set_debug_github_env({{"DEBUG_GITHUB_EVENT_PATH", ""}, {"DEBUG_GITHUB_REF", "refs/pull/77/merge"}}); + + auto ctx = runtime_context{}; + auto env = github::read_env(); + github::fill_context(env, ctx); + + REQUIRE(ctx.pr_number == 77); +} + +TEST_CASE("GitHub reporting is skipped for non pull request events without requiring a token", + "[cpp-lint-action][github]") { + auto guard = scope_guard{[] { env::set_cache({}); }}; + set_debug_github_env({{"DEBUG_GITHUB_TOKEN", ""}, + {"DEBUG_GITHUB_EVENT_NAME", github::github_event_push}, + {"DEBUG_GITHUB_REF", "refs/heads/main"}}); + + auto ctx = runtime_context{}; + ctx.enable_comment_on_issue = true; + ctx.enable_pull_request_review = true; + auto env = github::read_env(); + github::fill_context(env, ctx); + + REQUIRE_NOTHROW(github::prepare_reporting_context(ctx)); + REQUIRE(ctx.enable_comment_on_issue == false); + REQUIRE(ctx.enable_pull_request_review == false); + REQUIRE(ctx.report_notes.size() == 1); +} + +TEST_CASE("GitHub reporting requires token and pull request number for pull request events", + "[cpp-lint-action][github]") { + auto guard = scope_guard{[] { env::set_cache({}); }}; + set_debug_github_env({{"DEBUG_GITHUB_TOKEN", ""}, + {"DEBUG_GITHUB_EVENT_PATH", ""}, + {"DEBUG_GITHUB_REF", "refs/heads/feature"}}); + + auto ctx = runtime_context{}; + ctx.enable_comment_on_issue = true; + auto env = github::read_env(); + github::fill_context(env, ctx); + + REQUIRE_THROWS(github::prepare_reporting_context(ctx)); +} diff --git a/tests/test_program_options.cpp b/tests/test_program_options.cpp index 07074b67..2babfe29 100644 --- a/tests/test_program_options.cpp +++ b/tests/test_program_options.cpp @@ -16,6 +16,7 @@ #include "github/common.h" #include "program_options.h" +#include "tools/util.h" #include "utils/env_manager.h" #include @@ -90,10 +91,10 @@ TEST_CASE("Test fill context by program options", "[cpp-lint-action][program_opt } SECTION("enable_comment_on_issue should be passed into context") { - auto opts = make_opt("--target-revision=main", "--enable-step-summary=true"); + auto opts = make_opt("--target-revision=main", "--enable-comment-on-issue=false"); auto user_options = parse(opts.size(), opts.data(), desc); REQUIRE_NOTHROW(fill_context(user_options, context)); - REQUIRE(context.enable_step_summary == true); + REQUIRE(context.enable_comment_on_issue == false); } SECTION("enable_pull_request_review should be passed into context") { @@ -111,5 +112,34 @@ TEST_CASE("Test fill context by program options", "[cpp-lint-action][program_opt REQUIRE(context.enable_comment_on_issue == true); REQUIRE(context.enable_pull_request_review == false); REQUIRE(context.enable_action_output == true); + REQUIRE(context.disable_errors == false); + REQUIRE(context.jobs == 1); + REQUIRE(context.max_review_comments == 50); + } + + SECTION("disable_errors should be passed into context") { + auto opts = make_opt("--target-revision=main", "--disable-errors=true"); + auto user_options = parse(opts.size(), opts.data(), desc); + REQUIRE_NOTHROW(fill_context(user_options, context)); + REQUIRE(context.disable_errors == true); } + + SECTION("jobs and max-review-comments should be passed into context") { + auto opts = make_opt("--target-revision=main", "--jobs=3", "--max-review-comments=12"); + auto user_options = parse(opts.size(), opts.data(), desc); + REQUIRE_NOTHROW(fill_context(user_options, context)); + REQUIRE(context.jobs == 3); + REQUIRE(context.max_review_comments == 12); + } + + SECTION("jobs should reject zero") { + auto opts = make_opt("--target-revision=main", "--jobs=0"); + auto user_options = parse(opts.size(), opts.data(), desc); + REQUIRE_THROWS(fill_context(user_options, context)); + } +} + +TEST_CASE("Default clang tool version is pinned to the supported latest major", + "[cpp-lint-action][program_options][tool]") { + REQUIRE(tool::default_clang_tool_version == "22"); } diff --git a/tests/test_tool_availability.cpp b/tests/test_tool_availability.cpp new file mode 100644 index 00000000..d009bef6 --- /dev/null +++ b/tests/test_tool_availability.cpp @@ -0,0 +1,511 @@ +/* + * Copyright (c) 2024 Emmett Zhang + * + * Licensed under the Apache License Version 2.0 with LLVM Exceptions + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://llvm.org/LICENSE.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "context.h" +#include "github/common.h" +#include "program_options.h" +#include "test_common.h" +#include "tools/base_creator.h" +#include "tools/clang_format/clang_format.h" +#include "tools/clang_format/general/impl.h" +#include "tools/clang_tidy/clang_tidy.h" +#include "tools/clang_tidy/general/impl.h" +#include "tools/util.h" +#include "utils/shell.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +using namespace lint; +using namespace lint::tool; + +namespace { + class temporary_dir { + public: + explicit temporary_dir(std::string name) { + static auto counter = std::atomic_uint64_t{0}; + const auto stamp = std::chrono::steady_clock::now().time_since_epoch().count(); + path_ = std::filesystem::temp_directory_path() + / (std::move(name) + "_" + std::to_string(stamp) + "_" + + std::to_string(counter.fetch_add(1))); + std::filesystem::create_directories(path_); + } + + ~temporary_dir() { + std::error_code ec; + std::filesystem::remove_all(path_, ec); + } + + temporary_dir(const temporary_dir &) = delete; + temporary_dir &operator=(const temporary_dir &) = delete; + + auto path() const -> const std::filesystem::path & { + return path_; + } + + private: + std::filesystem::path path_; + }; + + void write_text_file(const std::filesystem::path &path, const std::string &content) { + auto file = std::ofstream{path}; + REQUIRE(file.is_open()); + file << content; + } + + auto read_text_file(const std::filesystem::path &path) -> std::string { + auto file = std::ifstream{path}; + REQUIRE(file.is_open()); + return {std::istreambuf_iterator{file}, std::istreambuf_iterator{}}; + } + + void make_executable(const std::filesystem::path &path) { + std::filesystem::permissions(path, + std::filesystem::perms::owner_exec + | std::filesystem::perms::group_exec + | std::filesystem::perms::others_exec, + std::filesystem::perm_options::add); + } + + auto write_fake_clang_format(const std::filesystem::path &dir) -> std::filesystem::path { + const auto path = dir / "clang-format-fake"; + write_text_file(path, R"sh(#!/bin/sh +log="${FAKE_CLANG_LOG:-}" +if [ -n "$log" ]; then + printf '%s\n' "$0 $*" >> "$log" +fi + +for arg in "$@"; do + if [ "$arg" = "--version" ]; then + printf 'Ubuntu clang-format version 22.0.1\n' + exit 0 + fi +done + +last="" +for arg in "$@"; do + last="$arg" +done +base=${last##*/} + +case "$base" in + format_malformed*) + printf 'not xml\n' + exit 0 + ;; + format_error*) + printf 'fake clang-format error for %s\n' "$base" >&2 + exit 7 + ;; + format_fail*) + printf '%s\n' ' ' + exit 0 + ;; + *) + printf '%s\n' '' + exit 0 + ;; +esac +)sh"); + make_executable(path); + return path; + } + + auto write_fake_clang_tidy(const std::filesystem::path &dir) -> std::filesystem::path { + const auto path = dir / "clang-tidy-fake"; + write_text_file(path, R"sh(#!/bin/sh +log="${FAKE_CLANG_LOG:-}" +if [ -n "$log" ]; then + printf '%s\n' "$0 $*" >> "$log" +fi + +for arg in "$@"; do + if [ "$arg" = "--version" ]; then + printf 'Ubuntu LLVM version 22.0.1\n' + exit 0 + fi +done + +last="" +for arg in "$@"; do + last="$arg" +done +base=${last##*/} + +case "$base" in + tidy_malformed*) + printf 'this is not a clang-tidy diagnostic\n' + exit 1 + ;; + tidy_error*) + printf 'fake clang-tidy error for %s\n' "$base" >&2 + exit 9 + ;; + tidy_fail*) + printf '%s:1:5: warning: fake tidy failure [cppcoreguidelines-avoid-non-const-global-variables]\n' "$last" + printf 'int n;\n' + printf ' ^\n' + printf '1 warning generated.\n' >&2 + exit 1 + ;; + *) + exit 0 + ;; +esac +)sh"); + make_executable(path); + return path; + } + + auto make_context(repo_t &repo, const std::string &target, const std::string &source) + -> runtime_context { + auto context = runtime_context{}; + context.repo_path = repo.get_path(); + context.target = target; + context.source = source; + fill_git_info(context); + return context; + } + + auto commit_changes(repo_t &repo) -> std::string { + auto id = repo.commit_changes(); + id.erase(std::find(id.begin(), id.end(), '\0'), id.end()); + return id; + } + + auto contains_text(const std::string &text, const std::string &needle) -> bool { + return text.find(needle) != std::string::npos; + } + + auto get_env_value(const char *name) -> std::string { + const auto *value = std::getenv(name); + return value == nullptr ? std::string{} : std::string{value}; + } + + template + auto make_opt(Opts &&...opts) -> std::array { + return {const_cast("cpp-lint-action"), // NOLINT + const_cast(std::forward(opts))...}; + } + + template + auto parse_opt(const program_options::options_description &desc, Opts &&...opts) + -> program_options::variables_map { + auto inputs = make_opt(std::forward(opts)...); + return program_options::parse(inputs.size(), inputs.data(), desc); + } + + auto make_tool_desc(const clang_format::creator &format_creator, + const clang_tidy::creator &tidy_creator) + -> program_options::options_description { + auto desc = program_options::create_desc(); + format_creator.register_option(desc); + tidy_creator.register_option(desc); + return desc; + } +} // namespace + +TEST_CASE("Fake clang-format keeps format checks available without a local clang install", + "[cpp-lint-action][tool][clang_format][fake]") { + auto fake_dir = temporary_dir{"cpp_lint_action_fake_format"}; + auto binary = write_fake_clang_format(fake_dir.path()); + + auto repo = repo_t{}; + repo.add_file("format_pass.cpp", "int value = 0;\n"); + repo.add_file("format_fail.cpp", "int value = 0;\n"); + auto target = commit_changes(repo); + repo.rewrite_file("format_pass.cpp", "int value = 1;\n"); + repo.rewrite_file("format_fail.cpp", "int value = 2;\n"); + auto source = commit_changes(repo); + + auto context = make_context(repo, target, source); + + auto option = clang_format::option_t{}; + option.enabled = true; + option.binary = binary.string(); + option.version = "22.0.1"; + + auto format = clang_format::clang_format_general{option}; + format.check(context); + + REQUIRE(!format.result.final_passed); + REQUIRE(format.result.passes.size() == 1); + REQUIRE(format.result.fails.size() == 1); + REQUIRE(format.result.passes.contains("format_pass.cpp")); + REQUIRE(format.result.fails.contains("format_fail.cpp")); + REQUIRE(format.result.failed_commands.size() == 1); + REQUIRE(contains_text(format.result.failed_commands.front(), "format_fail.cpp")); +} + +TEST_CASE("Fake clang-format covers command errors and malformed XML", + "[cpp-lint-action][tool][clang_format][fake]") { + auto fake_dir = temporary_dir{"cpp_lint_action_fake_format_errors"}; + auto binary = write_fake_clang_format(fake_dir.path()); + + auto option = clang_format::option_t{}; + option.enabled = true; + option.binary = binary.string(); + option.version = "22.0.1"; + + SECTION("non-zero exit code is reported as a failed file") { + auto repo = repo_t{}; + repo.add_file("format_error.cpp", "int value = 0;\n"); + auto target = commit_changes(repo); + repo.rewrite_file("format_error.cpp", "int value = 1;\n"); + auto source = commit_changes(repo); + + auto context = make_context(repo, target, source); + auto format = clang_format::clang_format_general{option}; + format.check(context); + + REQUIRE(!format.result.final_passed); + REQUIRE(format.result.fails.contains("format_error.cpp")); + REQUIRE(contains_text(format.result.fails.at("format_error.cpp").tool_stderr, + "fake clang-format error")); + } + + SECTION("malformed replacement XML throws a parsing error") { + auto repo = repo_t{}; + repo.add_file("format_malformed.cpp", "int value = 0;\n"); + auto target = commit_changes(repo); + repo.rewrite_file("format_malformed.cpp", "int value = 1;\n"); + auto source = commit_changes(repo); + + auto context = make_context(repo, target, source); + auto format = clang_format::clang_format_general{option}; + REQUIRE_THROWS(format.check(context)); + } +} + +TEST_CASE("Fake clang-tidy keeps tidy checks available without a local clang install", + "[cpp-lint-action][tool][clang_tidy][fake]") { + auto fake_dir = temporary_dir{"cpp_lint_action_fake_tidy"}; + auto binary = write_fake_clang_tidy(fake_dir.path()); + + auto repo = repo_t{}; + repo.add_file("tidy_pass.cpp", "int value = 0;\n"); + repo.add_file("tidy_fail.cpp", "int value = 0;\n"); + auto target = commit_changes(repo); + repo.rewrite_file("tidy_pass.cpp", "int value = 1;\n"); + repo.rewrite_file("tidy_fail.cpp", "int value = 2;\n"); + auto source = commit_changes(repo); + + auto context = make_context(repo, target, source); + + auto option = clang_tidy::option_t{}; + option.enabled = true; + option.binary = binary.string(); + option.version = "22.0.1"; + + auto tidy = clang_tidy::clang_tidy_general{option}; + tidy.check(context); + + REQUIRE(!tidy.result.final_passed); + REQUIRE(tidy.result.passes.size() == 1); + REQUIRE(tidy.result.fails.size() == 1); + REQUIRE(tidy.result.passes.contains("tidy_pass.cpp")); + REQUIRE(tidy.result.fails.contains("tidy_fail.cpp")); + REQUIRE(tidy.result.fails.at("tidy_fail.cpp").diags.size() == 1); + REQUIRE(tidy.result.fails.at("tidy_fail.cpp").diags.front().header.serverity == "warning"); + REQUIRE(tidy.result.fails.at("tidy_fail.cpp").diags.front().header.checks + == "cppcoreguidelines-avoid-non-const-global-variables"); +} + +TEST_CASE("Fake clang-tidy covers command errors and malformed diagnostics", + "[cpp-lint-action][tool][clang_tidy][fake]") { + auto fake_dir = temporary_dir{"cpp_lint_action_fake_tidy_errors"}; + auto binary = write_fake_clang_tidy(fake_dir.path()); + + auto option = clang_tidy::option_t{}; + option.enabled = true; + option.binary = binary.string(); + option.version = "22.0.1"; + + SECTION("non-zero exit code is reported as a failed file") { + auto repo = repo_t{}; + repo.add_file("tidy_error.cpp", "int value = 0;\n"); + auto target = commit_changes(repo); + repo.rewrite_file("tidy_error.cpp", "int value = 1;\n"); + auto source = commit_changes(repo); + + auto context = make_context(repo, target, source); + auto tidy = clang_tidy::clang_tidy_general{option}; + tidy.check(context); + + REQUIRE(!tidy.result.final_passed); + REQUIRE(tidy.result.fails.contains("tidy_error.cpp")); + REQUIRE(contains_text(tidy.result.fails.at("tidy_error.cpp").tool_stderr, + "fake clang-tidy error")); + } + + SECTION("malformed diagnostics still produce a failed file with raw stdout") { + auto repo = repo_t{}; + repo.add_file("tidy_malformed.cpp", "int value = 0;\n"); + auto target = commit_changes(repo); + repo.rewrite_file("tidy_malformed.cpp", "int value = 1;\n"); + auto source = commit_changes(repo); + + auto context = make_context(repo, target, source); + auto tidy = clang_tidy::clang_tidy_general{option}; + tidy.check(context); + + REQUIRE(!tidy.result.final_passed); + REQUIRE(tidy.result.fails.contains("tidy_malformed.cpp")); + REQUIRE(tidy.result.fails.at("tidy_malformed.cpp").diags.empty()); + REQUIRE(contains_text(tidy.result.fails.at("tidy_malformed.cpp").tool_stdout, + "not a clang-tidy diagnostic")); + } +} + +TEST_CASE("Tool creators accept explicit fake binaries and reject ambiguous tool selection", + "[cpp-lint-action][program_options][tool][fake]") { + auto fake_dir = temporary_dir{"cpp_lint_action_fake_creator"}; + auto format_binary = write_fake_clang_format(fake_dir.path()); + auto tidy_binary = write_fake_clang_tidy(fake_dir.path()); + + auto format_creator = clang_format::creator{}; + auto tidy_creator = clang_tidy::creator{}; + auto desc = make_tool_desc(format_creator, tidy_creator); + + SECTION("explicit binaries are enough to create enabled tools") { + auto vars = parse_opt(desc, + "--target-revision=main", + ("--clang-format-binary=" + format_binary.string()).c_str(), + ("--clang-tidy-binary=" + tidy_binary.string()).c_str()); + + auto creators = std::vector{}; + creators.emplace_back(std::make_unique()); + creators.emplace_back(std::make_unique()); + + auto tools = create_enabled_tools(creators, vars); + REQUIRE(tools.size() == 2); + REQUIRE(tools[0]->version() == "22.0.1"); + REQUIRE(tools[1]->version() == "22.0.1"); + } + + SECTION("specifying both binary and version is rejected") { + auto vars = parse_opt(desc, + "--target-revision=main", + "--clang-format-version=22", + ("--clang-format-binary=" + format_binary.string()).c_str(), + ("--clang-tidy-binary=" + tidy_binary.string()).c_str()); + REQUIRE_THROWS(format_creator.create_tool(vars)); + } +} + +TEST_CASE("End-to-end binary smoke test runs tools, writes outputs, and controls exit code", + "[cpp-lint-action][e2e][fake]") { + auto fake_dir = temporary_dir{"cpp_lint_action_e2e"}; + auto format_binary = write_fake_clang_format(fake_dir.path()); + auto tidy_binary = write_fake_clang_tidy(fake_dir.path()); + + auto repo = repo_t{}; + repo.add_file("format_fail.cpp", "int value = 0;\n"); + repo.add_file("tidy_fail.cpp", "int value = 0;\n"); + auto target = commit_changes(repo); + repo.rewrite_file("format_fail.cpp", "int value = 1;\n"); + repo.rewrite_file("tidy_fail.cpp", "int value = 2;\n"); + auto source = commit_changes(repo); + + auto run_action = [&](bool disable_errors) { + auto run_dir = temporary_dir{"cpp_lint_action_e2e_run"}; + auto output_file = run_dir.path() / "github-output.txt"; + auto summary_file = run_dir.path() / "github-summary.md"; + auto log_file = run_dir.path() / "fake-tools.log"; + write_text_file(output_file, ""); + write_text_file(summary_file, ""); + write_text_file(log_file, ""); + + auto args = std::vector{ + "--log-level=error", + "--target-revision=" + target, + "--enable-comment-on-issue=false", + "--enable-pull-request-review=false", + "--enable-step-summary=true", + "--enable-action-output=true", + std::string{"--disable-errors="} + (disable_errors ? "true" : "false"), + "--jobs=2", + "--max-review-comments=25", + "--enable-clang-format=true", + "--enable-clang-format-fastly-exit=false", + "--clang-format-binary=" + format_binary.string(), + "--enable-clang-tidy=true", + "--enable-clang-tidy-fastly-exit=false", + "--clang-tidy-binary=" + tidy_binary.string(), + "--clang-tidy-allow-no-checks=true", + "--clang-tidy-enable-check-profile=false", + }; + + auto env = shell::envrionment{ + {"PATH", get_env_value("PATH")}, + {"DEBUG_CPP_LINT_ACTION", "1"}, + {"DEBUG_GITHUB_REPOSITORY", "owner/repo"}, + {"DEBUG_GITHUB_TOKEN", "github-token"}, + {"DEBUG_GITHUB_EVENT_NAME", github::github_event_push}, + {"DEBUG_GITHUB_BASE_REF", "main"}, + {"DEBUG_GITHUB_HEAD_REF", "feature"}, + {"DEBUG_GITHUB_REF", "refs/heads/feature"}, + {"DEBUG_GITHUB_REF_TYPE", "branch"}, + {"DEBUG_GITHUB_SHA", source}, + {"DEBUG_GITHUB_WORKSPACE", repo.get_path().string()}, + {github::github_output, output_file.string()}, + {github::github_step_summary, summary_file.string()}, + {"FAKE_CLANG_LOG", log_file.string()}, + }; + + auto result = + shell::execute(CPP_LINT_ACTION_BINARY, args, env, repo.get_path().string()); + return std::tuple{ + result, + read_text_file(output_file), + read_text_file(summary_file), + read_text_file(log_file), + }; + }; + + auto [failed_run, failed_output, failed_summary, failed_log] = run_action(false); + CAPTURE(failed_run.exit_code); + CAPTURE(failed_run.std_out); + CAPTURE(failed_run.std_err); + REQUIRE(failed_run.exit_code == 1); + REQUIRE(contains_text(failed_output, "clang_format_failed_number=1")); + REQUIRE(contains_text(failed_output, "clang_tidy_failed_number=1")); + REQUIRE(contains_text(failed_summary, "Analysis Report Generated")); + REQUIRE(contains_text(failed_summary, "format_fail.cpp")); + REQUIRE(contains_text(failed_summary, "tidy_fail.cpp")); + REQUIRE(contains_text(failed_summary, "Steps to reproduce")); + REQUIRE(contains_text(failed_log, "clang-format-fake")); + REQUIRE(contains_text(failed_log, "clang-tidy-fake")); + REQUIRE(contains_text(failed_log, "format_fail.cpp")); + REQUIRE(contains_text(failed_log, "tidy_fail.cpp")); + + auto [disabled_run, disabled_output, disabled_summary, disabled_log] = run_action(true); + REQUIRE(disabled_run.exit_code == 0); + REQUIRE(contains_text(disabled_output, "clang_format_failed_number=1")); + REQUIRE(contains_text(disabled_output, "clang_tidy_failed_number=1")); +}