Add shellcheck check

[Mickael-Martin]

To refer on https://github.com/YunoHost/package_linter/issues/24 Maybe, some modifications will be done on other checks to obtain a good compatibility (I try it this my package)

[alexAubin]

So I tested it on my side and it seem to spit out a lot of messages per script ... namely the double-quoting of variable which is not exactly the standard among apps :/ I doubt that the app folks will be happy about seeing so many messages so I disabled the corresponding error category

Testing on various apps, I see it still shows a lot of various error and many of them might not be so relevant, idk ...

[Psycojoker]

So I tested it on my side and it seem to spit out a lot of messages per script ... namely the double-quoting of variable which is not exactly the standard among apps :/ I doubt that the app folks will be happy about seeing so many messages so I disabled the corresponding error category

Does that still cover if [ $stuff == 'other_sutff' ]? Because that's a super common error that will fail if there is a space in $stuff and that should REALLY be quoted.

[maniackcrudelis]

double-quoting of variable which is not exactly the standard among apps :/ I doubt that the app folks will be happy about seeing so many messages

Especially for that "error" which is not one... Still, the case shown by Bram is indeed an error. About double quotes globally, as I said many times, one should know why and when using double quotes instead of stuffing scripts of double quotes everywhere.

[alexAubin]

Does that still cover if [ $stuff == 'other_sutff' ] ?

It probably doesn't, but I doubt one can simply enable a check for this case while disabling this check for all the other case :s

[Mickael-Martin]

So I tested it on my side and it seem to spit out a lot of messages per script ... namely the double-quoting of variable which is not exactly the standard among apps :/ I doubt that the app folks will be happy about seeing so many messages so I disabled the corresponding error category

Does that still cover if [ $stuff == 'other_sutff' ]? Because that's a super common error that will fail if there is a space in $stuff and that should REALLY be quoted.

Yes :

in /tmp/test.sh line 2:
if [ $1 == "test" ];then
     ^-- SC2086: Double quote to prevent globbing and word splitting.

For me, https://github.com/koalaman/shellcheck/wiki/SC2086 is a common mistake and bash devs must to check their code to fix it. This error causes several broken code, make an exception is not a good choice in my opinion.

[maniackcrudelis]

What about if [ $val -eq 0 ] or if [ $val == stuff ]? Those are not errors.

[Mickael-Martin]

What about if [ $val -eq 0 ] or if [ $val == stuff ]? Those are not errors.

The first example is a comparison of integers, so you don't need double quotes, for the second, if you have space in val var, your condition cannot works.

[maniackcrudelis]

In both cases you don't need double quotes. If the comparison is between $val and stuff, then you don't expect any space. If I use such a comparison, then I know that $val will not contain any space.

[maniackcrudelis]

I already said that many times. An if condition is a simple case, but there many others cases not as simple as an if. One should now how, when and why using double quotes instead of putting it everywhere that's easy to know you would maybe need some.

[Mickael-Martin]

But you have an error if your $val contains space(s) or contain nothing, if you not test for this $val before. For comparison string, double quotes are a best practice, you have many example of security issues and bypassing check without var non-double quoted (like https://wiki.bash-hackers.org/syntax/quoting, or https://unix.stackexchange.com/questions/171346/security-implications-of-forgetting-to-quote-a-variable-in-bash-posix-shells )

[maniackcrudelis]

if $val contains a space or is empty. In my example, I didn't quote either $val or stuff. That was on purpose, because I know neither one or the other will contain a space or be empty. For sure in a if there's no risk, but there's many others cases you have to know what you're doing. Packagers should learn how to deal with double quotes!

[andretheolauret]

Looks not still under development

[alexAubin]

Closing because we're not gonna enforce shellcheck of whatever, if anything, work should go into reducing the amount of bash code people have to write/maintain in the context of app packaging