Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shellcheck complains about hook script #1226

Closed
1 task done
TrevorBurnham opened this issue Dec 20, 2022 · 3 comments
Closed
1 task done

Shellcheck complains about hook script #1226

TrevorBurnham opened this issue Dec 20, 2022 · 3 comments

Comments

@TrevorBurnham
Copy link

TrevorBurnham commented Dec 20, 2022

Troubleshoot

Context
(This is a minor issue: Functionality isn't affected.)

Scripts generated by husky add contain the line:

. "$(dirname -- "$0")/_/husky.sh"

Shellcheck, the popular shell script linter, doesn't care for that line. It complains:

Not following: ./_/husky.sh was not specified as input (see shellcheck -x). shellcheck(SC1091)

The rule in question, SC1091, is complaining about Shellcheck not being able to access the referenced file. I can't think of a way to satisfy the rule, so it may be that the best solution is to have husky add emit a shellcheck disable directive above it:

# shellcheck disable=SC1091
. "$(dirname -- "$0")/_/husky.sh"

Of course, another solution would be for developers to simply ignore the warning, but I believe it's very common for devs who use VS Code to have the Shellcheck extension installed (it has 500,000+ downloads) and enabled by default, so an extra line to silence the warning might avoid some confusion. Or maybe there's a more Shellcheck-friendly way to run husky.sh.

@paescuj
Copy link
Contributor

paescuj commented Jan 5, 2023

Hey @TrevorBurnham, thanks for raising this issue!

Actually, the problem here is that the VS Code extension is spawning shellcheck without the -x / --external-sources by default, which means shellcheck isn't following external files at all.
The warning goes away when running shellcheck with the option mentioned above or by specifying the source file explicitly. That is, shellcheck is able to follow the file in that case without the need for a directive.

I do realize that this might be confusing for devs who have the VS Code extension installed, but I feel like it's simply wrong to disable the check here.
In fact, I think the more sensible solution would be to raise an issue at https://github.com/vscode-shellcheck/vscode-shellcheck asking to turn on the -x / --external-sources by default.

That being said, your report inspired me to go through all shell scripts in this repo and fix the reported issues by shellcheck where applicable (see #1230).

@TrevorBurnham
Copy link
Author

Thanks for getting back to me! I've raised the issue with the vscode-shellcheck folks: vscode-shellcheck/vscode-shellcheck#883

@paescuj
Copy link
Contributor

paescuj commented Jan 5, 2023

Nice, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants