mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
[chore](build) Add code style enforcement documentation, skills, and clang-tidy script (#62307)
Problem Summary: The AGENTS.md and Copilot skills had no documentation about code style enforcement workflows for BE (C++ clang-format/clang-tidy) or FE (Java checkstyle). This made it unclear how and when to run formatting and static analysis tools before committing code changes. This PR adds: 1. A "Code Style Enforcement" subsection under "Coding Standards" in AGENTS.md covering BE clang-format, BE clang-tidy, and FE checkstyle. 2. Three new skills: `be-code-style`, `fe-code-style`, `clang-tidy-check`, each providing step-by-step procedures for fixing style issues. 3. A new `build-support/run-clang-tidy.sh` script that runs clang-tidy incrementally on **only changed lines** (not entire files) by parsing git diff, avoiding noise from pre-existing code. ### Release note None ### Check List (For Author) - Test: Manual test - Verified check-format.sh, run-clang-tidy.sh (--help, --files, --base, --full, line-level filtering logic), and FE checkstyle all work correctly - Behavior changed: No - Does this need documentation: No (this IS documentation) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
1 parent
61ab06990e
commit
3d7594249f
5 files changed
+692
No files matched your search
@@ -0,0 +1,67 @@
|
||||
---
|
||||
name: be-code-style
|
||||
description: Fix BE (C++) code formatting issues using clang-format
|
||||
compatibility: opencode
|
||||
---
|
||||
|
||||
## What I do
|
||||
|
||||
Fix C++ code formatting issues in the BE and Cloud modules using the project's clang-format configuration (v16).
|
||||
|
||||
## When to use me
|
||||
|
||||
- Before committing BE/Cloud C++ code changes
|
||||
- When CI reports clang-format failures
|
||||
- When you need to check or fix C++ code style
|
||||
|
||||
## Procedure
|
||||
|
||||
### Step 1: Auto-fix formatting
|
||||
|
||||
Run the project's formatting script, which enforces clang-format v16:
|
||||
|
||||
```bash
|
||||
build-support/clang-format.sh
|
||||
```
|
||||
|
||||
This formats all C++ files under `be/src`, `be/test`, `cloud/src`, `cloud/test` in-place, respecting `.clang-format` and `.clang-format-ignore`.
|
||||
|
||||
**Important**: Always use this script instead of invoking `clang-format` directly. The script checks that clang-format version 16 is installed and exits with an error if the wrong version is found. Using a different version will produce inconsistent formatting.
|
||||
|
||||
### Step 2: Check-only (no modification)
|
||||
|
||||
To verify formatting without modifying files:
|
||||
|
||||
```bash
|
||||
build-support/check-format.sh
|
||||
```
|
||||
|
||||
This outputs a diff of any formatting violations and exits non-zero if there are any.
|
||||
|
||||
### Step 3: Review and commit
|
||||
|
||||
After running `clang-format.sh`, review the changes with `git diff` to verify only formatting was changed, then stage and commit.
|
||||
|
||||
## Key Configuration
|
||||
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `.clang-format` | Main formatting rules (Google-based, 100 col, 4-space indent) |
|
||||
| `.clang-format-ignore` | Files excluded from formatting (third-party, generated) |
|
||||
| `build-support/run_clang_format.py` | Python wrapper for parallel execution |
|
||||
|
||||
## Excluded Directories
|
||||
|
||||
The following are excluded from formatting (see `.clang-format-ignore`):
|
||||
- `be/src/apache-orc/*`, `be/src/clucene/*`, `be/src/gutil/*`
|
||||
- `be/src/glibc-compatibility/*`
|
||||
- Specific third-party vendored files (mustache, sse2neon, utf8_check)
|
||||
- `cloud/src/common/defer.h`
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
| Problem | Solution |
|
||||
|---------|----------|
|
||||
| `clang-format not found` | Install clang-format v16 or set `CLANG_FORMAT_BINARY` env var |
|
||||
| `version is not 16` | Install clang-format v16; on macOS: `brew install llvm@16` |
|
||||
| Files not being formatted | Check `.clang-format-ignore` for exclusions |
|
||||
@@ -0,0 +1,138 @@
|
||||
---
|
||||
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 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 to diagnostics on those lines where possible, reducing noise from pre-existing code. Diagnostics from included headers that were not part of the diff are filtered out, though some edge cases may still appear.
|
||||
|
||||
## 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. **The relevant module must be built first** — clang-tidy needs `compile_commands.json` generated during the CMake build. For BE files, build BE; for Cloud files, build Cloud.
|
||||
2. **clang-tidy must be installed** — the project uses clang-tidy from the LDB toolchain.
|
||||
|
||||
## Procedure
|
||||
|
||||
### Step 1: Build the relevant module (if not already done)
|
||||
|
||||
For **BE** files:
|
||||
```bash
|
||||
./build.sh --be -j${DORIS_PARALLELISM}
|
||||
```
|
||||
This generates `compile_commands.json` in `be/build_Release/` or `be/build_ASAN/`.
|
||||
|
||||
For **Cloud** files:
|
||||
```bash
|
||||
./build.sh --cloud -j${DORIS_PARALLELISM}
|
||||
```
|
||||
This generates `compile_commands.json` in `cloud/build_Release/` or `cloud/build_ASAN/`.
|
||||
|
||||
**Note**: A single compilation database typically covers only one module (BE or Cloud). If your changes span both modules, you may need to run clang-tidy twice with different `--build-dir` values.
|
||||
|
||||
### 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 (required for Cloud if BE build dir was auto-detected)
|
||||
build-support/run-clang-tidy.sh --build-dir be/build_ASAN
|
||||
|
||||
# Check Cloud files with Cloud compilation database
|
||||
build-support/run-clang-tidy.sh --build-dir cloud/build_ASAN
|
||||
|
||||
# Apply auto-fixes where possible (note: fixes may apply beyond changed lines)
|
||||
build-support/run-clang-tidy.sh --fix
|
||||
```
|
||||
|
||||
**Important**: By default, only warnings on changed lines are reported (diagnostics from headers not in the diff are filtered out). This prevents the agent from "fixing" unrelated pre-existing warnings in large files. Use `--full` to see all warnings if needed.
|
||||
|
||||
### Step 3: Interpret and fix warnings
|
||||
|
||||
clang-tidy output looks like:
|
||||
|
||||
```
|
||||
be/src/vec/functions/foo.cpp:42:5: warning: use auto when initializing with a cast [modernize-use-auto]
|
||||
be/src/vec/functions/foo.cpp:55:1: warning: function 'process' exceeds recommended size [readability-function-size]
|
||||
```
|
||||
|
||||
**Fix approach:**
|
||||
1. Fix all warnings that represent real issues (bugs, performance, readability)
|
||||
2. For false positives or intentional patterns, add `// NOLINT(check-name)` with a brief justification
|
||||
3. Report any warnings you cannot reasonably fix
|
||||
|
||||
### Step 4: Verify
|
||||
|
||||
Re-run the script to confirm all warnings are resolved:
|
||||
|
||||
```bash
|
||||
build-support/run-clang-tidy.sh
|
||||
```
|
||||
|
||||
## Enabled Checks (from `.clang-tidy`)
|
||||
|
||||
| Category | Examples |
|
||||
|----------|----------|
|
||||
| `clang-diagnostic-*` | Compiler diagnostics |
|
||||
| `clang-analyzer-*` | Static analysis (null deref, use-after-free, etc.) |
|
||||
| `bugprone-*` | Use-after-move, redundant branch condition, unused RAII |
|
||||
| `modernize-*` | Auto, range-for, nullptr (excludes trailing return, nodiscard) |
|
||||
| `readability-*` | Function size (≤80 lines), cognitive complexity (≤50) |
|
||||
| `performance-*` | String find, inefficient algorithm, move-const-arg |
|
||||
| `misc-redundant-expression` | Redundant expressions |
|
||||
|
||||
## Excluded Directories
|
||||
|
||||
Files in these paths are automatically skipped:
|
||||
- `be/src/apache-orc/`, `be/src/clucene/`, `be/src/gutil/`
|
||||
- `be/src/glibc-compatibility/`
|
||||
- `contrib/` (all third-party code)
|
||||
- Generated code directories
|
||||
|
||||
## NOLINT Usage
|
||||
|
||||
When a clang-tidy warning cannot be fixed, suppress it with a comment:
|
||||
|
||||
```cpp
|
||||
// Good: specific check name + reason
|
||||
int x = (int)y; // NOLINT(modernize-use-auto): explicit cast for clarity
|
||||
|
||||
// Bad: blanket suppression without reason
|
||||
int x = (int)y; // NOLINT
|
||||
```
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
| Problem | Solution |
|
||||
|---------|----------|
|
||||
| `compile_commands.json not found` | Build BE first: `./build.sh --be -j${DORIS_PARALLELISM}` |
|
||||
| `clang-tidy not found` | Install via LDB toolchain or `apt install clang-tidy-16` |
|
||||
| Too many warnings on old code | Use `--base` to diff against a branch, ensuring only your changes are checked |
|
||||
| Warning in header included by your file | Only fix if the header is also in your changeset; otherwise note it in your report |
|
||||
@@ -0,0 +1,102 @@
|
||||
---
|
||||
name: fe-code-style
|
||||
description: Fix FE (Java) code style issues using Checkstyle
|
||||
compatibility: opencode
|
||||
---
|
||||
|
||||
## What I do
|
||||
|
||||
Diagnose and fix Java code style issues in the FE module using the project's Checkstyle configuration.
|
||||
|
||||
## When to use me
|
||||
|
||||
- Before committing FE Java code changes
|
||||
- When FE build fails due to checkstyle violations
|
||||
- When you need to understand FE style rules
|
||||
|
||||
## Procedure
|
||||
|
||||
### Step 1: Run checkstyle
|
||||
|
||||
Checkstyle is integrated into the Maven build and runs automatically during `mvn validate`. To check style only (without full compilation):
|
||||
|
||||
```bash
|
||||
cd fe && mvn checkstyle:check -pl fe-core
|
||||
```
|
||||
|
||||
Or as part of the normal build:
|
||||
|
||||
```bash
|
||||
./build.sh --fe -j${DORIS_PARALLELISM}
|
||||
```
|
||||
|
||||
If checkstyle fails, the build will fail with error messages showing the file, line number, and rule violated.
|
||||
|
||||
### Step 2: Interpret violations
|
||||
|
||||
Checkstyle output looks like:
|
||||
|
||||
```
|
||||
[ERROR] src/main/java/.../Foo.java:[42:5] (imports) UnusedImports: Unused import - java.util.List.
|
||||
[ERROR] src/main/java/.../Bar.java:[10] (header) RegexpHeader: Line does not match expected header line...
|
||||
```
|
||||
|
||||
Each error shows: `[file]:[line:col] (category) RuleName: description`.
|
||||
|
||||
### Step 3: Fix common violations
|
||||
|
||||
| Violation | Fix |
|
||||
|-----------|-----|
|
||||
| `RegexpHeader` | Add/fix Apache License header at file top |
|
||||
| `UnusedImports` | Remove unused import statements |
|
||||
| `LineLength` (>120 chars) | Break long lines |
|
||||
| `IllegalImport` (shaded classes) | Use the non-shaded equivalent (see `import-control.xml`) |
|
||||
| `FileTabCharacter` | Replace tabs with spaces |
|
||||
| `NewlineAtEndOfFile` | Ensure file ends with a newline |
|
||||
| `MergeConflictMarker` | Resolve git merge conflicts |
|
||||
|
||||
### Step 4: Verify fix
|
||||
|
||||
After fixing, re-run checkstyle to confirm:
|
||||
|
||||
```bash
|
||||
cd fe && mvn checkstyle:check -pl fe-core
|
||||
```
|
||||
|
||||
## Key Configuration
|
||||
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `fe/check/checkstyle/checkstyle.xml` | Main rules (license header, line length 120, encoding, imports) |
|
||||
| `fe/check/checkstyle/suppressions.xml` | Rule exclusions (test files, nereids, large files) |
|
||||
| `fe/check/checkstyle/import-control.xml` | Import restrictions (no shaded classes, no old logging APIs, no Lombok in nereids) |
|
||||
| `fe/check/checkstyle/checkstyle-apache-header.txt` | Apache License 2.0 header template |
|
||||
| `build-support/IntelliJ-code-format.xml` | IntelliJ formatter scheme (120 col, import organization) |
|
||||
|
||||
## Suppression Rules
|
||||
|
||||
Some files/packages have relaxed rules (see `suppressions.xml`):
|
||||
- Test files: Javadoc rules suppressed
|
||||
- Non-nereids code: Some strict rules suppressed
|
||||
- Specific large files: Individual suppressions
|
||||
|
||||
## Excluded Code
|
||||
|
||||
The following are excluded from checkstyle (see `fe/pom.xml`):
|
||||
- `**/apache/doris/thrift/**/*` (generated Thrift code)
|
||||
- `**/apache/parquet/**/*` (generated Parquet code)
|
||||
|
||||
## Import Restrictions (key rules from import-control.xml)
|
||||
|
||||
- **No shaded imports**: Do not use `org.apache.doris.thirdparty.*` directly
|
||||
- **No old logging**: Do not use `org.apache.commons.logging` or `java.util.logging`
|
||||
- **No SimpleDateFormat**: Use `java.time` APIs instead
|
||||
- **No Lombok in nereids**: `lombok` is disallowed in `org.apache.doris.nereids` package
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
| Problem | Solution |
|
||||
|---------|----------|
|
||||
| Build fails on `validate` phase | Run `mvn checkstyle:check -pl fe-core` to see specific violations |
|
||||
| Can't find checkstyle config | Ensure you're running from the `fe/` directory |
|
||||
| IntelliJ formatting differs | Import `build-support/IntelliJ-code-format.xml` via Settings → Editor → Code Style |
|
||||
@@ -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 modified C++ files against the `.clang-tidy` config. The script parses `git diff` to filter warnings to changed lines where possible, reducing noise from pre-existing code (diagnostics from included headers may still appear). For Cloud C++ files, pass `--build-dir` pointing to the Cloud compilation database (e.g., `cloud/build_ASAN`). 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.
|
||||
|
||||
**FE (Java) Style**: Checkstyle is integrated into the Maven build (`maven-checkstyle-plugin`). Running `build.sh --fe` automatically validates style via `mvn validate`. If checkstyle fails, fix the reported issues according to `fe/check/checkstyle/checkstyle.xml`. See the `fe-code-style` skill for details.
|
||||
|
||||
## Code Review
|
||||
|
||||
When conducting code review (including self-review and review tasks), it is necessary to complete the key checkpoints according to our `code-review` skill and provide conclusions for each key checkpoint (if applicable) as part of the final written description. Other content does not require individual responses; just check them during the review process.
|
||||
|
||||
Executable
+375
@@ -0,0 +1,375 @@
|
||||
#!/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 (note: fixes may
|
||||
# apply to entire diagnostics, not just 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() {
|
||||
local exit_code="${1:-0}"
|
||||
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 (note: fixes may apply"
|
||||
echo " to entire diagnostics, not just changed lines)"
|
||||
echo " --full Report all warnings in changed files"
|
||||
echo " -h, --help Show this help"
|
||||
exit "${exit_code}"
|
||||
}
|
||||
|
||||
# Parse arguments
|
||||
BASE_REF=""
|
||||
MERGE_BASE=""
|
||||
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 1
|
||||
;;
|
||||
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" \
|
||||
"${DORIS_HOME}/cloud/build_ASAN" \
|
||||
"${DORIS_HOME}/cloud/build_Release" \
|
||||
"${DORIS_HOME}/cloud/ut_build_ASAN" \
|
||||
"${DORIS_HOME}/cloud/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
|
||||
# Validate the base ref before proceeding
|
||||
if ! git rev-parse --verify "${BASE_REF}" &>/dev/null; then
|
||||
echo "Error: invalid git ref '${BASE_REF}'. Check for typos or fetch the remote first."
|
||||
exit 1
|
||||
fi
|
||||
# Use merge-base so we only see files changed on THIS branch,
|
||||
# not files that changed on the base branch after we forked.
|
||||
MERGE_BASE=$(git merge-base "${BASE_REF}" HEAD)
|
||||
mapfile -t FILES_TO_CHECK < <(git diff --name-only --diff-filter=ACMR "${MERGE_BASE}" -- 'be/src/' 'be/test/' 'cloud/src/' 'cloud/test/')
|
||||
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 "${MERGE_BASE}" -- "${FILTERED_FILES[@]}")
|
||||
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.
|
||||
# Files not present in the diff (e.g. headers included by changed files) are
|
||||
# treated as "not changed" so their diagnostics are filtered out.
|
||||
is_line_changed() {
|
||||
local file="$1"
|
||||
local line_num="$2"
|
||||
local ranges="${CHANGED_LINES_MAP["${file}"]}"
|
||||
|
||||
if [[ -z "${ranges}" ]]; then
|
||||
# No range info: file was not in the diff.
|
||||
# In --files mode, build_changed_lines_map is not called, so this function
|
||||
# is never reached (full-file mode is used instead). For diff-based runs,
|
||||
# treat unknown files as not changed to avoid header noise.
|
||||
return 1
|
||||
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} ==="
|
||||
TIDY_EXIT=0
|
||||
OUTPUT=$("${CLANG_TIDY}" "${TIDY_ARGS[@]}" "${f}" 2>&1) || TIDY_EXIT=$?
|
||||
|
||||
# Detect real analysis failures (missing compile command, broken AST, etc.)
|
||||
# but NOT promoted warnings. WarningsAsErrors: "*" in .clang-tidy causes
|
||||
# normal findings to be emitted as "error: ... [-warnings-as-errors]"; those
|
||||
# are diagnostics, not analysis failures.
|
||||
if echo "${OUTPUT}" | grep -E "(^|: )(error|fatal error):" | grep -qv "\-warnings-as-errors\]"; then
|
||||
echo "${OUTPUT}"
|
||||
echo ""
|
||||
echo "Error: clang-tidy could not analyze ${f} (see above)."
|
||||
HAS_WARNINGS=1
|
||||
TOTAL_WARNINGS=$((TOTAL_WARNINGS + 1))
|
||||
continue
|
||||
fi
|
||||
|
||||
# Match both "warning:" lines and promoted "error: ... [-warnings-as-errors]"
|
||||
# lines as diagnostics.
|
||||
DIAG_PATTERN="(warning:|error:.*-warnings-as-errors)"
|
||||
if echo "${OUTPUT}" | grep -qE "${DIAG_PATTERN}"; then
|
||||
if [[ "${FULL_FILE_MODE}" == "true" || ${#SPECIFIC_FILES[@]} -gt 0 ]]; then
|
||||
# Full mode or --files: show all warnings
|
||||
echo "${OUTPUT}"
|
||||
COUNT=$(echo "${OUTPUT}" | grep -cE "${DIAG_PATTERN}" || true)
|
||||
TOTAL_WARNINGS=$((TOTAL_WARNINGS + COUNT))
|
||||
HAS_WARNINGS=1
|
||||
else
|
||||
# Line-level filtering: only show warnings on changed lines
|
||||
FILE_WARNINGS=""
|
||||
FILE_COUNT=0
|
||||
while IFS= read -r wline; do
|
||||
# Parse "filepath:line:col: warning/error: ..."
|
||||
if [[ "$wline" =~ ^([^:]+):([0-9]+):[0-9]+:\ (warning|error): ]]; then
|
||||
local_file="${BASH_REMATCH[1]}"
|
||||
local_line="${BASH_REMATCH[2]}"
|
||||
# Normalize: clang-tidy may emit absolute or relative paths
|
||||
local_file_rel="${local_file#"${DORIS_HOME}/"}"
|
||||
if is_line_changed "${local_file_rel}" "${local_line}"; then
|
||||
FILE_WARNINGS+="${wline}"$'\n'
|
||||
FILE_COUNT=$((FILE_COUNT + 1))
|
||||
fi
|
||||
fi
|
||||
done <<< "$(echo "${OUTPUT}" | grep -E "${DIAG_PATTERN}")"
|
||||
|
||||
if [[ ${FILE_COUNT} -gt 0 ]]; then
|
||||
echo "${FILE_WARNINGS}"
|
||||
TOTAL_WARNINGS=$((TOTAL_WARNINGS + FILE_COUNT))
|
||||
HAS_WARNINGS=1
|
||||
else
|
||||
echo " No warnings on changed lines."
|
||||
fi
|
||||
fi
|
||||
else
|
||||
echo " No warnings."
|
||||
fi
|
||||
echo ""
|
||||
done
|
||||
|
||||
echo "========================================"
|
||||
if [[ ${HAS_WARNINGS} -eq 1 ]]; then
|
||||
echo "clang-tidy found ${TOTAL_WARNINGS} warning(s) on changed lines."
|
||||
echo "Please fix the warnings above, or add // NOLINT(check-name) with justification."
|
||||
exit 1
|
||||
else
|
||||
echo "clang-tidy: all checks passed."
|
||||
exit 0
|
||||
fi
|
||||
Reference in new issue
Block a user