From 4d375ddb9aa56b4ed115dde52b1d4ff4a1dc4744 Mon Sep 17 00:00:00 2001 From: "John E. Malmberg" Date: Mon, 1 Jun 2026 16:50:35 -0500 Subject: [PATCH] SRE-3607: Fix linter issue reporting Fix issue where a later linter success could prevent the reporting of an earlier linter failure. - .flake8: New file, that will allow longer than 79 character lines. - README.md: Documentation fixes. - bin/run-linters.sh: Fix reporting of issues. - bin/sync-consumer.sh: Add .flake8 to initial file list. - checks/linters/groovylint/check-implicit-bindings.sh: Fix false positive for environment block in Jenkinsfiles. - vscode-project-words.txt: Minor update. Signed-off-by: John E. Malmberg --- .flake8 | 2 + README.md | 52 +++++++++++++++--- bin/run-linters.sh | 48 ++++++---------- bin/sync-consumer.sh | 14 ++++- .../groovylint/check-implicit-bindings.sh | 55 ++++++++++++++++--- vscode-project-words.txt | 1 + 6 files changed, 124 insertions(+), 48 deletions(-) create mode 100644 .flake8 diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..7da1f96 --- /dev/null +++ b/.flake8 @@ -0,0 +1,2 @@ +[flake8] +max-line-length = 100 diff --git a/README.md b/README.md index 97e474a..9c6c8cf 100644 --- a/README.md +++ b/README.md @@ -69,8 +69,6 @@ Update to latest: ```bash git submodule update --remote code_checking -git add code_checking -git commit -m "Update code_checking submodule" ``` Consumer repositories keep: @@ -117,6 +115,25 @@ managed section. It also seeds baseline `.gitignore`, consumer root when those files are missing. Running `sync-consumer` means you do not need to run `setup-github-workflow.sh` separately. +After `sync-consumer` completes, commit all integration files in a single commit +using the site commit message standards from +[docs/git-commit-message-guidelines.md](docs/git-commit-message-guidelines.md). +Example commit message format: + +```text +TKT-XXXX: Add code-checking submodule integration + +Integrated code-checking as a top-level submodule to provide shared linting +and check scripts across the repository. Bootstraps baseline workflows, +pre-commit hooks configuration, and IDE settings. + +Consumer integration: +- Added .github/workflows with code-checking checks +- Configured pre-commit hooks to use code-checking entrypoints +- Seeded baseline configuration files (.yamllint, cspell.config.yaml, etc.) +- Updated README.md with code-checking managed section +``` + To skip README updates for a specific run: ```bash @@ -124,7 +141,8 @@ To skip README updates for a specific run: ``` For an initial consumer-repo integration commit after running -`sync-consumer`, stage and review these files: +`sync-consumer`, stage all of these files together and commit with a proper +commit message following the site standards: - `.github/workflows/` (may need to add newly created files instead) - `.gitignore` (if seeded) @@ -135,20 +153,36 @@ For an initial consumer-repo integration commit after running - `.yamllint` (if seeded) - `vscode-project-words.txt` (if seeded) +The `code_checking` submodule directory itself should also be staged: + +- `code_checking/` + The `code_checking` submodule was previously added. Changes inside that directory are not required for this integration commit. +Example staging commands: + ```bash -git add .github/workflows/ # May need to add newly added files instead. -git add .gitignore # seeded if missing +git add .github/workflows/ +git add .gitignore git add .gitmodules -git add .pre-commit-config.yaml # if setup-dev was run +git add .pre-commit-config.yaml +git add .pylint +git add .yamllint +git add code_checking +git add cspell.config.yaml git add README.md -git add cspell.config.yaml # seeded if missing -git add .yamllint # seeded if missing -git add vscode-project-words.txt # seeded if missing +git add vscode-project-words.txt ``` +Then commit with a proper message: + +```bash +git commit +``` + +This will open your editor to write a commit message following the guidelines. + Do not stage `code-checking-ref` for normal integration commits. An intentional validation PR may track it temporarily when testing a `code_checking` PR ref. diff --git a/bin/run-linters.sh b/bin/run-linters.sh index 9fceea3..32dbe80 100755 --- a/bin/run-linters.sh +++ b/bin/run-linters.sh @@ -123,34 +123,28 @@ for linter in "${REQUIRED_LINTERS[@]}"; do case "${linter}" in shellcheck) run_args=("${run_args_common[@]}") - if ! "${LIB_ROOT}/checks/linters/shellcheck/run.sh" "${run_args[@]}"; then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/shellcheck/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; groovylint) run_args=("${run_args_common[@]}") - if ! "${LIB_ROOT}/checks/linters/groovylint/run.sh" "${run_args[@]}"; then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/groovylint/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; markdownlint) run_args=("${run_args_common[@]}") - if ! "${LIB_ROOT}/checks/linters/markdownlint/run.sh" "${run_args[@]}"; - then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/markdownlint/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; yamllint) run_args=("${run_args_common[@]}") - if ! "${LIB_ROOT}/checks/linters/yamllint/run.sh" "${run_args[@]}"; then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/yamllint/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; python) run_args=("${run_args_common[@]}") - if ! "${LIB_ROOT}/checks/linters/python/run.sh" "${run_args[@]}"; then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/python/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; copyright) run_args=("${run_args_common[@]}") @@ -160,15 +154,13 @@ for linter in "${REQUIRED_LINTERS[@]}"; do run_args+=(--no-stage) fi fi - if ! "${LIB_ROOT}/checks/linters/copyright/run.sh" "${run_args[@]}"; then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/copyright/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; codespell) run_args=("${run_args_common[@]}") - if ! "${LIB_ROOT}/checks/linters/codespell/run.sh" "${run_args[@]}"; then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/codespell/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; text-hygiene) run_args=("${run_args_common[@]}") @@ -178,17 +170,13 @@ for linter in "${REQUIRED_LINTERS[@]}"; do run_args+=(--no-stage) fi fi - if ! "${LIB_ROOT}/checks/linters/text-hygiene/run.sh" "${run_args[@]}"; - then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/text-hygiene/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; filename-portability) run_args=("${run_args_common[@]}") - if ! "${LIB_ROOT}/checks/linters/filename-portability/run.sh" "${run_args[@]}"; - then - linter_rc=$? - fi + "${LIB_ROOT}/checks/linters/filename-portability/run.sh" "${run_args[@]}" \ + || linter_rc=$? ;; *) echo "Unknown linter selected: ${linter}" >&2 diff --git a/bin/sync-consumer.sh b/bin/sync-consumer.sh index 786b7d8..a2415dc 100755 --- a/bin/sync-consumer.sh +++ b/bin/sync-consumer.sh @@ -228,8 +228,8 @@ if [[ "${UPDATE_README}" == true ]]; then This repository uses the shared \`code_checking\` submodule. -- Framework documentation: [code_checking README](./${SUBMODULE_PATH}/README.md) -- Integration guide: +* Framework documentation: [code_checking README](./${SUBMODULE_PATH}/README.md) +* Integration guide: [code_checking integration](./${SUBMODULE_PATH}/docs/integration.md) ${END_MARKER}" @@ -307,4 +307,14 @@ if [[ ! -f "${TARGET_ROOT}/vscode-project-words.txt" ]]; then fi fi +if [[ ! -f "${TARGET_ROOT}/.flake8" ]]; then + FLAKE8_BASELINE="${LIB_ROOT}/.flake8" + if [[ -f "${FLAKE8_BASELINE}" ]]; then + cp "${FLAKE8_BASELINE}" "${TARGET_ROOT}/.flake8" + echo "[sync-consumer] created .flake8 from code_checking baseline" + else + echo "[sync-consumer] baseline not found: ${FLAKE8_BASELINE}" >&2 + fi +fi + echo "[sync-consumer] complete" diff --git a/checks/linters/groovylint/check-implicit-bindings.sh b/checks/linters/groovylint/check-implicit-bindings.sh index b605d08..8ea101c 100755 --- a/checks/linters/groovylint/check-implicit-bindings.sh +++ b/checks/linters/groovylint/check-implicit-bindings.sh @@ -21,13 +21,54 @@ for file_path in "$@"; do # Guard against Groovy script binding side effects from bare assignments like # `myVar = value`. In Jenkins pipelines these become shared script properties # and can cause nondeterministic behavior in parallel execution. - if grep -nE '^[[:space:]]*[A-Za-z_][A-Za-z0-9_]*[[:space:]]*=[^=~]' \ - "${file_path}" >/dev/null; then - echo "[groovylint] implicit script-binding assignment detected: ${file_path}" >&2 - grep -nE '^[[:space:]]*[A-Za-z_][A-Za-z0-9_]*[[:space:]]*=[^=~]' \ - "${file_path}" >&2 - violations=1 - fi + # Exclude assignments inside declarative `environment { }` blocks (legitimate). + violations_in_file=$(awk ' + BEGIN { in_environment = 0; env_brace_depth = 0 } + /environment[[:space:]]*\{/ { + in_environment = 1 + env_brace_depth += gsub(/\{/, "{") + env_brace_depth -= gsub(/\}/, "}") + next + } + in_environment { + env_brace_depth += gsub(/\{/, "{") + env_brace_depth -= gsub(/\}/, "}") + if (env_brace_depth <= 0) { + in_environment = 0 + } + next + } + !in_environment && /^[[:space:]]*[A-Za-z_][A-Za-z0-9_]*[[:space:]]*=[^=~]/ { + print NR": "$0 + exit 1 + } + END { exit 0 } + ' "${file_path}" 2>/dev/null) || { + if [[ -n "${violations_in_file}" ]]; then + echo "[groovylint] implicit script-binding assignment detected: ${file_path}" >&2 + awk ' + BEGIN { in_environment = 0; env_brace_depth = 0 } + /environment[[:space:]]*\{/ { + in_environment = 1 + env_brace_depth += gsub(/\{/, "{") + env_brace_depth -= gsub(/\}/, "}") + next + } + in_environment { + env_brace_depth += gsub(/\{/, "{") + env_brace_depth -= gsub(/\}/, "}") + if (env_brace_depth <= 0) { + in_environment = 0 + } + next + } + !in_environment && /^[[:space:]]*[A-Za-z_][A-Za-z0-9_]*[[:space:]]*=[^=~]/ { + print NR": "$0 + } + ' "${file_path}" >&2 + violations=1 + fi + } done if [[ ${violations} -ne 0 ]]; then diff --git a/vscode-project-words.txt b/vscode-project-words.txt index 392e5e1..16a9366 100644 --- a/vscode-project-words.txt +++ b/vscode-project-words.txt @@ -11,6 +11,7 @@ filemode gitfs gitlink groovylint +gsub gvars isort LASTEXITCODE