[Lint] Improve clang-tidy and clang-format scripts running in lint

.clang-tidy

@@ -9,9 +9,9 @@
 # -modernize-pass-by-value (too restrictive)
 # -modernize-return-braced-init-list (inconsistent style)
 # -modernize-use-emplace (more subtle behavior)
+# -modernize-use-nodiscard (too much noise)
 # -modernize-use-trailing-return-type (inconsistent style)
-# -modernize-use-trailing-return-type (inconsistent style)
-# -readability-convert-member-functions-to-static (potentially too restrictive)
+# Other readability-* rules (potentially too noisy, inconsistent style)
 # Other rules not mentioned here or below (not yet evaluated)
 #
 # TODO: enable google-* and readability-* families of checks.
@@ -30,6 +30,7 @@ Checks: >
   -modernize-pass-by-value,
   -modernize-return-braced-init-list,
   -modernize-use-emplace,
+  -modernize-use-nodiscard,
   -modernize-use-trailing-return-type,
   performance-*,
   readability-avoid-const-params-in-decls,
@@ -38,6 +39,16 @@ Checks: >
   readability-container-size-empty,
   readability-delete-null-pointer,
   readability-else-after-return,
+  readability-implicit-bool-conversion,
+  readability-make-member-function-const,
+  readability-misleading-indentation,
+  readability-misplaced-array-index,
+  readability-named-parameter,
+  readability-non-const-parameter,
+  readability-redundant-*,
+  readability-static-definition-in-anonymous-namespace,
+  readability-string-compare,
+  readability-suspicious-call-argument,
 
 CheckOptions:
   # Reduce noisiness of the bugprone-narrowing-conversions check.

.gitignore

@@ -196,6 +196,9 @@ tags.temp
 # tools
 tools/prometheus*
 
+# Generated for clang-tidy
+compile_commands.json
+
 # ray project files
 project-id
 .mypy_cache/

ci/travis/check-git-clang-format-output.sh

@@ -1,22 +1,30 @@
 #!/bin/bash
 
+printWarning() {
+    printf '\033[31mLINT WARNING (clang-format):\033[0m %s\n' "$@"
+}
+
+printInfo() {
+    printf '\033[34mLINT (clang-format):\033[0m %s\n' "$@"
+}
+
 # Compare against the master branch, because most development is done against it.
 base_commit="$(git merge-base HEAD master)"
-if [ "$base_commit" = "$(git rev-parse HEAD)" ]; then
+if [ "$base_commit" = "$(git rev-parse HEAD)" ] && [ "$(git status --porcelain | wc -l)" -eq 0 ]; then
   # Prefix of master branch, so compare against parent commit
   base_commit="$(git rev-parse HEAD^)"
-  echo "Running clang-format against parent commit $base_commit"
+  printInfo "Running clang-format against parent commit $base_commit"
 else
-  echo "Running clang-format against parent commit $base_commit from master branch"
+  printInfo "Running clang-format against commit $base_commit from master branch"
 fi
 
 exclude_regex="(.*thirdparty/|.*redismodule.h|.*.java|.*.jsx?|.*.tsx?)"
 output="$(ci/travis/git-clang-format --commit "$base_commit" --diff --exclude "$exclude_regex")"
 if [ "$output" = "no modified files to format" ] || [ "$output" = "clang-format did not modify any files" ] ; then
-  echo "clang-format passed."
+  printInfo "clang-format passed."
   exit 0
 else
-  echo "clang-format failed:"
-  echo "$output"
+  printWarning "clang-format failed:"
+  printWarning "$output"
   exit 1
 fi

ci/travis/check-git-clang-tidy-output.sh

@@ -1,47 +1,49 @@
 #!/bin/bash
 
-# TODO: integrate this script into pull request workflow.
-
-printError() {
-    printf '\033[31mERROR:\033[0m %s\n' "$@"
+printWarning() {
+    printf '\033[31mLINT WARNING (clang-tidy):\033[0m %s\n' "$@"
 }
 
 printInfo() {
-    printf '\033[32mINFO:\033[0m %s\n' "$@"
+    printf '\033[34mLINT (clang-tidy):\033[0m %s\n' "$@"
 }
 
 log_err() {
-    printError "Setting up clang-tidy encountered an error"
+    printWarning "Setting up clang-tidy encountered an error"
 }
 
 set -eo pipefail
 
 trap '[ $? -eq 0 ] || log_err' EXIT
 
-printInfo "Fetching workspace info ..."
-
-WORKSPACE=$(bazel info workspace)
-BAZEL_ROOT=$(bazel info execution_root)
+# Compare against the master branch, because most development is done against it.
+base_commit="$(git merge-base HEAD master)"
+if [ "$base_commit" = "$(git rev-parse HEAD)" ] && [ "$(git status --porcelain | wc -l)" -eq 0 ]; then
+  # Prefix of master branch, so compare against parent commit
+  base_commit="$(git rev-parse HEAD^)"
+  printInfo "Running clang-tidy against parent commit $base_commit"
+else
+  printInfo "Running clang-tidy against commit $base_commit from master branch"
+fi
 
-printInfo "Generating compilation database ..."
+WORKSPACE=$(bazel info workspace 2>/dev/null)
+BAZEL_ROOT=$(bazel info execution_root 2>/dev/null)
 
 case "${OSTYPE}" in
   linux*)
-    printInfo "Running on Linux, using clang to build C++ targets. Please make sure it is installed with install-llvm-binaries.sh"
+    printInfo "Generating compile commands with clang (on Linux) ..."
     bazel build //ci/generate_compile_commands:extract_compile_command //:ray_pkg --config=llvm \
         --experimental_action_listener=//ci/generate_compile_commands:compile_command_listener;;
   darwin*)
-    printInfo "Running on MacOS, assuming default C++ compiler is clang."
+    printInfo "Generating compile commands with clang (on MacOS) ..."
     bazel build //ci/generate_compile_commands:extract_compile_command //:ray_pkg \
         --experimental_action_listener=//ci/generate_compile_commands:compile_command_listener;;
   msys*)
-    printInfo "Running on Windows, using clang-cl to build C++ targets. Please make sure it is installed."
+    printInfo "Generating compile commands with clang (on Windows) ..."
     CC=clang-cl bazel build //ci/generate_compile_commands:extract_compile_command //:ray_pkg \
         --experimental_action_listener=//ci/generate_compile_commands:compile_command_listener;;
 esac
 
-printInfo "Assembling compilation database ..."
-
 TMPFILE=$(mktemp)
 printf '[\n' >"$TMPFILE"
 find "$BAZEL_ROOT" -name '*.compile_command.json' -exec cat {} + >>"$TMPFILE"
@@ -58,35 +60,21 @@ fi
 OUTFILE=$WORKSPACE/compile_commands.json
 
 if hash jq 2>/dev/null; then
-    printInfo "Formatting compilation database ..."
     jq . "$TMPFILE" >"$OUTFILE"
 else
-    printInfo "Can not find jq. Skip formatting compilation database."
     cp --no-preserve=mode "$TMPFILE" "$OUTFILE"
 fi
 
-# Compare against the master branch, because most development is done against it.
-base_commit="$(git merge-base HEAD master)"
-if [ "$base_commit" = "$(git rev-parse HEAD)" ]; then
-  # Prefix of master branch, so compare against parent commit
-  base_commit="$(git rev-parse HEAD^)"
-  printInfo "Running clang-tidy against parent commit $base_commit"
-else
-  printInfo "Running clang-tidy against parent commit $base_commit from master branch"
-fi
-
 trap - EXIT
 
-if git diff -U0 "$base_commit" | ci/travis/clang-tidy-diff.py -p1 -fix; then
+printInfo "Running clang-tidy ..."
+output="$(git diff -U0 "$base_commit" | ci/travis/clang-tidy-diff.py -p1 -fix)"
+if [[ ! "$output" =~ "error: " ]]; then
   printInfo "clang-tidy passed."
 else
-  printError "clang-tidy failed. See above for details including suggested fixes."
-  printError
-  printError "If you think the warning is too aggressive, the proposed fix is incorrect or are unsure about how to"
-  printError "fix, feel free to raise the issue on the PR or Anyscale #learning-cplusplus Slack channel."
-  printError
-  printError "To run clang-tidy locally with fix suggestions, make sure clang and clang-tidy are installed and"
-  printError "available in PATH (version 12 is preferred). Then run"
-  printError "scripts/check-git-clang-tidy-output.sh"
-  printError "from repo root."
+  printWarning "clang-tidy issued warnings. See below for details. Suggested fixes have also been applied."
+  printWarning "$output"
+  printWarning "If a warning is too pedantic, a proposed fix is incorrect or you are unsure about how to fix a warning,"
+  printWarning "feel free to raise the issue on the pull request."
+  printWarning "clang-tidy warnings can also be suppressed with NOLINT"
 fi

ci/travis/clang-tidy-diff.py

@@ -251,7 +251,7 @@ def main():
     start_workers(max_task_count, run_tidy, task_queue, lock, args.timeout)
 
     # Form the common args list.
-    common_clang_tidy_args = []
+    common_clang_tidy_args = ["--use-color"]
     if args.fix:
         common_clang_tidy_args.append("-fix")
     if args.checks != "":