From f99c4bf3d6ff71aa7dc05f50018f9eefc8f9e068 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Mon, 3 Feb 2025 06:36:20 +0900 Subject: [PATCH] tools: Update tidy.sh and related configs --- .cspell.json | 15 +- .editorconfig | 4 +- tools/tidy.sh | 605 ++++++++++++++++++++++++++++++++++---------------- 3 files changed, 426 insertions(+), 198 deletions(-) diff --git a/.cspell.json b/.cspell.json index 5d32285..860713a 100644 --- a/.cspell.json +++ b/.cspell.json @@ -27,24 +27,25 @@ "ignoreRegExpList": [ // Copyright notice "Copyright .*", + "SPDX-(File|Snippet)CopyrightText: .*", // GHA actions/workflows - "uses: .+@", + "uses: .+@[\\w_.-]+", // GHA context (repo name, owner name, etc.) - "github.\\w+ (=|!)= '.+'", + "github.[\\w_.-]+ (=|!)= '[^']+'", // GH username "( |\\[)@[\\w_-]+", // Git config username - "git config user.name .*", - // Username in todo comment + "git config( --[^ ]+)? user.name .*", + // Username in TODO|FIXME comment "(TODO|FIXME)\\([\\w_., -]+\\)", // Cargo.toml authors - "authors *= *\\[.*\\]", - "\".* <[\\w_.+-]+@[\\w.-]+>\"" + "authors *= *\\[[^\\]]*\\]", + "\"[^\"]* <[\\w_.+-]+@[\\w.-]+>\"" ], "languageSettings": [ { "languageId": ["*"], - "dictionaries": ["bash", "rust"] + "dictionaries": ["bash", "cpp-refined", "rust"] } ], "ignorePaths": [] diff --git a/.editorconfig b/.editorconfig index 3aa2b38..98bc898 100644 --- a/.editorconfig +++ b/.editorconfig @@ -11,12 +11,14 @@ indent_style = space insert_final_newline = true trim_trailing_whitespace = true -[*.{json,md,rb,sh,yml,yaml}] +[*.{css,html,json,md,rb,sh,yml,yaml}] indent_size = 2 [*.{js,yml,yaml}] quote_type = single [*.sh] +# https://google.github.io/styleguide/shellguide.html#s5.3-pipelines binary_next_line = true +# https://google.github.io/styleguide/shellguide.html#s5.5-case-statement switch_case_indent = true diff --git a/tools/tidy.sh b/tools/tidy.sh index 284bf1b..54d4122 100755 --- a/tools/tidy.sh +++ b/tools/tidy.sh @@ -18,12 +18,46 @@ cd -- "$(dirname -- "$0")"/.. # - shfmt # - shellcheck # - cargo, rustfmt (if Rust code exists) -# - clang-format (if C/C++ code exists) +# - clang-format (if C/C++/Protobuf code exists) +# - parse-dockerfile (if Dockerfile exists) # # This script is shared with other repositories, so there may also be # checks for files not included in this repository, but they will be # skipped if the corresponding files do not exist. +retry() { + for i in {1..10}; do + if "$@"; then + return 0 + else + sleep "${i}" + fi + done + "$@" +} +error() { + if [[ -n "${GITHUB_ACTIONS:-}" ]]; then + printf '::error::%s\n' "$*" + else + printf >&2 'error: %s\n' "$*" + fi + should_fail=1 +} +warn() { + if [[ -n "${GITHUB_ACTIONS:-}" ]]; then + printf '::warning::%s\n' "$*" + else + printf >&2 'warning: %s\n' "$*" + fi +} +info() { + printf >&2 'info: %s\n' "$*" +} +print_fenced() { + printf '=======================================\n' + printf '%s' "$*" + printf '=======================================\n\n' +} check_diff() { if [[ -n "${CI:-}" ]]; then if ! git --no-pager diff --exit-code "$@"; then @@ -37,13 +71,13 @@ check_diff() { } check_config() { if [[ ! -e "$1" ]]; then - error "could not found $1 in the repository root" + error "could not found $1 in the repository root${2:-}" fi } check_install() { for tool in "$@"; do if ! type -P "${tool}" >/dev/null; then - if [[ "${tool}" == "python3" ]]; then + if [[ "${tool}" == 'python3' ]]; then if type -P python >/dev/null; then continue fi @@ -53,33 +87,29 @@ check_install() { fi done } -retry() { - for i in {1..10}; do - if "$@"; then - return 0 - else - sleep "${i}" - fi - done - "$@" -} -error() { - if [[ -n "${GITHUB_ACTIONS:-}" ]]; then - printf '::error::%s\n' "$*" - else - printf >&2 'error: %s\n' "$*" +check_unused() { + local kind="$1" + shift + local res + res=$(ls_files "$@") + if [[ -n "${res}" ]]; then + error "the following files are unused because there is no ${kind}; consider removing them" + print_fenced "${res}"$'\n' fi - should_fail=1 } -warn() { - if [[ -n "${GITHUB_ACTIONS:-}" ]]; then - printf '::warning::%s\n' "$*" - else - printf >&2 'warning: %s\n' "$*" +check_alt() { + local recommended=$1 + local not_recommended=$2 + if [[ -n "$3" ]]; then + error "please use ${recommended} instead of ${not_recommended} for consistency" + print_fenced "$3"$'\n' fi } -info() { - printf >&2 'info: %s\n' "$*" +check_hidden() { + local res + for file in "$@"; do + check_alt ".${file}" "${file}" "$(comm -23 <(ls_files "*${file}") <(ls_files "*.${file}"))" + done } sed_rhs_escape() { sed 's/\\/\\\\/g; s/\&/\\\&/g; s/\//\\\//g' <<<"$1" @@ -105,7 +135,7 @@ fi exe='' py_suffix='' if type -P python3 >/dev/null; then - py_suffix='3' + py_suffix=3 fi venv_bin=.venv/bin yq() { @@ -118,7 +148,7 @@ tomlq() { } case "$(uname -s)" in Linux) - if [[ "$(uname -o)" == "Android" ]]; then + if [[ "$(uname -o)" == 'Android' ]]; then ostype=android else ostype=linux @@ -130,25 +160,24 @@ case "$(uname -s)" in OpenBSD) ostype=openbsd ;; DragonFly) ostype=dragonfly ;; SunOS) - if [[ "$(/usr/bin/uname -o)" == "illumos" ]]; then + if [[ "$(/usr/bin/uname -o)" == 'illumos' ]]; then ostype=illumos else ostype=solaris # Solaris /usr/bin/* are not POSIX-compliant (e.g., grep has no -q, -E, -F), # and POSIX-compliant commands are in /usr/xpg{4,6,7}/bin. # https://docs.oracle.com/cd/E88353_01/html/E37853/xpg-7.html - if [[ "${PATH}" != *"/usr/xpg4/bin"* ]]; then + if [[ "${PATH}" != *'/usr/xpg4/bin'* ]]; then export PATH="/usr/xpg4/bin:${PATH}" fi # GNU/BSD grep/sed is required to run some checks, but most checks are okay with other POSIX grep/sed. # Solaris /usr/xpg4/bin/grep has -q, -E, -F, but no -o (non-POSIX). # Solaris /usr/xpg4/bin/sed has no -E (POSIX.1-2024) yet. - if type -P ggrep >/dev/null; then - grep() { ggrep "$@"; } - fi - if type -P gsed >/dev/null; then - sed() { gsed "$@"; } - fi + for tool in sed grep; do + if type -P "g${tool}" >/dev/null; then + eval "${tool}() { g${tool} \"\$@\"; }" + fi + done fi ;; MINGW* | MSYS* | CYGWIN* | Windows_NT) @@ -158,9 +187,9 @@ case "$(uname -s)" in if type -P jq >/dev/null; then # https://github.com/jqlang/jq/issues/1854 _tmp=$(jq -r .a <<<'{}') - if [[ "${_tmp}" != "null" ]]; then + if [[ "${_tmp}" != 'null' ]]; then _tmp=$(jq -b -r .a 2>/dev/null <<<'{}' || true) - if [[ "${_tmp}" == "null" ]]; then + if [[ "${_tmp}" == 'null' ]]; then jq() { command jq -b "$@"; } else jq() { command jq "$@" | tr -d '\r'; } @@ -195,7 +224,7 @@ while IFS=$'\n' read -r line; do exclude_from_ls_files_no_symlink+=("${line}"); git ls-files --deleted } | LC_ALL=C sort -u) ls_files() { - if [[ "${1:-}" == "--include-symlink" ]]; then + if [[ "${1:-}" == '--include-symlink' ]]; then shift comm -23 <(git ls-files "$@" | LC_ALL=C sort) <(printf '%s\n' ${exclude_from_ls_files_no_symlink[@]+"${exclude_from_ls_files_no_symlink[@]}"}) else @@ -206,7 +235,8 @@ ls_files() { # Rust (if exists) if [[ -n "$(ls_files '*.rs')" ]]; then info "checking Rust code style" - check_config .rustfmt.toml + check_config .rustfmt.toml "; consider adding with reference to https://github.com/taiki-e/cargo-hack/blob/HEAD/.rustfmt.toml" + check_config .clippy.toml "; consider adding with reference to https://github.com/taiki-e/cargo-hack/blob/HEAD/.clippy.toml" if check_install cargo jq python3; then # `cargo fmt` cannot recognize files not included in the current workspace and modules # defined inside macros, so run rustfmt directly. @@ -231,31 +261,6 @@ if [[ -n "$(ls_files '*.rs')" ]]; then error "please replace \`.cast()\` with \`.cast::()\`:" printf '%s\n' "${cast_without_turbofish}" fi - # Sync readme and crate-level doc. - first=1 - for readme in $(ls_files '*README.md'); do - if ! grep -Eq '^' "${readme}"; then - continue - fi - lib="$(dirname -- "${readme}")/src/lib.rs" - if [[ -n "${first}" ]]; then - first='' - info "checking readme and crate-level doc are synchronized" - fi - if ! grep -Eq '^' "${readme}"; then - bail "missing '' comment in ${readme}" - fi - if ! grep -Eq '^' "${lib}"; then - bail "missing '' comment in ${lib}" - fi - if ! grep -Eq '^' "${lib}"; then - bail "missing '' comment in ${lib}" - fi - new=$(tr '\n' '\a' <"${readme}" | grep -Eo '.*') - new=$(tr '\n' '\a' <"${lib}" | sed "s/.*/$(sed_rhs_escape "${new}")/" | tr '\a' '\n') - printf '%s\n' "${new}" >|"${lib}" - check_diff "${lib}" - done # Make sure that public Rust crates don't contain executables and binaries. executables='' binaries='' @@ -269,7 +274,7 @@ if [[ -n "$(ls_files '*.rs')" ]]; then has_root_crate='' for pkg in $(jq -c '. as $metadata | .workspace_members[] as $id | $metadata.packages[] | select(.id == $id)' <<<"${metadata}"); do eval "$(jq -r '@sh "publish=\(.publish) manifest_path=\(.manifest_path)"' <<<"${pkg}")" - if [[ "$(tomlq -c '.lints' "${manifest_path}")" == "null" ]]; then + if [[ "$(tomlq -c '.lints' "${manifest_path}")" == 'null' ]]; then error "no [lints] table in ${manifest_path} please add '[lints]' with 'workspace = true'" fi # Publishing is unrestricted if null, and forbidden if an empty array. @@ -292,6 +297,7 @@ if [[ -n "$(ls_files '*.rs')" ]]; then fi done if [[ -n "${has_public_crate}" ]]; then + check_config .deny.toml "; consider adding with reference to https://github.com/taiki-e/cargo-hack/blob/HEAD/.deny.toml" info "checking public crates don't contain executables and binaries" for p in $(ls_files --include-symlink); do # Skip directories. @@ -321,26 +327,145 @@ if [[ -n "$(ls_files '*.rs')" ]]; then done if [[ -n "${executables}" ]]; then error "file-permissions-check failed: executables are only allowed to be present in directories that are excluded from crates.io" - printf '=======================================\n' - printf '%s' "${executables}" - printf '=======================================\n' + print_fenced "${executables}" fi if [[ -n "${binaries}" ]]; then error "file-permissions-check failed: binaries are only allowed to be present in directories that are excluded from crates.io" - printf '=======================================\n' - printf '%s' "${binaries}" - printf '=======================================\n' + print_fenced "${binaries}" fi fi fi -elif [[ -e .rustfmt.toml ]]; then - error ".rustfmt.toml is unused" + # Sync markdown to rustdoc. + first=1 + for markdown in $(ls_files '*.md'); do + markers=$(grep -En '^' "${markdown}" || true) + # BSD wc's -l emits spaces before number. + if [[ ! "$(LC_ALL=C wc -l <<<"${markers}")" =~ ^\ *2$ ]]; then + if [[ -n "${markers}" ]]; then + error "inconsistent '' marker found in ${markdown}" + printf '%s\n' "${markers}" + fi + continue + fi + start_marker=$(head -n1 <<<"${markers}") + end_marker=$(head -n2 <<<"${markers}" | tail -n1) + if [[ "${start_marker}" == *"tidy:sync-markdown-to-rustdoc:end"* ]] || [[ "${end_marker}" == *"tidy:sync-markdown-to-rustdoc:start"* ]]; then + error "inconsistent '' marker found in ${markdown}" + printf '%s\n' "${markers}" + continue + fi + if [[ -n "${first}" ]]; then + first='' + info "syncing markdown to rustdoc" + fi + lib="${start_marker#*:<\!-- tidy:sync-markdown-to-rustdoc:start:}" + if [[ "${start_marker}" == "${lib}" ]]; then + error "missing path in '' marker in ${markdown}" + printf '%s\n' "${markers}" + continue + fi + lib="${lib% -->}" + lib="$(dirname -- "${markdown}")/${lib}" + markers=$(grep -En '^' "${lib}" || true) + # BSD wc's -l emits spaces before number. + if [[ ! "$(LC_ALL=C wc -l <<<"${markers}")" =~ ^\ *2$ ]]; then + if [[ -n "${markers}" ]]; then + error "inconsistent '' marker found in ${lib}" + printf '%s\n' "${markers}" + else + error "missing '' marker in ${lib}" + fi + continue + fi + start_marker=$(head -n1 <<<"${markers}") + end_marker=$(head -n2 <<<"${markers}" | tail -n1) + if [[ "${start_marker}" == *"tidy:sync-markdown-to-rustdoc:end"* ]] || [[ "${end_marker}" == *"tidy:sync-markdown-to-rustdoc:start"* ]]; then + error "inconsistent '' marker found in ${lib}" + printf '%s\n' "${markers}" + continue + fi + new=''$'\a' + empty_line_re='^ *$' + gfm_alert_re='^> {0,4}\[!.*\] *$' + rust_code_block_re='^ *```(rust|rs) *$' + code_block_attr='' + in_alert='' + first_line=1 + ignore='' + while IFS='' read -rd$'\a' line; do + if [[ -n "${ignore}" ]]; then + if [[ "${line}" == ''* ]]; then + ignore='' + fi + continue + fi + if [[ -n "${first_line}" ]]; then + # Ignore start marker. + first_line='' + continue + elif [[ -n "${in_alert}" ]]; then + if [[ "${line}" =~ ${empty_line_re} ]]; then + in_alert='' + new+=$'\a'""$'\a' + fi + elif [[ "${line}" =~ ${gfm_alert_re} ]]; then + alert="${line#*[\!}" + alert="${alert%%]*}" + alert=$(tr '[:lower:]' '[:upper:]' <<<"${alert%%]*}") + alert_lower=$(tr '[:upper:]' '[:lower:]' <<<"${alert}") + case "${alert}" in + NOTE | TIP | IMPORTANT) alert_sign='ⓘ' ;; + WARNING | CAUTION) alert_sign='⚠' ;; + *) + error "unknown alert type '${alert}' found; please use one of the types listed in " + new+="${line}"$'\a' + continue + ;; + esac + in_alert=1 + new+="
"$'\a\a' + new+="> **${alert_sign} ${alert:0:1}${alert_lower:1}**"$'\a>\a' + continue + fi + if [[ "${line}" =~ ${rust_code_block_re} ]]; then + code_block_attr="${code_block_attr#<\!-- tidy:sync-markdown-to-rustdoc:code-block:}" + code_block_attr="${code_block_attr%% -->*}" + new+="${line/\`\`\`*/\`\`\`}${code_block_attr}"$'\a' + code_block_attr='' + continue + fi + if [[ -n "${code_block_attr}" ]]; then + error "'${code_block_attr}' ignored because there is no subsequent Rust code block" + code_block_attr='' + fi + if [[ "${line}" == ''* ]]; then + code_block_attr="${line}" + continue + fi + if [[ "${line}" == ''* ]]; then + if [[ "${new}" == *$'\a\a' ]]; then + new="${new%$'\a'}" + fi + ignore=1 + continue + fi + new+="${line}"$'\a' + done < <(tr '\n' '\a' <"${markdown}" | grep -Eo '.*') + new+='' + new=$(tr '\n' '\a' <"${lib}" | sed "s/.*/$(sed_rhs_escape "${new}")/" | tr '\a' '\n') + printf '%s\n' "${new}" >|"${lib}" + check_diff "${lib}" + done + printf '\n' +else + check_unused "Rust code" '*.cargo*' '*clippy.toml' '*deny.toml' '*rustfmt.toml' '*Cargo.toml' '*Cargo.lock' fi +check_hidden clippy.toml deny.toml rustfmt.toml -# C/C++ (if exists) -clang_format_ext=('*.c' '*.h' '*.cpp' '*.hpp') +# C/C++/Protobuf (if exists) +clang_format_ext=('*.c' '*.h' '*.cpp' '*.hpp' '*.proto') if [[ -n "$(ls_files "${clang_format_ext[@]}")" ]]; then - info "checking C/C++ code style" + info "checking C/C++/Protobuf code style" check_config .clang-format if check_install clang-format; then IFS=' ' @@ -349,31 +474,21 @@ if [[ -n "$(ls_files "${clang_format_ext[@]}")" ]]; then clang-format -i $(ls_files "${clang_format_ext[@]}") check_diff $(ls_files "${clang_format_ext[@]}") fi -elif [[ -e .clang-format ]]; then - error ".clang-format is unused" + printf '\n' +else + check_unused "C/C++/Protobuf code" '*.clang-format*' fi +check_alt '.clang-format' '_clang-format' "$(ls_files '*_clang-format')" # https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html -cpp_alt_ext=('*.cc' '*.cp' '*.cxx' '*.C' '*.CPP' '*.c++') -hpp_alt_ext=('*.hh' '*.hp' '*.hxx' '*.H' '*.HPP' '*.h++') -if [[ -n "$(ls_files "${cpp_alt_ext[@]}")" ]]; then - error "please use '.cpp' for consistency" - printf '=======================================\n' - ls_files "${cpp_alt_ext[@]}" - printf '=======================================\n' -fi -if [[ -n "$(ls_files "${hpp_alt_ext[@]}")" ]]; then - error "please use '.hpp' for consistency" - printf '=======================================\n' - ls_files "${hpp_alt_ext[@]}" - printf '=======================================\n' -fi +check_alt '.cpp extension' 'other extensions' "$(ls_files '*.cc' '*.cp' '*.cxx' '*.C' '*.CPP' '*.c++')" +check_alt '.hpp extension' 'other extensions' "$(ls_files '*.hh' '*.hp' '*.hxx' '*.H' '*.HPP' '*.h++')" -# YAML/JavaScript/JSON (if exists) -prettier_ext=('*.yml' '*.yaml' '*.js' '*.json') +# YAML/HTML/CSS/JavaScript/JSON (if exists) +prettier_ext=('*.css' '*.html' '*.js' '*.json' '*.yml' '*.yaml') if [[ -n "$(ls_files "${prettier_ext[@]}")" ]]; then - info "checking YAML/JavaScript/JSON code style" + info "checking YAML/HTML/CSS/JavaScript/JSON code style" check_config .editorconfig - if [[ "${ostype}" == "solaris" ]] && [[ -n "${CI:-}" ]] && ! type -P npm >/dev/null; then + if [[ "${ostype}" == 'solaris' ]] && [[ -n "${CI:-}" ]] && ! type -P npm >/dev/null; then warn "this check is skipped on Solaris due to no node 18+ in upstream package manager" elif check_install npm; then IFS=' ' @@ -382,34 +497,36 @@ if [[ -n "$(ls_files "${prettier_ext[@]}")" ]]; then npx -y prettier -l -w $(ls_files "${prettier_ext[@]}") check_diff $(ls_files "${prettier_ext[@]}") fi + printf '\n' +else + check_unused "YAML/HTML/CSS/JavaScript/JSON file" '*.prettierignore' fi -if [[ -n "$(ls_files '*.yaml' | { grep -Fv '.markdownlint-cli2.yaml' || true; })" ]]; then - error "please use '.yml' instead of '.yaml' for consistency" - printf '=======================================\n' - ls_files '*.yaml' | { grep -Fv '.markdownlint-cli2.yaml' || true; } - printf '=======================================\n' -fi +# https://prettier.io/docs/en/configuration +check_alt '.editorconfig' 'other configs' "$(ls_files '*.prettierrc*' '*prettier.config.*')" +check_alt '.yml extension' '.yaml extension' "$(ls_files '*.yaml' | { grep -Fv '.markdownlint-cli2.yaml' || true; })" # TOML (if exists) if [[ -n "$(ls_files '*.toml' | { grep -Fv '.taplo.toml' || true; })" ]]; then info "checking TOML style" check_config .taplo.toml - if [[ "${ostype}" == "solaris" ]] && [[ -n "${CI:-}" ]] && ! type -P npm >/dev/null; then + if [[ "${ostype}" == 'solaris' ]] && [[ -n "${CI:-}" ]] && ! type -P npm >/dev/null; then warn "this check is skipped on Solaris due to no node 18+ in upstream package manager" elif check_install npm; then info "running \`npx -y @taplo/cli fmt \$(git ls-files '*.toml')\`" RUST_LOG=warn npx -y @taplo/cli fmt $(ls_files '*.toml') check_diff $(ls_files '*.toml') fi -elif [[ -e .taplo.toml ]]; then - error ".taplo.toml is unused" + printf '\n' +else + check_unused "TOML file" '*taplo.toml' fi +check_hidden taplo.toml # Markdown (if exists) if [[ -n "$(ls_files '*.md')" ]]; then - info "checking Markdown style" + info "checking markdown style" check_config .markdownlint-cli2.yaml - if [[ "${ostype}" == "solaris" ]] && [[ -n "${CI:-}" ]] && ! type -P npm >/dev/null; then + if [[ "${ostype}" == 'solaris' ]] && [[ -n "${CI:-}" ]] && ! type -P npm >/dev/null; then warn "this check is skipped on Solaris due to no node 18+ in upstream package manager" elif check_install npm; then info "running \`npx -y markdownlint-cli2 \$(git ls-files '*.md')\`" @@ -417,18 +534,16 @@ if [[ -n "$(ls_files '*.md')" ]]; then error "check failed; please resolve the above markdownlint error(s)" fi fi -elif [[ -e .markdownlint-cli2.yaml ]]; then - error ".markdownlint-cli2.yaml is unused" -fi -if [[ -n "$(ls_files '*.markdown')" ]]; then - error "please use '.md' instead of '.markdown' for consistency" - printf '=======================================\n' - ls_files '*.markdown' - printf '=======================================\n' + printf '\n' +else + check_unused "markdown file" '*.markdownlint-cli2.yaml' fi +# https://github.com/DavidAnson/markdownlint-cli2#configuration +check_alt '.markdownlint-cli2.yaml' 'other configs' "$(ls_files '*.markdownlint-cli2.jsonc' '*.markdownlint-cli2.cjs' '*.markdownlint-cli2.mjs' '*.markdownlint.*')" +check_alt '.md extension' '*.markdown extension' "$(ls_files '*.markdown')" # Shell scripts -info "checking Shell scripts" +info "checking shell scripts" shell_files=() docker_files=() bash_files=() @@ -436,12 +551,13 @@ grep_ere_files=() sed_ere_files=() for p in $(ls_files '*.sh' '*Dockerfile*'); do case "${p}" in - tests/fixtures/* | */tests/fixtures/*) continue ;; + tests/fixtures/* | */tests/fixtures/* | *.json) continue ;; esac case "${p##*/}" in *.sh) shell_files+=("${p}") - if [[ "$(head -1 "${p}")" =~ ^#!/.*bash ]]; then + re='^#!/.*bash' + if [[ "$(head -1 "${p}")" =~ ${re} ]]; then bash_files+=("${p}") fi ;; @@ -457,7 +573,6 @@ for p in $(ls_files '*.sh' '*Dockerfile*'); do sed_ere_files+=("${p}") fi done -# TODO: .cirrus.yml workflows=() actions=() if [[ -d .github/workflows ]]; then @@ -468,7 +583,7 @@ if [[ -d .github/workflows ]]; then fi if [[ -n "$(ls_files '*action.yml')" ]]; then for p in $(ls_files '*action.yml'); do - if [[ "${p##*/}" == "action.yml" ]]; then + if [[ "${p##*/}" == 'action.yml' ]]; then actions+=("${p}") if ! grep -Fq 'shell: sh' "${p}"; then bash_files+=("${p}") @@ -480,54 +595,40 @@ fi res=$({ grep -En '(\[\[ .* ]]|(^|[^\$])\(\(.*\)\))( +#| *$)' "${bash_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "bare [[ ]] and (( )) may not work as intended: see https://github.com/koalaman/shellcheck/issues/2360 for more" - printf '=======================================\n' - printf '%s\n' "${res}" - printf '=======================================\n' + print_fenced "${res}"$'\n' fi # TODO: chmod|chown res=$({ grep -En '(^|[^0-9A-Za-z\."'\''-])(basename|cat|cd|cp|dirname|ln|ls|mkdir|mv|pushd|rm|rmdir|tee|touch)( +-[0-9A-Za-z]+)* +[^<>\|-]' "${bash_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "use \`--\` before path(s): see https://github.com/koalaman/shellcheck/issues/2707 / https://github.com/koalaman/shellcheck/issues/2612 / https://github.com/koalaman/shellcheck/issues/2305 / https://github.com/koalaman/shellcheck/issues/2157 / https://github.com/koalaman/shellcheck/issues/2121 / https://github.com/koalaman/shellcheck/issues/314 for more" - printf '=======================================\n' - printf '%s\n' "${res}" - printf '=======================================\n' + print_fenced "${res}"$'\n' fi res=$({ grep -En '(^|[^0-9A-Za-z\."'\''-])(LINES|RANDOM|PWD)=' "${bash_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "do not modify these built-in bash variables: see https://github.com/koalaman/shellcheck/issues/2160 / https://github.com/koalaman/shellcheck/issues/2559 for more" - printf '=======================================\n' - printf '%s\n' "${res}" - printf '=======================================\n' + print_fenced "${res}"$'\n' fi # perf res=$({ grep -En '(^|[^\\])\$\((cat) ' "${bash_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "use faster \`\$(' "${bash_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "\`type -P\` doesn't output to stderr; use \`>\` instead of \`&>\`" - printf '=======================================\n' - printf '%s\n' "${res}" - printf '=======================================\n' + print_fenced "${res}"$'\n' fi # TODO: multi-line case res=$({ grep -En '(^|[^0-9A-Za-z\."'\''-])(echo|printf )[^;)]* \|[^\|]' "${bash_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "use faster \`<<<...\` instead of \`echo ... |\`/\`printf ... |\`: see https://github.com/koalaman/shellcheck/issues/2593 for more" - printf '=======================================\n' - printf '%s\n' "${res}" - printf '=======================================\n' + print_fenced "${res}"$'\n' fi # style if [[ ${#grep_ere_files[@]} -gt 0 ]]; then @@ -536,18 +637,14 @@ if [[ ${#grep_ere_files[@]} -gt 0 ]]; then res=$({ grep -En '(^|[^0-9A-Za-z\."'\''-])(grep) +([^-]|-[^EFP-]|--[^hv])' "${grep_ere_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "please always use ERE (grep -E) instead of BRE for code consistency within a file" - printf '=======================================\n' - printf '%s\n' "${res}" - printf '=======================================\n' + print_fenced "${res}"$'\n' fi fi if [[ ${#sed_ere_files[@]} -gt 0 ]]; then res=$({ grep -En '(^|[^0-9A-Za-z\."'\''-])(sed) +([^-]|-[^E-]|--[^hv])' "${sed_ere_files[@]}" || true; } | { grep -Ev '^[^ ]+: *(#|//)' || true; } | LC_ALL=C sort) if [[ -n "${res}" ]]; then error "please always use ERE (sed -E) instead of BRE for code consistency within a file" - printf '=======================================\n' - printf '%s\n' "${res}" - printf '=======================================\n' + print_fenced "${res}"$'\n' fi fi if check_install shfmt; then @@ -558,7 +655,7 @@ if check_install shfmt; then fi check_diff "${shell_files[@]}" fi -if [[ "${ostype}" == "solaris" ]] && [[ -n "${CI:-}" ]] && ! type -P shellcheck >/dev/null; then +if [[ "${ostype}" == 'solaris' ]] && [[ -n "${CI:-}" ]] && ! type -P shellcheck >/dev/null; then warn "this check is skipped on Solaris due to no haskell/shellcheck in upstream package manager" elif check_install shellcheck; then check_config .shellcheckrc @@ -568,12 +665,132 @@ elif check_install shellcheck; then fi # Check scripts in dockerfile. if [[ ${#docker_files[@]} -gt 0 ]]; then - # SC2154 doesn't seem to work for ENV/ARG. - # SC2250 may not correct for ENV because $v and ${v} is sometime different: https://github.com/moby/moby/issues/42863 - shellcheck_exclude=SC2154,SC2250 - info "running \`shellcheck --shell bash --exclude ${shellcheck_exclude} \$(git ls-files '*Dockerfile*')\`" - if ! shellcheck --shell bash --exclude "${shellcheck_exclude}" "${docker_files[@]}"; then - error "check failed; please resolve the above shellcheck error(s)" + # Exclude SC2096 due to the way the temporary script is created. + shellcheck_exclude=SC2096 + info "running \`shellcheck --exclude ${shellcheck_exclude}\` for scripts in \$(git ls-files '*Dockerfile*')\`" + if [[ "${ostype}" == 'windows' ]]; then + # No such file or directory: '/proc/N/fd/N' + warn "this check is skipped on Windows due to upstream bug (failed to found fd created by <())" + elif [[ "${ostype}" == 'dragonfly' ]]; then + warn "this check is skipped on DragonFly BSD due to upstream bug (hang)" + elif check_install jq python3 parse-dockerfile; then + shellcheck_for_dockerfile() { + local text=$1 + local shell=$2 + local display_path=$3 + if [[ "${text}" == 'null' ]]; then + return + fi + text="#!${shell}"$'\n'"${text}" + case "${ostype}" in + windows) text=${text//\r/} ;; + esac + local color=auto + if [[ -t 1 ]] || [[ -n "${GITHUB_ACTIONS:-}" ]]; then + color=always + fi + if ! shellcheck --color="${color}" --exclude "${shellcheck_exclude}" <(printf '%s\n' "${text}") | sed "s/\/dev\/fd\/[0-9][0-9]*/$(sed_rhs_escape "${display_path}")/g"; then + error "check failed; please resolve the above shellcheck error(s)" + fi + } + for dockerfile_path in ${docker_files[@]+"${docker_files[@]}"}; do + dockerfile=$(parse-dockerfile "${dockerfile_path}") + normal_shell='' + for instruction in $(jq -c '.instructions[]' <<<"${dockerfile}"); do + instruction_kind=$(jq -r '.kind' <<<"${instruction}") + case "${instruction_kind}" in + FROM) + # https://docs.docker.com/reference/dockerfile/#from + # > Each FROM instruction clears any state created by previous instructions. + normal_shell='' + continue + ;; + ADD | ARG | CMD | COPY | ENTRYPOINT | ENV | EXPOSE | HEALTHCHECK | LABEL) ;; + # https://docs.docker.com/reference/build-checks/maintainer-deprecated/ + MAINTAINER) error "MAINTAINER instruction is deprecated in favor of using label" ;; + RUN) ;; + SHELL) + normal_shell='' + for argument in $(jq -c '.arguments[]' <<<"${instruction}"); do + value=$(jq -r '.value' <<<"${argument}") + if [[ -z "${normal_shell}" ]]; then + case "${value}" in + cmd | cmd.exe | powershell | powershell.exe) + # not unix shell + normal_shell="${value}" + break + ;; + esac + else + normal_shell+=' ' + fi + normal_shell+="${value}" + done + ;; + STOPSIGNAL | USER | VOLUME | WORKDIR) ;; + *) error "unknown instruction ${instruction_kind}" ;; + esac + arguments='' + # only shell-form RUN/ENTRYPOINT/CMD is run in a shell + case "${instruction_kind}" in + RUN) + if [[ "$(jq -r '.arguments.shell' <<<"${instruction}")" == 'null' ]]; then + continue + fi + arguments=$(jq -r '.arguments.shell.value' <<<"${instruction}") + if [[ -z "${arguments}" ]]; then + if [[ "$(jq -r '.here_docs[0]' <<<"${instruction}")" == 'null' ]]; then + error "empty RUN is useless (${dockerfile_path})" + continue + fi + if [[ "$(jq -r '.here_docs[1]' <<<"${instruction}")" != 'null' ]]; then + # TODO: + error "multi here-docs without command is not yet supported (${dockerfile_path})" + fi + arguments=$(jq -r '.here_docs[0].value' <<<"${instruction}") + if [[ "${arguments}" == '#!'* ]]; then + # TODO: + error "here-docs with shebang is not yet supported (${dockerfile_path})" + continue + fi + else + if [[ "$(jq -r '.here_docs[0]' <<<"${instruction}")" != 'null' ]]; then + # TODO: + error "sh/bash command with here-docs is not yet checked (${dockerfile_path})" + fi + fi + ;; + ENTRYPOINT | CMD) + if [[ "$(jq -r '.arguments.shell' <<<"${instruction}")" == 'null' ]]; then + continue + fi + arguments=$(jq -r '.arguments.shell.value' <<<"${instruction}") + if [[ -z "${normal_shell}" ]] && [[ -n "${arguments}" ]]; then + # https://docs.docker.com/reference/build-checks/json-args-recommended/ + error "JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals" + fi + ;; + HEALTHCHECK) + if [[ "$(jq -r '.arguments.kind' <<<"${instruction}")" != "CMD" ]]; then + continue + fi + if [[ "$(jq -r '.arguments.arguments.shell' <<<"${instruction}")" == 'null' ]]; then + continue + fi + arguments=$(jq -r '.arguments.arguments.shell.value' <<<"${instruction}") + ;; + *) continue ;; + esac + case "${normal_shell}" in + # not unix shell + cmd | cmd.exe | powershell | powershell.exe) continue ;; + # https://docs.docker.com/reference/dockerfile/#shell + '') shell='/bin/sh -c' ;; + *) shell="${normal_shell}" ;; + esac + shellcheck_for_dockerfile "${arguments}" "${shell}" "${dockerfile_path}" + done + done fi fi # Check scripts in YAML. @@ -581,17 +798,17 @@ elif check_install shellcheck; then # Exclude SC2096 due to the way the temporary script is created. shellcheck_exclude=SC2086,SC2096,SC2129 info "running \`shellcheck --exclude ${shellcheck_exclude}\` for scripts in .github/workflows/*.yml and **/action.yml" - if [[ "${ostype}" == "windows" ]]; then + if [[ "${ostype}" == 'windows' ]]; then # No such file or directory: '/proc/N/fd/N' warn "this check is skipped on Windows due to upstream bug (failed to found fd created by <())" - elif [[ "${ostype}" == "dragonfly" ]]; then + elif [[ "${ostype}" == 'dragonfly' ]]; then warn "this check is skipped on DragonFly BSD due to upstream bug (hang)" elif check_install jq python3; then shellcheck_for_gha() { local text=$1 local shell=$2 local display_path=$3 - if [[ "${text}" == "null" ]]; then + if [[ "${text}" == 'null' ]]; then return fi case "${shell}" in @@ -631,7 +848,8 @@ EOF esac default_shell=$(jq -r -c '.defaults.run.shell' <<<"${workflow}") # github's default is https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#defaultsrunshell - if [[ ! "${default_shell}" =~ ^bash\ --noprofile\ --norc\ -CeEux?o\ pipefail\ \{0}$ ]]; then + re='^bash --noprofile --norc -CeEux?o pipefail \{0}$' + if [[ ! "${default_shell}" =~ ${re} ]]; then error "${workflow_path}: defaults.run.shell should be 'bash --noprofile --norc -CeEuxo pipefail {0}' or 'bash --noprofile --norc -CeEuo pipefail {0}'" continue fi @@ -641,17 +859,17 @@ EOF job=$(jq -r '.value' <<<"${job}") n=0 job_default_shell=$(jq -r '.defaults.run.shell' <<<"${job}") - if [[ "${job_default_shell}" == "null" ]]; then + if [[ "${job_default_shell}" == 'null' ]]; then job_default_shell="${default_shell}" fi for step in $(jq -c '.steps[]' <<<"${job}"); do prepare='' eval "$(jq -r 'if .run then @sh "RUN=\(.run) shell=\(.shell)" else @sh "RUN=\(.with.run) prepare=\(.with.prepare) shell=\(.with.shell)" end' <<<"${step}")" - if [[ "${RUN}" == "null" ]]; then + if [[ "${RUN}" == 'null' ]]; then _=$((n++)) continue fi - if [[ "${shell}" == "null" ]]; then + if [[ "${shell}" == 'null' ]]; then if [[ -z "${prepare}" ]]; then shell="${job_default_shell}" elif grep -Eq '^ *chsh +-s +[^ ]+/bash' <<<"${prepare}"; then @@ -675,11 +893,11 @@ EOF for step in $(jq -c '.steps[]' <<<"${runs}"); do prepare='' eval "$(jq -r 'if .run then @sh "RUN=\(.run) shell=\(.shell)" else @sh "RUN=\(.with.run) prepare=\(.with.prepare) shell=\(.with.shell)" end' <<<"${step}")" - if [[ "${RUN}" == "null" ]]; then + if [[ "${RUN}" == 'null' ]]; then _=$((n++)) continue fi - if [[ "${shell}" == "null" ]]; then + if [[ "${shell}" == 'null' ]]; then if [[ -z "${prepare}" ]]; then error "\`shell: ..\` is required" continue @@ -697,6 +915,8 @@ EOF fi fi fi +printf '\n' +check_alt '.sh extension' '*.bash extension' "$(ls_files '*.bash')" # License check # TODO: This check is still experimental and does not track all files that should be tracked. @@ -706,9 +926,10 @@ if [[ -f tools/.tidy-check-license-headers ]]; then for p in $(comm -12 <(eval $(/dev/null; then + if [[ "${ostype}" == 'solaris' ]] && [[ -n "${CI:-}" ]] && ! type -P npm >/dev/null; then warn "this check is skipped on Solaris due to no node 18+ in upstream package manager" - elif [[ "${ostype}" == "illumos" ]]; then + elif [[ "${ostype}" == 'illumos' ]]; then warn "this check is skipped on illumos due to upstream bug (dictionaries are not loaded correctly)" elif check_install npm jq python3; then has_rust='' @@ -754,7 +975,7 @@ if [[ -f .cspell.json ]]; then has_rust=1 dependencies='' for manifest_path in $(ls_files '*Cargo.toml'); do - if [[ "${manifest_path}" != "Cargo.toml" ]] && [[ "$(tomlq -c '.workspace' "${manifest_path}")" == "null" ]]; then + if [[ "${manifest_path}" != "Cargo.toml" ]] && [[ "$(tomlq -c '.workspace' "${manifest_path}")" == 'null' ]]; then continue fi m=$(cargo metadata --format-version=1 --no-deps --manifest-path "${manifest_path}" || true) @@ -766,14 +987,14 @@ if [[ -f .cspell.json ]]; then dependencies=$(LC_ALL=C sort -f -u <<<"${dependencies//[0-9_-]/$'\n'}") fi config_old=$(<.cspell.json) - config_new=$(grep -Ev '^ *//' <<<"${config_old}" | jq 'del(.dictionaries[] | select(index("organization-dictionary") | not)) | del(.dictionaryDefinitions[] | select(.name == "organization-dictionary" | not))') + config_new=$({ grep -Ev '^ *//' <<<"${config_old}" || true; } | jq 'del(.dictionaries[] | select(index("organization-dictionary") | not)) | del(.dictionaryDefinitions[] | select(.name == "organization-dictionary" | not))') trap -- 'printf "%s\n" "${config_old}" >|.cspell.json; printf >&2 "%s\n" "${0##*/}: trapped SIGINT"; exit 1' SIGINT printf '%s\n' "${config_new}" >|.cspell.json dependencies_words='' if [[ -n "${has_rust}" ]]; then dependencies_words=$(npx -y cspell stdin --no-progress --no-summary --words-only --unique <<<"${dependencies}" || true) fi - all_words=$(npx -y cspell --no-progress --no-summary --words-only --unique $(ls_files | { grep -Fv "${project_dictionary}" || true; }) || true) + all_words=$(ls_files | { grep -Fv "${project_dictionary}" || true; } | npx -y cspell --file-list stdin --no-progress --no-summary --words-only --unique || true) printf '%s\n' "${config_old}" >|.cspell.json trap -- 'printf >&2 "%s\n" "${0##*/}: trapped SIGINT"; exit 1' SIGINT cat >|.github/.cspell/rust-dependencies.txt <>.github/.cspell/rust-dependencies.txt <<<"${dependencies_words}"$'\n' fi + if [[ -z "${CI:-}" ]]; then + REMOVE_UNUSED_WORDS=1 + fi if [[ -z "${REMOVE_UNUSED_WORDS:-}" ]]; then check_diff .github/.cspell/rust-dependencies.txt fi @@ -790,11 +1014,11 @@ EOF error "you may want to mark .github/.cspell/rust-dependencies.txt linguist-generated" fi - info "running \`npx -y cspell --no-progress --no-summary \$(git ls-files)\`" - if ! npx -y cspell --no-progress --no-summary $(ls_files); then + info "running \`git ls-files | npx -y cspell --file-list stdin --no-progress --no-summary\`" + if ! ls_files | npx -y cspell --file-list stdin --no-progress --no-summary; then error "spellcheck failed: please fix uses of below words or add to ${project_dictionary} if correct" printf '=======================================\n' - { npx -y cspell --no-progress --no-summary --words-only $(ls_files) || true; } | LC_ALL=C sort -f -u + { ls_files | npx -y cspell --file-list stdin --no-progress --no-summary --words-only || true; } | sed "s/'s$//g" | LC_ALL=C sort -f -u printf '=======================================\n\n' fi @@ -810,40 +1034,41 @@ EOF esac if [[ -n "${dup}" ]]; then error "duplicated words in dictionaries; please remove the following words from ${project_dictionary}" - printf '=======================================\n' - printf '%s\n' "${dup}" - printf '=======================================\n\n' + print_fenced "${dup}"$'\n' fi done # Make sure the project-specific dictionary does not contain unused words. if [[ -n "${REMOVE_UNUSED_WORDS:-}" ]]; then grep_args=() - for word in $(grep -Ev '^//.*' "${project_dictionary}" || true); do + for word in $(grep -Ev '^//' "${project_dictionary}" || true); do if ! grep -Eqi "^${word}$" <<<"${all_words}"; then grep_args+=(-e "^${word}$") fi done if [[ ${#grep_args[@]} -gt 0 ]]; then info "removing unused words from ${project_dictionary}" - res=$(grep -Ev "${grep_args[@]}" "${project_dictionary}") - printf '%s\n' "${res}" >|"${project_dictionary}" + res=$(grep -Ev "${grep_args[@]}" "${project_dictionary}" || true) + if [[ -n "${res}" ]]; then + printf '%s\n' "${res}" >|"${project_dictionary}" + else + printf '' >|"${project_dictionary}" + fi fi else unused='' - for word in $(grep -Ev '^//.*' "${project_dictionary}" || true); do + for word in $(grep -Ev '^//' "${project_dictionary}" || true); do if ! grep -Eqi "^${word}$" <<<"${all_words}"; then unused+="${word}"$'\n' fi done if [[ -n "${unused}" ]]; then - error "unused words in dictionaries; please remove the following words from ${project_dictionary} or run ${0##*/} with REMOVE_UNUSED_WORDS=1" - printf '=======================================\n' - printf '%s' "${unused}" - printf '=======================================\n' + error "unused words in dictionaries; please remove the following words from ${project_dictionary} or run ${0##*/} locally" + print_fenced "${unused}" fi fi fi + printf '\n' fi if [[ -n "${should_fail:-}" ]]; then