[ci] enforce 'shellcheck' checks

jameslamb commented 5 months ago

Summary

This project has several shell scripts. Those should be tested with shellcheck.

Motivation

Shell scripts are used in this project for 2 primary purposes:

building/testing in CI
building interactively
- e.g., build-python.sh is a public, user-facing API into building the Python package

Checking those scripts with static analyzers would help to reduce the risk of mistakes being silently missed. It'd also give us a chance of catching issues in code paths that aren't covered by CI.

Description

This should be done with pre-commit, using the latest version of https://pypi.org/project/shellcheck-py/.

It is not necessary to fix all of the problems in a single pull request.

To work on this, add the shellcheck-py config linked in the docs above, then run this:

pre-commit run --all-files

That'll generate a list of errors. Put up a pull request that fixes some of them, and explain in the PR description exactly which errors it fixes.

As of this writing, that check returned the following:

full list of errors (click me)

```text In .ci/check_python_dists.sh line 3: set -e -E -u ^-- SC3041 (warning): In POSIX sh, set flag -E is undefined. In .ci/check_python_dists.sh line 19: twine check --strict ${DIST_DIR}/* || exit 1 ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: twine check --strict "${DIST_DIR}"/* || exit 1 In .ci/check_python_dists.sh line 23: check-wheel-contents ${DIST_DIR}/*.whl || exit 1 ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: check-wheel-contents "${DIST_DIR}"/*.whl || exit 1 In .ci/check_python_dists.sh line 27: if [ $PY_MINOR_VER -gt 7 ]; then ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: if [ "$PY_MINOR_VER" -gt 7 ]; then In .ci/check_python_dists.sh line 37: ${DIST_DIR}/* || exit 1 ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${DIST_DIR}"/* || exit 1 In .ci/check_python_dists.sh line 38: elif { test $(uname -m) = "aarch64"; }; then ^---------^ SC2046 (warning): Quote this to prevent word splitting. In .ci/check_python_dists.sh line 45: ${DIST_DIR}/* || exit 1 ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${DIST_DIR}"/* || exit 1 In .ci/check_python_dists.sh line 52: ${DIST_DIR}/* || exit 1 ^---------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${DIST_DIR}"/* || exit 1 In .ci/install-clang-devel.sh line 35: clang-${CLANG_VERSION} \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: clang-"${CLANG_VERSION}" \ In .ci/install-clang-devel.sh line 36: clangd-${CLANG_VERSION} \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: clangd-"${CLANG_VERSION}" \ In .ci/install-clang-devel.sh line 37: clang-format-${CLANG_VERSION} \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: clang-format-"${CLANG_VERSION}" \ In .ci/install-clang-devel.sh line 38: clang-tidy-${CLANG_VERSION} \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: clang-tidy-"${CLANG_VERSION}" \ In .ci/install-clang-devel.sh line 39: clang-tools-${CLANG_VERSION} \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: clang-tools-"${CLANG_VERSION}" \ In .ci/install-clang-devel.sh line 40: lldb-${CLANG_VERSION} \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: lldb-"${CLANG_VERSION}" \ In .ci/install-clang-devel.sh line 41: lld-${CLANG_VERSION} \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: lld-"${CLANG_VERSION}" \ In .ci/install-clang-devel.sh line 42: llvm-${CLANG_VERSION}-dev \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: llvm-"${CLANG_VERSION}"-dev \ In .ci/install-clang-devel.sh line 43: llvm-${CLANG_VERSION}-tools \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: llvm-"${CLANG_VERSION}"-tools \ In .ci/install-clang-devel.sh line 44: libomp-${CLANG_VERSION}-dev \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: libomp-"${CLANG_VERSION}"-dev \ In .ci/install-clang-devel.sh line 45: libc++-${CLANG_VERSION}-dev \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: libc++-"${CLANG_VERSION}"-dev \ In .ci/install-clang-devel.sh line 46: libc++abi-${CLANG_VERSION}-dev \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: libc++abi-"${CLANG_VERSION}"-dev \ In .ci/install-clang-devel.sh line 47: libclang-common-${CLANG_VERSION}-dev \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: libclang-common-"${CLANG_VERSION}"-dev \ In .ci/install-clang-devel.sh line 48: libclang-${CLANG_VERSION}-dev \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: libclang-"${CLANG_VERSION}"-dev \ In .ci/install-clang-devel.sh line 49: libclang-cpp${CLANG_VERSION}-dev \ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: libclang-cpp"${CLANG_VERSION}"-dev \ In .ci/install-clang-devel.sh line 50: libunwind-${CLANG_VERSION}-dev ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: libunwind-"${CLANG_VERSION}"-dev In .ci/install-clang-devel.sh line 54: cp --remove-destination /usr/lib/llvm-${CLANG_VERSION}/bin/* /usr/bin/ ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp --remove-destination /usr/lib/llvm-"${CLANG_VERSION}"/bin/* /usr/bin/ In .ci/lint-cpp.sh line 20: ${cmake_files} \ ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${cmake_files}" \ In .ci/rerun_workflow.sh line 38: runs=$(echo $runs | jq --arg pr_number "$pr_number" --arg pr_branch "$pr_branch" 'map(select(.event == "pull_request" and ((.pull_requests | length) != 0 and (.pull_requests[0].number | tostring) == $pr_number or .head_branch == $pr_branch)))') ^---^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: runs=$(echo "$runs" | jq --arg pr_number "$pr_number" --arg pr_branch "$pr_branch" 'map(select(.event == "pull_request" and ((.pull_requests | length) != 0 and (.pull_requests[0].number | tostring) == $pr_number or .head_branch == $pr_branch)))') In .ci/rerun_workflow.sh line 39: runs=$(echo $runs | jq 'sort_by(.run_number) | reverse') ^---^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: runs=$(echo "$runs" | jq 'sort_by(.run_number) | reverse') In .ci/rerun_workflow.sh line 41: if [[ $(echo $runs | jq 'length') -gt 0 ]]; then ^---^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: if [[ $(echo "$runs" | jq 'length') -gt 0 ]]; then In .ci/rerun_workflow.sh line 46: "${GITHUB_API_URL}/repos/microsoft/LightGBM/actions/runs/$(echo $runs | jq '.[0].id')/rerun" ^---^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${GITHUB_API_URL}/repos/microsoft/LightGBM/actions/runs/$(echo "$runs" | jq '.[0].id')/rerun" In .ci/set_commit_status.sh line 42: --arg state $status \ ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: --arg state "$status" \ In .ci/setup.sh line 143: https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-$(uname)-${ARCH}.sh ^------^ SC2046 (warning): Quote this to prevent word splitting. ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-$(uname)-"${ARCH}".sh In .ci/setup.sh line 144: sh miniforge.sh -b -p $CONDA ^----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: sh miniforge.sh -b -p "$CONDA" In .ci/test.sh line 59: cmake -B build -S . -DBUILD_CPP_TEST=ON -DUSE_OPENMP=OFF -DUSE_DEBUG=ON $extra_cmake_opts ^---------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cmake -B build -S . -DBUILD_CPP_TEST=ON -DUSE_OPENMP=OFF -DUSE_DEBUG=ON "$extra_cmake_opts" In .ci/test.sh line 66: CONDA_PYTHON_REQUIREMENT="python=$PYTHON_VERSION[build=*cpython]" ^-- SC1087 (error): Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet). In .ci/test.sh line 69: mamba create -q -y -n $CONDA_ENV ${CONDA_PYTHON_REQUIREMENT} numpy ^-------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mamba create -q -y -n $CONDA_ENV "${CONDA_PYTHON_REQUIREMENT}" numpy In .ci/test.sh line 70: source activate $CONDA_ENV ^------^ SC1091 (info): Not following: activate was not specified as input (see shellcheck -x). In .ci/test.sh line 92: cp ./build/lightgbmlib.jar $BUILD_ARTIFACTSTAGINGDIRECTORY/lightgbmlib_$OS_NAME.jar ^-----------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp ./build/lightgbmlib.jar "$BUILD_ARTIFACTSTAGINGDIRECTORY"/lightgbmlib_"$OS_NAME".jar In .ci/test.sh line 99: ${CONDA_PYTHON_REQUIREMENT} \ ^-------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${CONDA_PYTHON_REQUIREMENT}" \ In .ci/test.sh line 107: source activate $CONDA_ENV ^------^ SC1091 (info): Not following: activate was not specified as input (see shellcheck -x). In .ci/test.sh line 128: source activate $CONDA_ENV ^------^ SC1091 (info): Not following: activate was not specified as input (see shellcheck -x). In .ci/test.sh line 131: rstcheck --report-level warning $(find . -type f -name "*.rst") || exit 1 ^-----------------------------^ SC2046 (warning): Quote this to prevent word splitting. In .ci/test.sh line 133: rstcheck --report-level warning --ignore-directives=autoclass,autofunction,autosummary,doxygenfile $(find . -type f -name "*.rst") || exit 1 ^-----------------------------^ SC2046 (warning): Quote this to prevent word splitting. In .ci/test.sh line 163: ${CONDA_REQUIREMENT_FILES} \ ^------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${CONDA_REQUIREMENT_FILES}" \ In .ci/test.sh line 164: ${CONDA_PYTHON_REQUIREMENT} \ ^-------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${CONDA_PYTHON_REQUIREMENT}" \ In .ci/test.sh line 167: source activate $CONDA_ENV ^------^ SC1091 (info): Not following: activate was not specified as input (see shellcheck -x). In .ci/test.sh line 173: for LIBOMP_ALIAS in libgomp.dylib libiomp5.dylib libomp.dylib; do sudo ln -sf "$(brew --cellar libomp)"/*/lib/libomp.dylib $CONDA_PREFIX/lib/$LIBOMP_ALIAS || exit 1; done ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: for LIBOMP_ALIAS in libgomp.dylib libiomp5.dylib libomp.dylib; do sudo ln -sf "$(brew --cellar libomp)"/*/lib/libomp.dylib "$CONDA_PREFIX"/lib/$LIBOMP_ALIAS || exit 1; done In .ci/test.sh line 179: pip install ./dist/lightgbm-$LGB_VER.tar.gz -v || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pip install ./dist/lightgbm-"$LGB_VER".tar.gz -v || exit 1 In .ci/test.sh line 181: cp ./dist/lightgbm-$LGB_VER.tar.gz $BUILD_ARTIFACTSTAGINGDIRECTORY || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. ^-----------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp ./dist/lightgbm-"$LGB_VER".tar.gz "$BUILD_ARTIFACTSTAGINGDIRECTORY" || exit 1 In .ci/test.sh line 190: cp dist/lightgbm-$LGB_VER-py3-none-macosx*.whl $BUILD_ARTIFACTSTAGINGDIRECTORY || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. ^-----------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp dist/lightgbm-"$LGB_VER"-py3-none-macosx*.whl "$BUILD_ARTIFACTSTAGINGDIRECTORY" || exit 1 In .ci/test.sh line 204: ./dist/lightgbm-$LGB_VER-py3-none-$PLATFORM.whl || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: ./dist/lightgbm-"$LGB_VER"-py3-none-"$PLATFORM".whl || exit 1 In .ci/test.sh line 207: cp dist/lightgbm-$LGB_VER-py3-none-$PLATFORM.whl $BUILD_ARTIFACTSTAGINGDIRECTORY || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. ^-------^ SC2086 (info): Double quote to prevent globbing and word splitting. ^-----------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp dist/lightgbm-"$LGB_VER"-py3-none-"$PLATFORM".whl "$BUILD_ARTIFACTSTAGINGDIRECTORY" || exit 1 In .ci/test.sh line 226: ./dist/lightgbm-$LGB_VER.tar.gz \ ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: ./dist/lightgbm-"$LGB_VER".tar.gz \ In .ci/test.sh line 233: pip install ./dist/lightgbm-$LGB_VER*.whl -v || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pip install ./dist/lightgbm-"$LGB_VER"*.whl -v || exit 1 In .ci/test.sh line 251: ./dist/lightgbm-$LGB_VER.tar.gz \ ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: ./dist/lightgbm-"$LGB_VER".tar.gz \ In .ci/test.sh line 258: pip install ./dist/lightgbm-$LGB_VER*.whl -v || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pip install ./dist/lightgbm-"$LGB_VER"*.whl -v || exit 1 In .ci/test.sh line 271: ./dist/lightgbm-$LGB_VER.tar.gz \ ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: ./dist/lightgbm-"$LGB_VER".tar.gz \ In .ci/test.sh line 278: pip install ./dist/lightgbm-$LGB_VER*.whl -v || exit 1 ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pip install ./dist/lightgbm-"$LGB_VER"*.whl -v || exit 1 In .ci/test.sh line 296: cp ./lib_lightgbm.dylib $BUILD_ARTIFACTSTAGINGDIRECTORY/lib_lightgbm.dylib ^-----------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp ./lib_lightgbm.dylib "$BUILD_ARTIFACTSTAGINGDIRECTORY"/lib_lightgbm.dylib In .ci/test.sh line 302: cp ./lib_lightgbm.so $BUILD_ARTIFACTSTAGINGDIRECTORY/lib_lightgbm.so ^-----------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp ./lib_lightgbm.so "$BUILD_ARTIFACTSTAGINGDIRECTORY"/lib_lightgbm.so In .ci/test.sh line 316: for f in *.py **/*.py; do python $f || exit 1; done # run all examples ^-- SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: for f in *.py **/*.py; do python "$f" || exit 1; done # run all examples In .ci/test.sh line 319: jupyter nbconvert --ExecutePreprocessor.timeout=180 --to notebook --execute --inplace *.ipynb || exit 1 # run all notebooks ^-- SC2035 (info): Use ./*glob* or -- *glob* so names with dashes won't become options. In .ci/test_r_package.sh line 23: R_MAJOR_VERSION=( ${R_VERSION//./ } ) ^---------------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a. In .ci/test_r_package.sh line 24: if [[ "${R_MAJOR_VERSION}" == "3" ]]; then ^----------------^ SC2128 (warning): Expanding an array without an index only gives the first element. In .ci/test_r_package.sh line 29: elif [[ "${R_MAJOR_VERSION}" == "4" ]]; then ^----------------^ SC2128 (warning): Expanding an array without an index only gives the first element. In .ci/test_r_package.sh line 73: autoconf=$(cat R-package/AUTOCONF_UBUNTU_VERSION) \ ^-- SC2046 (warning): Quote this to prevent word splitting. In .ci/test_r_package.sh line 79: https://github.com/Kitware/CMake/releases/download/v3.25.1/cmake-3.25.1-linux-${ARCH}.sh \ ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: https://github.com/Kitware/CMake/releases/download/v3.25.1/cmake-3.25.1-linux-"${ARCH}".sh \ In .ci/test_r_package.sh line 83: sudo sh cmake-3.25.1-linux-${ARCH}.sh --skip-license --prefix=/opt/cmake || exit 1 ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: sudo sh cmake-3.25.1-linux-"${ARCH}".sh --skip-license --prefix=/opt/cmake || exit 1 In .ci/test_r_package.sh line 103: curl -sL ${R_MAC_PKG_URL} -o R.pkg || exit 1 ^--------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: curl -sL "${R_MAC_PKG_URL}" -o R.pkg || exit 1 In .ci/test_r_package.sh line 105: -pkg $(pwd)/R.pkg \ ^----^ SC2046 (warning): Quote this to prevent word splitting. In .ci/test_r_package.sh line 111: if [[ "${R_MAJOR_VERSION}" == "3" ]]; then ^----------------^ SC2128 (warning): Expanding an array without an index only gives the first element. In .ci/test_r_package.sh line 170: cp ${PKG_TARBALL} packages ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp "${PKG_TARBALL}" packages In .ci/test_r_package.sh line 173: -v $(pwd)/packages:/rchk/packages \ ^----^ SC2046 (warning): Quote this to prevent word splitting. In .ci/test_r_package.sh line 176: 2>&1 > ${RCHK_LOG_FILE} \ ^--^ SC2069 (warning): To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify). In .ci/test_r_package.sh line 182: exit $( ^-- SC2046 (warning): Quote this to prevent word splitting. In .ci/test_r_package.sh line 195: mkdir -p ${R_CMD_CHECK_DIR} ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p "${R_CMD_CHECK_DIR}" In .ci/test_r_package.sh line 196: mv ${PKG_TARBALL} ${R_CMD_CHECK_DIR} ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting. ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mv "${PKG_TARBALL}" "${R_CMD_CHECK_DIR}" In .ci/test_r_package.sh line 197: cd ${R_CMD_CHECK_DIR} ^----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cd "${R_CMD_CHECK_DIR}" In .ci/test_r_package.sh line 204: R CMD check ${PKG_TARBALL} \ ^------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: R CMD check "${PKG_TARBALL}" \ In .ci/test_r_package.sh line 207: || check_succeeded="no" ^-------------^ SC2030 (info): Modification of check_succeeded is local (to subshell caused by (..) group). In .ci/test_r_package.sh line 224: if [[ $check_succeeded == "no" ]]; then ^--------------^ SC2031 (info): check_succeeded was modified in a subshell. That change might be lost. In .ci/test_r_package.sh line 241: passed_correct_r_version_to_cmake=$( ^-- SC2034 (warning): passed_correct_r_version_to_cmake appears unused. Verify use (or export if used externally). In .ci/trigger_dispatch_run.sh line 40: --arg pr_number "$(echo $pr | jq '.number')" \ ^-^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: --arg pr_number "$(echo "$pr" | jq '.number')" \ In .ci/trigger_dispatch_run.sh line 41: --arg pr_sha "$(echo $pr | jq '.head.sha')" \ ^-^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: --arg pr_sha "$(echo "$pr" | jq '.head.sha')" \ In .ci/trigger_dispatch_run.sh line 42: --arg pr_branch "$(echo $pr | jq '.head.ref')" \ ^-^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: --arg pr_branch "$(echo "$pr" | jq '.head.ref')" \ In R-package/recreate-configure.sh line 11: LGB_VERSION=$(cat VERSION.txt | sed "s/rc/-/g") ^---------^ SC2002 (style): Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead. In R-package/recreate-configure.sh line 23: autoconf=${AUTOCONF_VERSION} ^-----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: autoconf="${AUTOCONF_VERSION}" In build-cran-package.sh line 30: set -e -E -u ^-- SC3041 (warning): In POSIX sh, set flag -E is undefined. In build-cran-package.sh line 66: LGB_VERSION=$(cat VERSION.txt | sed "s/rc/-/g") ^---------^ SC2002 (style): Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead. In build-cran-package.sh line 94: cp external_libs/eigen/Eigen/${eigen_module} "${EIGEN_R_DIR}/${eigen_module}" ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp external_libs/eigen/Eigen/"${eigen_module}" "${EIGEN_R_DIR}/${eigen_module}" In build-cran-package.sh line 95: if [ ${eigen_module} != "Dense" ]; then ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: if [ "${eigen_module}" != "Dense" ]; then In build-cran-package.sh line 97: cp -R external_libs/eigen/Eigen/src/${eigen_module}/* "${EIGEN_R_DIR}/src/${eigen_module}/" ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: cp -R external_libs/eigen/Eigen/src/"${eigen_module}"/* "${EIGEN_R_DIR}/src/${eigen_module}/" In build-cran-package.sh line 133: Rscript -e 'cat(.Platform$OS.type == "windows" && R.version[["major"]] < 4)' ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that. In build-cran-package.sh line 145: for file in $(find . -name '*.h' -o -name '*.hpp' -o -name '*.cpp'); do ^-- SC2044 (warning): For loops over find output are fragile. Use find -exec or a while read loop. In build-cran-package.sh line 156: find . -name '*.h.bak' -o -name '*.hpp.bak' -o -name '*.cpp.bak' -exec rm {} \; ^---^ SC2146 (warning): This action ignores everything before the -o. Use  to group. In build-cran-package.sh line 167: rm *.bak ^-- SC2035 (info): Use ./*glob* or -- *glob* so names with dashes won't become options. In build-python.sh line 65: set -e -E -u ^-- SC3041 (warning): In POSIX sh, set flag -E is undefined. In build-python.sh line 96: if [[ "$1" != *=* ]]; ^---------------^ SC3010 (warning): In POSIX sh, [[ ]] is undefined. In build-python.sh line 103: if [[ "$1" != *=* ]]; ^---------------^ SC3010 (warning): In POSIX sh, [[ ]] is undefined. In build-python.sh line 110: if [[ "$1" != *=* ]]; ^---------------^ SC3010 (warning): In POSIX sh, [[ ]] is undefined. In build-python.sh line 117: if [[ "$1" != *=* ]]; ^---------------^ SC3010 (warning): In POSIX sh, [[ ]] is undefined. In build-python.sh line 124: if [[ "$1" != *=* ]]; ^---------------^ SC3010 (warning): In POSIX sh, [[ ]] is undefined. In build-python.sh line 131: if [[ "$1" != *=* ]]; ^---------------^ SC3010 (warning): In POSIX sh, [[ ]] is undefined. In build-python.sh line 262: external_libs/eigen/Eigen/${eigen_module} \ ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: external_libs/eigen/Eigen/"${eigen_module}" \ In build-python.sh line 263: ./lightgbm-python/external_libs/eigen/Eigen/${eigen_module} ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: ./lightgbm-python/external_libs/eigen/Eigen/"${eigen_module}" In build-python.sh line 264: if [ ${eigen_module} != "Dense" ]; then ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: if [ "${eigen_module}" != "Dense" ]; then In build-python.sh line 265: mkdir -p ./lightgbm-python/external_libs/eigen/Eigen/src/${eigen_module}/ ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: mkdir -p ./lightgbm-python/external_libs/eigen/Eigen/src/"${eigen_module}"/ In build-python.sh line 268: external_libs/eigen/Eigen/src/${eigen_module}/* \ ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: external_libs/eigen/Eigen/src/"${eigen_module}"/* \ In build-python.sh line 269: ./lightgbm-python/external_libs/eigen/Eigen/src/${eigen_module}/ ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: ./lightgbm-python/external_libs/eigen/Eigen/src/"${eigen_module}"/ In build-python.sh line 316: echo '[build-system]' >> ./pyproject.toml ^-- SC2129 (style): Consider using { cmd1; cmd2; } >> file instead of individual redirects. In build-python.sh line 350: ${BUILD_ARGS} \ ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${BUILD_ARGS}" \ In build-python.sh line 360: ${BUILD_ARGS} \ ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${BUILD_ARGS}" \ In build-python.sh line 372: ${PIP_INSTALL_ARGS} \ ^-----------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: "${PIP_INSTALL_ARGS}" \ In docs/build-docs.sh line 11: -o ${HOME}/miniforge.sh \ ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: -o "${HOME}"/miniforge.sh \ In docs/build-docs.sh line 14: /bin/bash ${HOME}/miniforge.sh -b -p ${CONDA} ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting. ^------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: /bin/bash "${HOME}"/miniforge.sh -b -p "${CONDA}" In docs/build-docs.sh line 23: source activate docs-env ^------^ SC1091 (info): Not following: activate was not specified as input (see shellcheck -x). For more information: https://www.shellcheck.net/wiki/SC1087 -- Use braces when expanding arrays,... https://www.shellcheck.net/wiki/SC2034 -- passed_correct_r_version_to_cmake... https://www.shellcheck.net/wiki/SC2044 -- For loops over find output are fr.. ```

References

StrikerRUS commented 3 months ago

@jameslamb WDYT about using PSScriptAnalyzer for our PowerShell scripts?

jameslamb commented 3 months ago

@jameslamb WDYT about using PSScriptAnalyzer for our PowerShell scripts?

I'm willing to try it!

I'm concerned that it'll be difficult to install, but maybe that's just because I don't know what "Powershell Gallery" is. I hope we won't have to build it from source on every CI run, and I hope we won't have to introduce a new lint CI job that runs on Windows. But I'd be curious to see what it catches. And it would definitely be helpful to get some automated feedback on the Powershell scripts here. If you're interested in preparing a PR that adds this, I'd be happy to review it.

StrikerRUS commented 3 months ago

@jameslamb

and I hope we won't have to introduce a new lint CI job that runs on Windows.

Looks like that tool is written in C# programming language. We can use dotnet-sdk to build C# programs on Linux.

Ref. https://github.com/BayesWitnesses/m2cgen/blob/9784632311986234032673cdbfd29fc4c5cb429d/Dockerfile#L48 https://github.com/BayesWitnesses/m2cgen/blob/9784632311986234032673cdbfd29fc4c5cb429d/tests/e2e/executors/c_sharp.py#L86-L92

StrikerRUS commented 1 month ago

@jameslamb

I'm concerned that it'll be difficult to install

GitHub Actions has preinstalled PowerShell. So installation is pretty simple actually!

Just tried the following .sh script on macOS runner:

pwsh -v
pwsh -command "Install-Module -Name PSScriptAnalyzer -Scope AllUsers -Force -SkipPublisherCheck"

git clone https://github.com/microsoft/LightGBM

read -r -d '' analyzer_cmd << EOM
Invoke-ScriptAnalyzer -Path ./LightGBM/.ci -Severity Warning -Recurse -Outvariable issues
\$errors   = \$issues.Where({\$_.Severity -eq 'Error'})
\$warnings = \$issues.Where({\$_.Severity -eq 'Warning'})
if (\$errors) {
    Write-Error "There were \$(\$errors.Count) errors and \$(\$warnings.Count) warnings total." -ErrorAction Stop
} else {
    Write-Output "There were \$(\$errors.Count) errors and \$(\$warnings.Count) warnings total."
}
EOM
pwsh -command "${analyzer_cmd}"

But I'd be curious to see what it catches

And got the following output:

PowerShell 7.4.5
Cloning into 'LightGBM'...
RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSUseApprovedVerbs                  Warning      test-r-pac 3     The cmdlet 'D
                                                 kage-windo       ownload-File-
                                                 ws.ps1           With-Retries'
                                                                   uses an
                                                                  unapproved
                                                                  verb.
PSUseApprovedVerbs                  Warning      test-r-pac 23    The cmdlet 'R
                                                 kage-windo       un-R-Code-Red
                                                 ws.ps1           irect-Stderr'
                                                                   uses an
                                                                  unapproved
                                                                  verb.
PSUseShouldProcessForStateChangingF Warning      test-r-pac 32    Function 'Rem
unctions                                         kage-windo       ove-From-Path
                                                 ws.ps1           ' has verb
                                                                  that could
                                                                  change
                                                                  system
                                                                  state.
                                                                  Therefore,
                                                                  the function
                                                                  has to
                                                                  support 'Shou
                                                                  ldProcess'.
PSUseSingularNouns                  Warning      test-r-pac 3     The cmdlet 'D
                                                 kage-windo       ownload-File-
                                                 ws.ps1           With-Retries'
                                                                   uses a
                                                                  plural noun.
                                                                  A singular
                                                                  noun should
                                                                  be used
                                                                  instead.
PSAvoidUsingCmdletAliases           Warning      test-r-pac 116   'cd' is an
                                                 kage-windo       alias of 'Set
                                                 ws.ps1           -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-r-pac 186   'cd' is an
                                                 kage-windo       alias of 'Set
                                                 ws.ps1           -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-r-pac 206   'echo' is an
                                                 kage-windo       alias of 'Wri
                                                 ws.ps1           te-Output'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-r-pac 219   'echo' is an
                                                 kage-windo       alias of 'Wri
                                                 ws.ps1           te-Output'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-r-pac 299   'cd' is an
                                                 kage-windo       alias of 'Set
                                                 ws.ps1           -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSPossibleIncorrectComparisonWithNu Warning      install-op 23    $null should
ll                                               encl.ps1         be on the
                                                                  left side of
                                                                  equality
                                                                  comparisons.
PSUseApprovedVerbs                  Warning      test-windo 1     The cmdlet 'C
                                                 ws.ps1           heck-Output'
                                                                  uses an
                                                                  unapproved
                                                                  verb.
PSAvoidUsingCmdletAliases           Warning      test-windo 81    'cd' is an
                                                 ws.ps1           alias of 'Set
                                                                  -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-windo 92    'cd' is an
                                                 ws.ps1           alias of 'Set
                                                                  -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-windo 107   'cd' is an
                                                 ws.ps1           alias of 'Set
                                                                  -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-windo 131   'cd' is an
                                                 ws.ps1           alias of 'Set
                                                                  -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
PSAvoidUsingCmdletAliases           Warning      test-windo 139   'cd' is an
                                                 ws.ps1           alias of 'Set
                                                                  -Location'.
                                                                  Alias can
                                                                  introduce
                                                                  possible
                                                                  problems and
                                                                  make scripts
                                                                  hard to
                                                                  maintain.
                                                                  Please
                                                                  consider
                                                                  changing
                                                                  alias to its
                                                                  full content.
There were 0 errors and 16 warnings total.

jameslamb commented 1 month ago

Thanks for that! @StrikerRUS I'd support a PR that introduces this, if you'd like to try one. I hope we can add it to the existing linting job https://github.com/microsoft/LightGBM/blob/bbeecc09af946c5ff9b84d1ada4749a9f26bca31/.ci/test.sh#L101 that runs on Linux, using pwsh on Linux.

I don't think this should be added to pre-commit configuration though, as that'd mean anyone wanting to run pre-commit checks locally would have to install pwsh on their system.

StrikerRUS commented 1 month ago

@jameslamb

I'd support a PR

Yeah, I'd like to try.

I don't think this should be added to pre-commit configuration though,

Agree.

microsoft / LightGBM