Copilot commented on code in PR #62307: URL: https://github.com/apache/doris/pull/62307#discussion_r3061806745
########## build-support/run-clang-tidy.sh: ########## @@ -0,0 +1,338 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +############################################################## +# This script runs clang-tidy on newly added or modified C++ +# code, reporting only warnings on changed lines to avoid +# noise from the existing codebase. +# +# Usage: +# build-support/run-clang-tidy.sh [options] +# +# Options: +# --base <ref> Git ref to diff against (default: auto-detect) +# --files <f1> <f2> Check specific files (all lines, no filtering) +# --build-dir <dir> Path to build dir with compile_commands.json +# --fix Apply clang-tidy auto-fixes (only on changed lines) +# --full Report all warnings in changed files, not just +# warnings on changed lines +# -h, --help Show this help +############################################################## + +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +DORIS_HOME=$(cd "${ROOT}/.." && pwd) +export DORIS_HOME + +# Directories excluded from clang-tidy (third-party / generated code) +EXCLUDED_PATTERNS=( + "be/src/apache-orc/" + "be/src/clucene/" + "be/src/gutil/" + "be/src/glibc-compatibility/" + "be/src/util/mustache/" + "be/src/util/sse2neo.h" + "be/src/util/sse2neon.h" + "be/src/util/utf8_check.cpp" + "cloud/src/common/defer.h" + "contrib/" +) + +# C++ file extensions to check +CPP_EXTENSIONS="c|h|cc|cpp|cxx|hpp|hxx|hh" + +usage() { + echo "Usage: $(basename "$0") [options]" + echo "" + echo "Run clang-tidy on newly added/modified C++ code." + echo "By default, only reports warnings on changed lines." + echo "" + echo "Options:" + echo " --base <ref> Git ref to diff against (default: auto-detect)" + echo " --files <f1> ... Check specific files (all lines, no line filter)" + echo " --build-dir <dir> Path to build dir with compile_commands.json" + echo " --fix Apply clang-tidy auto-fixes (changed lines only)" + echo " --full Report all warnings in changed files" + echo " -h, --help Show this help" + exit 0 +} Review Comment: `usage()` always exits with code 0. That means argument errors (e.g. unknown option) will still return success, which can hide mis-invocations in automation. Consider making `usage` accept an exit code (default 0) and calling it with non-zero for error cases, or explicitly `exit 1` on unknown options. ########## build-support/run-clang-tidy.sh: ########## @@ -0,0 +1,338 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +############################################################## +# This script runs clang-tidy on newly added or modified C++ +# code, reporting only warnings on changed lines to avoid +# noise from the existing codebase. +# +# Usage: +# build-support/run-clang-tidy.sh [options] +# +# Options: +# --base <ref> Git ref to diff against (default: auto-detect) +# --files <f1> <f2> Check specific files (all lines, no filtering) +# --build-dir <dir> Path to build dir with compile_commands.json +# --fix Apply clang-tidy auto-fixes (only on changed lines) +# --full Report all warnings in changed files, not just +# warnings on changed lines +# -h, --help Show this help +############################################################## + +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +DORIS_HOME=$(cd "${ROOT}/.." && pwd) +export DORIS_HOME + +# Directories excluded from clang-tidy (third-party / generated code) +EXCLUDED_PATTERNS=( + "be/src/apache-orc/" + "be/src/clucene/" + "be/src/gutil/" + "be/src/glibc-compatibility/" + "be/src/util/mustache/" + "be/src/util/sse2neo.h" + "be/src/util/sse2neon.h" + "be/src/util/utf8_check.cpp" + "cloud/src/common/defer.h" + "contrib/" +) + +# C++ file extensions to check +CPP_EXTENSIONS="c|h|cc|cpp|cxx|hpp|hxx|hh" + +usage() { + echo "Usage: $(basename "$0") [options]" + echo "" + echo "Run clang-tidy on newly added/modified C++ code." + echo "By default, only reports warnings on changed lines." + echo "" + echo "Options:" + echo " --base <ref> Git ref to diff against (default: auto-detect)" + echo " --files <f1> ... Check specific files (all lines, no line filter)" + echo " --build-dir <dir> Path to build dir with compile_commands.json" + echo " --fix Apply clang-tidy auto-fixes (changed lines only)" + echo " --full Report all warnings in changed files" + echo " -h, --help Show this help" + exit 0 +} + +# Parse arguments +BASE_REF="" +SPECIFIC_FILES=() +BUILD_DIR="" +FIX_MODE="" +FULL_FILE_MODE=false + +while [[ $# -gt 0 ]]; do + case "$1" in + --base) + BASE_REF="$2" + shift 2 + ;; + --files) + shift + while [[ $# -gt 0 && ! "$1" =~ ^-- ]]; do + SPECIFIC_FILES+=("$1") + shift + done + ;; + --build-dir) + BUILD_DIR="$2" + shift 2 + ;; + --fix) + FIX_MODE="-fix" + shift + ;; + --full) + FULL_FILE_MODE=true + shift + ;; + -h|--help) + usage + ;; + *) + echo "Unknown option: $1" + usage + ;; + esac +done + +# Find clang-tidy +CLANG_TIDY="${CLANG_TIDY_BINARY:-$(command -v clang-tidy 2>/dev/null || true)}" +if [[ -z "${CLANG_TIDY}" ]]; then + echo "Error: clang-tidy not found. Please install clang-tidy or set CLANG_TIDY_BINARY." + exit 1 +fi + +echo "Using clang-tidy: ${CLANG_TIDY}" +echo " Version: $("${CLANG_TIDY}" --version 2>&1 | head -1)" + +# Find compile_commands.json +if [[ -z "${BUILD_DIR}" ]]; then + for candidate in \ + "${DORIS_HOME}/be/build_ASAN" \ + "${DORIS_HOME}/be/build_Release" \ + "${DORIS_HOME}/be/ut_build_ASAN" \ + "${DORIS_HOME}/be/ut_build_Release"; do + if [[ -f "${candidate}/compile_commands.json" ]]; then + BUILD_DIR="${candidate}" + break + fi + done +fi + +if [[ -z "${BUILD_DIR}" || ! -f "${BUILD_DIR}/compile_commands.json" ]]; then + echo "Error: compile_commands.json not found." + echo "Please build BE first: ./build.sh --be -j\${DORIS_PARALLELISM}" + echo "Or specify --build-dir <path>" + exit 1 +fi + +echo "Using compile_commands.json from: ${BUILD_DIR}" + +# Determine files to check +cd "${DORIS_HOME}" + +FILES_TO_CHECK=() + +if [[ ${#SPECIFIC_FILES[@]} -gt 0 ]]; then + FILES_TO_CHECK=("${SPECIFIC_FILES[@]}") +else + if [[ -n "${BASE_REF}" ]]; then + mapfile -t FILES_TO_CHECK < <(git diff --name-only --diff-filter=ACMR "${BASE_REF}" -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + else + mapfile -t STAGED < <(git diff --cached --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t UNSTAGED < <(git diff --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t FILES_TO_CHECK < <(printf '%s\n' "${STAGED[@]}" "${UNSTAGED[@]}" | sort -u | grep -v '^$') + fi +fi + +# Filter to C++ files only, excluding third-party code +FILTERED_FILES=() +for f in "${FILES_TO_CHECK[@]}"; do + if [[ -z "$f" ]]; then + continue + fi + if ! echo "$f" | grep -qE "\.(${CPP_EXTENSIONS})$"; then + continue + fi + excluded=false + for pattern in "${EXCLUDED_PATTERNS[@]}"; do + if [[ "$f" == *"${pattern}"* ]]; then + excluded=true + break + fi + done + if [[ "${excluded}" == "false" && -f "$f" ]]; then + FILTERED_FILES+=("$f") + fi +done + +if [[ ${#FILTERED_FILES[@]} -eq 0 ]]; then + echo "No changed C++ files to check." + exit 0 +fi + +echo "" +echo "Files to check (${#FILTERED_FILES[@]}):" +for f in "${FILTERED_FILES[@]}"; do + echo " ${f}" +done +echo "" + +# --------------------------------------------------------------- +# Build a map of changed line ranges per file from git diff. +# This allows us to filter clang-tidy output to only report +# warnings on lines the developer actually touched. +# --------------------------------------------------------------- +declare -A CHANGED_LINES_MAP + +build_changed_lines_map() { + local diff_output + if [[ ${#SPECIFIC_FILES[@]} -gt 0 ]]; then + # --files mode: no line filtering + return + fi + + if [[ -n "${BASE_REF}" ]]; then + diff_output=$(git diff -U0 "${BASE_REF}" -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + else + # Combine staged + unstaged diffs + local staged_diff unstaged_diff + staged_diff=$(git diff --cached -U0 -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + unstaged_diff=$(git diff -U0 -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + diff_output="${staged_diff}"$'\n'"${unstaged_diff}" + fi + + # Parse unified diff hunks to extract changed line ranges. + # Hunk headers: @@ -old_start[,old_count] +new_start[,new_count] @@ + # We collect new_start..new_start+new_count-1 as changed lines. + local current_file="" + while IFS= read -r line; do + if [[ "$line" =~ ^diff\ --git\ a/(.+)\ b/(.+)$ ]]; then + current_file="${BASH_REMATCH[2]}" + elif [[ "$line" =~ ^@@.*\+([0-9]+)(,([0-9]+))?.*@@ ]]; then + local start="${BASH_REMATCH[1]}" + local count="${BASH_REMATCH[3]:-1}" + if [[ "${count}" -eq 0 ]]; then + continue + fi + local end=$((start + count - 1)) + # Append range to the file's entry (space-separated "start:end" pairs) + CHANGED_LINES_MAP["${current_file}"]+="${start}:${end} " + fi + done <<< "${diff_output}" +} + +# Check if a line number falls within any changed range for a file +is_line_changed() { + local file="$1" + local line_num="$2" + local ranges="${CHANGED_LINES_MAP["${file}"]}" + + if [[ -z "${ranges}" ]]; then + # No range info (e.g., new file or --files mode) — treat all lines as changed + return 0 + fi Review Comment: `is_line_changed()` treats missing/empty range info as “all lines changed”. For diagnostics coming from headers or other included files that weren’t modified (and thus have no entry in `CHANGED_LINES_MAP`), this will incorrectly pass the filter and re-introduce noise. Consider treating unknown files as “not changed” by default, and/or using clang-tidy’s native `-line-filter` so diagnostics are only produced for the intended changed ranges. ########## .claude/skills/clang-tidy-check/SKILL.md: ########## @@ -0,0 +1,127 @@ +--- +name: clang-tidy-check +description: Run clang-tidy on newly added/modified BE C++ code +compatibility: opencode +--- + +## What I do + +Run clang-tidy static analysis on **only the changed lines** of newly added or modified C++ files in the BE and Cloud modules, using the project's `.clang-tidy` configuration. The script parses `git diff` to identify changed line ranges and filters clang-tidy output accordingly, avoiding noise from pre-existing code. + +## When to use me + +- After building BE, before committing C++ code changes +- When CI reports clang-tidy warnings on your PR +- When you want to proactively check new code for common bugs and style issues + +## Prerequisites + +1. **BE must be built first** — clang-tidy needs `compile_commands.json` generated during the CMake build. +2. **clang-tidy must be installed** — the project uses clang-tidy from the LDB toolchain. + Review Comment: This skill claims clang-tidy runs on BE *and Cloud* changes, but the prerequisites/instructions assume only a BE build/BE `compile_commands.json`. In practice Cloud files need a Cloud compilation database (`cloud/build_*` or `cloud/ut_build_*`), and a single `-p` directory usually can’t cover both BE and Cloud at once. Please clarify the workflow (e.g., run twice with different `--build-dir`) or update the script to auto-handle Cloud build dirs. ########## build-support/run-clang-tidy.sh: ########## @@ -0,0 +1,338 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +############################################################## +# This script runs clang-tidy on newly added or modified C++ +# code, reporting only warnings on changed lines to avoid +# noise from the existing codebase. +# +# Usage: +# build-support/run-clang-tidy.sh [options] +# +# Options: +# --base <ref> Git ref to diff against (default: auto-detect) +# --files <f1> <f2> Check specific files (all lines, no filtering) +# --build-dir <dir> Path to build dir with compile_commands.json +# --fix Apply clang-tidy auto-fixes (only on changed lines) +# --full Report all warnings in changed files, not just +# warnings on changed lines +# -h, --help Show this help +############################################################## + +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +DORIS_HOME=$(cd "${ROOT}/.." && pwd) +export DORIS_HOME + +# Directories excluded from clang-tidy (third-party / generated code) +EXCLUDED_PATTERNS=( + "be/src/apache-orc/" + "be/src/clucene/" + "be/src/gutil/" + "be/src/glibc-compatibility/" + "be/src/util/mustache/" + "be/src/util/sse2neo.h" + "be/src/util/sse2neon.h" + "be/src/util/utf8_check.cpp" + "cloud/src/common/defer.h" + "contrib/" +) + +# C++ file extensions to check +CPP_EXTENSIONS="c|h|cc|cpp|cxx|hpp|hxx|hh" + +usage() { + echo "Usage: $(basename "$0") [options]" + echo "" + echo "Run clang-tidy on newly added/modified C++ code." + echo "By default, only reports warnings on changed lines." + echo "" + echo "Options:" + echo " --base <ref> Git ref to diff against (default: auto-detect)" + echo " --files <f1> ... Check specific files (all lines, no line filter)" + echo " --build-dir <dir> Path to build dir with compile_commands.json" + echo " --fix Apply clang-tidy auto-fixes (changed lines only)" + echo " --full Report all warnings in changed files" + echo " -h, --help Show this help" + exit 0 +} + +# Parse arguments +BASE_REF="" +SPECIFIC_FILES=() +BUILD_DIR="" +FIX_MODE="" +FULL_FILE_MODE=false + +while [[ $# -gt 0 ]]; do + case "$1" in + --base) + BASE_REF="$2" + shift 2 + ;; + --files) + shift + while [[ $# -gt 0 && ! "$1" =~ ^-- ]]; do + SPECIFIC_FILES+=("$1") + shift + done + ;; + --build-dir) + BUILD_DIR="$2" + shift 2 + ;; + --fix) + FIX_MODE="-fix" + shift + ;; + --full) + FULL_FILE_MODE=true + shift + ;; + -h|--help) + usage + ;; + *) + echo "Unknown option: $1" + usage + ;; + esac +done + +# Find clang-tidy +CLANG_TIDY="${CLANG_TIDY_BINARY:-$(command -v clang-tidy 2>/dev/null || true)}" +if [[ -z "${CLANG_TIDY}" ]]; then + echo "Error: clang-tidy not found. Please install clang-tidy or set CLANG_TIDY_BINARY." + exit 1 +fi + +echo "Using clang-tidy: ${CLANG_TIDY}" +echo " Version: $("${CLANG_TIDY}" --version 2>&1 | head -1)" + +# Find compile_commands.json +if [[ -z "${BUILD_DIR}" ]]; then + for candidate in \ + "${DORIS_HOME}/be/build_ASAN" \ + "${DORIS_HOME}/be/build_Release" \ + "${DORIS_HOME}/be/ut_build_ASAN" \ + "${DORIS_HOME}/be/ut_build_Release"; do + if [[ -f "${candidate}/compile_commands.json" ]]; then + BUILD_DIR="${candidate}" + break + fi + done +fi Review Comment: Auto-detection of `compile_commands.json` only checks `be/build_*` / `be/ut_build_*`, but this script also targets `cloud/src` and `cloud/test`. A BE compilation database typically won’t contain Cloud compile commands, so clang-tidy may fail to analyze Cloud files unless the user manually passes `--build-dir` (and the error is currently easy to miss). Consider detecting Cloud build dirs too (e.g. `cloud/build_*`, `cloud/ut_build_*`) or splitting checks by module/build-dir. ########## .claude/skills/clang-tidy-check/SKILL.md: ########## @@ -0,0 +1,127 @@ +--- +name: clang-tidy-check +description: Run clang-tidy on newly added/modified BE C++ code +compatibility: opencode +--- + +## What I do + +Run clang-tidy static analysis on **only the changed lines** of newly added or modified C++ files in the BE and Cloud modules, using the project's `.clang-tidy` configuration. The script parses `git diff` to identify changed line ranges and filters clang-tidy output accordingly, avoiding noise from pre-existing code. + +## When to use me + +- After building BE, before committing C++ code changes +- When CI reports clang-tidy warnings on your PR +- When you want to proactively check new code for common bugs and style issues + +## Prerequisites + +1. **BE must be built first** — clang-tidy needs `compile_commands.json` generated during the CMake build. +2. **clang-tidy must be installed** — the project uses clang-tidy from the LDB toolchain. + +## Procedure + +### Step 1: Build BE (if not already done) + +```bash +./build.sh --be -j${DORIS_PARALLELISM} +``` + +This generates `compile_commands.json` in the build directory (e.g., `be/build_Release/` or `be/build_ASAN/`). + +### Step 2: Run clang-tidy on changed files + +```bash +build-support/run-clang-tidy.sh +``` + +By default, this script: +- Detects changed C++ files via `git diff` (uncommitted changes + staged changes) +- Automatically finds `compile_commands.json` from the latest BE build +- Runs clang-tidy using the `.clang-tidy` config +- Skips files in excluded directories (third-party, generated code) +- Reports warnings grouped by file + +**Options:** + +```bash +# Compare against a specific branch (e.g., for PR review) +build-support/run-clang-tidy.sh --base origin/master + +# Check specific files (all lines, no line-level filtering) +build-support/run-clang-tidy.sh --files be/src/vec/functions/my_new_func.cpp + +# Report ALL warnings in changed files (no line filtering) +build-support/run-clang-tidy.sh --full + +# Specify build directory explicitly +build-support/run-clang-tidy.sh --build-dir be/build_ASAN + +# Apply auto-fixes where possible (only on changed lines) +build-support/run-clang-tidy.sh --fix +``` Review Comment: The `--fix` example says auto-fixes apply “only on changed lines”, but the current script doesn’t constrain clang-tidy diagnostics/fixes to a line filter; clang-tidy may rewrite other parts of the file. Either implement a real line filter in the script or adjust this wording so users aren’t surprised by broader edits. ########## build-support/run-clang-tidy.sh: ########## @@ -0,0 +1,338 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +############################################################## +# This script runs clang-tidy on newly added or modified C++ +# code, reporting only warnings on changed lines to avoid +# noise from the existing codebase. +# +# Usage: +# build-support/run-clang-tidy.sh [options] +# +# Options: +# --base <ref> Git ref to diff against (default: auto-detect) +# --files <f1> <f2> Check specific files (all lines, no filtering) +# --build-dir <dir> Path to build dir with compile_commands.json +# --fix Apply clang-tidy auto-fixes (only on changed lines) Review Comment: The help text claims `--fix` applies auto-fixes “(changed lines only)”, but the script doesn’t pass a `-line-filter` (or similar) to clang-tidy. In practice, clang-tidy may apply fixes anywhere in the file for diagnostics it emits, which can modify unrelated lines. Either implement a proper line filter (so both diagnostics and fixes are constrained) or adjust the `--fix` wording to avoid promising changed-line-only edits. ```suggestion # --fix Apply clang-tidy auto-fixes for reported diagnostics ``` ########## build-support/run-clang-tidy.sh: ########## @@ -0,0 +1,338 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +############################################################## +# This script runs clang-tidy on newly added or modified C++ +# code, reporting only warnings on changed lines to avoid +# noise from the existing codebase. +# +# Usage: +# build-support/run-clang-tidy.sh [options] +# +# Options: +# --base <ref> Git ref to diff against (default: auto-detect) +# --files <f1> <f2> Check specific files (all lines, no filtering) +# --build-dir <dir> Path to build dir with compile_commands.json +# --fix Apply clang-tidy auto-fixes (only on changed lines) +# --full Report all warnings in changed files, not just +# warnings on changed lines +# -h, --help Show this help +############################################################## + +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +DORIS_HOME=$(cd "${ROOT}/.." && pwd) +export DORIS_HOME + +# Directories excluded from clang-tidy (third-party / generated code) +EXCLUDED_PATTERNS=( + "be/src/apache-orc/" + "be/src/clucene/" + "be/src/gutil/" + "be/src/glibc-compatibility/" + "be/src/util/mustache/" + "be/src/util/sse2neo.h" + "be/src/util/sse2neon.h" + "be/src/util/utf8_check.cpp" + "cloud/src/common/defer.h" + "contrib/" +) + +# C++ file extensions to check +CPP_EXTENSIONS="c|h|cc|cpp|cxx|hpp|hxx|hh" + +usage() { + echo "Usage: $(basename "$0") [options]" + echo "" + echo "Run clang-tidy on newly added/modified C++ code." + echo "By default, only reports warnings on changed lines." + echo "" + echo "Options:" + echo " --base <ref> Git ref to diff against (default: auto-detect)" + echo " --files <f1> ... Check specific files (all lines, no line filter)" + echo " --build-dir <dir> Path to build dir with compile_commands.json" + echo " --fix Apply clang-tidy auto-fixes (changed lines only)" + echo " --full Report all warnings in changed files" + echo " -h, --help Show this help" + exit 0 +} + +# Parse arguments +BASE_REF="" +SPECIFIC_FILES=() +BUILD_DIR="" +FIX_MODE="" +FULL_FILE_MODE=false + +while [[ $# -gt 0 ]]; do + case "$1" in + --base) + BASE_REF="$2" + shift 2 + ;; + --files) + shift + while [[ $# -gt 0 && ! "$1" =~ ^-- ]]; do + SPECIFIC_FILES+=("$1") + shift + done + ;; + --build-dir) + BUILD_DIR="$2" + shift 2 + ;; + --fix) + FIX_MODE="-fix" + shift + ;; + --full) + FULL_FILE_MODE=true + shift + ;; + -h|--help) + usage + ;; + *) + echo "Unknown option: $1" + usage + ;; + esac +done + +# Find clang-tidy +CLANG_TIDY="${CLANG_TIDY_BINARY:-$(command -v clang-tidy 2>/dev/null || true)}" +if [[ -z "${CLANG_TIDY}" ]]; then + echo "Error: clang-tidy not found. Please install clang-tidy or set CLANG_TIDY_BINARY." + exit 1 +fi + +echo "Using clang-tidy: ${CLANG_TIDY}" +echo " Version: $("${CLANG_TIDY}" --version 2>&1 | head -1)" + +# Find compile_commands.json +if [[ -z "${BUILD_DIR}" ]]; then + for candidate in \ + "${DORIS_HOME}/be/build_ASAN" \ + "${DORIS_HOME}/be/build_Release" \ + "${DORIS_HOME}/be/ut_build_ASAN" \ + "${DORIS_HOME}/be/ut_build_Release"; do + if [[ -f "${candidate}/compile_commands.json" ]]; then + BUILD_DIR="${candidate}" + break + fi + done +fi + +if [[ -z "${BUILD_DIR}" || ! -f "${BUILD_DIR}/compile_commands.json" ]]; then + echo "Error: compile_commands.json not found." + echo "Please build BE first: ./build.sh --be -j\${DORIS_PARALLELISM}" + echo "Or specify --build-dir <path>" + exit 1 +fi + +echo "Using compile_commands.json from: ${BUILD_DIR}" + +# Determine files to check +cd "${DORIS_HOME}" + +FILES_TO_CHECK=() + +if [[ ${#SPECIFIC_FILES[@]} -gt 0 ]]; then + FILES_TO_CHECK=("${SPECIFIC_FILES[@]}") +else + if [[ -n "${BASE_REF}" ]]; then + mapfile -t FILES_TO_CHECK < <(git diff --name-only --diff-filter=ACMR "${BASE_REF}" -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + else + mapfile -t STAGED < <(git diff --cached --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t UNSTAGED < <(git diff --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t FILES_TO_CHECK < <(printf '%s\n' "${STAGED[@]}" "${UNSTAGED[@]}" | sort -u | grep -v '^$') + fi +fi + +# Filter to C++ files only, excluding third-party code +FILTERED_FILES=() +for f in "${FILES_TO_CHECK[@]}"; do + if [[ -z "$f" ]]; then + continue + fi + if ! echo "$f" | grep -qE "\.(${CPP_EXTENSIONS})$"; then + continue + fi + excluded=false + for pattern in "${EXCLUDED_PATTERNS[@]}"; do + if [[ "$f" == *"${pattern}"* ]]; then + excluded=true + break + fi + done + if [[ "${excluded}" == "false" && -f "$f" ]]; then + FILTERED_FILES+=("$f") + fi +done + +if [[ ${#FILTERED_FILES[@]} -eq 0 ]]; then + echo "No changed C++ files to check." + exit 0 +fi + +echo "" +echo "Files to check (${#FILTERED_FILES[@]}):" +for f in "${FILTERED_FILES[@]}"; do + echo " ${f}" +done +echo "" + +# --------------------------------------------------------------- +# Build a map of changed line ranges per file from git diff. +# This allows us to filter clang-tidy output to only report +# warnings on lines the developer actually touched. +# --------------------------------------------------------------- +declare -A CHANGED_LINES_MAP + +build_changed_lines_map() { + local diff_output + if [[ ${#SPECIFIC_FILES[@]} -gt 0 ]]; then + # --files mode: no line filtering + return + fi + + if [[ -n "${BASE_REF}" ]]; then + diff_output=$(git diff -U0 "${BASE_REF}" -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + else + # Combine staged + unstaged diffs + local staged_diff unstaged_diff + staged_diff=$(git diff --cached -U0 -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + unstaged_diff=$(git diff -U0 -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + diff_output="${staged_diff}"$'\n'"${unstaged_diff}" + fi + + # Parse unified diff hunks to extract changed line ranges. + # Hunk headers: @@ -old_start[,old_count] +new_start[,new_count] @@ + # We collect new_start..new_start+new_count-1 as changed lines. + local current_file="" + while IFS= read -r line; do + if [[ "$line" =~ ^diff\ --git\ a/(.+)\ b/(.+)$ ]]; then + current_file="${BASH_REMATCH[2]}" + elif [[ "$line" =~ ^@@.*\+([0-9]+)(,([0-9]+))?.*@@ ]]; then Review Comment: The diff hunk-header regex appears to require trailing spaces after the closing `@@` (`.*@@ `). Standard `git diff` hunk headers are `@@ ... @@` or `@@ ... @@ <context>` (typically 0 or 1 space), so this likely won’t match and `CHANGED_LINES_MAP` will stay empty. That breaks line-level filtering (it will effectively treat everything as changed). Update the regex to match `@@` regardless of trailing whitespace / context text. ```suggestion elif [[ "$line" =~ ^@@.*\+([0-9]+)(,([0-9]+))?.*@@([[:space:]].*)?$ ]]; then ``` ########## AGENTS.md: ########## @@ -16,6 +16,16 @@ When adding code, strictly follow existing similar code in similar contexts, inc After adding code, you must first conduct self-review and refactoring attempts to ensure good abstraction and reuse as much as possible. +### Code Style Enforcement + +All code must pass style checks before committing. Use the corresponding skill for detailed step-by-step procedures. + +**BE (C++) Formatting**: Run `build-support/clang-format.sh` to auto-fix formatting. This script enforces clang-format v16; do not use other versions. Run `build-support/check-format.sh` to check without modifying files. See the `be-code-style` skill for details. + +**BE (C++) Static Analysis**: After building BE (which generates `compile_commands.json`), run `build-support/run-clang-tidy.sh` to check **only the changed lines** in modified C++ files against the `.clang-tidy` config. The script parses `git diff` to filter warnings down to your actual changes, avoiding noise from pre-existing code. Try to fix all reported warnings; if a warning cannot be reasonably fixed, add a `// NOLINT` comment with justification and report it. See the `clang-tidy-check` skill for details. Review Comment: This section states `run-clang-tidy.sh` checks “only the changed lines”. With the current script implementation, diagnostics can still leak in from included headers (missing diff ranges are treated as changed), and clang-tidy errors can be mistaken for success. Either tighten the script (e.g. clang-tidy `-line-filter` + error handling) or soften the wording here to reflect the actual behavior/limitations. ```suggestion **BE (C++) Static Analysis**: After building BE (which generates `compile_commands.json`), run `build-support/run-clang-tidy.sh` to focus checks on changed lines in modified C++ files against the `.clang-tidy` config. The script parses `git diff` to reduce warnings from unrelated code, but it is not a perfect changed-lines-only filter and may still report diagnostics from surrounding or included code. Try to fix all reported warnings; if a warning cannot be reasonably fixed, add a `// NOLINT` comment with justification and report it. See the `clang-tidy-check` skill for details. ``` ########## build-support/run-clang-tidy.sh: ########## @@ -0,0 +1,338 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +############################################################## +# This script runs clang-tidy on newly added or modified C++ +# code, reporting only warnings on changed lines to avoid +# noise from the existing codebase. +# +# Usage: +# build-support/run-clang-tidy.sh [options] +# +# Options: +# --base <ref> Git ref to diff against (default: auto-detect) +# --files <f1> <f2> Check specific files (all lines, no filtering) +# --build-dir <dir> Path to build dir with compile_commands.json +# --fix Apply clang-tidy auto-fixes (only on changed lines) +# --full Report all warnings in changed files, not just +# warnings on changed lines +# -h, --help Show this help +############################################################## + +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +DORIS_HOME=$(cd "${ROOT}/.." && pwd) +export DORIS_HOME + +# Directories excluded from clang-tidy (third-party / generated code) +EXCLUDED_PATTERNS=( + "be/src/apache-orc/" + "be/src/clucene/" + "be/src/gutil/" + "be/src/glibc-compatibility/" + "be/src/util/mustache/" + "be/src/util/sse2neo.h" + "be/src/util/sse2neon.h" + "be/src/util/utf8_check.cpp" + "cloud/src/common/defer.h" + "contrib/" +) + +# C++ file extensions to check +CPP_EXTENSIONS="c|h|cc|cpp|cxx|hpp|hxx|hh" + +usage() { + echo "Usage: $(basename "$0") [options]" + echo "" + echo "Run clang-tidy on newly added/modified C++ code." + echo "By default, only reports warnings on changed lines." + echo "" + echo "Options:" + echo " --base <ref> Git ref to diff against (default: auto-detect)" + echo " --files <f1> ... Check specific files (all lines, no line filter)" + echo " --build-dir <dir> Path to build dir with compile_commands.json" + echo " --fix Apply clang-tidy auto-fixes (changed lines only)" + echo " --full Report all warnings in changed files" + echo " -h, --help Show this help" + exit 0 +} + +# Parse arguments +BASE_REF="" +SPECIFIC_FILES=() +BUILD_DIR="" +FIX_MODE="" +FULL_FILE_MODE=false + +while [[ $# -gt 0 ]]; do + case "$1" in + --base) + BASE_REF="$2" + shift 2 + ;; + --files) + shift + while [[ $# -gt 0 && ! "$1" =~ ^-- ]]; do + SPECIFIC_FILES+=("$1") + shift + done + ;; + --build-dir) + BUILD_DIR="$2" + shift 2 + ;; + --fix) + FIX_MODE="-fix" + shift + ;; + --full) + FULL_FILE_MODE=true + shift + ;; + -h|--help) + usage + ;; + *) + echo "Unknown option: $1" + usage + ;; + esac +done + +# Find clang-tidy +CLANG_TIDY="${CLANG_TIDY_BINARY:-$(command -v clang-tidy 2>/dev/null || true)}" +if [[ -z "${CLANG_TIDY}" ]]; then + echo "Error: clang-tidy not found. Please install clang-tidy or set CLANG_TIDY_BINARY." + exit 1 +fi + +echo "Using clang-tidy: ${CLANG_TIDY}" +echo " Version: $("${CLANG_TIDY}" --version 2>&1 | head -1)" + +# Find compile_commands.json +if [[ -z "${BUILD_DIR}" ]]; then + for candidate in \ + "${DORIS_HOME}/be/build_ASAN" \ + "${DORIS_HOME}/be/build_Release" \ + "${DORIS_HOME}/be/ut_build_ASAN" \ + "${DORIS_HOME}/be/ut_build_Release"; do + if [[ -f "${candidate}/compile_commands.json" ]]; then + BUILD_DIR="${candidate}" + break + fi + done +fi + +if [[ -z "${BUILD_DIR}" || ! -f "${BUILD_DIR}/compile_commands.json" ]]; then + echo "Error: compile_commands.json not found." + echo "Please build BE first: ./build.sh --be -j\${DORIS_PARALLELISM}" + echo "Or specify --build-dir <path>" + exit 1 +fi + +echo "Using compile_commands.json from: ${BUILD_DIR}" + +# Determine files to check +cd "${DORIS_HOME}" + +FILES_TO_CHECK=() + +if [[ ${#SPECIFIC_FILES[@]} -gt 0 ]]; then + FILES_TO_CHECK=("${SPECIFIC_FILES[@]}") +else + if [[ -n "${BASE_REF}" ]]; then + mapfile -t FILES_TO_CHECK < <(git diff --name-only --diff-filter=ACMR "${BASE_REF}" -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + else + mapfile -t STAGED < <(git diff --cached --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t UNSTAGED < <(git diff --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t FILES_TO_CHECK < <(printf '%s\n' "${STAGED[@]}" "${UNSTAGED[@]}" | sort -u | grep -v '^$') + fi Review Comment: Several `git diff` calls suppress errors (`2>/dev/null || true`). If `--base` is invalid (typo, missing remote ref), the script will silently act as if there are no changed files and exit 0. Consider surfacing git errors (at least for `--base` mode) and failing fast so users don’t get a false pass. ########## build-support/run-clang-tidy.sh: ########## @@ -0,0 +1,338 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +############################################################## +# This script runs clang-tidy on newly added or modified C++ +# code, reporting only warnings on changed lines to avoid +# noise from the existing codebase. +# +# Usage: +# build-support/run-clang-tidy.sh [options] +# +# Options: +# --base <ref> Git ref to diff against (default: auto-detect) +# --files <f1> <f2> Check specific files (all lines, no filtering) +# --build-dir <dir> Path to build dir with compile_commands.json +# --fix Apply clang-tidy auto-fixes (only on changed lines) +# --full Report all warnings in changed files, not just +# warnings on changed lines +# -h, --help Show this help +############################################################## + +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +DORIS_HOME=$(cd "${ROOT}/.." && pwd) +export DORIS_HOME + +# Directories excluded from clang-tidy (third-party / generated code) +EXCLUDED_PATTERNS=( + "be/src/apache-orc/" + "be/src/clucene/" + "be/src/gutil/" + "be/src/glibc-compatibility/" + "be/src/util/mustache/" + "be/src/util/sse2neo.h" + "be/src/util/sse2neon.h" + "be/src/util/utf8_check.cpp" + "cloud/src/common/defer.h" + "contrib/" +) + +# C++ file extensions to check +CPP_EXTENSIONS="c|h|cc|cpp|cxx|hpp|hxx|hh" + +usage() { + echo "Usage: $(basename "$0") [options]" + echo "" + echo "Run clang-tidy on newly added/modified C++ code." + echo "By default, only reports warnings on changed lines." + echo "" + echo "Options:" + echo " --base <ref> Git ref to diff against (default: auto-detect)" + echo " --files <f1> ... Check specific files (all lines, no line filter)" + echo " --build-dir <dir> Path to build dir with compile_commands.json" + echo " --fix Apply clang-tidy auto-fixes (changed lines only)" + echo " --full Report all warnings in changed files" + echo " -h, --help Show this help" + exit 0 +} + +# Parse arguments +BASE_REF="" +SPECIFIC_FILES=() +BUILD_DIR="" +FIX_MODE="" +FULL_FILE_MODE=false + +while [[ $# -gt 0 ]]; do + case "$1" in + --base) + BASE_REF="$2" + shift 2 + ;; + --files) + shift + while [[ $# -gt 0 && ! "$1" =~ ^-- ]]; do + SPECIFIC_FILES+=("$1") + shift + done + ;; + --build-dir) + BUILD_DIR="$2" + shift 2 + ;; + --fix) + FIX_MODE="-fix" + shift + ;; + --full) + FULL_FILE_MODE=true + shift + ;; + -h|--help) + usage + ;; + *) + echo "Unknown option: $1" + usage + ;; + esac +done + +# Find clang-tidy +CLANG_TIDY="${CLANG_TIDY_BINARY:-$(command -v clang-tidy 2>/dev/null || true)}" +if [[ -z "${CLANG_TIDY}" ]]; then + echo "Error: clang-tidy not found. Please install clang-tidy or set CLANG_TIDY_BINARY." + exit 1 +fi + +echo "Using clang-tidy: ${CLANG_TIDY}" +echo " Version: $("${CLANG_TIDY}" --version 2>&1 | head -1)" + +# Find compile_commands.json +if [[ -z "${BUILD_DIR}" ]]; then + for candidate in \ + "${DORIS_HOME}/be/build_ASAN" \ + "${DORIS_HOME}/be/build_Release" \ + "${DORIS_HOME}/be/ut_build_ASAN" \ + "${DORIS_HOME}/be/ut_build_Release"; do + if [[ -f "${candidate}/compile_commands.json" ]]; then + BUILD_DIR="${candidate}" + break + fi + done +fi + +if [[ -z "${BUILD_DIR}" || ! -f "${BUILD_DIR}/compile_commands.json" ]]; then + echo "Error: compile_commands.json not found." + echo "Please build BE first: ./build.sh --be -j\${DORIS_PARALLELISM}" + echo "Or specify --build-dir <path>" + exit 1 +fi + +echo "Using compile_commands.json from: ${BUILD_DIR}" + +# Determine files to check +cd "${DORIS_HOME}" + +FILES_TO_CHECK=() + +if [[ ${#SPECIFIC_FILES[@]} -gt 0 ]]; then + FILES_TO_CHECK=("${SPECIFIC_FILES[@]}") +else + if [[ -n "${BASE_REF}" ]]; then + mapfile -t FILES_TO_CHECK < <(git diff --name-only --diff-filter=ACMR "${BASE_REF}" -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + else + mapfile -t STAGED < <(git diff --cached --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t UNSTAGED < <(git diff --name-only --diff-filter=ACMR -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/' 2>/dev/null || true) + mapfile -t FILES_TO_CHECK < <(printf '%s\n' "${STAGED[@]}" "${UNSTAGED[@]}" | sort -u | grep -v '^$') + fi +fi + +# Filter to C++ files only, excluding third-party code +FILTERED_FILES=() +for f in "${FILES_TO_CHECK[@]}"; do + if [[ -z "$f" ]]; then + continue + fi + if ! echo "$f" | grep -qE "\.(${CPP_EXTENSIONS})$"; then + continue + fi + excluded=false + for pattern in "${EXCLUDED_PATTERNS[@]}"; do + if [[ "$f" == *"${pattern}"* ]]; then + excluded=true + break + fi + done + if [[ "${excluded}" == "false" && -f "$f" ]]; then + FILTERED_FILES+=("$f") + fi +done + +if [[ ${#FILTERED_FILES[@]} -eq 0 ]]; then + echo "No changed C++ files to check." + exit 0 +fi + +echo "" +echo "Files to check (${#FILTERED_FILES[@]}):" +for f in "${FILTERED_FILES[@]}"; do + echo " ${f}" +done +echo "" + +# --------------------------------------------------------------- +# Build a map of changed line ranges per file from git diff. +# This allows us to filter clang-tidy output to only report +# warnings on lines the developer actually touched. +# --------------------------------------------------------------- +declare -A CHANGED_LINES_MAP + +build_changed_lines_map() { + local diff_output + if [[ ${#SPECIFIC_FILES[@]} -gt 0 ]]; then + # --files mode: no line filtering + return + fi + + if [[ -n "${BASE_REF}" ]]; then + diff_output=$(git diff -U0 "${BASE_REF}" -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + else + # Combine staged + unstaged diffs + local staged_diff unstaged_diff + staged_diff=$(git diff --cached -U0 -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + unstaged_diff=$(git diff -U0 -- "${FILTERED_FILES[@]}" 2>/dev/null || true) + diff_output="${staged_diff}"$'\n'"${unstaged_diff}" + fi + + # Parse unified diff hunks to extract changed line ranges. + # Hunk headers: @@ -old_start[,old_count] +new_start[,new_count] @@ + # We collect new_start..new_start+new_count-1 as changed lines. + local current_file="" + while IFS= read -r line; do + if [[ "$line" =~ ^diff\ --git\ a/(.+)\ b/(.+)$ ]]; then + current_file="${BASH_REMATCH[2]}" + elif [[ "$line" =~ ^@@.*\+([0-9]+)(,([0-9]+))?.*@@ ]]; then + local start="${BASH_REMATCH[1]}" + local count="${BASH_REMATCH[3]:-1}" + if [[ "${count}" -eq 0 ]]; then + continue + fi + local end=$((start + count - 1)) + # Append range to the file's entry (space-separated "start:end" pairs) + CHANGED_LINES_MAP["${current_file}"]+="${start}:${end} " + fi + done <<< "${diff_output}" +} + +# Check if a line number falls within any changed range for a file +is_line_changed() { + local file="$1" + local line_num="$2" + local ranges="${CHANGED_LINES_MAP["${file}"]}" + + if [[ -z "${ranges}" ]]; then + # No range info (e.g., new file or --files mode) — treat all lines as changed + return 0 + fi + + for range in ${ranges}; do + local start="${range%%:*}" + local end="${range##*:}" + if [[ "${line_num}" -ge "${start}" && "${line_num}" -le "${end}" ]]; then + return 0 + fi + done + return 1 +} + +if [[ "${FULL_FILE_MODE}" == "false" && ${#SPECIFIC_FILES[@]} -eq 0 ]]; then + echo "Line-level filtering: only warnings on changed lines will be reported." + echo "Use --full to see all warnings in changed files." + echo "" + build_changed_lines_map +fi + +# Run clang-tidy +TIDY_ARGS=( + -p "${BUILD_DIR}" + --config-file="${DORIS_HOME}/.clang-tidy" +) + +if [[ -n "${FIX_MODE}" ]]; then + TIDY_ARGS+=("${FIX_MODE}") +fi + +HAS_WARNINGS=0 +TOTAL_WARNINGS=0 + +for f in "${FILTERED_FILES[@]}"; do + echo "=== Checking: ${f} ===" + OUTPUT=$("${CLANG_TIDY}" "${TIDY_ARGS[@]}" "${f}" 2>&1) || true + + if echo "${OUTPUT}" | grep -q "warning:"; then Review Comment: clang-tidy failures are currently ignored (`... ) || true`), and the script only looks for `warning:` in output. If clang-tidy emits `error:` (e.g. missing compile command for a file), this will be reported as “No warnings” and the script may exit 0, which defeats enforcement. Capture the clang-tidy exit status and/or scan for `error:`/`fatal error:` and fail the script when analysis couldn’t be performed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
