toxcore/other/analysis/run-clang-tidy

#!/usr/bin/env bash

CHECKS="*"
ERRORS="*"

# Need to investigate or disable and document these.
# =========================================================

# TODO(iphydf): Fix these.
ERRORS="$ERRORS,-cert-err34-c"
ERRORS="$ERRORS,-readability-suspicious-call-argument"

# TODO(iphydf): Fix once cimple 0.0.19 is released.
CHECKS="$CHECKS,-google-readability-casting"

# TODO(iphydf): Fix these.
CHECKS="$CHECKS,-bugprone-switch-missing-default-case"

# TODO(iphydf): We might want some of these. For the ones we don't want, add a
# comment explaining why not.
CHECKS="$CHECKS,-clang-analyzer-optin.performance.Padding"
CHECKS="$CHECKS,-hicpp-signed-bitwise"

# TODO(iphydf): Maybe fix these?
CHECKS="$CHECKS,-bugprone-implicit-widening-of-multiplication-result"
CHECKS="$CHECKS,-bugprone-integer-division"
CHECKS="$CHECKS,-misc-no-recursion"

# TODO(iphydf): Only happens in bootstrap_daemon. Fix it.
CHECKS="$CHECKS,-cppcoreguidelines-avoid-non-const-global-variables"

# TODO(iphydf): Probably fix these.
CHECKS="$CHECKS,-cert-err33-c"
CHECKS="$CHECKS,-cppcoreguidelines-avoid-magic-numbers"
CHECKS="$CHECKS,-readability-magic-numbers"

# TODO(iphydf): We're using a lot of macros for constants. Should we convert
# all of them to enum?
CHECKS="$CHECKS,-modernize-macro-to-enum"

# Documented disabled checks. We don't want these for sure.
# =========================================================

# https://stackoverflow.com/questions/58672959/why-does-clang-tidy-say-vsnprintf-has-an-uninitialized-va-list-argument
CHECKS="$CHECKS,-clang-analyzer-valist.Uninitialized"

# We pass a lot of ints around, so many function parameters are some kind of
# int type that can be converted from another int type. We won't be getting
# away from that anytime soon.
CHECKS="$CHECKS,-bugprone-easily-swappable-parameters"

# Callback handlers often don't use all their parameters. There's
# IgnoreVirtual, but that doesn't work for C-style callbacks.
CHECKS="$CHECKS,-misc-unused-parameters"

# We sometimes use #if 0.
CHECKS="$CHECKS,-readability-avoid-unconditional-preprocessor-if"

# We have better macro hygiene checks with tokstyle. We can never pass macro
# arguments that require parentheses inside the macro.
CHECKS="$CHECKS,-bugprone-macro-parentheses"

# We don't use memcpy_s.
CHECKS="$CHECKS,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling"

# __attribute__((nonnull)) causes this warning on defensive null checks.
CHECKS="$CHECKS,-clang-diagnostic-pointer-bool-conversion"
CHECKS="$CHECKS,-clang-diagnostic-tautological-pointer-compare"

# Conflicts with "Variable is assigned a value that is never used."
# [unreadVariable]
CHECKS="$CHECKS,-cppcoreguidelines-init-variables"

# Short variable names are used quite a lot, and we don't consider them a
# readability issue.
CHECKS="$CHECKS,-readability-identifier-length"

# Altera checks are for GPUs (OpenCL). Our code doesn't run on GPUs.
CHECKS="$CHECKS,-altera-id-dependent-backward-branch"
CHECKS="$CHECKS,-altera-struct-pack-align"
CHECKS="$CHECKS,-altera-unroll-loops"

# We target systems other than Android, so we don't want to use non-standard
# functions from the Android libc.
CHECKS="$CHECKS,-android-cloexec-accept"
CHECKS="$CHECKS,-android-cloexec-fopen"

# This catches all the feature test macros (_POSIX_SOURCE etc.).
CHECKS="$CHECKS,-bugprone-reserved-identifier"
CHECKS="$CHECKS,-cert-dcl37-c"
CHECKS="$CHECKS,-cert-dcl51-cpp"

# Too restrictive. This makes sense if the branch clone is very large, but not
# if it's a single line. It can make the code less readable.
CHECKS="$CHECKS,-bugprone-branch-clone"

# We intentionally send some not null-terminated strings in tests and use it for
# the toxencryptsave magic number.
CHECKS="$CHECKS,-bugprone-not-null-terminated-result"

# We don't want default labels in enum switches.
CHECKS="$CHECKS,-hicpp-multiway-paths-covered"

# This can make readability quite a bit worse when the 2 cases look very
# similar.
CHECKS="$CHECKS,-llvm-else-after-return"
CHECKS="$CHECKS,-readability-else-after-return"

# We need 'return;' in empty functions because cimple won't allow empty
# functions otherwise.
CHECKS="$CHECKS,-readability-redundant-control-flow"

# These are incredibly annoying, because things like
#   uint16_t a = 0, b = 1, c = a > b ? a : b;
#                                      ^
# Trip the checker, which is true, because of integer promotion, but also not
# very helpful as a diagnostic.
CHECKS="$CHECKS,-bugprone-narrowing-conversions"
CHECKS="$CHECKS,-cppcoreguidelines-narrowing-conversions"

# Mistakenly thinks that
#   const int a = 0, b = 1;
#   assert(a < b);
# is a constant expression in C (it is in C++ though, which is probably why it's
# mistaken), suggesting to replace 'assert()' with 'static_assert()' in cases
# where that won't work.
#
# There are ways to make 'static_assert()' work, but they are rather annoying --
# they are somewhat ugly, hurting the readability, and some are error-prone:
#
# - Turning 'a' and 'b' into enum constants would make it work, but this falls
#   apart if the enum types are compared against non-enums down the line
#     error: enumerated and non-enumerated type in conditional expression [-Werror=extra]
#
# - Turning 'a' and 'b' into pre-processor macros is the only option left, but
#   #defines and #undefs in the middle of a function hurt the readability and
#   are less idiomatic than simply using 'const int'.
CHECKS="$CHECKS,-cert-dcl03-c"
CHECKS="$CHECKS,-hicpp-static-assert"
CHECKS="$CHECKS,-misc-static-assert"

# Doesn't consider use of preprocessor macros as needing a header, breaking
# struct definitions that depend on size macros from e.g. crypto_core.h.
CHECKS="$CHECKS,-misc-include-cleaner"

set -eux

# TODO(iphydf): Add toxav.
DIRS=(
  other/bootstrap_daemon/src
  other
  toxcore
  toxcore/events
  toxencryptsave
)

copy_files() {
  find "${DIRS[@]}" \
    -maxdepth 1 -type d -exec mkdir -p "$1/{}" \;
  find "${DIRS[@]}" \
    -maxdepth 1 -name "*.[ch]" -exec cp "{}" "$1/{}" \;
}

run() {
  echo "Running clang-tidy in variant '$*'"
  EXTRA_ARGS=("$@")
  for i in "${!EXTRA_ARGS[@]}"; do
    EXTRA_ARGS[$i]="--extra-arg=${EXTRA_ARGS[$i]}"
  done
  ls .clang-tidy
  copy_files a
  if ! find "${DIRS[@]}" \
    -maxdepth 1 -name "*.[ch]" -print0 \
    | xargs -0 -n15 -P"$(nproc)" clang-tidy \
      -p="$PWD/_build" \
      --extra-arg=-DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE \
      "${EXTRA_ARGS[@]}" \
      --fix \
      --checks="$CHECKS" \
      --warnings-as-errors="$ERRORS" \
      --use-color; then
    copy_files b
    colordiff -ru a b
    rm -rf a b
    false
  fi
  rm -rf a
}

cmake . -B_build -GNinja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON

. other/analysis/variants.sh