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 1/6] 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" From 6a471a2de85bb7e2c9980b60ba5bc65f64bd3678 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Sat, 5 Oct 2024 07:09:23 -0400 Subject: [PATCH 2/6] Activate venv before calling clang-tidy. --- lint-tasks.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lint-tasks.yml b/lint-tasks.yml index 196eadb1..1b943e8b 100644 --- a/lint-tasks.yml +++ b/lint-tasks.yml @@ -101,6 +101,8 @@ tasks: --config-file=.clang-tidy deps: [":config-cmake-project", "cpp-configs", "venv"] cmd: |- + . "{{.G_LINT_VENV_DIR}}/bin/activate" + # 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' \ From 10e47644dac6f769ae7f64ec0a9ae7062f1933b4 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Sat, 5 Oct 2024 07:18:43 -0400 Subject: [PATCH 3/6] Fix typo. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 154a595c..e122f0ba 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ 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 +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 | From 4db65bc9a1bbadda9f8f10dde57f10ebd0f34d0b Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Sun, 6 Oct 2024 12:46:56 -0400 Subject: [PATCH 4/6] Move clang-format and clang-tidy dependencies into caller tasks. --- Taskfile.yml | 8 ++++++++ lint-tasks.yml | 18 ++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 7dc9e2fa..0f2296bc 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -36,6 +36,7 @@ tasks: CHECKSUM_FILE: "{{.G_CLP_FFI_JS_CHECKSUM}}" OUTPUT_DIR: "{{.G_CLP_FFI_JS_BUILD_DIR}}" sources: + - "{{.G_CLP_FFI_JS_BUILD_DIR}}/CMakeCache.txt" - "{{.G_EMSDK_CHECKSUM}}" - "{{.TASKFILE}}" - "CMakeLists.txt" @@ -58,6 +59,13 @@ tasks: config-cmake-project: internal: true + sources: + - "{{.G_EMSDK_CHECKSUM}}" + - "{{.TASKFILE}}" + - "CMakeLists.txt" + generates: + - "{{.G_CLP_FFI_JS_BUILD_DIR}}/CMakeCache.txt" + - "{{.G_CLP_FFI_JS_BUILD_DIR}}/compile_commands.json" deps: ["emsdk"] cmd: |- cmake \ diff --git a/lint-tasks.yml b/lint-tasks.yml index 1b943e8b..c0ae808d 100644 --- a/lint-tasks.yml +++ b/lint-tasks.yml @@ -1,7 +1,8 @@ version: "3" vars: - G_SRC_CLP_FFI_JS_DIR: "{{.ROOT_DIR}}/src/clp_ffi_js" + G_SRC_DIR: "{{.ROOT_DIR}}/src" + G_SRC_CLP_FFI_JS_DIR: "{{.G_SRC_DIR}}/clp_ffi_js" G_LINT_VENV_DIR: "{{.G_BUILD_DIR}}/lint-venv" tasks: @@ -37,6 +38,7 @@ tasks: - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.hpp" - "{{.TASKFILE}}" - "tools/yscope-dev-utils/lint-configs/.clang-format" + deps: ["cpp-configs", "venv"] cmds: - task: "clang-format" vars: @@ -44,6 +46,7 @@ tasks: cpp-format-fix: sources: *cpp_format_source_files + deps: ["cpp-configs", "venv"] cmds: - task: "clang-format" vars: @@ -56,12 +59,21 @@ tasks: # fix them. aliases: ["cpp-static-fix"] sources: + # Dependency sources + - "{{.G_CLP_FFI_JS_BUILD_DIR}}/_deps/*-src/**/*" + - "{{.G_SRC_DIR}}/submodules/**/*" + + # clp-ffi-js sources - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.cpp" - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.h" - "{{.G_SRC_CLP_FFI_JS_DIR}}/**/*.hpp" + + # Other sources + - "{{.G_CLP_FFI_JS_BUILD_DIR}}/compile_commands.json" + - "{{.ROOT_DIR}}/Taskfile.yml" - "{{.TASKFILE}}" - - "CMakeLists.txt" - "tools/yscope-dev-utils/lint-configs/.clang-tidy" + deps: [":config-cmake-project", "cpp-configs", "venv"] cmds: - task: "clang-tidy" @@ -84,7 +96,6 @@ tasks: internal: true requires: vars: ["FLAGS"] - deps: ["cpp-configs", "venv"] cmd: |- . "{{.G_LINT_VENV_DIR}}/bin/activate" find "{{.G_SRC_CLP_FFI_JS_DIR}}" \ @@ -99,7 +110,6 @@ tasks: FLAGS: >- -p {{.G_CLP_FFI_JS_BUILD_DIR}}/compile_commands.json --config-file=.clang-tidy - deps: [":config-cmake-project", "cpp-configs", "venv"] cmd: |- . "{{.G_LINT_VENV_DIR}}/bin/activate" From 0f76926fbde19d4177f79d3f257fbc813fe5de34 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Sun, 6 Oct 2024 12:58:39 -0400 Subject: [PATCH 5/6] Remove run: once where not strictly necessary. --- lint-tasks.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/lint-tasks.yml b/lint-tasks.yml index c0ae808d..4ab8d33a 100644 --- a/lint-tasks.yml +++ b/lint-tasks.yml @@ -17,7 +17,6 @@ tasks: - task: "yml-fix" cpp-configs: - run: "once" cmd: "{{.ROOT_DIR}}/tools/yscope-dev-utils/lint-configs/symlink-cpp-lint-configs.sh" cpp-check: @@ -134,7 +133,6 @@ tasks: - "{{.TASKFILE}}" - "lint-requirements.txt" generates: ["{{.CHECKSUM_FILE}}"] - run: "once" deps: - ":init" - task: ":utils:validate-checksum" From f9ed9acd0ad64d36f2e82f61612523bbbf524b5f Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Sun, 6 Oct 2024 12:59:21 -0400 Subject: [PATCH 6/6] Move config-cmake-project to internal tasks section. --- Taskfile.yml | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 0f2296bc..62cf55ee 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -57,24 +57,6 @@ tasks: DATA_DIR: "{{.OUTPUT_DIR}}" OUTPUT_FILE: "{{.CHECKSUM_FILE}}" - config-cmake-project: - internal: true - sources: - - "{{.G_EMSDK_CHECKSUM}}" - - "{{.TASKFILE}}" - - "CMakeLists.txt" - generates: - - "{{.G_CLP_FFI_JS_BUILD_DIR}}/CMakeCache.txt" - - "{{.G_CLP_FFI_JS_BUILD_DIR}}/compile_commands.json" - 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}}" @@ -144,6 +126,24 @@ tasks: DATA_DIR: "{{.OUTPUT_DIR}}" OUTPUT_FILE: "{{.CHECKSUM_FILE}}" + config-cmake-project: + internal: true + sources: + - "{{.G_EMSDK_CHECKSUM}}" + - "{{.TASKFILE}}" + - "CMakeLists.txt" + generates: + - "{{.G_CLP_FFI_JS_BUILD_DIR}}/CMakeCache.txt" + - "{{.G_CLP_FFI_JS_BUILD_DIR}}/compile_commands.json" + 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}}" + init: internal: true silent: true