[CI] Enable shellcheck for lint

[Potabk]

What this PR does / why we need it?

This pr purpose to enable shellcheck for pre-commit and make the corresponding clean code

Does this PR introduce any user-facing change?

How was this patch tested?

• vLLM version: main
• vLLM main: vllm-project/vllm@e93f4cc

[gemini-code-assist]

Code Review

This pull request enables shellcheck for linting shell scripts and applies fixes to the codebase. The changes are generally good and improve code quality. I've identified a critical command injection vulnerability in the shellcheck.sh script itself that should be addressed. I've also suggested improvements for the .shellcheckrc configuration to avoid globally disabling important checks, and a refactoring in one of the scripts to better align with shellcheck's recommendations.

[gemini-code-assist]

The current xargs command is vulnerable to command injection if a filename contains special characters like backticks or dollar signs. It's much safer to pass filenames as positional parameters to the sh -c script, rather than embedding them into the script string. This prevents the shell from interpreting any special characters in the filename.

Suggested change

	find . -path ./.git -prune -o -name "*.sh" -print0 \
	| xargs -0 -I {} sh -c 'git check-ignore -q "{}" || shellcheck -s bash "{}"'
	find . -path ./.git -prune -o -name "*.sh" -print0 \
	| xargs -0 -I {} sh -c 'git check-ignore -q "$1" || shellcheck -s bash "$1"' sh {}

[gemini-code-assist]

Disabling SC2155 and SC2164 globally is discouraged as they can hide potential bugs.

• SC2155: Warns about masking return values in assignments like VAR=$(command). If command fails, the script might not exit as expected, even with set -e. It's safer to declare and assign separately, or handle the error immediately.
• SC2164: Warns about unsafe cd commands. A failed cd can cause subsequent commands to execute in the wrong directory, which can be very dangerous. It's better to use cd ... || exit.

Instead of disabling these checks globally, it would be better to address the warnings in the code, or disable them on a case-by-case basis using shellcheck disable=... comments where they are confirmed to be false positives.

disable=SC1091,SC2004,SC2129

[github-actions]

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

• A PR should do only one thing, smaller PRs enable faster reviews.
• Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
• Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.39%. Comparing base (1bbb20e) to head (aa99bdd).
⚠️ Report is 28 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2848      +/-   ##
==========================================
+ Coverage   74.76%   75.39%   +0.62%     
==========================================
  Files         150      155       +5     
  Lines       20891    21123     +232     
==========================================
+ Hits        15620    15925     +305     
+ Misses       5271     5198      -73     

Flag	Coverage Δ
unittests	75.39% <ø> (+0.62%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.