koalaman / shellcheck

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

False positive for SC1003 (escape single quote) #1548

Open aschaepper opened 5 years ago

aschaepper commented 5 years ago

For bugs

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

According to: https://unix.stackexchange.com/questions/31947/how-to-add-a-newline-to-the-end-of-a-file

The following is a legit usecase of sed when trying to add a newline to the end of a file, (it does not append again if there is already a new line).

sed -i -e '$a\' file

Shellsheck.net now thinks I want to escape the single quote. Maybe you could refine this errormessage to a literal string which has at least 3 single quotes, that would indicate that i tried to escape a single quote with the backslash.

But if I only have 2 single quotes, I think that would be exactly the reason to not escape anything and also not show this warning?

This would also make the rule to match:

echo 'hello echo\' 

Here's what shellcheck currently says:

Line 323:
    sed -i -e '$a\' "$1"
                ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'.

Here's what I wanted or expected to see:

It should not say anything, the bashscript should be fine. But this is just my humble opinion. I just want to have my script to be "perfect" according to your rules :-).

Thanks for the great work I love your tool it helps me a lot!

koalaman commented 5 years ago

You're right, this is a false positive. The wiki suggests writing '$a'\\ or ignoring the suggestion.

I'm not sure if counting quotes in the line is the best heuristic, since sed '$a\\' 'my file.txt' would trigger while

echo "<img src='foo.jpg'>" | sed -e '
  s/\'\(.*\)\'/"\1"/g
'

would not. I do see that this old warning has a pretty high false positive rate though, particularly for tr and sed, so it's definitely worth looking into ways of reducing that.

Perhaps it could be as simple as looking for something that isn't whitespace or quotes after the ', since a lot of the true positives are things like echo 'It\'s a bug' and payload='{\'key\': 1234}' while the false positives are more like yours, and also things like echo 'c:\Program Files\'"$name"

xatier commented 4 years ago

Another false positive with the cut command, when using a \ as the delimiter.

$ cat foo 
#!/usr/bin/bash

echo 'a\b\c\d' | cut -d '\' -f 2

$ shellcheck foo 

In foo line 3:
echo 'a\b\c\d' | cut -d '\' -f 2
                         ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'.

For more information:
  https://www.shellcheck.net/wiki/SC1003 -- Want to escape a single quote? ec...

The workaround would be using cut -d "\\" -f 2, which is a little wordy.

zackw commented 11 months ago

A less wordy workaround is cut -d \\ -f 2 but IMHO the warning should not trigger when the offending single-quoted shell word is exactly '\'.