From 847176dbb3d705342f7c3ce734d62650d8e103c2 Mon Sep 17 00:00:00 2001 From: Anton Vinogradov Date: Fri, 26 Jun 2026 01:34:16 +0300 Subject: [PATCH] IGNITE-28822 Protected Classes CI check: reduce cognitive complexity of the detection script Collapse the two near-identical detection loops (added/deleted vs modified files) into a single pass and split the logic into small named functions (`changed_java_files`, `is_protected_class`, `filter_protected`) so the step reads as `changed_java_files | filter_protected`. Per file status the inspected revision is A -> head, D/M -> base. Drop the string accumulation with literal \n + `printf '%b'`, the process substitution and the parallel `HITS` variable; matches are written straight to the output file and detected with `[ -s ]`. This roughly halves the cyclomatic/cognitive complexity and removes the duplicated loop body. Side effect: the content-based grep removes a false positive where the previous diff-based grep matched the file path `org/apache/ignite/internal/Order*.java` in diff headers (regex dots match slashes). Co-Authored-By: Claude Opus 4.8 --- .github/workflows/check-protected-classes.yml | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/.github/workflows/check-protected-classes.yml b/.github/workflows/check-protected-classes.yml index 4abab25b790b8..6e8ab87f2b54e 100644 --- a/.github/workflows/check-protected-classes.yml +++ b/.github/workflows/check-protected-classes.yml @@ -39,26 +39,35 @@ jobs: run: | BASE_SHA=${{ github.event.pull_request.base.sha }} HEAD_SHA=${{ github.event.pull_request.head.sha }} - - HITS="" - - # New and deleted files: check diff content for @Order annotation. - for file in $(git diff --name-only --no-renames --diff-filter=AD "$BASE_SHA"..."$HEAD_SHA" -- '*.java'); do - if git diff "$BASE_SHA"..."$HEAD_SHA" -- "$file" | grep -q 'org.apache.ignite.internal.Order'; then - HITS="${HITS}${file}\n" - fi - done - - # Modified files: check base version content for @Order annotation. - for file in $(git diff --name-only --no-renames --diff-filter=M "$BASE_SHA"..."$HEAD_SHA" -- '*.java'); do - if git show "${BASE_SHA}:${file}" 2>/dev/null | grep -q 'org.apache.ignite.internal.Order'; then - HITS="${HITS}${file}\n" - fi - done - - if [ -n "$HITS" ]; then + ANNOTATION='org.apache.ignite.internal.Order' + HITS_FILE=/tmp/protected-hits.txt + + # Added/deleted/modified .java files in the PR as "\t" lines. + changed_java_files() { + git diff --name-status --no-renames --diff-filter=ADM "$BASE_SHA"..."$HEAD_SHA" -- '*.java' + } + + # A protected class carries the @Order annotation; an added file is + # defined by the new revision, a deleted/modified one by the base. + is_protected_class() { + local status=$1 file=$2 ref=$BASE_SHA + [ "$status" = A ] && ref=$HEAD_SHA + git show "$ref:$file" 2>/dev/null | grep -q "$ANNOTATION" + } + + # Read "\t" lines, emit the paths that are protected. + filter_protected() { + while IFS=$'\t' read -r status file; do + if is_protected_class "$status" "$file"; then + echo "$file" + fi + done + } + + changed_java_files | filter_protected > "$HITS_FILE" + + if [ -s "$HITS_FILE" ]; then echo "affected=true" >> "$GITHUB_OUTPUT" - printf '%b' "$HITS" > /tmp/protected-hits.txt fi - name: Comment on PR