Skip to content

CI: Run shellcheck and shfmt on shell scripts #2474

@hoffie

Description

@hoffie

What is the current behaviour and why should it be changed?

We currently don't have any automatic QA on shell scripts.

Describe possible approaches

To keep the growing number of shell scripts manageable and consistent, we should run shellcheck as a static shell script code analyzer to catch mistakes and shfmt to apply a consistent coding style (indentation, etc.), similar to clang-format.

Example output (should be empty when we introduce it in CI):
$ shellcheck $(find -name '*.sh' | grep -v libs/)

In ./linux/deploy_deb.sh line 19:
perl .github/actions_scripts/getChangelog.pl ChangeLog "${VERSION}" --line-per-entry | while read entry
                                                                                             ^--^ SC2162 (info): read without -r will mangle backslashes.


In ./mac/deploy_mac.sh line 45:
    qmake "${project_path}" -o "${build_path}/Makefile" "CONFIG+=release" ${@:2}
                                                                          ^----^ SC2068 (error): Double quote array expansions to avoid re-splitting elements.


In ./mac/deploy_mac.sh line 46:
    local target_name=$(sed -nE 's/^QMAKE_TARGET *= *(.*)$/\1/p' "${build_path}/Makefile")
          ^---------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./mac/deploy_mac.sh line 47:
    local job_count="$(sysctl -n hw.ncpu)"
          ^-------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./mac/deploy_mac.sh line 74:
    local app_version="$(sed -nE 's/^VERSION *= *(.*)$/\1/p' "${project_path}")"
          ^---------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./mac/deploy_mac.sh line 121:
    echo Please run this script from the Qt project directory where $(basename "${project_path}") is located.
                                                                    ^---------------------------^ SC2046 (warning): Quote this to prevent word splitting.


In ./mac/deploy_mac.sh line 122:
    echo Usage: mac/$(basename "${0}")
                    ^----------------^ SC2046 (warning): Quote this to prevent word splitting.


In ./mac/deploy_mac.sh line 135:
build_installer_image "${client_app}" "${server_app}"
                       ^-----------^ SC2154 (warning): client_app is referenced but not assigned.
                                       ^-----------^ SC2154 (warning): server_app is referenced but not assigned.


In ./tools/changelog-helper.sh line 32:
    local changelog=$(sed -rne '/^###.*'"${target_release//./\.}"'\b/,/^### '"${prev_release//./\.}"'\b/p' ChangeLog)
          ^-------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 33:
    local changelog_begin_position=$(grep -nP '^### .*\d+\.\d+\.\d+\b' ChangeLog | head -n1 | cut -d: -f1)
          ^----------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 60:
    local changelog_begin_position=$(grep -nP '^### .*\d+\.\d+\.\d+\b' ChangeLog | head -n1 | cut -d: -f1)
          ^----------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 61:
    local changelog_prev_release_position=$(grep -nP '^### .*\d+\.\d+\.\d+\b' ChangeLog | head -n2 | tail -n1 | cut -d: -f1)
          ^-----------------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 64:
    local changelog_header=$(head -n "${changelog_begin_position}" ChangeLog)
          ^--------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 67:
    local changelog_prev_releases=$(tail -n "+${changelog_prev_release_position}" ChangeLog)
          ^---------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 70:
    local changelog=$(sed -rne '/^###.*'"${target_release//./\.}"'\b/,/^### '"${prev_release//./\.}"'\b/p' ChangeLog | tail -n +2 | head -n -1)
          ^-------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 103:
        index=$(($index+1))
                 ^----^ SC2004 (style): $/${} is unnecessary on arithmetic variables.


In ./tools/changelog-helper.sh line 134:
    local json=$(gh pr view "${id/#/}" --json title,author,state)
          ^--^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 135:
    local state=$(jq -r .state <<<"${json}")
          ^---^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 136:
    local title=$(jq -r .title <<<"${json}" | sanitize_title)
          ^---^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 137:
    local author=$(jq -r .author.login <<<"${json}")
          ^----^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 142:
    local title_suggestion_in_pr=$(gh pr view "$id" --json body,comments,reviews --jq '(.body), (.comments[] .body), (.reviews[] .body)' | grep -oP '\bCHANGELOG:\s*\K([^\\]{5,})' | tail -n1 | sanitize_title)
          ^--------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 156:
    local lang=$(grep -oP 'Updated? \K(\S+)(?= app translations? for )' <<<"$title" || true)
          ^--^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 168:
    local changelog_before=$(head -n "${changelog_begin_position}" ChangeLog)
          ^--------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 169:
    local changelog_after=$(tail -n "+$((${changelog_begin_position}+1))" ChangeLog)
          ^-------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
                                         ^-------------------------^ SC2004 (style): $/${} is unnecessary on arithmetic variables.


In ./tools/changelog-helper.sh line 177:
    local changelog_begin_position=$(grep -nP '^### .*\d+\.\d+\.\d+\b' ChangeLog | head -n1 | cut -d: -f1)
          ^----------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 178:
    local changelog_prev_release_position=$(grep -nP '^### .*\d+\.\d+\.\d+\b' ChangeLog | head -n2 | tail -n1 | cut -d: -f1)
          ^-----------------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 181:
    local changelog_header=$(head -n "${changelog_begin_position}" ChangeLog)
          ^--------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 184:
    local changelog_prev_releases=$(tail -n "+${changelog_prev_release_position}" ChangeLog)
          ^---------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/changelog-helper.sh line 187:
    local changelog=$(sed -rne '/^###.*'"${target_release//./\.}"'\b/,/^### '"${prev_release//./\.}"'\b/p' ChangeLog | tail -n +2 | head -n -1)
          ^-------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/create-translation-issues.sh line 67:
BODY_TEMPLATE_APP='Hi ${SPLIT_TRANSLATORS},
                  ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.


In ./tools/create-translation-issues.sh line 82:
Fixes #<Insert this issue'"'"'s number here>
                             ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.


In ./tools/create-translation-issues.sh line 92:
BODY_TEMPLATE_WEB='Hi ${SPLIT_TRANSLATORS},
                  ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.


In ./tools/create-translation-issues.sh line 100:
- Start your work in your fork on a branch based on jamuluswebsite'"'"'s `${TRANSLATE_BRANCH}` branch.
                                                                      ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.


In ./tools/create-translation-issues.sh line 105:
- Link your PR(s) to this issue by including `Fixes #<Insert this issue'"'"'s number here>` in the PR content.
                                                                           ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.


In ./tools/create-translation-issues.sh line 113:
Fixes #<Insert this issue'"'"'s number here>
                             ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.


In ./tools/create-translation-issues.sh line 152:
        for LANG in $(cd _translator-files/po/ && ls -d *); do
                                                        ^-- SC2035 (info): Use ./*glob* or -- *glob* so names with dashes won't become options.


In ./tools/create-translation-issues.sh line 187:
    local body=$(
          ^--^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/create-translation-issues.sh line 202:
    local existing_issue=$(gh issue list --milestone "$MILESTONE" --state all --search "$title" --json number --jq '.[0].number' || true)
          ^------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/create-translation-issues.sh line 217:
    local online_body=$(gh issue view "$existing_issue" --json body --jq .body)
          ^---------^ SC2155 (warning): Declare and assign separately to avoid masking return values.


In ./tools/check-wininstaller-translations.sh line 9:
LANGUAGE_FILES="$(ls -1 src/res/translation/wininstaller/{??.nsi,??_??.nsi} | grep -vF "${BASE_LANG}.nsi")"
                  ^-- SC2010 (warning): Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.


In ./tools/check-wininstaller-translations.sh line 14:
    if grep -q '^!include "\${ROOT_PATH}\\'$(sed -re 's|/|\\\\|g' <<<"${LANGUAGE_FILE}")'"' "${BASE_DIR}/${INSTALLERLNG}"; then
               ^--------------------------^ SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.
                                         ^-- SC1003 (info): Want to escape a single quote? echo 'This is how it'\''s done'.
                                           ^-- SC2046 (warning): Quote this to prevent word splitting.


In ./tools/update-copyright-notices.sh line 9:
for file in $(find android ios linux mac src windows -regex '.*\.\(cpp\|h\|mm\)' -not -regex '\./\(\.git\|libs/\|moc_\|ui_\).*'); do
            ^-- SC2044 (warning): For loops over find output are fragile. Use find -exec or a while read loop.

For more information:
  https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
  https://www.shellcheck.net/wiki/SC2010 -- Don't use ls | grep. Use a glob o...
  https://www.shellcheck.net/wiki/SC2044 -- For loops over find output are fr...

Has this feature been discussed and generally agreed?

No.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions