From e073e65e5e29c08a1deb5219cd86411c57b7787b Mon Sep 17 00:00:00 2001 From: Vidar Holen Date: Sat, 3 Oct 2015 21:33:27 -0700 Subject: [PATCH] Use triple backticks rather than indenting for code. --- CentOS6.md | 2 +- Directive.md | 2 +- Home.mediawiki | 2 +- JUnit.md | 2 +- Parser-error.md | 8 ++-- SC1000.md | 10 +++-- SC1001.md | 20 +++++++--- SC1007.md | 28 +++++++++----- SC1009.md | 2 +- SC1010.md | 6 +-- SC1015.md | 14 ++++--- SC1016.md | 10 +++-- SC1018.md | 2 +- SC1035.md | 2 +- SC1037.md | 14 ++++--- SC1038.md | 22 ++++++----- SC1040.md | 26 ++++++++----- SC1045.md | 10 +++-- SC1065.md | 22 ++++++----- SC1066.md | 20 ++++++---- SC1068.md | 10 +++-- SC1072.md | 2 +- SC1077.md | 24 ++++++++---- SC1078.md | 28 +++++++++----- SC1079.md | 2 +- SC1081.md | 22 ++++++----- SC1083.md | 20 +++++++--- SC1084.md | 16 +++++--- SC1086.md | 22 ++++++----- SC1087.md | 10 +++-- SC1088.md | 18 ++++++--- SC1089.md | 36 +++++++++-------- SC1090.md | 14 ++++--- SC1091.md | 12 ++++-- SC1094.md | 12 ++++-- SC1097.md | 16 ++++++-- SC1098.md | 10 +++-- SC1099.md | 24 +++++++----- SC2001.md | 12 ++++-- SC2002.md | 4 +- SC2003.md | 18 +++++---- SC2004.md | 22 +++++++---- SC2006.md | 14 ++++--- SC2008.md | 12 ++++-- SC2009.md | 16 ++++++-- SC2010.md | 2 +- SC2012.md | 26 ++++++++----- SC2013.md | 54 ++++++++++++++++---------- SC2014.md | 20 ++++++---- SC2015.md | 22 ++++++----- SC2016.md | 20 ++++++---- SC2017.md | 14 ++++--- SC2020.md | 14 ++++--- SC2022.md | 14 ++++--- SC2024.md | 16 ++++---- SC2025.md | 10 +++-- SC2026.md | 36 ++++++++++------- SC2027.md | 18 ++++++--- SC2028.md | 20 ++++++---- SC2029.md | 20 +++++++--- SC2030.md | 2 +- SC2031.md | 44 +++++++++++++-------- SC2032.md | 2 +- SC2033.md | 22 +++++++---- SC2034.md | 38 +++++++++++------- SC2036.md | 12 ++++-- SC2038.md | 10 +++-- SC2039.md | 6 +-- SC2040.md | 2 +- SC2041.md | 22 ++++++----- SC2043.md | 32 ++++++++++------ SC2044.md | 100 +++++++++++++++++++++++++++--------------------- SC2048.md | 10 +++-- SC2051.md | 34 ++++++++-------- SC2055.md | 22 ++++++----- SC2059.md | 26 ++++++++----- SC2060.md | 10 +++-- SC2063.md | 14 ++++--- SC2064.md | 12 ++++-- SC2065.md | 2 +- SC2066.md | 32 ++++++++++------ SC2067.md | 24 ++++++++---- SC2068.md | 10 +++-- SC2069.md | 14 ++++--- SC2070.md | 50 ++++++++++++++---------- SC2072.md | 12 ++++-- SC2076.md | 10 +++-- SC2077.md | 14 ++++--- SC2082.md | 52 ++++++++++++++----------- SC2084.md | 46 ++++++++++++++-------- SC2087.md | 51 ++++++++++++++---------- SC2088.md | 12 ++++-- SC2089.md | 44 +++++++++++++-------- SC2090.md | 2 +- SC2091.md | 24 +++++++----- SC2092.md | 2 +- SC2094.md | 16 +++++--- SC2096.md | 14 ++++--- SC2097.md | 22 +++++++---- SC2098.md | 2 +- SC2101.md | 12 ++++-- SC2103.md | 50 +++++++++++++----------- SC2107.md | 10 +++-- SC2108.md | 10 +++-- SC2109.md | 10 +++-- SC2110.md | 10 +++-- SC2114.md | 14 +++++-- SC2115.md | 12 ++++-- SC2116.md | 10 +++-- SC2117.md | 18 +++++---- SC2119.md | 2 +- SC2120.md | 30 +++++++++------ SC2121.md | 12 ++++-- SC2122.md | 12 ++++-- SC2123.md | 20 ++++++---- SC2124.md | 32 ++++++++++------ SC2125.md | 18 +++++---- SC2126.md | 22 +++++++---- SC2128.md | 26 +++++++------ SC2129.md | 24 +++++++----- SC2130.md | 8 +++- SC2139.md | 10 +++-- SC2140.md | 38 ++++++++++++------ SC2141.md | 10 +++-- SC2142.md | 12 ++++-- SC2143.md | 24 +++++++----- SC2144.md | 32 +++++++++------- SC2145.md | 14 ++++--- SC2146.md | 16 +++++--- SC2147.md | 12 ++++-- SC2148.md | 22 +++++++---- SC2149.md | 47 ++++++++++++++--------- SC2150.md | 22 +++++++---- SC2151.md | 24 +++++++----- SC2152.md | 26 ++++++++----- SC2153.md | 14 ++++--- SC2154.md | 42 ++++++++++++-------- SC2155.md | 18 ++++++--- SC2156.md | 12 ++++-- SC2157.md | 22 ++++++----- SC2158.md | 24 +++++++----- SC2159.md | 24 +++++++----- SC2160.md | 24 +++++++----- SC2161.md | 24 +++++++----- SC2162.md | 14 ++++--- SC2163.md | 22 +++++++---- SC2164.md | 16 +++++--- SC2165.md | 34 ++++++++-------- SC2166.md | 12 ++++-- Sc2035.md | 2 +- Sc2045.md | 4 +- Sc2046.md | 8 ++-- Sc2086.md | 2 +- 153 files changed, 1741 insertions(+), 1049 deletions(-) diff --git a/CentOS6.md b/CentOS6.md index 86263f3..d58d21b 100644 --- a/CentOS6.md +++ b/CentOS6.md @@ -8,7 +8,7 @@ yum groupinstall "Development Tools" -y yum install gmp-devel # may need epel sudo ln -s /usr/lib64/libgmp.so.3 /usr/lib64/libgmp.so.10 -# From https://www.haskell.org/platform/linux.html#linux-generic +# From https://www.haskell.org/platform/linux.html#linux-generic wget https://haskell.org/platform/download/7.10.2/haskell-platform-7.10.2-a-unknown-linux-deb7.tar.gz tar xvzf haskell-platform-7.10.2-a-unknown-linux-deb7.tar.gz ./install-haskell-platform.sh diff --git a/Directive.md b/Directive.md index 652c87c..4fc6cca 100644 --- a/Directive.md +++ b/Directive.md @@ -17,4 +17,4 @@ and `source` to tell ShellCheck where to find a sourced file: Directives are scoped to the structure that follows it. For example, before a function it silences all warnings (or overrides all `source` statements) in the function Before a case statement, it silences all warnings in all branches of the case statement. -Silencing parser errors is purely cosmetic, and will not make ShellCheck continue. \ No newline at end of file +Silencing parser errors is purely cosmetic, and will not make ShellCheck continue. \ No newline at end of file diff --git a/Home.mediawiki b/Home.mediawiki index cfcc70c..864d233 100644 --- a/Home.mediawiki +++ b/Home.mediawiki @@ -1,3 +1,3 @@ -Welcome to the ShellCheck wiki! Any help with creating pages explaining errors is greatly appreciated! +Welcome to the ShellCheck wiki! Any help with creating pages explaining errors is greatly appreciated! Just copy the [[Template]] and give it a name like [[SC1000]], and bask in unending appreciation from shell scripters world wide! \ No newline at end of file diff --git a/JUnit.md b/JUnit.md index e874e49..ec95fd7 100644 --- a/JUnit.md +++ b/JUnit.md @@ -1,6 +1,6 @@ ## Getting JUnit XML from ShellCheck -ShellCheck does not have a JUnit XML formatter, but here you can find `checkstyle2junit.xslt`, a XSLT program that converts from Checkstyle XML output to JUnit XML. +ShellCheck does not have a JUnit XML formatter, but here you can find `checkstyle2junit.xslt`, a XSLT program that converts from Checkstyle XML output to JUnit XML. Here's shellcheck's checkstyle XML output: diff --git a/Parser-error.md b/Parser-error.md index 275bf8d..a42089f 100644 --- a/Parser-error.md +++ b/Parser-error.md @@ -8,12 +8,12 @@ Consider this script, with a missing double quote on line 1: echo "Finished" Bash says: - + file: line 2: unexpected EOF while looking for matching `"' file: line 3: syntax error: unexpected end of file Shellcheck says: - + In file line 1: ssh host "$cmd ^-- SC1009: The mentioned parser error was in this simple command. @@ -27,6 +27,6 @@ Shellcheck says: 1. One error showing the direct problem (SC1072, unexpected eof) 1. One error showing the construct being parsed (SC1073) 1. One info showing the outer construct being parsed (SC1009) -1. Potentially some specific suggestions, such as when missing a `fi`. +1. Potentially some specific suggestions, such as when missing a `fi`. -Here, shellcheck says that the command on line 1 is faulty, which makes it easier to find and fix the actual problem. \ No newline at end of file +Here, shellcheck says that the command on line 1 is faulty, which makes it easier to find and fix the actual problem. \ No newline at end of file diff --git a/SC1000.md b/SC1000.md index 8d3597f..e216366 100644 --- a/SC1000.md +++ b/SC1000.md @@ -2,16 +2,20 @@ ### Problematic code: - echo "$" +```sh +echo "$" +``` ### Correct code: - echo "\$" +```sh +echo "\$" +``` ### Rationale: `$` is special in double quotes, but there are some cases where it's interpreted literally: 1. Following a backslash: `echo "\$"` -2. In a context where the shell can't make sense of it, such as at the end of the string, (`"foo$"`) or before some constructs (`"$'foo'"`). +2. In a context where the shell can't make sense of it, such as at the end of the string, (`"foo$"`) or before some constructs (`"$'foo'"`). To avoid relying on strange and shell specific behavior, any `$` intended to be literal should be escaped with a backslash. diff --git a/SC1001.md b/SC1001.md index 7debdee..d3d702a 100644 --- a/SC1001.md +++ b/SC1001.md @@ -2,19 +2,27 @@ ### Problematic code: - echo Yay \o/ +```sh +echo Yay \o/ +``` or - \git status +```sh +\git status +``` ### Correct code: - echo 'Yay \o/' +```sh +echo 'Yay \o/' +``` or - command git status +```sh +command git status +``` ### Rationale: @@ -22,8 +30,8 @@ Escaping something that doesn't need escaping sometimes indicates a bug. If the backslash was supposed to be literal, single quote it. -If the purpose is to run an external command rather than an alias, prefer `command`. +If the purpose is to run an external command rather than an alias, prefer `command`. ### Exceptions -If you have an alias and a function (as opposed to an external command), you can either ignore this message or use `"name"` instead of `\name` to quiet ShellCheck. \ No newline at end of file +If you have an alias and a function (as opposed to an external command), you can either ignore this message or use `"name"` instead of `\name` to quiet ShellCheck. diff --git a/SC1007.md b/SC1007.md index febc449..08b6f3e 100644 --- a/SC1007.md +++ b/SC1007.md @@ -2,19 +2,27 @@ ### Problematic code: - # I want programs to show text in dutch! - LANGUAGE= nl +```sh +# I want programs to show text in dutch! +LANGUAGE= nl +``` - # I want to run the nl command with English error messages! - LANGUAGE= nl +```sh +# I want to run the nl command with English error messages! +LANGUAGE= nl +``` ### Correct code: - # I want programs to show text in dutch! - LANGUAGE=nl +```sh +# I want programs to show text in dutch! +LANGUAGE=nl +``` - # I want to run the nl command with English error messages! - LANGUAGE='' nl +```sh +# I want to run the nl command with English error messages! +LANGUAGE='' nl +``` ### Rationale: @@ -22,7 +30,7 @@ It's easy to think that `LANGUAGE= nl` would assign `"nl"` to the variable `LANG Instead, it runs `nl` (the "number lines" command) and sets `LANGUAGE` to an empty string in its environment. -Since trying to assign values this way is a common mistake, ShellCheck warns about it and asks you to be explicit when assigning empty strings (except for `IFS`, due to the common `IFS= read ..` idiom). +Since trying to assign values this way is a common mistake, ShellCheck warns about it and asks you to be explicit when assigning empty strings (except for `IFS`, due to the common `IFS= read ..` idiom). ### Exceptions -If you're familiar with this behavior and feel that the explicit version is unnecessary, you can [[ignore]] it. \ No newline at end of file +If you're familiar with this behavior and feel that the explicit version is unnecessary, you can [[ignore]] it. diff --git a/SC1009.md b/SC1009.md index 49d8bd5..871f03b 100644 --- a/SC1009.md +++ b/SC1009.md @@ -1,3 +1,3 @@ # The mentioned parser error was in ... -This info warning points to the start of what ShellCheck was parsing when it failed. See [Parser Error](Parser Error) for example and information. \ No newline at end of file +This info warning points to the start of what ShellCheck was parsing when it failed. See [Parser Error](Parser Error) for example and information. diff --git a/SC1010.md b/SC1010.md index 9f7dbc4..40fc864 100644 --- a/SC1010.md +++ b/SC1010.md @@ -2,11 +2,11 @@ ### Problematic code: - for f in *; do echo "$f" done +for f in *; do echo "$f" done ### Correct code: - for f in *; do echo "$f"; done +for f in *; do echo "$f"; done ### Rationale: @@ -14,4 +14,4 @@ ### Exceptions -If you're intentionally using `done` as a literal, you can quote it to make this clear to shellcheck (and also human readers), e.g. instead of `echo Task is done`, use `echo "Task is done"`. This makes no difference to the shell, but it will silence this warning. \ No newline at end of file +If you're intentionally using `done` as a literal, you can quote it to make this clear to shellcheck (and also human readers), e.g. instead of `echo Task is done`, use `echo "Task is done"`. This makes no difference to the shell, but it will silence this warning. diff --git a/SC1015.md b/SC1015.md index 90e6f68..0491cde 100644 --- a/SC1015.md +++ b/SC1015.md @@ -2,18 +2,22 @@ ### Problematic code: - echo “hello world” +```sh +echo “hello world” +``` ### Correct code: - echo "hello world" +```sh +echo "hello world" +``` ### Rationale: -Blog software and word processors frequently replaces ASCII quotes `""` with fancy Unicode quotes, `“”`. To bash, Unicode quotes are considered regular literals and not quotes at all. +Blog software and word processors frequently replaces ASCII quotes `""` with fancy Unicode quotes, `“”`. To bash, Unicode quotes are considered regular literals and not quotes at all. -Simply delete them and retype them in your editor. +Simply delete them and retype them in your editor. ### Exceptions -If you really want literal Unicode double quotes, you can put them in single quotes (or unicode single quotes in double quotes) to make shellcheck ignore them. \ No newline at end of file +If you really want literal Unicode double quotes, you can put them in single quotes (or unicode single quotes in double quotes) to make shellcheck ignore them. diff --git a/SC1016.md b/SC1016.md index 67991d3..d8976b5 100644 --- a/SC1016.md +++ b/SC1016.md @@ -2,11 +2,15 @@ ### Problematic code: - echo ‘hello world’ +```sh +echo ‘hello world’ +``` ### Correct code: - echo 'hello world' +```sh +echo 'hello world' +``` ### Rationale: @@ -14,4 +18,4 @@ Some software, like OS X, Word and Wordpress, may automatically replace your reg ### Exceptions -None \ No newline at end of file +None diff --git a/SC1018.md b/SC1018.md index 62a7177..b908900 100644 --- a/SC1018.md +++ b/SC1018.md @@ -4,4 +4,4 @@ You copy-pasted some code, probably from a blog or web site, which for formattin To humans, a zero-width space is invisible and a non-breaking space is indistinguishable from a regular space, but the shell does not agree. -If you have just a few, delete the indiciated space/word and retype it. If you have tons, do a search&replace in your editor (copy-paste an offending space into the search field, and type a regular space into the replace field), or use `sed -e $'s/\xC2\xA0/ /g' -e $'s/\xE2\x80\x8b//g' -i yourfile` to remove them. \ No newline at end of file +If you have just a few, delete the indiciated space/word and retype it. If you have tons, do a search&replace in your editor (copy-paste an offending space into the search field, and type a regular space into the replace field), or use `sed -e $'s/\xC2\xA0/ /g' -e $'s/\xE2\x80\x8b//g' -i yourfile` to remove them. diff --git a/SC1035.md b/SC1035.md index 2e7d43b..48f2722 100644 --- a/SC1035.md +++ b/SC1035.md @@ -17,4 +17,4 @@ Bourne shells are very whitespace sensitive. Adding or removing spaces can drast ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC1037.md b/SC1037.md index 18d493b..3ac1842 100644 --- a/SC1037.md +++ b/SC1037.md @@ -2,13 +2,17 @@ ### Problematic code: - echo "Ninth parameter: $9" - echo "Tenth parameter: $10" +```sh +echo "Ninth parameter: $9" +echo "Tenth parameter: $10" +``` ### Correct code: - echo "Ninth parameter: $9" - echo "Tenth parameter: ${10}" +```sh +echo "Ninth parameter: $9" +echo "Tenth parameter: ${10}" +``` ### Rationale: @@ -18,4 +22,4 @@ Curly braces are needed to tell the shell that both digits are part of the param ### Exceptions -If you wanted the trailing digits to be literal, `${1}0` will make this clear to both humans and shellcheck. \ No newline at end of file +If you wanted the trailing digits to be literal, `${1}0` will make this clear to both humans and shellcheck. diff --git a/SC1038.md b/SC1038.md index 6685534..8592c17 100644 --- a/SC1038.md +++ b/SC1038.md @@ -2,17 +2,21 @@ ### Problematic code: - while IFS= read -r line - do - printf "%q\n" "$line" - done <<(curl -s http://example.com) +```sh +while IFS= read -r line +do + printf "%q\n" "$line" +done <<(curl -s http://example.com) +``` ### Correct code: - while IFS= read -r line - do - printf "%q\n" "$line" - done < <(curl -s http://example.com) +```sh +while IFS= read -r line +do + printf "%q\n" "$line" +done < <(curl -s http://example.com) +``` ### Rationale: @@ -22,4 +26,4 @@ You probably meant to redirect `<` from process substitution `<(..)` instead. To ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC1040.md b/SC1040.md index 203f942..6e9e72c 100644 --- a/SC1040.md +++ b/SC1040.md @@ -4,23 +4,29 @@ Any code using `<<-` that is indented with spaces. `cat -T script` shows - cat <<- foo - Hello world - foo +```sh + cat <<- foo + Hello world + foo +``` ### Correct code: Code using `<<-` must be indented with tabs. `cat -T script` shows - ^Icat <<- foo - ^I^IHello world - ^Ifoo +```sh +^Icat <<- foo +^I^IHello world +^Ifoo +``` Or simply don't indent the end token: - cat <<- foo - Hello World - foo +```sh + cat <<- foo + Hello World +foo +``` ### Rationale: @@ -30,4 +36,4 @@ Your editor may be automatically replacing tabs with spaces, either when you typ ### Exceptions -None. But note that copy-pasting code to [shellcheck.net](http://www.shellcheck.net) may also turn correct tabs into spaces on some OS. \ No newline at end of file +None. But note that copy-pasting code to [shellcheck.net](http://www.shellcheck.net) may also turn correct tabs into spaces on some OS. diff --git a/SC1045.md b/SC1045.md index 5d1cad3..3daa222 100644 --- a/SC1045.md +++ b/SC1045.md @@ -2,11 +2,15 @@ ### Problematic code: - foo &; bar +```sh +foo &; bar +``` ### Correct code: - foo & bar +```sh +foo & bar +``` ### Rationale: @@ -16,4 +20,4 @@ Both `&` and `;` terminate the command. You should only use one of them. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC1065.md b/SC1065.md index e7be955..4add1cf 100644 --- a/SC1065.md +++ b/SC1065.md @@ -2,17 +2,21 @@ ### Problematic code: - foo(input) { - echo "$input" - } - foo("hello world"); +```sh +foo(input) { + echo "$input" +} +foo("hello world"); +``` ### Correct code: - foo() { - echo "$1" - } - foo "hello world" +```sh +foo() { + echo "$1" +} +foo "hello world" +``` ### Rationale: @@ -23,4 +27,4 @@ Shell script functions behave just like scripts and other commands: ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC1066.md b/SC1066.md index 4001ebb..5b9754b 100644 --- a/SC1066.md +++ b/SC1066.md @@ -2,22 +2,28 @@ ### Problematic code: - $greeting="Hello World" +```sh +$greeting="Hello World" +``` ### 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`: - name=foo - declare "$name=hello world" - echo "$foo" +```sh +name=foo +declare "$name=hello world" +echo "$foo" +``` ### Rationale: -Unlike Perl or PHP, `$` is not used when assigning to a variable. +Unlike Perl or PHP, `$` is not used when assigning to a variable. ### Exceptions -None \ No newline at end of file +None diff --git a/SC1068.md b/SC1068.md index 1583ef5..854d958 100644 --- a/SC1068.md +++ b/SC1068.md @@ -2,11 +2,15 @@ ### Problematic code: - foo = 42 +```sh +foo = 42 +``` ### Correct code: - foo=42 +```sh +foo=42 +``` ### Rationale: @@ -14,4 +18,4 @@ Shells are space sensitive. `foo=42` means to assign `42` to the variable `foo`. ### Exceptions -If you actually wanted to run a command named foo and provide `=` as the first argument, simply quote it to make ShellCheck be quiet: `foo "=" 42`. \ No newline at end of file +If you actually wanted to run a command named foo and provide `=` as the first argument, simply quote it to make ShellCheck be quiet: `foo "=" 42`. diff --git a/SC1072.md b/SC1072.md index 25a0c91..abc71d0 100644 --- a/SC1072.md +++ b/SC1072.md @@ -1,3 +1,3 @@ # Unexpected .. -See [Parser Error](Parser Error). \ No newline at end of file +See [Parser Error](Parser Error). diff --git a/SC1077.md b/SC1077.md index 5c73e0f..cff6e90 100644 --- a/SC1077.md +++ b/SC1077.md @@ -3,27 +3,35 @@ ### Problematic code: - echo "Your username is ´whoami´" +```sh +echo "Your username is ´whoami´" +``` ### Correct code: - echo "Your username is $(whoami)" # Preferred - echo "Your username is `whoami`" # Deprecated, will give [SC2006] +```sh +echo "Your username is $(whoami)" # Preferred +echo "Your username is `whoami`" # Deprecated, will give [SC2006] +``` ### Rationale: -In some fonts it's hard to tell ticks apart, but Bash strongly distinguishes between backticks (grave accent `` ` ``), forward ticks (acute accent `´`) and regular ticks (apostrophe `'`). +In some fonts it's hard to tell ticks apart, but Bash strongly distinguishes between backticks (grave accent `` ` ``), forward ticks (acute accent `´`) and regular ticks (apostrophe `'`). -Backticks start command expansions, while forward ticks are literal. To help spot bugs, ShellCheck parses backticks and forward ticks interchangeably. +Backticks start command expansions, while forward ticks are literal. To help spot bugs, ShellCheck parses backticks and forward ticks interchangeably. ### Exceptions 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: - 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 `` `..` ``. \ No newline at end of file +To nest forward ticks in command expansion, use `$(..)` instead of `` `..` ``. diff --git a/SC1078.md b/SC1078.md index 447f33e..c8a6b53 100644 --- a/SC1078.md +++ b/SC1078.md @@ -2,17 +2,21 @@ ### Problematic code: - greeting="hello - target="world" +```sh +greeting="hello +target="world" +``` ### Correct code: - greeting="hello" - target="world" +```sh +greeting="hello" +target="world" +``` ### Rationale: -The first line is missing a quote. +The first line is missing a quote. ShellCheck warns when it detects multi-line double quoted, single quoted or backticked strings when the character that follows it looks out of place (and gives a companion warning [[SC1079]] at that spot). @@ -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. - var='multiline - 'value +```sh +var='multiline +'value +``` can be rewritten for readability and to remove the warning: - var='multiline - value' +```sh +var='multiline +value' +``` -As always `` `..` `` should be rewritten to ``$(..)``. \ No newline at end of file +As always `` `..` `` should be rewritten to ``$(..)``. diff --git a/SC1079.md b/SC1079.md index eaebc66..11fa9d2 100644 --- a/SC1079.md +++ b/SC1079.md @@ -1,3 +1,3 @@ ## This is actually an end quote, but due to next char it looks suspect. -See companion warning [[SC1078]]. \ No newline at end of file +See companion warning [[SC1078]]. diff --git a/SC1081.md b/SC1081.md index 288e056..8464b89 100644 --- a/SC1081.md +++ b/SC1081.md @@ -2,17 +2,21 @@ ### Problematic code: - If true - Then - echo "hello" - Fi +```sh +If true +Then + echo "hello" +Fi +``` ### Correct code: - if true - then - echo "hello" - fi +```sh +if true +then + echo "hello" +fi +``` ### Rationale: @@ -20,4 +24,4 @@ Shells are case sensitive and do not accept `If` or `IF` in place of lowercase ` ### Exceptions -If you're aware of this and insist on naming a function `WHILE`, you can quote the name to prevent shellcheck from thinking you meant `while`. \ No newline at end of file +If you're aware of this and insist on naming a function `WHILE`, you can quote the name to prevent shellcheck from thinking you meant `while`. diff --git a/SC1083.md b/SC1083.md index a53e8fe..e073872 100644 --- a/SC1083.md +++ b/SC1083.md @@ -2,26 +2,33 @@ ### Problematic code: - rmf() { rm -f "$@" } +```sh +rmf() { rm -f "$@" } +``` or - eval echo \${foo} +```sh +eval echo \${foo} +``` ### Correct code: - rmf() { rm -f "$@"; } +```sh +rmf() { rm -f "$@"; } +``` and - eval "echo \${foo}" +```sh +eval "echo \${foo}" ### Rationale: Curly brackets are normally used as syntax in parameter expansion, command grouping and brace expansion. However, if they don't appear alone at the start of an expression or as part of a parameter or brace expansion, the shell silently treats them as literals. This frequently indicates a bug, so ShellCheck warns about it. -In the example function, the `}` is literal because it's not at the start of an expression. We fix it by adding a `;` before it. +In the example function, the `}` is literal because it's not at the start of an expression. We fix it by adding a `;` before it. In the example eval, the code works fine. However, we can quiet the warning and follow good practice by adding quotes around the literal data. @@ -29,4 +36,5 @@ ShellCheck does not warn about `{}`, since this is frequently used with `find` a ### 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}'`. \ No newline at end of file +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}'`. +``` diff --git a/SC1084.md b/SC1084.md index b74f6c4..9995bec 100644 --- a/SC1084.md +++ b/SC1084.md @@ -2,18 +2,22 @@ ### Problematic code: - !#/bin/sh - echo "Hello World" +```sh +!#/bin/sh +echo "Hello World" +``` ### Correct code: - #!/bin/sh - echo "Hello World" +```sh +#!/bin/sh +echo "Hello World" +``` ### Rationale: -The shebang has been accidentally swapped. The `#` should come first: `#!`, not `!#`. +The shebang has been accidentally swapped. The `#` should come first: `#!`, not `!#`. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC1086.md b/SC1086.md index 243e4b4..1b29c03 100644 --- a/SC1086.md +++ b/SC1086.md @@ -2,17 +2,21 @@ ### Problematic code: - for $var in * - do - echo "$var" - done +```sh +for $var in * +do + echo "$var" +done +``` ### Correct code: - for var in * - do - echo "$var" - done +```sh +for var in * +do + echo "$var" +done +``` ### Rationale: @@ -22,4 +26,4 @@ The `for` loop expects the variable's name, not its value (and the name can not ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC1087.md b/SC1087.md index 35817f0..c817623 100644 --- a/SC1087.md +++ b/SC1087.md @@ -2,11 +2,15 @@ ### Problematic code: - echo "$array[@]" +```sh +echo "$array[@]" +``` ### Correct code: - echo "${array[@]}" +```sh +echo "${array[@]}" +``` ### Rationale: @@ -16,4 +20,4 @@ Curly braces are needed to tell the shell that the square brackets are part of t ### Exceptions -If you want the square brackets to be treated literally or as a glob, you can use `${var}[idx]` to prevent this warning. \ No newline at end of file +If you want the square brackets to be treated literally or as a glob, you can use `${var}[idx]` to prevent this warning. diff --git a/SC1088.md b/SC1088.md index bf31e42..2c812b3 100644 --- a/SC1088.md +++ b/SC1088.md @@ -2,19 +2,27 @@ ### Problematic code: - grep ^(.*)\1$ file +```sh +grep ^(.*)\1$ file +``` or - var = myfunction(value) +```sh +var = myfunction(value) +``` ### Correct code: - grep '^(.*)\1$' file +```sh +grep '^(.*)\1$' file +``` or - var=$(myfunction value) +```sh +var=$(myfunction value) +``` ### Rationale: @@ -35,4 +43,4 @@ If you are trying to use parentheses for shell syntax, look up the actual syntax ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC1089.md b/SC1089.md index a2a0dcc..bf9e47c 100644 --- a/SC1089.md +++ b/SC1089.md @@ -2,18 +2,22 @@ ### Problematic code: - if true - then - echo hello - fi - fi +```sh +if true +then + echo hello +fi +fi +``` ### Correct code: - if true - then - echo hello - fi +```sh +if true +then + echo hello +fi +``` ### Rationale: @@ -21,14 +25,16 @@ 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: - var="foo - if [[ $var = "bar ] - then - echo true - fi +```sh +var="foo +if [[ $var = "bar ] +then + echo true +fi +``` In this case, the `if` ends up inside the double quotes, leaving the `then` dangling. ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC1090.md b/SC1090.md index 20d98e0..77d02de 100644 --- a/SC1090.md +++ b/SC1090.md @@ -2,19 +2,23 @@ ### Problematic code: - . "$(find_install_dir)/lib.sh" +```sh +. "$(find_install_dir)/lib.sh" +``` ### Correct code: - # shellcheck source=src/lib.sh - . "$(find_install_dir)/lib.sh" +```sh +# shellcheck source=src/lib.sh +. "$(find_install_dir)/lib.sh" +``` ### Rationale: ShellCheck is not able to include sourced files from paths that are determined at runtime. The file will not be read, potentially resulting in warnings about unassigned variables and similar. -Use a [[Directive]] to point shellcheck to a fixed location it can read instead. +Use a [[Directive]] to point shellcheck to a fixed location it can read instead. ### Exceptions: -If you don't care that ShellCheck is unable to account for the file, specify `# shellcheck source=/dev/null`. \ No newline at end of file +If you don't care that ShellCheck is unable to account for the file, specify `# shellcheck source=/dev/null`. diff --git a/SC1091.md b/SC1091.md index 618e0d4..900cb34 100644 --- a/SC1091.md +++ b/SC1091.md @@ -4,12 +4,16 @@ Reasons include: file not found, no permissions, not included on the command lin ### Problematic code: - source somefile +```sh +source somefile +``` ### Correct code: - # shellcheck disable=SC1091 - source somefile +```sh +# shellcheck disable=SC1091 +source somefile +``` ### Rationale: @@ -21,4 +25,4 @@ Feel free to ignore the error with a [[directive]]. ### Exceptions: -If you're fine with it, ignore the message with a [[directive]]. \ No newline at end of file +If you're fine with it, ignore the message with a [[directive]]. diff --git a/SC1094.md b/SC1094.md index 041f82f..a89f8d3 100644 --- a/SC1094.md +++ b/SC1094.md @@ -2,12 +2,16 @@ ### Problematic code: - source mylib +```sh +source mylib +``` ### Correct code: - # shellcheck disable=SC1094 - source mylib +```sh +# shellcheck disable=SC1094 +source mylib +``` (or fix `mylib`) @@ -19,4 +23,4 @@ Fix parsing error, or just disable it with a directive. ### Exceptions: -If the file is fine and this is due to a known `shellcheck` bug, you can ignore it with a [[directive]] as in the example. \ No newline at end of file +If the file is fine and this is due to a known `shellcheck` bug, you can ignore it with a [[directive]] as in the example. diff --git a/SC1097.md b/SC1097.md index 47c15f6..a2260e7 100644 --- a/SC1097.md +++ b/SC1097.md @@ -2,17 +2,23 @@ ### Problematic code: - var==value +```sh +var==value +``` ### Correct code: Assignment: - var=value +```sh +var=value +``` Comparison: - [ "$var" = value ] +```sh +[ "$var" = value ] +``` ### 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: - var="=sum(A1:A10)" +```sh +var="=sum(A1:A10)" +``` diff --git a/SC1098.md b/SC1098.md index 3739634..bc4b851 100644 --- a/SC1098.md +++ b/SC1098.md @@ -2,11 +2,15 @@ ### Problematic code: - eval $var=(a b) +```sh +eval $var=(a b) +``` ### Correct code: - eval "$var=(a b)" +```sh +eval "$var=(a b)" +``` ### Rationale: @@ -21,4 +25,4 @@ Since the expression is evaluated as shell script code anyways, it should be pas ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC1099.md b/SC1099.md index 0c5b1f1..b8f7157 100644 --- a/SC1099.md +++ b/SC1099.md @@ -2,22 +2,26 @@ ### Problematic code: - while sleep 1 - do# show time - date - done +```sh +while sleep 1 +do# show time + date +done +``` ### Correct code: - while sleep 1 - do # show time - date - done +```sh +while sleep 1 +do # show time + date +done +``` ### Rationale: -ShellCheck has noticed that you have a keyword immediately followed by a `#`. In order for the `#` to start a comment, it needs to come after a word boundary such as a space. +ShellCheck has noticed that you have a keyword immediately followed by a `#`. In order for the `#` to start a comment, it needs to come after a word boundary such as a space. ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2001.md b/SC2001.md index 98a3512..e73b791 100644 --- a/SC2001.md +++ b/SC2001.md @@ -2,11 +2,15 @@ ### Problematic code: - string="stirng" ; echo "$string" | sed -e "s/ir/ri/" +```sh +string="stirng" ; echo "$string" | sed -e "s/ir/ri/" +``` ### Correct code: - string="stirng" ; echo "${string//ir/ri}" +```sh +string="stirng" ; echo "${string//ir/ri}" +``` ### 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. - 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. diff --git a/SC2002.md b/SC2002.md index 945da56..420ac82 100644 --- a/SC2002.md +++ b/SC2002.md @@ -16,7 +16,7 @@ cat file | ( while read i; do echo "${i%?}"; done ) ### Rationale: -`cat` is a tool for con"cat"enating files. Reading a single file as input to a program is considered a [Useless Use Of Cat (UUOC)](http://en.wikipedia.org/wiki/Cat_(Unix)#Useless_use_of_cat). +`cat` is a tool for con"cat"enating files. Reading a single file as input to a program is considered a [Useless Use Of Cat (UUOC)](http://en.wikipedia.org/wiki/Cat_(Unix)#Useless_use_of_cat). It's more efficient and less roundabout to simply use redirection. This is especially true for programs that can benefit from seekable input, like `tail` or `tar`. @@ -24,4 +24,4 @@ Many tools also accept optional filenames, e.g. `grep -q foo file` instead of `c ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2003.md b/SC2003.md index 59019d8..cc335cf 100644 --- a/SC2003.md +++ b/SC2003.md @@ -2,13 +2,17 @@ ### Problematic code: - i=$(expr 1 + 2) - l=$(expr length "$var") +```sh +i=$(expr 1 + 2) +l=$(expr length "$var") +``` ### Correct code: - i=$((1+2)) - l=${#var} +```sh +i=$((1+2)) +l=${#var} +``` ### Rationale: @@ -18,8 +22,8 @@ ### Exceptions -`sh` doesn't have a great replacement for the `:` operator (regex match). ShellCheck tries not to warn when using expr with `:`, but e.g. `op=:; expr string "$op" regex` still trigger it. +`sh` doesn't have a great replacement for the `:` operator (regex match). ShellCheck tries not to warn when using expr with `:`, but e.g. `op=:; expr string "$op" regex` still trigger it. -Other than that, all uses of `expr` can be rewritten to use modern shell features instead. +Other than that, all uses of `expr` can be rewritten to use modern shell features instead. -Bash has `[[ string =~ regex ]]`, so not even `expr .. : ..` is necessary. \ No newline at end of file +Bash has `[[ string =~ regex ]]`, so not even `expr .. : ..` is necessary. diff --git a/SC2004.md b/SC2004.md index 46fd7f5..e363b84 100644 --- a/SC2004.md +++ b/SC2004.md @@ -2,20 +2,26 @@ ### Problematic code: - echo $(($n+1)) +```sh +echo $(($n+1)) +``` ### Correct code: - echo $((n+1)) +```sh +echo $((n+1)) +``` ### 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: - $ a='1+1' - $ echo $(($a * 5)) # becomes 1+1*5 - 6 - $ echo $((a * 5)) # evaluates as (1+1)*5 - 10 +```sh +$ a='1+1' +$ echo $(($a * 5)) # becomes 1+1*5 +6 +$ 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. \ No newline at end of file +The `$` is unavoidable for special variables like `$1` vs `1`, `$#` vs `#`. ShellCheck does not warn about these cases. diff --git a/SC2006.md b/SC2006.md index f932106..a7e19d3 100644 --- a/SC2006.md +++ b/SC2006.md @@ -1,12 +1,16 @@ -# Use $(..) instead of legacy \`..\` +# Use $(..) instead of legacy \`..\` ### Problematic code - echo "Current time: `date`" +```sh +echo "Current time: `date`" +``` ### Correct code - echo "Current time: $(date)" +```sh +echo "Current time: $(date)" +``` ### Rationale @@ -14,7 +18,7 @@ Backtick command substitution `` `..` `` is legacy syntax with several issues. 1. It has a series of undefined behaviors related to quoting in POSIX. 1. It imposes a custom escaping mode with surprising results. -1. It's exceptionally hard to nest. +1. It's exceptionally hard to nest. `$(..)` command substitution has none of these problems, and is therefore strongly encouraged. @@ -24,4 +28,4 @@ None. ### See also -- http://mywiki.wooledge.org/BashFAQ/082 \ No newline at end of file +- http://mywiki.wooledge.org/BashFAQ/082 diff --git a/SC2008.md b/SC2008.md index 9f85142..29c34eb 100644 --- a/SC2008.md +++ b/SC2008.md @@ -2,18 +2,22 @@ ### Problematic code: - find . | echo +```sh +find . | echo +``` ### Correct code: - find . +```sh +find . +``` ### Rationale: You are piping command output to `echo`, but `echo` ignores all piped input. -In particular, `echo` is not responsible for putting output on screen. Commands already output data, and with no further actions that will end up on screen. +In particular, `echo` is not responsible for putting output on screen. Commands already output data, and with no further actions that will end up on screen. ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2009.md b/SC2009.md index 9d3eb8a..b5dd626 100644 --- a/SC2009.md +++ b/SC2009.md @@ -2,11 +2,15 @@ ### Problematic Code: - ps ax | grep -v grep | grep "$service" > /dev/null +```sh +ps ax | grep -v grep | grep "$service" > /dev/null +``` ### Correct Code: - pgrep -f "$service" > /dev/null +```sh +pgrep -f "$service" > /dev/null +``` ### 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? - 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? - ps ax | grep "$pid" | cut -d" " -f16- +```sh +ps ax | grep "$pid" | cut -d" " -f16- +``` Both are valid cases where SC2009 is not valid. diff --git a/SC2010.md b/SC2010.md index 41432aa..6b2188c 100644 --- a/SC2010.md +++ b/SC2010.md @@ -1 +1 @@ -## Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames. \ No newline at end of file +## Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames. diff --git a/SC2012.md b/SC2012.md index 4433c9d..d1b294c 100644 --- a/SC2012.md +++ b/SC2012.md @@ -2,11 +2,15 @@ ### Problematic code: - ls -l | grep " $USER " | grep '\.txt$' +```sh +ls -l | grep " $USER " | grep '\.txt$' +``` ### Correct code: - find . -maxdepth 1 -name '*.txt' -user "$USER" +```sh +find . -maxdepth 1 -name '*.txt' -user "$USER" +``` ### Rationale: @@ -14,18 +18,20 @@ Here's an example: - $ ls -l - total 0 - -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 +```sh +$ ls -l +total 0 +-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. `ls` can usually be substituted for `find` if it's the filenames you're after. -If trying to parse out any other fields, first see whether `stat` (GNU, OS X, FreeBSD) or `find -printf` (GNU) can give you the data you want directly. +If trying to parse out any other fields, first see whether `stat` (GNU, OS X, FreeBSD) or `find -printf` (GNU) can give you the data you want directly. ### Exceptions: -If the information is intended for the user and not for processing (`ls -l ~/dir | nl; echo "Ok to delete these files?"`) you can ignore this error with a [[directive]]. \ No newline at end of file +If the information is intended for the user and not for processing (`ls -l ~/dir | nl; echo "Ok to delete these files?"`) you can ignore this error with a [[directive]]. diff --git a/SC2013.md b/SC2013.md index 738f52e..f5b5c88 100644 --- a/SC2013.md +++ b/SC2013.md @@ -2,24 +2,30 @@ ### Problematic code: - for line in $(cat file | grep -v '^ *#') - do - echo "Line: $line" - done +```sh +for line in $(cat file | grep -v '^ *#') +do + echo "Line: $line" +done +``` ### Correct code: - grep -v '^ *#' < file | while IFS= read -r line - do - echo "Line: $line" - done +```sh +grep -v '^ *#' < file | while IFS= read -r line +do + echo "Line: $line" +done +``` or without a subshell (bash, zsh, ksh): - while IFS= read -r line - do - echo "Line: $line" - done < <(grep -v '^ *#' < file) +```sh +while IFS= read -r line +do + echo "Line: $line" +done < <(grep -v '^ *#' < file) +``` ### Rationale: @@ -27,24 +33,30 @@ For loops by default (subject to `$IFS`) read word by word. Additionally, glob e Given this text file: - foo * - bar +```sh +foo * +bar +``` The for loop will print: - Line: foo - Line: aardwark.jpg - Line: bullfrog.jpg - ... +```sh +Line: foo +Line: aardwark.jpg +Line: bullfrog.jpg +... +``` The while loop will print: - Line: foo * - Line: bar +```sh +Line: foo * +Line: bar +``` ### Exceptions If you want to read word by word, you should still use a while read loop (e.g. with `read -a` to read words into an array). -Rare reasons for ignoring this message is if you don't care because your file only contains numbers and you're not interested in good practices, or if you've set `$IFS` appropriately and also disabled globbing. \ No newline at end of file +Rare reasons for ignoring this message is if you don't care because your file only contains numbers and you're not interested in good practices, or if you've set `$IFS` appropriately and also disabled globbing. diff --git a/SC2014.md b/SC2014.md index 4cbaf27..e33765b 100644 --- a/SC2014.md +++ b/SC2014.md @@ -2,23 +2,29 @@ ### Problematic code: - find . -name '*.tar' -exec tar xf {} -C "$(dirname {})" \; +```sh +find . -name '*.tar' -exec tar xf {} -C "$(dirname {})" \; +``` ### 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: -Bash evaluates any command substitutions before the command they feature in is executed. In this case, the command is `find`. This means that `$(dirname {})` will run **before** `find` runs, and not **while** `find` runs. +Bash evaluates any command substitutions before the command they feature in is executed. In this case, the command is `find`. This means that `$(dirname {})` will run **before** `find` runs, and not **while** `find` runs. 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" - mydir/myfile is in mydir +```sh +$ 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 . ### Exceptions: -If you don't care (or if you prefer) that it's only expanded once, like when dynamically selecting the executable to be used by all invocations, you can ignore this message. \ No newline at end of file +If you don't care (or if you prefer) that it's only expanded once, like when dynamically selecting the executable to be used by all invocations, you can ignore this message. diff --git a/SC2015.md b/SC2015.md index 44035ea..1183faf 100644 --- a/SC2015.md +++ b/SC2015.md @@ -2,16 +2,20 @@ ### Problematic code: - [[ $dryrun ]] && echo "Would delete file" || rm file +```sh +[[ $dryrun ]] && echo "Would delete file" || rm file +``` ### Correct code: - if [[ $dryrun ]] - then - echo "Would delete file" - else - rm file - fi +```sh +if [[ $dryrun ]] +then + echo "Would delete file" +else + rm file +fi +``` ### Rationale: @@ -19,11 +23,11 @@ It's common to use `A && B` to run `B` when `A` is true, and `A || C` to run `C` However, combining them into `A && B || C` is not the same as `if A then B else C`. -In this case, if `A` is true but `B` is false, `C` will run. +In this case, if `A` is true but `B` is false, `C` will run. For the code sample above, if the script was run with stdout closed for any reason (such as explicitly running `script --dryrun >&-`), echo would fail and the file would be deleted, even though `$dryrun` was set! If an `if` clause is used instead, this problem is avoided. ### Exceptions -Ignore this warning when you actually do intend to run C when either A or B fails. \ No newline at end of file +Ignore this warning when you actually do intend to run C when either A or B fails. diff --git a/SC2016.md b/SC2016.md index 4ef54cc..36f2156 100644 --- a/SC2016.md +++ b/SC2016.md @@ -2,13 +2,17 @@ ### Problematic code: - name=World - echo 'Hello $name' +```sh +name=World +echo 'Hello $name' +``` ### Correct code: - name=World - echo "Hello $name" +```sh +name=World +echo "Hello $name" +``` ### Rationale: @@ -18,10 +22,12 @@ 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: - echo '$1 USD is '"$rate GBP" +```sh +echo '$1 USD is '"$rate GBP" +``` ### Exceptions -If you want `$stuff` to be a literal dollar sign followed by the characters "stuff", you can ignore this message. +If you want `$stuff` to be a literal dollar sign followed by the characters "stuff", you can ignore this message. -ShellCheck tries to be smart about it, and won't warn when this is used with awk, perl and similar, but there are some inherent ambiguities like `'I have $1 in my wallet'`, which could be "one dollar" or "whatever's in the first parameter". \ No newline at end of file +ShellCheck tries to be smart about it, and won't warn when this is used with awk, perl and similar, but there are some inherent ambiguities like `'I have $1 in my wallet'`, which could be "one dollar" or "whatever's in the first parameter". diff --git a/SC2017.md b/SC2017.md index 31c9930..115d246 100644 --- a/SC2017.md +++ b/SC2017.md @@ -2,20 +2,24 @@ ### Problematic code: - percent=$((count/total*100)) +```sh +percent=$((count/total*100)) +``` ### Correct code: - percent=$((count*100/total)) +```sh +percent=$((count*100/total)) +``` ### Rationale: -If integer division is performed before multiplication, the intermediate result will be truncated causing a loss of precision. +If integer division is performed before multiplication, the intermediate result will be truncated causing a loss of precision. In this case, if `count=1` and `total=2`, then the problematic code results in `percent=0`, while the correct code gives `percent=50`. ### Exceptions: -If you want and expect truncation you can ignore this message. +If you want and expect truncation you can ignore this message. -ShellCheck doesn't warn when `b` and `c` are identical expressions, e.g. `a/10*10`, under the assumption that the intent is to rounded to the nearest 10 rather than the no-op of multiply by `1`. \ No newline at end of file +ShellCheck doesn't warn when `b` and `c` are identical expressions, e.g. `a/10*10`, under the assumption that the intent is to rounded to the nearest 10 rather than the no-op of multiply by `1`. diff --git a/SC2020.md b/SC2020.md index 1fa8c56..1e8b7e6 100644 --- a/SC2020.md +++ b/SC2020.md @@ -2,20 +2,24 @@ ### Problematic code: - echo 'hello world' | tr 'hello' 'goodbye' +```sh +echo 'hello world' | tr 'hello' 'goodbye' +``` ### Correct code: - echo 'hello world' | sed -e 's/hello/goodbye/g' +```sh +echo 'hello world' | sed -e 's/hello/goodbye/g' +``` ### Rationale: -`tr` is for `tr`ansliteration, turning some characters into other characters. It doesn't match strings or words, only individual characters. +`tr` is for `tr`ansliteration, turning some characters into other characters. It doesn't match strings or words, only individual characters. In this case, it transliterates h->g, e->o, l->d, o->y, resulting in the string "goddb wbrdd" instead of "goodbye world". -The solution is to use a tool that does string search and replace, such as sed. +The solution is to use a tool that does string search and replace, such as sed. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2022.md b/SC2022.md index 91dbd65..f93d831 100644 --- a/SC2022.md +++ b/SC2022.md @@ -1,22 +1,26 @@ # Note that unlike globs, o* here matches 'ooo' but not 'oscar' ### 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: - grep '^foo' +```sh +grep '^foo' +``` ### Rationale: As a glob, `foo*` means "Any string starting with foo", e.g. `food` and `foosball`. -As a regular expression, "foo*" means "f followed by 1 or more o's, anywhere", e.g. "mofo" or "keyfob". +As a regular expression, "foo*" means "f followed by 1 or more o's, anywhere", e.g. "mofo" or "keyfob". This construct is way more common as a glob than as a regex, so ShellCheck notifies you about it. ### Exceptions -If you're aware of the above, you can ignore this message. If you'd like shellcheck to be quiet, use a [[directive]] or `'fo[o]*'`. \ No newline at end of file +If you're aware of the above, you can ignore this message. If you'd like shellcheck to be quiet, use a [[directive]] or `'fo[o]*'`. diff --git a/SC2024.md b/SC2024.md index 41308e5..279c579 100644 --- a/SC2024.md +++ b/SC2024.md @@ -2,11 +2,11 @@ ### Problematic code: - sudo echo 'export FOO=bar' >> /etc/profile +sudo echo 'export FOO=bar' >> /etc/profile ### 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: @@ -18,15 +18,15 @@ There is nothing special about `tee`. It's just the simplest command that can bo Truncating: - echo 'data' | sudo dd of=file - echo 'data' | sudo sed 'w file' +echo 'data' | sudo dd of=file +echo 'data' | sudo sed 'w file' Appending: - echo 'data' | sudo awk '{ print $0 >> "file" }' - echo 'data' | sudo sh -c 'cat >> file' - +echo 'data' | sudo awk '{ print $0 >> "file" }' +echo 'data' | sudo sh -c 'cat >> file' + ### Exceptions -If you want to run a command as root but redirect as the normal user, you can ignore this message. \ No newline at end of file +If you want to run a command as root but redirect as the normal user, you can ignore this message. diff --git a/SC2025.md b/SC2025.md index e6a8edd..9e7b7ae 100644 --- a/SC2025.md +++ b/SC2025.md @@ -2,11 +2,15 @@ ### Problematic code: - PS1='\e[36m\$ \e(B\e[m' +```sh +PS1='\e[36m\$ \e(B\e[m' +``` ### Correct code: - PS1='\[\e[36m\]\$ \[\e(B\e[m\]' +```sh +PS1='\[\e[36m\]\$ \[\e(B\e[m\]' +``` ### Rationale: @@ -16,4 +20,4 @@ Note: ShellCheck offers this as a helpful hint and not a robust check. Don't rel ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2026.md b/SC2026.md index 7cd17c2..3be059d 100644 --- a/SC2026.md +++ b/SC2026.md @@ -2,36 +2,46 @@ ### Problematic code: - alias server_uptime='ssh $host 'uptime -p'' +```sh +alias server_uptime='ssh $host 'uptime -p'' +``` ### Correct code: - alias server_uptime='ssh $host '"'uptime -p'" +```sh +alias server_uptime='ssh $host '"'uptime -p'" +``` ### 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: - # v--------match--------v - alias server_uptime='ssh $host 'uptime -p'' - # ^--match--^ +```sh +# v--------match--------v +alias server_uptime='ssh $host 'uptime -p'' +# ^--match--^ +``` The shell, meanwhile, always terminates single quoted strings at the first possible single quote: - # v---match--v - alias server_uptime='ssh $host 'uptime -p'' - # ^^ +```sh +# v---match--v +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: - # v--match---v - alias server_uptime='ssh $host '"'uptime -p'" - # ^---match---^ +```sh +# v--match---v +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. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2027.md b/SC2027.md index 330becb..54b808a 100644 --- a/SC2027.md +++ b/SC2027.md @@ -2,11 +2,15 @@ ### 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: - echo "You enter $HOSTNAME. You can smell the wumpus." >> /etc/issue +```sh +echo "You enter $HOSTNAME. You can smell the wumpus." >> /etc/issue +``` ### Rationale: @@ -14,12 +18,14 @@ 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: - echo "You enter "$HOSTNAME". You can smell the wumpus." - |----------| |---------------------------| - Quoted No quotes Quoted +```sh +echo "You enter "$HOSTNAME". You can smell the wumpus." + |----------| |---------------------------| + 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. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2028.md b/SC2028.md index 316c750..76645cb 100644 --- a/SC2028.md +++ b/SC2028.md @@ -1,25 +1,31 @@ -# echo won't expand escape sequences. Consider printf. +# echo won't expand escape sequences. Consider printf. ### Problematic code: - echo "Name:\t$value" +```sh +echo "Name:\t$value" +``` ### Correct code: - printf "Name:\t%s\n" "$value" +```sh +printf "Name:\t%s\n" "$value" +``` ### Rationale: -Backslash escapes like `\t` and `\n` are not expanded by echo, and become literal backslash-t, backslash-n. +Backslash escapes like `\t` and `\n` are not expanded by echo, and become literal backslash-t, backslash-n. `printf` does expand these sequences, and should be used instead. Other, non-portable methods include `echo -e '\t'` and `echo $'\t'`. ShellCheck will warn if this is used in a script with shebang `#!/bin/sh`. -If you actually wanted a literal backslash-t, use +If you actually wanted a literal backslash-t, use - echo "\\t" +```sh +echo "\\t" +``` ### Exceptions -None \ No newline at end of file +None diff --git a/SC2029.md b/SC2029.md index 5591a4f..a2a67ce 100644 --- a/SC2029.md +++ b/SC2029.md @@ -2,21 +2,29 @@ ### Problematic code: - ssh host "echo $HOSTNAME" +```sh +ssh host "echo $HOSTNAME" +``` ### Correct code: - ssh host "echo \$HOSTNAME" +```sh +ssh host "echo \$HOSTNAME" +``` or - ssh host 'echo $HOSTNAME' +```sh +ssh host 'echo $HOSTNAME' +``` ### Rationale: 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. @@ -24,6 +32,6 @@ By escaping the `$` in `$HOSTNAME`, it will be transmitted literally and evaluat ### Exceptions -If you do want your string expanded on the client side, you can safely ignore this message. +If you do want your string expanded on the client side, you can safely ignore this message. -Keep in mind that the expanded string will be evaluated again on the server side, so for arbitrary variables and command output, you may need to add a layer of escaping with e.g. `printf %q`. \ No newline at end of file +Keep in mind that the expanded string will be evaluated again on the server side, so for arbitrary variables and command output, you may need to add a layer of escaping with e.g. `printf %q`. diff --git a/SC2030.md b/SC2030.md index 4599d47..36cffe0 100644 --- a/SC2030.md +++ b/SC2030.md @@ -1,3 +1,3 @@ # Modification of var is local (to subshell caused by pipeline). -See companion warning [[SC2031]]. \ No newline at end of file +See companion warning [[SC2031]]. diff --git a/SC2031.md b/SC2031.md index 7dbcc62..37af0f3 100644 --- a/SC2031.md +++ b/SC2031.md @@ -26,39 +26,51 @@ In `sh`, a temp file (better if fifo or fd) can be used instead of process subst Variables set in subshells are not available outside the subshell. This is a wide topic, and better described on the [Wooledge Bash Wiki](http://mywiki.wooledge.org/BashFAQ/024). Here are some constructs that cause subshells (shellcheck may not warn about all of them). In each case, you can replace `subshell1` by a command or function that sets a variable, e.g. simply `var=foo`, and the variable will appear to be unset after the command is run. Similarly, you can replace `regular` with `var=foo`, and it will be set afterwards: - + Pipelines: - subshell1 | subshell2 | subshell3 # Bash, Dash, Ash - subshell1 | subshell2 | regular # Ksh, Zsh +```sh +subshell1 | subshell2 | subshell3 # Bash, Dash, Ash +subshell1 | subshell2 | regular # Ksh, Zsh +``` Command substitution: - regular "$(subshell1)" "`subshell2`" +```sh +regular "$(subshell1)" "`subshell2`" +``` -Process substitution: +Process substitution: - regular <(subshell1) >(subshell2) +```sh +regular <(subshell1) >(subshell2) +``` Some forms of grouping: - ( subshell ) - { regular; } +```sh +( subshell ) +{ regular; } +``` Backgrounding: - subshell1 & - subshell2 & +```sh +subshell1 & +subshell2 & +``` Anything executed by external processes: - find . -exec subshell1 {} \; - find . -print0 | xargs -0 subshell2 - sudo subshell3 - su -c subshell4 +```sh +find . -exec subshell1 {} \; +find . -print0 | xargs -0 subshell2 +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. ### Exceptions -You can ignore this error if you don't care that the changes aren't reflected, because work on the value branches and shouldn't be recombined. \ No newline at end of file +You can ignore this error if you don't care that the changes aren't reflected, because work on the value branches and shouldn't be recombined. diff --git a/SC2032.md b/SC2032.md index fe1113a..9bd994c 100644 --- a/SC2032.md +++ b/SC2032.md @@ -1,3 +1,3 @@ # Use own script or sh -c '..' to run this from su. -See [[SC2033]]. \ No newline at end of file +See [[SC2033]]. diff --git a/SC2033.md b/SC2033.md index f24323d..9af0832 100644 --- a/SC2033.md +++ b/SC2033.md @@ -2,16 +2,20 @@ ### Problematic code: - foo() { bar --baz "$@"; frob --baz "$@"; }; - find . -exec foo {} + +```sh +foo() { bar --baz "$@"; frob --baz "$@"; }; +find . -exec foo {} + +``` ### Correct code: - find . -exec sh -c 'bar --baz "$@"; frob --baz "$@";' -- {} + +```sh +find . -exec sh -c 'bar --baz "$@"; frob --baz "$@";' -- {} + +``` ### Rationale: -Shell functions are only known to the shell. External commands like `find`, `xargs`, `su` and `sudo` do not recognize shell functions. +Shell functions are only known to the shell. External commands like `find`, `xargs`, `su` and `sudo` do not recognize shell functions. Instead, the function contents can be executed in a shell, either through `sh -c` or by creating a separate shell script as an executable file. @@ -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. - nobody() { - sudo -u "nobody" "$@" - } - \ No newline at end of file +```sh +nobody() { + sudo -u "nobody" "$@" +} + +``` diff --git a/SC2034.md b/SC2034.md index 66cbc4c..2a2741c 100644 --- a/SC2034.md +++ b/SC2034.md @@ -2,17 +2,21 @@ ### Problematic code: - foo=42 - echo "$FOO" +```sh +foo=42 +echo "$FOO" +``` ### Correct code: - foo=42 - echo "$foo" +```sh +foo=42 +echo "$foo" +``` ### Rationale: -Variables not used for anything are often associated with bugs, so ShellCheck warns about them. +Variables not used for anything are often associated with bugs, so ShellCheck warns about them. Also note that something like `local let foo=42` does not make a `let` statement local -- it instead declares an additional local variable named `let`. @@ -22,21 +26,27 @@ ShellCheck may not always realize that the variable is in use (especially with i For throwaway variables, consider using `_` as a dummy: - read _ last _ zip _ _ <<< "$str" - echo "$last, $zip" +```sh +read _ last _ zip _ _ <<< "$str" +echo "$last, $zip" +``` or use a directive to disable the warning: - # shellcheck disable=SC2034 - read first last email zip lat lng <<< "$str" - echo "$last, $zip" +```sh +# shellcheck disable=SC2034 +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: - bar=42 # will always appear unused - foo=bar - echo "${!foo}" +```sh +bar=42 # will always appear unused +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. -As always, there are ways to [[ignore]] this and other messages if they frequently get in your way. \ No newline at end of file +As always, there are ways to [[ignore]] this and other messages if they frequently get in your way. diff --git a/SC2036.md b/SC2036.md index d12b8d8..926d622 100644 --- a/SC2036.md +++ b/SC2036.md @@ -2,17 +2,21 @@ ### Problematic code: - sum=find | wc -l +```sh +sum=find | wc -l +``` ### Correct code: - sum=$(find | wc -l) +```sh +sum=$(find | wc -l) +``` ### Rationale: The intention in this code was that `sum` would in some way get the value of the command `find | wc -l`. -However, `|` has precedence over the `=`, so the command is a two stage pipeline consisting of `sum=find` and `wc -l`. +However, `|` has precedence over the `=`, so the command is a two stage pipeline consisting of `sum=find` and `wc -l`. `sum=find` is a plain string assignment. Since it happens by itself in an independent pipeline stage, it has no effect: it produces no output, and the variable disappears when the pipeline stage finishes. Because the assignment produces no output, `wc -l` will count 0 lines. @@ -20,4 +24,4 @@ To instead actually assign a variable with the output of a command, command subs ### Exceptions: -None. This warning is triggered whenever the first stage of a pipeline is a single assignment, which is never correct. \ No newline at end of file +None. This warning is triggered whenever the first stage of a pipeline is a single assignment, which is never correct. diff --git a/SC2038.md b/SC2038.md index 5ca762b..5f6c7e9 100644 --- a/SC2038.md +++ b/SC2038.md @@ -2,11 +2,15 @@ ### Problematic code: - find . -type f | xargs md5sum +```sh +find . -type f | xargs md5sum +``` ### Correct code: - find . -type f -print0 | xargs -0 md5sum +```sh +find . -type f -print0 | xargs -0 md5sum +``` ### Rationale: @@ -14,4 +18,4 @@ By default, `xargs` interprets spaces and quotes in an unsafe and unexpected way ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2039.md b/SC2039.md index cbde9c2..ff12d92 100644 --- a/SC2039.md +++ b/SC2039.md @@ -1,8 +1,8 @@ ## `#!/bin/sh` was specified, but *something* is not standard. -The shebang indicates that the script works with `/bin/sh`, but you are using non-standard features that may not be supported. +The shebang indicates that the script works with `/bin/sh`, but you are using non-standard features that may not be supported. -It may currently work for you, but it can or will fail on other OS, the same OS with different configurations or from different contexts (like initramfs/chroot), or different versions of the same OS, including future updates to your current system. +It may currently work for you, but it can or will fail on other OS, the same OS with different configurations or from different contexts (like initramfs/chroot), or different versions of the same OS, including future updates to your current system. Either declare a specific shell like `#!/usr/bin/env bash` to make sure this shell is always used, or rewrite the script in a portable way. @@ -143,4 +143,4 @@ reuse_quote "$@" ## Exception -Depends on what your expected POSIX shell providers would use. \ No newline at end of file +Depends on what your expected POSIX shell providers would use. diff --git a/SC2040.md b/SC2040.md index 6b47f46..9894465 100644 --- a/SC2040.md +++ b/SC2040.md @@ -4,4 +4,4 @@ The shebang indicates that the script works with `/bin/sh`, but you are using no Specify `#!/usr/bin/env bash` to ensure that bash (or your shell of choice) will be used, or rewrite the script to be more portable. -The Ubuntu wiki has [a list of portability issues](https://wiki.ubuntu.com/DashAsBinSh) and suggestions on how to rewrite them. \ No newline at end of file +The Ubuntu wiki has [a list of portability issues](https://wiki.ubuntu.com/DashAsBinSh) and suggestions on how to rewrite them. diff --git a/SC2041.md b/SC2041.md index 10bdde8..7cb2858 100644 --- a/SC2041.md +++ b/SC2041.md @@ -2,17 +2,21 @@ ### Problematic code: - for i in 'seq 1 10' - do - echo "$i" - done +```sh +for i in 'seq 1 10' +do + echo "$i" +done +``` ### Correct code: - for i in $(seq 1 10) - do - echo "$i" - done +```sh +for i in $(seq 1 10) +do + echo "$i" +done +``` ### Rationale: @@ -22,4 +26,4 @@ This is one of the many problems with backticks, so it's better to use `$(..)` t ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2043.md b/SC2043.md index cfa7395..b4328be 100644 --- a/SC2043.md +++ b/SC2043.md @@ -2,30 +2,40 @@ ### Problematic code: - for var in value - do - echo "$var" - done +```sh +for var in value +do + echo "$var" +done +``` ### Correct code: -Correct code depends on what you want to do. +Correct code depends on what you want to do. 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: - 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 - 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 - for var in $myvariable; do echo "$var"; done +```sh +for var in $myvariable; do echo "$var"; done +``` @@ -33,8 +43,8 @@ To iterate over *words* in a variable, instead of `for var in myvariable`, use ShellCheck has detected that your for loop iterates over a single, constant value. This is most likely a bug in your code, caused by you not expanding the value in the way you want. -You should make sure that whatever you loop over will expand into multiple words. +You should make sure that whatever you loop over will expand into multiple words. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2044.md b/SC2044.md index d75ff36..c9904ff 100644 --- a/SC2044.md +++ b/SC2044.md @@ -2,31 +2,35 @@ ### Problematic code: - for file in $(find mydir -mtime -7 -name '*.mp3') - do - let count++ - echo "Playing file no. $count" - play "$file" - done - echo "Played $count files" +```sh +for file in $(find mydir -mtime -7 -name '*.mp3') +do + let count++ + echo "Playing file no. $count" + play "$file" +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). ### Correct code: -There are many possible fixes, each with its pros and cons. +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: - while IFS= read -r -d '' file - do - let count++ - echo "Playing file no. $count" - play "$file" - done < <(find mydir -mtime -7 -name '*.mp3' -print0) - echo "Played $count files" +```sh +while IFS= read -r -d '' file +do + let count++ + echo "Playing file no. $count" + play "$file" +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. It requires Bash, and works with GNU, Busybox, OS X, FreeBSD and OpenBSD find, but not POSIX find. @@ -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: - shopt -s globstar nullglob - for file in mydir/**/*.mp3 - do - let count++ - echo "Playing file no. $count" - play "$file" - done - echo "Played $count files" +```sh +shopt -s globstar nullglob +for file in mydir/**/*.mp3 +do + let count++ + echo "Playing file no. $count" + play "$file" +done +echo "Played $count files" +``` This is bash specific. @@ -50,26 +56,30 @@ This is bash specific. If you need POSIX compliance, this is a fair approach: - find mydir ! -name "$(printf "*\n*")" -name '*.mp3' > tmp - while IFS= read -r file - do - let count++ - echo "Playing file #$count" - play "$file" - done < tmp - rm tmp - echo "Played $count files" +```sh +find mydir ! -name "$(printf "*\n*")" -name '*.mp3' > tmp +while IFS= read -r file +do + let count++ + echo "Playing file #$count" + play "$file" +done < tmp +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. -If you don't need variables to be available after the loop (here, if you don't need to print the final play count at the end), you can skip the `tmp` file and just pipe from `find` to `while`. +If you don't need variables to be available after the loop (here, if you don't need to print the final play count at the end), you can skip the `tmp` file and just pipe from `find` to `while`. ##### For simple commands with no aggregation 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 - find mydir -name '*.mp3' -exec play {} \; +```sh +# 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. @@ -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): - find mydir -name '*.mp3' -exec sh -c ' - echo "Playing ${1%.mp3}" - play "$1" - ' sh {} \; +```sh +find mydir -name '*.mp3' -exec sh -c ' + echo "Playing ${1%.mp3}" + 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. @@ -89,10 +101,10 @@ Note that using `+` instead of `\;`, and using an embedded `for file in "$@"` lo ### Rationale: -`for var in $(find ...)` loops rely on word splitting and will evaluate globs, which will wreck havoc with filenames containing whitespace or glob characters. +`for var in $(find ...)` loops rely on word splitting and will evaluate globs, which will wreck havoc with filenames containing whitespace or glob characters. -`find -exec` `for i in glob` and `find`+`while` do not rely on word splitting, so they avoid this problem. +`find -exec` `for i in glob` and `find`+`while` do not rely on word splitting, so they avoid this problem. ### Exceptions -If you know about and carefully apply `IFS=$'\n'` and `set -f`, you could choose to ignore this message. \ No newline at end of file +If you know about and carefully apply `IFS=$'\n'` and `set -f`, you could choose to ignore this message. diff --git a/SC2048.md b/SC2048.md index 0ee828d..bdf3d51 100644 --- a/SC2048.md +++ b/SC2048.md @@ -2,11 +2,15 @@ ### Problematic code: - cp $* ~/dir +```sh +cp $* ~/dir +``` ### Correct code: - cp "$@" ~/dir +```sh +cp "$@" ~/dir +``` ### Rationale: @@ -22,4 +26,4 @@ Since the latter is rarely expected or desired, ShellCheck warns about it. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2051.md b/SC2051.md index bc6c8f3..c1a62ad 100644 --- a/SC2051.md +++ b/SC2051.md @@ -2,17 +2,17 @@ ### Problematic code: - for i in {1..$n} - do - echo "$i" - done +for i in {1..$n} +do + echo "$i" +done ### Correct code: - for ((i=0; i&1 > /dev/null +```sh +firefox 2>&1 > /dev/null +``` ### Correct code: - firefox > /dev/null 2>&1 +```sh +firefox > /dev/null 2>&1 +``` ### Rationale: -Redirections are handled in order. +Redirections are handled in order. The problematic code means "Point stderr to where stdout is currently pointing (the terminal). Then point stdout to /dev/null". The correct code means "Point stdout to /dev/null. Then point stderr to where stdout is currently pointing (/dev/null)". -In other words, the problematic code hides stdout and shows stderr. The correct code hides both stderr and stdout, which is usually the intention. +In other words, the problematic code hides stdout and shows stderr. The correct code hides both stderr and stdout, which is usually the intention. ### Exceptions -If you want stderr as stdout and stdout to a file, you can ignore this message. \ No newline at end of file +If you want stderr as stdout and stdout to a file, you can ignore this message. diff --git a/SC2070.md b/SC2070.md index 9527789..e866fdf 100644 --- a/SC2070.md +++ b/SC2070.md @@ -2,40 +2,48 @@ ### Problematic code: - if [ -n $var ] - then - echo "var has a value" - else - echo "var is empty" - fi +```sh +if [ -n $var ] +then + echo "var has a value" +else + echo "var is empty" +fi +``` ### Correct code: In bash/ksh: - if [[ -n $var ]] - then - echo "var has a value" - else - echo "var is empty" - fi +```sh +if [[ -n $var ]] +then + echo "var has a value" +else + echo "var is empty" +fi +``` In POSIX: - if [ -n "$var" ] - then - echo "var has a value" - else - echo "var is empty" - fi +```sh +if [ -n "$var" ] +then + echo "var has a value" +else + echo "var is empty" +fi +``` ### Rationale: 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 ] - [ -n ] +```sh +[ -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 ]`. @@ -43,4 +51,4 @@ To fix this, either use `[[ -n $var ]]` which has fewer caveats than `[`, or quo ### Exceptions: -None \ No newline at end of file +None diff --git a/SC2072.md b/SC2072.md index b7c05e6..23f581f 100644 --- a/SC2072.md +++ b/SC2072.md @@ -2,12 +2,16 @@ ### Problematic code: - [[ 2 -lt 3.14 ]] +```sh +[[ 2 -lt 3.14 ]] +``` ### Correct code: - [[ 200 -lt 314 ]] # Use fixed point math - [[ $(echo "2 < 3.14" | bc) == 1 ]] # Use bc +```sh +[[ 200 -lt 314 ]] # Use fixed point math +[[ $(echo "2 < 3.14" | bc) == 1 ]] # Use bc +``` ### Rationale: @@ -15,4 +19,4 @@ Bash and Posix sh does not support decimals in numbers. Decimals should either b ### Exceptions -If the strings happen to be version numbers and you're using `<`, or `>` to compare them as strings, and you consider this an acceptable thing to do, then you can ignore this warning. \ No newline at end of file +If the strings happen to be version numbers and you're using `<`, or `>` to compare them as strings, and you consider this an acceptable thing to do, then you can ignore this warning. diff --git a/SC2076.md b/SC2076.md index 9b0aa21..6ebed34 100644 --- a/SC2076.md +++ b/SC2076.md @@ -2,11 +2,15 @@ ### Problematic code: - [[ $foo =~ "^fo+bar$" ]] +```sh +[[ $foo =~ "^fo+bar$" ]] +``` ### Correct code: - [[ $foo =~ ^fo+bar$ ]] +```sh +[[ $foo =~ ^fo+bar$ ]] +``` ### Rationale: @@ -24,4 +28,4 @@ If you do want to match literally just to do a plain substring search, e.g. `[[ * In Bash 3.2 and newer with shopt `compat31` *enabled*, quoted and unquoted patterns match identically. * In Bash 3.1 quoted and unquoted patterns match identically. -See http://stackoverflow.com/questions/218156/bash-regex-with-quotes \ No newline at end of file +See http://stackoverflow.com/questions/218156/bash-regex-with-quotes diff --git a/SC2077.md b/SC2077.md index a22260e..a2ae035 100644 --- a/SC2077.md +++ b/SC2077.md @@ -2,20 +2,24 @@ ### Problematic code: - [[ 0=1 ]] +```sh +[[ 0=1 ]] +``` ### Correct code: - [[ 0 = 1 ]] +```sh +[[ 0 = 1 ]] +``` ### Rationale: `[[ 0 = 1 ]]` means "check if 0 and 1 are equal". - -`[[ str ]]` is short form for `[[ -n str ]]`, and means "check if `str` is non-empty". It doesn't matter if `str` happens to contain `0=1`. + +`[[ str ]]` is short form for `[[ -n str ]]`, and means "check if `str` is non-empty". It doesn't matter if `str` happens to contain `0=1`. Always use spaces around the comparison operator in `[..]` and `[[..]]`, otherwise it won't be recognized as an operator. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2082.md b/SC2082.md index ffb294b..b96beec 100644 --- a/SC2082.md +++ b/SC2082.md @@ -2,46 +2,54 @@ ### Problematic code: - var_1="hello world" - n=1 - echo "${var_$n}" +```sh +var_1="hello world" +n=1 +echo "${var_$n}" +``` ### Correct code: Bash/ksh: - # Use arrays instead of dynamic names - declare -a var - var[1]="hello world" - n=1 - echo "${var[n]}" +```sh +# Use arrays instead of dynamic names +declare -a var +var[1]="hello world" +n=1 +echo "${var[n]}" +``` or - # Expand variable names dynamically - var_1="hello world" - n=1 - name="var_$n" - echo "${!name}" +```sh +# Expand variable names dynamically +var_1="hello world" +n=1 +name="var_$n" +echo "${!name}" +``` POSIX sh: - # Expand dynamically with eval - var_1="hello world" - n=1 - eval "tmp=\$var_$n" - echo "${tmp}" +```sh +# Expand dynamically with eval +var_1="hello world" +n=1 +eval "tmp=\$var_$n" +echo "${tmp}" +``` ### Rationale: -You can expand a variable `var_1` with `${var_1}`, but you can not generate the string `var_1` with an embedded expansion, like `${var_$n}`. +You can expand a variable `var_1` with `${var_1}`, but you can not generate the string `var_1` with an embedded expansion, like `${var_$n}`. -Instead, if at all possible, you should use an array. Bash and ksh support both numerical and associative arrays, and an example is shown above. +Instead, if at all possible, you should use an array. Bash and ksh support both numerical and associative arrays, and an example is shown above. -If you can't use arrays, you can indirectly reference variables by creating a temporary variable with its name, e.g. `myvar="var_$n"` and then expanding it indirectly with `${!myvar}`. This will give the contents of the variable `var_1`. +If you can't use arrays, you can indirectly reference variables by creating a temporary variable with its name, e.g. `myvar="var_$n"` and then expanding it indirectly with `${!myvar}`. This will give the contents of the variable `var_1`. If using POSIX sh, where neither arrays nor `${!var}` is available, `eval` can be used. You must be careful in sanitizing the data used to construct the variable name to avoid arbitrary code execution. ### Exceptions: -None \ No newline at end of file +None diff --git a/SC2084.md b/SC2084.md index bbb2764..ba9895b 100644 --- a/SC2084.md +++ b/SC2084.md @@ -2,42 +2,54 @@ ### Problematic code: - i=4 - $(( i++ )) +```sh +i=4 +$(( i++ )) +``` ### Correct code: Bash, Ksh: - i=4 - (( i++ )) +```sh +i=4 +(( i++ )) +``` POSIX (assuming `++` is supported): - i=4 - _=$(( i++ )) +```sh +i=4 +_=$(( i++ )) +``` Alternative POSIX version that does not preserve the exit code: - : $(( i++ )) +```sh +: $(( i++ )) +``` ### 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: - $ i=4 - $ $(( i++ )) - 4: command not found - $ echo $i - 5 +```sh +$ i=4 +$ $(( i++ )) +4: command not found +$ echo $i +5 +``` To avoid trying to execute the number as a command name, use one of the methods mentioned: - $ i=4 - $ _=$(( i++ )) - $ echo $i - 5 +```sh +$ i=4 +$ _=$(( i++ )) +$ echo $i +5 +``` ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2087.md b/SC2087.md index 10e2a64..108e2fb 100644 --- a/SC2087.md +++ b/SC2087.md @@ -2,42 +2,51 @@ ### Problematic code: - ssh host.example.com << EOF - echo "Logged in on $HOSTNAME" - EOF - +```sh +ssh host.example.com << EOF + echo "Logged in on $HOSTNAME" +EOF + ### Correct code: - ssh host.example.com << "EOF" - echo "Logged in on $HOSTNAME" - EOF +```sh +ssh host.example.com << "EOF" + echo "Logged in on $HOSTNAME" +EOF +``` ### Rationale: -When the end token of a here document is unquoted, parameter expansion and command substitution will happen on in contents of the here doc. +When the end token of a here document is unquoted, parameter expansion and command substitution will happen on in contents of the here doc. -This means that before sending the commands to the server, the client replaces `$HOSTNAME` with localhost, thereby sending `echo "Logged in on localhost"` to the server. This has the effect of printing the client's hostname instead of the server's. +This means that before sending the commands to the server, the client replaces `$HOSTNAME` with localhost, thereby sending `echo "Logged in on localhost"` to the server. This has the effect of printing the client's hostname instead of the server's. 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 - x="$(uname -a)" - echo "$x" - EOF +```sh +ssh host << EOF + x="$(uname -a)" + echo "$x" +EOF +``` will never print anything, neither client nor server details, since before evaluation, it will be expanded to: - x="Linux localhost ... x86_64 GNU/Linux" - echo "" +```sh + 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. ### Exceptions: -If the client should expand some or all variables, this message can and should be ignored. +If the client should expand some or all variables, this message can and should be ignored. -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 - echo "Logged in on \$HOSTNAME from $HOSTNAME" - EOF +```sh +ssh host.example.com << EOF + echo "Logged in on \$HOSTNAME from $HOSTNAME" +EOF +``` diff --git a/SC2088.md b/SC2088.md index 700cdca..d3596f5 100644 --- a/SC2088.md +++ b/SC2088.md @@ -2,18 +2,22 @@ ### Problematic code: - rm "~/Desktop/$filename" +```sh +rm "~/Desktop/$filename" +``` ### Correct code: - rm "$HOME/Desktop/$filename" +```sh +rm "$HOME/Desktop/$filename" +``` ### Rationale: Tilde does not expand to the user's home directory when it's single or double quoted. Use double quotes and `$HOME` instead. -Alternatively, the `~/` can be left unquoted, as in `rm ~/"Desktop/$filename"`. +Alternatively, the `~/` can be left unquoted, as in `rm ~/"Desktop/$filename"`. ### Exceptions -If you don't want the tilde to be expanded, you can ignore this message. \ No newline at end of file +If you don't want the tilde to be expanded, you can ignore this message. diff --git a/SC2089.md b/SC2089.md index 3d57b88..f833175 100644 --- a/SC2089.md +++ b/SC2089.md @@ -2,43 +2,53 @@ ### Problematic code: - args='-lh "My File.txt"' - ls $args +```sh +args='-lh "My File.txt"' +ls $args +``` ### Correct code: - args=(-lh "My File.txt") - ls "${args[@]}" +```sh +args=(-lh "My File.txt") +ls "${args[@]}" +``` ### Rationale: Bash does not interpret data as code. Consider almost any other languages, such as Python: - print 1+1 # prints 2 - a="1+1" - print a # prints 1+1, not 2 +```sh +print 1+1 # prints 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. -Similarly, `"My File.txt"` is Bash syntax for a single word with a space in it. However, passing a literal string containing this expression does not cause Bash to interpret it, see the quotes and produce the tokenized result. +Similarly, `"My File.txt"` is Bash syntax for a single word with a space in it. However, passing a literal string containing this expression does not cause Bash to interpret it, see the quotes and produce the tokenized result. 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: - quote() { local q=${1//\'/\'\\\'\'}; echo "'$q'"; } - args="-lh $(quote "My File.txt")" - eval ls "$args" # Do not use unless you understand implications +```sh +quote() { local q=${1//\'/\'\\\'\'}; echo "'$q'"; } +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: - for f in *.txt; do - args="-lh '$1'" # Example security exploit - eval ls "$args" # Do not copy and use - done +```sh +for f in *.txt; do + args="-lh '$1'" # Example security exploit + 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 /`. ### Exceptions -Few and far between. \ No newline at end of file +Few and far between. diff --git a/SC2090.md b/SC2090.md index 886298b..8c7bec4 100644 --- a/SC2090.md +++ b/SC2090.md @@ -1,3 +1,3 @@ # Quotes/backslashes in this variable will not be respected. -See companion warning, [[SC2089]]. \ No newline at end of file +See companion warning, [[SC2089]]. diff --git a/SC2091.md b/SC2091.md index 68cd3e3..97ec79e 100644 --- a/SC2091.md +++ b/SC2091.md @@ -2,21 +2,25 @@ ### Problematic code: - if $(which epstopdf) - then - echo "Found epstopdf" - fi +```sh +if $(which epstopdf) +then + echo "Found epstopdf" +fi +``` ### Correct code: - if which epstopdf - then - echo "Found epstopdf" - fi +```sh +if which epstopdf +then + echo "Found epstopdf" +fi +``` ### Rationale: -ShellCheck has detected that you have a command that just consists of a command substitution. +ShellCheck has detected that you have a command that just consists of a command substitution. This is typically done in order to try to get the shell to execute a command, because `$(..)` does indeed execute commands. However, it's also replaced by the output of that command. @@ -30,4 +34,4 @@ The solution is simply to remove the surrounding `$()`. This will execute the co ### Exceptions: -If you really want to execute the output of a command rather than the command itself, you can ignore this message. \ No newline at end of file +If you really want to execute the output of a command rather than the command itself, you can ignore this message. diff --git a/SC2092.md b/SC2092.md index fdbd924..9b17166 100644 --- a/SC2092.md +++ b/SC2092.md @@ -1,3 +1,3 @@ ## Remove backticks to avoid executing output. -Backticks does the same thing as `$(..)`. See [[SC2091]] for a description of the same problem with this syntax. \ No newline at end of file +Backticks does the same thing as `$(..)`. See [[SC2091]] for a description of the same problem with this syntax. diff --git a/SC2094.md b/SC2094.md index fd849b9..bbcb27f 100644 --- a/SC2094.md +++ b/SC2094.md @@ -2,23 +2,27 @@ ### 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: - 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: -Each step in a pipeline runs in parallel. +Each step in a pipeline runs in parallel. -In this case, `grep foo file.txt` will immediately try to read `file.txt` while `sed .. > file.txt` will immediately try to truncate it. +In this case, `grep foo file.txt` will immediately try to read `file.txt` while `sed .. > file.txt` will immediately try to truncate it. -This is a race condition, and results in the file being partially or (far more likely) entirely truncated. +This is a race condition, and results in the file being partially or (far more likely) entirely truncated. ### Exceptions You can ignore this error if: * The file is a device or named pipe. These files don't truncate in the same way. -* The command mentions the filename but doesn't read/write it, such as `echo log.txt > log.txt`. \ No newline at end of file +* The command mentions the filename but doesn't read/write it, such as `echo log.txt > log.txt`. diff --git a/SC2096.md b/SC2096.md index 9c7e1a8..2d09ae0 100644 --- a/SC2096.md +++ b/SC2096.md @@ -2,19 +2,23 @@ ### Problematic code: - #!/usr/bin/env bash -x +```sh +#!/usr/bin/env bash -x +``` ### Correct code: - #!/usr/bin/env bash - set -x +```sh +#!/usr/bin/env bash +set -x +``` ### Rationale: -Most operating systems, including Linux, FreeBSD and OS X, allow only a single parameter in the shebang. The example is equivalent to calling `env 'bash -x'` instead of `env 'bash' '-x'`, and it will therefore fail. +Most operating systems, including Linux, FreeBSD and OS X, allow only a single parameter in the shebang. The example is equivalent to calling `env 'bash -x'` instead of `env 'bash' '-x'`, and it will therefore fail. The shebang should be rewritten to use at most one parameter. Shell options can instead be set in the body of the script. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2097.md b/SC2097.md index 2c62b5e..7d756af 100644 --- a/SC2097.md +++ b/SC2097.md @@ -2,19 +2,25 @@ ### Problematic code: - name=World cmd -m "Hello $name" +```sh +name=World cmd -m "Hello $name" +``` ### Correct code: - export name=World - cmd -m "Hello $name" +```sh +export name=World +cmd -m "Hello $name" +``` To prevent setting the variable, this can also be done in a subshell: - ( - export name=World - cmd -m "Hello $name" - ) # 'name' does not leave this subshell +```sh +( + export name=World + cmd -m "Hello $name" +) # 'name' does not leave this subshell +``` ### Rationale: @@ -26,4 +32,4 @@ The solution is to set the variable and export the variable first. If limited sc ### Exceptions -In the strange and fabricated scenarios where the script and a program uses a variable name for two different purposes, you can ignore this message. This is hard to conceive, since scripts should use lowercase variable names specifically to avoid collisions with the environment. \ No newline at end of file +In the strange and fabricated scenarios where the script and a program uses a variable name for two different purposes, you can ignore this message. This is hard to conceive, since scripts should use lowercase variable names specifically to avoid collisions with the environment. diff --git a/SC2098.md b/SC2098.md index 0182fca..f9163bc 100644 --- a/SC2098.md +++ b/SC2098.md @@ -1,3 +1,3 @@ ## This expansion will not see the mentioned assignment. -See companion warning [[SC2097]]. \ No newline at end of file +See companion warning [[SC2097]]. diff --git a/SC2101.md b/SC2101.md index 4c4aaf3..31c059a 100644 --- a/SC2101.md +++ b/SC2101.md @@ -2,16 +2,20 @@ ### Problematic code: - gzip file[:digit:]*.txt +```sh +gzip file[:digit:]*.txt +``` ### Correct code: - gzip file[[:digit:]]*.txt +```sh +gzip file[[:digit:]]*.txt +``` ### Rationale: -Predefined character groups are supposed to be used inside character ranges. `[:digit:]` matches one of "digt:" just like `[abc]` matches one of "abc". `[[:digit:]]` matches a digit. +Predefined character groups are supposed to be used inside character ranges. `[:digit:]` matches one of "digt:" just like `[abc]` matches one of "abc". `[[:digit:]]` matches a digit. ### Exceptions -When passing an argument to `tr` which parses these by itself without relying on globbing, you should quote it instead, e.g. `tr -d '[:digit:]'` \ No newline at end of file +When passing an argument to `tr` which parses these by itself without relying on globbing, you should quote it instead, e.g. `tr -d '[:digit:]'` diff --git a/SC2103.md b/SC2103.md index ce18cc3..0377ede 100644 --- a/SC2103.md +++ b/SC2103.md @@ -2,41 +2,47 @@ ### Problematic code: - for dir in */ - do - cd "$dir" - convert index.png index.jpg - cd .. - done +```sh +for dir in */ +do + cd "$dir" + convert index.png index.jpg + cd .. +done +``` ### Correct code: - for dir in */ - do - ( - cd "$dir" || exit - convert index.png index.jpg - ) - done +```sh +for dir in */ +do + ( + cd "$dir" || exit + convert index.png index.jpg + ) +done +``` or - for dir in */ - do - cd "$dir" || exit - convert index.png index.jpg - cd .. - done +```sh +for dir in */ +do + cd "$dir" || exit + convert index.png index.jpg + cd .. +done +``` ### Rationale: -When doing `cd dir; somestuff; cd ..`, `cd dir` can fail when permissions are lacking, if the dir was deleted, or if `dir` is actually a file. +When doing `cd dir; somestuff; cd ..`, `cd dir` can fail when permissions are lacking, if the dir was deleted, or if `dir` is actually a file. In this case, `somestuff` will run in the wrong directory and `cd ..` will take you to an even more wrong directory. In a loop, this will likely cause the next `cd` to fail as well, propagating this error and running these commands far away from the intended directories. -Check `cd`s exit status and/or use subshells to limit the effects of `cd`. +Check `cd`s exit status and/or use subshells to limit the effects of `cd`. ### Exceptions -If you set variables you can't use a subshell. In that case, you should definitely check the exit status of `cd`, which will also silence this suggestion. \ No newline at end of file +If you set variables you can't use a subshell. In that case, you should definitely check the exit status of `cd`, which will also silence this suggestion. diff --git a/SC2107.md b/SC2107.md index 2fe7f26..d0ba84c 100644 --- a/SC2107.md +++ b/SC2107.md @@ -2,11 +2,15 @@ ### Problematic code: - [ "$1" = "-v" && -z "$2" ] +```sh +[ "$1" = "-v" && -z "$2" ] +``` ### Correct code: - [ "$1" = "-v" ] && [ -z "$2" ] +```sh +[ "$1" = "-v" ] && [ -z "$2" ] +``` ### Rationale: @@ -14,4 +18,4 @@ ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2108.md b/SC2108.md index 7987b2b..68e1213 100644 --- a/SC2108.md +++ b/SC2108.md @@ -2,11 +2,15 @@ ### Problematic code: - [[ "$1" = "-v" -a -z "$2" ]] +```sh +[[ "$1" = "-v" -a -z "$2" ]] +``` ### Correct code: - [[ "$1" = "-v" && -z "$2" ]] +```sh +[[ "$1" = "-v" && -z "$2" ]] +``` ### Rationale: @@ -14,4 +18,4 @@ ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2109.md b/SC2109.md index 3c24e56..45b34a7 100644 --- a/SC2109.md +++ b/SC2109.md @@ -2,11 +2,15 @@ ### Problematic code: - [ "$1" = "-v" || "$1" = "-help" ] +```sh +[ "$1" = "-v" || "$1" = "-help" ] +``` ### Correct code: - [ "$1" = "-v" ] || [ "$1" = "-help" ] +```sh +[ "$1" = "-v" ] || [ "$1" = "-help" ] +``` ### Rationale: @@ -14,4 +18,4 @@ ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2110.md b/SC2110.md index a449695..2509cb5 100644 --- a/SC2110.md +++ b/SC2110.md @@ -2,11 +2,15 @@ ### Problematic code: - [[ "$1" = "-v" -o "$1" = "-help" ]] +```sh +[[ "$1" = "-v" -o "$1" = "-help" ]] +``` ### Correct code: - [[ "$1" = "-v" || "$1" = "-help" ]] +```sh +[[ "$1" = "-v" || "$1" = "-help" ]] +``` ### Rationale: @@ -14,4 +18,4 @@ ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2114.md b/SC2114.md index feb95a6..6c501fa 100644 --- a/SC2114.md +++ b/SC2114.md @@ -2,11 +2,15 @@ ### Problematic code: - rm -rf /usr /lib/nvidia-current/xorg/xorg +```sh +rm -rf /usr /lib/nvidia-current/xorg/xorg +``` ### Correct code: - rm -rf /usr/lib/nvidia-current/xorg/xorg +```sh +rm -rf /usr/lib/nvidia-current/xorg/xorg +``` ### 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 `--`: - 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. \ No newline at end of file +This is an arbitrary convention to allow deleting such directories without having to use a [[directive]] to silence the warning. diff --git a/SC2115.md b/SC2115.md index 6c11d44..ea47463 100644 --- a/SC2115.md +++ b/SC2115.md @@ -2,11 +2,15 @@ ### Problematic code: - rm -rf "$STEAMROOT/"* +```sh +rm -rf "$STEAMROOT/"* +``` ### Correct code: - rm -rf "${STEAMROOT:?}/"* +```sh +rm -rf "${STEAMROOT:?}/"* +``` ### Rationale: @@ -14,8 +18,8 @@ If `STEAMROOT` is empty, this will [end up deleting everything](https://github.c Using `:?` will cause the command to fail if the variable is null or unset. Similarly, you can use `:-` to set a default value if applicable. -In the case command substitution, assign to a variable first and then use `:?`. This is relevant even if the command seems simple and obviously correct, since forks and execs can fail due to external system limits and conditions, resulting in a blank substitution. +In the case command substitution, assign to a variable first and then use `:?`. This is relevant even if the command seems simple and obviously correct, since forks and execs can fail due to external system limits and conditions, resulting in a blank substitution. ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2116.md b/SC2116.md index caddbf7..281bc28 100644 --- a/SC2116.md +++ b/SC2116.md @@ -2,11 +2,15 @@ ### Problematic code: - a=$(echo $?) +```sh +a=$(echo $?) +``` ### Correct code: - a="$?" +```sh +a="$?" +``` ### Rationale: @@ -14,4 +18,4 @@ Most of the time, this is an useless echo meaning it isn't doing anything that t ### Exceptions -None I am aware of at the moment. \ No newline at end of file +None I am aware of at the moment. diff --git a/SC2117.md b/SC2117.md index 591ea48..3152bd6 100644 --- a/SC2117.md +++ b/SC2117.md @@ -2,24 +2,28 @@ ### Problematic code: - whoami - su - whoami +```sh +whoami +su +whoami +``` ### Correct code: - whoami - sudo whoami +```sh +whoami +sudo whoami +``` ### Rationale: It's commonly believed that `su` makes a session run as another user. In reality, it starts an entirely new shell, independent of the one currently running your script. -`su; whoami` will start a root shell and wait for it to exit before running `whoami`. It will not start a root shell and then proceed to run `whoami` in it. +`su; whoami` will start a root shell and wait for it to exit before running `whoami`. It will not start a root shell and then proceed to run `whoami` in it. To run commands as another user, use `sudo some command` or `su -c 'some command'`. `sudo` is preferred when available, as it doesn't require additional quoting and can be configured to run passwordless if desired. ### Exceptions -If you're aware of the above and want to e.g. start an interactive shell for a user, feel free to ignore this message. \ No newline at end of file +If you're aware of the above and want to e.g. start an interactive shell for a user, feel free to ignore this message. diff --git a/SC2119.md b/SC2119.md index ed62523..c9098e1 100644 --- a/SC2119.md +++ b/SC2119.md @@ -1,3 +1,3 @@ # Use foo "$@" if function's $1 should mean script's $1. -See companion warning [[SC2120]]. \ No newline at end of file +See companion warning [[SC2120]]. diff --git a/SC2120.md b/SC2120.md index d51a80c..95e4b85 100644 --- a/SC2120.md +++ b/SC2120.md @@ -2,19 +2,23 @@ ### Problematic code: - sayhello() { - echo "Hello $1" - } - sayhello +```sh +sayhello() { + echo "Hello $1" +} +sayhello +``` `./myscript World` just prints "Hello " instead of "Hello World". ### Correct code: - sayhello() { - echo "Hello $1" - } - sayhello "$@" +```sh +sayhello() { + echo "Hello $1" +} +sayhello "$@" +``` `./myscript World` now prints "Hello World". @@ -22,13 +26,15 @@ In a function, `$1` and up refers to the function's parameters, not the script's parameters. -If you want to process your script's parameters in a function, you have to explicitly pass them. You can do this with `myfunction "$@"`. +If you want to process your script's parameters in a function, you have to explicitly pass them. You can do this with `myfunction "$@"`. 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 "$@"; } - second() { echo "The first script parameter is: $1"; } - first "$@" +```sh +first() { second "$@"; } +second() { echo "The first script parameter is: $1"; } +first "$@" +``` ### Exceptions diff --git a/SC2121.md b/SC2121.md index 9106366..7b758f9 100644 --- a/SC2121.md +++ b/SC2121.md @@ -2,12 +2,16 @@ ### Problematic code: - set var=42 - set var 42 +```sh +set var=42 +set var 42 +``` ### Correct code: - var=42 +```sh +var=42 +``` ### Rationale: @@ -17,4 +21,4 @@ To assign variables, use `var=value` with no `set` or other qualifiers. ### Exceptions -If you actually do want to set positional parameters, simply quoting them or using `--` will make shellcheck stop warning, e.g. `set -- var1 var2` or `set "foo=bar"`. \ No newline at end of file +If you actually do want to set positional parameters, simply quoting them or using `--` will make shellcheck stop warning, e.g. `set -- var1 var2` or `set "foo=bar"`. diff --git a/SC2122.md b/SC2122.md index 7ac4baf..eb456ce 100644 --- a/SC2122.md +++ b/SC2122.md @@ -2,16 +2,20 @@ ### Problematic code: - [[ a <= b ]] +```sh +[[ a <= b ]] +``` ### Correct code: - [[ ! a > b ]] +```sh +[[ ! a > b ]] +``` ### Rationale: -The operators `<=` and `>=` are not supported by Bourne shells. Instead of "less than or equal", rewrite as "not greater than". +The operators `<=` and `>=` are not supported by Bourne shells. Instead of "less than or equal", rewrite as "not greater than". ### Exceptions -None \ No newline at end of file +None diff --git a/SC2123.md b/SC2123.md index 7c59793..bb42dea 100644 --- a/SC2123.md +++ b/SC2123.md @@ -2,20 +2,26 @@ ### Problematic code: - PATH=/my/dir - cat "$PATH/myfile" +```sh +PATH=/my/dir +cat "$PATH/myfile" +``` ### Correct code: Good practice: always use lowercase for unexported variables. - path=/my/dir - cat "$path/myfile" +```sh +path=/my/dir +cat "$path/myfile" +``` Bad practice: use another uppercase name. - MYPATH=/my/dir - cat "$MYPATH/myfile" +```sh +MYPATH=/my/dir +cat "$MYPATH/myfile" +``` ### Rationale: @@ -27,4 +33,4 @@ Best shell scripting practice is to always use lowercase variable names to avoid ### Exceptions -If you're aware of the above and really do want to set your shell search path to `/my/dir`, you can ignore this warning. \ No newline at end of file +If you're aware of the above and really do want to set your shell search path to `/my/dir`, you can ignore this warning. diff --git a/SC2124.md b/SC2124.md index e4b95b3..e0d67ff 100644 --- a/SC2124.md +++ b/SC2124.md @@ -2,29 +2,37 @@ ### Problematic code: - var=$@ - for i in $var; do ..; done +```sh +var=$@ +for i in $var; do ..; done +``` or - set -- Hello World - msg=$@ - echo "You said $msg" +```sh +set -- Hello World +msg=$@ +echo "You said $msg" +``` ### Correct code: - var=( "$@" ) - for i in "${var[@]}"; do ..; done +```sh +var=( "$@" ) +for i in "${var[@]}"; do ..; done +``` or - set -- Hello World - msg=$* - echo "You said $msg" +```sh +set -- Hello World +msg=$* +echo "You said $msg" +``` ### Rationale: -Arrays and `$@` can contain multiple elements. Simple variables contain only one. When assigning multiple elements to one element, the default behavior depends on the shell (bash concatenates with spaces, zsh concatenates with first char of `IFS`). +Arrays and `$@` can contain multiple elements. Simple variables contain only one. When assigning multiple elements to one element, the default behavior depends on the shell (bash concatenates with spaces, zsh concatenates with first char of `IFS`). Since doing this usually indicates a bug, ShellCheck warns and asks you to be explicit about what you want. @@ -36,4 +44,4 @@ The same is true for `${@: -1}`, which results in 0 or 1 elements: `var=${*: -1} ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2125.md b/SC2125.md index 4b48d6a..5b877e9 100644 --- a/SC2125.md +++ b/SC2125.md @@ -2,22 +2,26 @@ ### Problematic code: - foo={1..9} - echo $foo +```sh +foo={1..9} +echo $foo +``` ### Correct code: - foo=( {1..9} ) - echo "${foo[@]}" +```sh +foo=( {1..9} ) +echo "${foo[@]}" +``` ### Rationale: -`echo *.png {1..9}` expands to all png files and numbers from 1 to 9, but `var=*.png` or `var={1..9}` will just assign the literal strings `'*.png'` and `'{1..9}'`. +`echo *.png {1..9}` expands to all png files and numbers from 1 to 9, but `var=*.png` or `var={1..9}` will just assign the literal strings `'*.png'` and `'{1..9}'`. -To make the variable contain all png files or 1 through 9, use an array as demonstrated. +To make the variable contain all png files or 1 through 9, use an array as demonstrated. If you intended to assign these values as literals, quote them (e.g. `var="*.png"`). ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2126.md b/SC2126.md index 3fbb4ce..6c89ec5 100644 --- a/SC2126.md +++ b/SC2126.md @@ -2,23 +2,29 @@ ### Problematic code: - grep foo | wc -l +```sh +grep foo | wc -l +``` ### Correct code: - grep -c foo +```sh +grep -c foo +``` ### Rationale: -This is purely a stylistic issue. `grep` can count lines without piping to `wc`. +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: - if grep -q pattern file - then - echo "The file contains the pattern" - fi +```sh +if grep -q pattern file +then + echo "The file contains the pattern" +fi +``` ### Exceptions -If you e.g. want to count characters instead of lines, and you actually care about the number and not just whether it's greater than 0. \ No newline at end of file +If you e.g. want to count characters instead of lines, and you actually care about the number and not just whether it's greater than 0. diff --git a/SC2128.md b/SC2128.md index 0e434f2..8d71c24 100644 --- a/SC2128.md +++ b/SC2128.md @@ -2,19 +2,23 @@ ### Problematic code: - myarray=(foo bar) - for f in $myarray - do - cat "$f" - done +```sh +myarray=(foo bar) +for f in $myarray +do + cat "$f" +done +``` ### Correct code: - myarray=(foo bar) - for f in "${myarray[@]}" - do - cat "$f" - done +```sh +myarray=(foo bar) +for f in "${myarray[@]}" +do + cat "$f" +done +``` ### Rationale: @@ -26,4 +30,4 @@ To get all elements as a single parameter, concatenated by the first character i ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2129.md b/SC2129.md index 06552bc..9671014 100644 --- a/SC2129.md +++ b/SC2129.md @@ -2,18 +2,22 @@ ### Problematic code: - echo foo >> file - date >> file - cat stuff >> file - +```sh +echo foo >> file +date >> file +cat stuff >> file + +``` ### Correct code: - { - echo foo - date - cat stuff - } >> file +```sh +{ + echo foo + date + cat stuff +} >> file +``` ### Rationale: @@ -21,4 +25,4 @@ Rather than adding `>> something` after every single line, you can simply group ### Exceptions -This is mainly a stylistic issue, and can freely be ignored. \ No newline at end of file +This is mainly a stylistic issue, and can freely be ignored. diff --git a/SC2130.md b/SC2130.md index 0b0eb8b..e2b7d9f 100644 --- a/SC2130.md +++ b/SC2130.md @@ -2,11 +2,15 @@ ### Problematic code: - [[ $foo -eq "Y" ]] +```sh +[[ $foo -eq "Y" ]] +``` ### Correct code: - [[ $foo = "Y" ]] +```sh +[[ $foo = "Y" ]] +``` ### Rationale: diff --git a/SC2139.md b/SC2139.md index 32e1228..3335b49 100644 --- a/SC2139.md +++ b/SC2139.md @@ -2,11 +2,15 @@ ### Problematic code: - alias whereami="echo $PWD" +```sh +alias whereami="echo $PWD" +``` ### Correct code: - alias whereami='echo $PWD' +```sh +alias whereami='echo $PWD' +``` ### Rationale: @@ -16,4 +20,4 @@ By using single quotes or escaping any expansions, we define the alias as `echo ### Exceptions -If you don't mind that your alias definition is expanded at define time, you can ignore this warning. \ No newline at end of file +If you don't mind that your alias definition is expanded at define time, you can ignore this warning. diff --git a/SC2140.md b/SC2140.md index 509d95d..64a7d8d 100644 --- a/SC2140.md +++ b/SC2140.md @@ -2,24 +2,32 @@ ### Problematic code: - echo "" > file.html +```sh +echo "" > file.html +``` or - export "var"="42" +```sh +export "var"="42" +``` ### Correct code: - echo "" > file.html +```sh +echo "" > file.html +``` or - export "var=42" +```sh +export "var=42" +``` ### Rationale: -This warning triggers when an unquoted literal string is found suspiciously sandwiched between two double quoted strings. +This warning triggers when an unquoted literal string is found suspiciously sandwiched between two double quoted strings. This usually indicates one of: @@ -28,13 +36,17 @@ This usually indicates one of: Without escaping, the inner two quotes of the sandwich (the end quote of the first section and the start quote of the second section) are no-ops. The following two statements are identical, so the quotes that were intended to be part of the html output are instead removed: - echo "" > file.html - echo "" > file.html +```sh +echo "" > file.html +echo "" > file.html +``` Similarly, these statements are identical, but work as intended: - export "var"="42" - export "var=42" +```sh +export "var"="42" +export "var=42" +``` ### Exceptions @@ -42,7 +54,9 @@ If you know that the quotes are ineffectual but you prefer it stylistically, you It's common not to realize that double quotes can span multiple elements, or to stylistically prefer to quote individual variables. For example, these statements are identical, but the first is laboriously and redundantly quoted: - http://"$user":"$password"@"$host"/"$path" - "http://$user:$password@$host/$path" +```sh +http://"$user":"$password"@"$host"/"$path" +"http://$user:$password@$host/$path" +``` -When ShellCheck detects the first style (i.e. the double quotes include only a single element each), it will suppress the warning. \ No newline at end of file +When ShellCheck detects the first style (i.e. the double quotes include only a single element each), it will suppress the warning. diff --git a/SC2141.md b/SC2141.md index fb35a19..4359097 100644 --- a/SC2141.md +++ b/SC2141.md @@ -2,11 +2,15 @@ ### Problematic code: - IFS="\t" +```sh +IFS="\t" +``` ### Correct code: - IFS=$'\t' +```sh +IFS=$'\t' +``` ### Rationale: @@ -14,4 +18,4 @@ ### Exceptions -It's extremely rare to want to split on the letter "n" or "t", rather than linefeed or tab. \ No newline at end of file +It's extremely rare to want to split on the letter "n" or "t", rather than linefeed or tab. diff --git a/SC2142.md b/SC2142.md index a1642c7..ce1e81b 100644 --- a/SC2142.md +++ b/SC2142.md @@ -2,12 +2,16 @@ ### Problematic code: - alias archive='mv "$@" /backup' +```sh +alias archive='mv "$@" /backup' +``` ### Correct code: - archive() { mv "$@" /backup; } - +```sh +archive() { mv "$@" /backup; } + +``` ### Rationale: @@ -15,4 +19,4 @@ Aliases just substitute the start of a command with something else. They therefo ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2143.md b/SC2143.md index 1fdcb7d..6f3904c 100644 --- a/SC2143.md +++ b/SC2143.md @@ -1,24 +1,28 @@ ## Use grep -q instead of comparing output with [ -n .. ]. ### Problematic code: - if [ "$(find . | grep 'IMG[0-9]')" ] - then - echo "Images found" - fi +```sh +if [ "$(find . | grep 'IMG[0-9]')" ] +then + echo "Images found" +fi +``` ### Correct code: - if find . | grep -q 'IMG[0-9]' - then - echo "Images found" - fi +```sh +if find . | grep -q 'IMG[0-9]' +then + echo "Images found" +fi +``` ### Rationale: -The problematic code has to iterate the entire directory and read all matching lines into memory before making a decision. +The problematic code has to iterate the entire directory and read all matching lines into memory before making a decision. The correct code is cleaner and stops at the first matching line, avoiding both iterating the rest of the directory and reading data into memory. ### Exceptions -The `pipefail` bash option may interfere with this rewrite, since the `if` will now in effect be evaluating the statuses of all commands instead of just the last one. Be careful using them together. \ No newline at end of file +The `pipefail` bash option may interfere with this rewrite, since the `if` will now in effect be evaluating the statuses of all commands instead of just the last one. Be careful using them together. diff --git a/SC2144.md b/SC2144.md index 6011309..2ff586b 100644 --- a/SC2144.md +++ b/SC2144.md @@ -2,25 +2,29 @@ ### Problematic code: - if [ -e dir/*.mp3 ] - then - echo "There are mp3 files." - fi +```sh +if [ -e dir/*.mp3 ] +then + echo "There are mp3 files." +fi +``` ### Correct code: - for file in dir/*.mp3 - do - if [ -e "$file" ] - then - echo "There are mp3 files" - break - fi - done +```sh +for file in dir/*.mp3 +do + if [ -e "$file" ] + then + echo "There are mp3 files" + break + fi +done +``` ### Rationale: -`[ -e file* ]` only works if there's 0 or 1 matches. If there are multiple, it becomes `[ -e file1 file2 ]`, and the test fails. +`[ -e file* ]` only works if there's 0 or 1 matches. If there are multiple, it becomes `[ -e file1 file2 ]`, and the test fails. `[[ -e file* ]]` doesn't work at all. @@ -28,4 +32,4 @@ Instead, use a for loop to expand the glob and check each result individually. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2145.md b/SC2145.md index 8ef3e4d..f3b4232 100644 --- a/SC2145.md +++ b/SC2145.md @@ -2,22 +2,26 @@ ### Problematic code: - printf "Error: %s\n" "Bad parameters: $@" +```sh +printf "Error: %s\n" "Bad parameters: $@" +``` ### Correct code: - printf "Error: %s\n" "Bad parameters: $*" +```sh +printf "Error: %s\n" "Bad parameters: $*" +``` ### Rationale: The behavior when concatenating a string and array is rarely intended. The preceeding string is prefixed to the first array element, while the succeeding string is appended to the last one. The middle array elements are unaffected. -E.g., with the parameters `foo`,`bar`,`baz`, `"--flag=$@"` is equivalent to the three arguments `"--flag=foo" "bar" "baz"`. +E.g., with the parameters `foo`,`bar`,`baz`, `"--flag=$@"` is equivalent to the three arguments `"--flag=foo" "bar" "baz"`. If the intention is to concatenate all the array elements into one argument, use `$*`. This concatenates based on `IFS`. -If the intention is to provide each array element as a separate argument, put the array expansion in its own argument. +If the intention is to provide each array element as a separate argument, put the array expansion in its own argument. ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2146.md b/SC2146.md index 7a0f296..d43abe3 100644 --- a/SC2146.md +++ b/SC2146.md @@ -2,19 +2,23 @@ ### Problematic code: - find . -name '*.avi' -o -name '*.mkv' -exec cp {} /media \; +```sh +find . -name '*.avi' -o -name '*.mkv' -exec cp {} /media \; +``` ### Correct code: - find . \( -name '*.avi' -o -name '*.mkv' \) -exec cp {} /media \; +```sh +find . \( -name '*.avi' -o -name '*.mkv' \) -exec cp {} /media \; +``` ### Rationale: In `find`, two predicates with no operator between them is considered a logical, short-circuiting AND (as if using `-a`). E.g., `-name '*.mkv' -exec ..` is the same as `-name '*.mkv' -a -exec ..`. -`-a` has higher precedence than `-o`, so `-name '*.avi' -o -name '*.mkv' -a -exec ..` is equivalent to `-name '*.avi' -o \( -name '*.mkv' -a -exec .. \)`. +`-a` has higher precedence than `-o`, so `-name '*.avi' -o -name '*.mkv' -a -exec ..` is equivalent to `-name '*.avi' -o \( -name '*.mkv' -a -exec .. \)`. -In other words, the problematic code means "if name matches `*.avi`, do nothing. Otherwise, if it matches `*.mkv`, execute a command.". +In other words, the problematic code means "if name matches `*.avi`, do nothing. Otherwise, if it matches `*.mkv`, execute a command.". In the correct code, we use `\( \)` to group to get the evaluation order we want. The correct code means "if name matches `*.avi` or `*.mkv`, then execute a command", which was what was intended. @@ -22,4 +26,6 @@ In the correct code, we use `\( \)` to group to get the evaluation order we want If you're aware of this, you can either ignore this error or group to make it explicit. For example, to decompress all gz files except tar.gz, you can use: - find . -name '*.tar.gz' -o \( -name '*.gz' -exec gzip -d {} + \) +```sh +find . -name '*.tar.gz' -o \( -name '*.gz' -exec gzip -d {} + \) +``` diff --git a/SC2147.md b/SC2147.md index d9c5ef5..3721df2 100644 --- a/SC2147.md +++ b/SC2147.md @@ -2,11 +2,15 @@ ### Problematic code: - PATH="$PATH:~/bin" +```sh +PATH="$PATH:~/bin" +``` ### Correct code: - PATH="$PATH:$HOME/bin" +```sh +PATH="$PATH:$HOME/bin" +``` ### Rationale: @@ -14,10 +18,10 @@ Having literal `~` in PATH is a bad idea. Bash handles it, but nothing else does This means that even if you're always using Bash, you should avoid it because any invoked program that relies on PATH will effectively ignore those entries. -For example, `make` may say `foo: Command not found` even though `foo` works fine from the shell and Make and Bash both use the same PATH. You'll get similar messages from any non-bash scripts invoked, and `whereis` will come up empty. +For example, `make` may say `foo: Command not found` even though `foo` works fine from the shell and Make and Bash both use the same PATH. You'll get similar messages from any non-bash scripts invoked, and `whereis` will come up empty. Use `$HOME` or full path instead. ### Exceptions -If your directory name actually contains a literal tilde, you can ignore this message. \ No newline at end of file +If your directory name actually contains a literal tilde, you can ignore this message. diff --git a/SC2148.md b/SC2148.md index 1f5fc1f..ddd3dc3 100644 --- a/SC2148.md +++ b/SC2148.md @@ -2,28 +2,34 @@ ### Problematic code: - echo "$RANDOM" # Does this work +```sh +echo "$RANDOM" # Does this work +``` ### Correct code: - #!/bin/sh - echo "$RANDOM" # Unsupported in sh. Produces warning. +```sh +#!/bin/sh +echo "$RANDOM" # Unsupported in sh. Produces warning. +``` or - #!/bin/bash - echo "$RANDOM" # Supported in bash. No warnings. +```sh +#!/bin/bash +echo "$RANDOM" # Supported in bash. No warnings. +``` ### Rationale: Different shells support different features. To give effective advice, ShellCheck needs to know which shell your script is going to run on. You will get a different numbers of warnings about different things depending on your target shell. -ShellCheck normally determines your target shell from the shebang (having e.g. `#!/bin/sh` as the first line). The shell can also be specified from the CLI with `-s`, e.g. `shellcheck -s sh file`. +ShellCheck normally determines your target shell from the shebang (having e.g. `#!/bin/sh` as the first line). The shell can also be specified from the CLI with `-s`, e.g. `shellcheck -s sh file`. If you don't specify shebang nor `-s`, ShellCheck gives this message and proceeds with some default (`bash`). -Note that this error can not be ignored with a [[directive]]. It is not a suggestion to improve your script, but a warning that ShellCheck lacks information it needs to be helpful. +Note that this error can not be ignored with a [[directive]]. It is not a suggestion to improve your script, but a warning that ShellCheck lacks information it needs to be helpful. ### Exceptions -None. Please either add a shebang or use `-s`. \ No newline at end of file +None. Please either add a shebang or use `-s`. diff --git a/SC2149.md b/SC2149.md index 84f72fb..2629bba 100644 --- a/SC2149.md +++ b/SC2149.md @@ -2,27 +2,34 @@ ### Problematic code: - # Regular array - index=42 - echo $((array[$index])) +```sh +# Regular array +index=42 +echo $((array[$index])) +``` or - # Associative array - index=banana - echo $((array[$index])) +```sh +# Associative array +index=banana +echo $((array[$index])) +``` ### Correct code: - - # Regular array - index=42 - echo $((array[index])) + +# Regular array +index=42 +echo $((array[index])) +``` or - # Associative array - index=banana - echo $((array[\$index])) +```sh +# Associative array +index=banana +echo $((array[\$index])) +``` ### Rationale: @@ -30,12 +37,14 @@ For a numerically indexed array, the `$` is mostly pointless and can be removed For associative arrays, the `$` should be escaped to avoid accidental dereferencing: - declare -A array - index='$1' - array[$index]=42 - echo "$(( array[$index] ))" # bash: array: bad array subscript - echo "$(( array[\$index] ))" # 42 +```sh +declare -A array +index='$1' +array[$index]=42 +echo "$(( array[$index] ))" # bash: array: bad array subscript +echo "$(( array[\$index] ))" # 42 +``` ### Exceptions -None. \ No newline at end of file +None. diff --git a/SC2150.md b/SC2150.md index 8a655ba..f834cca 100644 --- a/SC2150.md +++ b/SC2150.md @@ -2,27 +2,33 @@ ### Problematic code: - find . -type f -exec 'cat {} | wc -l' \; +```sh +find . -type f -exec 'cat {} | wc -l' \; +``` ### Correct code: - find . -type f -exec sh -c 'cat {} | wc -l' \; # Insecure - find . -type f -exec sh -c 'cat "$1" | wc -l' _ {} \; # Secure +```sh +find . -type f -exec sh -c 'cat {} | wc -l' \; # Insecure +find . -type f -exec sh -c 'cat "$1" | wc -l' _ {} \; # Secure +``` Sometimes the command can also be rewritten to not require `find` to invoke a shell: - find . -type f -exec wc -l {} \; | cut -d ' ' -f 1 +```sh +find . -type f -exec wc -l {} \; | cut -d ' ' -f 1 +``` ### Rationale: -find `-exec` and `-execdir` uses `execve(2)` style semantics, meaning it expects an executable and zero or more arguments that should be passed to it. +find `-exec` and `-execdir` uses `execve(2)` style semantics, meaning it expects an executable and zero or more arguments that should be passed to it. -It does not use `system(3)` style semantics, meaning it does not accept a shell command as a string, to be parsed and evaluated by the system's command interpreter. +It does not use `system(3)` style semantics, meaning it does not accept a shell command as a string, to be parsed and evaluated by the system's command interpreter. If you want `find` to execute a shell command, you have to specify `sh` (or `bash`) as the executable, `-c` as first argument and your shell command as the second. -To prevent command injection, the filename can be passed as a separate argument to sh and referenced as a positional parameter. +To prevent command injection, the filename can be passed as a separate argument to sh and referenced as a positional parameter. ### Exceptions -This warning would trigger falsely if executing a program with spaces in the path, if no other arguments were specified. \ No newline at end of file +This warning would trigger falsely if executing a program with spaces in the path, if no other arguments were specified. diff --git a/SC2151.md b/SC2151.md index 5128543..c33ac10 100644 --- a/SC2151.md +++ b/SC2151.md @@ -2,26 +2,30 @@ ### Problematic code: - myfunc() { - return foo bar - } +```sh +myfunc() { + return foo bar +} +``` ### Correct code: - myfunc() { - echo foo - echo bar - return 0 - } +```sh +myfunc() { + echo foo + echo bar + return 0 +} +``` ### Rationale: In bash, `return` can only be used to signal success or failure (0 = success, 1-255 = failure). -To return textual or multiple values from a function, write them to stdout and capture them with command substitution instead. +To return textual or multiple values from a function, write them to stdout and capture them with command substitution instead. See [[SC2152]] for more information. ### Exceptions: -None \ No newline at end of file +None diff --git a/SC2152.md b/SC2152.md index 0e67ddb..3590fbc 100644 --- a/SC2152.md +++ b/SC2152.md @@ -2,16 +2,20 @@ ### Problematic code: - myfunc() { - return "Hello $USER" - } +```sh +myfunc() { + return "Hello $USER" +} +``` ### Correct code: - myfunc() { - echo "Hello $USER" - return 0 - } +```sh +myfunc() { + echo "Hello $USER" + return 0 +} +``` ### Rationale: @@ -21,11 +25,13 @@ In bash, `return` can only be used to signal success or failure (0 = success, 1- Results should instead be written to stdout and captured: - message=$(myfunc) - echo "The function wrote: $message" +```sh +message=$(myfunc) +echo "The function wrote: $message" +``` In functions that return small integers, such as getting the cpu temperature, the value should still be written to stdout. `return` should be reserved for error conditions, such as "can't determine CPU temperature". ### Exceptions: -None \ No newline at end of file +None diff --git a/SC2153.md b/SC2153.md index fd28a3b..f1c2eff 100644 --- a/SC2153.md +++ b/SC2153.md @@ -2,13 +2,17 @@ ### Problematic code: - MY_VARIABLE="hello world" - echo "$MYVARIABLE" +```sh +MY_VARIABLE="hello world" +echo "$MYVARIABLE" +``` ### Correct code: - MY_VARIABLE="hello world" - echo "$MY_VARIABLE" +```sh +MY_VARIABLE="hello world" +echo "$MY_VARIABLE" +``` ### Rationale: @@ -18,4 +22,4 @@ Note: This error only triggers for environment variables (all uppercase variable ### Exceptions: -If you've double checked and ensured that you did not intend to reference the specified variable, you can disable this message with a [[directive]]. The message will also not appear for guarded references like `${ENVVAR:-default}` or `${ENVVAR:?Unset error message here}`. \ No newline at end of file +If you've double checked and ensured that you did not intend to reference the specified variable, you can disable this message with a [[directive]]. The message will also not appear for guarded references like `${ENVVAR:-default}` or `${ENVVAR:?Unset error message here}`. diff --git a/SC2154.md b/SC2154.md index 938f888..442ea1e 100644 --- a/SC2154.md +++ b/SC2154.md @@ -2,42 +2,54 @@ ### Problematic code: - var=name - n=42 - echo "$var_$n.jpg" # overextended +```sh +var=name +n=42 +echo "$var_$n.jpg" # overextended +``` or - target="world" - echo "hello $tagret" # misspelled +```sh +target="world" +echo "hello $tagret" # misspelled +``` or - echo "Result: ${mycmd -a myfile}" # trying to execute commands +```sh +echo "Result: ${mycmd -a myfile}" # trying to execute commands +``` ### Correct code: - var=name - n=42 - echo "${var}_$n.jpg" +```sh +var=name +n=42 +echo "${var}_$n.jpg" +``` or - target="world" - echo "hello $target" +```sh +target="world" +echo "hello $target" +``` or - echo "Result: $(mycmd -a myfile)" +```sh +echo "Result: $(mycmd -a myfile)" +``` ### Rationale: -ShellCheck has noticed that you reference a variable that is not assigned. Double check that the variable is indeed assigned, and that the name is not misspelled. +ShellCheck has noticed that you reference a variable that is not assigned. Double check that the variable is indeed assigned, and that the name is not misspelled. Note: This message only triggers for variables with lowercase characters in their name (`foo` and `kFOO` but not `FOO`) due to the standard convention of using lowercase variable names for unexported, local variables. ### Exceptions: -ShellCheck does not attempt to figure out runtime or dynamic assignments like with `source "$(date +%F).sh"` or `eval var=value`. +ShellCheck does not attempt to figure out runtime or dynamic assignments like with `source "$(date +%F).sh"` or `eval var=value`. -If you know for a fact that the variable is set, you can use `${var:?}` to fail if the variable is unset (or empty), or explicitly initialize/declare it with `var=""` or `declare var`. You can also disable the message with a [[directive]]. \ No newline at end of file +If you know for a fact that the variable is set, you can use `${var:?}` to fail if the variable is unset (or empty), or explicitly initialize/declare it with `var=""` or `declare var`. You can also disable the message with a [[directive]]. diff --git a/SC2155.md b/SC2155.md index d6f2fb9..f19caed 100644 --- a/SC2155.md +++ b/SC2155.md @@ -2,12 +2,16 @@ ### Problematic code: - export foo="$(mycmd)" +```sh +export foo="$(mycmd)" +``` ### Correct code: - export foo - foo=$(mycmd) +```sh +export foo +foo=$(mycmd) +``` ### Rationale: @@ -19,7 +23,9 @@ When first marked for export and assigned separately, the return value of the as If you intend to ignore the return value of an assignment, you can either ignore this warning or use - export foo - foo=$(mycmd) || true +```sh +export foo +foo=$(mycmd) || true +``` -Shellcheck does not warn about `export foo=bar` because `bar` is a literal and not a command substitution with an independent return value. It also does not warn about `local -r foo=$(cmd)`, where declaration and assignment must be in the same command. \ No newline at end of file +Shellcheck does not warn about `export foo=bar` because `bar` is a literal and not a command substitution with an independent return value. It also does not warn about `local -r foo=$(cmd)`, where declaration and assignment must be in the same command. diff --git a/SC2156.md b/SC2156.md index 483fc37..360c67c 100644 --- a/SC2156.md +++ b/SC2156.md @@ -2,18 +2,22 @@ ### Problematic code: - find . -name '*.mp3' -exec sh -c 'i="{}"; sox "$i" "${i%.mp3}.wav"' \; +```sh +find . -name '*.mp3' -exec sh -c 'i="{}"; sox "$i" "${i%.mp3}.wav"' \; +``` ### Correct code: - find . -name '*.mp3' -exec sh -c 'i="$1"; sox "$i" "${i%.mp3}.wav"' _ {} \; +```sh +find . -name '*.mp3' -exec sh -c 'i="$1"; sox "$i" "${i%.mp3}.wav"' _ {} \; +``` ### Rationale: In the problematic example, the filename is passed by injecting it into a shell string. Any shell metacharacters in the filename will be interpreted as part of the script, and not as part of the filename. This can break the script and allow arbitrary code execution exploits. -In the correct example, the filename is passed as a parameter. It will be safely treated as literal text. The `_` is a dummy string that becomes `$0` in the script. +In the correct example, the filename is passed as a parameter. It will be safely treated as literal text. The `_` is a dummy string that becomes `$0` in the script. ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2157.md b/SC2157.md index 95ba391..8df9804 100644 --- a/SC2157.md +++ b/SC2157.md @@ -2,17 +2,21 @@ ### Problematic code: - if [ "$foo " ] - then - echo "this is always true" - fi +```sh +if [ "$foo " ] +then + echo "this is always true" +fi +``` ### Correct code: - if [ "$foo" ] - then - echo "correctly checks value" - fi +```sh +if [ "$foo" ] +then + echo "correctly checks value" +fi +``` ### Rationale: @@ -22,4 +26,4 @@ Sometimes this is also caused by overquoting an example, e.g. `[ "$foo -gt 0" ]` ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2158.md b/SC2158.md index fd3c2d8..84d2ee2 100644 --- a/SC2158.md +++ b/SC2158.md @@ -2,24 +2,28 @@ ### Problematic code: - if [ false ] - then - echo "triggers anyways" - fi +```sh +if [ false ] +then + echo "triggers anyways" +fi +``` ### Correct code: - if false - then - echo "never triggers" - fi +```sh +if false +then + echo "never triggers" +fi +``` ### Rationale: -`[ str ]` checks whether `str` is non-empty. It doesn't matter if `str` is `false`, it will still be evaluated for non-emptyness. +`[ str ]` checks whether `str` is non-empty. It doesn't matter if `str` is `false`, it will still be evaluated for non-emptyness. Instead, use the command `false` which -- as the manual puts it -- does nothing, unsuccessfully. ### Exceptions: -None \ No newline at end of file +None diff --git a/SC2159.md b/SC2159.md index da26dd7..b1a8ad6 100644 --- a/SC2159.md +++ b/SC2159.md @@ -2,24 +2,28 @@ ### Problematic code: - if [ 0 ] - then - echo "always triggers" - fi +```sh +if [ 0 ] +then + echo "always triggers" +fi +``` ### Correct code: - if false - then - echo "never triggers" - fi +```sh +if false +then + echo "never triggers" +fi +``` ### Rationale: -`[ str ]` checks whether `str` is non-empty. It doesn't matter if `str` is `0`, it will still be evaluated for non-emptyness. +`[ str ]` checks whether `str` is non-empty. It doesn't matter if `str` is `0`, it will still be evaluated for non-emptyness. Instead, use the command `false` which -- as the manual puts it -- does nothing, unsuccessfully. ### Exceptions: -None \ No newline at end of file +None diff --git a/SC2160.md b/SC2160.md index 39ebeaa..f73f575 100644 --- a/SC2160.md +++ b/SC2160.md @@ -2,21 +2,25 @@ ### Problematic code: - if [ true ] - then - echo "always triggers" - fi +```sh +if [ true ] +then + echo "always triggers" +fi +``` ### Correct code: - if true - then - echo "always triggers" - fi +```sh +if true +then + echo "always triggers" +fi +``` ### Rationale: -This is a stylistic suggestion to use `true` instead of `[ true ]`. +This is a stylistic suggestion to use `true` instead of `[ true ]`. `[ true ]` seems to suggest that the value "true" is somehow relevant to the statement. This is not the case, it doesn't matter. You can replace it with `[ false ]` or `[ wombat ]`, and it will still always be true: @@ -30,4 +34,4 @@ It's therefore better to use it without brackets, so that the "true" actually ma ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2161.md b/SC2161.md index b9e8051..92c7cd1 100644 --- a/SC2161.md +++ b/SC2161.md @@ -2,21 +2,25 @@ ### Problematic code: - while [ 1 ] - do - echo "infinite loop" - done +```sh +while [ 1 ] +do + echo "infinite loop" +done +``` ### Correct code: - while true - do - echo "infinite loop" - done +```sh +while true +do + echo "infinite loop" +done +``` ### Rationale: -This is a stylistic suggestion to use `true` instead of `[ 1 ]`. +This is a stylistic suggestion to use `true` instead of `[ 1 ]`. `[ 1 ]` seems to suggest that the value "1" is somehow relevant to the statement. This is not the case: it doesn't matter. You can replace it with `[ 0 ]` or `[ wombat ]`, and it will still always be true. @@ -26,4 +30,4 @@ On bash, you can also use `(( 1 ))`, which evaluates to true much like in C. `(( ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2162.md b/SC2162.md index ce35476..d91d5e4 100644 --- a/SC2162.md +++ b/SC2162.md @@ -2,13 +2,17 @@ ### Problematic code: - echo "Enter name:" - read name +```sh +echo "Enter name:" +read name +``` ### Correct code: - echo "Enter name:" - read -r name +```sh +echo "Enter name:" +read -r name +``` ### Rationale: @@ -20,4 +24,4 @@ Note that `read -r` will still strip leading and trailing spaces. `IFS="" read - ### Exceptions: -If you want backslashes to affect field splitting and line terminators instead of being read, you can disable this message with a [[directive]]. \ No newline at end of file +If you want backslashes to affect field splitting and line terminators instead of being read, you can disable this message with a [[directive]]. diff --git a/SC2163.md b/SC2163.md index c98a97c..b47bc68 100644 --- a/SC2163.md +++ b/SC2163.md @@ -1,22 +1,28 @@ -## Exporting an expansion rather than a variable. +## Exporting an expansion rather than a variable. ### Problematic code: - MYVAR=foo - export $MYVAR +```sh +MYVAR=foo +export $MYVAR +``` ### Correct code: - MYVAR=foo - export MYVAR +```sh +MYVAR=foo +export MYVAR +``` ### Rationale: -`export` takes a variable name, but shellcheck has noticed that you give it an expanded variable instead. The problematic code does not export `MYVAR` but a variable called `foo` if any. +`export` takes a variable name, but shellcheck has noticed that you give it an expanded variable instead. The problematic code does not export `MYVAR` but a variable called `foo` if any. ### Exceptions: If you do want to export the variable's value, e.g. due to indirection, you can disable this message with a directive: - # shellcheck disable=SC2163 - export "$MYVAR" \ No newline at end of file +```sh +# shellcheck disable=SC2163 +export "$MYVAR" +``` diff --git a/SC2164.md b/SC2164.md index 5d9de23..d4c6b23 100644 --- a/SC2164.md +++ b/SC2164.md @@ -2,19 +2,23 @@ ### Problematic code: - cd generated_files - rm -r *.c +```sh +cd generated_files +rm -r *.c +``` ### Correct code: - cd generated_files || exit - rm -r *.c +```sh +cd generated_files || exit +rm -r *.c +``` ### Rationale: `cd` can fail for a variety of reasons: misspelled paths, missing directories, missing permissions, broken symlinks and more. -If/when it does, the script will keep going and do all its operations in the wrong direction. This can be messy, especially if the operations involve creating or deleting a lot of files. +If/when it does, the script will keep going and do all its operations in the wrong direction. This can be messy, especially if the operations involve creating or deleting a lot of files. You should therefore always check the condition of `cd`, either with `|| exit` as suggested, or things like `if cd somewhere; then ...; fi`. @@ -22,4 +26,4 @@ You should therefore always check the condition of `cd`, either with `|| exit` a ShellCheck does not give this warning when `cd` is on the left of a `||` or `&&`, or the condition of a `if`, `while` or `until` loop. Having a `set -e` command anywhere in the script will disable this message, even though it won't necessarily prevent the issue. -If you are accounting for `cd` failures in a way shellcheck doesn't realize, you can disable this message with a [[directive]]. \ No newline at end of file +If you are accounting for `cd` failures in a way shellcheck doesn't realize, you can disable this message with a [[directive]]. diff --git a/SC2165.md b/SC2165.md index 855d455..e98bcb9 100644 --- a/SC2165.md +++ b/SC2165.md @@ -4,23 +4,27 @@ And companion warning "This parent loop has its index variable overridden." ### Problematic code: - for((i=0; i<10; i++)) - do - for i in * - do - echo "$i" - done - done +```sh +for((i=0; i<10; i++)) +do + for i in * + do + echo "$i" + done +done +``` ### Correct code: - for((i=0; i<10; i++)) - do - for j in * - do - echo "$j" - done - done +```sh +for((i=0; i<10; i++)) +do + for j in * + do + echo "$j" + done +done +``` ### Rationale: @@ -32,4 +36,4 @@ In nested for-in loops, variable merely shadow each other and won't cause infini ### Exceptions: -None. \ No newline at end of file +None. diff --git a/SC2166.md b/SC2166.md index 34466c4..df35f56 100644 --- a/SC2166.md +++ b/SC2166.md @@ -4,18 +4,22 @@ And likewise, prefer `[ p ] || [ q ]` over `[ p -o q ]`. ### Problematic code: - [ "$1" = "test" -a -z "$2" ] +```sh +[ "$1" = "test" -a -z "$2" ] +``` ### Correct code: - [ "$1" = "test" ] && [ -z "$2" ] +```sh +[ "$1" = "test" ] && [ -z "$2" ] +``` ### Rationale: `-a` and `-o` to mean AND and OR in a `[ .. ]` test expression is not well defined. They are obsolescent extensions in [POSIX](http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html) and their behavior is almost always undefined. -Using multiple `[ .. ]` expressions with shell AND/OR operators `&&` and `||` is well defined and therefore preferred (but note that they have equal precedence, while `-a`/`-o` is unspecified but often implemented as `-a` having higher precedence). +Using multiple `[ .. ]` expressions with shell AND/OR operators `&&` and `||` is well defined and therefore preferred (but note that they have equal precedence, while `-a`/`-o` is unspecified but often implemented as `-a` having higher precedence). ### Exceptions: -None. \ No newline at end of file +None. diff --git a/Sc2035.md b/Sc2035.md index d388748..30bbb4a 100644 --- a/Sc2035.md +++ b/Sc2035.md @@ -11,7 +11,7 @@ Since files and arguments are strings passed the same way, programs can't properly determine which is which, and rely on dashes to determine what's what. -A file named `-f` (`touch -- -f`) will not be deleted by the problematic code. It will instead be interpreted as a command line option, and `rm` will even report success. +A file named `-f` (`touch -- -f`) will not be deleted by the problematic code. It will instead be interpreted as a command line option, and `rm` will even report success. Using `./*` will instead cause the glob to be expanded into `./-f`, which no program will treat as an option. diff --git a/Sc2045.md b/Sc2045.md index fe32c16..0fae3db 100644 --- a/Sc2045.md +++ b/Sc2045.md @@ -19,11 +19,11 @@ Also note that in Bash, `shopt -s nullglob` will allow the loop to run 0 times i ### Rationale: -When looping over a set of files, it's always better to use globs when possible. Using command expansion causes word splitting and glob expansion, which will cause problems for certain filenames (typically first seen when trying to process a file with spaces in the name). +When looping over a set of files, it's always better to use globs when possible. Using command expansion causes word splitting and glob expansion, which will cause problems for certain filenames (typically first seen when trying to process a file with spaces in the name). The following files can or will break the first loop: touch 'filename with spaces.wav' - touch 'filename with * globs.wav' + touch 'filename with * globs.wav' touch 'More_Globs[2003].wav' touch 'files_with_fønny_chæracters_in_certain_locales.wav' diff --git a/Sc2046.md b/Sc2046.md index fdd14d3..46a8b8f 100644 --- a/Sc2046.md +++ b/Sc2046.md @@ -17,16 +17,16 @@ ### Rationale: -When command expansions are unquoted, word splitting and globbing will occur. This often manifests itself by breaking when filenames contain spaces. +When command expansions are unquoted, word splitting and globbing will occur. This often manifests itself by breaking when filenames contain spaces. -Trying to fix it by adding quotes or escapes to the data will not work. Instead, quote the command substitution itself. +Trying to fix it by adding quotes or escapes to the data will not work. Instead, quote the command substitution itself. If the command substitution outputs multiple pieces of data, use a loop instead. ### Exceptions -In rare cases you actually want word splitting, such as in +In rare cases you actually want word splitting, such as in gcc $(pkg-config --libs openssl) client.c -This is because `pkg-config` outputs `-lssl -lcrypto`, which you want to break up by spaces into `-lssl` and `-lcrypto`. \ No newline at end of file +This is because `pkg-config` outputs `-lssl -lcrypto`, which you want to break up by spaces into `-lssl` and `-lcrypto`. \ No newline at end of file diff --git a/Sc2086.md b/Sc2086.md index 831fd8c..80f9c77 100644 --- a/Sc2086.md +++ b/Sc2086.md @@ -6,7 +6,7 @@ ### Correct code: echo "$1" -### Rationale +### Rationale The problematic code looks like "print the first argument". It's actually "Split the first argument by spaces, tabs and line feeds. Expand each of them as if it was a glob. Join all the resulting strings and filenames with spaces. Print the result." Quoting variables prevents word splitting and glob expansion, and prevents the script from breaking when input contains spaces, line feeds, glob characters and such.