From d884cc9722fb011b89e303197677d0ff96327e01 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Sat, 5 Oct 2024 03:27:25 -0400 Subject: [PATCH] Add tasks to run C++ static analysis using clang-tidy. --- README.md | 16 ++++++++++ Taskfile.yml | 21 ++++++++----- lint-requirements.txt | 1 + lint-tasks.yml | 68 ++++++++++++++++++++++++++++++++++++------- 4 files changed, 87 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index f1d91d34..154a595c 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,22 @@ To run all linting checks AND automatically fix any fixable issues: task lint:fix ``` +### Running specific linters +The commands above run all linting checks, but for performance you may want to run a subset (e.g., +if you only changed C+ files, you don't need to run the YAML linting checks) using one of the tasks +in the table below. + +| Task | Description | +|-------------------------|----------------------------------------------------------| +| `lint:cpp-check` | Runs the C++ linters (formatters and static analyzers). | +| `lint:cpp-fix` | Runs the C++ linters and fixes some violations. | +| `lint:cpp-format-check` | Runs the C++ formatters. | +| `lint:cpp-format-fix` | Runs the C++ formatters and fixes some violations. | +| `lint:cpp-static-check` | Runs the C++ static analyzers. | +| `lint:cpp-static-fix` | Runs the C++ static analyzers and fixes some violations. | +| `lint:yml-check` | Runs the YAML linters. | +| `lint:yml-fix` | Runs the YAML linters and fixes some violations. | + [bug-report]: https://github.com/y-scope/clp-ffi-js/issues/new?labels=bug&template=bug-report.yml [CLP]: https://github.com/y-scope/clp [emscripten]: https://emscripten.org diff --git a/Taskfile.yml b/Taskfile.yml index 240f33da..7dc9e2fa 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -48,14 +48,7 @@ tasks: CHECKSUM_FILE: "{{.CHECKSUM_FILE}}" DATA_DIR: "{{.OUTPUT_DIR}}" cmds: - - "mkdir -p '{{.OUTPUT_DIR}}'" - - |- - cmake \ - -G "Unix Makefiles" \ - -DCMAKE_TOOLCHAIN_FILE="{{.G_EMSDK_DIR}}/upstream/emscripten/cmake/Modules/Platform/\ - Emscripten.cmake" \ - -S "{{.ROOT_DIR}}" \ - -B "{{.OUTPUT_DIR}}" + - task: "config-cmake-project" - "cmake --build '{{.OUTPUT_DIR}}' --parallel --target '{{.G_CLP_FFI_JS_TARGET_NAME}}'" # This command must be last - task: "utils:compute-checksum" @@ -63,6 +56,17 @@ tasks: DATA_DIR: "{{.OUTPUT_DIR}}" OUTPUT_FILE: "{{.CHECKSUM_FILE}}" + config-cmake-project: + internal: true + deps: ["emsdk"] + cmd: |- + cmake \ + -G "Unix Makefiles" \ + -DCMAKE_TOOLCHAIN_FILE="{{.G_EMSDK_DIR}}/upstream/emscripten/cmake/Modules/Platform/\ + Emscripten.cmake" \ + -S "{{.ROOT_DIR}}" \ + -B "{{.G_CLP_FFI_JS_BUILD_DIR}}" + emsdk: vars: CHECKSUM_FILE: "{{.G_EMSDK_CHECKSUM}}" @@ -71,6 +75,7 @@ tasks: OUTPUT_DIR: "{{.G_EMSDK_DIR}}" sources: ["{{.TASKFILE}}"] generates: ["{{.CHECKSUM_FILE}}"] + run: "once" deps: - "init" - task: "utils:validate-checksum" diff --git a/lint-requirements.txt b/lint-requirements.txt index 0d86af9a..fd01caec 100644 --- a/lint-requirements.txt +++ b/lint-requirements.txt @@ -1,3 +1,4 @@ # Lock to v18.x until we can upgrade our code to meet v19's formatting standards. clang-format~=18.1 +clang-tidy>=19.1.0 yamllint>=1.35.1 diff --git a/lint-tasks.yml b/lint-tasks.yml index c474b9f1..196eadb1 100644 --- a/lint-tasks.yml +++ b/lint-tasks.yml @@ -16,10 +16,21 @@ tasks: - task: "yml-fix" cpp-configs: + run: "once" cmd: "{{.ROOT_DIR}}/tools/yscope-dev-utils/lint-configs/symlink-cpp-lint-configs.sh" cpp-check: - sources: &cpp_source_files + cmds: + - task: "cpp-format-check" + - task: "cpp-static-check" + + cpp-fix: + cmds: + - task: "cpp-format-fix" + - task: "cpp-static-fix" + + cpp-format-check: + sources: &cpp_format_source_files - "{{.G_SRC_CLP_FFI_JS_DIR}}/.clang-format" - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.cpp" - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.h" @@ -31,13 +42,29 @@ tasks: vars: FLAGS: "--dry-run" - cpp-fix: - sources: *cpp_source_files + cpp-format-fix: + sources: *cpp_format_source_files cmds: - task: "clang-format" vars: FLAGS: "-i" + cpp-static-check: + # Alias task to `cpp-static-fix` since we don't currently support automatic fixes. + # NOTE: clang-tidy does have the ability to fix some errors, but the fixes can be inaccurate. + # When we eventually determine which errors can be safely fixed, we'll allow clang-tidy to + # fix them. + aliases: ["cpp-static-fix"] + sources: + - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.cpp" + - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.h" + - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.hpp" + - "{{.TASKFILE}}" + - "CMakeLists.txt" + - "tools/yscope-dev-utils/lint-configs/.clang-tidy" + cmds: + - task: "clang-tidy" + yml: aliases: - "yml-check" @@ -58,14 +85,32 @@ tasks: requires: vars: ["FLAGS"] deps: ["cpp-configs", "venv"] - cmds: - - |- - . "{{.G_LINT_VENV_DIR}}/bin/activate" - find "{{.G_SRC_CLP_FFI_JS_DIR}}" \ - -type f \ - \( -iname "*.cpp" -o -iname "*.h" -o -iname "*.hpp" \) \ - -print0 | \ - xargs -0 clang-format {{.FLAGS}} -Werror + cmd: |- + . "{{.G_LINT_VENV_DIR}}/bin/activate" + find "{{.G_SRC_CLP_FFI_JS_DIR}}" \ + -type f \ + \( -iname "*.cpp" -o -iname "*.h" -o -iname "*.hpp" \) \ + -print0 | \ + xargs -0 clang-format {{.FLAGS}} -Werror + + clang-tidy: + internal: true + vars: + FLAGS: >- + -p {{.G_CLP_FFI_JS_BUILD_DIR}}/compile_commands.json + --config-file=.clang-tidy + deps: [":config-cmake-project", "cpp-configs", "venv"] + cmd: |- + # Pass emscripten's cflags to clang-tidy by prefixing each one with `--extra-arg` + EXTRA_ARGS=$("{{.G_EMSDK_DIR}}/upstream/emscripten/em++" --cflags \ + | tr ' ' '\n' \ + | sed 's/^/--extra-arg /' \ + | tr '\n' ' ') + find "{{.G_SRC_CLP_FFI_JS_DIR}}" \ + -type f \ + \( -iname "*.cpp" -o -iname "*.h" -o -iname "*.hpp" \) \ + -print0 | \ + xargs -0 clang-tidy {{.FLAGS}} $EXTRA_ARGS venv: internal: true @@ -77,6 +122,7 @@ tasks: - "{{.TASKFILE}}" - "lint-requirements.txt" generates: ["{{.CHECKSUM_FILE}}"] + run: "once" deps: - ":init" - task: ":utils:validate-checksum"