Use triple backticks rather than indenting for code.

Vidar Holen
2015-10-03 21:33:27 -07:00
parent c91b073c53
commit e073e65e5e
153 changed files with 1741 additions and 1049 deletions

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
echo "$" ```sh
echo "$"
```
### Correct code: ### Correct code:
echo "\$" ```sh
echo "\$"
```
### Rationale: ### Rationale:
`$` is special in double quotes, but there are some cases where it's interpreted literally: `$` is special in double quotes, but there are some cases where it's interpreted literally:

@@ -2,19 +2,27 @@
### Problematic code: ### Problematic code:
echo Yay \o/ ```sh
echo Yay \o/
```
or or
\git status ```sh
\git status
```
### Correct code: ### Correct code:
echo 'Yay \o/' ```sh
echo 'Yay \o/'
```
or or
command git status ```sh
command git status
```
### Rationale: ### Rationale:

@@ -2,19 +2,27 @@
### Problematic code: ### Problematic code:
# I want programs to show text in dutch! ```sh
LANGUAGE= nl # I want programs to show text in dutch!
LANGUAGE= nl
```
# I want to run the nl command with English error messages! ```sh
LANGUAGE= nl # I want to run the nl command with English error messages!
LANGUAGE= nl
```
### Correct code: ### Correct code:
# I want programs to show text in dutch! ```sh
LANGUAGE=nl # I want programs to show text in dutch!
LANGUAGE=nl
```
# I want to run the nl command with English error messages! ```sh
LANGUAGE='' nl # I want to run the nl command with English error messages!
LANGUAGE='' nl
```
### Rationale: ### Rationale:

@@ -2,11 +2,11 @@
### Problematic code: ### Problematic code:
for f in *; do echo "$f" done for f in *; do echo "$f" done
### Correct code: ### Correct code:
for f in *; do echo "$f"; done for f in *; do echo "$f"; done
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
echo “hello world” ```sh
echo “hello world”
```
### Correct code: ### Correct code:
echo "hello world" ```sh
echo "hello world"
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
echo hello world ```sh
echo hello world
```
### Correct code: ### Correct code:
echo 'hello world' ```sh
echo 'hello world'
```
### Rationale: ### Rationale:

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
echo "Ninth parameter: $9" ```sh
echo "Tenth parameter: $10" echo "Ninth parameter: $9"
echo "Tenth parameter: $10"
```
### Correct code: ### Correct code:
echo "Ninth parameter: $9" ```sh
echo "Tenth parameter: ${10}" echo "Ninth parameter: $9"
echo "Tenth parameter: ${10}"
```
### Rationale: ### Rationale:

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
while IFS= read -r line ```sh
do while IFS= read -r line
printf "%q\n" "$line" do
done <<(curl -s http://example.com) printf "%q\n" "$line"
done <<(curl -s http://example.com)
```
### Correct code: ### Correct code:
while IFS= read -r line ```sh
do while IFS= read -r line
printf "%q\n" "$line" do
done < <(curl -s http://example.com) printf "%q\n" "$line"
done < <(curl -s http://example.com)
```
### Rationale: ### Rationale:

@@ -4,23 +4,29 @@
Any code using `<<-` that is indented with spaces. `cat -T script` shows Any code using `<<-` that is indented with spaces. `cat -T script` shows
cat <<- foo ```sh
Hello world cat <<- foo
foo Hello world
foo
```
### Correct code: ### Correct code:
Code using `<<-` must be indented with tabs. `cat -T script` shows Code using `<<-` must be indented with tabs. `cat -T script` shows
^Icat <<- foo ```sh
^I^IHello world ^Icat <<- foo
^Ifoo ^I^IHello world
^Ifoo
```
Or simply don't indent the end token: Or simply don't indent the end token:
cat <<- foo ```sh
Hello World cat <<- foo
foo Hello World
foo
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
foo &; bar ```sh
foo &; bar
```
### Correct code: ### Correct code:
foo & bar ```sh
foo & bar
```
### Rationale: ### Rationale:

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
foo(input) { ```sh
echo "$input" foo(input) {
} echo "$input"
foo("hello world"); }
foo("hello world");
```
### Correct code: ### Correct code:
foo() { ```sh
echo "$1" foo() {
} echo "$1"
foo "hello world" }
foo "hello world"
```
### Rationale: ### Rationale:

@@ -2,17 +2,23 @@
### Problematic code: ### Problematic code:
$greeting="Hello World" ```sh
$greeting="Hello World"
```
### Correct code: ### Correct code:
greeting="Hello World" ```sh
greeting="Hello World"
```
Alternatively, if the goal was to assign to a variable whose name is in another variable (indirection), use `declare`: Alternatively, if the goal was to assign to a variable whose name is in another variable (indirection), use `declare`:
name=foo ```sh
declare "$name=hello world" name=foo
echo "$foo" declare "$name=hello world"
echo "$foo"
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
foo = 42 ```sh
foo = 42
```
### Correct code: ### Correct code:
foo=42 ```sh
foo=42
```
### Rationale: ### Rationale:

@@ -3,12 +3,16 @@
### Problematic code: ### Problematic code:
echo "Your username is ´whoami´" ```sh
echo "Your username is ´whoami´"
```
### Correct code: ### Correct code:
echo "Your username is $(whoami)" # Preferred ```sh
echo "Your username is `whoami`" # Deprecated, will give [SC2006] echo "Your username is $(whoami)" # Preferred
echo "Your username is `whoami`" # Deprecated, will give [SC2006]
```
### Rationale: ### Rationale:
@@ -20,10 +24,14 @@ Backticks start command expansions, while forward ticks are literal. To help spo
If you want to write out literal forward ticks, such as fancyful ascii quotation marks: If you want to write out literal forward ticks, such as fancyful ascii quotation marks:
echo "``Proprietary software is an injustice.´´ - Richard Stallman" ```sh
echo "``Proprietary software is an injustice.´´ - Richard Stallman"
```
use single quotes instead: use single quotes instead:
echo '``Proprietary software is an injustice.´´ - Richard Stallman' ```sh
echo '``Proprietary software is an injustice.´´ - Richard Stallman'
```
To nest forward ticks in command expansion, use `$(..)` instead of `` `..` ``. To nest forward ticks in command expansion, use `$(..)` instead of `` `..` ``.

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
greeting="hello ```sh
target="world" greeting="hello
target="world"
```
### Correct code: ### Correct code:
greeting="hello" ```sh
target="world" greeting="hello"
target="world"
```
### Rationale: ### Rationale:
@@ -20,12 +24,16 @@ ShellCheck warns when it detects multi-line double quoted, single quoted or back
If you do want a multiline variable, just make sure the character after it is a quote, space or line feed. If you do want a multiline variable, just make sure the character after it is a quote, space or line feed.
var='multiline ```sh
'value var='multiline
'value
```
can be rewritten for readability and to remove the warning: can be rewritten for readability and to remove the warning:
var='multiline ```sh
value' var='multiline
value'
```
As always `` `..` `` should be rewritten to ``$(..)``. As always `` `..` `` should be rewritten to ``$(..)``.

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
If true ```sh
Then If true
echo "hello" Then
Fi echo "hello"
Fi
```
### Correct code: ### Correct code:
if true ```sh
then if true
echo "hello" then
fi echo "hello"
fi
```
### Rationale: ### Rationale:

@@ -2,19 +2,26 @@
### Problematic code: ### Problematic code:
rmf() { rm -f "$@" } ```sh
rmf() { rm -f "$@" }
```
or or
eval echo \${foo} ```sh
eval echo \${foo}
```
### Correct code: ### Correct code:
rmf() { rm -f "$@"; } ```sh
rmf() { rm -f "$@"; }
```
and and
eval "echo \${foo}" ```sh
eval "echo \${foo}"
### Rationale: ### Rationale:
Curly brackets are normally used as syntax in parameter expansion, command grouping and brace expansion. Curly brackets are normally used as syntax in parameter expansion, command grouping and brace expansion.
@@ -30,3 +37,4 @@ ShellCheck does not warn about `{}`, since this is frequently used with `find` a
### Exceptions ### Exceptions
This error is harmless when the curly brackets are supposed to be literal, in e.g. `awk {'print $1'}`. However, it's cleaner and less error prone to simply include them inside the quotes: `awk '{print $1}'`. This error is harmless when the curly brackets are supposed to be literal, in e.g. `awk {'print $1'}`. However, it's cleaner and less error prone to simply include them inside the quotes: `awk '{print $1}'`.
```

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
!#/bin/sh ```sh
echo "Hello World" !#/bin/sh
echo "Hello World"
```
### Correct code: ### Correct code:
#!/bin/sh ```sh
echo "Hello World" #!/bin/sh
echo "Hello World"
```
### Rationale: ### Rationale:

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
for $var in * ```sh
do for $var in *
echo "$var" do
done echo "$var"
done
```
### Correct code: ### Correct code:
for var in * ```sh
do for var in *
echo "$var" do
done echo "$var"
done
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
echo "$array[@]" ```sh
echo "$array[@]"
```
### Correct code: ### Correct code:
echo "${array[@]}" ```sh
echo "${array[@]}"
```
### Rationale: ### Rationale:

@@ -2,19 +2,27 @@
### Problematic code: ### Problematic code:
grep ^(.*)\1$ file ```sh
grep ^(.*)\1$ file
```
or or
var = myfunction(value) ```sh
var = myfunction(value)
```
### Correct code: ### Correct code:
grep '^(.*)\1$' file ```sh
grep '^(.*)\1$' file
```
or or
var=$(myfunction value) ```sh
var=$(myfunction value)
```
### Rationale: ### Rationale:

@@ -2,18 +2,22 @@
### Problematic code: ### Problematic code:
if true ```sh
then if true
echo hello then
fi echo hello
fi fi
fi
```
### Correct code: ### Correct code:
if true ```sh
then if true
echo hello then
fi echo hello
fi
```
### Rationale: ### Rationale:
@@ -21,11 +25,13 @@ This error is typically seen when there are too many `fi`, `done` or `esac`s, or
In some cases, it can even be caused by bad quoting: In some cases, it can even be caused by bad quoting:
var="foo ```sh
if [[ $var = "bar ] var="foo
then if [[ $var = "bar ]
echo true then
fi echo true
fi
```
In this case, the `if` ends up inside the double quotes, leaving the `then` dangling. In this case, the `if` ends up inside the double quotes, leaving the `then` dangling.

@@ -2,12 +2,16 @@
### Problematic code: ### Problematic code:
. "$(find_install_dir)/lib.sh" ```sh
. "$(find_install_dir)/lib.sh"
```
### Correct code: ### Correct code:
# shellcheck source=src/lib.sh ```sh
. "$(find_install_dir)/lib.sh" # shellcheck source=src/lib.sh
. "$(find_install_dir)/lib.sh"
```
### Rationale: ### Rationale:

@@ -4,12 +4,16 @@ Reasons include: file not found, no permissions, not included on the command lin
### Problematic code: ### Problematic code:
source somefile ```sh
source somefile
```
### Correct code: ### Correct code:
# shellcheck disable=SC1091 ```sh
source somefile # shellcheck disable=SC1091
source somefile
```
### Rationale: ### Rationale:

@@ -2,12 +2,16 @@
### Problematic code: ### Problematic code:
source mylib ```sh
source mylib
```
### Correct code: ### Correct code:
# shellcheck disable=SC1094 ```sh
source mylib # shellcheck disable=SC1094
source mylib
```
(or fix `mylib`) (or fix `mylib`)

@@ -2,17 +2,23 @@
### Problematic code: ### Problematic code:
var==value ```sh
var==value
```
### Correct code: ### Correct code:
Assignment: Assignment:
var=value ```sh
var=value
```
Comparison: Comparison:
[ "$var" = value ] ```sh
[ "$var" = value ]
```
### Rationale: ### Rationale:
@@ -26,4 +32,6 @@ ShellCheck has noticed that you're using `==` in an unexpected way. The two most
If you wanted to assign a literal equals sign, use quotes to make this clear: If you wanted to assign a literal equals sign, use quotes to make this clear:
var="=sum(A1:A10)" ```sh
var="=sum(A1:A10)"
```

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
eval $var=(a b) ```sh
eval $var=(a b)
```
### Correct code: ### Correct code:
eval "$var=(a b)" ```sh
eval "$var=(a b)"
```
### Rationale: ### Rationale:

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
while sleep 1 ```sh
do# show time while sleep 1
date do# show time
done date
done
```
### Correct code: ### Correct code:
while sleep 1 ```sh
do # show time while sleep 1
date do # show time
done date
done
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
string="stirng" ; echo "$string" | sed -e "s/ir/ri/" ```sh
string="stirng" ; echo "$string" | sed -e "s/ir/ri/"
```
### Correct code: ### Correct code:
string="stirng" ; echo "${string//ir/ri}" ```sh
string="stirng" ; echo "${string//ir/ri}"
```
### Rationale: ### Rationale:
@@ -16,6 +20,8 @@ Let's assume somewhere earlier in your code you have put data into a variable (E
Occasionally a more complex sed substitution is required. For example, getting the last character of a string. Occasionally a more complex sed substitution is required. For example, getting the last character of a string.
string="stirng" ; echo "$string" | sed -e "s/^.*\(.\)$/\1/" ```sh
string="stirng" ; echo "$string" | sed -e "s/^.*\(.\)$/\1/"
```
This is a bit simple for the example and there are alternative ways of doing this in the shell, but this SC2001 flags on several of my crazy complex sed commands which are beyond the scope of this example. Utilizing some of the more complex capabilities of sed is required occasionally and it is safe to ignore SC2001. This is a bit simple for the example and there are alternative ways of doing this in the shell, but this SC2001 flags on several of my crazy complex sed commands which are beyond the scope of this example. Utilizing some of the more complex capabilities of sed is required occasionally and it is safe to ignore SC2001.

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
i=$(expr 1 + 2) ```sh
l=$(expr length "$var") i=$(expr 1 + 2)
l=$(expr length "$var")
```
### Correct code: ### Correct code:
i=$((1+2)) ```sh
l=${#var} i=$((1+2))
l=${#var}
```
### Rationale: ### Rationale:

@@ -2,20 +2,26 @@
### Problematic code: ### Problematic code:
echo $(($n+1)) ```sh
echo $(($n+1))
```
### Correct code: ### Correct code:
echo $((n+1)) ```sh
echo $((n+1))
```
### Rationale: ### Rationale:
The `$` on regular variables in arithmetic contexts is unnecessary, and can even lead to subtle bugs. This is because the contents of `$((..))` is first expanded into a string, and then evaluated as an expression: The `$` on regular variables in arithmetic contexts is unnecessary, and can even lead to subtle bugs. This is because the contents of `$((..))` is first expanded into a string, and then evaluated as an expression:
$ a='1+1' ```sh
$ echo $(($a * 5)) # becomes 1+1*5 $ a='1+1'
6 $ echo $(($a * 5)) # becomes 1+1*5
$ echo $((a * 5)) # evaluates as (1+1)*5 6
10 $ echo $((a * 5)) # evaluates as (1+1)*5
10
```
The `$` is unavoidable for special variables like `$1` vs `1`, `$#` vs `#`. ShellCheck does not warn about these cases. The `$` is unavoidable for special variables like `$1` vs `1`, `$#` vs `#`. ShellCheck does not warn about these cases.

@@ -2,11 +2,15 @@
### Problematic code ### Problematic code
echo "Current time: `date`" ```sh
echo "Current time: `date`"
```
### Correct code ### Correct code
echo "Current time: $(date)" ```sh
echo "Current time: $(date)"
```
### Rationale ### Rationale

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
find . | echo ```sh
find . | echo
```
### Correct code: ### Correct code:
find . ```sh
find .
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic Code: ### Problematic Code:
ps ax | grep -v grep | grep "$service" > /dev/null ```sh
ps ax | grep -v grep | grep "$service" > /dev/null
```
### Correct Code: ### Correct Code:
pgrep -f "$service" > /dev/null ```sh
pgrep -f "$service" > /dev/null
```
### Rationale: ### Rationale:
@@ -16,11 +20,15 @@ If you are just after a pid from a running program, then pgrep is a much safer a
What if you have the pid and you are looking for the matching program name? What if you have the pid and you are looking for the matching program name?
pid=123; ps ax | grep "$pid" ```sh
pid=123; ps ax | grep "$pid"
```
What if you want a range of the ps field, like from the 16th space to the end of the line? What if you want a range of the ps field, like from the 16th space to the end of the line?
ps ax | grep "$pid" | cut -d" " -f16- ```sh
ps ax | grep "$pid" | cut -d" " -f16-
```
Both are valid cases where SC2009 is not valid. Both are valid cases where SC2009 is not valid.

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
ls -l | grep " $USER " | grep '\.txt$' ```sh
ls -l | grep " $USER " | grep '\.txt$'
```
### Correct code: ### Correct code:
find . -maxdepth 1 -name '*.txt' -user "$USER" ```sh
find . -maxdepth 1 -name '*.txt' -user "$USER"
```
### Rationale: ### Rationale:
@@ -14,11 +18,13 @@
Here's an example: Here's an example:
$ ls -l ```sh
total 0 $ ls -l
-rw-r----- 1 me me 0 Feb 5 20:11 foo?bar total 0
-rw-r----- 1 me me 0 Feb 5 2011 foo?bar -rw-r----- 1 me me 0 Feb 5 20:11 foo?bar
-rw-r----- 1 me me 0 Feb 5 20:11 foo?bar -rw-r----- 1 me me 0 Feb 5 2011 foo?bar
-rw-r----- 1 me me 0 Feb 5 20:11 foo?bar
```
It shows three seemingly identical filenames, and did you spot the time format change? How it formats and what it redacts can differ between locale settings, `ls` version, and whether output is a tty. It shows three seemingly identical filenames, and did you spot the time format change? How it formats and what it redacts can differ between locale settings, `ls` version, and whether output is a tty.

@@ -2,24 +2,30 @@
### Problematic code: ### Problematic code:
for line in $(cat file | grep -v '^ *#') ```sh
do for line in $(cat file | grep -v '^ *#')
echo "Line: $line" do
done echo "Line: $line"
done
```
### Correct code: ### Correct code:
grep -v '^ *#' < file | while IFS= read -r line ```sh
do grep -v '^ *#' < file | while IFS= read -r line
echo "Line: $line" do
done echo "Line: $line"
done
```
or without a subshell (bash, zsh, ksh): or without a subshell (bash, zsh, ksh):
while IFS= read -r line ```sh
do while IFS= read -r line
echo "Line: $line" do
done < <(grep -v '^ *#' < file) echo "Line: $line"
done < <(grep -v '^ *#' < file)
```
### Rationale: ### Rationale:
@@ -27,20 +33,26 @@ For loops by default (subject to `$IFS`) read word by word. Additionally, glob e
Given this text file: Given this text file:
foo * ```sh
bar foo *
bar
```
The for loop will print: The for loop will print:
Line: foo ```sh
Line: aardwark.jpg Line: foo
Line: bullfrog.jpg Line: aardwark.jpg
... Line: bullfrog.jpg
...
```
The while loop will print: The while loop will print:
Line: foo * ```sh
Line: bar Line: foo *
Line: bar
```
### Exceptions ### Exceptions

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
find . -name '*.tar' -exec tar xf {} -C "$(dirname {})" \; ```sh
find . -name '*.tar' -exec tar xf {} -C "$(dirname {})" \;
```
### Correct code: ### Correct code:
find . -name '*.tar' -exec sh -c 'tar xf "$1" -C "$(dirname "$1")"' _ {} \; ```sh
find . -name '*.tar' -exec sh -c 'tar xf "$1" -C "$(dirname "$1")"' _ {} \;
```
### Rationale: ### Rationale:
@@ -14,8 +18,10 @@ Bash evaluates any command substitutions before the command they feature in is e
To run shell code for each file, we can write a tiny script and inline it with `sh -c`. We add `_` as a dummy argument that becomes `$0`, and a filename argument that becomes `$1` in the inlined script: To run shell code for each file, we can write a tiny script and inline it with `sh -c`. We add `_` as a dummy argument that becomes `$0`, and a filename argument that becomes `$1` in the inlined script:
$ sh -c 'echo "$1 is in $(dirname "$1")"' _ "mydir/myfile" ```sh
mydir/myfile is in mydir $ sh -c 'echo "$1 is in $(dirname "$1")"' _ "mydir/myfile"
mydir/myfile is in mydir
```
This command can be executed by `find -exec`, with `{}` as the filename argument. It executes shell which interprets the inlined script once for each file. Note that the inlined script is single quoted, again to ensure that the expansion does not happen prematurely . This command can be executed by `find -exec`, with `{}` as the filename argument. It executes shell which interprets the inlined script once for each file. Note that the inlined script is single quoted, again to ensure that the expansion does not happen prematurely .

@@ -2,16 +2,20 @@
### Problematic code: ### Problematic code:
[[ $dryrun ]] && echo "Would delete file" || rm file ```sh
[[ $dryrun ]] && echo "Would delete file" || rm file
```
### Correct code: ### Correct code:
if [[ $dryrun ]] ```sh
then if [[ $dryrun ]]
echo "Would delete file" then
else echo "Would delete file"
rm file else
fi rm file
fi
```
### Rationale: ### Rationale:

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
name=World ```sh
echo 'Hello $name' name=World
echo 'Hello $name'
```
### Correct code: ### Correct code:
name=World ```sh
echo "Hello $name" name=World
echo "Hello $name"
```
### Rationale: ### Rationale:
@@ -18,7 +22,9 @@ If you want to use the values of variables and such, use double quotes instead.
Note that if you have other items that needs single quoting, you can use both in a single word: Note that if you have other items that needs single quoting, you can use both in a single word:
echo '$1 USD is '"$rate GBP" ```sh
echo '$1 USD is '"$rate GBP"
```
### Exceptions ### Exceptions

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
percent=$((count/total*100)) ```sh
percent=$((count/total*100))
```
### Correct code: ### Correct code:
percent=$((count*100/total)) ```sh
percent=$((count*100/total))
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
echo 'hello world' | tr 'hello' 'goodbye' ```sh
echo 'hello world' | tr 'hello' 'goodbye'
```
### Correct code: ### Correct code:
echo 'hello world' | sed -e 's/hello/goodbye/g' ```sh
echo 'hello world' | sed -e 's/hello/goodbye/g'
```
### Rationale: ### Rationale:

@@ -1,13 +1,17 @@
# Note that unlike globs, o* here matches 'ooo' but not 'oscar' # Note that unlike globs, o* here matches 'ooo' but not 'oscar'
### Problematic code: ### Problematic code:
grep 'foo*' ```sh
grep 'foo*'
```
when wanting to match `food` and `foosball`, but not `mofo` or `keyfob`. when wanting to match `food` and `foosball`, but not `mofo` or `keyfob`.
### Correct code: ### Correct code:
grep '^foo' ```sh
grep '^foo'
```
### Rationale: ### Rationale:

@@ -2,11 +2,11 @@
### Problematic code: ### Problematic code:
sudo echo 'export FOO=bar' >> /etc/profile sudo echo 'export FOO=bar' >> /etc/profile
### Correct code: ### Correct code:
echo 'export FOO=bar' | sudo tee -a /etc/profile > /dev/null echo 'export FOO=bar' | sudo tee -a /etc/profile > /dev/null
### Rationale: ### Rationale:
@@ -18,13 +18,13 @@ There is nothing special about `tee`. It's just the simplest command that can bo
Truncating: Truncating:
echo 'data' | sudo dd of=file echo 'data' | sudo dd of=file
echo 'data' | sudo sed 'w file' echo 'data' | sudo sed 'w file'
Appending: Appending:
echo 'data' | sudo awk '{ print $0 >> "file" }' echo 'data' | sudo awk '{ print $0 >> "file" }'
echo 'data' | sudo sh -c 'cat >> file' echo 'data' | sudo sh -c 'cat >> file'
### Exceptions ### Exceptions

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
PS1='\e[36m\$ \e(B\e[m' ```sh
PS1='\e[36m\$ \e(B\e[m'
```
### Correct code: ### Correct code:
PS1='\[\e[36m\]\$ \[\e(B\e[m\]' ```sh
PS1='\[\e[36m\]\$ \[\e(B\e[m\]'
```
### Rationale: ### Rationale:

@@ -2,33 +2,43 @@
### Problematic code: ### Problematic code:
alias server_uptime='ssh $host 'uptime -p'' ```sh
alias server_uptime='ssh $host 'uptime -p''
```
### Correct code: ### Correct code:
alias server_uptime='ssh $host '"'uptime -p'" ```sh
alias server_uptime='ssh $host '"'uptime -p'"
```
### Rationale: ### Rationale:
In the first case, the user has four single quotes on a line, wishfully hoping that the shell will match them up as outer quotes around a string with literal single quotes: In the first case, the user has four single quotes on a line, wishfully hoping that the shell will match them up as outer quotes around a string with literal single quotes:
# v--------match--------v ```sh
alias server_uptime='ssh $host 'uptime -p'' # v--------match--------v
# ^--match--^ alias server_uptime='ssh $host 'uptime -p''
# ^--match--^
```
The shell, meanwhile, always terminates single quoted strings at the first possible single quote: The shell, meanwhile, always terminates single quoted strings at the first possible single quote:
# v---match--v ```sh
alias server_uptime='ssh $host 'uptime -p'' # v---match--v
# ^^ alias server_uptime='ssh $host 'uptime -p''
# ^^
```
Which is the same thing as `alias server_uptime='ssh $host uptime' -p`. Which is the same thing as `alias server_uptime='ssh $host uptime' -p`.
There is no way to nest single quotes. However, single quotes can be placed literally in double quotes, so we can instead concatenate a single quoted string and a double quoted string: There is no way to nest single quotes. However, single quotes can be placed literally in double quotes, so we can instead concatenate a single quoted string and a double quoted string:
# v--match---v ```sh
alias server_uptime='ssh $host '"'uptime -p'" # v--match---v
# ^---match---^ alias server_uptime='ssh $host '"'uptime -p'"
# ^---match---^
```
This results in an alias with embedded single quotes. This results in an alias with embedded single quotes.

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
echo "You enter "$HOSTNAME". You can smell the wumpus." >> /etc/issue ```sh
echo "You enter "$HOSTNAME". You can smell the wumpus." >> /etc/issue
```
### Correct code: ### Correct code:
echo "You enter $HOSTNAME. You can smell the wumpus." >> /etc/issue ```sh
echo "You enter $HOSTNAME. You can smell the wumpus." >> /etc/issue
```
### Rationale: ### Rationale:
@@ -14,9 +18,11 @@ Always quoting variables and command expansions is good practice, but blindly pu
In this case, ShellCheck has noticed that the quotes around the expansion are unquoting it, because the left quote is terminating an existing double quoted string, while the right quote starts a new one: In this case, ShellCheck has noticed that the quotes around the expansion are unquoting it, because the left quote is terminating an existing double quoted string, while the right quote starts a new one:
echo "You enter "$HOSTNAME". You can smell the wumpus." ```sh
|----------| |---------------------------| echo "You enter "$HOSTNAME". You can smell the wumpus."
Quoted No quotes Quoted |----------| |---------------------------|
Quoted No quotes Quoted
```
If the quotes were supposed to be literal, they should be escaped. If the quotes were supposed to quote an expansion (as in the example), they should be removed because this is already a double quoted string. If the quotes were supposed to be literal, they should be escaped. If the quotes were supposed to quote an expansion (as in the example), they should be removed because this is already a double quoted string.

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
echo "Name:\t$value" ```sh
echo "Name:\t$value"
```
### Correct code: ### Correct code:
printf "Name:\t%s\n" "$value" ```sh
printf "Name:\t%s\n" "$value"
```
### Rationale: ### Rationale:
@@ -18,7 +22,9 @@ Other, non-portable methods include `echo -e '\t'` and `echo $'\t'`. ShellCheck
If you actually wanted a literal backslash-t, use If you actually wanted a literal backslash-t, use
echo "\\t" ```sh
echo "\\t"
```
### Exceptions ### Exceptions

@@ -2,21 +2,29 @@
### Problematic code: ### Problematic code:
ssh host "echo $HOSTNAME" ```sh
ssh host "echo $HOSTNAME"
```
### Correct code: ### Correct code:
ssh host "echo \$HOSTNAME" ```sh
ssh host "echo \$HOSTNAME"
```
or or
ssh host 'echo $HOSTNAME' ```sh
ssh host 'echo $HOSTNAME'
```
### Rationale: ### Rationale:
Bash expands all arguments that are not escaped/singlequoted. This means that the problematic code is identical to Bash expands all arguments that are not escaped/singlequoted. This means that the problematic code is identical to
ssh host "echo clienthostname" ```sh
ssh host "echo clienthostname"
```
and will print out the client's hostname, not the server's hostname. and will print out the client's hostname, not the server's hostname.

@@ -29,33 +29,45 @@ Here are some constructs that cause subshells (shellcheck may not warn about all
Pipelines: Pipelines:
subshell1 | subshell2 | subshell3 # Bash, Dash, Ash ```sh
subshell1 | subshell2 | regular # Ksh, Zsh subshell1 | subshell2 | subshell3 # Bash, Dash, Ash
subshell1 | subshell2 | regular # Ksh, Zsh
```
Command substitution: Command substitution:
regular "$(subshell1)" "`subshell2`" ```sh
regular "$(subshell1)" "`subshell2`"
```
Process substitution: Process substitution:
regular <(subshell1) >(subshell2) ```sh
regular <(subshell1) >(subshell2)
```
Some forms of grouping: Some forms of grouping:
( subshell ) ```sh
{ regular; } ( subshell )
{ regular; }
```
Backgrounding: Backgrounding:
subshell1 & ```sh
subshell2 & subshell1 &
subshell2 &
```
Anything executed by external processes: Anything executed by external processes:
find . -exec subshell1 {} \; ```sh
find . -print0 | xargs -0 subshell2 find . -exec subshell1 {} \;
sudo subshell3 find . -print0 | xargs -0 subshell2
su -c subshell4 sudo subshell3
su -c subshell4
```
This applies not only to setting variables, but also setting shell options and changing directories. This applies not only to setting variables, but also setting shell options and changing directories.

@@ -2,12 +2,16 @@
### Problematic code: ### Problematic code:
foo() { bar --baz "$@"; frob --baz "$@"; }; ```sh
find . -exec foo {} + foo() { bar --baz "$@"; frob --baz "$@"; };
find . -exec foo {} +
```
### Correct code: ### Correct code:
find . -exec sh -c 'bar --baz "$@"; frob --baz "$@";' -- {} + ```sh
find . -exec sh -c 'bar --baz "$@"; frob --baz "$@";' -- {} +
```
### Rationale: ### Rationale:
@@ -19,7 +23,9 @@ Instead, the function contents can be executed in a shell, either through `sh -c
If you're intentionally passing a word that happens to have the same name as a declared function, you can quote it to make shellcheck ignore it, e.g. If you're intentionally passing a word that happens to have the same name as a declared function, you can quote it to make shellcheck ignore it, e.g.
nobody() { ```sh
sudo -u "nobody" "$@" nobody() {
} sudo -u "nobody" "$@"
}
```

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
foo=42 ```sh
echo "$FOO" foo=42
echo "$FOO"
```
### Correct code: ### Correct code:
foo=42 ```sh
echo "$foo" foo=42
echo "$foo"
```
### Rationale: ### Rationale:
@@ -22,20 +26,26 @@ ShellCheck may not always realize that the variable is in use (especially with i
For throwaway variables, consider using `_` as a dummy: For throwaway variables, consider using `_` as a dummy:
read _ last _ zip _ _ <<< "$str" ```sh
echo "$last, $zip" read _ last _ zip _ _ <<< "$str"
echo "$last, $zip"
```
or use a directive to disable the warning: or use a directive to disable the warning:
# shellcheck disable=SC2034 ```sh
read first last email zip lat lng <<< "$str" # shellcheck disable=SC2034
echo "$last, $zip" read first last email zip lat lng <<< "$str"
echo "$last, $zip"
```
For indirection, there's not much you can do without rewriting to use arrays or similar: For indirection, there's not much you can do without rewriting to use arrays or similar:
bar=42 # will always appear unused ```sh
foo=bar bar=42 # will always appear unused
echo "${!foo}" foo=bar
echo "${!foo}"
```
This is expected behavior, and not a bug. There is no good way to statically analyze indirection in shell scripts, just like static C analyzers have a hard time preventing segfaults. This is expected behavior, and not a bug. There is no good way to statically analyze indirection in shell scripts, just like static C analyzers have a hard time preventing segfaults.

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
sum=find | wc -l ```sh
sum=find | wc -l
```
### Correct code: ### Correct code:
sum=$(find | wc -l) ```sh
sum=$(find | wc -l)
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
find . -type f | xargs md5sum ```sh
find . -type f | xargs md5sum
```
### Correct code: ### Correct code:
find . -type f -print0 | xargs -0 md5sum ```sh
find . -type f -print0 | xargs -0 md5sum
```
### Rationale: ### Rationale:

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
for i in 'seq 1 10' ```sh
do for i in 'seq 1 10'
echo "$i" do
done echo "$i"
done
```
### Correct code: ### Correct code:
for i in $(seq 1 10) ```sh
do for i in $(seq 1 10)
echo "$i" do
done echo "$i"
done
```
### Rationale: ### Rationale:

@@ -2,10 +2,12 @@
### Problematic code: ### Problematic code:
for var in value ```sh
do for var in value
echo "$var" do
done echo "$var"
done
```
### Correct code: ### Correct code:
@@ -13,19 +15,27 @@ Correct code depends on what you want to do.
To iterate over files in a directory, instead of `for var in /my/dir` use: To iterate over files in a directory, instead of `for var in /my/dir` use:
for var in /my/dir/* ; do echo "$var"; done ```sh
for var in /my/dir/* ; do echo "$var"; done
```
To iterate over lines in a file or command output, use a while read loop instead: To iterate over lines in a file or command output, use a while read loop instead:
mycommand | while IFS= read -r line; do echo "$line"; done ```sh
mycommand | while IFS= read -r line; do echo "$line"; done
```
To iterate over *words* written to a command or function's stdout, instead of `for var in myfunction`, use To iterate over *words* written to a command or function's stdout, instead of `for var in myfunction`, use
for var in $(myfunction); do echo "$var"; done ```sh
for var in $(myfunction); do echo "$var"; done
```
To iterate over *words* in a variable, instead of `for var in myvariable`, use To iterate over *words* in a variable, instead of `for var in myvariable`, use
for var in $myvariable; do echo "$var"; done ```sh
for var in $myvariable; do echo "$var"; done
```

@@ -2,13 +2,15 @@
### Problematic code: ### Problematic code:
for file in $(find mydir -mtime -7 -name '*.mp3') ```sh
do for file in $(find mydir -mtime -7 -name '*.mp3')
let count++ do
echo "Playing file no. $count" let count++
play "$file" echo "Playing file no. $count"
done play "$file"
echo "Played $count files" done
echo "Played $count files"
```
This will fail for filenames containing spaces and similar, such as `My File.mp3`, and has a series of potential globbing issues depending on other filenames in the directory like (if you have `MyFile2.mp3` and `MyFile[2014].mp3`, the former file will play twice and the latter will not play at all). This will fail for filenames containing spaces and similar, such as `My File.mp3`, and has a series of potential globbing issues depending on other filenames in the directory like (if you have `MyFile2.mp3` and `MyFile[2014].mp3`, the former file will play twice and the latter will not play at all).
@@ -18,13 +20,15 @@ There are many possible fixes, each with its pros and cons.
The most general fix (that requires the least amount of thinking to apply) is having `find` output a `\0` separated list of files and consuming them in a `while read` loop: The most general fix (that requires the least amount of thinking to apply) is having `find` output a `\0` separated list of files and consuming them in a `while read` loop:
while IFS= read -r -d '' file ```sh
do while IFS= read -r -d '' file
let count++ do
echo "Playing file no. $count" let count++
play "$file" echo "Playing file no. $count"
done < <(find mydir -mtime -7 -name '*.mp3' -print0) play "$file"
echo "Played $count files" done < <(find mydir -mtime -7 -name '*.mp3' -print0)
echo "Played $count files"
```
In usage it's very similar to the `for` loop: it gets its output from a `find` statement, it executes a shell script body, it allows updating/aggregating variables, and the variables are available when the loop ends. In usage it's very similar to the `for` loop: it gets its output from a `find` statement, it executes a shell script body, it allows updating/aggregating variables, and the variables are available when the loop ends.
@@ -34,14 +38,16 @@ It requires Bash, and works with GNU, Busybox, OS X, FreeBSD and OpenBSD find, b
If you don't need `find` logic like `-mtime -7` and just use it to match globs recursively (all `*.mp3` files under a directory), you can instead use `globstar` and `nullglob` instead of `find`, and still use a `for` loop: If you don't need `find` logic like `-mtime -7` and just use it to match globs recursively (all `*.mp3` files under a directory), you can instead use `globstar` and `nullglob` instead of `find`, and still use a `for` loop:
shopt -s globstar nullglob ```sh
for file in mydir/**/*.mp3 shopt -s globstar nullglob
do for file in mydir/**/*.mp3
let count++ do
echo "Playing file no. $count" let count++
play "$file" echo "Playing file no. $count"
done play "$file"
echo "Played $count files" done
echo "Played $count files"
```
This is bash specific. This is bash specific.
@@ -50,15 +56,17 @@ This is bash specific.
If you need POSIX compliance, this is a fair approach: If you need POSIX compliance, this is a fair approach:
find mydir ! -name "$(printf "*\n*")" -name '*.mp3' > tmp ```sh
while IFS= read -r file find mydir ! -name "$(printf "*\n*")" -name '*.mp3' > tmp
do while IFS= read -r file
let count++ do
echo "Playing file #$count" let count++
play "$file" echo "Playing file #$count"
done < tmp play "$file"
rm tmp done < tmp
echo "Played $count files" rm tmp
echo "Played $count files"
```
The only problem is for filenames containing line feeds. A `! -name "$(printf "*\n*")"` has been added to simply skip these files, just in case there are any. The only problem is for filenames containing line feeds. A `! -name "$(printf "*\n*")"` has been added to simply skip these files, just in case there are any.
@@ -68,8 +76,10 @@ If you don't need variables to be available after the loop (here, if you don't n
If you don't need a shell script loop body or any form of variable like if we only wanted to play the file, we can dramatically simplify while maintaining POSIX compatibility: If you don't need a shell script loop body or any form of variable like if we only wanted to play the file, we can dramatically simplify while maintaining POSIX compatibility:
# Simple and POSIX ```sh
find mydir -name '*.mp3' -exec play {} \; # Simple and POSIX
find mydir -name '*.mp3' -exec play {} \;
```
This does not allow things like `let counter++` because `let` is a shell builtin, not an external command. This does not allow things like `let counter++` because `let` is a shell builtin, not an external command.
@@ -77,10 +87,12 @@ This does not allow things like `let counter++` because `let` is a shell builtin
If we do need a shell script body but no aggregation, you can do the above but invoking `sh` (this is still POSIX): If we do need a shell script body but no aggregation, you can do the above but invoking `sh` (this is still POSIX):
find mydir -name '*.mp3' -exec sh -c ' ```sh
echo "Playing ${1%.mp3}" find mydir -name '*.mp3' -exec sh -c '
play "$1" echo "Playing ${1%.mp3}"
' sh {} \; play "$1"
' sh {} \;
```
This would not be possible without `sh`, because `${1%.mp3}` is a shell construct that `find` can't evaluate by itself. If we had tried to `let counter++` in this loop, we would have found that the value never changes. This would not be possible without `sh`, because `${1%.mp3}` is a shell construct that `find` can't evaluate by itself. If we had tried to `let counter++` in this loop, we would have found that the value never changes.

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
cp $* ~/dir ```sh
cp $* ~/dir
```
### Correct code: ### Correct code:
cp "$@" ~/dir ```sh
cp "$@" ~/dir
```
### Rationale: ### Rationale:

@@ -2,17 +2,17 @@
### Problematic code: ### Problematic code:
for i in {1..$n} for i in {1..$n}
do do
echo "$i" echo "$i"
done done
### Correct code: ### Correct code:
for ((i=0; i<n; i++)) for ((i=0; i<n; i++))
do do
echo "$i" echo "$i"
done done
### Rationale: ### Rationale:
@@ -20,16 +20,16 @@ In Bash, brace expansion happens before variable expansion. This means that brac
For integers, use an arithmetic for loop instead. For zero-padded numbers or letters, use of eval may be warranted: For integers, use an arithmetic for loop instead. For zero-padded numbers or letters, use of eval may be warranted:
from="a" to="m" from="a" to="m"
for c in $(eval "echo {$from..$to}"); do echo "$c"; done for c in $(eval "echo {$from..$to}"); do echo "$c"; done
or more carefully (if `from`/`to` could be user input, or if the brace expansion could have spaces): or more carefully (if `from`/`to` could be user input, or if the brace expansion could have spaces):
from="a" to="m" from="a" to="m"
while IFS= read -d '' -r c while IFS= read -d '' -r c
do do
echo "Read $c" echo "Read $c"
done < <(eval "printf '%s\0' $(printf "{%q..%q}.jpg" "$from" "$to")") done < <(eval "printf '%s\0' $(printf "{%q..%q}.jpg" "$from" "$to")")
### Exceptions ### Exceptions

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
if [[ $1 != foo || $1 != bar ]] ```sh
then if [[ $1 != foo || $1 != bar ]]
echo "$1 is not foo or bar" then
fi echo "$1 is not foo or bar"
fi
```
### Correct code: ### Correct code:
if [[ $1 != foo && $1 != bar ]] ```sh
then if [[ $1 != foo && $1 != bar ]]
echo "$1 is not foo or bar" then
fi echo "$1 is not foo or bar"
fi
```
### Rationale: ### Rationale:

@@ -2,19 +2,25 @@
### Problematic code: ### Problematic code:
printf "Hello, $NAME\n" ```sh
printf "Hello, $NAME\n"
```
### Correct code: ### Correct code:
printf "Hello, %s\n" "$NAME" ```sh
printf "Hello, %s\n" "$NAME"
```
### Rationale: ### Rationale:
`printf` interprets escape sequences and format specifiers in the format string. If variables are included, any escape sequences or format specifiers in the data will be interpreted too, when you most likely wanted to treat it as data. Example: `printf` interprets escape sequences and format specifiers in the format string. If variables are included, any escape sequences or format specifiers in the data will be interpreted too, when you most likely wanted to treat it as data. Example:
coverage='96%' ```sh
printf "Unit test coverage: %s\n" "$coverage" coverage='96%'
printf "Unit test coverage: $coverage\n" printf "Unit test coverage: %s\n" "$coverage"
printf "Unit test coverage: $coverage\n"
```
The first printf writes `Unit test coverage: 96%`. The first printf writes `Unit test coverage: 96%`.
@@ -24,7 +30,9 @@ The second writes ``bash: printf: `\': invalid format character``
Sometimes you may actually want to interpret data as a format string, like in: Sometimes you may actually want to interpret data as a format string, like in:
hexToAscii() { printf "\x$1"; } ```sh
hexToAscii 21 hexToAscii() { printf "\x$1"; }
hexToAscii 21
```
Like all warnings, you can selectively silence this warning with a [directive](Directive). Like all warnings, you can selectively silence this warning with a [directive](Directive).

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
tr -cd [:digit:] ```sh
tr -cd [:digit:]
```
### Correct code: ### Correct code:
tr -cd '[:digit:]' ```sh
tr -cd '[:digit:]'
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
grep '*foo*' ```sh
grep '*foo*'
```
### Correct code: ### Correct code:
grep 'foo' # or more explicitly, grep '.*foo.*' ```sh
grep 'foo' # or more explicitly, grep '.*foo.*'
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
trap "echo \"Finished on $(date)\"" EXIT ```sh
trap "echo \"Finished on $(date)\"" EXIT
```
### Correct code: ### Correct code:
trap 'echo "Finished on $(date)"' EXIT ```sh
trap 'echo "Finished on $(date)"' EXIT
```
### Rationale: ### Rationale:

@@ -2,30 +2,40 @@
### Problematic code: ### Problematic code:
for s in "$(mycommand)"; do echo "$s"; done ```sh
for s in "$(mycommand)"; do echo "$s"; done
```
### Correct code: ### Correct code:
The correct code depends on your intention. Let's say you're in a directory with the files `file.png` and `My cat.png`, and you want to loop over a command that outputs (or variable that contains): The correct code depends on your intention. Let's say you're in a directory with the files `file.png` and `My cat.png`, and you want to loop over a command that outputs (or variable that contains):
hello world ```sh
My *.png hello world
My *.png
```
#### Loop over each line without globbing (`hello world`, `My *.png`) #### Loop over each line without globbing (`hello world`, `My *.png`)
mycommand | while IFS= read -r s; do echo "$s"; done ```sh
mycommand | while IFS= read -r s; do echo "$s"; done
```
#### Loop over each word with globbing (`hello`, `world`, `My`, `file.png`, `My cat.png`): #### Loop over each word with globbing (`hello`, `world`, `My`, `file.png`, `My cat.png`):
# relies on the fact that IFS by default contains space-tab-linefeed ```sh
for s in $(mycommand); do echo "$s"; done # relies on the fact that IFS by default contains space-tab-linefeed
for s in $(mycommand); do echo "$s"; done
```
#### Loop over each line with globbing (`hello world`, `My cat.png`) #### Loop over each line with globbing (`hello world`, `My cat.png`)
# explicitly set IFS to contain only a line feed ```sh
IFS=' # explicitly set IFS to contain only a line feed
' IFS='
for s in $(mycommand); do echo "$s"; done '
for s in $(mycommand); do echo "$s"; done
```
### Rationale: ### Rationale:

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
find . -type f -exec shellcheck {} | wc -l \; ```sh
find . -exec echo {} ; find . -type f -exec shellcheck {} | wc -l \;
find . -exec echo {} ;
```
### Correct code: ### Correct code:
find . -type f -exec sh -c 'shellcheck "$1" | wc -l' -- {} \; ```sh
find . -exec echo {} \; find . -type f -exec sh -c 'shellcheck "$1" | wc -l' -- {} \;
find . -exec echo {} \;
```
### Rationale: ### Rationale:
@@ -18,11 +22,15 @@
To instead go through each file and run `foo file && bar file` on it, invoke a shell that can interpret `&&`: To instead go through each file and run `foo file && bar file` on it, invoke a shell that can interpret `&&`:
find . -exec sh 'foo "$1" && bar "$1"' -- {} \; ```sh
find . -exec sh 'foo "$1" && bar "$1"' -- {} \;
```
You can also use find `-a` instead of shell `&&`: You can also use find `-a` instead of shell `&&`:
find . -exec foo {} \; -a -exec bar {} \; ```sh
find . -exec foo {} \; -a -exec bar {} \;
```
This will have the same effect (`-a` is also the default when two commands are specified, and can therefore be omitted). This will have the same effect (`-a` is also the default when two commands are specified, and can therefore be omitted).

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
cp $@ ~/dir ```sh
cp $@ ~/dir
```
### Correct code: ### Correct code:
cp "$@" ~/dir ```sh
cp "$@" ~/dir
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
firefox 2>&1 > /dev/null ```sh
firefox 2>&1 > /dev/null
```
### Correct code: ### Correct code:
firefox > /dev/null 2>&1 ```sh
firefox > /dev/null 2>&1
```
### Rationale: ### Rationale:

@@ -2,40 +2,48 @@
### Problematic code: ### Problematic code:
if [ -n $var ] ```sh
then if [ -n $var ]
echo "var has a value" then
else echo "var has a value"
echo "var is empty" else
fi echo "var is empty"
fi
```
### Correct code: ### Correct code:
In bash/ksh: In bash/ksh:
if [[ -n $var ]] ```sh
then if [[ -n $var ]]
echo "var has a value" then
else echo "var has a value"
echo "var is empty" else
fi echo "var is empty"
fi
```
In POSIX: In POSIX:
if [ -n "$var" ] ```sh
then if [ -n "$var" ]
echo "var has a value" then
else echo "var has a value"
echo "var is empty" else
fi echo "var is empty"
fi
```
### Rationale: ### Rationale:
When `$var` is unquoted, a blank value will cause it to wordsplit and disappear. If `$var` is empty, these two statements are identical: When `$var` is unquoted, a blank value will cause it to wordsplit and disappear. If `$var` is empty, these two statements are identical:
[ -n $var ] ```sh
[ -n ] [ -n $var ]
[ -n ]
```
`[ string ]` is shorthand for testing if a string is empty. This is still true if `string` happens to be `-n`. `[ -n ]` is therefore true, and by extension so is `[ -n $var ]`. `[ string ]` is shorthand for testing if a string is empty. This is still true if `string` happens to be `-n`. `[ -n ]` is therefore true, and by extension so is `[ -n $var ]`.

@@ -2,12 +2,16 @@
### Problematic code: ### Problematic code:
[[ 2 -lt 3.14 ]] ```sh
[[ 2 -lt 3.14 ]]
```
### Correct code: ### Correct code:
[[ 200 -lt 314 ]] # Use fixed point math ```sh
[[ $(echo "2 < 3.14" | bc) == 1 ]] # Use bc [[ 200 -lt 314 ]] # Use fixed point math
[[ $(echo "2 < 3.14" | bc) == 1 ]] # Use bc
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[[ $foo =~ "^fo+bar$" ]] ```sh
[[ $foo =~ "^fo+bar$" ]]
```
### Correct code: ### Correct code:
[[ $foo =~ ^fo+bar$ ]] ```sh
[[ $foo =~ ^fo+bar$ ]]
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[[ 0=1 ]] ```sh
[[ 0=1 ]]
```
### Correct code: ### Correct code:
[[ 0 = 1 ]] ```sh
[[ 0 = 1 ]]
```
### Rationale: ### Rationale:

@@ -2,35 +2,43 @@
### Problematic code: ### Problematic code:
var_1="hello world" ```sh
n=1 var_1="hello world"
echo "${var_$n}" n=1
echo "${var_$n}"
```
### Correct code: ### Correct code:
Bash/ksh: Bash/ksh:
# Use arrays instead of dynamic names ```sh
declare -a var # Use arrays instead of dynamic names
var[1]="hello world" declare -a var
n=1 var[1]="hello world"
echo "${var[n]}" n=1
echo "${var[n]}"
```
or or
# Expand variable names dynamically ```sh
var_1="hello world" # Expand variable names dynamically
n=1 var_1="hello world"
name="var_$n" n=1
echo "${!name}" name="var_$n"
echo "${!name}"
```
POSIX sh: POSIX sh:
# Expand dynamically with eval ```sh
var_1="hello world" # Expand dynamically with eval
n=1 var_1="hello world"
eval "tmp=\$var_$n" n=1
echo "${tmp}" eval "tmp=\$var_$n"
echo "${tmp}"
```
### Rationale: ### Rationale:

@@ -2,41 +2,53 @@
### Problematic code: ### Problematic code:
i=4 ```sh
$(( i++ )) i=4
$(( i++ ))
```
### Correct code: ### Correct code:
Bash, Ksh: Bash, Ksh:
i=4 ```sh
(( i++ )) i=4
(( i++ ))
```
POSIX (assuming `++` is supported): POSIX (assuming `++` is supported):
i=4 ```sh
_=$(( i++ )) i=4
_=$(( i++ ))
```
Alternative POSIX version that does not preserve the exit code: Alternative POSIX version that does not preserve the exit code:
: $(( i++ )) ```sh
: $(( i++ ))
```
### Rationale: ### Rationale:
`$((..))` expands to a number. If it's the only word on the line, the shell will try to execute this number as a command name: `$((..))` expands to a number. If it's the only word on the line, the shell will try to execute this number as a command name:
$ i=4 ```sh
$ $(( i++ )) $ i=4
4: command not found $ $(( i++ ))
$ echo $i 4: command not found
5 $ echo $i
5
```
To avoid trying to execute the number as a command name, use one of the methods mentioned: To avoid trying to execute the number as a command name, use one of the methods mentioned:
$ i=4 ```sh
$ _=$(( i++ )) $ i=4
$ echo $i $ _=$(( i++ ))
5 $ echo $i
5
```
### Exceptions: ### Exceptions:

@@ -2,15 +2,18 @@
### Problematic code: ### Problematic code:
ssh host.example.com << EOF ```sh
echo "Logged in on $HOSTNAME" ssh host.example.com << EOF
EOF echo "Logged in on $HOSTNAME"
EOF
### Correct code: ### Correct code:
ssh host.example.com << "EOF" ```sh
echo "Logged in on $HOSTNAME" ssh host.example.com << "EOF"
EOF echo "Logged in on $HOSTNAME"
EOF
```
### Rationale: ### Rationale:
@@ -20,15 +23,19 @@ This means that before sending the commands to the server, the client replaces `
Scripts with any kind of variable use are especially problematic because all references will be expanded before the script run. For example, Scripts with any kind of variable use are especially problematic because all references will be expanded before the script run. For example,
ssh host << EOF ```sh
x="$(uname -a)" ssh host << EOF
echo "$x" x="$(uname -a)"
EOF echo "$x"
EOF
```
will never print anything, neither client nor server details, since before evaluation, it will be expanded to: will never print anything, neither client nor server details, since before evaluation, it will be expanded to:
x="Linux localhost ... x86_64 GNU/Linux" ```sh
echo "" x="Linux localhost ... x86_64 GNU/Linux"
echo ""
```
By quoting the here token, local expansion will not take place, so the server sees `echo "Logged in on $HOSTNAME"` which is expanded and printed with the server's hostname, which is usually the intention. By quoting the here token, local expansion will not take place, so the server sees `echo "Logged in on $HOSTNAME"` which is expanded and printed with the server's hostname, which is usually the intention.
@@ -38,6 +45,8 @@ If the client should expand some or all variables, this message can and should b
To expand a mix of local and remote variables, the here doc end token should be unquoted, and the remote variables should be escaped, e.g. To expand a mix of local and remote variables, the here doc end token should be unquoted, and the remote variables should be escaped, e.g.
ssh host.example.com << EOF ```sh
echo "Logged in on \$HOSTNAME from $HOSTNAME" ssh host.example.com << EOF
EOF echo "Logged in on \$HOSTNAME from $HOSTNAME"
EOF
```

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
rm "~/Desktop/$filename" ```sh
rm "~/Desktop/$filename"
```
### Correct code: ### Correct code:
rm "$HOME/Desktop/$filename" ```sh
rm "$HOME/Desktop/$filename"
```
### Rationale: ### Rationale:

@@ -2,21 +2,27 @@
### Problematic code: ### Problematic code:
args='-lh "My File.txt"' ```sh
ls $args args='-lh "My File.txt"'
ls $args
```
### Correct code: ### Correct code:
args=(-lh "My File.txt") ```sh
ls "${args[@]}" args=(-lh "My File.txt")
ls "${args[@]}"
```
### Rationale: ### Rationale:
Bash does not interpret data as code. Consider almost any other languages, such as Python: Bash does not interpret data as code. Consider almost any other languages, such as Python:
print 1+1 # prints 2 ```sh
a="1+1" print 1+1 # prints 2
print a # prints 1+1, not 2 a="1+1"
print a # prints 1+1, not 2
```
Here, `1+1` is Python syntax for adding numbers. However, passing a literal string containing this expression does not cause Python to interpret it, see the `+` and produce the calculated result. Here, `1+1` is Python syntax for adding numbers. However, passing a literal string containing this expression does not cause Python to interpret it, see the `+` and produce the calculated result.
@@ -26,16 +32,20 @@ The solution is to use an array instead, whenever possible.
If you due to `sh` compatibility can't use arrays, you can use `eval` instead. However, this is very insecure and easy to get wrong, leading to various forms of security vulnerabilities and breakage: If you due to `sh` compatibility can't use arrays, you can use `eval` instead. However, this is very insecure and easy to get wrong, leading to various forms of security vulnerabilities and breakage:
quote() { local q=${1//\'/\'\\\'\'}; echo "'$q'"; } ```sh
args="-lh $(quote "My File.txt")" quote() { local q=${1//\'/\'\\\'\'}; echo "'$q'"; }
eval ls "$args" # Do not use unless you understand implications args="-lh $(quote "My File.txt")"
eval ls "$args" # Do not use unless you understand implications
```
If you ever accidentally forget to use proper quotes, such as with: If you ever accidentally forget to use proper quotes, such as with:
for f in *.txt; do ```sh
args="-lh '$1'" # Example security exploit for f in *.txt; do
eval ls "$args" # Do not copy and use args="-lh '$1'" # Example security exploit
done eval ls "$args" # Do not copy and use
done
```
Then you can use `touch "'; rm -rf \$'\x2F'; '.txt"` (or someone can trick you into downloading a file with this name, or create a zip file or git repo containing it, or changing their nick and have your chat client create the file for a chat log, or...), and running the script to list your files will run the command `rm -rf /`. Then you can use `touch "'; rm -rf \$'\x2F'; '.txt"` (or someone can trick you into downloading a file with this name, or create a zip file or git repo containing it, or changing their nick and have your chat client create the file for a chat log, or...), and running the script to list your files will run the command `rm -rf /`.

@@ -2,17 +2,21 @@
### Problematic code: ### Problematic code:
if $(which epstopdf) ```sh
then if $(which epstopdf)
echo "Found epstopdf" then
fi echo "Found epstopdf"
fi
```
### Correct code: ### Correct code:
if which epstopdf ```sh
then if which epstopdf
echo "Found epstopdf" then
fi echo "Found epstopdf"
fi
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
grep foo file.txt | sed -e 's/foo/bar/g' > file.txt ```sh
grep foo file.txt | sed -e 's/foo/bar/g' > file.txt
```
### Correct code: ### Correct code:
grep foo file.txt | sed -e 's/foo/bar/g' > tmpfile && mv tmpfile file.txt ```sh
grep foo file.txt | sed -e 's/foo/bar/g' > tmpfile && mv tmpfile file.txt
```
### Rationale: ### Rationale:

@@ -2,12 +2,16 @@
### Problematic code: ### Problematic code:
#!/usr/bin/env bash -x ```sh
#!/usr/bin/env bash -x
```
### Correct code: ### Correct code:
#!/usr/bin/env bash ```sh
set -x #!/usr/bin/env bash
set -x
```
### Rationale: ### Rationale:

@@ -2,19 +2,25 @@
### Problematic code: ### Problematic code:
name=World cmd -m "Hello $name" ```sh
name=World cmd -m "Hello $name"
```
### Correct code: ### Correct code:
export name=World ```sh
cmd -m "Hello $name" export name=World
cmd -m "Hello $name"
```
To prevent setting the variable, this can also be done in a subshell: To prevent setting the variable, this can also be done in a subshell:
( ```sh
export name=World (
cmd -m "Hello $name" export name=World
) # 'name' does not leave this subshell cmd -m "Hello $name"
) # 'name' does not leave this subshell
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
gzip file[:digit:]*.txt ```sh
gzip file[:digit:]*.txt
```
### Correct code: ### Correct code:
gzip file[[:digit:]]*.txt ```sh
gzip file[[:digit:]]*.txt
```
### Rationale: ### Rationale:

@@ -2,32 +2,38 @@
### Problematic code: ### Problematic code:
for dir in */ ```sh
do for dir in */
cd "$dir" do
convert index.png index.jpg cd "$dir"
cd .. convert index.png index.jpg
done cd ..
done
```
### Correct code: ### Correct code:
for dir in */ ```sh
do for dir in */
( do
cd "$dir" || exit (
convert index.png index.jpg cd "$dir" || exit
) convert index.png index.jpg
done )
done
```
or or
for dir in */ ```sh
do for dir in */
cd "$dir" || exit do
convert index.png index.jpg cd "$dir" || exit
cd .. convert index.png index.jpg
done cd ..
done
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[ "$1" = "-v" && -z "$2" ] ```sh
[ "$1" = "-v" && -z "$2" ]
```
### Correct code: ### Correct code:
[ "$1" = "-v" ] && [ -z "$2" ] ```sh
[ "$1" = "-v" ] && [ -z "$2" ]
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[[ "$1" = "-v" -a -z "$2" ]] ```sh
[[ "$1" = "-v" -a -z "$2" ]]
```
### Correct code: ### Correct code:
[[ "$1" = "-v" && -z "$2" ]] ```sh
[[ "$1" = "-v" && -z "$2" ]]
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[ "$1" = "-v" || "$1" = "-help" ] ```sh
[ "$1" = "-v" || "$1" = "-help" ]
```
### Correct code: ### Correct code:
[ "$1" = "-v" ] || [ "$1" = "-help" ] ```sh
[ "$1" = "-v" ] || [ "$1" = "-help" ]
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[[ "$1" = "-v" -o "$1" = "-help" ]] ```sh
[[ "$1" = "-v" -o "$1" = "-help" ]]
```
### Correct code: ### Correct code:
[[ "$1" = "-v" || "$1" = "-help" ]] ```sh
[[ "$1" = "-v" || "$1" = "-help" ]]
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
rm -rf /usr /lib/nvidia-current/xorg/xorg ```sh
rm -rf /usr /lib/nvidia-current/xorg/xorg
```
### Correct code: ### Correct code:
rm -rf /usr/lib/nvidia-current/xorg/xorg ```sh
rm -rf /usr/lib/nvidia-current/xorg/xorg
```
### Rationale: ### Rationale:
@@ -18,6 +22,8 @@ Due to an accidental space, it deleted `/usr` instead of just the particular dir
In cases of chroot, initramfs and similar, it's reasonable to delete otherwise important directories. Due to this, Shellcheck will not warn if the command contains `--`: In cases of chroot, initramfs and similar, it's reasonable to delete otherwise important directories. Due to this, Shellcheck will not warn if the command contains `--`:
rm -rf -- /usr ```sh
rm -rf -- /usr
```
This is an arbitrary convention to allow deleting such directories without having to use a [[directive]] to silence the warning. This is an arbitrary convention to allow deleting such directories without having to use a [[directive]] to silence the warning.

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
rm -rf "$STEAMROOT/"* ```sh
rm -rf "$STEAMROOT/"*
```
### Correct code: ### Correct code:
rm -rf "${STEAMROOT:?}/"* ```sh
rm -rf "${STEAMROOT:?}/"*
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
a=$(echo $?) ```sh
a=$(echo $?)
```
### Correct code: ### Correct code:
a="$?" ```sh
a="$?"
```
### Rationale: ### Rationale:

@@ -2,14 +2,18 @@
### Problematic code: ### Problematic code:
whoami ```sh
su whoami
whoami su
whoami
```
### Correct code: ### Correct code:
whoami ```sh
sudo whoami whoami
sudo whoami
```
### Rationale: ### Rationale:

@@ -2,19 +2,23 @@
### Problematic code: ### Problematic code:
sayhello() { ```sh
echo "Hello $1" sayhello() {
} echo "Hello $1"
sayhello }
sayhello
```
`./myscript World` just prints "Hello " instead of "Hello World". `./myscript World` just prints "Hello " instead of "Hello World".
### Correct code: ### Correct code:
sayhello() { ```sh
echo "Hello $1" sayhello() {
} echo "Hello $1"
sayhello "$@" }
sayhello "$@"
```
`./myscript World` now prints "Hello World". `./myscript World` now prints "Hello World".
@@ -26,9 +30,11 @@ If you want to process your script's parameters in a function, you have to expli
Note that `"$@"` refers to the current context's positional parameters, so if you call a function from a function, you have to pass in `"$@"` to both of them: Note that `"$@"` refers to the current context's positional parameters, so if you call a function from a function, you have to pass in `"$@"` to both of them:
first() { second "$@"; } ```sh
second() { echo "The first script parameter is: $1"; } first() { second "$@"; }
first "$@" second() { echo "The first script parameter is: $1"; }
first "$@"
```
### Exceptions ### Exceptions

@@ -2,12 +2,16 @@
### Problematic code: ### Problematic code:
set var=42 ```sh
set var 42 set var=42
set var 42
```
### Correct code: ### Correct code:
var=42 ```sh
var=42
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[[ a &lt;= b ]] ```sh
[[ a &lt;= b ]]
```
### Correct code: ### Correct code:
[[ ! a > b ]] ```sh
[[ ! a > b ]]
```
### Rationale: ### Rationale:

@@ -2,20 +2,26 @@
### Problematic code: ### Problematic code:
PATH=/my/dir ```sh
cat "$PATH/myfile" PATH=/my/dir
cat "$PATH/myfile"
```
### Correct code: ### Correct code:
Good practice: always use lowercase for unexported variables. Good practice: always use lowercase for unexported variables.
path=/my/dir ```sh
cat "$path/myfile" path=/my/dir
cat "$path/myfile"
```
Bad practice: use another uppercase name. Bad practice: use another uppercase name.
MYPATH=/my/dir ```sh
cat "$MYPATH/myfile" MYPATH=/my/dir
cat "$MYPATH/myfile"
```
### Rationale: ### Rationale:

@@ -2,25 +2,33 @@
### Problematic code: ### Problematic code:
var=$@ ```sh
for i in $var; do ..; done var=$@
for i in $var; do ..; done
```
or or
set -- Hello World ```sh
msg=$@ set -- Hello World
echo "You said $msg" msg=$@
echo "You said $msg"
```
### Correct code: ### Correct code:
var=( "$@" ) ```sh
for i in "${var[@]}"; do ..; done var=( "$@" )
for i in "${var[@]}"; do ..; done
```
or or
set -- Hello World ```sh
msg=$* set -- Hello World
echo "You said $msg" msg=$*
echo "You said $msg"
```
### Rationale: ### Rationale:

@@ -2,13 +2,17 @@
### Problematic code: ### Problematic code:
foo={1..9} ```sh
echo $foo foo={1..9}
echo $foo
```
### Correct code: ### Correct code:
foo=( {1..9} ) ```sh
echo "${foo[@]}" foo=( {1..9} )
echo "${foo[@]}"
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
grep foo | wc -l ```sh
grep foo | wc -l
```
### Correct code: ### Correct code:
grep -c foo ```sh
grep -c foo
```
### Rationale: ### Rationale:
@@ -14,10 +18,12 @@ This is purely a stylistic issue. `grep` can count lines without piping to `wc`.
Note that in many cases, this number is only used to see whether there are matches (i.e. `> 0`). In these cases, it's better and more efficient to use `grep -q` and check its exit status: Note that in many cases, this number is only used to see whether there are matches (i.e. `> 0`). In these cases, it's better and more efficient to use `grep -q` and check its exit status:
if grep -q pattern file ```sh
then if grep -q pattern file
echo "The file contains the pattern" then
fi echo "The file contains the pattern"
fi
```
### Exceptions ### Exceptions

@@ -2,19 +2,23 @@
### Problematic code: ### Problematic code:
myarray=(foo bar) ```sh
for f in $myarray myarray=(foo bar)
do for f in $myarray
cat "$f" do
done cat "$f"
done
```
### Correct code: ### Correct code:
myarray=(foo bar) ```sh
for f in "${myarray[@]}" myarray=(foo bar)
do for f in "${myarray[@]}"
cat "$f" do
done cat "$f"
done
```
### Rationale: ### Rationale:

@@ -2,18 +2,22 @@
### Problematic code: ### Problematic code:
echo foo >> file ```sh
date >> file echo foo >> file
cat stuff >> file date >> file
cat stuff >> file
```
### Correct code: ### Correct code:
{ ```sh
echo foo {
date echo foo
cat stuff date
} >> file cat stuff
} >> file
```
### Rationale: ### Rationale:

@@ -2,11 +2,15 @@
### Problematic code: ### Problematic code:
[[ $foo -eq "Y" ]] ```sh
[[ $foo -eq "Y" ]]
```
### Correct code: ### Correct code:
[[ $foo = "Y" ]] ```sh
[[ $foo = "Y" ]]
```
### Rationale: ### Rationale:

Some files were not shown because too many files have changed in this diff Show More