From 8d181d10ad13e754c354ab7fbdcafc1833e0a315 Mon Sep 17 00:00:00 2001
From: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
Date: Mon, 7 Oct 2024 03:44:32 -0400
Subject: [PATCH] Add tasks to run C++ static analysis using clang-tidy;
 Separate CMake project config into its own task. (#18)

---
 README.md             | 16 +++++++++
 Taskfile.yml          | 27 ++++++++++----
 lint-requirements.txt |  1 +
 lint-tasks.yml        | 82 ++++++++++++++++++++++++++++++++++++-------
 4 files changed, 106 insertions(+), 20 deletions(-)

diff --git a/README.md b/README.md
index f1d91d34..e122f0ba 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 59e263b2..13b600a5 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"
@@ -48,13 +49,7 @@ tasks:
           CHECKSUM_FILE: "{{.CHECKSUM_FILE}}"
           DATA_DIR: "{{.OUTPUT_DIR}}"
     cmds:
-      - "mkdir -p '{{.OUTPUT_DIR}}'"
-      - |-
-        cmake \
-        -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"
@@ -70,6 +65,7 @@ tasks:
       OUTPUT_DIR: "{{.G_EMSDK_DIR}}"
     sources: ["{{.TASKFILE}}"]
     generates: ["{{.CHECKSUM_FILE}}"]
+    run: "once"
     deps:
       - "init"
       - task: "utils:validate-checksum"
@@ -130,6 +126,23 @@ 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 \
+      -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
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..4ab8d33a 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:
@@ -19,25 +20,62 @@ tasks:
     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"
       - "{{.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:
           FLAGS: "--dry-run"
 
-  cpp-fix:
-    sources: *cpp_source_files
+  cpp-format-fix:
+    sources: *cpp_format_source_files
+    deps: ["cpp-configs", "venv"]
     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:
+      # 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}}"
+      - "tools/yscope-dev-utils/lint-configs/.clang-tidy"
+    deps: [":config-cmake-project", "cpp-configs", "venv"]
+    cmds:
+      - task: "clang-tidy"
+
   yml:
     aliases:
       - "yml-check"
@@ -57,15 +95,33 @@ tasks:
     internal: true
     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
+    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' \
+        | 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