search

koalaman / shellcheck

ShellCheck, a static analysis tool for shell scripts
https://www.shellcheck.net
GNU General Public License v3.0
36.38k stars 1.78k forks source link

TODO: SC2046 adding quotes as recommended caused script to fail?? #2080

Open msetzerii opened 4 years ago

msetzerii commented 4 years ago

For bugs

For new checks and feature suggestions

Here's a snippet or screenshot that shows the problem:

#!/usr/bin/bash
for a in *; do file "$a" | grep -E '(sh script|Bourne|POSIX)' | cut -f 1 -d: ; done >file-list
shellcheck -Serror $(<file-list) >errors-found
shellcheck $(<file-list) >warnings-found
gedit errors-found warnings-found

Here's what shellcheck currently says:

Line 3  SC2046: Quote this to prevent word splitting.
Line 4  SC2046: Quote this to prevent word splitting.

Here's what I wanted or expected to see:

TODO: Describe expected/desired output

If I add the quotes it fails?? Here is detailed results. Have script shellcheck-dir

!/usr/bin/bash

for a in *; do file "$a" | grep -E '(sh script|Bourne|POSIX)' | cut -f 1 -d: ; done >file-list shellcheck -Serror $(<file-list) >errors-found shellcheck $(<file-list) >warnings-found gedit errors-found warnings-found

This version works fine, but running shellcheck on it, it gives these SC2046 messages.

In shellcheck-dir line 3: shellcheck -Serror $(<file-list) >errors-found ^-----------^ SC2046: Quote this to prevent word splitting.

In shellcheck-dir line 4: shellcheck $(<file-list) >warnings-found ^-----------^ SC2046: Quote this to prevent word splitting.

For more information: https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...

With thoses changes it fails big time??

Example: In a directory with just a few scripts file-list contains fixdir3 fixdir3c fixhtml fixsed9c mkfixsed.sh

5 lines with filenames followed by 0a (linefeed)

shellcheck $(<file-list) works fine shellcheck "$(<file-list) fails

shellcheck "$(<file-list)" fixdir3 fixdir3c fixhtml fixsed9c mkfixsed.sh: fixdir3 fixdir3c fixhtml fixsed9c mkfixsed.sh: openBinaryFile: does not exist (No such file or directory)

marc-h38 commented 4 years ago

This code is wrong with or without quotes. Without quotes it will fail whenever one filename has spaces in it.

This code is a variation on the "parsing ls" mistake https://mywiki.wooledge.org/ParsingLs

PS: also try file --brief --mime-type

msetzerii commented 4 years ago

I could see where if files have spaces in names, those would fail to work, since it would treat each part as a separate option? But without the quotes, it seems to process the other files, and just fails on the files with the space?? Created a modified file-list2 file, but inserted a space for fixhtml to fix html Without the quotes, it seems to process all the other files just fine? [root@setzconote staffx19]# shellcheck $(<file-list2) fix: fix: openBinaryFile: does not exist (No such file or directory) html: html: openBinaryFile: does not exist (No such file or directory)

With the quotes - it seems to fail for all the files, not just the one??

[root@setzconote staffx19]# shellcheck "$(<file-list2)" fixdir3 fixdir3c fix html fixsed9c mkfixsed.sh: fixdir3 fixdir3c fix html fixsed9c mkfixsed.sh: openBinaryFile: does not exist (No such file or directory)

If I put two quotes at beginning and end, it goes back to working for all the files without spaces?

[root@setzconote staffx19]# shellcheck ""$(<file-list2)"" fix: fix: openBinaryFile: does not exist (No such file or directory) html: html: openBinaryFile: does not exist (No such file or directory)

Used Echo to see what difference was? echo $(<file-list2) lists all file names with spaces between each one. echo "$(<file-list2) lists all file names with linefeeds between them. echo ""$(<file-list2) lists all file names with spaces between each one again.

msetzerii commented 4 years ago

Done some more testing and modified script Running it in /usr/bin directory for 4868 total files with 478 being shell scripts. None of the files has spaces so that isn't an issue for the tests. had to add the -x -W 0 to process or the results had a few differences. The -Serror files didn't need the -x.

Piping the list into shellcheck versus calling the program for each file had time go from about 1:20 to 1:26 for each process. So not a large increase. The use of the while read instead of the for does seem to allow for processing of files with spaces, but doesn't apply to the /usr/bin directory, since it has none.

!/usr/bin/bash

echo -n "" >errors echo -n "" >warn for a in ./*; do file "$a" | grep -E '(sh script|Bourne|POSIX)' | cut -f 1 -d: ; done >file-list time shellcheck -W 0 -Serror $(<file-list) >errors-found time shellcheck -x -W 0 $(<file-list) >warnings-found time while read -r line; do shellcheck -W 0 -Serror "$line"; done >errors time while read -r line; do shellcheck -x -W 0 "$line"; done >warn

Just noticed in last post, was missing some quotes. Used Echo to see what difference was? echo $(<file-list2) lists all file names with space between each one. echo "$(<file-list2)" lists all file names with linefeeds between them. echo ""$(<file-list2)"" lists all file names with space between each one again.

msetzerii commented 4 years ago

Seem to have found a way to handle having spaces in file names and the issue of the quotes around things?? Note: Added /tmp/ to see if it would speed up processing, but seems to take same amount of time. Script allows for passing severity and has it default to style if blank or misspelled. Probable a better way, but it seems to work Ran in /usr/bin that has about 5000 files, but only 488 are supported script files. Use the filter to only get the supported ones. There were two files in directory that used bsh script and wish script. They would report the unsupported shell, so found a method to filter them out. Using while with read -r allows support of file names that had blanks Also, handles check with the quotes so no errors reported. for was taking about 1:20, and while takes about 1:26, but the for called shellcheck a single time with piping the file in, whereas the while has to call shellcheck 488 times. The -x and -W 0 needed to be added to keep the output exactly the same. Running it with error results in 768 lines in result.

!/usr/bin/bash

if [[ "$1" != "error" && "$1" != "warning" && "$1" != "info" ]] ; then s=style; else s="$1"; fi echo -n "" >/tmp/"$s"-out for a in ./*; do file "$a" | grep -E '(/ksh script|/dash script|/sh script|Bourne|POSIX)' | cut -f 1 -d: ; done >/tmp/file-list time while read -r line; do shellcheck -x -W 0 -S"$s" "$line"; done </tmp/file-list >>/tmp/"$s"-out gedit /tmp/"$s"-out

Thanks for feedback. Don't know if this would or would not be of interest to others. Just found an error is firefox script, and was just wonder if other scripts might have errors. Seems with 768 lines, there are a number.

marc-h38 commented 4 years ago

Don't know if this would or would not be of interest to others.

Maybe somewhere else but not here. Bugs are meant to be focused on only one very specific issue and their description should include the smallest possible amount of information and reproduction steps. This bug and #2079 tell stories about a journey of yours, I doubt anyone will read them entirely (I didn't). If you have specific issues I suggest you close this and #2079 and re-file with a focused description and the minimum amount of code needed to reproduce. Mentioning Fedora (or any other OS) it not useful because many people don't use it.