diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,481 @@ +## v0.8.0 - 2021-11-06 +### Added +- `disable=all` now conveniently disables all warnings +- `external-sources=true` directive can be added to .shellcheckrc to make + shellcheck behave as if `-x` was specified. +- Optional `check-extra-masked-returns` for pointing out commands with + suppressed exit codes (SC2312). +- Optional `require-double-brackets` for recommending \[\[ ]] (SC2292). +- SC2286-SC2288: Warn when command name ends in a symbol like `/.)'"` +- SC2289: Warn when command name contains tabs or linefeeds +- SC2291: Warn about repeated unquoted spaces between words in echo +- SC2292: Suggest [[ over [ in Bash/Ksh scripts (optional) +- SC2293/SC2294: Warn when calling `eval` with arrays +- SC2295: Warn about "${x#$y}" treating $y as a pattern when not quoted +- SC2296-SC2301: Improved warnings for bad parameter expansions +- SC2302/SC2303: Warn about loops over array values when using them as keys +- SC2304-SC2306: Warn about unquoted globs in expr arguments +- SC2307: Warn about insufficient number of arguments to expr +- SC2308: Suggest other approaches for non-standard expr extensions +- SC2313: Warn about `read` with unquoted, array indexed variable + +### Fixed +- SC2102 about repetitions in ranges no longer triggers on [[ -v arr[xx] ]] +- SC2155 now recognizes `typeset` and local read-only `declare` statements +- SC2181 now tries to avoid triggering for error handling functions +- SC2290: Warn about misused = in declare & co, which were not caught by SC2270+ +- The flag --color=auto no longer outputs color when TERM is "dumb" or unset + +### Changed +- SC2048: Warning about $\* now also applies to ${array[\*]} +- SC2181 now only triggers on single condition tests like `[ $? = 0 ]`. +- Quote warnings are now emitted for declaration utilities in sh +- Leading `_` can now be used to suppress warnings about unused variables +- TTY output now includes warning level in text as well as color + +### Removed +- SC1004: Literal backslash+linefeed in '' was found to be usually correct + + +## v0.7.2 - 2021-04-19 +### Added +- `disable` directives can now be a range, e.g. `disable=SC3000-SC4000` +- SC1143: Warn about line continuations in comments +- SC2259/SC2260: Warn when redirections override pipes +- SC2261: Warn about multiple competing redirections +- SC2262/SC2263: Warn about aliases declared and used in the same parsing unit +- SC2264: Warn about wrapper functions that blatantly recurse +- SC2265/SC2266: Warn when using & or | with test statements +- SC2267: Warn when using xargs -i instead of -I +- SC2268: Warn about unnecessary x-comparisons like `[ x$var = xval ]` + +### Fixed +- SC1072/SC1073 now respond to disable annotations, though ignoring parse errors + is still purely cosmetic and does not allow ShellCheck to continue. +- Improved error reporting for trailing tokens after ]/]] and compound commands +- `#!/usr/bin/env -S shell` is now handled correctly +- Here docs with \r are now parsed correctly and give better warnings + +### Changed +- Assignments are now parsed to spec, without leniency for leading $ or spaces +- POSIX/dash unsupported feature warnings now have individual SC3xxx codes +- SC1090: A leading `$x/` or `$(x)/` is now treated as `./` when locating files +- SC2154: Variables appearing in -z/-n tests are no longer considered unassigned +- SC2270-SC2285: Improved warnings about misused `=`, e.g. `${var}=42` + + +## v0.7.1 - 2020-04-04 +### Fixed +- `-f diff` no longer claims that it found more issues when it didn't +- Known empty variables now correctly trigger SC2086 +- ShellCheck should now be compatible with Cabal 3 +- SC2154 and all command-specific checks now trigger for builtins + called with `builtin` + +### Added +- SC1136: Warn about unexpected characters after ]/]] +- SC2254: Suggest quoting expansions in case statements +- SC2255: Suggest using `$((..))` in `[ 2*3 -eq 6 ]` +- SC2256: Warn about translated strings that are known variables +- SC2257: Warn about arithmetic mutation in redirections +- SC2258: Warn about trailing commas in for loop elements + +### Changed +- SC2230: 'command -v' suggestion is now off by default (-i deprecate-which) +- SC1081: Keywords are now correctly parsed case sensitively, with a warning + + +## v0.7.0 - 2019-07-28 +### Added +- Precompiled binaries for macOS and Linux aarch64 +- Preliminary support for fix suggestions +- New `-f diff` unified diff format for auto-fixes +- Files containing Bats tests can now be checked +- Directory wide directives can now be placed in a `.shellcheckrc` +- Optional checks: Use `--list-optional` to show a list of tests, + Enable with `-o` flags or `enable=name` directives +- Source paths: Use `-P dir1:dir2` or a `source-path=dir1` directive + to specify search paths for sourced files. +- json1 format like --format=json but treats tabs as single characters +- Recognize FLAGS variables created by the shflags library. +- Site-specific changes can now be made in Custom.hs for ease of patching +- SC2154: Also warn about unassigned uppercase variables (optional) +- SC2252: Warn about `[ $a != x ] || [ $a != y ]`, similar to SC2055 +- SC2251: Inform about ineffectual ! in front of commands +- SC2250: Warn about variable references without braces (optional) +- SC2249: Warn about `case` with missing default case (optional) +- SC2248: Warn about unquoted variables without special chars (optional) +- SC2247: Warn about $"(cmd)" and $"{var}" +- SC2246: Warn if a shebang's interpreter ends with / +- SC2245: Warn that Ksh ignores all but the first glob result in `[` +- SC2243/SC2244: Suggest using explicit -n for `[ $foo ]` (optional) +- SC1135: Suggest not ending double quotes just to make $ literal + +### Changed +- If a directive or shebang is not specified, a `.bash/.bats/.dash/.ksh` + extension will be used to infer the shell type when present. +- Disabling SC2120 on a function now disables SC2119 on call sites + +### Fixed +- SC2183 no longer warns about missing printf args for `%()T` + +## v0.6.0 - 2018-12-02 +### Added +- Command line option --severity/-S for filtering by minimum severity +- Command line option --wiki-link-count/-W for showing wiki links +- SC2152/SC2151: Warn about bad `exit` values like `1234` and `"foo"` +- SC2236/SC2237: Suggest -n/-z instead of ! -z/-n +- SC2238: Warn when redirecting to a known command name, e.g. ls > rm +- SC2239: Warn if the shebang is not an absolute path, e.g. #!bin/sh +- SC2240: Warn when passing additional arguments to dot (.) in sh/dash +- SC1133: Better diagnostics when starting a line with |/||/&& + +### Changed +- Most warnings now have useful end positions +- SC1117 about unknown double-quoted escape sequences has been retired + +### Fixed +- SC2021 no longer triggers for equivalence classes like `[=e=]` +- SC2221/SC2222 no longer mistriggers on fall-through case branches +- SC2081 about glob matches in `[ .. ]` now also triggers for `!=` +- SC2086 no longer warns about spaces in `$#` +- SC2164 no longer suggests subshells for `cd ..; cmd; cd ..` +- `read -a` is now correctly considered an array assignment +- SC2039 no longer warns about LINENO now that it's POSIX + +## v0.5.0 - 2018-05-31 +### Added +- SC2233/SC2234/SC2235: Suggest removing or replacing (..) around tests +- SC2232: Warn about invalid arguments to sudo +- SC2231: Suggest quoting expansions in for loop globs +- SC2229: Warn about 'read $var' +- SC2227: Warn about redirections in the middle of 'find' commands +- SC2224/SC2225/SC2226: Warn when using mv/cp/ln without a destination +- SC2223: Quote warning specific to `: ${var=value}` +- SC1131: Warn when using `elseif` or `elsif` +- SC1128: Warn about blanks/comments before shebang +- SC1127: Warn about C-style comments + +### Fixed +- Annotations intended for a command's here documents now work +- Escaped characters inside groups in =~ regexes now parse +- Associative arrays are now respected in arithmetic contexts +- SC1087 about `$var[@]` now correctly triggers on any index +- Bad expansions in here documents are no longer ignored +- FD move operations like {fd}>1- now parse correctly + +### Changed +- Here docs are now terminated as per spec, rather than by presumed intent +- SC1073: 'else if' is now parsed correctly and not like 'elif' +- SC2163: 'export $name' can now be silenced with 'export ${name?}' +- SC2183: Now warns when printf arg count is not a multiple of format count + +## v0.4.7 - 2017-12-08 +### Added +- Statically linked binaries for Linux and Windows (see README.md)! +- `-a` flag to also include warnings in `source`d files +- SC2221/SC2222: Warn about overridden case branches +- SC2220: Warn about unhandled error cases in getopt loops +- SC2218: Warn when using functions before they're defined +- SC2216/SC2217: Warn when piping/redirecting to mv/cp and other non-readers +- SC2215: Warn about commands starting with leading dash +- SC2214: Warn about superfluous getopt flags +- SC2213: Warn about unhandled getopt flags +- SC2212: Suggest `false` over `[ ]` +- SC2211: Warn when using a glob as a command name +- SC2210: Warn when redirecting to an integer, e.g. `foo 1>2` +- SC2206/SC2207: Suggest alternatives when using word splitting in arrays +- SC1117: Warn about double quoted, undefined backslash sequences +- SC1113/SC1114/SC1115: Recognized more malformed shebangs + +### Fixed +- `[ -v foo ]` no longer warns if `foo` is undefined +- SC2037 is now suppressed by quotes, e.g. `PAGER="cat" man foo` +- Ksh nested array declarations now parse correctly +- Parameter Expansion without colons are now recognized, e.g. `${foo+bar}` +- The `lastpipe` option is now respected with regard to subshell warnings +- `\(` is now respected for grouping in `[` +- Leading `\` is now ignored for commands, to allow alias suppression +- Comments are now allowed after directives to e.g. explain 'disable' + + +## v0.4.6 - 2017-03-26 +### Added +- SC2204/SC2205: Warn about `( -z foo )` and `( foo -eq bar )` +- SC2200/SC2201: Warn about brace expansion in [/[[ +- SC2198/SC2199: Warn about arrays in [/[[ +- SC2196/SC2197: Warn about deprecated egrep/fgrep +- SC2195: Warn about unmatchable case branches +- SC2194: Warn about constant 'case' statements +- SC2193: Warn about `[[ file.png == *.mp3 ]]` and other unmatchables +- SC2188/SC2189: Warn about redirections without commands +- SC2186: Warn about deprecated `tempfile` +- SC1109: Warn when finding `&`/`>`/`<` unquoted +- SC1108: Warn about missing spaces in `[ var= foo ]` + +### Changed +- All files are now read as UTF-8 with lenient latin1 fallback, ignoring locale +- Unicode quotes are no longer considered syntactic quotes +- `ash` scripts will now be checked as `dash` with a warning + +### Fixed +- `-c` no longer suggested when using `grep -o | wc` +- Comments and whitespace are now allowed before filewide directives +- Here doc delimiters with esoteric quoting like `foo""` are now handled +- SC2095 about `ssh` in while read loops is now suppressed when using `-n` +- `%(%Y%M%D)T` now recognized as a single formatter in `printf` checks +- `grep -F` now suppresses regex related suggestions +- Command name checks now recognize busybox applet names + + +## v0.4.5 - 2016-10-21 +### Added +- A Docker build (thanks, kpankonen!) +- SC2185: Suggest explicitly adding path for `find` +- SC2184: Warn about unsetting globs (e.g. `unset foo[1]`) +- SC2183: Warn about `printf` with more formatters than variables +- SC2182: Warn about ignored arguments with `printf` +- SC2181: Suggest using command directly instead of `if [ $? -eq 0 ]` +- SC1106: Warn when using `test` operators in `(( 1 -eq 2 ))` + +### Changed +- Unrecognized directives now causes a warning rather than parse failure. + +### Fixed +- Indices in associative arrays are now parsed correctly +- Missing shebang warning squashed when specifying with a directive +- Ksh multidimensional arrays are now supported +- Variables in substring ${a:x:y} expansions now count as referenced +- SC1102 now also handles ambiguous `$((` +- Using `$(seq ..)` will no longer suggest quoting +- SC2148 (missing shebang) is now suppressed when using shell directives +- `[ a '>' b ]` is now recognized as being correctly escaped + + +## v0.4.4 - 2016-05-15 +### Added +- Haskell Stack support (thanks, Arguggi!) +- SC2179/SC2178: Warn when assigning/appending strings to arrays +- SC1102: Warn about ambiguous `$(((` +- SC1101: Warn when \\ linebreaks have trailing spaces + +### Changed +- Directives directly after the shebang now apply to the entire file + +### Fixed +- `{$i..10}` is now flagged similar to `{1..$i}` + + +## v0.4.3 - 2016-01-13 +### Fixed +- Build now works on GHC 7.6.3 as found on Debian Stable/Ubuntu LTS + + +## v0.4.2 - 2016-01-09 +### Added +- First class support for the `dash` shell +- The `--color` flag similar to ls/grep's (thanks, haguenau!) +- SC2174: Warn about unexpected behavior of `mkdir -pm` (thanks, eatnumber1!) +- SC2172: Warn about non-portable use of signal numbers in `trap` +- SC2171: Warn about `]]` without leading `[[` +- SC2168: Warn about `local` outside functions + +### Fixed +- Warnings about unchecked `cd` will no longer trigger with `set -e` +- `[ a -nt/-ot/-ef b ]` no longer warns about being constant +- Quoted test operators like `[ foo "<" bar ]` now parse +- Escaped quotes in backticks now parse correctly + + +## v0.4.1 - 2015-09-05 +### Fixed +- Added missing files to Cabal, fixing the build + + +## v0.4.0 - 2015-09-05 +### Added +- Support for following `source`d files +- Support for setting default flags in `SHELLCHECK_OPTS` +- An `--external-sources` flag for following arbitrary `source`d files +- A `source` directive to override the filename to `source` +- SC2166: Suggest using `[ p ] && [ q ]` over `[ p -a q ]` +- SC2165: Warn when nested `for` loops use the same variable name +- SC2164: Warn when using `cd` without checking that it succeeds +- SC2163: Warn about `export $var` +- SC2162: Warn when using `read` without `-r` +- SC2157: Warn about `[ "$var " ]` and similar never-empty string matches + +### Fixed +- `cat -vnE file` and similar will no longer flag as UUOC +- Nested trinary operators in `(( ))` now parse correctly +- Ksh `${ ..; }` command expansions now parse + + +## v0.3.8 - 2015-06-20 +### Changed +- ShellCheck's license has changed from AGPLv3 to GPLv3. + +### Added +- SC2156: Warn about injecting filenames in `find -exec sh -c "{}" \;` + +### Fixed +- Variables and command substitutions in brace expansions are now parsed +- ANSI colors are now disabled on Windows +- Empty scripts now parse + + +## v0.3.7 - 2015-04-16 +### Fixed +- Build now works on GHC 7.10 +- Use `regex-tdfa` over `regex-compat` since the latter crashes on OS X. + +## v0.3.6 - 2015-03-28 +### Added +- SC2155: Warn about masked return values in `export foo=$(exit 1)` +- SC2154: Warn when a lowercase variable is referenced but not assigned +- SC2152/SC2151: Warn about bad `return` values like `1234` and `"foo"` +- SC2150: Warn about `find -exec "shell command" \;` + +### Fixed +- `coproc` is now supported +- Trinary operator now recognized in `((..))` + +### Removed +- Zsh support has been removed + + +## v0.3.5 - 2014-11-09 +### Added +- SC2148: Warn when not including a shebang +- SC2147: Warn about literal ~ in PATH +- SC1086: Warn about `$` in for loop variables, e.g. `for $i in ..` +- SC1084: Warn when the shebang uses `!#` instead of `#!` + +### Fixed +- Empty and comment-only backtick expansions now parse +- Variables used in PS1/PROMPT\_COMMAND/trap now count as referenced +- ShellCheck now skips unreadable files and directories +- `-f gcc` on empty files no longer crashes +- Variables in $".." are now considered quoted +- Warnings about expansions in single quotes now include backticks + + +## v0.3.4 - 2014-07-08 +### Added +- SC2146: Warn about precedence when combining `find -o` with actions +- SC2145: Warn when concatenating arrays and strings + +### Fixed +- Case statements now support `;&` and `;;&` +- Indices in array declarations now parse correctly +- `let` expressions now parsed as arithmetic expressions +- Escaping is now respected in here documents + +### Changed +- Completely drop Makefile in favor of Cabal (thanks rodrigosetti!) + + +## v0.3.3 - 2014-05-29 +### Added +- SC2144: Warn when using globs in `[/[[` +- SC2143: Suggesting using `grep -q` over `[ "$(.. | grep)" ]` +- SC2142: Warn when referencing positional parameters in aliases +- SC2141: Warn about suspicious IFS assignments like `IFS="\n"` +- SC2140: Warn about bad embedded quotes like `echo "var="value""` +- SC2130: Warn when using `-eq` on strings +- SC2139: Warn about define time expansions in alias definitions +- SC2129: Suggest command grouping over `a >> log; b >> log; c >> log` +- SC2128: Warn when expanding arrays without an index +- SC2126: Suggest `grep -c` over `grep|wc` +- SC2123: Warn about accidentally overriding `$PATH`, e.g. `PATH=/my/dir` +- SC1083: Warn about literal `{/}` outside of quotes +- SC1082: Warn about UTF-8 BOMs + +### Fixed +- SC2051 no longer triggers for `{1,$n}`, only `{1..$n}` +- Improved detection of single quoted `sed` variables, e.g. `sed '$s///'` +- Stop warning about single quoted variables in `PS1` and similar +- Support for Zsh short form loops, `=(..)` + +### Removed +- SC1000 about unescaped lonely `$`, e.g. `grep "^foo$"` + + +## v0.3.2 - 2014-03-22 +### Added +- SC2121: Warn about trying to `set` variables, e.g. `set var = value` +- SC2120/SC2119: Warn when a function uses `$1..` if none are ever passed +- SC2117: Warn when using `su` in interactive mode, e.g. `su foo; whoami` +- SC2116: Detect useless use of echo, e.g. `for i in $(echo $var)` +- SC2115/SC2114: Detect some catastrophic `rm -r "$empty/"` mistakes +- SC1081: Warn when capitalizing keywords like `While` +- SC1077: Warn when using acute accents instead of backticks + +### Fixed +- Shells are now properly recognized in shebangs containing flags +- Stop warning about math on decimals in ksh/zsh +- Stop warning about decimal comparisons with `=`, e.g. `[ $version = 1.2 ]` +- Parsing of `|&` +- `${a[x]}` not counting as a reference of `x` +- `(( x[0] ))` not counting as a reference of `x` + + +## v0.3.1 - 2014-02-03 +### Added +- The `-s` flag to specify shell dialect +- SC2105/SC2104: Warn about `break/continue` outside loops +- SC1076: Detect invalid `[/[[` arithmetic like `[ 1 + 2 = 3 ]` +- SC1075: Suggest using `elif` over `else if` + +### Fixed +- Don't warn when comma separating elements in brace expansions +- Improved detection of single quoted `sed` variables, e.g. `sed '$d'` +- Parsing of arithmetic for loops using `{..}` instead of `do..done` +- Don't treat the last pipeline stage as a subshell in ksh/zsh + + +## v0.3.0 - 2014-01-19 +### Added +- A man page (thanks Dridi!) +- GCC compatible error reporting (`shellcheck -f gcc`) +- CheckStyle compatible XML error reporting (`shellcheck -f checkstyle`) +- Error codes for each warning, e.g. SC1234 +- Allow disabling warnings with `# shellcheck disable=SC1234` +- Allow disabling warnings with `--exclude` +- SC2103: Suggest using subshells over `cd foo; bar; cd ..` +- SC2102: Warn about duplicates in char ranges, e.g. `[10-15]` +- SC2101: Warn about named classes not inside a char range, e.g. `[:digit:]` +- SC2100/SC2099: Warn about bad math expressions like `i=i+5` +- SC2098/SC2097: Warn about `foo=bar echo $foo` +- SC2095: Warn when using `ssh`/`ffmpeg` in `while read` loops +- Better warnings for missing here doc tokens + +### Fixed +- Don't warn when single quoting variables with `ssh/perl/eval` +- `${!var}` is now counted as a variable reference + +### Removed +- Suggestions about using parameter expansion over basename +- The `jsoncheck` binary. Use `shellcheck -f json` instead. + + +## v0.2.0 - 2013-10-27 +### Added +- Suggest `./*` instead of `*` when passing globs to commands +- Suggest `pgrep` over `ps | grep` +- Warn about unicode quotes +- Warn about assigned but unused variables +- Inform about client side expansion when using `ssh` + +### Fixed +- CLI tool now uses exit codes and stderr canonically +- Parsing of extglobs containing empty patterns +- Parsing of bash style `eval foo=(bar)` +- Parsing of expansions in here documents +- Parsing of function names containing :+- +- Don't warn about `find|xargs` when using `-print0` + + +## v0.1.0 - 2013-07-23 +### Added +- First release diff --git a/LICENSE b/LICENSE --- a/LICENSE +++ b/LICENSE @@ -681,4 +681,4 @@ may consider it more useful to permit linking proprietary applications with the library. If this is what you want to do, use the GNU Lesser General Public License instead of this License. But first, please read -. +. diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -1,5 +1,6 @@ -[![Build Status](https://travis-ci.org/koalaman/shellcheck.svg?branch=master)](https://travis-ci.org/koalaman/shellcheck) +[![Build Status](https://github.com/koalaman/shellcheck/actions/workflows/build.yml/badge.svg)](https://github.com/koalaman/shellcheck/actions/workflows/build.yml) + # ShellCheck - A shell script static analysis tool ShellCheck is a GPLv3 tool that gives warnings and suggestions for bash/sh shell scripts: @@ -112,10 +113,6 @@ * [CircleCI](https://circleci.com) via the [ShellCheck Orb](https://circleci.com/orbs/registry/orb/circleci/shellcheck) * [Github](https://github.com/features/actions) (only Linux) -Services and platforms with third party plugins: - -* [SonarQube](https://www.sonarqube.org/) through [sonar-shellcheck-plugin](https://github.com/emerald-squad/sonar-shellcheck-plugin) - Most other services, including [GitLab](https://about.gitlab.com/), let you install ShellCheck yourself, either through the system's package manager (see [Installing](#installing)), or by downloading and unpacking a [binary release](#installing-a-pre-compiled-binary). @@ -143,7 +140,7 @@ On Debian based distros: - apt-get install shellcheck + sudo apt install shellcheck On Arch Linux based distros: @@ -157,8 +154,8 @@ On EPEL based distros: - yum -y install epel-release - yum install ShellCheck + sudo yum -y install epel-release + sudo yum install ShellCheck On Fedora based distros: @@ -242,6 +239,19 @@ sudo mv shellcheck.1 /usr/share/man/man1 ``` +### pre-commit + +To run ShellCheck via [pre-commit](https://pre-commit.com/), add the hook to your `.pre-commit-config.yaml`: + +``` +repos: +- repo: https://github.com/koalaman/shellcheck-precommit + rev: v0.7.2 + hooks: + - id: shellcheck +# args: ["--severity=warning"] # Optionally only show errors and warnings +``` + ### Travis CI Travis CI has now integrated ShellCheck by default, so you don't need to manually install it. @@ -348,6 +358,7 @@ echo 'Don\'t try this at home' # Attempting to escape ' in '' echo 'Path is $PATH' # Variables in single quotes trap "echo Took ${SECONDS}s" 0 # Prematurely expanded trap +unset var[i] # Array index treated as glob ``` ### Conditionals @@ -366,6 +377,7 @@ [ grep -q foo file ] # Command without $(..) [[ "$$file" == *.jpg ]] # Comparisons that can't succeed (( 1 -lt 2 )) # Using test operators in ((..)) +[ x ] & [ y ] | [ z ] # Accidental backgrounding and piping ``` ### Frequently misused commands @@ -437,6 +449,8 @@ cmd | read bar; echo $bar # Assignments in subshells cat foo | cp bar # Piping to commands that don't read printf '%s: %s\n' foo # Mismatches in printf argument count +eval "${array[@]}" # Lost word boundaries in array eval +for i in "${x[@]}"; do ${x[$i]} # Using array value as key ``` ### Robustness @@ -461,6 +475,7 @@ echo {1..$n} # Works in ksh, but not bash/dash/sh echo {1..10} # Works in ksh and bash, but not dash/sh echo -n 42 # Works in ksh, bash and dash, undefined in sh +expr match str regex # Unportable alias for `expr str : regex` trap 'exit 42' sigint # Unportable signal spec cmd &> file # Unportable redirection operator read foo < /dev/tcp/host/22 # Unportable intercepted files @@ -481,10 +496,15 @@ echo "Hello world" # Carriage return / DOS line endings echo hello \ # Trailing spaces after \ var=42 echo $var # Expansion of inlined environment -#!/bin/bash -x -e # Common shebang errors +!# bin/bash -x -e # Common shebang errors echo $((n/180*100)) # Unnecessary loss of precision ls *[:digit:].txt # Bad character class globs sed 's/foo/bar/' file > file # Redirecting to input +var2=$var2 # Variable assigned to itself +[ x$var = xval ] # Antiquated x-comparisons +ls() { ls -l "$@"; } # Infinitely recursive wrapper +alias ls='ls -l'; ls foo # Alias used before it takes effect +for x; do for x; do # Nested loop uses same variable while getopts "a" f; do case $f in "b") # Unhandled getopts flags ``` diff --git a/Setup.hs b/Setup.hs deleted file mode 100644 --- a/Setup.hs +++ /dev/null @@ -1,2 +0,0 @@ -import Distribution.Simple -main = defaultMain diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.7.2 +Version: 0.8.0 Synopsis: Shell script analysis tool License: GPL-3 License-file: LICENSE @@ -8,7 +8,7 @@ Maintainer: vidar@vidarholen.net Homepage: https://www.shellcheck.net/ Build-Type: Simple -Cabal-Version: >= 1.10 +Cabal-Version: 1.18 Bug-reports: https://github.com/koalaman/shellcheck/issues Description: The goals of ShellCheck are: @@ -22,9 +22,11 @@ * To point out subtle caveats, corner cases and pitfalls, that may cause an advanced user's otherwise working script to fail under future circumstances. +Extra-Doc-Files: + README.md + CHANGELOG.md Extra-Source-Files: -- documentation - README.md shellcheck.1.md -- A script to build the man page using pandoc manpage @@ -126,4 +128,3 @@ ShellCheck default-language: Haskell98 main-is: test/shellcheck.hs - diff --git a/shellcheck.1.md b/shellcheck.1.md --- a/shellcheck.1.md +++ b/shellcheck.1.md @@ -112,6 +112,9 @@ line (plus `/dev/null`). This option allows following any file the script may `source`. + This option may also be enabled using `external-sources=true` in + `.shellcheckrc`. This flag takes precedence. + **FILES...** : One or more script files to check, or "-" for standard input. @@ -234,11 +237,20 @@ The command can be a simple command like `echo foo`, or a compound command like a function definition, subshell block or loop. A range can be be specified with a dash, e.g. `disable=SC3000-SC4000` to exclude 3xxx. + All warnings can be disabled with `disable=all`. **enable** : Enable an optional check by name, as listed with **--list-optional**. Only file-wide `enable` directives are considered. +**external-sources** +: Set to `true` in `.shellcheckrc` to always allow ShellCheck to open + arbitrary files from 'source' statements (the way most tools do). + + This option defaults to `false` only due to ShellCheck's origin as a + remote service for checking untrusted scripts. It can safely be enabled + for normal development. + **source** : Overrides the filename included by a `source`/`.` statement. This can be used to tell shellcheck where to look for a file whose name is determined @@ -270,6 +282,9 @@ source-path=SCRIPTDIR source-path=/mnt/chroot + # Allow opening any 'source'd file, even if not specified as input + external-sources=true + # Turn on warnings for unquoted variables with safe values enable=quote-safe-variables @@ -320,20 +335,47 @@ Windows users seeing `commitBuffer: invalid argument (invalid character)` should set their terminal to use UTF-8 with `chcp 65001`. -# AUTHORS +# KNOWN INCOMPATIBILITIES -ShellCheck is developed and maintained by Vidar Holen, with assistance from a -long list of wonderful contributors. +(If nothing in this section makes sense, you are unlikely to be affected by it) +To avoid confusing and misguided suggestions, ShellCheck requires function +bodies to be either `{ brace groups; }` or `( subshells )`, and function names +containing `[]*=!` are only recognized after a `function` keyword. + +The following unconventional function definitions are identical in Bash, +but ShellCheck only recognizes the latter. + + [x!=y] () [[ $1 ]] + function [x!=y] () { [[ $1 ]]; } + +Shells without the `function` keyword do not allow these characters in function +names to begin with. Function names containing `{}` are not supported at all. + +Further, if ShellCheck sees `[x!=y]` it will assume this is an invalid +comparison. To invoke the above function, quote the command as in `'[x!=y]'`, +or to retain the same globbing behavior, use `command [x!=y]`. + +ShellCheck imposes additional restrictions on the `[` command to help diagnose +common invalid uses. While `[ $x= 1 ]` is defined in POSIX, ShellCheck will +assume it was intended as the much more likely comparison `[ "$x" = 1 ]` and +fail accordingly. For unconventional or dynamic uses of the `[` command, use +`test` or `\[` instead. + # REPORTING BUGS Bugs and issues can be reported on GitHub: https://github.com/koalaman/shellcheck/issues +# AUTHORS + +ShellCheck is developed and maintained by Vidar Holen, with assistance from a +long list of wonderful contributors. + # COPYRIGHT -Copyright 2012-2019, Vidar Holen and contributors. +Copyright 2012-2021, Vidar Holen and contributors. Licensed under the GNU General Public License version 3 or later, see https://gnu.org/licenses/gpl.html diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -234,7 +234,7 @@ process :: FilePath -> IO Status process filename = do - input <- siReadFile sys filename + input <- siReadFile sys Nothing filename either (reportFailure filename) check input where check contents = do @@ -389,6 +389,7 @@ throwError SyntaxFailure return (Prelude.read num :: Integer) +ioInterface :: Options -> [FilePath] -> IO (SystemInterface IO) ioInterface options files = do inputs <- mapM normalize files cache <- newIORef emptyCache @@ -402,14 +403,14 @@ emptyCache :: Map.Map FilePath String emptyCache = Map.empty - get cache inputs file = do + get cache inputs rcSuggestsExternal file = do map <- readIORef cache case Map.lookup file map of Just x -> return $ Right x - Nothing -> fetch cache inputs file + Nothing -> fetch cache inputs rcSuggestsExternal file - fetch cache inputs file = do - ok <- allowable inputs file + fetch cache inputs rcSuggestsExternal file = do + ok <- allowable rcSuggestsExternal inputs file if ok then (do (contents, shouldCache) <- inputFile file @@ -417,13 +418,16 @@ modifyIORef cache $ Map.insert file contents return $ Right contents ) `catch` handler - else return $ Left (file ++ " was not specified as input (see shellcheck -x).") + else + if rcSuggestsExternal == Just False + then return $ Left (file ++ " was not specified as input, and external files were disabled via directive.") + else return $ Left (file ++ " was not specified as input (see shellcheck -x).") where handler :: IOException -> IO (Either ErrorMessage String) handler ex = return . Left $ show ex - allowable inputs x = - if externalSources options + allowable rcSuggestsExternal inputs x = + if fromMaybe (externalSources options) rcSuggestsExternal then return True else do path <- normalize x @@ -497,7 +501,7 @@ b <- p x if b then pure (Just x) else acc - findSourceFile inputs sourcePathFlag currentScript sourcePathAnnotation original = + findSourceFile inputs sourcePathFlag currentScript rcSuggestsExternal sourcePathAnnotation original = if isAbsolute original then let (_, relative) = splitDrive original @@ -506,7 +510,7 @@ find original original where find filename deflt = do - sources <- findM ((allowable inputs) `andM` doesFileExist) $ + sources <- findM ((allowable rcSuggestsExternal inputs) `andM` doesFileExist) $ (adjustPath filename):(map (( filename) . adjustPath) $ sourcePathFlag ++ sourcePathAnnotation) case sources of Nothing -> return deflt diff --git a/src/ShellCheck/AST.hs b/src/ShellCheck/AST.hs --- a/src/ShellCheck/AST.hs +++ b/src/ShellCheck/AST.hs @@ -150,6 +150,7 @@ | SourceOverride String | ShellOverride String | SourcePath String + | ExternalSources Bool deriving (Show, Eq) data ConditionType = DoubleBracket | SingleBracket deriving (Show, Eq) diff --git a/src/ShellCheck/ASTLib.hs b/src/ShellCheck/ASTLib.hs --- a/src/ShellCheck/ASTLib.hs +++ b/src/ShellCheck/ASTLib.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2019 Vidar Holen + Copyright 2012-2021 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -59,11 +59,29 @@ T_NormalWord _ l -> any willSplit l _ -> False -isGlob T_Extglob {} = True -isGlob T_Glob {} = True -isGlob (T_NormalWord _ l) = any isGlob l -isGlob _ = False +isGlob t = case t of + T_Extglob {} -> True + T_Glob {} -> True + T_NormalWord _ l -> any isGlob l || hasSplitRange l + _ -> False + where + -- foo[x${var}y] gets parsed as foo,[,x,$var,y], + -- so check if there's such an interval + hasSplitRange l = + let afterBracket = dropWhile (not . isHalfOpenRange) l + in any isClosingRange afterBracket + isHalfOpenRange t = + case t of + T_Literal _ "[" -> True + _ -> False + + isClosingRange t = + case t of + T_Literal _ str -> ']' `elem` str + _ -> False + + -- Is this shell word a constant? isConstant token = case token of @@ -228,6 +246,39 @@ listToArgs = map (\x -> ("", (x, x))) + +-- Generic getOpts that doesn't rely on a format string, but may also be inaccurate. +-- This provides a best guess interpretation instead of failing when new options are added. +-- +-- "--" is treated as end of arguments +-- "--anything[=foo]" is treated as a long option without argument +-- "-any" is treated as -a -n -y, with the next arg as an option to -y unless it starts with - +-- anything else is an argument +getGenericOpts :: [Token] -> [(String, (Token, Token))] +getGenericOpts = process + where + process (token:rest) = + case getLiteralStringDef "\0" token of + "--" -> map (\c -> ("", (c,c))) rest + '-':'-':word -> (takeWhile (`notElem` "\0=") word, (token, token)) : process rest + '-':optString -> + let opts = takeWhile (/= '\0') optString + in + case rest of + next:_ | "-" `isPrefixOf` getLiteralStringDef "\0" next -> + map (\c -> ([c], (token, token))) opts ++ process rest + next:remainder -> + case reverse opts of + last:initial -> + map (\c -> ([c], (token, token))) (reverse initial) + ++ [([last], (token, next))] + ++ process remainder + [] -> process remainder + [] -> map (\c -> ([c], (token, token))) opts + _ -> ("", (token, token)) : process rest + process [] = [] + + -- Is this an expansion of multiple items of an array? isArrayExpansion (T_DollarBraced _ _ l) = let string = concat $ oversimplify l in @@ -236,14 +287,14 @@ isArrayExpansion _ = False -- Is it possible that this arg becomes multiple args? -mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t +mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f False t where - f (T_DollarBraced _ _ l) = + f quoted (T_DollarBraced _ _ l) = let string = concat $ oversimplify l in - "!" `isPrefixOf` string - f (T_DoubleQuoted _ parts) = any f parts - f (T_NormalWord _ parts) = any f parts - f _ = False + not quoted || "!" `isPrefixOf` string + f quoted (T_DoubleQuoted _ parts) = any (f True) parts + f quoted (T_NormalWord _ parts) = any (f quoted) parts + f _ _ = False -- Is it certain that this word will becomes multiple words? willBecomeMultipleArgs t = willConcatInAssignment t || f t @@ -251,7 +302,6 @@ f T_Extglob {} = True f T_Glob {} = True f T_BraceExpansion {} = True - f (T_DoubleQuoted _ parts) = any f parts f (T_NormalWord _ parts) = any f parts f _ = False diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs --- a/src/ShellCheck/Analytics.hs +++ b/src/ShellCheck/Analytics.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2019 Vidar Holen + Copyright 2012-2021 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -42,7 +42,7 @@ import Data.Maybe import Data.Ord import Data.Semigroup -import Debug.Trace +import Debug.Trace -- STRIP import qualified Data.Map.Strict as Map import Test.QuickCheck.All (forAllProperties) import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) @@ -65,6 +65,7 @@ ,checkArrayAssignmentIndices ,checkUseBeforeDefinition ,checkAliasUsedInSameParsingUnit + ,checkArrayValueUsedAsIndex ] runAnalytics :: AnalysisSpec -> [TokenComment] @@ -136,7 +137,7 @@ ,checkValidCondOps ,checkGlobbedRegex ,checkTestRedirects - ,checkIndirectExpansion + ,checkBadParameterSubstitution ,checkPS1Assignments ,checkBackticks ,checkInexplicablyUnquoted @@ -169,7 +170,6 @@ ,checkTestArgumentSplitting ,checkConcatenatedDollarAt ,checkTildeInPath - ,checkMaskedReturns ,checkReadWithoutR ,checkLoopVariableReassignment ,checkTrailingBracket @@ -197,6 +197,8 @@ ,checkEqualsInCommand ,checkSecondArgIsComparison ,checkComparisonWithLeadingX + ,checkCommandWithTrailingSymbol + ,checkUnquotedParameterExpansionPattern ] optionalChecks = map fst optionalTreeChecks @@ -244,6 +246,27 @@ cdPositive = "echo $VAR", cdNegative = "VAR=hello; echo $VAR" }, checkUnassignedReferences' True) + + ,(newCheckDescription { + cdName = "require-double-brackets", + cdDescription = "Require [[ and warn about [ in Bash/Ksh", + cdPositive = "[ -e /etc/issue ]", + cdNegative = "[[ -e /etc/issue ]]" + }, checkRequireDoubleBracket) + + ,(newCheckDescription { + cdName = "check-set-e-suppressed", + cdDescription = "Notify when set -e is suppressed during function invocation", + cdPositive = "set -e; func() { cp *.txt ~/backup; rm *.txt; }; func && echo ok", + cdNegative = "set -e; func() { cp *.txt ~/backup; rm *.txt; }; func; echo ok" + }, checkSetESuppressed) + + ,(newCheckDescription { + cdName = "check-extra-masked-returns", + cdDescription = "Check for additional cases where exit codes are masked", + cdPositive = "rm -r \"$(get_chroot_dir)/home\"", + cdNegative = "set -e; dir=\"$(get_chroot_dir)\"; rm -r \"$dir/home\"" + }, checkExtraMaskedReturns) ] optionalCheckMap :: Map.Map String (Parameters -> Token -> [TokenComment]) @@ -381,9 +404,27 @@ repInsertionPoint = InsertBefore } -surroundWidth id params s = fixWith [replaceStart id params 0 s, replaceEnd id params 0 s] +surroundWith id params s = fixWith [replaceStart id params 0 s, replaceEnd id params 0 s] fixWith fixes = newFix { fixReplacements = fixes } +analyse f t = execState (doAnalysis f t) [] + +-- Make a map from functions to definition IDs +functions t = Map.fromList $ analyse findFunctions t +findFunctions (T_Function id _ _ name _) + = modify ((name, id):) +findFunctions _ = return () + +-- Make a map from aliases to definition IDs +aliases t = Map.fromList $ analyse findAliases t +findAliases t@(T_SimpleCommand _ _ (_:args)) + | t `isUnqualifiedCommand` "alias" = mapM_ getAlias args +findAliases _ = return () +getAlias arg = + let string = onlyLiteralString arg + in when ('=' `elem` string) $ + modify ((takeWhile (/= '=') string, getId arg):) + prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" checkEchoWc _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ @@ -472,7 +513,9 @@ prop_checkUuoc1 = verify checkUuoc "cat foo | grep bar" prop_checkUuoc2 = verifyNot checkUuoc "cat * | grep bar" -prop_checkUuoc3 = verify checkUuoc "cat $var | grep bar" +prop_checkUuoc3 = verify checkUuoc "cat \"$var\" | grep bar" +prop_checkUuoc3b = verifyNot checkUuoc "cat $var | grep bar" +prop_checkUuoc3c = verifyNot checkUuoc "cat \"${!var}\" | grep bar" prop_checkUuoc4 = verifyNot checkUuoc "cat $var" prop_checkUuoc5 = verifyNot checkUuoc "cat \"$@\"" prop_checkUuoc6 = verifyNot checkUuoc "cat -n | grep bar" @@ -499,6 +542,8 @@ prop_checkPipePitfalls14 = verifyNot checkPipePitfalls "foo | grep -o bar | wc -cmwL" prop_checkPipePitfalls15 = verifyNot checkPipePitfalls "foo | grep bar | wc -cmwL" prop_checkPipePitfalls16 = verifyNot checkPipePitfalls "foo | grep -r bar | wc -l" +prop_checkPipePitfalls17 = verifyNot checkPipePitfalls "foo | grep -l bar | wc -l" +prop_checkPipePitfalls18 = verifyNot checkPipePitfalls "foo | grep -L bar | wc -l" checkPipePitfalls _ (T_Pipeline id _ commands) = do for ["find", "xargs"] $ \(find:xargs:_) -> @@ -520,7 +565,9 @@ let flagsGrep = maybe [] (map snd . getAllFlags) $ getCommand grep flagsWc = maybe [] (map snd . getAllFlags) $ getCommand wc in - unless (any (`elem` ["o", "only-matching", "r", "R", "recursive"]) flagsGrep || any (`elem` ["m", "chars", "w", "words", "c", "bytes", "L", "max-line-length"]) flagsWc || null flagsWc) $ + unless (any (`elem` ["l", "files-with-matches", "L", "files-without-matches", "o", "only-matching", "r", "R", "recursive"]) flagsGrep + || any (`elem` ["m", "chars", "w", "words", "c", "bytes", "L", "max-line-length"]) flagsWc + || null flagsWc) $ style (getId grep) 2126 "Consider using grep -c instead of grep|wc -l." didLs <- fmap or . sequence $ [ @@ -621,7 +668,7 @@ prop_checkForInQuoted8 = verify checkForInQuoted "for f in 'ls', 'grep', 'mv'; do true; done" prop_checkForInQuoted9 = verifyNot checkForInQuoted "for f in 'ls,' 'grep,' 'mv'; do true; done" checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [word@(T_DoubleQuoted id list)]] _) - | any (\x -> willSplit x && not (mayBecomeMultipleArgs x)) list + | any willSplit list && not (mayBecomeMultipleArgs word) || maybe False wouldHaveBeenGlob (getLiteralString word) = err id 2066 "Since you double quoted this, it will not word split, and the loop will only run once." checkForInQuoted _ (T_ForIn _ f [T_NormalWord _ [T_SingleQuoted id _]] _) = @@ -728,6 +775,7 @@ prop_checkUnquotedExpansions7 = verifyNot checkUnquotedExpansions "cat << foo\n$(ls)\nfoo" prop_checkUnquotedExpansions8 = verifyNot checkUnquotedExpansions "set -- $(seq 1 4)" prop_checkUnquotedExpansions9 = verifyNot checkUnquotedExpansions "echo foo `# inline comment`" +prop_checkUnquotedExpansions10 = verify checkUnquotedExpansions "#!/bin/sh\nexport var=$(val)" checkUnquotedExpansions params = check where @@ -737,7 +785,7 @@ check _ = return () tree = parentMap params examine t contents = - unless (null contents || shouldBeSplit t || isQuoteFree tree t || usedAsCommandName tree t) $ + unless (null contents || shouldBeSplit t || isQuoteFree (shellType params) tree t || usedAsCommandName tree t) $ warn (getId t) 2046 "Quote this to prevent word splitting." shouldBeSplit t = @@ -752,6 +800,7 @@ prop_checkRedirectToSame6 = verifyNot checkRedirectToSame "echo foo > foo" prop_checkRedirectToSame7 = verifyNot checkRedirectToSame "sed 's/foo/bar/g' file | sponge file" prop_checkRedirectToSame8 = verifyNot checkRedirectToSame "while read -r line; do _=\"$fname\"; done <\"$fname\"" +prop_checkRedirectToSame9 = verifyNot checkRedirectToSame "while read -r line; do cat < \"$fname\"; done <\"$fname\"" checkRedirectToSame params s@(T_Pipeline _ _ list) = mapM_ (\l -> (mapM_ (\x -> doAnalysis (checkOccurrences x) l) (getAllRedirs list))) list where @@ -760,6 +809,7 @@ checkOccurrences t@(T_NormalWord exceptId x) u@(T_NormalWord newId y) | exceptId /= newId && x == y + && not (isInput t && isInput u) && not (isOutput t && isOutput u) && not (special t) && not (any isHarmlessCommand [t,u]) @@ -778,6 +828,13 @@ _ -> [] getRedirs _ = [] special x = "/dev/" `isPrefixOf` concat (oversimplify x) + isInput t = + case drop 1 $ getPath (parentMap params) t of + T_IoFile _ op _:_ -> + case op of + T_Less _ -> True + _ -> False + _ -> False isOutput t = case drop 1 $ getPath (parentMap params) t of T_IoFile _ op _:_ -> @@ -820,10 +877,21 @@ prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done" prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*" prop_checkDollarStar3 = verifyNot checkDollarStar "[[ $* = 'a b' ]]" +prop_checkDollarStar4 = verify checkDollarStar "for f in ${var[*]}; do ..; done" +prop_checkDollarStar5 = verify checkDollarStar "ls ${*//foo/bar}" +prop_checkDollarStar6 = verify checkDollarStar "ls ${var[*]%%.*}" +prop_checkDollarStar7 = verify checkDollarStar "ls ${*}" +prop_checkDollarStar8 = verifyNot checkDollarStar "ls ${#*}" +prop_checkDollarStar9 = verify checkDollarStar "ls ${arr[*]}" +prop_checkDollarStar10 = verifyNot checkDollarStar "ls ${#arr[*]}" checkDollarStar p t@(T_NormalWord _ [T_DollarBraced id _ l]) - | concat (oversimplify l) == "*" && - not (isStrictlyQuoteFree (parentMap p) t) = + | not (isStrictlyQuoteFree (shellType p) (parentMap p) t) = do + let str = concat (oversimplify l) + when ("*" `isPrefixOf` str) $ warn id 2048 "Use \"$@\" (with quotes) to prevent whitespace problems." + when ("[*]" `isPrefixOf` (getBracedModifier str) && isVariableChar (headOrDefault '!' str)) $ + warn id 2048 "Use \"${array[@]}\" (with quotes) to prevent whitespace problems." + checkDollarStar _ _ = return () @@ -838,7 +906,7 @@ prop_checkUnquotedDollarAt8 = verifyNot checkUnquotedDollarAt "echo \"${args[@]:+${args[@]}}\"" prop_checkUnquotedDollarAt9 = verifyNot checkUnquotedDollarAt "echo ${args[@]:+\"${args[@]}\"}" prop_checkUnquotedDollarAt10 = verifyNot checkUnquotedDollarAt "echo ${@+\"$@\"}" -checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not $ isStrictlyQuoteFree (parentMap p) word = +checkUnquotedDollarAt p word@(T_NormalWord _ parts) | not $ isStrictlyQuoteFree (shellType p) (parentMap p) word = forM_ (find isArrayExpansion parts) $ \x -> unless (isQuotedAlternativeReference x) $ err (getId x) 2068 @@ -851,7 +919,7 @@ prop_checkConcatenatedDollarAt4 = verifyNot checkConcatenatedDollarAt "echo $@" prop_checkConcatenatedDollarAt5 = verifyNot checkConcatenatedDollarAt "echo \"${arr[@]}\"" checkConcatenatedDollarAt p word@T_NormalWord {} - | not $ isQuoteFree (parentMap p) word + | not $ isQuoteFree (shellType p) (parentMap p) word || null (drop 1 parts) = mapM_ for array where @@ -952,8 +1020,8 @@ checkStderrRedirect _ _ = return () -lt x = trace ("Tracing " ++ show x) x -ltt t = trace ("Tracing " ++ show t) +lt x = trace ("Tracing " ++ show x) x -- STRIP +ltt t = trace ("Tracing " ++ show t) -- STRIP prop_checkSingleQuotedVariables = verify checkSingleQuotedVariables "echo '$foo'" @@ -1019,6 +1087,7 @@ ,"dpkg-query" ,"jq" -- could also check that user provides --arg ,"rename" + ,"rg" ,"unset" ,"git filter-branch" ,"mumps -run %XCMD" @@ -1087,6 +1156,12 @@ prop_checkNumberComparisons14 = verifyNot checkNumberComparisons "[[ foo < bar ]]" prop_checkNumberComparisons15 = verifyNot checkNumberComparisons "[ $foo '>' $bar ]" prop_checkNumberComparisons16 = verify checkNumberComparisons "[ foo -eq 'y' ]" +prop_checkNumberComparisons17 = verify checkNumberComparisons "[[ 'foo' -eq 2 ]]" +prop_checkNumberComparisons18 = verify checkNumberComparisons "[[ foo -eq 2 ]]" +prop_checkNumberComparisons19 = verifyNot checkNumberComparisons "foo=1; [[ foo -eq 2 ]]" +prop_checkNumberComparisons20 = verify checkNumberComparisons "[[ 2 -eq / ]]" +prop_checkNumberComparisons21 = verify checkNumberComparisons "[[ foo -eq foo ]]" + checkNumberComparisons params (TC_Binary id typ op lhs rhs) = do if isNum lhs || isNum rhs then do @@ -1112,9 +1187,9 @@ when (op `elem` arithmeticBinaryTestOps) $ do mapM_ checkDecimals [lhs, rhs] - when (typ == SingleBracket) $ - checkStrings [lhs, rhs] + mapM_ checkString [lhs, rhs] + where hasStringComparison = shellType params /= Sh isLtGt = flip elem ["<", "\\<", ">", "\\>"] @@ -1126,19 +1201,45 @@ decimalError = "Decimals are not supported. " ++ "Either use integers only, or use bc or awk to compare." - checkStrings = - mapM_ stringError . find isNonNum + checkString t = + let + asString = getLiteralStringDef "\0" t + isVar = isVariableName asString + kind = if isVar then "a variable" else "an arithmetic expression" + fix = if isVar then "$var" else "$((expr))" + in + when (isNonNum t) $ + if typ == SingleBracket + then + err (getId t) 2170 $ + "Invalid number for " ++ op ++ ". Use " ++ seqv op ++ + " to compare as string (or use " ++ fix ++ + " to expand as " ++ kind ++ ")." + else + -- We should warn if any of the following holds: + -- The string is not a variable name + -- Any part of it is quoted + -- It's not a recognized variable name + when (not isVar || any isQuotes (getWordParts t) || asString `notElem` assignedVariables) $ + warn (getId t) 2309 $ + op ++ " treats this as " ++ kind ++ ". " ++ + "Use " ++ seqv op ++ " to compare as string (or expand explicitly with " ++ fix ++ ")." + assignedVariables :: [String] + assignedVariables = mapMaybe f (variableFlow params) + where + f t = do + Assignment (_, _, name, _) <- return t + return name + isNonNum t = not . all numChar $ onlyLiteralString t numChar x = isDigit x || x `elem` "+-. " - stringError t = err (getId t) 2170 $ - "Numerical " ++ op ++ " does not dereference in [..]. Expand or use string operator." - isNum t = case oversimplify t of [v] -> all isDigit v _ -> False + isFraction t = case oversimplify t of [v] -> isJust $ matchRegex floatRegex v @@ -1584,34 +1685,84 @@ prop_checkBackticks3 = verifyNot checkBackticks "echo `#inlined comment` foo" checkBackticks params (T_Backticked id list) | not (null list) = addComment $ - makeCommentWithFix StyleC id 2006 "Use $(...) notation instead of legacy backticked `...`." + makeCommentWithFix StyleC id 2006 "Use $(...) notation instead of legacy backticks `...`." (fixWith [replaceStart id params 1 "$(", replaceEnd id params 1 ")"]) checkBackticks _ _ = return () -prop_checkIndirectExpansion1 = verify checkIndirectExpansion "${foo$n}" -prop_checkIndirectExpansion2 = verifyNot checkIndirectExpansion "${foo//$n/lol}" -prop_checkIndirectExpansion3 = verify checkIndirectExpansion "${$#}" -prop_checkIndirectExpansion4 = verify checkIndirectExpansion "${var${n}_$((i%2))}" -prop_checkIndirectExpansion5 = verifyNot checkIndirectExpansion "${bar}" -checkIndirectExpansion _ (T_DollarBraced i _ (T_NormalWord _ contents)) - | isIndirection contents = - err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval." + +prop_checkBadParameterSubstitution1 = verify checkBadParameterSubstitution "${foo$n}" +prop_checkBadParameterSubstitution2 = verifyNot checkBadParameterSubstitution "${foo//$n/lol}" +prop_checkBadParameterSubstitution3 = verify checkBadParameterSubstitution "${$#}" +prop_checkBadParameterSubstitution4 = verify checkBadParameterSubstitution "${var${n}_$((i%2))}" +prop_checkBadParameterSubstitution5 = verifyNot checkBadParameterSubstitution "${bar}" +prop_checkBadParameterSubstitution6 = verify checkBadParameterSubstitution "${\"bar\"}" +prop_checkBadParameterSubstitution7 = verify checkBadParameterSubstitution "${{var}" +prop_checkBadParameterSubstitution8 = verify checkBadParameterSubstitution "${$(x)//x/y}" +prop_checkBadParameterSubstitution9 = verifyNot checkBadParameterSubstitution "$# ${#} $! ${!} ${!#} ${#!}" +prop_checkBadParameterSubstitution10 = verify checkBadParameterSubstitution "${'foo'}" +prop_checkBadParameterSubstitution11 = verify checkBadParameterSubstitution "${${x%.*}##*/}" + +checkBadParameterSubstitution _ t = + case t of + (T_DollarBraced i _ (T_NormalWord _ contents@(first:_))) -> + if isIndirection contents + then err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval." + else checkFirst first + _ -> return () + where + isIndirection vars = let list = mapMaybe isIndirectionPart vars in not (null list) && and list + isIndirectionPart t = - case t of T_DollarExpansion _ _ -> Just True - T_Backticked _ _ -> Just True - T_DollarBraced _ _ _ -> Just True - T_DollarArithmetic _ _ -> Just True + case t of T_DollarExpansion {} -> Just True + T_Backticked {} -> Just True + T_DollarBraced {} -> Just True + T_DollarArithmetic {} -> Just True T_Literal _ s -> if all isVariableChar s then Nothing else Just False _ -> Just False -checkIndirectExpansion _ _ = return () + checkFirst t = + case t of + T_Literal id (c:_) -> + if isVariableChar c || isSpecialVariableChar c + then return () + else err id 2296 $ "Parameter expansions can't start with " ++ e4m [c] ++ ". Double check syntax." + T_ParamSubSpecialChar {} -> return () + + T_DoubleQuoted id [T_Literal _ s] | isVariable s -> + err id 2297 "Double quotes must be outside ${}: ${\"invalid\"} vs \"${valid}\"." + + T_DollarBraced id braces _ | isUnmodifiedParameterExpansion t -> + err id 2298 $ + (if braces then "${${x}}" else "${$x}") + ++ " is invalid. For expansion, use ${x}. For indirection, use arrays, ${!x} or (for sh) eval." + + T_DollarBraced {} -> + err (getId t) 2299 "Parameter expansions can't be nested. Use temporary variables." + + _ | isCommandSubstitution t -> + err (getId t) 2300 "Parameter expansion can't be applied to command substitutions. Use temporary variables." + + _ -> err (getId t) 2301 $ "Parameter expansion starts with unexpected " ++ name t ++ ". Double check syntax." + + isVariable str = + case str of + [c] -> isVariableStartChar c || isSpecialVariableChar c || isDigit c + x -> isVariableName x + + name t = + case t of + T_SingleQuoted {} -> "quotes" + T_DoubleQuoted {} -> "quotes" + _ -> "syntax" + + prop_checkInexplicablyUnquoted1 = verify checkInexplicablyUnquoted "echo 'var='value';'" prop_checkInexplicablyUnquoted2 = verifyNot checkInexplicablyUnquoted "'foo'*" prop_checkInexplicablyUnquoted3 = verifyNot checkInexplicablyUnquoted "wget --user-agent='something'" @@ -1621,6 +1772,9 @@ prop_checkInexplicablyUnquoted7 = verifyNot checkInexplicablyUnquoted "${dir/\"foo\"/\"bar\"}" prop_checkInexplicablyUnquoted8 = verifyNot checkInexplicablyUnquoted " 'foo'\\\n 'bar'" prop_checkInexplicablyUnquoted9 = verifyNot checkInexplicablyUnquoted "[[ $x =~ \"foo\"(\"bar\"|\"baz\") ]]" +prop_checkInexplicablyUnquoted10 = verifyNot checkInexplicablyUnquoted "cmd ${x+--name=\"$x\" --output=\"$x.out\"}" +prop_checkInexplicablyUnquoted11 = verifyNot checkInexplicablyUnquoted "echo \"foo\"/\"bar\"" +prop_checkInexplicablyUnquoted12 = verifyNot checkInexplicablyUnquoted "declare \"foo\"=\"bar\"" checkInexplicablyUnquoted params (T_NormalWord id tokens) = mapM_ check (tails tokens) where check (T_SingleQuoted _ _:T_Literal id str:_) @@ -1632,19 +1786,23 @@ T_DollarExpansion id _ -> warnAboutExpansion id T_DollarBraced id _ _ -> warnAboutExpansion id T_Literal id s - | not (quotesSingleThing a && quotesSingleThing b || isRegex (getPath (parentMap params) trapped)) -> + | not (quotesSingleThing a && quotesSingleThing b + || s `elem` ["=", ":", "/"] + || isSpecial (getPath (parentMap params) trapped) + ) -> warnAboutLiteral id _ -> return () check _ = return () -- Regexes for [[ .. =~ re ]] are parsed with metacharacters like ()| as unquoted - -- literals, so avoid overtriggering on these. - isRegex t = + -- literals. The same is true for ${x+"foo" "bar"}. Avoid overtriggering on these. + isSpecial t = case t of (T_Redirecting {} : _) -> False + T_DollarBraced {} : _ -> True (a:(TC_Binary _ _ "=~" lhs rhs):rest) -> getId a == getId rhs - _:rest -> isRegex rest + _:rest -> isSpecial rest _ -> False -- If the surrounding quotes quote single things, like "$foo"_and_then_some_"$stuff", @@ -1693,6 +1851,7 @@ prop_checkSpuriousExec7 = verifyNot checkSpuriousExec "exec file; echo failed; exit 3" prop_checkSpuriousExec8 = verifyNot checkSpuriousExec "exec {origout}>&1- >tmp.log 2>&1; bar" prop_checkSpuriousExec9 = verify checkSpuriousExec "for file in rc.d/*; do exec \"$file\"; done" +prop_checkSpuriousExec10 = verifyNot checkSpuriousExec "exec file; r=$?; printf >&2 'failed\n'; return $r" checkSpuriousExec _ = doLists where doLists (T_Script _ _ cmds) = doList cmds False @@ -1708,7 +1867,8 @@ stripCleanup = reverse . dropWhile cleanup . reverse cleanup (T_Pipeline _ _ [cmd]) = - isCommandMatch cmd (`elem` ["echo", "exit"]) + isCommandMatch cmd (`elem` ["echo", "exit", "printf", "return"]) + || isAssignment cmd cleanup _ = False doList = doList' . stripCleanup @@ -1786,6 +1946,9 @@ prop_subshellAssignmentCheck18 = verifyTree subshellAssignmentCheck "( exec {n}>&2; ); echo $n" prop_subshellAssignmentCheck19 = verifyNotTree subshellAssignmentCheck "#!/bin/bash\nshopt -s lastpipe; echo a | read -r b; echo \"$b\"" prop_subshellAssignmentCheck20 = verifyTree subshellAssignmentCheck "@test 'foo' { a=1; }\n@test 'bar' { echo $a; }\n" +prop_subshellAssignmentCheck21 = verifyNotTree subshellAssignmentCheck "test1() { echo foo | if [[ $var ]]; then echo $var; fi; }; test2() { echo $var; }" +prop_subshellAssignmentCheck22 = verifyNotTree subshellAssignmentCheck "( [[ -n $foo || -z $bar ]] ); echo $foo $bar" +prop_subshellAssignmentCheck23 = verifyNotTree subshellAssignmentCheck "( export foo ); echo $foo" subshellAssignmentCheck params t = let flow = variableFlow params check = findSubshelled flow [("oops",[])] Map.empty @@ -1793,8 +1956,11 @@ findSubshelled [] _ _ = return () -findSubshelled (Assignment x@(_, _, str, _):rest) ((reason,scope):lol) deadVars = - findSubshelled rest ((reason, x:scope):lol) $ Map.insert str Alive deadVars +findSubshelled (Assignment x@(_, _, str, data_):rest) scopes@((reason,scope):restscope) deadVars = + if isTrueAssignmentSource data_ + then findSubshelled rest ((reason, x:scope):restscope) $ Map.insert str Alive deadVars + else findSubshelled rest scopes deadVars + findSubshelled (Reference (_, readToken, str):rest) scopes deadVars = do unless (shouldIgnore str) $ case Map.findWithDefault Alive str deadVars of Alive -> return () @@ -1880,6 +2046,9 @@ prop_checkSpacefulness41= verifyNotTree checkSpacefulness "exec $1 --flags" prop_checkSpacefulness42= verifyNotTree checkSpacefulness "run $1 --flags" prop_checkSpacefulness43= verifyNotTree checkSpacefulness "$foo=42" +prop_checkSpacefulness44= verifyTree checkSpacefulness "#!/bin/sh\nexport var=$value" +prop_checkSpacefulness45= verifyNotTree checkSpacefulness "wait -zzx -p foo; echo $foo" +prop_checkSpacefulness46= verifyNotTree checkSpacefulness "x=0; (( x += 1 )); echo $x" data SpaceStatus = SpaceSome | SpaceNone | SpaceEmpty deriving (Eq) instance Semigroup SpaceStatus where @@ -1941,7 +2110,7 @@ (getId t) == (getId me) && (parentId == getId cmd) _ -> False -addDoubleQuotesAround params token = (surroundWidth (getId token) params "\"") +addDoubleQuotesAround params token = (surroundWith (getId token) params "\"") checkSpacefulness' :: (SpaceStatus -> Token -> String -> Writer [TokenComment] ()) -> Parameters -> Token -> [TokenComment] @@ -1961,7 +2130,7 @@ isExpansion token && not (isArrayExpansion token) -- There's another warning for this && not (isCountingReference token) - && not (isQuoteFree parents token) + && not (isQuoteFree (shellType params) parents token) && not (isQuotedAlternativeReference token) && not (usedAsCommandName parents token) @@ -1970,6 +2139,7 @@ where emit x = tell [x] + writeF _ (TA_Assignment {}) name _ = setSpaces name SpaceNone >> return [] writeF _ _ name (DataString SourceExternal) = setSpaces name SpaceSome >> return [] writeF _ _ name (DataString SourceInteger) = setSpaces name SpaceNone >> return [] @@ -2079,7 +2249,7 @@ return $ case assignment of Just j | not (isParamTo parents "eval" expr) - && not (isQuoteFree parents expr) + && not (isQuoteFree (shellType params) parents expr) && not (squashesQuotes expr) -> [ makeComment WarningC j 2089 $ @@ -2150,21 +2320,11 @@ findExecFlags = ["-exec", "-execdir", "-ok"] dropFlags = dropWhile (\x -> "-" `isPrefixOf` fst x) - -- Make a map from functions/aliases to definition IDs - analyse f t = execState (doAnalysis f t) [] - functions = Map.fromList $ analyse findFunctions t - findFunctions (T_Function id _ _ name _) = modify ((name, id):) - findFunctions t@(T_SimpleCommand id _ (_:args)) - | t `isUnqualifiedCommand` "alias" = mapM_ getAlias args - findFunctions _ = return () - getAlias arg = - let string = onlyLiteralString arg - in when ('=' `elem` string) $ - modify ((takeWhile (/= '=') string, getId arg):) + functionsAndAliases = Map.union (functions t) (aliases t) checkArg cmd (_, arg) = sequence_ $ do literalArg <- getUnquotedLiteral arg -- only consider unquoted literals - definitionId <- Map.lookup literalArg functions + definitionId <- Map.lookup literalArg functionsAndAliases return $ do warn (getId arg) 2033 "Shell functions can't be passed to external commands." @@ -2220,6 +2380,10 @@ prop_checkUnused45= verifyTree checkUnusedAssignments "readonly foo=bar" prop_checkUnused46= verifyTree checkUnusedAssignments "readonly foo=(bar)" prop_checkUnused47= verifyNotTree checkUnusedAssignments "a=1; alias hello='echo $a'" +prop_checkUnused48= verifyNotTree checkUnusedAssignments "_a=1" +prop_checkUnused49= verifyNotTree checkUnusedAssignments "declare -A array; key=a; [[ -v array[$key] ]]" +prop_checkUnused50= verifyNotTree checkUnusedAssignments "foofunc() { :; }; typeset -fx foofunc" + checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) where flow = variableFlow params @@ -2236,8 +2400,9 @@ unused = Map.assocs $ Map.difference assignments references warnFor (name, token) = - warn (getId token) 2034 $ - name ++ " appears unused. Verify use (or export if used externally)." + unless ("_" `isPrefixOf` name) $ + warn (getId token) 2034 $ + name ++ " appears unused. Verify use (or export if used externally)." stripSuffix = takeWhile isVariableChar defaultMap = Map.fromList $ zip internalVariables $ repeat () @@ -2291,6 +2456,9 @@ prop_checkUnassignedReferences_minusZBraced = verifyNotTree checkUnassignedReferences "if [ -z \"${x}\" ]; then echo \"\"; fi" prop_checkUnassignedReferences_minusNDefault = verifyNotTree checkUnassignedReferences "if [ -n \"${x:-}\" ]; then echo $x; fi" prop_checkUnassignedReferences_minusZDefault = verifyNotTree checkUnassignedReferences "if [ -z \"${x:-}\" ]; then echo \"\"; fi" +prop_checkUnassignedReferences50 = verifyNotTree checkUnassignedReferences "echo ${foo:+bar}" +prop_checkUnassignedReferences51 = verifyNotTree checkUnassignedReferences "echo ${foo:+$foo}" +prop_checkUnassignedReferences52 = verifyNotTree checkUnassignedReferences "wait -p pid; echo $pid" checkUnassignedReferences = checkUnassignedReferences' False checkUnassignedReferences' includeGlobals params t = warnings @@ -2323,8 +2491,8 @@ warningForGlobals var place = do match <- getBestMatch var - return $ warn (getId place) 2153 $ - "Possible misspelling: " ++ var ++ " may not be assigned, but " ++ match ++ " is." + return $ info (getId place) 2153 $ + "Possible misspelling: " ++ var ++ " may not be assigned. Did you mean " ++ match ++ "?" warningForLocals var place = return $ warn (getId place) 2154 $ @@ -2339,7 +2507,7 @@ warningFor (var, place) = do guard $ isVariableName var - guard . not $ isInArray var place || isGuarded place + guard . not $ isException var place || isGuarded place (if includeGlobals || isLocal var then warningForLocals else warningForGlobals) var place @@ -2348,11 +2516,22 @@ -- Due to parsing, foo=( [bar]=baz ) parses 'bar' as a reference even for assoc arrays. -- Similarly, ${foo[bar baz]} may not be referencing bar/baz. Just skip these. - isInArray var t = any isArray $ getPath (parentMap params) t + -- We can also have ${foo:+$foo} should be treated like [[ -n $foo ]] && echo $foo + isException var t = any shouldExclude $ getPath (parentMap params) t where - isArray T_Array {} = True - isArray (T_DollarBraced _ _ l) | var /= getBracedReference (concat $ oversimplify l) = True - isArray _ = False + shouldExclude t = + case t of + T_Array {} -> True + (T_DollarBraced _ _ l) -> + let str = concat $ oversimplify l + ref = getBracedReference str + mod = getBracedModifier str + in + -- Either we're used as an array index like ${arr[here]} + ref /= var || + -- or the reference is guarded by a parent, ${here:+foo$here} + "+" `isPrefixOf` mod || ":+" `isPrefixOf` mod + _ -> False isGuarded (T_DollarBraced _ _ v) = rest `matches` guardRegex @@ -2505,10 +2684,13 @@ prop_checkCharRangeGlob2 = verifyNot checkCharRangeGlob "ls *[[:digit:]].jpg" prop_checkCharRangeGlob3 = verify checkCharRangeGlob "ls [10-15]" prop_checkCharRangeGlob4 = verifyNot checkCharRangeGlob "ls [a-zA-Z]" -prop_checkCharRangeGlob5 = verifyNot checkCharRangeGlob "tr -d [a-zA-Z]" -- tr has 2060 +prop_checkCharRangeGlob5 = verifyNot checkCharRangeGlob "tr -d [aa]" -- tr has 2060 prop_checkCharRangeGlob6 = verifyNot checkCharRangeGlob "[[ $x == [!!]* ]]" +prop_checkCharRangeGlob7 = verifyNot checkCharRangeGlob "[[ -v arr[keykey] ]]" +prop_checkCharRangeGlob8 = verifyNot checkCharRangeGlob "[[ arr[keykey] -gt 1 ]]" +prop_checkCharRangeGlob9 = verifyNot checkCharRangeGlob "read arr[keykey]" -- tr has 2313 checkCharRangeGlob p t@(T_Glob id str) | - isCharClass str && not (isParamTo (parentMap p) "tr" t) = + isCharClass str && not isIgnoredCommand && not (isDereferenced t) = if ":" `isPrefixOf` contents && ":" `isSuffixOf` contents && contents /= ":" @@ -2525,6 +2707,19 @@ '!':rest -> rest '^':rest -> rest x -> x + + isIgnoredCommand = fromMaybe False $ do + cmd <- getClosestCommand (parentMap p) t + return $ isCommandMatch cmd (`elem` ["tr", "read"]) + + -- Check if this is a dereferencing context like [[ -v array[operandhere] ]] + isDereferenced = fromMaybe False . msum . map isDereferencingOp . getPath (parentMap p) + isDereferencingOp t = + case t of + TC_Binary _ DoubleBracket str _ _ -> return $ isDereferencingBinaryOp str + TC_Unary _ _ str _ -> return $ str == "-v" + T_SimpleCommand {} -> return False + _ -> Nothing checkCharRangeGlob _ _ = return () @@ -2947,31 +3142,6 @@ err (getId token) 2255 "[ ] does not apply arithmetic evaluation. Evaluate with $((..)) for numbers, or use string comparator for strings." -prop_checkMaskedReturns1 = verify checkMaskedReturns "f() { local a=$(false); }" -prop_checkMaskedReturns2 = verify checkMaskedReturns "declare a=$(false)" -prop_checkMaskedReturns3 = verify checkMaskedReturns "declare a=\"`false`\"" -prop_checkMaskedReturns4 = verify checkMaskedReturns "readonly a=$(false)" -prop_checkMaskedReturns5 = verify checkMaskedReturns "readonly a=\"`false`\"" -prop_checkMaskedReturns6 = verifyNot checkMaskedReturns "declare a; a=$(false)" -prop_checkMaskedReturns7 = verifyNot checkMaskedReturns "f() { local -r a=$(false); }" -prop_checkMaskedReturns8 = verifyNot checkMaskedReturns "a=$(false); readonly a" -checkMaskedReturns _ t@(T_SimpleCommand id _ (cmd:rest)) = sequence_ $ do - name <- getCommandName t - guard $ name `elem` ["declare", "export", "readonly"] - || name == "local" && "r" `notElem` map snd (getAllFlags t) - return $ mapM_ checkArgs rest - where - checkArgs (T_Assignment id _ _ _ word) | any hasReturn $ getWordParts word = - warn id 2155 "Declare and assign separately to avoid masking return values." - checkArgs _ = return () - - hasReturn t = case t of - T_Backticked {} -> True - T_DollarExpansion {} -> True - _ -> False -checkMaskedReturns _ _ = return () - - prop_checkReadWithoutR1 = verify checkReadWithoutR "read -a foo" prop_checkReadWithoutR2 = verifyNot checkReadWithoutR "read -ar foo" prop_checkReadWithoutR3 = verifyNot checkReadWithoutR "read -t 0" @@ -3046,6 +3216,7 @@ prop_checkLoopVariableReassignment1 = verify checkLoopVariableReassignment "for i in *; do for i in *.bar; do true; done; done" prop_checkLoopVariableReassignment2 = verify checkLoopVariableReassignment "for i in *; do for((i=0; i<3; i++)); do true; done; done" prop_checkLoopVariableReassignment3 = verifyNot checkLoopVariableReassignment "for i in *; do for j in *.bar; do true; done; done" +prop_checkLoopVariableReassignment4 = verifyNot checkLoopVariableReassignment "for _ in *; do for _ in *.bar; do true; done; done" checkLoopVariableReassignment params token = sequence_ $ case token of T_ForIn {} -> check @@ -3054,6 +3225,7 @@ where check = do str <- loopVariable token + guard $ str /= "_" next <- find (\x -> loopVariable x == Just str) path return $ do warn (getId token) 2165 "This nested loop overrides the index variable of its parent." @@ -3104,27 +3276,85 @@ prop_checkReturnAgainstZero7 = verify checkReturnAgainstZero "(( $? == 0 ))" prop_checkReturnAgainstZero8 = verify checkReturnAgainstZero "(( $? ))" prop_checkReturnAgainstZero9 = verify checkReturnAgainstZero "(( ! $? ))" -checkReturnAgainstZero _ token = +prop_checkReturnAgainstZero10 = verifyNot checkReturnAgainstZero "x=$(( $? > 0 ))" +prop_checkReturnAgainstZero11 = verify checkReturnAgainstZero "(( ! ! ! $? ))" +prop_checkReturnAgainstZero12 = verify checkReturnAgainstZero "[ ! $? -eq 0 ]" +prop_checkReturnAgainstZero13 = verifyNot checkReturnAgainstZero "(( ! $? && $? > 42))" +prop_checkReturnAgainstZero14 = verifyNot checkReturnAgainstZero "[[ -e foo || $? -eq 0 ]]" +prop_checkReturnAgainstZero15 = verifyNot checkReturnAgainstZero "(( $?, n=1 ))" +prop_checkReturnAgainstZero16 = verifyNot checkReturnAgainstZero "(( $? || $? == 4 ))" +prop_checkReturnAgainstZero17 = verifyNot checkReturnAgainstZero "(( $? + 0 ))" +prop_checkReturnAgainstZero18 = verifyNot checkReturnAgainstZero "f() { if [ $? -eq 0 ]; then :; fi; }" +prop_checkReturnAgainstZero19 = verifyNot checkReturnAgainstZero "f() ( [ $? -eq 0 ] || exit 42; )" +prop_checkReturnAgainstZero20 = verify checkReturnAgainstZero "f() { if :; then x; [ $? -eq 0 ] && exit; fi; }" +prop_checkReturnAgainstZero21 = verify checkReturnAgainstZero "(( ( $? ) ))" +prop_checkReturnAgainstZero22 = verify checkReturnAgainstZero "[[ ( $? -eq 0 ) ]]" +checkReturnAgainstZero params token = case token of - TC_Binary id _ _ lhs rhs -> check lhs rhs - TA_Binary id _ lhs rhs -> check lhs rhs - TA_Unary id _ exp - | isExitCode exp -> message (getId exp) + TC_Binary id _ op lhs rhs -> check op lhs rhs + TA_Binary id op lhs rhs + | op `elem` [">", "<", ">=", "<=", "==", "!="] -> check op lhs rhs + TA_Unary id op@"!" exp + | isExitCode exp -> message (checksSuccessLhs op) (getId exp) TA_Sequence _ [exp] - | isExitCode exp -> message (getId exp) + | isExitCode exp -> message False (getId exp) _ -> return () where - check lhs rhs = + -- We don't want to warn about composite expressions like + -- [[ $? -eq 0 || $? -eq 4 ]] since these can be annoying to rewrite. + isOnlyTestInCommand t = + case getPath (parentMap params) t of + _:(T_Condition {}):_ -> True + _:(T_Arithmetic {}):_ -> True + _:(TA_Sequence _ [_]):(T_Arithmetic {}):_ -> True + + -- Some negations and groupings are also fine + _:next@(TC_Unary _ _ "!" _):_ -> isOnlyTestInCommand next + _:next@(TA_Unary _ "!" _):_ -> isOnlyTestInCommand next + _:next@(TC_Group {}):_ -> isOnlyTestInCommand next + _:next@(TA_Sequence _ [_]):_ -> isOnlyTestInCommand next + _ -> False + + -- TODO: Do better $? tracking and filter on whether + -- the target command is in the same function + getFirstCommandInFunction = f + where + f t = case t of + T_Function _ _ _ _ x -> f x + T_BraceGroup _ (x:_) -> f x + T_Subshell _ (x:_) -> f x + T_Annotation _ _ x -> f x + T_AndIf _ x _ -> f x + T_OrIf _ x _ -> f x + T_Pipeline _ _ (x:_) -> f x + T_Redirecting _ _ (T_IfExpression _ (((x:_),_):_) _) -> f x + x -> x + + isFirstCommandInFunction = fromMaybe False $ do + let path = getPath (parentMap params) token + func <- listToMaybe $ filter isFunction path + cmd <- getClosestCommand (parentMap params) token + return $ getId cmd == getId (getFirstCommandInFunction func) + + -- Is "$? op 0" trying to check if the command succeeded? + checksSuccessLhs op = not $ op `elem` ["-gt", "-ne", "!=", "!"] + -- Is "0 op $?" trying to check if the command succeeded? + checksSuccessRhs op = op `notElem` ["-ne", "!="] + + check op lhs rhs = if isZero rhs && isExitCode lhs - then message (getId lhs) - else when (isZero lhs && isExitCode rhs) $ message (getId rhs) + then message (checksSuccessLhs op) (getId lhs) + else when (isZero lhs && isExitCode rhs) $ message (checksSuccessRhs op) (getId rhs) isZero t = getLiteralString t == Just "0" isExitCode t = case getWordParts t of [T_DollarBraced _ _ l] -> concat (oversimplify l) == "?" _ -> False - message id = style id 2181 "Check exit code directly with e.g. 'if mycmd;', not indirectly with $?." + message forSuccess id = when (isOnlyTestInCommand token && not isFirstCommandInFunction) $ style id 2181 $ + "Check exit code directly with e.g. 'if " ++ (if forSuccess then "" else "! ") ++ "mycmd;', not indirectly with $?." + + prop_checkRedirectedNowhere1 = verify checkRedirectedNowhere "> file" prop_checkRedirectedNowhere2 = verify checkRedirectedNowhere "> file | grep foo" prop_checkRedirectedNowhere3 = verify checkRedirectedNowhere "grep foo | > bar" @@ -3194,7 +3424,7 @@ T_Literal id str <- parts let (before, after) = break ('=' ==) str guard $ all isDigit before && not (null after) - return $ warnWithFix id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it." (surroundWidth id params "\"") + return $ warnWithFix id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it." (surroundWith id params "\"") in if null literalEquals && isAssociative then warn (getId t) 2190 "Elements in associative arrays need index, e.g. array=( [index]=value ) ." @@ -3588,21 +3818,27 @@ suggest t = info (getId t) 2231 "Quote expansions in this for loop glob to prevent wordsplitting, e.g. \"$dir\"/*.txt ." + prop_checkSubshelledTests1 = verify checkSubshelledTests "a && ( [ b ] || ! [ c ] )" prop_checkSubshelledTests2 = verify checkSubshelledTests "( [ a ] )" prop_checkSubshelledTests3 = verify checkSubshelledTests "( [ a ] && [ b ] || test c )" prop_checkSubshelledTests4 = verify checkSubshelledTests "( [ a ] && { [ b ] && [ c ]; } )" +prop_checkSubshelledTests5 = verifyNot checkSubshelledTests "( [[ ${var:=x} = y ]] )" +prop_checkSubshelledTests6 = verifyNot checkSubshelledTests "( [[ $((i++)) = 10 ]] )" +prop_checkSubshelledTests7 = verifyNot checkSubshelledTests "( [[ $((i+=1)) = 10 ]] )" +prop_checkSubshelledTests8 = verify checkSubshelledTests "# shellcheck disable=SC2234\nf() ( [[ x ]] )" + checkSubshelledTests params t = case t of - T_Subshell id list | all isTestStructure list -> + T_Subshell id list | all isTestStructure list && (not (hasAssignment t)) -> case () of -- Special case for if (test) and while (test) _ | isCompoundCondition (getPath (parentMap params) t) -> - style id 2233 "Remove superfluous (..) around condition." + style id 2233 "Remove superfluous (..) around condition to avoid subshell overhead." - -- Special case for ([ x ]) - _ | isSingleTest list -> - style id 2234 "Remove superfluous (..) around test command." + -- Special case for ([ x ]), except for func() ( [ x ] ) + _ | isSingleTest list && (not $ isFunctionBody (getPath (parentMap params) t)) -> + style id 2234 "Remove superfluous (..) around test command to avoid subshell overhead." -- General case for ([ x ] || [ y ] && etc) _ -> style id 2235 "Use { ..; } instead of (..) to avoid subshell overhead." @@ -3614,6 +3850,11 @@ [c] | isTestCommand c -> True _ -> False + isFunctionBody path = + case path of + (_:f:_) -> isFunction f + _ -> False + isTestStructure t = case t of T_Banged _ t -> isTestStructure t @@ -3644,6 +3885,19 @@ T_UntilExpression {} : _ -> True _ -> False + hasAssignment t = isNothing $ doAnalysis guardNotAssignment t + guardNotAssignment t = + case t of + TA_Assignment {} -> Nothing + TA_Unary _ s _ -> guard . not $ "++" `isInfixOf` s || "--" `isInfixOf` s + T_DollarBraced _ _ l -> + let str = concat $ oversimplify l + modifier = getBracedModifier str + in + guard . not $ "=" `isPrefixOf` modifier || ":=" `isPrefixOf` modifier + T_DollarBraceCommandExpansion {} -> Nothing + _ -> Just () + -- Skip any parent of a T_Subshell until we reach something interesting skippable t = case t of @@ -4155,11 +4409,11 @@ _ | "===" `isPrefixOf` s -> borderMsg (getId originalToken) _ -> prefixMsg (getId cmd) - -- $var==42 + -- '$var==42' _ | "==" `isInfixOf` s -> badComparisonMsg (getId cmd) - -- ${foo[x]}=42 and $foo=42 + -- '${foo[x]}=42' and '$foo=42' [T_DollarBraced id braced l] | "=" `isPrefixOf` s -> do let variableStr = concat $ oversimplify l let variableReference = getBracedReference variableStr @@ -4172,22 +4426,22 @@ && "]" `isSuffixOf` variableModifier case () of - -- $foo=bar should already have caused a parse-time SC1066 + -- '$foo=bar' should already have caused a parse-time SC1066 -- _ | not braced && isPlain -> -- return () _ | variableStr == "" -> -- Don't try to fix ${}=foo genericMsg (getId cmd) - -- $#=42 or ${#var}=42 + -- '$#=42' or '${#var}=42' _ | "#" `isPrefixOf` variableStr -> genericMsg (getId cmd) - -- ${0}=42 + -- '${0}=42' _ | variableStr == "0" -> assign0Msg id $ fixWith [replaceToken id params "BASH_ARGV0"] - -- $2=2 + -- '$2=2' _ | isPositional -> positionalMsg id @@ -4253,6 +4507,355 @@ case t of T_NormalWord _ (x:_) -> getId x _ -> getId t + + +prop_checkCommandWithTrailingSymbol1 = verify checkCommandWithTrailingSymbol "/" +prop_checkCommandWithTrailingSymbol2 = verify checkCommandWithTrailingSymbol "/foo/ bar/baz" +prop_checkCommandWithTrailingSymbol3 = verify checkCommandWithTrailingSymbol "/" +prop_checkCommandWithTrailingSymbol4 = verifyNot checkCommandWithTrailingSymbol "/*" +prop_checkCommandWithTrailingSymbol5 = verifyNot checkCommandWithTrailingSymbol "$foo/$bar" +prop_checkCommandWithTrailingSymbol6 = verify checkCommandWithTrailingSymbol "foo, bar" +prop_checkCommandWithTrailingSymbol7 = verifyNot checkCommandWithTrailingSymbol ". foo.sh" +prop_checkCommandWithTrailingSymbol8 = verifyNot checkCommandWithTrailingSymbol ": foo" +prop_checkCommandWithTrailingSymbol9 = verifyNot checkCommandWithTrailingSymbol "/usr/bin/python[23] file.py" + +checkCommandWithTrailingSymbol _ t = + case t of + T_SimpleCommand _ _ (cmd:_) -> + let str = fromJust $ getLiteralStringExt (\_ -> Just "x") cmd + last = lastOrDefault 'x' str + in + case str of + "." -> return () -- The . command + ":" -> return () -- The : command + " " -> return () -- Probably caught by SC1101 + "//" -> return () -- Probably caught by SC1127 + "" -> err (getId cmd) 2286 "This empty string is interpreted as a command name. Double check syntax (or use 'true' as a no-op)." + _ | last == '/' -> err (getId cmd) 2287 "This is interpreted as a command name ending with '/'. Double check syntax." + _ | last `elem` "\\.,([{<>}])#\"\'% " -> warn (getId cmd) 2288 ("This is interpreted as a command name ending with " ++ (format last) ++ ". Double check syntax.") + _ | '\t' `elem` str -> err (getId cmd) 2289 "This is interpreted as a command name containing a tab. Double check syntax." + _ | '\n' `elem` str -> err (getId cmd) 2289 "This is interpreted as a command name containing a linefeed. Double check syntax." + _ -> return () + _ -> return () + where + format x = + case x of + ' ' -> "space" + '\'' -> "apostrophe" + '\"' -> "doublequote" + x -> '\'' : x : "\'" + + +prop_checkRequireDoubleBracket1 = verifyTree checkRequireDoubleBracket "[ -x foo ]" +prop_checkRequireDoubleBracket2 = verifyTree checkRequireDoubleBracket "[ foo -o bar ]" +prop_checkRequireDoubleBracket3 = verifyNotTree checkRequireDoubleBracket "#!/bin/sh\n[ -x foo ]" +prop_checkRequireDoubleBracket4 = verifyNotTree checkRequireDoubleBracket "[[ -x foo ]]" +checkRequireDoubleBracket params = + if isBashLike params + then nodeChecksToTreeCheck [check] params + else const [] + where + check _ t = case t of + T_Condition id SingleBracket _ -> + styleWithFix id 2292 "Prefer [[ ]] over [ ] for tests in Bash/Ksh." (fixFor t) + _ -> return () + + fixFor t = fixWith $ + if isSimple t + then + [ + replaceStart (getId t) params 0 "[", + replaceEnd (getId t) params 0 "]" + ] + else [] + + -- We don't tag operators like < and -o well enough to replace them, + -- so just handle the simple cases. + isSimple t = case t of + T_Condition _ _ s -> isSimple s + TC_Binary _ _ op _ _ -> not $ any (\x -> x `elem` op) "<>" + TC_Unary {} -> True + TC_Nullary {} -> True + _ -> False + + +prop_checkUnquotedParameterExpansionPattern1 = verify checkUnquotedParameterExpansionPattern "echo \"${var#$x}\"" +prop_checkUnquotedParameterExpansionPattern2 = verify checkUnquotedParameterExpansionPattern "echo \"${var%%$(x)}\"" +prop_checkUnquotedParameterExpansionPattern3 = verifyNot checkUnquotedParameterExpansionPattern "echo \"${var[#$x]}\"" +prop_checkUnquotedParameterExpansionPattern4 = verifyNot checkUnquotedParameterExpansionPattern "echo \"${var%\"$x\"}\"" + +checkUnquotedParameterExpansionPattern params x = + case x of + T_DollarBraced _ True word@(T_NormalWord _ (T_Literal _ s : rest@(_:_))) -> do + let modifier = getBracedModifier $ concat $ oversimplify word + when ("%" `isPrefixOf` modifier || "#" `isPrefixOf` modifier) $ + mapM_ check rest + _ -> return () + where + check t = + case t of + T_DollarBraced {} -> inform t + T_DollarExpansion {} -> inform t + T_Backticked {} -> inform t + _ -> return () + + inform t = + infoWithFix (getId t) 2295 + "Expansions inside ${..} need to be quoted separately, otherwise they match as patterns." $ + surroundWith (getId t) params "\"" + + +prop_checkArrayValueUsedAsIndex1 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr[i]}; done" +prop_checkArrayValueUsedAsIndex2 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr[$i]}; done" +prop_checkArrayValueUsedAsIndex3 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo $((arr[i])); done" +prop_checkArrayValueUsedAsIndex4 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr1[@]} ${arr2[@]}; do echo ${arr1[$i]}; done" +prop_checkArrayValueUsedAsIndex5 = verifyTree checkArrayValueUsedAsIndex "for i in ${arr1[@]} ${arr2[@]}; do echo ${arr2[$i]}; done" +prop_checkArrayValueUsedAsIndex7 = verifyNotTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr[K]}; done" +prop_checkArrayValueUsedAsIndex8 = verifyNotTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do i=42; echo ${arr[i]}; done" +prop_checkArrayValueUsedAsIndex9 = verifyNotTree checkArrayValueUsedAsIndex "for i in ${arr[@]}; do echo ${arr2[i]}; done" + +checkArrayValueUsedAsIndex params _ = + doVariableFlowAnalysis read write Map.empty (variableFlow params) + where + write loop@T_ForIn {} _ name (DataString (SourceFrom words)) = do + modify $ Map.insert name (loop, mapMaybe f words) + return [] + where + f x = do + name <- getArrayName x + return (x, name) + + write _ _ name _ = do + modify $ Map.delete name + return [] + + read _ t name = do + varMap <- get + return $ fromMaybe [] $ do + (loop, arrays) <- Map.lookup name varMap + (arrayRef, arrayName) <- getArrayIfUsedAsIndex name t + -- Is this one of the 'for' arrays? + (loopWord, _) <- find ((==arrayName) . snd) arrays + -- Are we still in this loop? + guard $ getId loop `elem` map getId (getPath parents t) + return [ + makeComment WarningC (getId loopWord) 2302 "This loops over values. To loop over keys, use \"${!array[@]}\".", + makeComment WarningC (getId arrayRef) 2303 $ (e4m name) ++ " is an array value, not a key. Use directly or loop over keys instead." + ] + + parents = parentMap params + + getArrayName :: Token -> Maybe String + getArrayName t = do + [T_DollarBraced _ _ l] <- return $ getWordParts t + let str = concat $ oversimplify l + guard $ getBracedModifier str == "[@]" && not ("!" `isPrefixOf` str) + return $ getBracedReference str + + -- This is much uglier than it should be + getArrayIfUsedAsIndex :: String -> Token -> Maybe (Token, String) + getArrayIfUsedAsIndex name t = + case t of + T_DollarBraced _ _ list -> do + let ref = getBracedReference $ concat $ oversimplify list + guard $ ref == name + -- We found a $name. Look up the chain to see if it's ${arr[$name]} + list@T_NormalWord {} <- Map.lookup (getId t) parents + (T_DollarBraced _ _ parentList) <- Map.lookup (getId list) parents + (T_Literal _ head : index : T_Literal _ tail : _) <- return $ getWordParts parentList + let str = concat $ oversimplify list + let modifier = getBracedModifier str + guard $ getId index == getId t + guard $ "[${VAR}]" `isPrefixOf` modifier + return (t, getBracedReference str) + + T_NormalWord wordId list -> do + -- We found just name. Check if it's part of ${something[name]} + parent@(T_DollarBraced _ _ parentList) <- Map.lookup wordId parents + let str = concat $ oversimplify t + let modifier = getBracedModifier str + guard $ ("[" ++ name ++ "]") `isPrefixOf` modifier + return (parent, getBracedReference str) + + TA_Variable indexId ref [] -> do + -- We found arithmetic name. See if it's part of arithmetic arr[name] + guard $ ref == name + (TA_Sequence seqId [element]) <- Map.lookup indexId parents + guard $ getId element == indexId + parent@(TA_Variable arrayId arrayName [element]) <- Map.lookup seqId parents + guard $ getId element == seqId + return (parent, arrayName) + + _ -> Nothing + +prop_checkSetESuppressed1 = verifyTree checkSetESuppressed "set -e; f(){ :; }; x=$(f)" +prop_checkSetESuppressed2 = verifyNotTree checkSetESuppressed "f(){ :; }; x=$(f)" +prop_checkSetESuppressed3 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; x=$(set -e; f)" +prop_checkSetESuppressed4 = verifyTree checkSetESuppressed "set -e; f(){ :; }; baz=$(set -e; f) || :" +prop_checkSetESuppressed5 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; baz=$(echo \"\") || :" +prop_checkSetESuppressed6 = verifyTree checkSetESuppressed "set -e; f(){ :; }; f && echo" +prop_checkSetESuppressed7 = verifyTree checkSetESuppressed "set -e; f(){ :; }; f || echo" +prop_checkSetESuppressed8 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; echo && f" +prop_checkSetESuppressed9 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; echo || f" +prop_checkSetESuppressed10 = verifyTree checkSetESuppressed "set -e; f(){ :; }; ! f" +prop_checkSetESuppressed11 = verifyTree checkSetESuppressed "set -e; f(){ :; }; if f; then :; fi" +prop_checkSetESuppressed12 = verifyTree checkSetESuppressed "set -e; f(){ :; }; if set -e; f; then :; fi" +prop_checkSetESuppressed13 = verifyTree checkSetESuppressed "set -e; f(){ :; }; while f; do :; done" +prop_checkSetESuppressed14 = verifyTree checkSetESuppressed "set -e; f(){ :; }; while set -e; f; do :; done" +prop_checkSetESuppressed15 = verifyTree checkSetESuppressed "set -e; f(){ :; }; until f; do :; done" +prop_checkSetESuppressed16 = verifyTree checkSetESuppressed "set -e; f(){ :; }; until set -e; f; do :; done" +prop_checkSetESuppressed17 = verifyNotTree checkSetESuppressed "set -e; f(){ :; }; g(){ :; }; g f" +prop_checkSetESuppressed18 = verifyNotTree checkSetESuppressed "set -e; shopt -s inherit_errexit; f(){ :; }; x=$(f)" +checkSetESuppressed params t = + if hasSetE params then runNodeAnalysis checkNode params t else [] + where + checkNode _ (T_SimpleCommand _ _ (cmd:_)) = when (isFunction cmd) (checkCmd cmd) + checkNode _ _ = return () + + functions_ = functions t + + isFunction cmd = isJust $ do + literalArg <- getUnquotedLiteral cmd + Map.lookup literalArg functions_ + + checkCmd cmd = go $ getPath (parentMap params) cmd + where + go (child:parent:rest) = do + case parent of + T_Banged _ condition | child `isIn` [condition] -> informConditional "a ! condition" cmd + T_AndIf _ condition _ | child `isIn` [condition] -> informConditional "an && condition" cmd + T_OrIf _ condition _ | child `isIn` [condition] -> informConditional "an || condition" cmd + T_IfExpression _ condition _ | child `isIn` concatMap fst condition -> informConditional "an 'if' condition" cmd + T_UntilExpression _ condition _ | child `isIn` condition -> informConditional "an 'until' condition" cmd + T_WhileExpression _ condition _ | child `isIn` condition -> informConditional "a 'while' condition" cmd + T_DollarExpansion {} | not $ errExitEnabled parent -> informUninherited cmd + T_Backticked {} | not $ errExitEnabled parent -> informUninherited cmd + _ -> return () + go (parent:rest) + go _ = return () + + informConditional condType t = + info (getId t) 2310 ( + "This function is invoked in " ++ condType ++ " so set -e " ++ + "will be disabled. Invoke separately if failures should " ++ + "cause the script to exit.") + informUninherited t = + info (getId t) 2311 ( + "Bash implicitly disabled set -e for this function " ++ + "invocation because it's inside a command substitution. " ++ + "Add set -e; before it or enable inherit_errexit.") + errExitEnabled t = hasInheritErrexit params || containsSetE t + isIn t cmds = getId t `elem` map getId cmds + + +prop_checkExtraMaskedReturns1 = verifyTree checkExtraMaskedReturns "cat < <(ls)" +prop_checkExtraMaskedReturns2 = verifyTree checkExtraMaskedReturns "read -ra arr <(ls)" +prop_checkExtraMaskedReturns3 = verifyTree checkExtraMaskedReturns "ls >(cat)" +prop_checkExtraMaskedReturns4 = verifyTree checkExtraMaskedReturns "false | true" +prop_checkExtraMaskedReturns5 = verifyNotTree checkExtraMaskedReturns "set -o pipefail; false | true" +prop_checkExtraMaskedReturns6 = verifyNotTree checkExtraMaskedReturns "false | true || true" +prop_checkExtraMaskedReturns7 = verifyTree checkExtraMaskedReturns "true $(false)" +prop_checkExtraMaskedReturns8 = verifyTree checkExtraMaskedReturns "x=$(false)$(true)" +prop_checkExtraMaskedReturns9 = verifyNotTree checkExtraMaskedReturns "x=$(false)true" +prop_checkExtraMaskedReturns10 = verifyTree checkExtraMaskedReturns "x=`false``false`" +prop_checkExtraMaskedReturns11 = verifyTree checkExtraMaskedReturns "x=\"$(false)$(true)\"" +prop_checkExtraMaskedReturns12 = verifyTree checkExtraMaskedReturns "x=\"$(false)\"\"$(true)\"" +prop_checkExtraMaskedReturns13 = verifyTree checkExtraMaskedReturns "true <<<$(false)" +prop_checkExtraMaskedReturns14 = verifyNotTree checkExtraMaskedReturns "echo asdf | false" +prop_checkExtraMaskedReturns15 = verifyNotTree checkExtraMaskedReturns "readonly x=$(false)" +prop_checkExtraMaskedReturns16 = verifyTree checkExtraMaskedReturns "readarray -t files < <(ls)" +prop_checkExtraMaskedReturns17 = verifyNotTree checkExtraMaskedReturns "x=( $(false) false )" +prop_checkExtraMaskedReturns18 = verifyTree checkExtraMaskedReturns "x=( $(false) $(false) )" +prop_checkExtraMaskedReturns19 = verifyNotTree checkExtraMaskedReturns "x=( $(false) [4]=false )" +prop_checkExtraMaskedReturns20 = verifyTree checkExtraMaskedReturns "x=( $(false) [4]=$(false) )" +prop_checkExtraMaskedReturns21 = verifyTree checkExtraMaskedReturns "cat << foo\n $(false)\nfoo" +prop_checkExtraMaskedReturns22 = verifyTree checkExtraMaskedReturns "[[ $(false) ]]" +prop_checkExtraMaskedReturns23 = verifyNotTree checkExtraMaskedReturns "x=$(false) y=z" +prop_checkExtraMaskedReturns24 = verifyNotTree checkExtraMaskedReturns "x=$(( $(date +%s) ))" +prop_checkExtraMaskedReturns25 = verifyTree checkExtraMaskedReturns "echo $(( $(date +%s) ))" +prop_checkExtraMaskedReturns26 = verifyNotTree checkExtraMaskedReturns "x=( $(false) )" +prop_checkExtraMaskedReturns27 = verifyTree checkExtraMaskedReturns "x=$(false) false" +prop_checkExtraMaskedReturns28 = verifyTree checkExtraMaskedReturns "x=$(false) y=$(false)" +prop_checkExtraMaskedReturns29 = verifyNotTree checkExtraMaskedReturns "false < <(set -e)" +prop_checkExtraMaskedReturns30 = verifyNotTree checkExtraMaskedReturns "false < <(shopt -s cdspell)" +prop_checkExtraMaskedReturns31 = verifyNotTree checkExtraMaskedReturns "false < <(dirname \"${BASH_SOURCE[0]}\")" +prop_checkExtraMaskedReturns32 = verifyNotTree checkExtraMaskedReturns "false < <(basename \"${BASH_SOURCE[0]}\")" +prop_checkExtraMaskedReturns33 = verifyNotTree checkExtraMaskedReturns "{ false || true; } | true" +prop_checkExtraMaskedReturns34 = verifyNotTree checkExtraMaskedReturns "{ false || :; } | true" +prop_checkExtraMaskedReturns35 = verifyTree checkExtraMaskedReturns "f() { local -r x=$(false); }" + +checkExtraMaskedReturns params t = runNodeAnalysis findMaskingNodes params t + where + findMaskingNodes _ (T_Arithmetic _ list) = findMaskedNodesInList [list] + findMaskingNodes _ (T_Array _ list) = findMaskedNodesInList $ allButLastSimpleCommands list + findMaskingNodes _ (T_Condition _ _ condition) = findMaskedNodesInList [condition] + findMaskingNodes _ (T_DoubleQuoted _ list) = findMaskedNodesInList $ allButLastSimpleCommands list + findMaskingNodes _ (T_HereDoc _ _ _ _ list) = findMaskedNodesInList list + findMaskingNodes _ (T_HereString _ word) = findMaskedNodesInList [word] + findMaskingNodes _ (T_NormalWord _ parts) = findMaskedNodesInList $ allButLastSimpleCommands parts + findMaskingNodes _ (T_Pipeline _ _ cmds) | not (hasPipefail params) = findMaskedNodesInList $ allButLastSimpleCommands cmds + findMaskingNodes _ (T_ProcSub _ _ list) = findMaskedNodesInList list + findMaskingNodes _ (T_SimpleCommand _ assigns (_:args)) = findMaskedNodesInList $ assigns ++ args + findMaskingNodes _ (T_SimpleCommand _ assigns []) = findMaskedNodesInList $ allButLastSimpleCommands assigns + findMaskingNodes _ _ = return () + + findMaskedNodesInList = mapM_ (doAnalysis findMaskedNodes) + + isMaskedNode t = not (isHarmlessCommand t || isCheckedElsewhere t || isMaskDeliberate t) + findMaskedNodes t@(T_SimpleCommand _ _ (_:_)) = when (isMaskedNode t) $ inform t + findMaskedNodes t@T_Condition {} = when (isMaskedNode t) $ inform t + findMaskedNodes _ = return () + + containsSimpleCommand t = isNothing $ doAnalysis go t + where + go t = case t of + T_SimpleCommand {} -> fail "" + _ -> return () + + allButLastSimpleCommands cmds = + if null simpleCommands then [] else init simpleCommands + where + simpleCommands = filter containsSimpleCommand cmds + + inform t = info (getId t) 2312 ("Consider invoking this command " + ++ "separately to avoid masking its return value (or use '|| true' " + ++ "to ignore).") + + isMaskDeliberate t = hasParent isOrIf t + where + isOrIf _ (T_OrIf _ _ (T_Pipeline _ _ [T_Redirecting _ _ cmd])) + = getCommandBasename cmd `elem` [Just "true", Just ":"] + isOrIf _ _ = False + + isCheckedElsewhere t = hasParent isDeclaringCommand t + where + isDeclaringCommand t _ = fromMaybe False $ do + cmd <- getCommand t + basename <- getCommandBasename cmd + return $ + case basename of + -- local -r x=$(false) is intentionally ignored for SC2155 + "local" | "r" `elem` (map snd $ getAllFlags cmd) -> False + _ -> basename `elem` declaringCommands + + isHarmlessCommand t = fromMaybe False $ do + basename <- getCommandBasename t + return $ basename `elem` [ + "echo" + ,"basename" + ,"dirname" + ,"printf" + ,"set" + ,"shopt" + ] + + parentChildPairs t = go $ parents params t + where + go (child:parent:rest) = (parent, child):go (parent:rest) + go _ = [] + + hasParent pred t = any (uncurry pred) (parentChildPairs t) + return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/src/ShellCheck/AnalyzerLib.hs b/src/ShellCheck/AnalyzerLib.hs --- a/src/ShellCheck/AnalyzerLib.hs +++ b/src/ShellCheck/AnalyzerLib.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2019 Vidar Holen + Copyright 2012-2021 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -79,8 +79,12 @@ data Parameters = Parameters { -- Whether this script has the 'lastpipe' option set/default. hasLastpipe :: Bool, + -- Whether this script has the 'inherit_errexit' option set/default. + hasInheritErrexit :: Bool, -- Whether this script has 'set -e' anywhere. hasSetE :: Bool, + -- Whether this script has 'set -o pipefail' anywhere. + hasPipefail :: Bool, -- A linear (bad) analysis of data flow variableFlow :: [StackData], -- A map from Id to parent Token @@ -142,7 +146,7 @@ prRoot pr let spec = defaultSpec pr let params = makeParameters spec - return . not . null $ runChecker params c + return . not . null $ filterByAnnotation spec params $ runChecker params c makeComment :: Severity -> Id -> Code -> String -> TokenComment makeComment severity id code note = @@ -167,6 +171,8 @@ errWithFix = addCommentWithFix ErrorC warnWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m () warnWithFix = addCommentWithFix WarningC +infoWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m () +infoWithFix = addCommentWithFix InfoC styleWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m () styleWithFix = addCommentWithFix StyleC @@ -178,7 +184,8 @@ makeCommentWithFix severity id code str fix = let comment = makeComment severity id code str withFix = comment { - tcFix = Just fix + -- If fix is empty, pretend it wasn't there. + tcFix = if null (fixReplacements fix) then Nothing else Just fix } in force withFix @@ -193,7 +200,18 @@ Dash -> False Sh -> False Ksh -> True, - + hasInheritErrexit = + case shellType params of + Bash -> containsInheritErrexit root + Dash -> True + Sh -> True + Ksh -> False, + hasPipefail = + case shellType params of + Bash -> containsPipefail root + Dash -> True + Sh -> True + Ksh -> containsPipefail root, shellTypeSpecified = isJust (asShellType spec) || isJust (asFallbackShell spec), parentMap = getParentTree root, variableFlow = getVariableFlow params root, @@ -216,19 +234,34 @@ _ -> False re = mkRegex "[[:space:]]-[^-]*e" --- Does this script mention 'shopt -s lastpipe' anywhere? --- Also used as a hack. -containsLastpipe root = +containsPipefail root = isNothing $ doAnalysis (guard . not . isPipefail) root + where + isPipefail t = + case t of + T_SimpleCommand {} -> + t `isUnqualifiedCommand` "set" && + ("pipefail" `elem` oversimplify t || + "o" `elem` map snd (getAllFlags t)) + _ -> False + +containsShopt shopt root = isNothing $ doAnalysis (guard . not . isShoptLastPipe) root where isShoptLastPipe t = case t of T_SimpleCommand {} -> t `isUnqualifiedCommand` "shopt" && - ("lastpipe" `elem` oversimplify t) + (shopt `elem` oversimplify t) _ -> False +-- Does this script mention 'shopt -s inherit_errexit' anywhere? +containsInheritErrexit = containsShopt "inherit_errexit" +-- Does this script mention 'shopt -s lastpipe' anywhere? +-- Also used as a hack. +containsLastpipe = containsShopt "lastpipe" + + prop_determineShell0 = determineShellTest "#!/bin/sh" == Sh prop_determineShell1 = determineShellTest "#!/usr/bin/env ksh" == Ksh prop_determineShell2 = determineShellTest "" == Bash @@ -287,14 +320,14 @@ isQuoteFree = isQuoteFreeNode False -isQuoteFreeNode strict tree t = +isQuoteFreeNode strict shell tree t = isQuoteFreeElement t || - headOrDefault False (mapMaybe isQuoteFreeContext (drop 1 $ getPath tree t)) + (fromMaybe False $ msum $ map isQuoteFreeContext $ drop 1 $ getPath tree t) where -- Is this node self-quoting in itself? isQuoteFreeElement t = case t of - T_Assignment {} -> True + T_Assignment {} -> assignmentIsQuoting t T_FdRedirect {} -> True _ -> False @@ -306,7 +339,7 @@ TC_Binary _ DoubleBracket _ _ _ -> return True TA_Sequence {} -> return True T_Arithmetic {} -> return True - T_Assignment {} -> return True + T_Assignment {} -> return $ assignmentIsQuoting t T_Redirecting {} -> return False T_DoubleQuoted _ _ -> return True T_DollarDoubleQuoted _ _ -> return True @@ -318,6 +351,18 @@ T_SelectIn {} -> return (not strict) _ -> Nothing + -- Check whether this assigment is self-quoting due to being a recognized + -- assignment passed to a Declaration Utility. This will soon be required + -- by POSIX: https://austingroupbugs.net/view.php?id=351 + assignmentIsQuoting t = shellParsesParamsAsAssignments || not (isAssignmentParamToCommand t) + shellParsesParamsAsAssignments = shell /= Sh + + -- Is this assignment a parameter to a command like export/typeset/etc? + isAssignmentParamToCommand (T_Assignment id _ _ _ _) = + case Map.lookup id tree of + Just (T_SimpleCommand _ _ (_:args)) -> id `elem` (map getId args) + _ -> False + -- Check if a token is a parameter to a certain command by name: -- Example: isParamTo (parentMap params) "sed" t isParamTo :: Map.Map Id Token -> String -> Token -> Bool @@ -482,14 +527,8 @@ -- Count [[ -v foo ]] as an "assignment". -- This is to prevent [ -v foo ] being unassigned or unused. - TC_Unary id _ "-v" token -> do - str <- fmap (takeWhile (/= '[')) $ -- Quoted index - flip getLiteralStringExt token $ \x -> - case x of - T_Glob _ s -> return s -- Unquoted index - _ -> [] - - guard . not . null $ str + TC_Unary id _ "-v" token -> maybeToList $ do + str <- getVariableForTestDashV token return (t, token, str, DataString SourceChecked) TC_Unary _ _ "-n" token -> markAsChecked t token @@ -532,14 +571,12 @@ -- Consider 'export/declare -x' a reference, since it makes the var available getReferencedVariableCommand base@(T_SimpleCommand _ _ (T_NormalWord _ (T_Literal _ x:_):rest)) = case x of + "declare" -> forDeclare + "typeset" -> forDeclare + "export" -> if "f" `elem` flags then [] else concatMap getReference rest - "declare" -> if - any (`elem` flags) ["x", "p"] && - (not $ any (`elem` flags) ["f", "F"]) - then concatMap getReference rest - else [] "local" -> if "x" `elem` flags then concatMap getReference rest else [] @@ -550,6 +587,13 @@ "alias" -> [(base, token, name) | token <- rest, name <- getVariablesFromLiteralToken token] _ -> [] where + forDeclare = + if + any (`elem` flags) ["x", "p"] && + (not $ any (`elem` flags) ["f", "F"]) + then concatMap getReference rest + else [] + getReference t@(T_Assignment _ _ name _ value) = [(t, t, name)] getReference t@(T_NormalWord _ [T_Literal _ name]) | not ("-" `isPrefixOf` name) = [(t, t, name)] getReference _ = [] @@ -589,8 +633,8 @@ "export" -> if "f" `elem` flags then [] else concatMap getModifierParamString rest - "declare" -> if any (`elem` flags) ["F", "f", "p"] then [] else declaredVars - "typeset" -> declaredVars + "declare" -> forDeclare + "typeset" -> forDeclare "local" -> concatMap getModifierParamString rest "readonly" -> @@ -602,6 +646,7 @@ return (base, base, "@", DataString $ SourceFrom params) "printf" -> maybeToList $ getPrintfVariable rest + "wait" -> maybeToList $ getWaitVariable rest "mapfile" -> maybeToList $ getMapfileArray base rest "readarray" -> maybeToList $ getMapfileArray base rest @@ -621,6 +666,8 @@ T_NormalWord id1 [T_DoubleQuoted id2 [T_Literal id3 (stripEquals s)]] stripEqualsFrom t = t + forDeclare = if any (`elem` flags) ["F", "f", "p"] then [] else declaredVars + declaredVars = concatMap (getModifierParam defaultType) rest where defaultType = if any (`elem` flags) ["a", "A"] then DataArray else DataString @@ -659,16 +706,16 @@ _ -> return (t:fromMaybe [] (getSetParams rest)) getSetParams [] = Nothing - getPrintfVariable list = f $ map (\x -> (x, getLiteralString x)) list - where - f ((_, Just "-v") : (t, Just var) : _) = return (base, t, varName, varType $ SourceFrom list) - where - (varName, varType) = case elemIndex '[' var of - Just i -> (take i var, DataArray) - Nothing -> (var, DataString) - f (_:rest) = f rest - f [] = fail "not found" + getPrintfVariable list = getFlagAssignedVariable "v" (SourceFrom list) $ getBsdOpts "v:" list + getWaitVariable list = getFlagAssignedVariable "p" SourceInteger $ return $ getGenericOpts list + getFlagAssignedVariable str dataSource maybeFlags = do + flags <- maybeFlags + (_, (flag, value)) <- find ((== str) . fst) flags + variableName <- getLiteralStringExt (const $ return "!") value + let (baseName, index) = span (/= '[') variableName + return (base, value, baseName, (if null index then DataString else DataArray) dataSource) + -- mapfile has some curious syntax allowing flags plus 0..n variable names -- where only the first non-option one is used if any. getMapfileArray base rest = parseArgs `mplus` fallback @@ -708,6 +755,20 @@ where re = mkRegex "(\\[.*\\])" +-- Given a NormalWord like foo or foo[$bar], get foo. +-- Primarily used to get references for [[ -v foo[bar] ]] +getVariableForTestDashV :: Token -> Maybe String +getVariableForTestDashV t = do + str <- takeWhile ('[' /=) <$> getLiteralStringExt toStr t + guard $ isVariableName str + return str + where + -- foo[bar] gets parsed with [bar] as a glob, so undo that + toStr (T_Glob _ s) = return s + -- Turn foo[$x] into foo[\0] so that we can get the constant array name + -- in a non-constant expression (while filtering out foo$x[$y]) + toStr _ = return "\0" + prop_getOffsetReferences1 = getOffsetReferences ":bar" == ["bar"] prop_getOffsetReferences2 = getOffsetReferences ":bar:baz" == ["bar", "baz"] prop_getOffsetReferences3 = getOffsetReferences "[foo]:bar" == ["bar"] @@ -737,7 +798,7 @@ TC_Unary id _ "-v" token -> getIfReference t token TC_Unary id _ "-R" token -> getIfReference t token TC_Binary id DoubleBracket op lhs rhs -> - if isDereferencing op + if isDereferencingBinaryOp op then concatMap (getIfReference t) [lhs, rhs] else [] @@ -766,17 +827,16 @@ T_Glob _ s -> return s -- Also when parsed as globs _ -> [] - getIfReference context token = do - str@(h:_) <- getLiteralStringExt literalizer token - when (isDigit h) $ fail "is a number" + getIfReference context token = maybeToList $ do + str <- getVariableForTestDashV token return (context, token, getBracedReference str) - isDereferencing = (`elem` ["-eq", "-ne", "-lt", "-le", "-gt", "-ge"]) - isArithmeticAssignment t = case getPath parents t of this: TA_Assignment _ "=" lhs _ :_ -> lhs == t _ -> False +isDereferencingBinaryOp = (`elem` ["-eq", "-ne", "-lt", "-le", "-gt", "-ge"]) + dataTypeFrom defaultType v = (case v of T_Array {} -> DataArray; _ -> defaultType) $ SourceFrom [v] @@ -801,6 +861,7 @@ isVariableStartChar x = x == '_' || isAsciiLower x || isAsciiUpper x isVariableChar x = isVariableStartChar x || isDigit x +isSpecialVariableChar = (`elem` "*@#?-$!") variableNameRegex = mkRegex "[_a-zA-Z][_a-zA-Z0-9]*" prop_isVariableName1 = isVariableName "_fo123" @@ -846,7 +907,7 @@ let name = takeWhile isVariableChar s guard . not $ null name return name - getSpecial (c:_) | c `elem` "*@#?-$!" = return [c] + getSpecial (c:_) | isSpecialVariableChar c = return [c] getSpecial _ = fail "empty or not special" nameExpansion ('!':next:rest) = do -- e.g. ${!foo*bar*} @@ -859,6 +920,8 @@ prop_getBracedModifier1 = getBracedModifier "foo:bar:baz" == ":bar:baz" prop_getBracedModifier2 = getBracedModifier "!var:-foo" == ":-foo" prop_getBracedModifier3 = getBracedModifier "foo[bar]" == "[bar]" +prop_getBracedModifier4 = getBracedModifier "foo[@]@Q" == "[@]@Q" +prop_getBracedModifier5 = getBracedModifier "@@Q" == "@Q" getBracedModifier s = headOrDefault "" $ do let var = getBracedReference s a <- dropModifier s @@ -877,6 +940,10 @@ headOrDefault _ (a:_) = a headOrDefault def _ = def +-- Get the last element or a default. Like `last` but safe. +lastOrDefault def [] = def +lastOrDefault _ list = last list + --- Get element n of a list, or Nothing. Like `!!` but safe. (!!!) list i = case drop i list of @@ -933,6 +1000,35 @@ Ksh -> True Dash -> False Sh -> False + +-- Returns whether a token is a parameter expansion without any modifiers. +-- True for $var ${var} $1 $# +-- False for ${#var} ${var[x]} ${var:-0} +isUnmodifiedParameterExpansion t = + case t of + T_DollarBraced _ False _ -> True + T_DollarBraced _ _ list -> + let str = concat $ oversimplify list + in getBracedReference str == str + _ -> False + +isTrueAssignmentSource c = + case c of + DataString SourceChecked -> False + DataString SourceDeclaration -> False + DataArray SourceChecked -> False + DataArray SourceDeclaration -> False + _ -> True + +modifiesVariable params token name = + or $ map check flow + where + flow = getVariableFlow params token + check t = + case t of + Assignment (_, _, n, source) -> isTrueAssignmentSource source && n == name + _ -> False + return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/src/ShellCheck/Checker.hs b/src/ShellCheck/Checker.hs --- a/src/ShellCheck/Checker.hs +++ b/src/ShellCheck/Checker.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2019 Vidar Holen + Copyright 2012-2020 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -156,6 +156,11 @@ siFindSource = mapper } +checkWithRcIncludesAndSourcePath rc includes mapper = getErrors + (mockRcFile rc $ mockedSystemInterface includes) { + siFindSource = mapper + } + prop_findsParseIssue = check "echo \"$12\"" == [1037] prop_commentDisablesParseIssue1 = @@ -301,6 +306,13 @@ csScript = "#shellcheck disable=SC2148\nfoo" } +prop_canDisableAllWarnings = result == [2086] + where + result = checkWithSpec [] emptyCheckSpec { + csFilename = "file.sh", + csScript = "#!/bin/sh\necho $1\n#shellcheck disable=all\necho `echo $1`" + } + prop_canDisableParseErrors = null $ result where result = checkWithSpec [] emptyCheckSpec { @@ -384,7 +396,7 @@ prop_sourcePathRedirectsName = result == [2086] where - f "dir/myscript" _ "lib" = return "foo/lib" + f "dir/myscript" _ _ "lib" = return "foo/lib" result = checkWithIncludesAndSourcePath [("foo/lib", "echo $1")] f emptyCheckSpec { csScript = "#!/bin/bash\nsource lib", csFilename = "dir/myscript", @@ -393,7 +405,7 @@ prop_sourcePathAddsAnnotation = result == [2086] where - f "dir/myscript" ["mypath"] "lib" = return "foo/lib" + f "dir/myscript" _ ["mypath"] "lib" = return "foo/lib" result = checkWithIncludesAndSourcePath [("foo/lib", "echo $1")] f emptyCheckSpec { csScript = "#!/bin/bash\n# shellcheck source-path=mypath\nsource lib", csFilename = "dir/myscript", @@ -402,13 +414,75 @@ prop_sourcePathRedirectsDirective = result == [2086] where - f "dir/myscript" _ "lib" = return "foo/lib" - f _ _ _ = return "/dev/null" + f "dir/myscript" _ _ "lib" = return "foo/lib" + f _ _ _ _ = return "/dev/null" result = checkWithIncludesAndSourcePath [("foo/lib", "echo $1")] f emptyCheckSpec { csScript = "#!/bin/bash\n# shellcheck source=lib\nsource kittens", csFilename = "dir/myscript", csCheckSourced = True } + +prop_rcCanAllowExternalSources = result == [2086] + where + f "dir/myscript" (Just True) _ "mylib" = return "resolved/mylib" + f a b c d = error $ show ("Unexpected", a, b, c, d) + result = checkWithRcIncludesAndSourcePath "external-sources=true" [("resolved/mylib", "echo $1")] f emptyCheckSpec { + csScript = "#!/bin/bash\nsource mylib", + csFilename = "dir/myscript", + csCheckSourced = True + } + +prop_rcCanDenyExternalSources = result == [2086] + where + f "dir/myscript" (Just False) _ "mylib" = return "resolved/mylib" + f a b c d = error $ show ("Unexpected", a, b, c, d) + result = checkWithRcIncludesAndSourcePath "external-sources=false" [("resolved/mylib", "echo $1")] f emptyCheckSpec { + csScript = "#!/bin/bash\nsource mylib", + csFilename = "dir/myscript", + csCheckSourced = True + } + +prop_rcCanLeaveExternalSourcesUnspecified = result == [2086] + where + f "dir/myscript" Nothing _ "mylib" = return "resolved/mylib" + f a b c d = error $ show ("Unexpected", a, b, c, d) + result = checkWithRcIncludesAndSourcePath "" [("resolved/mylib", "echo $1")] f emptyCheckSpec { + csScript = "#!/bin/bash\nsource mylib", + csFilename = "dir/myscript", + csCheckSourced = True + } + +prop_fileCanDisableExternalSources = result == [2006, 2086] + where + f "dir/myscript" (Just True) _ "withExternal" = return "withExternal" + f "dir/myscript" (Just False) _ "withoutExternal" = return "withoutExternal" + f a b c d = error $ show ("Unexpected", a, b, c, d) + result = checkWithRcIncludesAndSourcePath "external-sources=true" [("withExternal", "echo $1"), ("withoutExternal", "_=`foo`")] f emptyCheckSpec { + csScript = "#!/bin/bash\ntrue\nsource withExternal\n# shellcheck external-sources=false\nsource withoutExternal", + csFilename = "dir/myscript", + csCheckSourced = True + } + +prop_fileCannotEnableExternalSources = result == [1144] + where + f "dir/myscript" Nothing _ "foo" = return "foo" + f a b c d = error $ show ("Unexpected", a, b, c, d) + result = checkWithRcIncludesAndSourcePath "" [("foo", "true")] f emptyCheckSpec { + csScript = "#!/bin/bash\n# shellcheck external-sources=true\nsource foo", + csFilename = "dir/myscript", + csCheckSourced = True + } + +prop_fileCannotEnableExternalSources2 = result == [1144] + where + f "dir/myscript" (Just False) _ "foo" = return "foo" + f a b c d = error $ show ("Unexpected", a, b, c, d) + result = checkWithRcIncludesAndSourcePath "external-sources=false" [("foo", "true")] f emptyCheckSpec { + csScript = "#!/bin/bash\n# shellcheck external-sources=true\nsource foo", + csFilename = "dir/myscript", + csCheckSourced = True + } + return [] runTests = $quickCheckAll diff --git a/src/ShellCheck/Checks/Commands.hs b/src/ShellCheck/Checks/Commands.hs --- a/src/ShellCheck/Checks/Commands.hs +++ b/src/ShellCheck/Checks/Commands.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2019 Vidar Holen + Copyright 2012-2021 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -57,7 +57,7 @@ commandChecks = [ checkTr ,checkFindNameGlob - ,checkNeedlessExpr + ,checkExpr ,checkGrepRe ,checkTrapQuotes ,checkReturn @@ -95,8 +95,13 @@ ,checkSourceArgs ,checkChmodDashr ,checkXargsDashi + ,checkUnquotedEchoSpaces + ,checkEvalArray ] + ++ map checkArgComparison declaringCommands + ++ map checkMaskedReturns declaringCommands + optionalChecks = map fst optionalCommandChecks optionalCommandChecks :: [(CheckDescription, CommandCheck)] optionalCommandChecks = [ @@ -131,19 +136,31 @@ prop_checkGetOptsS4 = checkGetOpts "-f -x" ["f"] [] $ getOpts (True, True) "f:" [] prop_checkGetOptsS5 = checkGetOpts "-fx" [] [] $ getOpts (True, True) "fx:" [] +prop_checkGenericOptsS1 = checkGetOpts "-f x" ["f"] [] $ return . getGenericOpts +prop_checkGenericOptsS2 = checkGetOpts "-abc x" ["a", "b", "c"] [] $ return . getGenericOpts +prop_checkGenericOptsS3 = checkGetOpts "-abc -x" ["a", "b", "c", "x"] [] $ return . getGenericOpts +prop_checkGenericOptsS4 = checkGetOpts "-x" ["x"] [] $ return . getGenericOpts + -- Long options prop_checkGetOptsL1 = checkGetOpts "--foo=bar baz" ["foo"] ["baz"] $ getOpts (True, False) "" [("foo", True)] prop_checkGetOptsL2 = checkGetOpts "--foo bar baz" ["foo"] ["baz"] $ getOpts (True, False) "" [("foo", True)] prop_checkGetOptsL3 = checkGetOpts "--foo baz" ["foo"] ["baz"] $ getOpts (True, True) "" [] prop_checkGetOptsL4 = checkGetOpts "--foo baz" [] [] $ getOpts (True, False) "" [] +prop_checkGenericOptsL1 = checkGetOpts "--foo=bar" ["foo"] [] $ return . getGenericOpts +prop_checkGenericOptsL2 = checkGetOpts "--foo bar" ["foo"] ["bar"] $ return . getGenericOpts +prop_checkGenericOptsL3 = checkGetOpts "-x --foo" ["x", "foo"] [] $ return . getGenericOpts + -- Know when to terminate prop_checkGetOptsT1 = checkGetOpts "-a x -b" ["a", "b"] ["x"] $ getOpts (True, True) "ab" [] prop_checkGetOptsT2 = checkGetOpts "-a x -b" ["a"] ["x","-b"] $ getOpts (False, True) "ab" [] prop_checkGetOptsT3 = checkGetOpts "-a -- -b" ["a"] ["-b"] $ getOpts (True, True) "ab" [] prop_checkGetOptsT4 = checkGetOpts "-a -- -b" ["a", "b"] [] $ getOpts (True, True) "a:b" [] +prop_checkGenericOptsT1 = checkGetOpts "-x -- -y" ["x"] ["-y"] $ return . getGenericOpts +prop_checkGenericOptsT2 = checkGetOpts "-xy --" ["x", "y"] [] $ return . getGenericOpts + buildCommandMap :: [CommandCheck] -> Map.Map CommandName (Token -> Analysis) buildCommandMap = foldl' addCheck Map.empty where @@ -235,20 +252,75 @@ acc b -prop_checkNeedlessExpr = verify checkNeedlessExpr "foo=$(expr 3 + 2)" -prop_checkNeedlessExpr2 = verify checkNeedlessExpr "foo=`echo \\`expr 3 + 2\\``" -prop_checkNeedlessExpr3 = verifyNot checkNeedlessExpr "foo=$(expr foo : regex)" -prop_checkNeedlessExpr4 = verifyNot checkNeedlessExpr "foo=$(expr foo \\< regex)" -checkNeedlessExpr = CommandCheck (Basename "expr") f where - f t = +prop_checkExpr = verify checkExpr "foo=$(expr 3 + 2)" +prop_checkExpr2 = verify checkExpr "foo=`echo \\`expr 3 + 2\\``" +prop_checkExpr3 = verifyNot checkExpr "foo=$(expr foo : regex)" +prop_checkExpr4 = verifyNot checkExpr "foo=$(expr foo \\< regex)" +prop_checkExpr5 = verify checkExpr "# shellcheck disable=SC2003\nexpr match foo bar" +prop_checkExpr6 = verify checkExpr "# shellcheck disable=SC2003\nexpr foo : fo*" +prop_checkExpr7 = verify checkExpr "# shellcheck disable=SC2003\nexpr 5 -3" +prop_checkExpr8 = verifyNot checkExpr "# shellcheck disable=SC2003\nexpr \"$@\"" +prop_checkExpr9 = verifyNot checkExpr "# shellcheck disable=SC2003\nexpr 5 $rest" +prop_checkExpr10 = verify checkExpr "# shellcheck disable=SC2003\nexpr length \"$var\"" +prop_checkExpr11 = verify checkExpr "# shellcheck disable=SC2003\nexpr foo > bar" +prop_checkExpr12 = verify checkExpr "# shellcheck disable=SC2003\nexpr 1 | 2" +prop_checkExpr13 = verify checkExpr "# shellcheck disable=SC2003\nexpr 1 * 2" +prop_checkExpr14 = verify checkExpr "# shellcheck disable=SC2003\nexpr \"$x\" >= \"$y\"" + +checkExpr = CommandCheck (Basename "expr") f where + f t = do when (all (`notElem` exceptions) (words $ arguments t)) $ style (getId $ getCommandTokenOrThis t) 2003 "expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]]." + + case arguments t of + [lhs, op, rhs] -> do + checkOp lhs + case getWordParts op of + [T_Glob _ "*"] -> + err (getId op) 2304 + "* must be escaped to multiply: \\*. Modern $((x * y)) avoids this issue." + [T_Literal _ ":"] | isGlob rhs -> + warn (getId rhs) 2305 + "Quote regex argument to expr to avoid it expanding as a glob." + _ -> return () + + [single] | not (willSplit single) -> + warn (getId single) 2307 + "'expr' expects 3+ arguments but sees 1. Make sure each operator/operand is a separate argument, and escape <>&|." + + [first, second] | + (fromMaybe "" $ getLiteralString first) /= "length" + && not (willSplit first || willSplit second) -> do + checkOp first + warn (getId t) 2307 + "'expr' expects 3+ arguments, but sees 2. Make sure each operator/operand is a separate argument, and escape <>&|." + + (first:rest) -> do + checkOp first + forM_ rest $ \t -> + -- We already find 95%+ of multiplication and regex earlier, so don't bother classifying this further. + when (isGlob t) $ warn (getId t) 2306 "Escape glob characters in arguments to expr to avoid pathname expansion." + + _ -> return () + -- These operators are hard to replicate in POSIX - exceptions = [ ":", "<", ">", "<=", ">=" ] + exceptions = [ ":", "<", ">", "<=", ">=", + -- We can offer better suggestions for these + "match", "length", "substr", "index"] words = mapMaybe getLiteralString + checkOp side = + case getLiteralString side of + Just "match" -> msg "'expr match' has unspecified results. Prefer 'expr str : regex'." + Just "length" -> msg "'expr length' has unspecified results. Prefer ${#var}." + Just "substr" -> msg "'expr substr' has unspecified results. Prefer 'cut' or ${var#???}." + Just "index" -> msg "'expr index' has unspecified results. Prefer x=${var%%[chars]*}; $((${#x}+1))." + _ -> return () + where + msg = info (getId side) 2308 + prop_checkGrepRe1 = verify checkGrepRe "cat foo | grep *.mp3" prop_checkGrepRe2 = verify checkGrepRe "grep -Ev cow*test *.mp3" prop_checkGrepRe3 = verify checkGrepRe "grep --regex=*.mp3 file" @@ -718,6 +790,7 @@ prop_checkReadExpansions6 = verify checkReadExpansions "read -a $var" prop_checkReadExpansions7 = verifyNot checkReadExpansions "read $1" prop_checkReadExpansions8 = verifyNot checkReadExpansions "read ${var?}" +prop_checkReadExpansions9 = verify checkReadExpansions "read arr[val]" checkReadExpansions = CommandCheck (Exactly "read") check where options = getGnuOpts flagsForRead @@ -725,13 +798,26 @@ opts <- options $ arguments cmd return [y | (x,(_, y)) <- opts, null x || x == "a"] - check cmd = mapM_ warning $ getVars cmd - warning t = sequence_ $ do + check cmd = do + mapM_ dollarWarning $ getVars cmd + mapM_ arrayWarning $ arguments cmd + + dollarWarning t = sequence_ $ do name <- getSingleUnmodifiedBracedString t guard $ isVariableName name -- e.g. not $1 return . warn (getId t) 2229 $ "This does not read '" ++ name ++ "'. Remove $/${} for that, or use ${var?} to quiet." + arrayWarning word = + when (any isUnquotedBracket $ getWordParts word) $ + warn (getId word) 2313 $ + "Quote array indices to avoid them expanding as globs." + + isUnquotedBracket t = + case t of + T_Glob _ ('[':_) -> True + _ -> False + -- Return the single variable expansion that makes up this word, if any. -- e.g. $foo -> $foo, "$foo"'' -> $foo , "hello $name" -> Nothing getSingleUnmodifiedBracedString :: Token -> Maybe String @@ -771,6 +857,9 @@ prop_checkUnsetGlobs1 = verify checkUnsetGlobs "unset foo[1]" prop_checkUnsetGlobs2 = verifyNot checkUnsetGlobs "unset foo" +prop_checkUnsetGlobs3 = verify checkUnsetGlobs "unset foo[$i]" +prop_checkUnsetGlobs4 = verify checkUnsetGlobs "unset foo[x${i}y]" +prop_checkUnsetGlobs5 = verifyNot checkUnsetGlobs "unset foo][" checkUnsetGlobs = CommandCheck (Exactly "unset") (mapM_ check . arguments) where check arg = @@ -870,15 +959,29 @@ prop_checkWhileGetoptsCase3 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do case $x in a) foo;; b) bar;; *) :;esac; done" prop_checkWhileGetoptsCase4 = verifyNot checkWhileGetoptsCase "while getopts 'a:123' x; do case $x in a) foo;; [0-9]) bar;; esac; done" prop_checkWhileGetoptsCase5 = verifyNot checkWhileGetoptsCase "while getopts 'a:' x; do case $x in a) foo;; \\?) bar;; *) baz;; esac; done" +prop_checkWhileGetoptsCase6 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do case $y in a) foo;; esac; done" +prop_checkWhileGetoptsCase7 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do case x$x in xa) foo;; xb) foo;; esac; done" +prop_checkWhileGetoptsCase8 = verifyNot checkWhileGetoptsCase "while getopts 'a:b' x; do x=a; case $x in a) foo;; esac; done" checkWhileGetoptsCase = CommandCheck (Exactly "getopts") f where f :: Token -> Analysis - f t@(T_SimpleCommand _ _ (cmd:arg1:_)) = do + f t@(T_SimpleCommand _ _ (cmd:arg1:name:_)) = do path <- getPathM t + params <- ask sequence_ $ do options <- getLiteralString arg1 + getoptsVar <- getLiteralString name (T_WhileExpression _ _ body) <- findFirst whileLoop path - caseCmd <- mapMaybe findCase body !!! 0 + caseCmd@(T_CaseExpression _ var _) <- mapMaybe findCase body !!! 0 + + -- Make sure getopts name and case variable matches + [T_DollarBraced _ _ bracedWord] <- return $ getWordParts var + [T_Literal _ caseVar] <- return $ getWordParts bracedWord + guard $ caseVar == getoptsVar + + -- Make sure the variable isn't modified + guard . not $ modifiesVariable params (T_BraceGroup (Id 0) body) getoptsVar + return $ check (getId arg1) (map (:[]) $ filter (/= ':') options) caseCmd f _ = return () @@ -1142,6 +1245,146 @@ (option, value) <- lookup "i" opts return $ info (getId option) 2267 "GNU xargs -i is deprecated in favor of -I{}" parseOpts = getBsdOpts "0oprtxadR:S:J:L:l:n:P:s:e:E:i:I:" + + +prop_checkArgComparison1 = verify (checkArgComparison "declare") "declare a = b" +prop_checkArgComparison2 = verify (checkArgComparison "declare") "declare a =b" +prop_checkArgComparison3 = verifyNot (checkArgComparison "declare") "declare a=b" +prop_checkArgComparison4 = verify (checkArgComparison "export") "export a +=b" +prop_checkArgComparison7 = verifyNot (checkArgComparison "declare") "declare -a +i foo" +prop_checkArgComparison8 = verify (checkArgComparison "let") "let x = 0" +-- This mirrors checkSecondArgIsComparison but for arguments to local/readonly/declare/export +checkArgComparison cmd = CommandCheck (Exactly cmd) wordsWithEqual + where + wordsWithEqual t = mapM_ check $ arguments t + check arg = do + sequence_ $ do + str <- getLeadingUnquotedString arg + case str of + '=':_ -> + return $ err (headId arg) 2290 $ + "Remove spaces around = to assign." + '+':'=':_ -> + return $ err (headId arg) 2290 $ + "Remove spaces around += to append." + _ -> Nothing + + -- 'let' is parsed as a sequence of arithmetic expansions, + -- so we want the additional warning for "x=" + when (cmd == "let") $ sequence_ $ do + token <- getTrailingUnquotedLiteral arg + str <- getLiteralString token + guard $ "=" `isSuffixOf` str + return $ err (getId token) 2290 $ + "Remove spaces around = to assign." + + headId t = + case t of + T_NormalWord _ (x:_) -> getId x + _ -> getId t + + +prop_checkMaskedReturns1 = verify (checkMaskedReturns "local") "f() { local a=$(false); }" +prop_checkMaskedReturns2 = verify (checkMaskedReturns "declare") "declare a=$(false)" +prop_checkMaskedReturns3 = verify (checkMaskedReturns "declare") "declare a=\"`false`\"" +prop_checkMaskedReturns4 = verify (checkMaskedReturns "readonly") "readonly a=$(false)" +prop_checkMaskedReturns5 = verify (checkMaskedReturns "readonly") "readonly a=\"`false`\"" +prop_checkMaskedReturns6 = verifyNot (checkMaskedReturns "declare") "declare a; a=$(false)" +prop_checkMaskedReturns7 = verifyNot (checkMaskedReturns "local") "f() { local -r a=$(false); }" +prop_checkMaskedReturns8 = verifyNot (checkMaskedReturns "readonly") "a=$(false); readonly a" +prop_checkMaskedReturns9 = verify (checkMaskedReturns "typeset") "#!/bin/ksh\n f() { typeset -r x=$(false); }" +prop_checkMaskedReturns10 = verifyNot (checkMaskedReturns "typeset") "#!/bin/ksh\n function f { typeset -r x=$(false); }" +prop_checkMaskedReturns11 = verifyNot (checkMaskedReturns "typeset") "#!/bin/bash\n f() { typeset -r x=$(false); }" +prop_checkMaskedReturns12 = verify (checkMaskedReturns "typeset") "typeset -r x=$(false);" +prop_checkMaskedReturns13 = verify (checkMaskedReturns "typeset") "f() { typeset -g x=$(false); }" +prop_checkMaskedReturns14 = verify (checkMaskedReturns "declare") "declare x=${ false; }" +prop_checkMaskedReturns15 = verify (checkMaskedReturns "declare") "f() { declare x=$(false); }" +checkMaskedReturns str = CommandCheck (Exactly str) checkCmd + where + checkCmd t = do + path <- getPathM t + shell <- asks shellType + sequence_ $ do + name <- getCommandName t + + let flags = map snd (getAllFlags t) + let hasDashR = "r" `elem` flags + let hasDashG = "g" `elem` flags + let isInScopedFunction = any (isScopedFunction shell) path + + let isLocal = not hasDashG && isLocalInFunction name && isInScopedFunction + let isReadOnly = name == "readonly" || hasDashR + + -- Don't warn about local variables that are declared readonly, + -- because the workaround `local x; x=$(false); local -r x;` is annoying + guard . not $ isLocal && isReadOnly + + return $ mapM_ checkArgs $ arguments t + + checkArgs (T_Assignment id _ _ _ word) | any hasReturn $ getWordParts word = + warn id 2155 "Declare and assign separately to avoid masking return values." + checkArgs _ = return () + + isLocalInFunction = (`elem` ["local", "declare", "typeset"]) + isScopedFunction shell t = + case t of + T_BatsTest {} -> True + -- In ksh, only functions declared with 'function' have their own scope + T_Function _ (FunctionKeyword hasFunction) _ _ _ -> shell /= Ksh || hasFunction + _ -> False + + hasReturn t = case t of + T_Backticked {} -> True + T_DollarExpansion {} -> True + T_DollarBraceCommandExpansion {} -> True + _ -> False + + +prop_checkUnquotedEchoSpaces1 = verify checkUnquotedEchoSpaces "echo foo bar" +prop_checkUnquotedEchoSpaces2 = verifyNot checkUnquotedEchoSpaces "echo foo" +prop_checkUnquotedEchoSpaces3 = verifyNot checkUnquotedEchoSpaces "echo foo bar" +prop_checkUnquotedEchoSpaces4 = verifyNot checkUnquotedEchoSpaces "echo 'foo bar'" +prop_checkUnquotedEchoSpaces5 = verifyNot checkUnquotedEchoSpaces "echo a > myfile.txt b" +prop_checkUnquotedEchoSpaces6 = verifyNot checkUnquotedEchoSpaces " echo foo\\\n bar" +checkUnquotedEchoSpaces = CommandCheck (Basename "echo") check + where + check t = do + let args = arguments t + m <- asks tokenPositions + redir <- getClosestCommandM t + sequence_ $ do + let positions = mapMaybe (\c -> Map.lookup (getId c) m) args + let pairs = zip positions (drop 1 positions) + (T_Redirecting _ redirTokens _) <- redir + let redirPositions = mapMaybe (\c -> fst <$> Map.lookup (getId c) m) redirTokens + guard $ any (hasSpacesBetween redirPositions) pairs + return $ info (getId t) 2291 "Quote repeated spaces to avoid them collapsing into one." + + hasSpacesBetween redirs ((a,b), (c,d)) = + posLine a == posLine d + && ((posColumn c) - (posColumn b)) >= 4 + && not (any (\x -> b < x && x < c) redirs) + + +prop_checkEvalArray1 = verify checkEvalArray "eval $@" +prop_checkEvalArray2 = verify checkEvalArray "eval \"${args[@]}\"" +prop_checkEvalArray3 = verify checkEvalArray "eval \"${args[@]@Q}\"" +prop_checkEvalArray4 = verifyNot checkEvalArray "eval \"${args[*]@Q}\"" +prop_checkEvalArray5 = verifyNot checkEvalArray "eval \"$*\"" +checkEvalArray = CommandCheck (Exactly "eval") (mapM_ check . concatMap getWordParts . arguments) + where + check t = + when (isArrayExpansion t) $ + if isEscaped t + then style (getId t) 2293 "When eval'ing @Q-quoted words, use * rather than @ as the index." + else warn (getId t) 2294 "eval negates the benefit of arrays. Drop eval to preserve whitespace/symbols (or eval as string)." + + isEscaped q = + case q of + -- Match ${arr[@]@Q} and ${@@Q} and such + T_DollarBraced _ _ l -> 'Q' `elem` getBracedModifier (concat $ oversimplify l) + _ -> False + return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/src/ShellCheck/Checks/ShellSupport.hs b/src/ShellCheck/Checks/ShellSupport.hs --- a/src/ShellCheck/Checks/ShellSupport.hs +++ b/src/ShellCheck/Checks/ShellSupport.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2016 Vidar Holen + Copyright 2012-2020 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -180,7 +180,7 @@ prop_checkBashisms96 = verifyNot checkBashisms "#!/bin/dash\necho $_" prop_checkBashisms97 = verify checkBashisms "#!/bin/sh\necho ${var,}" prop_checkBashisms98 = verify checkBashisms "#!/bin/sh\necho ${var^^}" -prop_checkBashisms99 = verifyNot checkBashisms "#!/bin/dash\necho [^f]oo" +prop_checkBashisms99 = verify checkBashisms "#!/bin/dash\necho [^f]oo" checkBashisms = ForShell [Sh, Dash] $ \t -> do params <- ask kludge params t @@ -235,7 +235,7 @@ where file = onlyLiteralString word isNetworked = any (`isPrefixOf` file) ["/dev/tcp", "/dev/udp"] - bashism (T_Glob id str) | not isDash && "[^" `isInfixOf` str = + bashism (T_Glob id str) | "[^" `isInfixOf` str = warnMsg id 3026 "^ in place of ! in glob bracket expressions is" bashism t@(TA_Variable id str _) | isBashVariable str = diff --git a/src/ShellCheck/Data.hs b/src/ShellCheck/Data.hs --- a/src/ShellCheck/Data.hs +++ b/src/ShellCheck/Data.hs @@ -4,7 +4,7 @@ import Data.Version (showVersion) import Paths_ShellCheck (version) -shellcheckVersion = showVersion version +shellcheckVersion = showVersion version -- VERSIONSTRING internalVariables = [ -- Generic @@ -138,3 +138,5 @@ _ -> Nothing flagsForRead = "sreu:n:N:i:p:a:t:" + +declaringCommands = ["local", "declare", "export", "readonly", "typeset", "let"] diff --git a/src/ShellCheck/Formatter/CheckStyle.hs b/src/ShellCheck/Formatter/CheckStyle.hs --- a/src/ShellCheck/Formatter/CheckStyle.hs +++ b/src/ShellCheck/Formatter/CheckStyle.hs @@ -48,7 +48,7 @@ fileGroups = groupWith sourceFile comments outputGroup group = do let filename = sourceFile (head group) - result <- (siReadFile sys) filename + result <- siReadFile sys (Just True) filename let contents = either (const "") id result outputFile filename contents group diff --git a/src/ShellCheck/Formatter/Diff.hs b/src/ShellCheck/Formatter/Diff.hs --- a/src/ShellCheck/Formatter/Diff.hs +++ b/src/ShellCheck/Formatter/Diff.hs @@ -38,9 +38,6 @@ import Test.QuickCheck -import Debug.Trace -ltt x = trace (show x) x - format :: FormatterOptions -> IO Formatter format options = do foundIssues <- newIORef False @@ -90,7 +87,7 @@ mapM_ output $ M.toList fixmap where output (name, fix) = do - file <- (siReadFile sys) name + file <- siReadFile sys (Just True) name case file of Right contents -> do putStrLn $ formatDoc color $ makeDiff name contents fix diff --git a/src/ShellCheck/Formatter/Format.hs b/src/ShellCheck/Formatter/Format.hs --- a/src/ShellCheck/Formatter/Format.hs +++ b/src/ShellCheck/Formatter/Format.hs @@ -28,6 +28,7 @@ import Data.List import System.IO import System.Info +import System.Environment -- A formatter that carries along an arbitrary piece of data data Formatter = Formatter { @@ -68,12 +69,14 @@ shouldOutputColor :: ColorOption -> IO Bool -shouldOutputColor colorOption = do - term <- hIsTerminalDevice stdout - let windows = "mingw" `isPrefixOf` os - let isUsableTty = term && not windows - let useColor = case colorOption of - ColorAlways -> True - ColorNever -> False - ColorAuto -> isUsableTty - return useColor +shouldOutputColor colorOption = + case colorOption of + ColorAlways -> return True + ColorNever -> return False + ColorAuto -> do + isTerminal <- hIsTerminalDevice stdout + term <- lookupEnv "TERM" + let windows = "mingw" `isPrefixOf` os + let dumbTerm = term `elem` [Just "dumb", Just "", Nothing] + let isUsableTty = isTerminal && not windows && not dumbTerm + return isUsableTty diff --git a/src/ShellCheck/Formatter/GCC.hs b/src/ShellCheck/Formatter/GCC.hs --- a/src/ShellCheck/Formatter/GCC.hs +++ b/src/ShellCheck/Formatter/GCC.hs @@ -43,7 +43,7 @@ f :: [PositionedComment] -> IO () f group = do let filename = sourceFile (head group) - result <- (siReadFile sys) filename + result <- siReadFile sys (Just True) filename let contents = either (const "") id result outputResult filename contents group diff --git a/src/ShellCheck/Formatter/JSON1.hs b/src/ShellCheck/Formatter/JSON1.hs --- a/src/ShellCheck/Formatter/JSON1.hs +++ b/src/ShellCheck/Formatter/JSON1.hs @@ -117,7 +117,7 @@ f :: [PositionedComment] -> IO () f group = do let filename = sourceFile (head group) - result <- siReadFile sys filename + result <- siReadFile sys (Just True) filename let contents = either (const "") id result let comments' = makeNonVirtual comments contents modifyIORef ref (\x -> comments' ++ x) diff --git a/src/ShellCheck/Formatter/TTY.hs b/src/ShellCheck/Formatter/TTY.hs --- a/src/ShellCheck/Formatter/TTY.hs +++ b/src/ShellCheck/Formatter/TTY.hs @@ -121,7 +121,7 @@ outputForFile color sys comments = do let fileName = sourceFile (head comments) - result <- (siReadFile sys) fileName + result <- siReadFile sys (Just True) fileName let contents = either (const "") id result let fileLinesList = lines contents let lineCount = length fileLinesList @@ -174,7 +174,7 @@ cuteIndent :: PositionedComment -> String cuteIndent comment = replicate (fromIntegral $ colNo comment - 1) ' ' ++ - makeArrow ++ " " ++ code (codeNo comment) ++ ": " ++ messageText comment + makeArrow ++ " " ++ code (codeNo comment) ++ " (" ++ severityText comment ++ "): " ++ messageText comment where arrow n = '^' : replicate (fromIntegral $ n-2) '-' ++ "^" makeArrow = diff --git a/src/ShellCheck/Interface.hs b/src/ShellCheck/Interface.hs --- a/src/ShellCheck/Interface.hs +++ b/src/ShellCheck/Interface.hs @@ -73,14 +73,18 @@ data SystemInterface m = SystemInterface { - -- | Read a file by filename, or return an error - siReadFile :: String -> m (Either ErrorMessage String), -- | Given: + -- What annotations say about including external files (if anything) + -- A resolved filename from siFindSource + -- Read the file or return an error + siReadFile :: Maybe Bool -> String -> m (Either ErrorMessage String), + -- | Given: -- the current script, + -- what annotations say about including external files (if anything) -- a list of source-path annotations in effect, -- and a sourced file, -- find the sourced file - siFindSource :: String -> [String] -> String -> m FilePath, + siFindSource :: String -> Maybe Bool -> [String] -> String -> m FilePath, -- | Get the configuration file (name, contents) for a filename siGetConfig :: String -> m (Maybe (FilePath, String)) } @@ -313,11 +317,11 @@ siGetConfig = const $ return Nothing } where - rf file = return $ + rf _ file = return $ case find ((== file) . fst) files of Nothing -> Left "File not included in mock." Just (_, contents) -> Right contents - fs _ _ file = return file + fs _ _ _ file = return file mockRcFile rcfile mock = mock { siGetConfig = const . return $ Just (".shellcheckrc", rcfile) diff --git a/src/ShellCheck/Parser.hs b/src/ShellCheck/Parser.hs --- a/src/ShellCheck/Parser.hs +++ b/src/ShellCheck/Parser.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2019 Vidar Holen + Copyright 2012-2021 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -37,7 +37,7 @@ import Data.List (isPrefixOf, isInfixOf, isSuffixOf, partition, sortBy, intercalate, nub, find) import Data.Maybe import Data.Monoid -import Debug.Trace +import Debug.Trace -- STRIP import GHC.Exts (sortWith) import Prelude hiding (readList) import System.IO @@ -66,7 +66,7 @@ variableStart = upper <|> lower <|> oneOf "_" variableChars = upper <|> lower <|> digit <|> oneOf "_" -- Chars to allow in function names -functionChars = variableChars <|> oneOf ":+?-./^@" +functionChars = variableChars <|> oneOf ":+?-./^@," -- Chars to allow in functions using the 'function' keyword extendedFunctionChars = functionChars <|> oneOf "[]*=!" specialVariable = oneOf (concat specialVariables) @@ -984,12 +984,13 @@ prop_readAnnotation5 = isOk readAnnotation "# shellcheck disable=SC2002 # All cats are precious\n" prop_readAnnotation6 = isOk readAnnotation "# shellcheck disable=SC1234 # shellcheck foo=bar\n" prop_readAnnotation7 = isOk readAnnotation "# shellcheck disable=SC1000,SC2000-SC3000,SC1001\n" +prop_readAnnotation8 = isOk readAnnotation "# shellcheck disable=all\n" readAnnotation = called "shellcheck directive" $ do try readAnnotationPrefix many1 linewhitespace - readAnnotationWithoutPrefix + readAnnotationWithoutPrefix True -readAnnotationWithoutPrefix = do +readAnnotationWithoutPrefix sandboxed = do values <- many1 readKey optional readAnyComment void linefeed <|> eof <|> do @@ -1004,8 +1005,12 @@ key <- many1 (letter <|> char '-') char '=' <|> fail "Expected '=' after directive key" annotations <- case key of - "disable" -> readRange `sepBy` char ',' + "disable" -> readElement `sepBy` char ',' where + readElement = readRange <|> readAll + readAll = do + string "all" + return $ DisableComment 0 1000000 readRange = do from <- readCode to <- choice [ char '-' *> readCode, return $ from+1 ] @@ -1035,6 +1040,21 @@ "This shell type is unknown. Use e.g. sh or bash." return [ShellOverride shell] + "external-sources" -> do + pos <- getPosition + value <- many1 letter + case value of + "true" -> + if sandboxed + then do + parseNoteAt pos ErrorC 1144 "external-sources can only be enabled in .shellcheckrc, not in individual files." + return [] + else return [ExternalSources True] + "false" -> return [ExternalSources False] + _ -> do + parseNoteAt pos ErrorC 1145 "Unknown external-sources value. Expected true/false." + return [] + _ -> do parseNoteAt keyPos WarningC 1107 "This directive is unknown. It will be ignored." anyChar `reluctantlyTill` whitespace @@ -1372,6 +1392,8 @@ prop_readGlob6 = isOk readGlob "[\\|]" prop_readGlob7 = isOk readGlob "[^[]" prop_readGlob8 = isOk readGlob "[*?]" +prop_readGlob9 = isOk readGlob "[!]^]" +prop_readGlob10 = isOk readGlob "[]]" readGlob = readExtglob <|> readSimple <|> readClass <|> readGlobbyLiteral where readSimple = do @@ -1379,22 +1401,25 @@ c <- oneOf "*?" id <- endSpan start return $ T_Glob id [c] - -- Doesn't handle weird things like [^]a] and [$foo]. fixme? readClass = try $ do start <- startSpan char '[' - s <- many1 (predefined <|> readNormalLiteralPart "]" <|> globchars) + negation <- charToString (oneOf "!^") <|> return "" + leadingBracket <- charToString (oneOf "]") <|> return "" + s <- many (predefined <|> readNormalLiteralPart "]" <|> globchars) + guard $ not (null leadingBracket) || not (null s) char ']' id <- endSpan start - return $ T_Glob id $ "[" ++ concat s ++ "]" + return $ T_Glob id $ "[" ++ concat (negation:leadingBracket:s) ++ "]" where - globchars = fmap return . oneOf $ "!$[" ++ extglobStartChars + globchars = charToString $ oneOf $ "![" ++ extglobStartChars predefined = do try $ string "[:" s <- many1 letter string ":]" return $ "[:" ++ s ++ ":]" + charToString = fmap return readGlobbyLiteral = do start <- startSpan c <- extglobStart <|> char '[' @@ -1486,7 +1511,6 @@ case x of '\'' -> parseProblemAt pos InfoC 1003 "Want to escape a single quote? echo 'This is how it'\\''s done'."; - '\n' -> parseProblemAt pos InfoC 1004 "This backslash+linefeed is literal. Break outside single quotes if you just want to break the line." _ -> return () return [s] @@ -1996,12 +2020,14 @@ word <- readNormalWord return $ T_HereString id word +prop_readNewlineList1 = isOk readScript "&> /dev/null echo foo" readNewlineList = many1 ((linefeed <|> carriageReturn) `thenSkip` spacing) <* checkBadBreak where checkBadBreak = optional $ do pos <- getPosition try $ lookAhead (oneOf "|&") -- See if the next thing could be |, || or && + notFollowedBy2 (string "&>") -- Except &> or &>> which is valid parseProblemAt pos ErrorC 1133 "Unexpected start of line. If breaking lines, |/||/&& should be at the end of the previous one." readLineBreak = optional readNewlineList @@ -2061,6 +2087,7 @@ prop_readSimpleCommand5 = isOk readSimpleCommand "time if true; then echo foo; fi" prop_readSimpleCommand6 = isOk readSimpleCommand "time -p ( ls -l; )" prop_readSimpleCommand7 = isOk readSimpleCommand "\\ls" +prop_readSimpleCommand7b = isOk readSimpleCommand "\\:" prop_readSimpleCommand8 = isWarning readSimpleCommand "// Lol" prop_readSimpleCommand9 = isWarning readSimpleCommand "/* Lolbert */" prop_readSimpleCommand10 = isWarning readSimpleCommand "/**** Lolbert */" @@ -2168,10 +2195,12 @@ if filename == "/dev/null" -- always allow /dev/null then return (Right "", filename) else do + allAnnotations <- getCurrentAnnotations True currentScript <- Mr.asks currentFilename - paths <- mapMaybe getSourcePath <$> getCurrentAnnotations True - resolved <- system $ siFindSource sys currentScript paths filename - contents <- system $ siReadFile sys resolved + let paths = mapMaybe getSourcePath allAnnotations + let externalSources = listToMaybe $ mapMaybe getExternalSources allAnnotations + resolved <- system $ siFindSource sys currentScript externalSources paths filename + contents <- system $ siReadFile sys externalSources resolved return (contents, resolved) case input of Left err -> do @@ -2205,6 +2234,11 @@ SourcePath x -> Just x _ -> Nothing + getExternalSources t = + case t of + ExternalSources b -> Just b + _ -> Nothing + -- If the word has a single expansion as the directory, try stripping it -- This affects `$foo/bar` but not `${foo}-dir/bar` or `/foo/$file` stripDynamicPrefix word = @@ -2321,7 +2355,7 @@ -- Ignore alias suppression optional . try $ do char '\\' - lookAhead $ variableChars + lookAhead $ variableChars <|> oneOf ":." readCmdWord readCmdWord = do @@ -2807,7 +2841,7 @@ startPos <- getPosition expression <- readStringForParser readCmdWord let (unQuoted, newPos) = kludgeAwayQuotes expression startPos - subParse newPos readArithmeticContents unQuoted + subParse newPos (readArithmeticContents <* eof) unQuoted kludgeAwayQuotes :: String -> SourcePos -> (String, SourcePos) kludgeAwayQuotes s p = @@ -3194,7 +3228,7 @@ prop_readConfigKVs5 = isOk readConfigKVs "# shellcheck accepts annotation-like comments in rc files\ndisable=1234" readConfigKVs = do anySpacingOrComment - annotations <- many (readAnnotationWithoutPrefix <* anySpacingOrComment) + annotations <- many (readAnnotationWithoutPrefix False <* anySpacingOrComment) eof return $ concat annotations anySpacingOrComment = @@ -3338,13 +3372,13 @@ -- For printf debugging: print the value of an expression -- Example: return $ dump $ T_Literal id [c] -dump :: Show a => a -> a -dump x = trace (show x) x +dump :: Show a => a -> a -- STRIP +dump x = trace (show x) x -- STRIP -- Like above, but print a specific expression: -- Example: return $ dumps ("Returning: " ++ [c]) $ T_Literal id [c] -dumps :: Show x => x -> a -> a -dumps t = trace (show t) +dumps :: Show x => x -> a -> a -- STRIP +dumps t = trace (show t) -- STRIP parseWithNotes parser = do item <- parser diff --git a/striptests b/striptests --- a/striptests +++ b/striptests @@ -29,6 +29,7 @@ state = 0; } + /STRIP/ { next; } /LANGUAGE TemplateHaskell/ { next; } /^import.*Test\./ { next; } @@ -75,4 +76,3 @@ do modify "$file" detestify done -