diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -8,45 +8,45 @@ The goals of ShellCheck are -- To point out and clarify typical beginner's syntax issues that cause a shell +* To point out and clarify typical beginner's syntax issues that cause a shell to give cryptic error messages. -- To point out and clarify typical intermediate level semantic problems that +* To point out and clarify typical intermediate level semantic problems that cause a shell to behave strangely and counter-intuitively. -- To point out subtle caveats, corner cases and pitfalls that may cause an +* To point out subtle caveats, corner cases and pitfalls that may cause an advanced user's otherwise working script to fail under future circumstances. See [the gallery of bad code](README.md#user-content-gallery-of-bad-code) for examples of what ShellCheck can help you identify! ## Table of Contents -- [How to use](#how-to-use) - - [On the web](#on-the-web) - - [From your terminal](#from-your-terminal) - - [In your editor](#in-your-editor) - - [In your build or test suites](#in-your-build-or-test-suites) -- [Installing](#installing) -- [Travis CI](#travis-ci) -- [Compiling from source](#compiling-from-source) - - [Installing Cabal](#installing-cabal) - - [Compiling ShellCheck](#compiling-shellcheck) - - [Running tests](#running-tests) -- [Gallery of bad code](#gallery-of-bad-code) - - [Quoting](#quoting) - - [Conditionals](#conditionals) - - [Frequently misused commands](#frequently-misused-commands) - - [Common beginner's mistakes](#common-beginners-mistakes) - - [Style](#style) - - [Data and typing errors](#data-and-typing-errors) - - [Robustness](#robustness) - - [Portability](#portability) - - [Miscellaneous](#miscellaneous) -- [Testimonials](#testimonials) -- [Ignoring issues](#ignoring-issues) -- [Reporting bugs](#reporting-bugs) -- [Contributing](#contributing) -- [Copyright](#copyright) +* [How to use](#how-to-use) + * [On the web](#on-the-web) + * [From your terminal](#from-your-terminal) + * [In your editor](#in-your-editor) + * [In your build or test suites](#in-your-build-or-test-suites) +* [Installing](#installing) +* [Compiling from source](#compiling-from-source) + * [Installing Cabal](#installing-cabal) + * [Compiling ShellCheck](#compiling-shellcheck) + * [Running tests](#running-tests) +* [Gallery of bad code](#gallery-of-bad-code) + * [Quoting](#quoting) + * [Conditionals](#conditionals) + * [Frequently misused commands](#frequently-misused-commands) + * [Common beginner's mistakes](#common-beginners-mistakes) + * [Style](#style) + * [Data and typing errors](#data-and-typing-errors) + * [Robustness](#robustness) + * [Portability](#portability) + * [Miscellaneous](#miscellaneous) +* [Testimonials](#testimonials) +* [Ignoring issues](#ignoring-issues) +* [Reporting bugs](#reporting-bugs) +* [Contributing](#contributing) +* [Copyright](#copyright) +* [Other Resources](#other-resources) ## How to use @@ -54,7 +54,7 @@ ### On the web -Paste a shell script on https://www.shellcheck.net for instant feedback. +Paste a shell script on for instant feedback. [ShellCheck.net](https://www.shellcheck.net) is always synchronized to the latest git commit, and is the easiest way to give ShellCheck a go. Tell your friends! @@ -85,9 +85,46 @@ ### In your build or test suites While ShellCheck is mostly intended for interactive use, it can easily be added to builds or test suites. +It makes canonical use of exit codes, so you can just add a `shellcheck` command as part of the process. -ShellCheck makes canonical use of exit codes, and can output simple JSON, CheckStyle compatible XML, GCC compatible warnings as well as human readable text (with or without ANSI colors). See the [Integration](https://github.com/koalaman/shellcheck/wiki/Integration) wiki page for more documentation. +For example, in a Makefile: +```Makefile +check-scripts: + # Fail if any of these files have warnings + shellcheck myscripts/*.sh +``` + +or in a Travis CI `.travis.yml` file: + +```yaml +script: + # Fail if any of these files have warnings + - shellcheck myscripts/*.sh +``` + +Services and platforms that have ShellCheck pre-installed and ready to use: + +* [Travis CI](https://travis-ci.org/) +* [Codacy](https://www.codacy.com/) +* [Code Climate](https://codeclimate.com/) +* [Code Factor](https://www.codefactor.io/) + +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-the-shellcheck-binary). + +It's a good idea to manually install a specific ShellCheck version regardless. This avoids +any surprise build breaks when a new version with new warnings is published. + +For customized filtering or reporting, ShellCheck can output simple JSON, CheckStyle compatible XML, +GCC compatible warnings as well as human readable text (with or without ANSI colors). See the +[Integration](https://github.com/koalaman/shellcheck/wiki/Integration) wiki page for more documentation. + ## Installing The easiest way to install ShellCheck locally is through your package manager. @@ -141,16 +178,24 @@ zypper in ShellCheck -Or use OneClickInstall - https://software.opensuse.org/package/ShellCheck +Or use OneClickInstall - On Solus: eopkg install shellcheck - -On Windows (via [scoop](http://scoop.sh)): - scoop install shellcheck +On Windows (via [chocolatey](https://chocolatey.org/packages/shellcheck)): +```cmd +C:\> choco install shellcheck +``` + +Or Windows (via [scoop](http://scoop.sh)): + +```cmd +C:\> scoop install shellcheck +``` + From Snap Store: snap install --channel=edge shellcheck @@ -158,8 +203,8 @@ From Docker Hub: ```sh -docker pull koalaman/shellcheck:stable # Or :v0.4.7 for that version, or :latest for daily builds -docker run -v "$PWD:/mnt" koalaman/shellcheck myscript +docker run --rm -v "$PWD:/mnt" koalaman/shellcheck:stable myscript +# Or :v0.4.7 for that version, or :latest for daily builds ``` or use `koalaman/shellcheck-alpine` if you want a larger Alpine Linux based image to extend. It works exactly like a regular Alpine image, but has shellcheck preinstalled. @@ -168,32 +213,39 @@ * [Linux, x86_64](https://storage.googleapis.com/shellcheck/shellcheck-stable.linux.x86_64.tar.xz) (statically linked) * [Linux, armv6hf](https://storage.googleapis.com/shellcheck/shellcheck-stable.linux.armv6hf.tar.xz), i.e. Raspberry Pi (statically linked) +* [Linux, aarch64](https://storage.googleapis.com/shellcheck/shellcheck-stable.linux.armv6hf.tar.xz) aka ARM64 (statically linked) +* [MacOS, x86_64](https://shellcheck.storage.googleapis.com/shellcheck-stable.darwin.x86_64.tar.xz) * [Windows, x86](https://storage.googleapis.com/shellcheck/shellcheck-stable.zip) or see the [storage bucket listing](https://shellcheck.storage.googleapis.com/index.html) for checksums, older versions and the latest daily builds. Distro packages already come with a `man` page. If you are building from source, it can be installed with: - pandoc -s -t man shellcheck.1.md -o shellcheck.1 - sudo mv shellcheck.1 /usr/share/man/man1 +```console +pandoc -s -f markdown-smart -t man shellcheck.1.md -o shellcheck.1 +sudo mv shellcheck.1 /usr/share/man/man1 +``` -## Travis CI +### Travis CI Travis CI has now integrated ShellCheck by default, so you don't need to manually install it. -If you still want to do so in order to upgrade at your leisure or ensure the latest release, follow the steps to install the shellcheck binary, bellow. +If you still want to do so in order to upgrade at your leisure or ensure you're +using the latest release, follow the steps below to install a binary version. -## Installing the shellcheck binary +### Installing a pre-compiled binary -*Pre-requisite*: the program 'xz' needs to be installed on the system. -To install it on debian/ubuntu/linux mint, run `apt install xz-utils`. -To install it on Redhat/Fedora/CentOS, run `yum -y install xz`. +The pre-compiled binaries come in `tar.xz` files. To decompress them, make sure +`xz` is installed. +On Debian/Ubuntu/Mint, you can `apt install xz-utils`. +On Redhat/Fedora/CentOS, `yum -y install xz`. +A simple installer may do something like: + ```bash -export scversion="stable" # or "v0.4.7", or "latest" -wget "https://storage.googleapis.com/shellcheck/shellcheck-${scversion}.linux.x86_64.tar.xz" -tar --xz -xvf shellcheck-"${scversion}".linux.x86_64.tar.xz -cp shellcheck-"${scversion}"/shellcheck /usr/bin/ +scversion="stable" # or "v0.4.7", or "latest" +wget -qO- "https://storage.googleapis.com/shellcheck/shellcheck-${scversion?}.linux.x86_64.tar.xz" | tar -xJv +cp "shellcheck-${scversion}/shellcheck" /usr/bin/ shellcheck --version ``` @@ -207,11 +259,9 @@ On MacOS (OS X), you can do a fast install of Cabal using brew, which takes a couple of minutes instead of more than 30 minutes if you try to compile it from source. - brew install cask - brew cask install haskell-platform - cabal install cabal-install + $ brew install cabal-install -On MacPorts, the package is instead called `hs-cabal-install`, while native Windows users should install the latest version of the Haskell platform from https://www.haskell.org/platform/ +On MacPorts, the package is instead called `hs-cabal-install`, while native Windows users should install the latest version of the Haskell platform from Verify that `cabal` is installed and update its dependency list with @@ -247,12 +297,15 @@ make sure to use a TrueType font, not a Raster font, and set the active codepage to UTF-8 (65001) with `chcp`: - > chcp 65001 - Active code page: 65001 +```cmd +chcp 65001 +``` In Powershell ISE, you may need to additionally update the output encoding: - > [Console]::OutputEncoding = [System.Text.Encoding]::UTF8 +```powershell +[Console]::OutputEncoding = [System.Text.Encoding]::UTF8 +``` ### Running tests @@ -430,13 +483,13 @@ Issues can be ignored via environmental variable, command line, individually or globally within a file: -https://github.com/koalaman/shellcheck/wiki/Ignore + ## Reporting bugs Please use the GitHub issue tracker for any bugs or feature suggestions: -https://github.com/koalaman/shellcheck/issues + ## Contributing @@ -451,11 +504,12 @@ ShellCheck is licensed under the GNU General Public License, v3. A copy of this license is included in the file [LICENSE](LICENSE). -Copyright 2012-2018, Vidar 'koala_man' Holen and contributors. +Copyright 2012-2019, [Vidar 'koala_man' Holen](https://github.com/koalaman/) and contributors. Happy ShellChecking! +## Other Resources -## Other Resources * The wiki has [long form descriptions](https://github.com/koalaman/shellcheck/wiki/Checks) for each warning, e.g. [SC2221](https://github.com/koalaman/shellcheck/wiki/SC2221). * ShellCheck does not attempt to enforce any kind of formatting or indenting style, so also check out [shfmt](https://github.com/mvdan/sh)! + diff --git a/ShellCheck.cabal b/ShellCheck.cabal --- a/ShellCheck.cabal +++ b/ShellCheck.cabal @@ -1,5 +1,5 @@ Name: ShellCheck -Version: 0.6.0 +Version: 0.7.0 Synopsis: Shell script analysis tool License: GPL-3 License-file: LICENSE @@ -49,14 +49,18 @@ build-depends: semigroups build-depends: + aeson, + array, -- GHC 7.6.3 (base 4.6.0.1) is buggy (#1131, #1119) in optimized mode. -- Just disable that version entirely to fail fast. - aeson, base > 4.6.0.1 && < 5, bytestring, containers >= 0.5, - directory, + deepseq >= 1.4.0.0, + Diff >= 0.2.0, + directory >= 1.2.3.0, mtl >= 2.2.1, + filepath, parsec, regex-tdfa, QuickCheck >= 2.7.4, @@ -70,13 +74,18 @@ ShellCheck.AnalyzerLib ShellCheck.Checker ShellCheck.Checks.Commands + ShellCheck.Checks.Custom ShellCheck.Checks.ShellSupport ShellCheck.Data + ShellCheck.Fixer ShellCheck.Formatter.Format ShellCheck.Formatter.CheckStyle + ShellCheck.Formatter.Diff ShellCheck.Formatter.GCC ShellCheck.Formatter.JSON + ShellCheck.Formatter.JSON1 ShellCheck.Formatter.TTY + ShellCheck.Formatter.Quiet ShellCheck.Interface ShellCheck.Parser ShellCheck.Regex @@ -89,29 +98,37 @@ semigroups build-depends: aeson, + array, base >= 4 && < 5, bytestring, - ShellCheck, containers, - directory, + deepseq >= 1.4.0.0, + Diff >= 0.2.0, + directory >= 1.2.3.0, mtl >= 2.2.1, + filepath, parsec >= 3.0, QuickCheck >= 2.7.4, - regex-tdfa + regex-tdfa, + ShellCheck main-is: shellcheck.hs test-suite test-shellcheck type: exitcode-stdio-1.0 build-depends: aeson, + array, base >= 4 && < 5, bytestring, - ShellCheck, containers, - directory, + deepseq >= 1.4.0.0, + Diff >= 0.2.0, + directory >= 1.2.3.0, mtl >= 2.2.1, + filepath, parsec, QuickCheck >= 2.7.4, - regex-tdfa + regex-tdfa, + ShellCheck main-is: test/shellcheck.hs diff --git a/shellcheck.1 b/shellcheck.1 --- a/shellcheck.1 +++ b/shellcheck.1 @@ -42,8 +42,7 @@ Emit warnings in sourced files. Normally, \f[C]shellcheck\f[] will only warn about issues in the specified files. -With this option, any issues in sourced files files will also be -reported. +With this option, any issues in sourced files will also be reported. .RS .RE .TP @@ -56,6 +55,14 @@ .RS .RE .TP +.B \f[B]\-i\f[]\ \f[I]CODE1\f[][,\f[I]CODE2\f[]...],\ \f[B]\-\-include=\f[]\f[I]CODE1\f[][,\f[I]CODE2\f[]...] +Explicitly include only the specified codes in the report. +Subsequent \f[B]\-i\f[] options are cumulative, but all the codes can be +specified at once, comma\-separated as a single argument. +Include options override any provided exclude options. +.RS +.RE +.TP .B \f[B]\-e\f[]\ \f[I]CODE1\f[][,\f[I]CODE2\f[]...],\ \f[B]\-\-exclude=\f[]\f[I]CODE1\f[][,\f[I]CODE2\f[]...] Explicitly exclude the specified codes from the report. Subsequent \f[B]\-e\f[] options are cumulative, but all the codes can be @@ -71,23 +78,53 @@ .RS .RE .TP -.B \f[B]\-S\f[]\ \f[I]SEVERITY\f[],\ \f[B]\-\-severity=\f[]\f[I]severity\f[] -Specify minimum severity of errors to consider. -Valid values are \f[I]error\f[], \f[I]warning\f[], \f[I]info\f[] and -\f[I]style\f[]. -The default is \f[I]style\f[]. +.B \f[B]\-\-list\-optional\f[] +Output a list of known optional checks. +These can be enabled with \f[B]\-o\f[] flags or \f[B]enable\f[] +directives. .RS .RE .TP +.B \f[B]\-\-norc\f[] +Don\[aq]t try to look for .shellcheckrc configuration files. +.RS +.RE +.TP +.B \f[B]\-o\f[]\ \f[I]NAME1\f[][,\f[I]NAME2\f[]...],\ \f[B]\-\-enable=\f[]\f[I]NAME1\f[][,\f[I]NAME2\f[]...] +Enable optional checks. +The special name \f[I]all\f[] enables all of them. +Subsequent \f[B]\-o\f[] options accumulate. +This is equivalent to specifying \f[B]enable\f[] directives. +.RS +.RE +.TP +.B \f[B]\-P\f[]\ \f[I]SOURCEPATH\f[],\ \f[B]\-\-source\-path=\f[]\f[I]SOURCEPATH\f[] +Specify paths to search for sourced files, separated by \f[C]:\f[] on +Unix and \f[C];\f[] on Windows. +This is equivalent to specifying \f[C]search\-path\f[] directives. +.RS +.RE +.TP .B \f[B]\-s\f[]\ \f[I]shell\f[],\ \f[B]\-\-shell=\f[]\f[I]shell\f[] Specify Bourne shell dialect. Valid values are \f[I]sh\f[], \f[I]bash\f[], \f[I]dash\f[] and \f[I]ksh\f[]. -The default is to use the file\[aq]s shebang, or \f[I]bash\f[] if the -target shell can\[aq]t be determined. +The default is to deduce the shell from the file\[aq]s \f[C]shell\f[] +directive, shebang, or \f[C]\&.bash/.bats/.dash/.ksh\f[] extension, in +that order. +\f[I]sh\f[] refers to POSIX \f[C]sh\f[] (not the system\[aq]s), and will +warn of portability issues. .RS .RE .TP +.B \f[B]\-S\f[]\ \f[I]SEVERITY\f[],\ \f[B]\-\-severity=\f[]\f[I]severity\f[] +Specify minimum severity of errors to consider. +Valid values in order of severity are \f[I]error\f[], \f[I]warning\f[], +\f[I]info\f[] and \f[I]style\f[]. +The default is \f[I]style\f[]. +.RS +.RE +.TP .B \f[B]\-V\f[],\ \f[B]\-\-version\f[] Print version information and exit. .RS @@ -108,6 +145,11 @@ This option allows following any file the script may \f[C]source\f[]. .RS .RE +.TP +.B \f[B]FILES...\f[] +One or more script files to check, or "\-" for standard input. +.RS +.RE .SH FORMATS .TP .B \f[B]tty\f[] @@ -158,32 +200,71 @@ .fi .RE .TP -.B \f[B]json\f[] +.B \f[B]diff\f[] +Auto\-fixes in unified diff format. +Can be piped to \f[C]git\ apply\f[] or \f[C]patch\ \-p1\f[] to +automatically apply fixes. +.RS +.IP +.nf +\f[C] +\-\-\-\ a/test.sh ++++\ b/test.sh +\@\@\ \-2,6\ +2,6\ \@\@ +\ ##\ Example\ of\ a\ broken\ script. +\ for\ f\ in\ $(ls\ *.m3u) +\ do +\-\ \ grep\ \-qi\ hq.*mp3\ $f\ \\ ++\ \ grep\ \-qi\ hq.*mp3\ "$f"\ \\ +\ \ \ \ \ &&\ echo\ \-e\ \[aq]Playlist\ $f\ contains\ a\ HQ\ file\ in\ mp3\ format\[aq] +\ done +\f[] +.fi +.RE +.TP +.B \f[B]json1\f[] Json is a popular serialization format that is more suitable for web applications. ShellCheck\[aq]s json is compact and contains only the bare minimum. +Tabs are counted as 1 character. .RS .IP .nf \f[C] -[ -\ \ { -\ \ \ \ "file":\ "filename", -\ \ \ \ "line":\ lineNumber, -\ \ \ \ "column":\ columnNumber, -\ \ \ \ "level":\ "severitylevel", -\ \ \ \ "code":\ errorCode, -\ \ \ \ "message":\ "warning\ message" -\ \ }, -\ \ ... -] +{ +\ \ comments:\ [ +\ \ \ \ { +\ \ \ \ \ \ "file":\ "filename", +\ \ \ \ \ \ "line":\ lineNumber, +\ \ \ \ \ \ "column":\ columnNumber, +\ \ \ \ \ \ "level":\ "severitylevel", +\ \ \ \ \ \ "code":\ errorCode, +\ \ \ \ \ \ "message":\ "warning\ message" +\ \ \ \ }, +\ \ \ \ ... +\ \ ] +} \f[] .fi .RE +.TP +.B \f[B]json\f[] +This is a legacy version of the \f[B]json1\f[] format. +It\[aq]s a raw array of comments, and all offsets have a tab stop of 8. +.RS +.RE +.TP +.B \f[B]quiet\f[] +Suppress all normal output. +Exit with zero if no issues are found, otherwise exit with one. +Stops processing after the first issue. +.RS +.RE .SH DIRECTIVES .PP -ShellCheck directives can be specified as comments in the shell script -before a command or block: +ShellCheck directives can be specified as comments in the shell script. +If they appear before the first command, they are considered file\-wide. +Otherwise, they apply to the immediately following command or block: .IP .nf \f[C] @@ -234,6 +315,13 @@ .RS .RE .TP +.B \f[B]enable\f[] +Enable an optional check by name, as listed with +\f[B]\-\-list\-optional\f[]. +Only file\-wide \f[C]enable\f[] directives are considered. +.RS +.RE +.TP .B \f[B]source\f[] Overrides the filename included by a \f[C]source\f[]/\f[C]\&.\f[] statement. @@ -243,6 +331,18 @@ .RS .RE .TP +.B \f[B]source\-path\f[] +Add a directory to the search path for \f[C]source\f[]/\f[C]\&.\f[] +statements (by default, only ShellCheck\[aq]s working directory is +included). +Absolute paths will also be rooted in these paths. +The special path \f[C]SCRIPTDIR\f[] can be used to specify the currently +checked script\[aq]s directory, as in \f[C]source\-path=SCRIPTDIR\f[] or +\f[C]source\-path=SCRIPTDIR/../libs\f[]. +Multiple paths accumulate, and \f[C]\-P\f[] takes precedence over them. +.RS +.RE +.TP .B \f[B]shell\f[] Overrides the shell detected from the shebang. This is useful for files meant to be included (and thus lacking a @@ -250,6 +350,46 @@ \[aq]disable=2039\[aq]. .RS .RE +.SH RC FILES +.PP +Unless \f[C]\-\-norc\f[] is used, ShellCheck will look for a file +\f[C]\&.shellcheckrc\f[] or \f[C]shellcheckrc\f[] in the script\[aq]s +directory and each parent directory. +If found, it will read \f[C]key=value\f[] pairs from it and treat them +as file\-wide directives. +.PP +Here is an example \f[C]\&.shellcheckrc\f[]: +.IP +.nf +\f[C] +#\ Look\ for\ \[aq]source\[aq]d\ files\ relative\ to\ the\ checked\ script, +#\ and\ also\ look\ for\ absolute\ paths\ in\ /mnt/chroot +source\-path=SCRIPTDIR +source\-path=/mnt/chroot + +#\ Turn\ on\ warnings\ for\ unquoted\ variables\ with\ safe\ values +enable=quote\-safe\-variables + +#\ Turn\ on\ warnings\ for\ unassigned\ uppercase\ variables +enable=check\-unassigned\-uppercase + +#\ Allow\ using\ `which`\ since\ it\ gives\ full\ paths\ and\ is\ common\ enough +disable=SC2230 +\f[] +.fi +.PP +If no \f[C]\&.shellcheckrc\f[] is found in any of the parent +directories, ShellCheck will look in \f[C]~/.shellcheckrc\f[] followed +by the XDG config directory (usually \f[C]~/.config/shellcheckrc\f[]) on +Unix, or \f[C]%APPDATA%/shellcheckrc\f[] on Windows. +Only the first file found will be used. +.PP +Note for Snap users: the Snap sandbox disallows access to hidden files. +Use \f[C]shellcheckrc\f[] without the dot instead. +.PP +Note for Docker users: ShellCheck will only be able to look for files +that are mounted in the container, so \f[C]~/.shellcheckrc\f[] will not +be read. .SH ENVIRONMENT VARIABLES .PP The environment variable \f[C]SHELLCHECK_OPTS\f[] can be set with @@ -290,9 +430,10 @@ Windows users seeing \f[C]commitBuffer:\ invalid\ argument\ (invalid\ character)\f[] should set their terminal to use UTF\-8 with \f[C]chcp\ 65001\f[]. -.SH AUTHOR +.SH AUTHORS .PP -ShellCheck is written and maintained by Vidar Holen. +ShellCheck is developed and maintained by Vidar Holen, with assistance +from a long list of wonderful contributors. .SH REPORTING BUGS .PP Bugs and issues can be reported on GitHub: @@ -300,7 +441,7 @@ https://github.com/koalaman/shellcheck/issues .SH COPYRIGHT .PP -Copyright 2012\-2015, Vidar Holen. +Copyright 2012\-2019, Vidar Holen and contributors. Licensed under the GNU General Public License version 3 or later, see https://gnu.org/licenses/gpl.html .SH SEE ALSO diff --git a/shellcheck.1.md b/shellcheck.1.md --- a/shellcheck.1.md +++ b/shellcheck.1.md @@ -29,14 +29,13 @@ + For scripts starting with `#!/bin/ksh` (or using `-s ksh`), ShellCheck will not warn at all, as `ksh` supports decimals in arithmetic contexts. - # OPTIONS **-a**,\ **--check-sourced** : Emit warnings in sourced files. Normally, `shellcheck` will only warn about issues in the specified files. With this option, any issues in - sourced files files will also be reported. + sourced files will also be reported. **-C**[*WHEN*],\ **--color**[=*WHEN*] @@ -44,6 +43,13 @@ is *auto*. **--color** without an argument is equivalent to **--color=always**. +**-i**\ *CODE1*[,*CODE2*...],\ **--include=***CODE1*[,*CODE2*...] + +: Explicitly include only the specified codes in the report. Subsequent **-i** + options are cumulative, but all the codes can be specified at once, + comma-separated as a single argument. Include options override any provided + exclude options. + **-e**\ *CODE1*[,*CODE2*...],\ **--exclude=***CODE1*[,*CODE2*...] : Explicitly exclude the specified codes from the report. Subsequent **-e** @@ -56,17 +62,40 @@ standard output. Subsequent **-f** options are ignored, see **FORMATS** below for more information. -**-S**\ *SEVERITY*,\ **--severity=***severity* +**--list-optional** -: Specify minimum severity of errors to consider. Valid values are *error*, - *warning*, *info* and *style*. The default is *style*. +: Output a list of known optional checks. These can be enabled with **-o** + flags or **enable** directives. +**--norc** + +: Don't try to look for .shellcheckrc configuration files. + +**-o**\ *NAME1*[,*NAME2*...],\ **--enable=***NAME1*[,*NAME2*...] + +: Enable optional checks. The special name *all* enables all of them. + Subsequent **-o** options accumulate. This is equivalent to specifying + **enable** directives. + +**-P**\ *SOURCEPATH*,\ **--source-path=***SOURCEPATH* + +: Specify paths to search for sourced files, separated by `:` on Unix and + `;` on Windows. This is equivalent to specifying `search-path` + directives. + **-s**\ *shell*,\ **--shell=***shell* : Specify Bourne shell dialect. Valid values are *sh*, *bash*, *dash* and *ksh*. - The default is to use the file's shebang, or *bash* if the target shell - can't be determined. + The default is to deduce the shell from the file's `shell` directive, + shebang, or `.bash/.bats/.dash/.ksh` extension, in that order. *sh* refers to + POSIX `sh` (not the system's), and will warn of portability issues. +**-S**\ *SEVERITY*,\ **--severity=***severity* + +: Specify minimum severity of errors to consider. Valid values in order of + severity are *error*, *warning*, *info* and *style*. + The default is *style*. + **-V**,\ **--version** : Print version information and exit. @@ -83,7 +112,11 @@ line (plus `/dev/null`). This option allows following any file the script may `source`. +**FILES...** +: One or more script files to check, or "-" for standard input. + + # FORMATS **tty** @@ -119,28 +152,60 @@ ... -**json** +**diff** +: Auto-fixes in unified diff format. Can be piped to `git apply` or `patch -p1` + to automatically apply fixes. + + --- a/test.sh + +++ b/test.sh + @@ -2,6 +2,6 @@ + ## Example of a broken script. + for f in $(ls *.m3u) + do + - grep -qi hq.*mp3 $f \ + + grep -qi hq.*mp3 "$f" \ + && echo -e 'Playlist $f contains a HQ file in mp3 format' + done + + +**json1** + : Json is a popular serialization format that is more suitable for web applications. ShellCheck's json is compact and contains only the bare - minimum. + minimum. Tabs are counted as 1 character. - [ - { - "file": "filename", - "line": lineNumber, - "column": columnNumber, - "level": "severitylevel", - "code": errorCode, - "message": "warning message" - }, - ... - ] + { + comments: [ + { + "file": "filename", + "line": lineNumber, + "column": columnNumber, + "level": "severitylevel", + "code": errorCode, + "message": "warning message" + }, + ... + ] + } +**json** + +: This is a legacy version of the **json1** format. It's a raw array of + comments, and all offsets have a tab stop of 8. + +**quiet** + +: Suppress all normal output. Exit with zero if no issues are found, + otherwise exit with one. Stops processing after the first issue. + + # DIRECTIVES -ShellCheck directives can be specified as comments in the shell script -before a command or block: +ShellCheck directives can be specified as comments in the shell script. +If they appear before the first command, they are considered file-wide. +Otherwise, they apply to the immediately following command or block: + # shellcheck key=value key=value command-or-structure @@ -169,17 +234,64 @@ The command can be a simple command like `echo foo`, or a compound command like a function definition, subshell block or loop. +**enable** +: Enable an optional check by name, as listed with **--list-optional**. + Only file-wide `enable` directives are considered. + **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 at runtime, or to skip a source by telling it to use `/dev/null`. +**source-path** +: Add a directory to the search path for `source`/`.` statements (by default, + only ShellCheck's working directory is included). Absolute paths will also + be rooted in these paths. The special path `SCRIPTDIR` can be used to + specify the currently checked script's directory, as in + `source-path=SCRIPTDIR` or `source-path=SCRIPTDIR/../libs`. Multiple + paths accumulate, and `-P` takes precedence over them. + **shell** : Overrides the shell detected from the shebang. This is useful for files meant to be included (and thus lacking a shebang), or possibly as a more targeted alternative to 'disable=2039'. +# RC FILES + +Unless `--norc` is used, ShellCheck will look for a file `.shellcheckrc` or +`shellcheckrc` in the script's directory and each parent directory. If found, +it will read `key=value` pairs from it and treat them as file-wide directives. + +Here is an example `.shellcheckrc`: + + # Look for 'source'd files relative to the checked script, + # and also look for absolute paths in /mnt/chroot + source-path=SCRIPTDIR + source-path=/mnt/chroot + + # Turn on warnings for unquoted variables with safe values + enable=quote-safe-variables + + # Turn on warnings for unassigned uppercase variables + enable=check-unassigned-uppercase + + # Allow using `which` since it gives full paths and is common enough + disable=SC2230 + +If no `.shellcheckrc` is found in any of the parent directories, ShellCheck +will look in `~/.shellcheckrc` followed by the XDG config directory +(usually `~/.config/shellcheckrc`) on Unix, or `%APPDATA%/shellcheckrc` on +Windows. Only the first file found will be used. + +Note for Snap users: the Snap sandbox disallows access to hidden files. +Use `shellcheckrc` without the dot instead. + +Note for Docker users: ShellCheck will only be able to look for files that +are mounted in the container, so `~/.shellcheckrc` will not be read. + + # ENVIRONMENT VARIABLES + The environment variable `SHELLCHECK_OPTS` can be set with default flags: export SHELLCHECK_OPTS='--shell=bash --exclude=SC2016' @@ -198,6 +310,7 @@ + 4: ShellCheck was invoked with bad options (e.g. unknown formatter). # LOCALE + This version of ShellCheck is only available in English. All files are leniently decoded as UTF-8, with a fallback of ISO-8859-1 for invalid sequences. `LC_CTYPE` is respected for output, and defaults to UTF-8 for @@ -206,19 +319,22 @@ Windows users seeing `commitBuffer: invalid argument (invalid character)` should set their terminal to use UTF-8 with `chcp 65001`. -# AUTHOR -ShellCheck is written and maintained by Vidar Holen. +# AUTHORS +ShellCheck is developed and maintained by Vidar Holen, with assistance from a +long list of wonderful contributors. + # REPORTING BUGS + Bugs and issues can be reported on GitHub: https://github.com/koalaman/shellcheck/issues # COPYRIGHT -Copyright 2012-2015, Vidar Holen. + +Copyright 2012-2019, Vidar Holen and contributors. Licensed under the GNU General Public License version 3 or later, see https://gnu.org/licenses/gpl.html - # SEE ALSO diff --git a/shellcheck.hs b/shellcheck.hs --- a/shellcheck.hs +++ b/shellcheck.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -} +import qualified ShellCheck.Analyzer import ShellCheck.Checker import ShellCheck.Data import ShellCheck.Interface @@ -24,9 +25,12 @@ import qualified ShellCheck.Formatter.CheckStyle import ShellCheck.Formatter.Format +import qualified ShellCheck.Formatter.Diff import qualified ShellCheck.Formatter.GCC import qualified ShellCheck.Formatter.JSON +import qualified ShellCheck.Formatter.JSON1 import qualified ShellCheck.Formatter.TTY +import qualified ShellCheck.Formatter.Quiet import Control.Exception import Control.Monad @@ -46,6 +50,7 @@ import System.Directory import System.Environment import System.Exit +import System.FilePath import System.IO data Flag = Flag String String @@ -67,6 +72,7 @@ data Options = Options { checkSpec :: CheckSpec, externalSources :: Bool, + sourcePaths :: [FilePath], formatterOptions :: FormatterOptions, minSeverity :: Severity } @@ -74,6 +80,7 @@ defaultOptions = Options { checkSpec = emptyCheckSpec, externalSources = False, + sourcePaths = [], formatterOptions = newFormatterOptions { foColorOption = ColorAuto }, @@ -87,11 +94,23 @@ Option "C" ["color"] (OptArg (maybe (Flag "color" "always") (Flag "color")) "WHEN") "Use color (auto, always, never)", + Option "i" ["include"] + (ReqArg (Flag "include") "CODE1,CODE2..") "Consider only given types of warnings", Option "e" ["exclude"] (ReqArg (Flag "exclude") "CODE1,CODE2..") "Exclude types of warnings", Option "f" ["format"] (ReqArg (Flag "format") "FORMAT") $ "Output format (" ++ formatList ++ ")", + Option "" ["list-optional"] + (NoArg $ Flag "list-optional" "true") "List checks disabled by default", + Option "" ["norc"] + (NoArg $ Flag "norc" "true") "Don't look for .shellcheckrc files", + Option "o" ["enable"] + (ReqArg (Flag "enable") "check1,check2..") + "List of optional checks to enable (or 'all')", + Option "P" ["source-path"] + (ReqArg (Flag "source-path") "SOURCEPATHS") + "Specify path when looking for sourced files (\"SCRIPTDIR\" for script's dir)", Option "s" ["shell"] (ReqArg (Flag "shell") "SHELLNAME") "Specify dialect (sh, bash, dash, ksh)", @@ -102,10 +121,13 @@ (NoArg $ Flag "version" "true") "Print version information", Option "W" ["wiki-link-count"] (ReqArg (Flag "wiki-link-count") "NUM") - "The number of wiki links to show, when applicable.", + "The number of wiki links to show, when applicable", Option "x" ["external-sources"] - (NoArg $ Flag "externals" "true") "Allow 'source' outside of FILES" + (NoArg $ Flag "externals" "true") "Allow 'source' outside of FILES", + Option "" ["help"] + (NoArg $ Flag "help" "true") "Show this usage summary and exit" ] +getUsageInfo = usageInfo usageHeader options printErr = lift . hPutStrLn stderr @@ -114,15 +136,18 @@ case getOpt Permute options argv of (opts, files, []) -> return (opts, files) (_, _, errors) -> do - printErr $ concat errors ++ "\n" ++ usageInfo usageHeader options + printErr $ concat errors ++ "\n" ++ getUsageInfo throwError SyntaxFailure formats :: FormatterOptions -> Map.Map String (IO Formatter) formats options = Map.fromList [ ("checkstyle", ShellCheck.Formatter.CheckStyle.format), + ("diff", ShellCheck.Formatter.Diff.format options), ("gcc", ShellCheck.Formatter.GCC.format), ("json", ShellCheck.Formatter.JSON.format), - ("tty", ShellCheck.Formatter.TTY.format options) + ("json1", ShellCheck.Formatter.JSON1.format), + ("tty", ShellCheck.Formatter.TTY.format options), + ("quiet", ShellCheck.Formatter.Quiet.format options) ] formatList = intercalate ", " names @@ -267,10 +292,30 @@ } } + Flag "include" str -> do + new <- mapM parseNum $ filter (not . null) $ split ',' str + let old = csIncludedWarnings . checkSpec $ options + return options { + checkSpec = (checkSpec options) { + csIncludedWarnings = + if null new + then old + else Just new `mappend` old + } + } + Flag "version" _ -> do liftIO printVersion throwError NoProblems + Flag "list-optional" _ -> do + liftIO printOptional + throwError NoProblems + + Flag "help" _ -> do + liftIO $ putStrLn getUsageInfo + throwError NoProblems + Flag "externals" _ -> return options { externalSources = True @@ -284,6 +329,12 @@ } } + Flag "source-path" str -> do + let paths = splitSearchPath str + return options { + sourcePaths = (sourcePaths options) ++ paths + } + Flag "sourced" _ -> return options { checkSpec = (checkSpec options) { @@ -307,7 +358,26 @@ } } - _ -> return options + Flag "norc" _ -> + return options { + checkSpec = (checkSpec options) { + csIgnoreRC = True + } + } + + Flag "enable" value -> + let cs = checkSpec options in return options { + checkSpec = cs { + csOptionalChecks = (csOptionalChecks cs) ++ split ',' value + } + } + + -- This flag is handled specially in 'process' + Flag "format" _ -> return options + + Flag str _ -> do + printErr $ "Internal error for --" ++ str ++ ". Please file a bug :(" + return options where die s = do printErr s @@ -322,12 +392,16 @@ ioInterface options files = do inputs <- mapM normalize files cache <- newIORef emptyCache + configCache <- newIORef ("", Nothing) return SystemInterface { - siReadFile = get cache inputs + siReadFile = get cache inputs, + siFindSource = findSourceFile inputs (sourcePaths options), + siGetConfig = getConfig configCache } where emptyCache :: Map.Map FilePath String emptyCache = Map.empty + get cache inputs file = do map <- readIORef cache case Map.lookup file map of @@ -344,7 +418,6 @@ return $ Right contents ) `catch` handler 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 @@ -362,6 +435,82 @@ fallback :: FilePath -> IOException -> IO FilePath fallback path _ = return path + -- Returns the name and contents of .shellcheckrc for the given file + getConfig cache filename = do + path <- normalize filename + let dir = takeDirectory path + (previousPath, result) <- readIORef cache + if dir == previousPath + then return result + else do + paths <- getConfigPaths dir + result <- findConfig paths + writeIORef cache (dir, result) + return result + + findConfig paths = + case paths of + (file:rest) -> do + contents <- readConfig file + if isJust contents + then return contents + else findConfig rest + [] -> return Nothing + + -- Get a list of candidate filenames. This includes .shellcheckrc + -- in all parent directories, plus the user's home dir and xdg dir. + -- The dot is optional for Windows and Snap users. + getConfigPaths dir = do + let next = takeDirectory dir + rest <- if next /= dir + then getConfigPaths next + else defaultPaths `catch` + ((const $ return []) :: IOException -> IO [FilePath]) + return $ (dir ".shellcheckrc") : (dir "shellcheckrc") : rest + + defaultPaths = do + home <- getAppUserDataDirectory "shellcheckrc" + xdg <- getXdgDirectory XdgConfig "shellcheckrc" + return [home, xdg] + + readConfig file = do + exists <- doesFileExist file + if exists + then do + (contents, _) <- inputFile file `catch` handler file + return $ Just (file, contents) + else + return Nothing + where + handler :: FilePath -> IOException -> IO (String, Bool) + handler file err = do + putStrLn $ file ++ ": " ++ show err + return ("", True) + + andM a b arg = do + first <- a arg + if not first then return False else b arg + + findSourceFile inputs sourcePathFlag currentScript sourcePathAnnotation original = + if isAbsolute original + then + let (_, relative) = splitDrive original + in find relative original + else + find original original + where + find filename deflt = do + sources <- filterM ((allowable inputs) `andM` doesFileExist) $ + (adjustPath filename):(map ( filename) $ map adjustPath $ sourcePathFlag ++ sourcePathAnnotation) + case sources of + [] -> return deflt + (first:_) -> return first + scriptdir = dropFileName currentScript + adjustPath str = + case (splitDirectories str) of + ("SCRIPTDIR":rest) -> joinPath (scriptdir:rest) + _ -> str + inputFile file = do (handle, shouldCache) <- if file == "-" @@ -418,3 +567,14 @@ putStrLn $ "version: " ++ shellcheckVersion putStrLn "license: GNU General Public License, version 3" putStrLn "website: https://www.shellcheck.net" + +printOptional = do + mapM f list + where + list = sortOn cdName ShellCheck.Analyzer.optionalChecks + f item = do + putStrLn $ "name: " ++ cdName item + putStrLn $ "desc: " ++ cdDescription item + putStrLn $ "example: " ++ cdPositive item + putStrLn $ "fix: " ++ cdNegative item + putStrLn "" diff --git a/src/ShellCheck/AST.hs b/src/ShellCheck/AST.hs --- a/src/ShellCheck/AST.hs +++ b/src/ShellCheck/AST.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -17,14 +17,17 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -} +{-# LANGUAGE DeriveGeneric, DeriveAnyClass #-} module ShellCheck.AST where +import GHC.Generics (Generic) import Control.Monad.Identity +import Control.DeepSeq import Text.Parsec import qualified ShellCheck.Regex as Re import Prelude hiding (id) -newtype Id = Id Int deriving (Show, Eq, Ord) +newtype Id = Id Int deriving (Show, Eq, Ord, Generic, NFData) data Quoted = Quoted | Unquoted deriving (Show, Eq) data Dashed = Dashed | Undashed deriving (Show, Eq) @@ -73,7 +76,7 @@ | T_DSEMI Id | T_Do Id | T_DollarArithmetic Id Token - | T_DollarBraced Id Token + | T_DollarBraced Id Bool Token | T_DollarBracket Id Token | T_DollarDoubleQuoted Id [Token] | T_DollarExpansion Id [Token] @@ -118,7 +121,7 @@ | T_Rbrace Id | T_Redirecting Id [Token] Token | T_Rparen Id - | T_Script Id String [Token] + | T_Script Id Token [Token] -- Shebang T_Literal, followed by script. | T_Select Id | T_SelectIn Id String [Token] [Token] | T_Semi Id @@ -136,12 +139,15 @@ | T_CoProcBody Id Token | T_Include Id Token | T_SourceCommand Id Token Token + | T_BatsTest Id Token Token deriving (Show) data Annotation = DisableComment Integer + | EnableComment String | SourceOverride String | ShellOverride String + | SourcePath String deriving (Show, Eq) data ConditionType = DoubleBracket | SingleBracket deriving (Show, Eq) @@ -247,7 +253,7 @@ delve (T_Function id a b name body) = d1 body $ T_Function id a b name delve (T_Condition id typ token) = d1 token $ T_Condition id typ delve (T_Extglob id str l) = dl l $ T_Extglob id str - delve (T_DollarBraced id op) = d1 op $ T_DollarBraced id + delve (T_DollarBraced id braced op) = d1 op $ T_DollarBraced id braced delve (T_HereDoc id d q str l) = dl l $ T_HereDoc id d q str delve (TC_And id typ str t1 t2) = d2 t1 t2 $ TC_And id typ str @@ -273,6 +279,7 @@ delve (T_CoProcBody id t) = d1 t $ T_CoProcBody id delve (T_Include id script) = d1 script $ T_Include id delve (T_SourceCommand id includer t_include) = d2 includer t_include $ T_SourceCommand id + delve (T_BatsTest id name t) = d2 name t $ T_BatsTest id delve t = return t getId :: Token -> Id @@ -316,7 +323,7 @@ T_NormalWord id _ -> id T_DoubleQuoted id _ -> id T_DollarExpansion id _ -> id - T_DollarBraced id _ -> id + T_DollarBraced id _ _ -> id T_DollarArithmetic id _ -> id T_BraceExpansion id _ -> id T_ParamSubSpecialChar id _ -> id @@ -377,6 +384,7 @@ T_UnparsedIndex id _ _ -> id TC_Empty id _ -> id TA_Variable id _ _ -> id + T_BatsTest id _ _ -> id blank :: Monad m => Token -> m () blank = const $ return () 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-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -81,7 +81,7 @@ (T_NormalWord _ l) -> [concat (concatMap oversimplify l)] (T_DoubleQuoted _ l) -> [concat (concatMap oversimplify l)] (T_SingleQuoted _ s) -> [s] - (T_DollarBraced _ _) -> ["${VAR}"] + (T_DollarBraced _ _ _) -> ["${VAR}"] (T_DollarArithmetic _ _) -> ["${VAR}"] (T_DollarExpansion _ _) -> ["${VAR}"] (T_Backticked _ _) -> ["${VAR}"] @@ -133,11 +133,11 @@ return $ "-" `isPrefixOf` str -- Given a T_DollarBraced, return a simplified version of the string contents. -bracedString (T_DollarBraced _ l) = concat $ oversimplify l +bracedString (T_DollarBraced _ _ l) = concat $ oversimplify l bracedString _ = error "Internal shellcheck error, please report! (bracedString on non-variable)" -- Is this an expansion of multiple items of an array? -isArrayExpansion t@(T_DollarBraced _ _) = +isArrayExpansion t@(T_DollarBraced _ _ _) = let string = bracedString t in "@" `isPrefixOf` string || not ("#" `isPrefixOf` string) && "[@]" `isInfixOf` string @@ -146,7 +146,7 @@ -- Is it possible that this arg becomes multiple args? mayBecomeMultipleArgs t = willBecomeMultipleArgs t || f t where - f t@(T_DollarBraced _ _) = + f t@(T_DollarBraced _ _ _) = let string = bracedString t in "!" `isPrefixOf` string f (T_DoubleQuoted _ parts) = any f parts @@ -351,6 +351,14 @@ isFunction t = case t of T_Function {} -> True; _ -> False +-- Bats tests are functions for the purpose of 'local' and such +isFunctionLike t = + case t of + T_Function {} -> True + T_BatsTest {} -> True + _ -> False + + isBraceExpansion t = case t of T_BraceExpansion {} -> True; _ -> False -- Get the lists of commands from tokens that contain them, such as @@ -485,8 +493,21 @@ -- Is this an expansion that can be quoted, -- e.g. $(foo) `foo` $foo (but not {foo,})? isQuoteableExpansion t = case t of + T_DollarBraced {} -> True + _ -> isCommandSubstitution t + +isCommandSubstitution t = case t of T_DollarExpansion {} -> True T_DollarBraceCommandExpansion {} -> True T_Backticked {} -> True - T_DollarBraced {} -> True _ -> False + + +-- Is this a T_Annotation that ignores a specific code? +isAnnotationIgnoringCode code t = + case t of + T_Annotation _ anns _ -> any hasNum anns + _ -> False + where + hasNum (DisableComment ts) = code == ts + hasNum _ = 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-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -19,7 +19,7 @@ -} {-# LANGUAGE TemplateHaskell #-} {-# LANGUAGE FlexibleContexts #-} -module ShellCheck.Analytics (runAnalytics, ShellCheck.Analytics.runTests) where +module ShellCheck.Analytics (runAnalytics, optionalChecks, ShellCheck.Analytics.runTests) where import ShellCheck.AST import ShellCheck.ASTLib @@ -49,9 +49,7 @@ -- Checks that are run on the AST root treeChecks :: [Parameters -> Token -> [TokenComment]] treeChecks = [ - runNodeAnalysis - (\p t -> (mapM_ ((\ f -> f t) . (\ f -> f p)) - nodeChecks)) + nodeChecksToTreeCheck nodeChecks ,subshellAssignmentCheck ,checkSpacefulness ,checkQuotesInLiterals @@ -69,7 +67,14 @@ runAnalytics :: AnalysisSpec -> [TokenComment] runAnalytics options = - runList options treeChecks + runList options treeChecks ++ runList options optionalChecks + where + root = asScript options + optionals = getEnableDirectives root ++ asOptionalChecks options + optionalChecks = + if "all" `elem` optionals + then map snd optionalTreeChecks + else mapMaybe (\c -> Map.lookup c optionalCheckMap) optionals runList :: AnalysisSpec -> [Parameters -> Token -> [TokenComment]] -> [TokenComment] @@ -79,13 +84,27 @@ params = makeParameters spec notes = concatMap (\f -> f params root) list +getEnableDirectives root = + case root of + T_Annotation _ list _ -> mapMaybe getEnable list + _ -> [] + where + getEnable t = + case t of + EnableComment s -> return s + _ -> Nothing checkList l t = concatMap (\f -> f t) l - -- Checks that are run on each node in the AST runNodeAnalysis f p t = execWriter (doAnalysis (f p) t) +-- Perform multiple node checks in a single iteration over the tree +nodeChecksToTreeCheck checkList = + runNodeAnalysis + (\p t -> (mapM_ ((\ f -> f t) . (\ f -> f p)) + checkList)) + nodeChecks :: [Parameters -> Token -> Writer [TokenComment] ()] nodeChecks = [ checkUuoc @@ -170,9 +189,62 @@ ,checkSubshelledTests ,checkInvertedStringTest ,checkRedirectionToCommand + ,checkDollarQuoteParen + ,checkUselessBang ] +optionalChecks = map fst optionalTreeChecks + +prop_verifyOptionalExamples = all check optionalTreeChecks + where + check (desc, check) = + verifyTree check (cdPositive desc) + && verifyNotTree check (cdNegative desc) + +optionalTreeChecks :: [(CheckDescription, (Parameters -> Token -> [TokenComment]))] +optionalTreeChecks = [ + (newCheckDescription { + cdName = "quote-safe-variables", + cdDescription = "Suggest quoting variables without metacharacters", + cdPositive = "var=hello; echo $var", + cdNegative = "var=hello; echo \"$var\"" + }, checkVerboseSpacefulness) + + ,(newCheckDescription { + cdName = "avoid-nullary-conditions", + cdDescription = "Suggest explicitly using -n in `[ $var ]`", + cdPositive = "[ \"$var\" ]", + cdNegative = "[ -n \"$var\" ]" + }, nodeChecksToTreeCheck [checkNullaryExpansionTest]) + + ,(newCheckDescription { + cdName = "add-default-case", + cdDescription = "Suggest adding a default case in `case` statements", + cdPositive = "case $? in 0) echo 'Success';; esac", + cdNegative = "case $? in 0) echo 'Success';; *) echo 'Fail' ;; esac" + }, nodeChecksToTreeCheck [checkDefaultCase]) + + ,(newCheckDescription { + cdName = "require-variable-braces", + cdDescription = "Suggest putting braces around all variable references", + cdPositive = "var=hello; echo $var", + cdNegative = "var=hello; echo ${var}" + }, nodeChecksToTreeCheck [checkVariableBraces]) + + ,(newCheckDescription { + cdName = "check-unassigned-uppercase", + cdDescription = "Warn when uppercase variables are unassigned", + cdPositive = "echo $VAR", + cdNegative = "VAR=hello; echo $VAR" + }, checkUnassignedReferences' True) + ] + +optionalCheckMap :: Map.Map String (Parameters -> Token -> [TokenComment]) +optionalCheckMap = Map.fromList $ map item optionalTreeChecks + where + item (desc, check) = (cdName desc, check) + wouldHaveBeenGlob s = '*' `elem` s verify :: (Parameters -> Token -> Writer [TokenComment] ()) -> String -> Bool @@ -199,8 +271,13 @@ checkNode f = producesComments (runNodeAnalysis f) producesComments :: (Parameters -> Token -> [TokenComment]) -> String -> Maybe Bool producesComments f s = do - root <- pScript s - return . not . null $ runList (defaultSpec root) [f] + let pr = pScript s + prRoot pr + let spec = defaultSpec pr + let params = makeParameters spec + return . not . null $ + filterByAnnotation spec params $ + runList spec [f] -- Copied from https://wiki.haskell.org/Edit_distance dist :: Eq a => [a] -> [a] -> Int @@ -230,7 +307,9 @@ isCondition [] = False isCondition [_] = False isCondition (child:parent:rest) = - getId child `elem` map getId (getConditionChildren parent) || isCondition (parent:rest) + case child of + T_BatsTest {} -> True -- count anything in a @test as conditional + _ -> getId child `elem` map getId (getConditionChildren parent) || isCondition (parent:rest) where getConditionChildren t = case t of @@ -241,6 +320,43 @@ T_UntilExpression id c l -> take 1 . reverse $ c _ -> [] +-- helpers to build replacements +replaceStart id params n r = + let tp = tokenPositions params + (start, _) = tp Map.! id + new_end = start { + posColumn = posColumn start + n + } + depth = length $ getPath (parentMap params) (T_EOF id) + in + newReplacement { + repStartPos = start, + repEndPos = new_end, + repString = r, + repPrecedence = depth, + repInsertionPoint = InsertAfter + } +replaceEnd id params n r = + let tp = tokenPositions params + (_, end) = tp Map.! id + new_start = end { + posColumn = posColumn end - n + } + new_end = end { + posColumn = posColumn end + } + depth = length $ getPath (parentMap params) (T_EOF id) + in + newReplacement { + repStartPos = new_start, + repEndPos = new_end, + repString = r, + repPrecedence = depth, + repInsertionPoint = InsertBefore + } +surroundWidth id params s = fixWith [replaceStart id params 0 s, replaceEnd id params 0 s] +fixWith fixes = newFix { fixReplacements = fixes } + prop_checkEchoWc3 = verify checkEchoWc "n=$(echo $foo | wc -c)" checkEchoWc _ (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ @@ -422,7 +538,7 @@ prop_checkShebangParameters1 = verifyTree checkShebangParameters "#!/usr/bin/env bash -x\necho cow" prop_checkShebangParameters2 = verifyNotTree checkShebangParameters "#! /bin/sh -l " checkShebangParameters p (T_Annotation _ _ t) = checkShebangParameters p t -checkShebangParameters _ (T_Script id sb _) = +checkShebangParameters _ (T_Script _ (T_Literal id sb) _) = [makeComment ErrorC id 2096 "On most OS, shebangs can only specify a single parameter." | length (words sb) > 2] prop_checkShebang1 = verifyNotTree checkShebang "#!/usr/bin/env bash -x\necho cow" @@ -435,19 +551,26 @@ prop_checkShebang8 = verifyTree checkShebang "#!bin/sh\ntrue" prop_checkShebang9 = verifyNotTree checkShebang "# shellcheck shell=sh\ntrue" prop_checkShebang10= verifyNotTree checkShebang "#!foo\n# shellcheck shell=sh ignore=SC2239\ntrue" +prop_checkShebang11= verifyTree checkShebang "#!/bin/sh/\ntrue" +prop_checkShebang12= verifyTree checkShebang "#!/bin/sh/ -xe\ntrue" checkShebang params (T_Annotation _ list t) = if any isOverride list then [] else checkShebang params t where isOverride (ShellOverride _) = True isOverride _ = False -checkShebang params (T_Script id sb _) = execWriter $ do +checkShebang params (T_Script _ (T_Literal id sb) _) = execWriter $ do unless (shellTypeSpecified params) $ do when (sb == "") $ err id 2148 "Tips depend on target shell and yours is unknown. Add a shebang." when (executableFromShebang sb == "ash") $ warn id 2187 "Ash scripts will be checked as Dash. Add '# shellcheck shell=dash' to silence." - unless (null sb || "/" `isPrefixOf` sb) $ - err id 2239 "Ensure the shebang uses an absolute path to the interpreter." + unless (null sb) $ do + unless ("/" `isPrefixOf` sb) $ + err id 2239 "Ensure the shebang uses an absolute path to the interpreter." + case words sb of + first:_ -> + when ("/" `isSuffixOf` first) $ + err id 2246 "This shebang specifies a directory. Ensure the interpreter is a file." prop_checkForInQuoted = verify checkForInQuoted "for f in \"$(ls)\"; do echo foo; done" @@ -650,7 +773,7 @@ prop_checkDollarStar = verify checkDollarStar "for f in $*; do ..; done" prop_checkDollarStar2 = verifyNot checkDollarStar "a=$*" prop_checkDollarStar3 = verifyNot checkDollarStar "[[ $* = 'a b' ]]" -checkDollarStar p t@(T_NormalWord _ [b@(T_DollarBraced id _)]) +checkDollarStar p t@(T_NormalWord _ [b@(T_DollarBraced id _ _)]) | bracedString b == "*" = unless (isStrictlyQuoteFree (parentMap p) t) $ warn id 2048 "Use \"$@\" (with quotes) to prevent whitespace problems." @@ -717,11 +840,12 @@ prop_checkArrayWithoutIndex7 = verifyTree checkArrayWithoutIndex "a=(a b); a+=c" prop_checkArrayWithoutIndex8 = verifyTree checkArrayWithoutIndex "declare -a foo; foo=bar;" prop_checkArrayWithoutIndex9 = verifyTree checkArrayWithoutIndex "read -r -a arr <<< 'foo bar'; echo \"$arr\"" +prop_checkArrayWithoutIndex10= verifyTree checkArrayWithoutIndex "read -ra arr <<< 'foo bar'; echo \"$arr\"" checkArrayWithoutIndex params _ = doVariableFlowAnalysis readF writeF defaultMap (variableFlow params) where defaultMap = Map.fromList $ map (\x -> (x,())) arrayVariables - readF _ (T_DollarBraced id token) _ = do + readF _ (T_DollarBraced id _ token) _ = do map <- get return . maybeToList $ do name <- getLiteralString token @@ -805,6 +929,8 @@ prop_checkSingleQuotedVariables15= verifyNot checkSingleQuotedVariables "git filter-branch 'test $GIT_COMMIT'" prop_checkSingleQuotedVariables16= verify checkSingleQuotedVariables "git '$a'" prop_checkSingleQuotedVariables17= verifyNot checkSingleQuotedVariables "rename 's/(.)a/$1/g' *" +prop_checkSingleQuotedVariables18= verifyNot checkSingleQuotedVariables "echo '``'" +prop_checkSingleQuotedVariables19= verifyNot checkSingleQuotedVariables "echo '```'" checkSingleQuotedVariables params t@(T_SingleQuoted id s) = when (s `matches` re) $ @@ -850,7 +976,7 @@ TC_Unary _ _ "-v" _ -> True _ -> False - re = mkRegex "\\$[{(0-9a-zA-Z_]|`.*`" + re = mkRegex "\\$[{(0-9a-zA-Z_]|`[^`]+`" sedContra = mkRegex "\\$[{dpsaic]($|[^a-zA-Z])" getFindCommand (T_SimpleCommand _ _ words) = @@ -1163,11 +1289,12 @@ prop_checkArithmeticDeref13= verifyNot checkArithmeticDeref "(( $$ ))" prop_checkArithmeticDeref14= verifyNot checkArithmeticDeref "(( $! ))" prop_checkArithmeticDeref15= verifyNot checkArithmeticDeref "(( ${!var} ))" -checkArithmeticDeref params t@(TA_Expansion _ [b@(T_DollarBraced id _)]) = +prop_checkArithmeticDeref16= verifyNot checkArithmeticDeref "(( ${x+1} + ${x=42} ))" +checkArithmeticDeref params t@(TA_Expansion _ [b@(T_DollarBraced id _ _)]) = unless (isException $ bracedString b) getWarning where isException [] = True - isException s = any (`elem` "/.:#%?*@$-!") s || isDigit (head s) + isException s = any (`elem` "/.:#%?*@$-!+=^,") s || isDigit (head s) getWarning = fromMaybe noWarning . msum . map warningFor $ parents params t warningFor t = case t of @@ -1198,12 +1325,17 @@ prop_checkComparisonAgainstGlob4 = verifyNot checkComparisonAgainstGlob "[ $cow = foo ]" prop_checkComparisonAgainstGlob5 = verify checkComparisonAgainstGlob "[[ $cow != $bar ]]" prop_checkComparisonAgainstGlob6 = verify checkComparisonAgainstGlob "[ $f != /* ]" -checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _])) +checkComparisonAgainstGlob _ (TC_Binary _ DoubleBracket op _ (T_NormalWord id [T_DollarBraced _ _ _])) | op `elem` ["=", "==", "!="] = warn id 2053 $ "Quote the right-hand side of " ++ op ++ " in [[ ]] to prevent glob matching." -checkComparisonAgainstGlob _ (TC_Binary _ SingleBracket op _ word) +checkComparisonAgainstGlob params (TC_Binary _ SingleBracket op _ word) | op `elem` ["=", "==", "!="] && isGlob word = - err (getId word) 2081 "[ .. ] can't match globs. Use [[ .. ]] or case statement." + err (getId word) 2081 msg + where + msg = if isBashLike params + then "[ .. ] can't match globs. Use [[ .. ]] or case statement." + else "[ .. ] can't match globs. Use a case statement." + checkComparisonAgainstGlob _ _ = return () prop_checkCommarrays1 = verify checkCommarrays "a=(1, 2)" @@ -1212,6 +1344,7 @@ prop_checkCommarrays4 = verifyNot checkCommarrays "cow=('one,' 'two')" prop_checkCommarrays5 = verify checkCommarrays "a=([a]=b, [c]=d)" prop_checkCommarrays6 = verify checkCommarrays "a=([a]=b,[c]=d,[e]=f)" +prop_checkCommarrays7 = verify checkCommarrays "a=(1,2)" checkCommarrays _ (T_Array id l) = when (any (isCommaSeparated . literal) l) $ warn id 2054 "Use spaces, not commas, to separate array elements." @@ -1219,9 +1352,9 @@ literal (T_IndexedElement _ _ l) = literal l literal (T_NormalWord _ l) = concatMap literal l literal (T_Literal _ str) = str - literal _ = "str" + literal _ = "" - isCommaSeparated str = "," `isSuffixOf` str || length (filter (== ',') str) > 1 + isCommaSeparated = elem ',' checkCommarrays _ _ = return () prop_checkOrNeq1 = verify checkOrNeq "if [[ $lol -ne cow || $lol -ne foo ]]; then echo foo; fi" @@ -1229,14 +1362,39 @@ prop_checkOrNeq3 = verify checkOrNeq "[ \"$a\" != lol || \"$a\" != foo ]" prop_checkOrNeq4 = verifyNot checkOrNeq "[ a != $cow || b != $foo ]" prop_checkOrNeq5 = verifyNot checkOrNeq "[[ $a != /home || $a != */public_html/* ]]" +prop_checkOrNeq6 = verify checkOrNeq "[ $a != a ] || [ $a != b ]" +prop_checkOrNeq7 = verify checkOrNeq "[ $a != a ] || [ $a != b ] || true" +prop_checkOrNeq8 = verifyNot checkOrNeq "[[ $a != x || $a != x ]]" -- This only catches the most idiomatic cases. Fixme? + +-- For test-level "or": [ x != y -o x != z ] checkOrNeq _ (TC_Or id typ op (TC_Binary _ _ op1 lhs1 rhs1 ) (TC_Binary _ _ op2 lhs2 rhs2)) - | lhs1 == lhs2 && (op1 == op2 && (op1 == "-ne" || op1 == "!=")) && not (any isGlob [rhs1,rhs2]) = - warn id 2055 $ "You probably wanted " ++ (if typ == SingleBracket then "-a" else "&&") ++ " here." + | (op1 == op2 && (op1 == "-ne" || op1 == "!=")) && lhs1 == lhs2 && rhs1 /= rhs2 && not (any isGlob [rhs1,rhs2]) = + warn id 2055 $ "You probably wanted " ++ (if typ == SingleBracket then "-a" else "&&") ++ " here, otherwise it's always true." +-- For arithmetic context "or" checkOrNeq _ (TA_Binary id "||" (TA_Binary _ "!=" word1 _) (TA_Binary _ "!=" word2 _)) | word1 == word2 = - warn id 2056 "You probably wanted && here." + warn id 2056 "You probably wanted && here, otherwise it's always true." + +-- For command level "or": [ x != y ] || [ x != z ] +checkOrNeq _ (T_OrIf id lhs rhs) = potentially $ do + (lhs1, op1, rhs1) <- getExpr lhs + (lhs2, op2, rhs2) <- getExpr rhs + guard $ op1 == op2 && op1 `elem` ["-ne", "!="] + guard $ lhs1 == lhs2 && rhs1 /= rhs2 + guard . not $ any isGlob [rhs1, rhs2] + return $ warn id 2252 "You probably wanted && here, otherwise it's always true." + where + getExpr x = + case x of + T_OrIf _ lhs _ -> getExpr lhs -- Fetches x and y in `T_OrIf x (T_OrIf y z)` + T_Pipeline _ _ [x] -> getExpr x + T_Redirecting _ _ c -> getExpr c + T_Condition _ _ c -> getExpr c + TC_Binary _ _ op lhs rhs -> return (lhs, op, rhs) + _ -> fail "" + checkOrNeq _ _ = return () @@ -1335,8 +1493,10 @@ prop_checkBackticks1 = verify checkBackticks "echo `foo`" prop_checkBackticks2 = verifyNot checkBackticks "echo $(foo)" prop_checkBackticks3 = verifyNot checkBackticks "echo `#inlined comment` foo" -checkBackticks _ (T_Backticked id list) | not (null list) = - style id 2006 "Use $(...) notation instead of legacy backticked `...`." +checkBackticks params (T_Backticked id list) | not (null list) = + addComment $ + makeCommentWithFix StyleC id 2006 "Use $(...) notation instead of legacy backticked `...`." + (fixWith [replaceStart id params 1 "$(", replaceEnd id params 1 ")"]) checkBackticks _ _ = return () prop_checkIndirectExpansion1 = verify checkIndirectExpansion "${foo$n}" @@ -1344,7 +1504,7 @@ prop_checkIndirectExpansion3 = verify checkIndirectExpansion "${$#}" prop_checkIndirectExpansion4 = verify checkIndirectExpansion "${var${n}_$((i%2))}" prop_checkIndirectExpansion5 = verifyNot checkIndirectExpansion "${bar}" -checkIndirectExpansion _ (T_DollarBraced i (T_NormalWord _ contents)) = +checkIndirectExpansion _ (T_DollarBraced i _ (T_NormalWord _ contents)) = when (isIndirection contents) $ err i 2082 "To expand via indirection, use arrays, ${!name} or (for sh only) eval." where @@ -1354,7 +1514,7 @@ isIndirectionPart t = case t of T_DollarExpansion _ _ -> Just True T_Backticked _ _ -> Just True - T_DollarBraced _ _ -> Just True + T_DollarBraced _ _ _ -> Just True T_DollarArithmetic _ _ -> Just True T_Literal _ s -> if all isVariableChar s then Nothing @@ -1371,7 +1531,8 @@ prop_checkInexplicablyUnquoted6 = verifyNot checkInexplicablyUnquoted "\"$dir\"some_stuff\"$file\"" prop_checkInexplicablyUnquoted7 = verifyNot checkInexplicablyUnquoted "${dir/\"foo\"/\"bar\"}" prop_checkInexplicablyUnquoted8 = verifyNot checkInexplicablyUnquoted " 'foo'\\\n 'bar'" -checkInexplicablyUnquoted _ (T_NormalWord id tokens) = mapM_ check (tails tokens) +prop_checkInexplicablyUnquoted9 = verifyNot checkInexplicablyUnquoted "[[ $x =~ \"foo\"(\"bar\"|\"baz\") ]]" +checkInexplicablyUnquoted params (T_NormalWord id tokens) = mapM_ check (tails tokens) where check (T_SingleQuoted _ _:T_Literal id str:_) | not (null str) && all isAlphaNum str = @@ -1380,19 +1541,28 @@ check (T_DoubleQuoted _ a:trapped:T_DoubleQuoted _ b:_) = case trapped of T_DollarExpansion id _ -> warnAboutExpansion id - T_DollarBraced id _ -> warnAboutExpansion id + T_DollarBraced id _ _ -> warnAboutExpansion id T_Literal id s -> - unless (quotesSingleThing a && quotesSingleThing b) $ + unless (quotesSingleThing a && quotesSingleThing b || isRegex (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 = + case t of + (T_Redirecting {} : _) -> False + (a:(TC_Binary _ _ "=~" lhs rhs):rest) -> getId a == getId rhs + _:rest -> isRegex rest + _ -> False + -- If the surrounding quotes quote single things, like "$foo"_and_then_some_"$stuff", -- the quotes were probably intentional and harmless. quotesSingleThing x = case x of [T_DollarExpansion _ _] -> True - [T_DollarBraced _ _] -> True + [T_DollarBraced _ _ _] -> True [T_Backticked _ _] -> True _ -> False @@ -1433,17 +1603,18 @@ prop_checkSpuriousExec6 = verify checkSpuriousExec "exec foo > file; cmd" 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" checkSpuriousExec _ = doLists where - doLists (T_Script _ _ cmds) = doList cmds - doLists (T_BraceGroup _ cmds) = doList cmds - doLists (T_WhileExpression _ _ cmds) = doList cmds - doLists (T_UntilExpression _ _ cmds) = doList cmds - doLists (T_ForIn _ _ _ cmds) = doList cmds - doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds + doLists (T_Script _ _ cmds) = doList cmds False + doLists (T_BraceGroup _ cmds) = doList cmds False + doLists (T_WhileExpression _ _ cmds) = doList cmds True + doLists (T_UntilExpression _ _ cmds) = doList cmds True + doLists (T_ForIn _ _ _ cmds) = doList cmds True + doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds True doLists (T_IfExpression _ thens elses) = do - mapM_ (\(_, l) -> doList l) thens - doList elses + mapM_ (\(_, l) -> doList l False) thens + doList elses False doLists _ = return () stripCleanup = reverse . dropWhile cleanup . reverse @@ -1452,10 +1623,15 @@ cleanup _ = False doList = doList' . stripCleanup - doList' t@(current:following:_) = do + -- The second parameter is True if we are in a loop + -- In that case we should emit the warning also if `exec' is the last statement + doList' t@(current:following:_) False = do commentIfExec current - doList (tail t) - doList' _ = return () + doList (tail t) False + doList' (current:tail) True = do + commentIfExec current + doList tail True + doList' _ _ = return () commentIfExec (T_Pipeline id _ list) = mapM_ commentIfExec $ take 1 list @@ -1525,6 +1701,7 @@ prop_subshellAssignmentCheck17 = verifyNotTree subshellAssignmentCheck "foo=${ { bar=$(baz); } 2>&1; }; echo $foo $bar" 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" subshellAssignmentCheck params t = let flow = variableFlow params check = findSubshelled flow [("oops",[])] Map.empty @@ -1611,37 +1788,71 @@ prop_checkSpacefulness34= verifyTree checkSpacefulness "declare foo$n=$1" prop_checkSpacefulness35= verifyNotTree checkSpacefulness "echo ${1+\"$1\"}" prop_checkSpacefulness36= verifyNotTree checkSpacefulness "arg=$#; echo $arg" +prop_checkSpacefulness37= verifyNotTree checkSpacefulness "@test 'status' {\n [ $status -eq 0 ]\n}" +prop_checkSpacefulness37v = verifyTree checkVerboseSpacefulness "@test 'status' {\n [ $status -eq 0 ]\n}" -checkSpacefulness params t = +-- This is slightly awkward because we want to support structured +-- optional checks based on nearly the same logic +checkSpacefulness params = checkSpacefulness' onFind params + where + emit x = tell [x] + onFind spaces token _ = + when spaces $ + if isDefaultAssignment (parentMap params) token + then + emit $ makeComment InfoC (getId token) 2223 + "This default assignment may cause DoS due to globbing. Quote it." + else + emit $ makeCommentWithFix InfoC (getId token) 2086 + "Double quote to prevent globbing and word splitting." + (addDoubleQuotesAround params token) + + isDefaultAssignment parents token = + let modifier = getBracedModifier $ bracedString token in + any (`isPrefixOf` modifier) ["=", ":="] + && isParamTo parents ":" token + + +prop_checkSpacefulness4v= verifyTree checkVerboseSpacefulness "foo=3; foo=$(echo $foo)" +prop_checkSpacefulness8v= verifyTree checkVerboseSpacefulness "a=foo\\ bar; a=foo; rm $a" +prop_checkSpacefulness28v = verifyTree checkVerboseSpacefulness "exec {n}>&1; echo $n" +prop_checkSpacefulness36v = verifyTree checkVerboseSpacefulness "arg=$#; echo $arg" +checkVerboseSpacefulness params = checkSpacefulness' onFind params + where + onFind spaces token name = + when (not spaces && name `notElem` specialVariablesWithoutSpaces) $ + tell [makeCommentWithFix StyleC (getId token) 2248 + "Prefer double quoting even when variables don't contain special characters." + (addDoubleQuotesAround params token)] + +addDoubleQuotesAround params token = (surroundWidth (getId token) params "\"") +checkSpacefulness' + :: (Bool -> Token -> String -> Writer [TokenComment] ()) -> + Parameters -> Token -> [TokenComment] +checkSpacefulness' onFind params t = doVariableFlowAnalysis readF writeF (Map.fromList defaults) (variableFlow params) where defaults = zip variablesWithoutSpaces (repeat False) - hasSpaces name = do - map <- get - return $ Map.findWithDefault True name map + hasSpaces name = gets (Map.findWithDefault True name) setSpaces name bool = modify $ Map.insert name bool readF _ token name = do spaces <- hasSpaces name - return [warning | - isExpansion token && spaces + let needsQuoting = + isExpansion token && not (isArrayExpansion token) -- There's another warning for this && not (isCountingReference token) && not (isQuoteFree parents token) && not (isQuotedAlternativeReference token) - && not (usedAsCommandName parents token)] + && not (usedAsCommandName parents token) + + return . execWriter $ when needsQuoting $ onFind spaces token name + where - warning = - if isDefaultAssignment (parentMap params) token - then - makeComment InfoC (getId token) 2223 - "This default assignment may cause DoS due to globbing. Quote it." - else - makeComment InfoC (getId token) 2086 - "Double quote to prevent globbing and word splitting." + emit x = tell [x] writeF _ _ name (DataString SourceExternal) = setSpaces name True >> return [] writeF _ _ name (DataString SourceInteger) = setSpaces name False >> return [] @@ -1658,7 +1869,7 @@ isExpansion t = case t of - (T_DollarBraced _ _ ) -> True + (T_DollarBraced _ _ _ ) -> True _ -> False isSpacefulWord :: (String -> Bool) -> [Token] -> Bool @@ -1672,7 +1883,7 @@ T_Extglob {} -> True T_Literal _ s -> s `containsAny` globspace T_SingleQuoted _ s -> s `containsAny` globspace - T_DollarBraced _ _ -> spacefulF $ getBracedReference $ bracedString x + T_DollarBraced _ _ _ -> spacefulF $ getBracedReference $ bracedString x T_NormalWord _ w -> isSpacefulWord spacefulF w T_DoubleQuoted _ w -> isSpacefulWord spacefulF w _ -> False @@ -1680,12 +1891,24 @@ globspace = "*?[] \t\n" containsAny s = any (`elem` s) - isDefaultAssignment parents token = - let modifier = getBracedModifier $ bracedString token in - isExpansion token - && any (`isPrefixOf` modifier) ["=", ":="] - && isParamTo parents ":" token +prop_CheckVariableBraces1 = verify checkVariableBraces "a='123'; echo $a" +prop_CheckVariableBraces2 = verifyNot checkVariableBraces "a='123'; echo ${a}" +prop_CheckVariableBraces3 = verifyNot checkVariableBraces "#shellcheck disable=SC2016\necho '$a'" +prop_CheckVariableBraces4 = verifyNot checkVariableBraces "echo $* $1" +checkVariableBraces params t = + case t of + T_DollarBraced id False _ -> + unless (name `elem` unbracedVariables) $ + styleWithFix id 2250 + "Prefer putting braces around variable references even when not strictly required." + (fixFor t) + _ -> return () + where + name = getBracedReference $ bracedString t + fixFor token = fixWith [replaceStart (getId token) params 1 "${" + ,replaceEnd (getId token) params 0 "}"] + prop_checkQuotesInLiterals1 = verifyTree checkQuotesInLiterals "param='--foo=\"bar\"'; app $param" prop_checkQuotesInLiterals1a= verifyTree checkQuotesInLiterals "param=\"--foo='lolbar'\"; app $param" prop_checkQuotesInLiterals2 = verifyNotTree checkQuotesInLiterals "param='--foo=\"bar\"'; app \"$param\"" @@ -1716,7 +1939,7 @@ return [] writeF _ _ _ _ = return [] - forToken map (T_DollarBraced id t) = + forToken map (T_DollarBraced id _ t) = -- skip getBracedReference here to avoid false positives on PE Map.lookup (concat . oversimplify $ t) map forToken quoteMap (T_DoubleQuoted id tokens) = @@ -1730,7 +1953,7 @@ squashesQuotes t = case t of - T_DollarBraced id _ -> "#" `isPrefixOf` bracedString t + T_DollarBraced id _ _ -> "#" `isPrefixOf` bracedString t _ -> False readF _ expr name = do @@ -1741,40 +1964,66 @@ && not (isQuoteFree parents expr) && not (squashesQuotes expr) then [ - makeComment WarningC (fromJust assignment) 2089 - "Quotes/backslashes will be treated literally. Use an array.", + makeComment WarningC (fromJust assignment) 2089 $ + "Quotes/backslashes will be treated literally. " ++ suggestion, makeComment WarningC (getId expr) 2090 "Quotes/backslashes in this variable will not be respected." ] else []) + suggestion = + if supportsArrays (shellType params) + then "Use an array." + else "Rewrite using set/\"$@\" or functions." prop_checkFunctionsUsedExternally1 = verifyTree checkFunctionsUsedExternally "foo() { :; }; sudo foo" prop_checkFunctionsUsedExternally2 = - verifyTree checkFunctionsUsedExternally "alias f='a'; xargs -n 1 f" + verifyTree checkFunctionsUsedExternally "alias f='a'; xargs -0 f" +prop_checkFunctionsUsedExternally2b= + verifyNotTree checkFunctionsUsedExternally "alias f='a'; find . -type f" +prop_checkFunctionsUsedExternally2c= + verifyTree checkFunctionsUsedExternally "alias f='a'; find . -type f -exec f +" prop_checkFunctionsUsedExternally3 = verifyNotTree checkFunctionsUsedExternally "f() { :; }; echo f" prop_checkFunctionsUsedExternally4 = verifyNotTree checkFunctionsUsedExternally "foo() { :; }; sudo \"foo\"" +prop_checkFunctionsUsedExternally5 = + verifyTree checkFunctionsUsedExternally "foo() { :; }; ssh host foo" +prop_checkFunctionsUsedExternally6 = + verifyNotTree checkFunctionsUsedExternally "foo() { :; }; ssh host echo foo" +prop_checkFunctionsUsedExternally7 = + verifyNotTree checkFunctionsUsedExternally "install() { :; }; sudo apt-get install foo" checkFunctionsUsedExternally params t = runNodeAnalysis checkCommand params t where - invokingCmds = [ - "chroot", - "find", - "screen", - "ssh", - "su", - "sudo", - "xargs" - ] checkCommand _ t@(T_SimpleCommand _ _ (cmd:args)) = - let name = fromMaybe "" $ getCommandBasename t in - when (name `elem` invokingCmds) $ - mapM_ (checkArg name) args + case getCommandBasename t of + Just name -> do + let argStrings = map (\x -> (fromMaybe "" $ getLiteralString x, x)) args + let candidates = getPotentialCommands name argStrings + mapM_ (checkArg name) candidates + _ -> return () checkCommand _ _ = return () + -- Try to pick out the argument[s] that may be commands + getPotentialCommands name argAndString = + case name of + "chroot" -> firstNonFlag + "screen" -> firstNonFlag + "sudo" -> firstNonFlag + "xargs" -> firstNonFlag + "tmux" -> firstNonFlag + "ssh" -> take 1 $ drop 1 $ dropFlags argAndString + "find" -> take 1 $ drop 1 $ + dropWhile (\x -> fst x `notElem` findExecFlags) argAndString + _ -> [] + where + firstNonFlag = take 1 $ dropFlags argAndString + 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):) @@ -1782,10 +2031,11 @@ | t `isUnqualifiedCommand` "alias" = mapM_ getAlias args findFunctions _ = return () getAlias arg = - let string = concat $ oversimplify arg + let string = onlyLiteralString arg in when ('=' `elem` string) $ modify ((takeWhile (/= '=') string, getId arg):) - checkArg cmd arg = potentially $ do + + checkArg cmd (_, arg) = potentially $ do literalArg <- getUnquotedLiteral arg -- only consider unquoted literals definitionId <- Map.lookup literalArg functions return $ do @@ -1835,6 +2085,12 @@ prop_checkUnused38= verifyTree checkUnusedAssignments "(( a=42 ))" prop_checkUnused39= verifyNotTree checkUnusedAssignments "declare -x -f foo" prop_checkUnused40= verifyNotTree checkUnusedAssignments "arr=(1 2); num=2; echo \"${arr[@]:num}\"" +prop_checkUnused41= verifyNotTree checkUnusedAssignments "@test 'foo' {\ntrue\n}\n" +prop_checkUnused42= verifyNotTree checkUnusedAssignments "DEFINE_string foo '' ''; echo \"${FLAGS_foo}\"" +prop_checkUnused43= verifyTree checkUnusedAssignments "DEFINE_string foo '' ''" +prop_checkUnused44= verifyNotTree checkUnusedAssignments "DEFINE_string \"foo$ibar\" x y" +prop_checkUnused45= verifyTree checkUnusedAssignments "readonly foo=bar" +prop_checkUnused46= verifyTree checkUnusedAssignments "readonly foo=(bar)" checkUnusedAssignments params t = execWriter (mapM_ warnFor unused) where flow = variableFlow params @@ -1893,7 +2149,11 @@ prop_checkUnassignedReferences34= verifyNotTree checkUnassignedReferences "declare -A foo; (( foo[bar] ))" prop_checkUnassignedReferences35= verifyNotTree checkUnassignedReferences "echo ${arr[foo-bar]:?fail}" prop_checkUnassignedReferences36= verifyNotTree checkUnassignedReferences "read -a foo -r <<<\"foo bar\"; echo \"$foo\"" -checkUnassignedReferences params t = warnings +prop_checkUnassignedReferences37= verifyNotTree checkUnassignedReferences "var=howdy; printf -v 'array[0]' %s \"$var\"; printf %s \"${array[0]}\";" +prop_checkUnassignedReferences38= verifyTree (checkUnassignedReferences' True) "echo $VAR" + +checkUnassignedReferences = checkUnassignedReferences' False +checkUnassignedReferences' includeGlobals params t = warnings where (readMap, writeMap) = execState (mapM tally $ variableFlow params) (Map.empty, Map.empty) defaultAssigned = Map.fromList $ map (\a -> (a, ())) $ filter (not . null) internalVariables @@ -1938,8 +2198,11 @@ return $ " (did you mean '" ++ match ++ "'?)" warningFor var place = do + guard $ isVariableName var guard . not $ isInArray var place || isGuarded place - (if isLocal var then warningForLocals else warningForGlobals) var place + (if includeGlobals || isLocal var + then warningForLocals + else warningForGlobals) var place warnings = execWriter . sequence $ mapMaybe (uncurry warningFor) unassigned @@ -1948,10 +2211,10 @@ isInArray var t = any isArray $ getPath (parentMap params) t where isArray T_Array {} = True - isArray b@(T_DollarBraced _ _) | var /= getBracedReference (bracedString b) = True + isArray b@(T_DollarBraced _ _ _) | var /= getBracedReference (bracedString b) = True isArray _ = False - isGuarded (T_DollarBraced _ v) = + isGuarded (T_DollarBraced _ _ v) = rest `matches` guardRegex where name = concat $ oversimplify v @@ -2037,7 +2300,7 @@ prop_checkPrefixAssign1 = verify checkPrefixAssignmentReference "var=foo echo $var" prop_checkPrefixAssign2 = verifyNot checkPrefixAssignmentReference "var=$(echo $var) cmd" -checkPrefixAssignmentReference params t@(T_DollarBraced id value) = +checkPrefixAssignmentReference params t@(T_DollarBraced id _ value) = check path where name = getBracedReference $ bracedString t @@ -2084,27 +2347,20 @@ prop_checkCdAndBack3 = verifyNot checkCdAndBack "while [[ $PWD != / ]]; do cd ..; done" prop_checkCdAndBack4 = verify checkCdAndBack "cd $tmp; foo; cd -" prop_checkCdAndBack5 = verifyNot checkCdAndBack "cd ..; foo; cd .." -checkCdAndBack params = doLists +prop_checkCdAndBack6 = verify checkCdAndBack "for dir in */; do cd \"$dir\"; some_cmd; cd ..; done" +prop_checkCdAndBack7 = verifyNot checkCdAndBack "set -e; for dir in */; do cd \"$dir\"; some_cmd; cd ..; done" +prop_checkCdAndBack8 = verifyNot checkCdAndBack "cd tmp\nfoo\n# shellcheck disable=SC2103\ncd ..\n" +checkCdAndBack params t = + unless (hasSetE params) $ mapM_ doList $ getCommandSequences t where - shell = shellType params - doLists (T_ForIn _ _ _ cmds) = doList cmds - doLists (T_ForArithmetic _ _ _ _ cmds) = doList cmds - doLists (T_WhileExpression _ _ cmds) = doList cmds - doLists (T_UntilExpression _ _ cmds) = doList cmds - doLists (T_Script _ _ cmds) = doList cmds - doLists (T_IfExpression _ thens elses) = do - mapM_ (\(_, l) -> doList l) thens - doList elses - doLists _ = return () - isCdRevert t = case oversimplify t of - ["cd", p] -> p `elem` ["..", "-"] + [_, p] -> p `elem` ["..", "-"] _ -> False - getCmd (T_Annotation id _ x) = getCmd x - getCmd (T_Pipeline id _ [x]) = getCommandName x - getCmd _ = Nothing + getCandidate (T_Annotation _ _ x) = getCandidate x + getCandidate (T_Pipeline id _ [x]) | x `isCommand` "cd" = return x + getCandidate _ = Nothing findCdPair list = case list of @@ -2114,14 +2370,9 @@ else findCdPair (b:rest) _ -> Nothing - - doList list = - let cds = filter ((== Just "cd") . getCmd) list in - potentially $ do - cd <- findCdPair cds - return $ info cd 2103 message - - message = "Use a ( subshell ) to avoid having to cd back." + doList list = potentially $ do + cd <- findCdPair $ mapMaybe getCandidate list + return $ info cd 2103 "Use a ( subshell ) to avoid having to cd back." prop_checkLoopKeywordScope1 = verify checkLoopKeywordScope "continue 2" prop_checkLoopKeywordScope2 = verify checkLoopKeywordScope "for f; do ( break; ); done" @@ -2197,6 +2448,7 @@ prop_checkUnpassedInFunctions10= verifyNotTree checkUnpassedInFunctions "foo() { echo $!; }; foo;" prop_checkUnpassedInFunctions11= verifyNotTree checkUnpassedInFunctions "foo() { bar() { echo $1; }; bar baz; }; foo;" prop_checkUnpassedInFunctions12= verifyNotTree checkUnpassedInFunctions "foo() { echo ${!var*}; }; foo;" +prop_checkUnpassedInFunctions13= verifyNotTree checkUnpassedInFunctions "# shellcheck disable=SC2120\nfoo() { echo $1; }\nfoo\n" checkUnpassedInFunctions params root = execWriter $ mapM_ warnForGroup referenceGroups where @@ -2248,18 +2500,25 @@ updateWith x@(name, _, _) = Map.insertWith (++) name [x] warnForGroup group = - when (all isArgumentless group) $ do - mapM_ suggestParams group - warnForDeclaration group + when (all isArgumentless group) $ + -- Allow ignoring SC2120 on the function to ignore all calls + let (name, func) = getFunction group + ignoring = shouldIgnoreCode params 2120 func + in unless ignoring $ do + mapM_ suggestParams group + warnForDeclaration func name suggestParams (name, _, thing) = info (getId thing) 2119 $ "Use " ++ name ++ " \"$@\" if function's $1 should mean script's $1." - warnForDeclaration ((name, _, _):_) = - warn (getId . fromJust $ Map.lookup name functionMap) 2120 $ + warnForDeclaration func name = + warn (getId func) 2120 $ name ++ " references arguments, but none are ever passed." + getFunction ((name, _, _):_) = + (name, fromJust $ Map.lookup name functionMap) + prop_checkOverridingPath1 = verify checkOverridingPath "PATH=\"$var/$foo\"" prop_checkOverridingPath2 = verify checkOverridingPath "PATH=\"mydir\"" prop_checkOverridingPath3 = verify checkOverridingPath "PATH=/cow/foo" @@ -2417,8 +2676,10 @@ prop_checkTestArgumentSplitting14 = verify checkTestArgumentSplitting "[[ \"$@\" == \"\" ]]" prop_checkTestArgumentSplitting15 = verifyNot checkTestArgumentSplitting "[[ \"$*\" == \"\" ]]" prop_checkTestArgumentSplitting16 = verifyNot checkTestArgumentSplitting "[[ -v foo[123] ]]" +prop_checkTestArgumentSplitting17 = verifyNot checkTestArgumentSplitting "#!/bin/ksh\n[ -e foo* ]" +prop_checkTestArgumentSplitting18 = verify checkTestArgumentSplitting "#!/bin/ksh\n[ -d foo* ]" checkTestArgumentSplitting :: Parameters -> Token -> Writer [TokenComment] () -checkTestArgumentSplitting _ t = +checkTestArgumentSplitting params t = case t of (TC_Unary _ typ op token) | isGlob token -> if op == "-v" @@ -2427,8 +2688,16 @@ err (getId token) 2208 $ "Use [[ ]] or quote arguments to -v to avoid glob expansion." else - err (getId token) 2144 $ - op ++ " doesn't work with globs. Use a for loop." + if (typ == SingleBracket && shellType params == Ksh) + then + -- Ksh appears to stop processing after unrecognized tokens, so operators + -- will effectively work with globs, but only the first match. + when (op `elem` ['-':c:[] | c <- "bcdfgkprsuwxLhNOGRS" ]) $ + warn (getId token) 2245 $ + op ++ " only applies to the first expansion of this glob. Use a loop to check any/all." + else + err (getId token) 2144 $ + op ++ " doesn't work with globs. Use a for loop." (TC_Nullary _ typ token) -> do checkBraces typ token @@ -2527,23 +2796,31 @@ prop_checkUncheckedPopd7 = verifyNotTree checkUncheckedCdPushdPopd "#!/bin/bash -e\npopd\nrm bar" prop_checkUncheckedPopd8 = verifyNotTree checkUncheckedCdPushdPopd "set -o errexit; popd; rm bar" prop_checkUncheckedPopd9 = verifyNotTree checkUncheckedCdPushdPopd "popd -n foo" +prop_checkUncheckedPopd10 = verifyNotTree checkUncheckedCdPushdPopd "cd ../.." +prop_checkUncheckedPopd11 = verifyNotTree checkUncheckedCdPushdPopd "cd ../.././.." +prop_checkUncheckedPopd12 = verifyNotTree checkUncheckedCdPushdPopd "cd /" +prop_checkUncheckedPopd13 = verifyTree checkUncheckedCdPushdPopd "cd ../../.../.." checkUncheckedCdPushdPopd params root = if hasSetE params then [] else execWriter $ doAnalysis checkElement root where - checkElement t@T_SimpleCommand {} = - when(name t `elem` ["cd", "pushd", "popd"] + checkElement t@T_SimpleCommand {} = do + let name = getName t + when(name `elem` ["cd", "pushd", "popd"] && not (isSafeDir t) - && not (name t `elem` ["pushd", "popd"] && ("n" `elem` map snd (getAllFlags t))) + && not (name `elem` ["pushd", "popd"] && ("n" `elem` map snd (getAllFlags t))) && not (isCondition $ getPath (parentMap params) t)) $ - warn (getId t) 2164 "Use 'cd ... || exit' or 'cd ... || return' in case cd fails." + warnWithFix (getId t) 2164 + ("Use '" ++ name ++ " ... || exit' or '" ++ name ++ " ... || return' in case " ++ name ++ " fails.") + (fixWith [replaceEnd (getId t) params 0 " || exit"]) checkElement _ = return () - name t = fromMaybe "" $ getCommandName t + getName t = fromMaybe "" $ getCommandName t isSafeDir t = case oversimplify t of - [_, ".."] -> True; + [_, str] -> str `matches` regex _ -> False + regex = mkRegex "^/*((\\.|\\.\\.)/+)*(\\.|\\.\\.)?$" 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" @@ -2590,7 +2867,7 @@ parameters = oversimplify command guard $ opposite `notElem` parameters return $ warn id 2171 $ - "Found trailing " ++ str ++ " outside test. Missing " ++ opposite ++ "?" + "Found trailing " ++ str ++ " outside test. Add missing " ++ opposite ++ " or quote if intentional." _ -> return () invert s = case s of @@ -2653,7 +2930,7 @@ case drop 1 $ getPath (parentMap params) t of T_DollarExpansion _ [_] : _ -> True T_Backticked _ [_] : _ -> True - T_Annotation _ _ u : _ -> isInExpansion u + t@T_Annotation {} : _ -> isInExpansion t _ -> False getDanglingRedirect token = case token of @@ -2695,7 +2972,7 @@ T_Literal id str -> [(id,str)] _ -> [] guard $ '=' `elem` str - return $ warn id 2191 "The = here is literal. To assign by index, use ( [index]=value ) with no spaces. To keep as literal, quote it." + 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 "\"") in if null literalEquals && isAssociative then warn (getId t) 2190 "Elements in associative arrays need index, e.g. array=( [index]=value ) ." @@ -2712,7 +2989,7 @@ prop_checkUnmatchableCases7 = verifyNot checkUnmatchableCases "case $f in $(x)) true;; asdf) false;; esac" prop_checkUnmatchableCases8 = verify checkUnmatchableCases "case $f in cow) true;; bar|cow) false;; esac" prop_checkUnmatchableCases9 = verifyNot checkUnmatchableCases "case $f in x) true;;& x) false;; esac" -checkUnmatchableCases _ t = +checkUnmatchableCases params t = case t of T_CaseExpression _ word list -> do -- Check all patterns for whether they can ever match @@ -2737,6 +3014,7 @@ where fst3 (x,_,_) = x snd3 (_,x,_) = x + tp = tokenPositions params check target candidate = potentially $ do candidateGlob <- wordToPseudoGlob candidate guard . not $ pseudoGlobsCanOverlap target candidateGlob @@ -2747,10 +3025,16 @@ checkDoms ((glob, Just x), rest) = case filter (\(_, p) -> x `pseudoGlobIsSuperSetof` p) valids of ((first,_):_) -> do - warn (getId glob) 2221 "This pattern always overrides a later one." - warn (getId first) 2222 "This pattern never matches because of a previous pattern." + warn (getId glob) 2221 $ "This pattern always overrides a later one" <> patternContext (getId first) + warn (getId first) 2222 $ "This pattern never matches because of a previous pattern" <> patternContext (getId glob) _ -> return () where + patternContext :: Id -> String + patternContext id = + case posLine . fst <$> Map.lookup id tp of + Just l -> " on line " <> show l <> "." + _ -> "." + valids = concatMap f rest f (x, Just y) = [(x,y)] f _ = [] @@ -2805,14 +3089,14 @@ T_DollarExpansion id _ -> forCommand id T_DollarBraceCommandExpansion id _ -> forCommand id T_Backticked id _ -> forCommand id - T_DollarBraced id str | + T_DollarBraced id _ str | not (isCountingReference part) && not (isQuotedAlternativeReference part) && not (getBracedReference (bracedString part) `elem` variablesWithoutSpaces) -> warn id 2206 $ if shellType params == Ksh - then "Quote to prevent word splitting, or split robustly with read -A or while read." - else "Quote to prevent word splitting, or split robustly with mapfile or read -a." + then "Quote to prevent word splitting/globbing, or split robustly with read -A or while read." + else "Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a." _ -> return () forCommand id = @@ -3058,6 +3342,104 @@ unless (str == "file") $ -- This would be confusing warn id 2238 "Redirecting to/from command name instead of file. Did you want pipes/xargs (or quote to ignore)?" _ -> return () + +prop_checkNullaryExpansionTest1 = verify checkNullaryExpansionTest "[[ $(a) ]]" +prop_checkNullaryExpansionTest2 = verify checkNullaryExpansionTest "[[ $a ]]" +prop_checkNullaryExpansionTest3 = verifyNot checkNullaryExpansionTest "[[ $a=1 ]]" +prop_checkNullaryExpansionTest4 = verifyNot checkNullaryExpansionTest "[[ -n $(a) ]]" +prop_checkNullaryExpansionTest5 = verify checkNullaryExpansionTest "[[ \"$a$b\" ]]" +prop_checkNullaryExpansionTest6 = verify checkNullaryExpansionTest "[[ `x` ]]" +checkNullaryExpansionTest params t = + case t of + TC_Nullary _ _ word -> + case getWordParts word of + [t] | isCommandSubstitution t -> + styleWithFix id 2243 "Prefer explicit -n to check for output (or run command without [/[[ to check for success)." fix + + -- If they're constant, you get SC2157 &co + x | all (not . isConstant) x -> + styleWithFix id 2244 "Prefer explicit -n to check non-empty string (or use =/-ne to check boolean/integer)." fix + _ -> return () + where + id = getId word + fix = fixWith [replaceStart id params 0 "-n "] + _ -> return () + + +prop_checkDollarQuoteParen1 = verify checkDollarQuoteParen "$\"(foo)\"" +prop_checkDollarQuoteParen2 = verify checkDollarQuoteParen "$\"{foo}\"" +prop_checkDollarQuoteParen3 = verifyNot checkDollarQuoteParen "\"$(foo)\"" +prop_checkDollarQuoteParen4 = verifyNot checkDollarQuoteParen "$\"..\"" +checkDollarQuoteParen params t = + case t of + T_DollarDoubleQuoted id ((T_Literal _ (c:_)):_) | c `elem` "({" -> + warnWithFix id 2247 "Flip leading $ and \" if this should be a quoted substitution." (fix id) + _ -> return () + where + fix id = fixWith [replaceStart id params 2 "\"$"] + +prop_checkDefaultCase1 = verify checkDefaultCase "case $1 in a) true ;; esac" +prop_checkDefaultCase2 = verify checkDefaultCase "case $1 in ?*?) true ;; *? ) true ;; esac" +prop_checkDefaultCase3 = verifyNot checkDefaultCase "case $1 in x|*) true ;; esac" +prop_checkDefaultCase4 = verifyNot checkDefaultCase "case $1 in **) true ;; esac" +checkDefaultCase _ t = + case t of + T_CaseExpression id _ list -> + unless (any canMatchAny list) $ + info id 2249 "Consider adding a default *) case, even if it just exits with error." + _ -> return () + where + canMatchAny (_, list, _) = any canMatchAny' list + -- hlint objects to 'pattern' as a variable name + canMatchAny' pat = fromMaybe False $ do + pg <- wordToExactPseudoGlob pat + return $ pseudoGlobIsSuperSetof pg [PGMany] + +prop_checkUselessBang1 = verify checkUselessBang "set -e; ! true; rest" +prop_checkUselessBang2 = verifyNot checkUselessBang "! true; rest" +prop_checkUselessBang3 = verify checkUselessBang "set -e; while true; do ! true; done" +prop_checkUselessBang4 = verifyNot checkUselessBang "set -e; if ! true; then true; fi" +prop_checkUselessBang5 = verifyNot checkUselessBang "set -e; ( ! true )" +prop_checkUselessBang6 = verify checkUselessBang "set -e; { ! true; }" +prop_checkUselessBang7 = verifyNot checkUselessBang "set -e; x() { ! [ x ]; }" +prop_checkUselessBang8 = verifyNot checkUselessBang "set -e; if { ! true; }; then true; fi" +prop_checkUselessBang9 = verifyNot checkUselessBang "set -e; while ! true; do true; done" +checkUselessBang params t = when (hasSetE params) $ mapM_ check (getNonReturningCommands t) + where + check t = + case t of + T_Banged id cmd | not $ isCondition (getPath (parentMap params) t) -> + addComment $ makeCommentWithFix InfoC id 2251 + "This ! is not on a condition and skips errexit. Use `&& exit 1` instead, or make sure $? is checked." + (fixWith [replaceStart id params 1 "", replaceEnd (getId cmd) params 0 " && exit 1"]) + _ -> return () + + -- Get all the subcommands that aren't likely to be the return value + getNonReturningCommands :: Token -> [Token] + getNonReturningCommands t = + case t of + T_Script _ _ list -> dropLast list + T_BraceGroup _ list -> if isFunctionBody t then dropLast list else list + T_Subshell _ list -> dropLast list + T_WhileExpression _ conds cmds -> dropLast conds ++ cmds + T_UntilExpression _ conds cmds -> dropLast conds ++ cmds + T_ForIn _ _ _ list -> list + T_ForArithmetic _ _ _ _ list -> list + T_Annotation _ _ t -> getNonReturningCommands t + T_IfExpression _ conds elses -> + concatMap (dropLast . fst) conds ++ concatMap snd conds ++ elses + _ -> [] + + isFunctionBody t = + case getPath (parentMap params) t of + _:T_Function {}:_-> True + _ -> False + + dropLast t = + case t of + [_] -> [] + x:rest -> x : dropLast rest + _ -> [] return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/src/ShellCheck/Analyzer.hs b/src/ShellCheck/Analyzer.hs --- a/src/ShellCheck/Analyzer.hs +++ b/src/ShellCheck/Analyzer.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -17,7 +17,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -} -module ShellCheck.Analyzer (analyzeScript) where +module ShellCheck.Analyzer (analyzeScript, ShellCheck.Analyzer.optionalChecks) where import ShellCheck.Analytics import ShellCheck.AnalyzerLib @@ -25,6 +25,7 @@ import Data.List import Data.Monoid import qualified ShellCheck.Checks.Commands +import qualified ShellCheck.Checks.Custom import qualified ShellCheck.Checks.ShellSupport @@ -41,5 +42,10 @@ checkers params = mconcat $ map ($ params) [ ShellCheck.Checks.Commands.checker, + ShellCheck.Checks.Custom.checker, ShellCheck.Checks.ShellSupport.checker + ] + +optionalChecks = mconcat $ [ + ShellCheck.Analytics.optionalChecks ] 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-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -20,27 +20,29 @@ {-# LANGUAGE FlexibleContexts #-} {-# LANGUAGE TemplateHaskell #-} module ShellCheck.AnalyzerLib where -import ShellCheck.AST -import ShellCheck.ASTLib -import ShellCheck.Data -import ShellCheck.Interface -import ShellCheck.Parser -import ShellCheck.Regex -import Control.Arrow (first) -import Control.Monad.Identity -import Control.Monad.RWS -import Control.Monad.State -import Control.Monad.Writer -import Data.Char -import Data.List -import qualified Data.Map as Map -import Data.Maybe -import Data.Semigroup +import ShellCheck.AST +import ShellCheck.ASTLib +import ShellCheck.Data +import ShellCheck.Interface +import ShellCheck.Parser +import ShellCheck.Regex -import Test.QuickCheck.All (forAllProperties) -import Test.QuickCheck.Test (maxSuccess, quickCheckWithResult, stdArgs) +import Control.Arrow (first) +import Control.DeepSeq +import Control.Monad.Identity +import Control.Monad.RWS +import Control.Monad.State +import Control.Monad.Writer +import Data.Char +import Data.List +import Data.Maybe +import Data.Semigroup +import qualified Data.Map as Map +import Test.QuickCheck.All (forAllProperties) +import Test.QuickCheck.Test (maxSuccess, quickCheckWithResult, stdArgs) + type Analysis = AnalyzerM () type AnalyzerM a = RWS Parameters [TokenComment] Cache a nullCheck = const $ return () @@ -75,14 +77,23 @@ composeAnalyzers f g x = f x >> g x data Parameters = Parameters { - hasLastpipe :: Bool, -- Whether this script has the 'lastpipe' option set/default. - hasSetE :: Bool, -- Whether this script has 'set -e' anywhere. - variableFlow :: [StackData], -- A linear (bad) analysis of data flow - parentMap :: Map.Map Id Token, -- A map from Id to parent Token - shellType :: Shell, -- The shell type, such as Bash or Ksh - shellTypeSpecified :: Bool, -- True if shell type was forced via flags - rootNode :: Token -- The root node of the AST - } + -- Whether this script has the 'lastpipe' option set/default. + hasLastpipe :: Bool, + -- Whether this script has 'set -e' anywhere. + hasSetE :: Bool, + -- A linear (bad) analysis of data flow + variableFlow :: [StackData], + -- A map from Id to parent Token + parentMap :: Map.Map Id Token, + -- The shell type, such as Bash or Ksh + shellType :: Shell, + -- True if shell type was forced via flags + shellTypeSpecified :: Bool, + -- The root node of the AST + rootNode :: Token, + -- map from token id to start and end position + tokenPositions :: Map.Map Id (Position, Position) + } deriving (Show) -- TODO: Cache results of common AST ops here data Cache = Cache {} @@ -109,11 +120,12 @@ data VariableState = Dead Token String | Alive deriving (Show) -defaultSpec root = spec { +defaultSpec pr = spec { asShellType = Nothing, asCheckSourced = False, - asExecutionMode = Executed -} where spec = newAnalysisSpec root + asExecutionMode = Executed, + asTokenPositions = prTokenPositions pr +} where spec = newAnalysisSpec (fromJust $ prRoot pr) pScript s = let @@ -121,13 +133,14 @@ psFilename = "script", psScript = s } - in prRoot . runIdentity $ parseScript (mockedSystemInterface []) pSpec + in runIdentity $ parseScript (mockedSystemInterface []) pSpec -- For testing. If parsed, returns whether there are any comments producesComments :: Checker -> String -> Maybe Bool producesComments c s = do - root <- pScript s - let spec = defaultSpec root + let pr = pScript s + prRoot pr + let spec = defaultSpec pr let params = makeParameters spec return . not . null $ runChecker params c @@ -142,7 +155,7 @@ } } -addComment note = tell [note] +addComment note = note `deepseq` tell [note] warn :: MonadWriter [TokenComment] m => Id -> Code -> String -> m () warn id code str = addComment $ makeComment WarningC id code str @@ -150,10 +163,27 @@ info id code str = addComment $ makeComment InfoC id code str style id code str = addComment $ makeComment StyleC id code str +warnWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m () +warnWithFix = addCommentWithFix WarningC +styleWithFix :: MonadWriter [TokenComment] m => Id -> Code -> String -> Fix -> m () +styleWithFix = addCommentWithFix StyleC + +addCommentWithFix :: MonadWriter [TokenComment] m => Severity -> Id -> Code -> String -> Fix -> m () +addCommentWithFix severity id code str fix = + addComment $ makeCommentWithFix severity id code str fix + +makeCommentWithFix :: Severity -> Id -> Code -> String -> Fix -> TokenComment +makeCommentWithFix severity id code str fix = + let comment = makeComment severity id code str + withFix = comment { + tcFix = Just fix + } + in withFix `deepseq` withFix + makeParameters spec = let params = Parameters { rootNode = root, - shellType = fromMaybe (determineShell root) $ asShellType spec, + shellType = fromMaybe (determineShell (asFallbackShell spec) root) $ asShellType spec, hasSetE = containsSetE root, hasLastpipe = case shellType params of @@ -162,9 +192,10 @@ Sh -> False Ksh -> True, - shellTypeSpecified = isJust $ asShellType spec, + shellTypeSpecified = isJust (asShellType spec) || isJust (asFallbackShell spec), parentMap = getParentTree root, - variableFlow = getVariableFlow params root + variableFlow = getVariableFlow params root, + tokenPositions = asTokenPositions spec } in params where root = asScript spec @@ -175,7 +206,7 @@ where isSetE t = case t of - T_Script _ str _ -> str `matches` re + T_Script _ (T_Literal _ str) _ -> str `matches` re T_SimpleCommand {} -> t `isUnqualifiedCommand` "set" && ("errexit" `elem` oversimplify t || @@ -196,19 +227,21 @@ _ -> False -prop_determineShell0 = determineShell (fromJust $ pScript "#!/bin/sh") == Sh -prop_determineShell1 = determineShell (fromJust $ pScript "#!/usr/bin/env ksh") == Ksh -prop_determineShell2 = determineShell (fromJust $ pScript "") == Bash -prop_determineShell3 = determineShell (fromJust $ pScript "#!/bin/sh -e") == Sh -prop_determineShell4 = determineShell (fromJust $ pScript - "#!/bin/ksh\n#shellcheck shell=sh\nfoo") == Sh -prop_determineShell5 = determineShell (fromJust $ pScript - "#shellcheck shell=sh\nfoo") == Sh -prop_determineShell6 = determineShell (fromJust $ pScript "#! /bin/sh") == Sh -prop_determineShell7 = determineShell (fromJust $ pScript "#! /bin/ash") == Dash -determineShell t = fromMaybe Bash $ do +prop_determineShell0 = determineShellTest "#!/bin/sh" == Sh +prop_determineShell1 = determineShellTest "#!/usr/bin/env ksh" == Ksh +prop_determineShell2 = determineShellTest "" == Bash +prop_determineShell3 = determineShellTest "#!/bin/sh -e" == Sh +prop_determineShell4 = determineShellTest "#!/bin/ksh\n#shellcheck shell=sh\nfoo" == Sh +prop_determineShell5 = determineShellTest "#shellcheck shell=sh\nfoo" == Sh +prop_determineShell6 = determineShellTest "#! /bin/sh" == Sh +prop_determineShell7 = determineShellTest "#! /bin/ash" == Dash +prop_determineShell8 = determineShellTest' (Just Ksh) "#!/bin/sh" == Sh + +determineShellTest = determineShellTest' Nothing +determineShellTest' fallbackShell = determineShell fallbackShell . fromJust . prRoot . pScript +determineShell fallbackShell t = fromMaybe Bash $ do shellString <- foldl mplus Nothing $ getCandidates t - shellForExecutable shellString + shellForExecutable shellString `mplus` fallbackShell where forAnnotation t = case t of @@ -219,7 +252,7 @@ getCandidates (T_Annotation _ annotations s) = map forAnnotation annotations ++ [Just $ fromShebang s] - fromShebang (T_Script _ s t) = executableFromShebang s + fromShebang (T_Script _ (T_Literal _ s) _) = executableFromShebang s -- Given a string like "/bin/bash" or "/usr/bin/env dash", -- return the shell basename like "bash" or "dash" @@ -357,10 +390,6 @@ parents params = getPath (parentMap params) -pathTo t = do - parents <- reader parentMap - return $ getPath parents t - -- Find the first match in a list where the predicate is Just True. -- Stops if it's Just False and ignores Nothing. findFirst :: (a -> Maybe Bool) -> [a] -> Maybe a @@ -404,6 +433,7 @@ assignFirst T_ForIn {} = True assignFirst T_SelectIn {} = True + assignFirst (T_BatsTest {}) = True assignFirst _ = False setRead t = @@ -421,6 +451,7 @@ T_Backticked _ _ -> SubshellScope "`..` expansion" T_Backgrounded _ _ -> SubshellScope "backgrounding &" T_Subshell _ _ -> SubshellScope "(..) group" + T_BatsTest {} -> SubshellScope "@bats test" T_CoProcBody _ _ -> SubshellScope "coproc" T_Redirecting {} -> if fromMaybe False causesSubshell @@ -461,6 +492,12 @@ guard $ op `elem` ["=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", "&=", "^=", "|="] return (t, t, name, DataString $ SourceFrom [rhs]) + T_BatsTest {} -> [ + (t, t, "lines", DataArray SourceExternal), + (t, t, "status", DataString SourceInteger), + (t, t, "output", DataString SourceExternal) + ] + -- Count [[ -v foo ]] as an "assignment". -- This is to prevent [ -v foo ] being unassigned or unused. TC_Unary id _ "-v" token -> maybeToList $ do @@ -473,7 +510,7 @@ guard . not . null $ str return (t, token, str, DataString SourceChecked) - T_DollarBraced _ l -> maybeToList $ do + T_DollarBraced _ _ l -> maybeToList $ do let string = bracedString t let modifier = getBracedModifier string guard $ ":=" `isPrefixOf` modifier @@ -509,10 +546,6 @@ (not $ any (`elem` flags) ["f", "F"]) then concatMap getReference rest else [] - "readonly" -> - if any (`elem` flags) ["f", "p"] - then [] - else concatMap getReference rest "trap" -> case rest of head:_ -> map (\x -> (head, head, x)) $ getVariablesFromLiteralToken head @@ -569,6 +602,11 @@ "mapfile" -> maybeToList $ getMapfileArray base rest "readarray" -> maybeToList $ getMapfileArray base rest + "DEFINE_boolean" -> maybeToList $ getFlagVariable rest + "DEFINE_float" -> maybeToList $ getFlagVariable rest + "DEFINE_integer" -> maybeToList $ getFlagVariable rest + "DEFINE_string" -> maybeToList $ getFlagVariable rest + _ -> [] where flags = map snd $ getAllFlags base @@ -620,7 +658,11 @@ getPrintfVariable list = f $ map (\x -> (x, getLiteralString x)) list where - f ((_, Just "-v") : (t, Just var) : _) = return (base, t, var, DataString $ SourceFrom list) + 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" @@ -634,10 +676,23 @@ return (base, lastArg, name, DataArray SourceExternal) -- get all the array variables used in read, e.g. read -a arr - getReadArrayVariables args = do + getReadArrayVariables args = map (getLiteralArray . snd) - (filter (\(x,_) -> getLiteralString x == Just "-a") (zip (args) (tail args))) + (filter (isArrayFlag . fst) (zip args (tail args))) + isArrayFlag x = fromMaybe False $ do + str <- getLiteralString x + return $ case str of + '-':'-':_ -> False + '-':str -> 'a' `elem` str + _ -> False + + -- get the FLAGS_ variable created by a shflags DEFINE_ call + getFlagVariable (n:v:_) = do + name <- getLiteralString n + return (base, n, "FLAGS_" ++ name, DataString $ SourceExternal) + getFlagVariable _ = Nothing + getModifiedVariableCommand _ = [] getIndexReferences s = fromMaybe [] $ do @@ -661,7 +716,7 @@ getReferencedVariables parents t = case t of - T_DollarBraced id l -> let str = bracedString t in + T_DollarBraced id _ l -> let str = bracedString t in (t, t, getBracedReference str) : map (\x -> (l, l, x)) ( getIndexReferences str @@ -680,6 +735,12 @@ then concatMap (getIfReference t) [lhs, rhs] else [] + T_BatsTest {} -> [ -- pretend @test references vars to avoid warnings + (t, t, "lines"), + (t, t, "status"), + (t, t, "output") + ] + t@(T_FdRedirect _ ('{':var) op) -> -- {foo}>&- references and closes foo [(t, t, takeWhile (/= '}') var) | isClosingFileOp op] x -> getReferencedVariableCommand x @@ -730,7 +791,7 @@ -- False: .*foo.* isConfusedGlobRegex :: String -> Bool isConfusedGlobRegex ('*':_) = True -isConfusedGlobRegex [x,'*'] | x /= '\\' = True +isConfusedGlobRegex [x,'*'] | x `notElem` "\\." = True isConfusedGlobRegex _ = False isVariableStartChar x = x == '_' || isAsciiLower x || isAsciiUpper x @@ -840,18 +901,17 @@ shouldIgnore note = any (shouldIgnoreFor (getCode note)) $ getPath parents (T_Bang $ tcId note) - shouldIgnoreFor num (T_Annotation _ anns _) = - any hasNum anns - where - hasNum (DisableComment ts) = num == ts - hasNum _ = False shouldIgnoreFor _ T_Include {} = not $ asCheckSourced asSpec - shouldIgnoreFor _ _ = False + shouldIgnoreFor code t = isAnnotationIgnoringCode code t parents = parentMap params getCode = cCode . tcComment +shouldIgnoreCode params code t = + any (isAnnotationIgnoringCode code) $ + getPath (parentMap params) t + -- Is this a ${#anything}, to get string length or array count? -isCountingReference (T_DollarBraced id token) = +isCountingReference (T_DollarBraced id _ token) = case concat $ oversimplify token of '#':_ -> True _ -> False @@ -860,7 +920,7 @@ -- FIXME: doesn't handle ${a:+$var} vs ${a:+"$var"} isQuotedAlternativeReference t = case t of - T_DollarBraced _ _ -> + T_DollarBraced _ _ _ -> getBracedModifier (bracedString t) `matches` re _ -> False where @@ -898,6 +958,17 @@ else do more <- process rest2 return $ (flag1, token1) : more + +supportsArrays shell = shell == Bash || shell == Ksh + +-- Returns true if the shell is Bash or Ksh (sorry for the name, Ksh) +isBashLike :: Parameters -> Bool +isBashLike params = + case shellType params of + Bash -> True + Ksh -> True + Dash -> False + Sh -> 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-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -42,11 +42,23 @@ return $ newPositionedComment { pcStartPos = fst span, pcEndPos = snd span, - pcComment = tcComment t + pcComment = tcComment t, + pcFix = tcFix t } where fail = error "Internal shellcheck error: id doesn't exist. Please report!" +shellFromFilename filename = foldl mplus Nothing candidates + where + shellExtensions = [(".ksh", Ksh) + ,(".bash", Bash) + ,(".bats", Bash) + ,(".dash", Dash)] + -- The `.sh` is too generic to determine the shell: + -- We fallback to Bash in this case and emit SC2148 if there is no shebang + candidates = + map (\(ext,sh) -> if ext `isSuffixOf` filename then Just sh else Nothing) shellExtensions + checkScript :: Monad m => SystemInterface m -> CheckSpec -> m CheckResult checkScript sys spec = do results <- checkScript (csScript spec) @@ -60,23 +72,37 @@ psFilename = csFilename spec, psScript = contents, psCheckSourced = csCheckSourced spec, + psIgnoreRC = csIgnoreRC spec, psShellTypeOverride = csShellTypeOverride spec } let parseMessages = prComments result + let tokenPositions = prTokenPositions result + let analysisSpec root = + as { + asScript = root, + asShellType = csShellTypeOverride spec, + asFallbackShell = shellFromFilename $ csFilename spec, + asCheckSourced = csCheckSourced spec, + asExecutionMode = Executed, + asTokenPositions = tokenPositions, + asOptionalChecks = csOptionalChecks spec + } where as = newAnalysisSpec root let analysisMessages = fromMaybe [] $ (arComments . analyzeScript . analysisSpec) <$> prRoot result - let translator = tokenToPosition (prTokenPositions result) + let translator = tokenToPosition tokenPositions return . nub . sortMessages . filter shouldInclude $ (parseMessages ++ map translator analysisMessages) shouldInclude pc = - let code = cCode (pcComment pc) + severity <= csMinSeverity spec && + case csIncludedWarnings spec of + Nothing -> code `notElem` csExcludedWarnings spec + Just includedWarnings -> code `elem` includedWarnings + where + code = cCode (pcComment pc) severity = cSeverity (pcComment pc) - in - code `notElem` csExcludedWarnings spec && - severity <= csMinSeverity spec sortMessages = sortBy (comparing order) order pc = @@ -90,13 +116,6 @@ cMessage comment) getPosition = pcStartPos - analysisSpec root = - as { - asScript = root, - asShellType = csShellTypeOverride spec, - asCheckSourced = csCheckSourced spec, - asExecutionMode = Executed - } where as = newAnalysisSpec root getErrors sys spec = sort . map getCode . crComments $ @@ -122,6 +141,21 @@ csCheckSourced = True } +checkOptionIncludes includes src = + checkWithSpec [] emptyCheckSpec { + csScript = src, + csIncludedWarnings = includes, + csCheckSourced = True + } + +checkWithRc rc = getErrors + (mockRcFile rc $ mockedSystemInterface []) + +checkWithIncludesAndSourcePath includes mapper = getErrors + (mockedSystemInterface includes) { + siFindSource = mapper + } + prop_findsParseIssue = check "echo \"$12\"" == [1037] prop_commentDisablesParseIssue1 = @@ -176,9 +210,13 @@ prop_worksWhenSourcing = null $ checkWithIncludes [("lib", "bar=1")] "source lib; echo \"$bar\"" +prop_worksWhenSourcingWithDashDash = + null $ checkWithIncludes [("lib", "bar=1")] "source -- lib; echo \"$bar\"" + prop_worksWhenDotting = null $ checkWithIncludes [("lib", "bar=1")] ". lib; echo \"$bar\"" +-- FIXME: This should really be giving [1093], "recursively sourced" prop_noInfiniteSourcing = [] == checkWithIncludes [("lib", "source lib")] "source lib" @@ -200,6 +238,12 @@ prop_recursiveParsing = [1037] == checkRecursive [("lib", "echo \"$10\"")] "source lib" +prop_nonRecursiveAnalysis = + [] == checkWithIncludes [("lib", "echo $1")] "source lib" + +prop_nonRecursiveParsing = + [] == checkWithIncludes [("lib", "echo \"$10\"")] "source lib" + prop_sourceDirectiveDoesntFollowFile = null $ checkWithIncludes [("foo", "source bar"), ("bar", "baz=3")] @@ -227,6 +271,124 @@ prop_sourcePartOfOriginalScript = -- #1181: -x disabled posix warning for 'source' 2039 `elem` checkWithIncludes [("./saywhat.sh", "echo foo")] "#!/bin/sh\nsource ./saywhat.sh" + +prop_spinBug1413 = null $ check "fun() {\n# shellcheck disable=SC2188\n> /dev/null\n}\n" + +prop_deducesTypeFromExtension = null result + where + result = checkWithSpec [] emptyCheckSpec { + csFilename = "file.ksh", + csScript = "(( 3.14 ))" + } + +prop_deducesTypeFromExtension2 = result == [2079] + where + result = checkWithSpec [] emptyCheckSpec { + csFilename = "file.bash", + csScript = "(( 3.14 ))" + } + +prop_shExtensionDoesntMatter = result == [2148] + where + result = checkWithSpec [] emptyCheckSpec { + csFilename = "file.sh", + csScript = "echo 'hello world'" + } + +prop_sourcedFileUsesOriginalShellExtension = result == [2079] + where + result = checkWithSpec [("file.ksh", "(( 3.14 ))")] emptyCheckSpec { + csFilename = "file.bash", + csScript = "source file.ksh", + csCheckSourced = True + } + +prop_canEnableOptionalsWithSpec = result == [2244] + where + result = checkWithSpec [] emptyCheckSpec { + csFilename = "file.sh", + csScript = "#!/bin/sh\n[ \"$1\" ]", + csOptionalChecks = ["avoid-nullary-conditions"] + } + +prop_optionIncludes1 = + -- expect 2086, but not included, so nothing reported + null $ checkOptionIncludes (Just [2080]) "#!/bin/sh\n var='a b'\n echo $var" + +prop_optionIncludes2 = + -- expect 2086, included, so it is reported + [2086] == checkOptionIncludes (Just [2086]) "#!/bin/sh\n var='a b'\n echo $var" + +prop_optionIncludes3 = + -- expect 2086, no inclusions provided, so it is reported + [2086] == checkOptionIncludes Nothing "#!/bin/sh\n var='a b'\n echo $var" + +prop_optionIncludes4 = + -- expect 2086 & 2154, only 2154 included, so only that's reported + [2154] == checkOptionIncludes (Just [2154]) "#!/bin/sh\n var='a b'\n echo $var\n echo $bar" + + +prop_readsRcFile = result == [] + where + result = checkWithRc "disable=2086" emptyCheckSpec { + csScript = "#!/bin/sh\necho $1", + csIgnoreRC = False + } + +prop_canUseNoRC = result == [2086] + where + result = checkWithRc "disable=2086" emptyCheckSpec { + csScript = "#!/bin/sh\necho $1", + csIgnoreRC = True + } + +prop_NoRCWontLookAtFile = result == [2086] + where + result = checkWithRc (error "Fail") emptyCheckSpec { + csScript = "#!/bin/sh\necho $1", + csIgnoreRC = True + } + +prop_brokenRcGetsWarning = result == [1134, 2086] + where + result = checkWithRc "rofl" emptyCheckSpec { + csScript = "#!/bin/sh\necho $1", + csIgnoreRC = False + } + +prop_canEnableOptionalsWithRc = result == [2244] + where + result = checkWithRc "enable=avoid-nullary-conditions" emptyCheckSpec { + csScript = "#!/bin/sh\n[ \"$1\" ]" + } + +prop_sourcePathRedirectsName = result == [2086] + where + f "dir/myscript" _ "lib" = return "foo/lib" + result = checkWithIncludesAndSourcePath [("foo/lib", "echo $1")] f emptyCheckSpec { + csScript = "#!/bin/bash\nsource lib", + csFilename = "dir/myscript", + csCheckSourced = True + } + +prop_sourcePathAddsAnnotation = result == [2086] + where + 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", + csCheckSourced = True + } + +prop_sourcePathRedirectsDirective = result == [2086] + where + 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 + } 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-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -94,6 +94,7 @@ ,checkSudoRedirect ,checkSudoArgs ,checkSourceArgs + ,checkChmodDashr ] buildCommandMap :: [CommandCheck] -> Map.Map CommandName (Token -> Analysis) @@ -208,6 +209,14 @@ prop_checkGrepRe13= verifyNot checkGrepRe "grep -- -foo bar*" prop_checkGrepRe14= verifyNot checkGrepRe "grep -e -foo bar*" prop_checkGrepRe15= verifyNot checkGrepRe "grep --regex -foo bar*" +prop_checkGrepRe16= verifyNot checkGrepRe "grep --include 'Foo*' file" +prop_checkGrepRe17= verifyNot checkGrepRe "grep --exclude 'Foo*' file" +prop_checkGrepRe18= verifyNot checkGrepRe "grep --exclude-dir 'Foo*' file" +prop_checkGrepRe19= verify checkGrepRe "grep -- 'Foo*' file" +prop_checkGrepRe20= verifyNot checkGrepRe "grep --fixed-strings 'Foo*' file" +prop_checkGrepRe21= verifyNot checkGrepRe "grep -o 'x*' file" +prop_checkGrepRe22= verifyNot checkGrepRe "grep --only-matching 'x*' file" +prop_checkGrepRe23= verifyNot checkGrepRe "grep '.*' file" checkGrepRe = CommandCheck (Basename "grep") check where check cmd = f cmd (arguments cmd) @@ -230,7 +239,7 @@ when (isGlob re) $ warn (getId re) 2062 "Quote the grep pattern so the shell won't interpret it." - unless (cmd `hasFlag` "F") $ do + unless (any (`elem` flags) grepGlobFlags) $ do let string = concat $ oversimplify re if isConfusedGlobRegex string then warn (getId re) 2063 "Grep uses regex, but this looks like a glob." @@ -238,6 +247,9 @@ char <- getSuspiciousRegexWildcard string return $ info (getId re) 2022 $ "Note that unlike globs, " ++ [char] ++ "* here matches '" ++ [char, char, char] ++ "' but not '" ++ wordStartingWith char ++ "'." + where + flags = map snd $ getAllFlags cmd + grepGlobFlags = ["fixed-strings", "F", "include", "exclude", "exclude-dir", "o", "only-matching"] wordStartingWith c = head . filter ([c] `isPrefixOf`) $ candidates @@ -270,7 +282,7 @@ warning id = warn id 2064 "Use single quotes, otherwise this expands now rather than when signalled." checkExpansions (T_DollarExpansion id _) = warning id checkExpansions (T_Backticked id _) = warning id - checkExpansions (T_DollarBraced id _) = warning id + checkExpansions (T_DollarBraced id _ _) = warning id checkExpansions (T_DollarArithmetic id _) = warning id checkExpansions _ = return () @@ -477,7 +489,7 @@ checkInteractiveSu = CommandCheck (Basename "su") f where f cmd = when (length (arguments cmd) <= 1) $ do - path <- pathTo cmd + path <- getPathM cmd when (all undirected path) $ info (getId cmd) 2117 "To run commands as another user, use su -c or sudo." @@ -526,54 +538,85 @@ prop_checkPrintfVar16= verifyNot checkPrintfVar "printf $'string'" prop_checkPrintfVar17= verify checkPrintfVar "printf '%-*s\\n' 1" prop_checkPrintfVar18= verifyNot checkPrintfVar "printf '%-*s\\n' 1 2" +prop_checkPrintfVar19= verifyNot checkPrintfVar "printf '%(%s)T'" +prop_checkPrintfVar20= verifyNot checkPrintfVar "printf '%d %(%s)T' 42" +prop_checkPrintfVar21= verify checkPrintfVar "printf '%d %(%s)T'" checkPrintfVar = CommandCheck (Exactly "printf") (f . arguments) where f (doubledash:rest) | getLiteralString doubledash == Just "--" = f rest f (dashv:var:rest) | getLiteralString dashv == Just "-v" = f rest f (format:params) = check format params f _ = return () - countFormats string = - case string of - '%':'%':rest -> countFormats rest - '%':'(':rest -> 1 + countFormats (dropWhile (/= ')') rest) - '%':rest -> regexBasedCountFormats rest + countFormats (dropWhile (/= '%') rest) - _:rest -> countFormats rest - [] -> 0 - - regexBasedCountFormats rest = - maybe 1 (foldl (\acc group -> acc + (if group == "*" then 1 else 0)) 1) (matchRegex re rest) - where - -- constructed based on specifications in "man printf" - re = mkRegex "#?-?\\+? ?0?(\\*|\\d*).?(\\d*|\\*)[diouxXfFeEgGaAcsb]" - -- \____ _____/\___ ____/ \____ ____/\________ ________/ - -- V V V V - -- flags field width precision format character - -- field width and precision can be specified with a '*' instead of a digit, - -- in which case printf will accept one more argument for each '*' used check format more = do fromMaybe (return ()) $ do string <- getLiteralString format - let vars = countFormats string - - return $ do - when (vars == 0 && more /= []) $ - err (getId format) 2182 - "This printf format string has no variables. Other arguments are ignored." - - when (vars > 0 - && ((length more) `mod` vars /= 0 || null more) - && all (not . mayBecomeMultipleArgs) more) $ - warn (getId format) 2183 $ - "This format string has " ++ show vars ++ " variables, but is passed " ++ show (length more) ++ " arguments." + let formats = getPrintfFormats string + let formatCount = length formats + let argCount = length more + return $ + case () of + () | argCount == 0 && formatCount == 0 -> + return () -- This is fine + () | formatCount == 0 && argCount > 0 -> + err (getId format) 2182 + "This printf format string has no variables. Other arguments are ignored." + () | any mayBecomeMultipleArgs more -> + return () -- We don't know so trust the user + () | argCount < formatCount && onlyTrailingTs formats argCount -> + return () -- Allow trailing %()Ts since they use the current time + () | argCount > 0 && argCount `mod` formatCount == 0 -> + return () -- Great: a suitable number of arguments + () -> + warn (getId format) 2183 $ + "This format string has " ++ show formatCount ++ " variables, but is passed " ++ show argCount ++ " arguments." unless ('%' `elem` concat (oversimplify format) || isLiteral format) $ info (getId format) 2059 "Don't use variables in the printf format string. Use printf \"..%s..\" \"$foo\"." + where + onlyTrailingTs format argCount = + all (== 'T') $ drop argCount format +prop_checkGetPrintfFormats1 = getPrintfFormats "%s" == "s" +prop_checkGetPrintfFormats2 = getPrintfFormats "%0*s" == "*s" +prop_checkGetPrintfFormats3 = getPrintfFormats "%(%s)T" == "T" +prop_checkGetPrintfFormats4 = getPrintfFormats "%d%%%(%s)T" == "dT" +prop_checkGetPrintfFormats5 = getPrintfFormats "%bPassed: %d, %bFailed: %d%b, Skipped: %d, %bErrored: %d%b\\n" == "bdbdbdbdb" +getPrintfFormats = getFormats + where + -- Get the arguments in the string as a string of type characters, + -- e.g. "Hello %s" -> "s" and "%(%s)T %0*d\n" -> "T*d" + getFormats :: String -> String + getFormats string = + case string of + '%':'%':rest -> getFormats rest + '%':'(':rest -> + case dropWhile (/= ')') rest of + ')':c:trailing -> c : getFormats trailing + _ -> "" + '%':rest -> regexBasedGetFormats rest + _:rest -> getFormats rest + [] -> "" + regexBasedGetFormats rest = + case matchRegex re rest of + Just [width, precision, typ, rest] -> + (if width == "*" then "*" else "") ++ + (if precision == "*" then "*" else "") ++ + typ ++ getFormats rest + Nothing -> take 1 rest ++ getFormats rest + where + -- constructed based on specifications in "man printf" + re = mkRegex "#?-?\\+? ?0?(\\*|\\d*)\\.?(\\d*|\\*)([diouxXfFeEgGaAcsbq])(.*)" + -- \____ _____/\___ ____/ \____ ____/\_________ _________/ \ / + -- V V V V V + -- flags field width precision format character rest + -- field width and precision can be specified with a '*' instead of a digit, + -- in which case printf will accept one more argument for each '*' used + prop_checkUuoeCmd1 = verify checkUuoeCmd "echo $(date)" prop_checkUuoeCmd2 = verify checkUuoeCmd "echo `date`" prop_checkUuoeCmd3 = verify checkUuoeCmd "echo \"$(date)\"" @@ -763,7 +806,7 @@ checkLocalScope = CommandCheck (Exactly "local") $ \t -> whenShell [Bash, Dash] $ do -- Ksh allows it, Sh doesn't support local path <- getPathM t - unless (any isFunction path) $ + unless (any isFunctionLike path) $ err (getId $ getCommandTokenOrThis t) 2168 "'local' is only valid in functions." prop_checkDeprecatedTempfile1 = verify checkDeprecatedTempfile "var=$(tempfile)" @@ -888,7 +931,7 @@ getPotentialPath = getLiteralStringExt f where f (T_Glob _ str) = return str - f (T_DollarBraced _ word) = + f (T_DollarBraced _ _ word) = let var = onlyLiteralString word in -- This shouldn't handle non-colon cases. if any (`isInfixOf` var) [":?", ":-", ":="] @@ -1033,6 +1076,17 @@ (file:arg1:_) -> warn (getId arg1) 2240 $ "The dot command does not support arguments in sh/dash. Set them as variables." _ -> return () + +prop_checkChmodDashr1 = verify checkChmodDashr "chmod -r 0755 dir" +prop_checkChmodDashr2 = verifyNot checkChmodDashr "chmod -R 0755 dir" +prop_checkChmodDashr3 = verifyNot checkChmodDashr "chmod a-r dir" +checkChmodDashr = CommandCheck (Basename "chmod") f + where + f t = mapM_ check $ arguments t + check t = potentially $ do + flag <- getLiteralString t + guard $ flag == "-r" + return $ warn (getId t) 2253 "Use -R to recurse, or explicitly a-r to remove read permissions." return [] runTests = $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |]) diff --git a/src/ShellCheck/Checks/Custom.hs b/src/ShellCheck/Checks/Custom.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Checks/Custom.hs @@ -0,0 +1,21 @@ +{- + This empty file is provided for ease of patching in site specific checks. + However, there are no guarantees regarding compatibility between versions. +-} + +{-# LANGUAGE TemplateHaskell #-} +module ShellCheck.Checks.Custom (checker, ShellCheck.Checks.Custom.runTests) where + +import ShellCheck.AnalyzerLib +import Test.QuickCheck + +checker :: Parameters -> Checker +checker params = Checker { + perScript = const $ return (), + perToken = const $ return () + } + +prop_CustomTestsWork = True + +return [] +runTests = $quickCheckAll 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 @@ -33,6 +33,7 @@ import Data.List import Data.Maybe import qualified Data.Map as Map +import qualified Data.Set as Set import Test.QuickCheck.All (forAllProperties) import Test.QuickCheck.Test (quickCheckWithResult, stdArgs, maxSuccess) @@ -136,6 +137,46 @@ prop_checkBashisms54= verify checkBashisms "#!/bin/sh\nfoo+=bar" prop_checkBashisms55= verify checkBashisms "#!/bin/sh\necho ${@%foo}" prop_checkBashisms56= verifyNot checkBashisms "#!/bin/sh\necho ${##}" +prop_checkBashisms57= verifyNot checkBashisms "#!/bin/dash\nulimit -c 0" +prop_checkBashisms58= verify checkBashisms "#!/bin/sh\nulimit -c 0" +prop_checkBashisms59 = verify checkBashisms "#!/bin/sh\njobs -s" +prop_checkBashisms60 = verifyNot checkBashisms "#!/bin/sh\njobs -p" +prop_checkBashisms61 = verifyNot checkBashisms "#!/bin/sh\njobs -lp" +prop_checkBashisms62 = verify checkBashisms "#!/bin/sh\nexport -f foo" +prop_checkBashisms63 = verifyNot checkBashisms "#!/bin/sh\nexport -p" +prop_checkBashisms64 = verify checkBashisms "#!/bin/sh\nreadonly -a" +prop_checkBashisms65 = verifyNot checkBashisms "#!/bin/sh\nreadonly -p" +prop_checkBashisms66 = verifyNot checkBashisms "#!/bin/sh\ncd -P ." +prop_checkBashisms67 = verify checkBashisms "#!/bin/sh\ncd -P -e ." +prop_checkBashisms68 = verify checkBashisms "#!/bin/sh\numask -p" +prop_checkBashisms69 = verifyNot checkBashisms "#!/bin/sh\numask -S" +prop_checkBashisms70 = verify checkBashisms "#!/bin/sh\ntrap -l" +prop_checkBashisms71 = verify checkBashisms "#!/bin/sh\ntype -a ls" +prop_checkBashisms72 = verifyNot checkBashisms "#!/bin/sh\ntype ls" +prop_checkBashisms73 = verify checkBashisms "#!/bin/sh\nunset -n namevar" +prop_checkBashisms74 = verifyNot checkBashisms "#!/bin/sh\nunset -f namevar" +prop_checkBashisms75 = verifyNot checkBashisms "#!/bin/sh\necho \"-n foo\"" +prop_checkBashisms76 = verifyNot checkBashisms "#!/bin/sh\necho \"-ne foo\"" +prop_checkBashisms77 = verifyNot checkBashisms "#!/bin/sh\necho -Q foo" +prop_checkBashisms78 = verify checkBashisms "#!/bin/sh\necho -ne foo" +prop_checkBashisms79 = verify checkBashisms "#!/bin/sh\nhash -l" +prop_checkBashisms80 = verifyNot checkBashisms "#!/bin/sh\nhash -r" +prop_checkBashisms81 = verifyNot checkBashisms "#!/bin/dash\nhash -v" +prop_checkBashisms82 = verifyNot checkBashisms "#!/bin/sh\nset -v +o allexport -o errexit -C" +prop_checkBashisms83 = verifyNot checkBashisms "#!/bin/sh\nset --" +prop_checkBashisms84 = verify checkBashisms "#!/bin/sh\nset -o pipefail" +prop_checkBashisms85 = verify checkBashisms "#!/bin/sh\nset -B" +prop_checkBashisms86 = verifyNot checkBashisms "#!/bin/dash\nset -o emacs" +prop_checkBashisms87 = verify checkBashisms "#!/bin/sh\nset -o emacs" +prop_checkBashisms88 = verifyNot checkBashisms "#!/bin/sh\nset -- wget -o foo 'https://some.url'" +prop_checkBashisms89 = verifyNot checkBashisms "#!/bin/sh\nopts=$-\nset -\"$opts\"" +prop_checkBashisms90 = verifyNot checkBashisms "#!/bin/sh\nset -o \"$opt\"" +prop_checkBashisms91 = verify checkBashisms "#!/bin/sh\nwait -n" +prop_checkBashisms92 = verify checkBashisms "#!/bin/sh\necho $((16#FF))" +prop_checkBashisms93 = verify checkBashisms "#!/bin/sh\necho $(( 10#$(date +%m) ))" +prop_checkBashisms94 = verify checkBashisms "#!/bin/sh\n[ -v var ]" +prop_checkBashisms95 = verify checkBashisms "#!/bin/sh\necho $_" +prop_checkBashisms96 = verifyNot checkBashisms "#!/bin/dash\necho $_" checkBashisms = ForShell [Sh, Dash] $ \t -> do params <- ask kludge params t @@ -170,6 +211,8 @@ warnMsg id "== in place of = is" bashism (TC_Binary id SingleBracket "=~" _ _) = warnMsg id "=~ regex matching is" + bashism (TC_Unary id SingleBracket "-v" _) = + warnMsg id "unary -v (in place of [ -n \"${var+x}\" ]) is" bashism (TC_Unary id _ "-a" _) = warnMsg id "unary -a in place of -e is" bashism (TA_Unary id op _) @@ -194,7 +237,7 @@ bashism t@(TA_Variable id str _) | isBashVariable str = warnMsg id $ str ++ " is" - bashism t@(T_DollarBraced id token) = do + bashism t@(T_DollarBraced id _ token) = do mapM_ check expansion when (isBashVariable var) $ warnMsg id $ var ++ " is" @@ -222,21 +265,72 @@ warnMsg id "` [(Id, String)] + getLiteralArgs (first:rest) = fromMaybe [] $ do + str <- getLiteralString first + return $ (getId first, str) : getLiteralArgs rest + getLiteralArgs [] = [] + -- Check a flag-option pair (such as -o errexit) + checkOptions (flag@(fid,flag') : opt@(oid,opt') : rest) + | flag' `matches` oFlagRegex = do + when (opt' `notElem` longOptions) $ + warnMsg oid $ "set option " <> opt' <> " is" + checkFlags (flag:rest) + | otherwise = checkFlags (flag:opt:rest) + checkOptions (flag:rest) = checkFlags (flag:rest) + checkOptions _ = return () + + -- Check that each option in a sequence of flags + -- (such as -aveo) is valid + checkFlags (flag@(fid, flag'):rest) + | startsOption flag' = do + unless (flag' `matches` validFlagsRegex) $ + forM_ (tail flag') $ \letter -> + when (letter `notElem` optionsSet) $ + warnMsg fid $ "set flag " <> ('-':letter:" is") + checkOptions rest + | beginsWithDoubleDash flag' = do + warnMsg fid $ "set flag " <> flag' <> " is" + checkOptions rest + -- Either a word that doesn't start with a dash, or simply '--', + -- so stop checking. + | otherwise = return () + checkFlags [] = return () + + options = "abCefhmnuvxo" + optionsSet = Set.fromList options + startsOption = (`matches` mkRegex "^(\\+|-[^-])") + oFlagRegex = mkRegex $ "^[-+][" <> options <> "]*o$" + validFlagsRegex = mkRegex $ "^[-+]([" <> options <> "]+o?|o)$" + beginsWithDoubleDash = (`matches` mkRegex "^--.+$") + longOptions = Set.fromList + [ "allexport", "errexit", "ignoreeof", "monitor", "noclobber" + , "noexec", "noglob", "nolog", "notify" , "nounset", "verbose" + , "vi", "xtrace" ] + bashism t@(T_SimpleCommand id _ (cmd:rest)) = let name = fromMaybe "" $ getCommandName t flags = getLeadingFlags t @@ -244,7 +338,8 @@ when (name `elem` unsupportedCommands) $ warnMsg id $ "'" ++ name ++ "' is" potentially $ do - allowed <- Map.lookup name allowedFlags + allowed' <- Map.lookup name allowedFlags + allowed <- allowed' (word, flag) <- listToMaybe $ filter (\x -> (not . null . snd $ x) && snd x `notElem` allowed) flags return . warnMsg (getId word) $ name ++ " -" ++ flag ++ " is" @@ -279,16 +374,28 @@ "typeset" ] ++ if not isDash then ["local"] else [] allowedFlags = Map.fromList [ - ("exec", []), - ("export", ["-p"]), - ("printf", []), - ("read", if isDash then ["r", "p"] else ["r"]), - ("ulimit", ["f"]) + ("cd", Just ["L", "P"]), + ("exec", Just []), + ("export", Just ["p"]), + ("hash", Just $ if isDash then ["r", "v"] else ["r"]), + ("jobs", Just ["l", "p"]), + ("printf", Just []), + ("read", Just $ if isDash then ["r", "p"] else ["r"]), + ("readonly", Just ["p"]), + ("trap", Just []), + ("type", Just []), + ("ulimit", if isDash then Nothing else Just ["f"]), + ("umask", Just ["S"]), + ("unset", Just ["f", "v"]), + ("wait", Just []) ] bashism t@(T_SourceCommand id src _) = let name = fromMaybe "" $ getCommandName src - in do - when (name == "source") $ warnMsg id "'source' in place of '.' is" + in when (name == "source") $ warnMsg id "'source' in place of '.' is" + bashism (TA_Expansion _ (T_Literal id str : _)) | str `matches` radix = + when (str `matches` radix) $ warnMsg id "arithmetic base conversion is" + where + radix = mkRegex "^[0-9]+#" bashism _ = return () varChars="_0-9a-zA-Z" @@ -303,10 +410,11 @@ ] bashVars = [ "OSTYPE", "MACHTYPE", "HOSTTYPE", "HOSTNAME", - "DIRSTACK", "EUID", "UID", "SHLVL", "PIPESTATUS", "SHELLOPTS" + "DIRSTACK", "EUID", "UID", "SHLVL", "PIPESTATUS", "SHELLOPTS", + "_" ] bashDynamicVars = [ "RANDOM", "SECONDS" ] - dashVars = [ ] + dashVars = [ "_" ] isBashVariable var = (var `elem` bashDynamicVars || var `elem` bashVars && not (isAssigned var)) @@ -318,30 +426,44 @@ _ -> False prop_checkEchoSed1 = verify checkEchoSed "FOO=$(echo \"$cow\" | sed 's/foo/bar/g')" +prop_checkEchoSed1b= verify checkEchoSed "FOO=$(sed 's/foo/bar/g' <<< \"$cow\")" prop_checkEchoSed2 = verify checkEchoSed "rm $(echo $cow | sed -e 's,foo,bar,')" +prop_checkEchoSed2b= verify checkEchoSed "rm $(sed -e 's,foo,bar,' <<< $cow)" checkEchoSed = ForShell [Bash, Ksh] f where + f (T_Redirecting id lefts r) = + when (any redirectHereString lefts) $ + checkSed id rcmd + where + redirectHereString :: Token -> Bool + redirectHereString t = case t of + (T_FdRedirect _ _ T_HereString{}) -> True + _ -> False + rcmd = oversimplify r + f (T_Pipeline id _ [a, b]) = when (acmd == ["echo", "${VAR}"]) $ - case bcmd of - ["sed", v] -> checkIn v - ["sed", "-e", v] -> checkIn v - _ -> return () + checkSed id bcmd where - -- This should have used backreferences, but TDFA doesn't support them - sedRe = mkRegex "^s(.)([^\n]*)g?$" - isSimpleSed s = fromMaybe False $ do - [first,rest] <- matchRegex sedRe s - let delimiters = filter (== head first) rest - guard $ length delimiters == 2 - return True - acmd = oversimplify a bcmd = oversimplify b - checkIn s = - when (isSimpleSed s) $ - style id 2001 "See if you can use ${variable//search/replace} instead." + f _ = return () + + checkSed id ["sed", v] = checkIn id v + checkSed id ["sed", "-e", v] = checkIn id v + checkSed _ _ = return () + + -- This should have used backreferences, but TDFA doesn't support them + sedRe = mkRegex "^s(.)([^\n]*)g?$" + isSimpleSed s = fromMaybe False $ do + [first,rest] <- matchRegex sedRe s + let delimiters = filter (== head first) rest + guard $ length delimiters == 2 + return True + checkIn id s = + when (isSimpleSed s) $ + style id 2001 "See if you can use ${variable//search/replace} instead." prop_checkBraceExpansionVars1 = verify checkBraceExpansionVars "echo {1..$n}" diff --git a/src/ShellCheck/Data.hs b/src/ShellCheck/Data.hs --- a/src/ShellCheck/Data.hs +++ b/src/ShellCheck/Data.hs @@ -36,15 +36,31 @@ -- Ksh , ".sh.version" + + -- shflags + , "FLAGS_ARGC", "FLAGS_ARGV", "FLAGS_ERROR", "FLAGS_FALSE", "FLAGS_HELP", + "FLAGS_PARENT", "FLAGS_RESERVED", "FLAGS_TRUE", "FLAGS_VERSION", + "flags_error", "flags_return" ] -variablesWithoutSpaces = [ - "$", "-", "?", "!", "#", +specialVariablesWithoutSpaces = [ + "$", "-", "?", "!", "#" + ] +variablesWithoutSpaces = specialVariablesWithoutSpaces ++ [ "BASHPID", "BASH_ARGC", "BASH_LINENO", "BASH_SUBSHELL", "EUID", "LINENO", "OPTIND", "PPID", "RANDOM", "SECONDS", "SHELLOPTS", "SHLVL", "UID", "COLUMNS", "HISTFILESIZE", "HISTSIZE", "LINES" + + -- shflags + , "FLAGS_ERROR", "FLAGS_FALSE", "FLAGS_TRUE" ] +specialVariables = specialVariablesWithoutSpaces ++ ["@", "*"] + +unbracedVariables = specialVariables ++ [ + "0", "1", "2", "3", "4", "5", "6", "7", "8", "9" + ] + arrayVariables = [ "BASH_ALIASES", "BASH_ARGC", "BASH_ARGV", "BASH_CMDS", "BASH_LINENO", "BASH_REMATCH", "BASH_SOURCE", "BASH_VERSINFO", "COMP_WORDS", "COPROC", @@ -109,6 +125,7 @@ case name of "sh" -> return Sh "bash" -> return Bash + "bats" -> return Bash "dash" -> return Dash "ash" -> return Dash -- There's also a warning for this. "ksh" -> return Ksh diff --git a/src/ShellCheck/Fixer.hs b/src/ShellCheck/Fixer.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Fixer.hs @@ -0,0 +1,409 @@ +{- + Copyright 2018-2019 Vidar Holen, Ng Zhi An + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} + +{-# LANGUAGE TemplateHaskell #-} +module ShellCheck.Fixer (applyFix, removeTabStops, mapPositions, Ranged(..), runTests) where + +import ShellCheck.Interface +import Control.Monad.State +import Data.Array +import Data.List +import Data.Semigroup +import GHC.Exts (sortWith) +import Test.QuickCheck + +-- The Ranged class is used for types that has a start and end position. +class Ranged a where + start :: a -> Position + end :: a -> Position + overlap :: a -> a -> Bool + overlap x y = + (yStart >= xStart && yStart < xEnd) || (yStart < xStart && yEnd > xStart) + where + yStart = start y + yEnd = end y + xStart = start x + xEnd = end x + -- Set a new start and end position on a Ranged + setRange :: (Position, Position) -> a -> a + +-- Tests auto-verify that overlap commutes +assertOverlap x y = overlap x y && overlap y x +assertNoOverlap x y = not (overlap x y) && not (overlap y x) + +prop_overlap_contiguous = assertNoOverlap + (tFromStart 10 12 "foo" 1) + (tFromStart 12 14 "bar" 2) + +prop_overlap_adjacent_zerowidth = assertNoOverlap + (tFromStart 3 3 "foo" 1) + (tFromStart 3 3 "bar" 2) + +prop_overlap_enclosed = assertOverlap + (tFromStart 3 5 "foo" 1) + (tFromStart 1 10 "bar" 2) + +prop_overlap_partial = assertOverlap + (tFromStart 1 5 "foo" 1) + (tFromStart 3 7 "bar" 2) + + +instance Ranged PositionedComment where + start = pcStartPos + end = pcEndPos + setRange (s, e) pc = pc { + pcStartPos = s, + pcEndPos = e + } + +instance Ranged Replacement where + start = repStartPos + end = repEndPos + setRange (s, e) r = r { + repStartPos = s, + repEndPos = e + } + +-- The Monoid instance for Fix merges fixes that do not conflict. +-- TODO: Make an efficient 'mconcat' +instance Monoid Fix where + mempty = newFix + mappend = (<>) + +instance Semigroup Fix where + f1 <> f2 = + -- FIXME: This might need to also discard adjacent zero-width ranges for + -- when two fixes change the same AST node, e.g. `foo` -> "$(foo)" + if or [ r2 `overlap` r1 | r1 <- fixReplacements f1, r2 <- fixReplacements f2 ] + then f1 + else newFix { + fixReplacements = fixReplacements f1 ++ fixReplacements f2 + } + +-- Conveniently apply a transformation to positions in a Fix +mapPositions :: (Position -> Position) -> Fix -> Fix +mapPositions f = adjustFix + where + adjustReplacement rep = + rep { + repStartPos = f $ repStartPos rep, + repEndPos = f $ repEndPos rep + } + adjustFix fix = + fix { + fixReplacements = map adjustReplacement $ fixReplacements fix + } + +-- Rewrite a Ranged from a tabstop of 8 to 1 +removeTabStops :: Ranged a => a -> Array Int String -> a +removeTabStops range ls = + let startColumn = realignColumn lineNo colNo range + endColumn = realignColumn endLineNo endColNo range + startPosition = (start range) { posColumn = startColumn } + endPosition = (end range) { posColumn = endColumn } in + setRange (startPosition, endPosition) range + where + realignColumn lineNo colNo c = + if lineNo c > 0 && lineNo c <= fromIntegral (length ls) + then real (ls ! fromIntegral (lineNo c)) 0 0 (colNo c) + else colNo c + real _ r v target | target <= v = r + -- hit this case at the end of line, and if we don't hit the target + -- return real + (target - v) + real [] r v target = r + (target - v) + real ('\t':rest) r v target = real rest (r+1) (v + 8 - (v `mod` 8)) target + real (_:rest) r v target = real rest (r+1) (v+1) target + lineNo = posLine . start + endLineNo = posLine . end + colNo = posColumn . start + endColNo = posColumn . end + + +-- A replacement that spans multiple line is applied by: +-- 1. merging the affected lines into a single string using `unlines` +-- 2. apply the replacement as if it only spanned a single line +-- The tricky part is adjusting the end column of the replacement +-- (the end line doesn't matter because there is only one line) +-- +-- aaS <--- start of replacement (row 1 column 3) +-- bbbb +-- cEc +-- \------- end of replacement (row 3 column 2) +-- +-- a flattened string will look like: +-- +-- "aaS\nbbbb\ncEc\n" +-- +-- The column of E has to be adjusted by: +-- 1. lengths of lines to be replaced, except the end row itself +-- 2. end column of the replacement +-- 3. number of '\n' by `unlines` +multiToSingleLine :: [Fix] -> Array Int String -> ([Fix], String) +multiToSingleLine fixes lines = + (map (mapPositions adjust) fixes, unlines $ elems lines) + where + -- A prefix sum tree from line number to column shift. + -- FIXME: The tree will be totally unbalanced. + shiftTree :: PSTree Int + shiftTree = + foldl (\t (n,s) -> addPSValue (n+1) (length s + 1) t) newPSTree $ + assocs lines + singleString = unlines $ elems lines + adjust pos = + pos { + posLine = 1, + posColumn = (posColumn pos) + + (fromIntegral $ getPrefixSum (fromIntegral $ posLine pos) shiftTree) + } + +-- Apply a fix and return resulting lines. +-- The number of lines can increase or decrease with no obvious mapping back, so +-- the function does not return an array. +applyFix :: Fix -> Array Int String -> [String] +applyFix fix fileLines = + let + untabbed = fix { + fixReplacements = + map (\c -> removeTabStops c fileLines) $ + fixReplacements fix + } + (adjustedFixes, singleLine) = multiToSingleLine [untabbed] fileLines + in + lines . runFixer $ applyFixes2 adjustedFixes singleLine + + +-- start and end comes from pos, which is 1 based +prop_doReplace1 = doReplace 0 0 "1234" "A" == "A1234" -- technically not valid +prop_doReplace2 = doReplace 1 1 "1234" "A" == "A1234" +prop_doReplace3 = doReplace 1 2 "1234" "A" == "A234" +prop_doReplace4 = doReplace 3 3 "1234" "A" == "12A34" +prop_doReplace5 = doReplace 4 4 "1234" "A" == "123A4" +prop_doReplace6 = doReplace 5 5 "1234" "A" == "1234A" +doReplace start end o r = + let si = fromIntegral (start-1) + ei = fromIntegral (end-1) + (x, xs) = splitAt si o + (y, z) = splitAt (ei - si) xs + in + x ++ r ++ z + +-- Fail if the 'expected' string is not result when applying 'fixes' to 'original'. +testFixes :: String -> String -> [Fix] -> Bool +testFixes expected original fixes = + actual == expected + where + actual = runFixer (applyFixes2 fixes original) + + +-- A Fixer allows doing repeated modifications of a string where each +-- replacement automatically accounts for shifts from previous ones. +type Fixer a = State (PSTree Int) a + +-- Apply a single replacement using its indices into the original string. +-- It does not handle multiple lines, all line indices must be 1. +applyReplacement2 :: Replacement -> String -> Fixer String +applyReplacement2 rep string = do + tree <- get + let transform pos = pos + getPrefixSum pos tree + let originalPos = (repStartPos rep, repEndPos rep) + (oldStart, oldEnd) = tmap (fromInteger . posColumn) originalPos + (newStart, newEnd) = tmap transform (oldStart, oldEnd) + + let (l1, l2) = tmap posLine originalPos in + when (l1 /= 1 || l2 /= 1) $ + error "ShellCheck internal error, please report: bad cross-line fix" + + let replacer = repString rep + let shift = (length replacer) - (oldEnd - oldStart) + let insertionPoint = + case repInsertionPoint rep of + InsertBefore -> oldStart + InsertAfter -> oldEnd+1 + put $ addPSValue insertionPoint shift tree + + return $ doReplace newStart newEnd string replacer + where + tmap f (a,b) = (f a, f b) + +-- Apply a list of Replacements in the correct order +applyReplacements2 :: [Replacement] -> String -> Fixer String +applyReplacements2 reps str = + foldM (flip applyReplacement2) str $ + reverse $ sortWith repPrecedence reps + +-- Apply all fixes with replacements in the correct order +applyFixes2 :: [Fix] -> String -> Fixer String +applyFixes2 fixes = applyReplacements2 (concatMap fixReplacements fixes) + +-- Get the final value of a Fixer. +runFixer :: Fixer a -> a +runFixer f = evalState f newPSTree + + + +-- A Prefix Sum Tree that lets you look up the sum of values at and below an index. +-- It's implemented essentially as a Fenwick tree without the bit-based balancing. +-- The last Num is the sum of the left branch plus current element. +data PSTree n = PSBranch n (PSTree n) (PSTree n) n | PSLeaf + deriving (Show) + +newPSTree :: Num n => PSTree n +newPSTree = PSLeaf + +-- Get the sum of values whose keys are <= 'target' +getPrefixSum :: (Ord n, Num n) => n -> PSTree n -> n +getPrefixSum = f 0 + where + f sum _ PSLeaf = sum + f sum target (PSBranch pivot left right cumulative) = + case () of + _ | target < pivot -> f sum target left + _ | target > pivot -> f (sum+cumulative) target right + _ -> sum+cumulative + +-- Add a value to the Prefix Sum tree at the given index. +-- Values accumulate: addPSValue 42 2 . addPSValue 42 3 == addPSValue 42 5 +addPSValue :: (Ord n, Num n) => n -> n -> PSTree n -> PSTree n +addPSValue key value tree = if value == 0 then tree else f tree + where + f PSLeaf = PSBranch key PSLeaf PSLeaf value + f (PSBranch pivot left right sum) = + case () of + _ | key < pivot -> PSBranch pivot (f left) right (sum + value) + _ | key > pivot -> PSBranch pivot left (f right) sum + _ -> PSBranch pivot left right (sum + value) + +prop_pstreeSumsCorrectly kvs targets = + let + -- Trivial O(n * m) implementation + dumbPrefixSums :: [(Int, Int)] -> [Int] -> [Int] + dumbPrefixSums kvs targets = + let prefixSum target = sum . map snd . filter (\(k,v) -> k <= target) $ kvs + in map prefixSum targets + -- PSTree O(n * log m) implementation + smartPrefixSums :: [(Int, Int)] -> [Int] -> [Int] + smartPrefixSums kvs targets = + let tree = foldl (\tree (pos, shift) -> addPSValue pos shift tree) PSLeaf kvs + in map (\x -> getPrefixSum x tree) targets + in smartPrefixSums kvs targets == dumbPrefixSums kvs targets + + +-- Semi-convenient functions for constructing tests. +testFix :: [Replacement] -> Fix +testFix list = newFix { + fixReplacements = list + } + +tFromStart :: Int -> Int -> String -> Int -> Replacement +tFromStart start end repl order = + newReplacement { + repStartPos = newPosition { + posLine = 1, + posColumn = fromIntegral start + }, + repEndPos = newPosition { + posLine = 1, + posColumn = fromIntegral end + }, + repString = repl, + repPrecedence = order, + repInsertionPoint = InsertAfter + } + +tFromEnd start end repl order = + (tFromStart start end repl order) { + repInsertionPoint = InsertBefore + } + +prop_simpleFix1 = testFixes "hello world" "hell world" [ + testFix [ + tFromEnd 5 5 "o" 1 + ]] + +prop_anchorsLeft = testFixes "-->foobar<--" "--><--" [ + testFix [ + tFromStart 4 4 "foo" 1, + tFromStart 4 4 "bar" 2 + ]] + +prop_anchorsRight = testFixes "-->foobar<--" "--><--" [ + testFix [ + tFromEnd 4 4 "bar" 1, + tFromEnd 4 4 "foo" 2 + ]] + +prop_anchorsBoth1 = testFixes "-->foobar<--" "--><--" [ + testFix [ + tFromStart 4 4 "bar" 2, + tFromEnd 4 4 "foo" 1 + ]] + +prop_anchorsBoth2 = testFixes "-->foobar<--" "--><--" [ + testFix [ + tFromEnd 4 4 "foo" 2, + tFromStart 4 4 "bar" 1 + ]] + +prop_composeFixes1 = testFixes "cd \"$1\" || exit" "cd $1" [ + testFix [ + tFromStart 4 4 "\"" 10, + tFromEnd 6 6 "\"" 10 + ], + testFix [ + tFromEnd 6 6 " || exit" 5 + ]] + +prop_composeFixes2 = testFixes "$(\"$1\")" "`$1`" [ + testFix [ + tFromStart 1 2 "$(" 5, + tFromEnd 4 5 ")" 5 + ], + testFix [ + tFromStart 2 2 "\"" 10, + tFromEnd 4 4 "\"" 10 + ]] + +prop_composeFixes3 = testFixes "(x)[x]" "xx" [ + testFix [ + tFromStart 1 1 "(" 4, + tFromEnd 2 2 ")" 3, + tFromStart 2 2 "[" 2, + tFromEnd 3 3 "]" 1 + ]] + +prop_composeFixes4 = testFixes "(x)[x]" "xx" [ + testFix [ + tFromStart 1 1 "(" 4, + tFromStart 2 2 "[" 3, + tFromEnd 2 2 ")" 2, + tFromEnd 3 3 "]" 1 + ]] + +prop_composeFixes5 = testFixes "\"$(x)\"" "`x`" [ + testFix [ + tFromStart 1 2 "$(" 2, + tFromEnd 3 4 ")" 2, + tFromStart 1 1 "\"" 1, + tFromEnd 4 4 "\"" 1 + ]] + + +return [] +runTests = $quickCheckAll 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 @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net diff --git a/src/ShellCheck/Formatter/Diff.hs b/src/ShellCheck/Formatter/Diff.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Formatter/Diff.hs @@ -0,0 +1,255 @@ +{- + Copyright 2019 Vidar 'koala_man' Holen + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +{-# LANGUAGE TemplateHaskell #-} +module ShellCheck.Formatter.Diff (format, ShellCheck.Formatter.Diff.runTests) where + +import ShellCheck.Interface +import ShellCheck.Fixer +import ShellCheck.Formatter.Format + +import Control.Monad +import Data.Algorithm.Diff +import Data.Array +import Data.IORef +import Data.List +import qualified Data.Monoid as Monoid +import Data.Maybe +import qualified Data.Map as M +import GHC.Exts (sortWith) +import System.IO +import System.FilePath + +import Test.QuickCheck + +import Debug.Trace +ltt x = trace (show x) x + +format :: FormatterOptions -> IO Formatter +format options = do + didOutput <- newIORef False + shouldColor <- shouldOutputColor (foColorOption options) + let color = if shouldColor then colorize else nocolor + return Formatter { + header = return (), + footer = checkFooter didOutput color, + onFailure = reportFailure color, + onResult = reportResult didOutput color + } + + +contextSize = 3 +red = 31 +green = 32 +yellow = 33 +cyan = 36 +bold = 1 + +nocolor n = id +colorize n s = (ansi n) ++ s ++ (ansi 0) +ansi n = "\x1B[" ++ show n ++ "m" + +printErr :: ColorFunc -> String -> IO () +printErr color = hPutStrLn stderr . color bold . color red +reportFailure color file msg = printErr color $ file ++ ": " ++ msg + +checkFooter didOutput color = do + output <- readIORef didOutput + unless output $ + printErr color "Issues were detected, but none were auto-fixable. Use another format to see them." + +type ColorFunc = (Int -> String -> String) +data LFStatus = LinefeedMissing | LinefeedOk +data DiffDoc a = DiffDoc String LFStatus [DiffRegion a] +data DiffRegion a = DiffRegion (Int, Int) (Int, Int) [Diff a] + +reportResult :: (IORef Bool) -> ColorFunc -> CheckResult -> SystemInterface IO -> IO () +reportResult didOutput color result sys = do + let comments = crComments result + let suggestedFixes = mapMaybe pcFix comments + let fixmap = buildFixMap suggestedFixes + mapM_ output $ M.toList fixmap + where + output (name, fix) = do + file <- (siReadFile sys) name + case file of + Right contents -> do + putStrLn $ formatDoc color $ makeDiff name contents fix + writeIORef didOutput True + Left msg -> reportFailure color name msg + +hasTrailingLinefeed str = + case str of + [] -> True + _ -> last str == '\n' + +coversLastLine regions = + case regions of + [] -> False + _ -> (fst $ last regions) + +-- TODO: Factor this out into a unified diff library because we're doing a lot +-- of the heavy lifting anyways. +makeDiff :: String -> String -> Fix -> DiffDoc String +makeDiff name contents fix = do + let hunks = groupDiff $ computeDiff contents fix + let lf = if coversLastLine hunks && not (hasTrailingLinefeed contents) + then LinefeedMissing + else LinefeedOk + DiffDoc name lf $ findRegions hunks + +computeDiff :: String -> Fix -> [Diff String] +computeDiff contents fix = + let old = lines contents + array = listArray (1, fromIntegral $ (length old)) old + new = applyFix fix array + in getDiff old new + +-- Group changes into hunks +groupDiff :: [Diff a] -> [(Bool, [Diff a])] +groupDiff = filter (\(_, l) -> not (null l)) . hunt [] + where + -- Churn through 'Both's until we find a difference + hunt current [] = [(False, reverse current)] + hunt current (x@Both {}:rest) = hunt (x:current) rest + hunt current list = + let (context, previous) = splitAt contextSize current + in (False, reverse previous) : gather context 0 list + + -- Pick out differences until we find a run of Both's + gather current n [] = + let (extras, patch) = splitAt (max 0 $ n - contextSize) current + in [(True, reverse patch), (False, reverse extras)] + + gather current n list@(Both {}:_) | n == contextSize*2 = + let (context, previous) = splitAt contextSize current + in (True, reverse previous) : hunt context list + + gather current n (x@Both {}:rest) = gather (x:current) (n+1) rest + gather current n (x:rest) = gather (x:current) 0 rest + +-- Get line numbers for hunks +findRegions :: [(Bool, [Diff String])] -> [DiffRegion String] +findRegions = find' 1 1 + where + find' _ _ [] = [] + find' left right ((output, run):rest) = + let (dl, dr) = countDelta run + remainder = find' (left+dl) (right+dr) rest + in + if output + then DiffRegion (left, dl) (right, dr) run : remainder + else remainder + +-- Get left/right line counts for a hunk +countDelta :: [Diff a] -> (Int, Int) +countDelta = count' 0 0 + where + count' left right [] = (left, right) + count' left right (x:rest) = + case x of + Both {} -> count' (left+1) (right+1) rest + First {} -> count' (left+1) right rest + Second {} -> count' left (right+1) rest + +formatRegion :: ColorFunc -> LFStatus -> DiffRegion String -> String +formatRegion color lf (DiffRegion left right diffs) = + let header = color cyan ("@@ -" ++ (tup left) ++ " +" ++ (tup right) ++" @@") + in + unlines $ header : reverse (getStrings lf (reverse diffs)) + where + noLF = "\\ No newline at end of file" + + getStrings LinefeedOk list = map format list + getStrings LinefeedMissing list@((Both _ _):_) = noLF : map format list + getStrings LinefeedMissing list@((First _):_) = noLF : map format list + getStrings LinefeedMissing (last:rest) = format last : getStrings LinefeedMissing rest + + tup (a,b) = (show a) ++ "," ++ (show b) + format (Both x _) = ' ':x + format (First x) = color red $ '-':x + format (Second x) = color green $ '+':x + +splitLast [] = ([], []) +splitLast x = + let (last, rest) = splitAt 1 $ reverse x + in (reverse rest, last) + +formatDoc color (DiffDoc name lf regions) = + let (most, last) = splitLast regions + in + (color bold $ "--- " ++ ("a" name)) ++ "\n" ++ + (color bold $ "+++ " ++ ("b" name)) ++ "\n" ++ + concatMap (formatRegion color LinefeedOk) most ++ + concatMap (formatRegion color lf) last + +-- Create a Map from filename to Fix +buildFixMap :: [Fix] -> M.Map String Fix +buildFixMap fixes = perFile + where + splitFixes = concatMap splitFixByFile fixes + perFile = groupByMap (posFile . repStartPos . head . fixReplacements) splitFixes + +-- There are currently no multi-file fixes, but let's handle it anyways +splitFixByFile :: Fix -> [Fix] +splitFixByFile fix = map makeFix $ groupBy sameFile (fixReplacements fix) + where + sameFile rep1 rep2 = (posFile $ repStartPos rep1) == (posFile $ repStartPos rep2) + makeFix reps = newFix { fixReplacements = reps } + +groupByMap :: (Ord k, Monoid v) => (v -> k) -> [v] -> M.Map k v +groupByMap f = M.fromListWith Monoid.mappend . map (\x -> (f x, x)) + +-- For building unit tests +b n = Both n n +l = First +r = Second + +prop_identifiesProperContext = groupDiff [b 1, b 2, b 3, b 4, l 5, b 6, b 7, b 8, b 9] == + [(False, [b 1]), -- Omitted + (True, [b 2, b 3, b 4, l 5, b 6, b 7, b 8]), -- A change with three lines of context + (False, [b 9])] -- Omitted + +prop_includesContextFromStartIfNecessary = groupDiff [b 4, l 5, b 6, b 7, b 8, b 9] == + [ -- Nothing omitted + (True, [b 4, l 5, b 6, b 7, b 8]), -- A change with three lines of context + (False, [b 9])] -- Omitted + +prop_includesContextUntilEndIfNecessary = groupDiff [b 4, l 5] == + [ -- Nothing omitted + (True, [b 4, l 5]) + ] -- Nothing Omitted + +prop_splitsIntoMultipleHunks = groupDiff [l 1, b 1, b 2, b 3, b 4, b 5, b 6, b 7, r 8] == + [ -- Nothing omitted + (True, [l 1, b 1, b 2, b 3]), + (False, [b 4]), + (True, [b 5, b 6, b 7, r 8]) + ] -- Nothing Omitted + +prop_splitsIntoMultipleHunksUnlessTouching = groupDiff [l 1, b 1, b 2, b 3, b 4, b 5, b 6, r 7] == + [ + (True, [l 1, b 1, b 2, b 3, b 4, b 5, b 6, r 7]) + ] + +prop_countDeltasWorks = countDelta [b 1, l 2, r 3, r 4, b 5] == (3,4) +prop_countDeltasWorks2 = countDelta [] == (0,0) + +return [] +runTests = $quickCheckAll 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 @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -21,7 +21,14 @@ import ShellCheck.Data import ShellCheck.Interface +import ShellCheck.Fixer +import Control.Monad +import Data.Array +import Data.List +import System.IO +import System.Info + -- A formatter that carries along an arbitrary piece of data data Formatter = Formatter { header :: IO (), @@ -50,21 +57,23 @@ makeNonVirtual comments contents = map fix comments where - ls = lines contents - fix c = c { - pcStartPos = (pcStartPos c) { - posColumn = realignColumn lineNo colNo c - } - , pcEndPos = (pcEndPos c) { - posColumn = realignColumn endLineNo endColNo c - } + list = lines contents + arr = listArray (1, length list) list + untabbedFix f = newFix { + fixReplacements = map (\r -> removeTabStops r arr) (fixReplacements f) } - realignColumn lineNo colNo c = - if lineNo c > 0 && lineNo c <= fromIntegral (length ls) - then real (ls !! fromIntegral (lineNo c - 1)) 0 0 (colNo c) - else colNo c - real _ r v target | target <= v = r - real [] r v _ = r -- should never happen - real ('\t':rest) r v target = - real rest (r+1) (v + 8 - (v `mod` 8)) target - real (_:rest) r v target = real rest (r+1) (v+1) target + fix c = (removeTabStops c arr) { + pcFix = fmap untabbedFix (pcFix c) + } + + +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 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 @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net diff --git a/src/ShellCheck/Formatter/JSON.hs b/src/ShellCheck/Formatter/JSON.hs --- a/src/ShellCheck/Formatter/JSON.hs +++ b/src/ShellCheck/Formatter/JSON.hs @@ -1,6 +1,6 @@ {-# LANGUAGE OverloadedStrings #-} {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -30,6 +30,7 @@ import System.IO import qualified Data.ByteString.Lazy.Char8 as BL +format :: IO Formatter format = do ref <- newIORef [] return Formatter { @@ -39,7 +40,25 @@ footer = finish ref } -instance ToJSON (PositionedComment) where +instance ToJSON Replacement where + toJSON replacement = + let start = repStartPos replacement + end = repEndPos replacement + str = repString replacement in + object [ + "precedence" .= repPrecedence replacement, + "insertionPoint" .= + case repInsertionPoint replacement of + InsertBefore -> "beforeStart" :: String + InsertAfter -> "afterEnd", + "line" .= posLine start, + "column" .= posColumn start, + "endLine" .= posLine end, + "endColumn" .= posColumn end, + "replacement" .= str + ] + +instance ToJSON PositionedComment where toJSON comment = let start = pcStartPos comment end = pcEndPos comment @@ -52,7 +71,8 @@ "endColumn" .= posColumn end, "level" .= severityText comment, "code" .= cCode c, - "message" .= cMessage c + "message" .= cMessage c, + "fix" .= pcFix comment ] toEncoding comment = @@ -68,13 +88,23 @@ <> "level" .= severityText comment <> "code" .= cCode c <> "message" .= cMessage c + <> "fix" .= pcFix comment ) +instance ToJSON Fix where + toJSON fix = object [ + "replacements" .= fixReplacements fix + ] + outputError file msg = hPutStrLn stderr $ file ++ ": " ++ msg -collectResult ref result _ = - modifyIORef ref (\x -> crComments result ++ x) +collectResult ref cr sys = mapM_ f groups + where + comments = crComments cr + groups = groupWith sourceFile comments + f :: [PositionedComment] -> IO () + f group = modifyIORef ref (\x -> comments ++ x) + finish ref = do list <- readIORef ref BL.putStrLn $ encode list - diff --git a/src/ShellCheck/Formatter/JSON1.hs b/src/ShellCheck/Formatter/JSON1.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Formatter/JSON1.hs @@ -0,0 +1,127 @@ +{-# LANGUAGE OverloadedStrings #-} +{- + Copyright 2012-2019 Vidar Holen + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +module ShellCheck.Formatter.JSON1 (format) where + +import ShellCheck.Interface +import ShellCheck.Formatter.Format + +import Data.Aeson +import Data.IORef +import Data.Monoid +import GHC.Exts +import System.IO +import qualified Data.ByteString.Lazy.Char8 as BL + +format :: IO Formatter +format = do + ref <- newIORef [] + return Formatter { + header = return (), + onResult = collectResult ref, + onFailure = outputError, + footer = finish ref + } + +data Json1Output = Json1Output { + comments :: [PositionedComment] + } + +instance ToJSON Json1Output where + toJSON result = object [ + "comments" .= comments result + ] + toEncoding result = pairs ( + "comments" .= comments result + ) + +instance ToJSON Replacement where + toJSON replacement = + let start = repStartPos replacement + end = repEndPos replacement + str = repString replacement in + object [ + "precedence" .= repPrecedence replacement, + "insertionPoint" .= + case repInsertionPoint replacement of + InsertBefore -> "beforeStart" :: String + InsertAfter -> "afterEnd", + "line" .= posLine start, + "column" .= posColumn start, + "endLine" .= posLine end, + "endColumn" .= posColumn end, + "replacement" .= str + ] + +instance ToJSON PositionedComment where + toJSON comment = + let start = pcStartPos comment + end = pcEndPos comment + c = pcComment comment in + object [ + "file" .= posFile start, + "line" .= posLine start, + "endLine" .= posLine end, + "column" .= posColumn start, + "endColumn" .= posColumn end, + "level" .= severityText comment, + "code" .= cCode c, + "message" .= cMessage c, + "fix" .= pcFix comment + ] + + toEncoding comment = + let start = pcStartPos comment + end = pcEndPos comment + c = pcComment comment in + pairs ( + "file" .= posFile start + <> "line" .= posLine start + <> "endLine" .= posLine end + <> "column" .= posColumn start + <> "endColumn" .= posColumn end + <> "level" .= severityText comment + <> "code" .= cCode c + <> "message" .= cMessage c + <> "fix" .= pcFix comment + ) + +instance ToJSON Fix where + toJSON fix = object [ + "replacements" .= fixReplacements fix + ] + +outputError file msg = hPutStrLn stderr $ file ++ ": " ++ msg + +collectResult ref cr sys = mapM_ f groups + where + comments = crComments cr + groups = groupWith sourceFile comments + f :: [PositionedComment] -> IO () + f group = do + let filename = sourceFile (head group) + result <- siReadFile sys filename + let contents = either (const "") id result + let comments' = makeNonVirtual comments contents + modifyIORef ref (\x -> comments' ++ x) + +finish ref = do + list <- readIORef ref + BL.putStrLn $ encode $ Json1Output { comments = list } diff --git a/src/ShellCheck/Formatter/Quiet.hs b/src/ShellCheck/Formatter/Quiet.hs new file mode 100644 --- /dev/null +++ b/src/ShellCheck/Formatter/Quiet.hs @@ -0,0 +1,36 @@ +{- + Copyright 2019 Austin Voecks + + This file is part of ShellCheck. + https://www.shellcheck.net + + ShellCheck is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + ShellCheck is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +-} +module ShellCheck.Formatter.Quiet (format) where + +import ShellCheck.Interface +import ShellCheck.Formatter.Format + +import Control.Monad +import Data.IORef +import System.Exit + +format :: FormatterOptions -> IO Formatter +format options = + return Formatter { + header = return (), + footer = return (), + onFailure = \ _ _ -> exitFailure, + onResult = \ result _ -> unless (null $ crComments result) exitFailure + } 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 @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -19,12 +19,17 @@ -} module ShellCheck.Formatter.TTY (format) where +import ShellCheck.Fixer import ShellCheck.Interface import ShellCheck.Formatter.Format import Control.Monad +import Data.Array +import Data.Foldable +import Data.Ord import Data.IORef import Data.List +import Data.Maybe import GHC.Exts import System.IO import System.Info @@ -33,6 +38,8 @@ -- An arbitrary Ord thing to order warnings type Ranking = (Char, Severity, Integer) +-- Ansi coloring function +type ColorFunc = (String -> String -> String) format :: FormatterOptions -> IO Formatter format options = do @@ -50,6 +57,7 @@ "warning" -> 33 -- yellow "info" -> 32 -- green "style" -> 32 -- green + "verbose" -> 32 -- green "message" -> 1 -- bold "source" -> 0 -- none _ -> 0 -- none @@ -115,22 +123,55 @@ let fileName = sourceFile (head comments) result <- (siReadFile sys) fileName let contents = either (const "") id result - let fileLines = lines contents - let lineCount = fromIntegral $ length fileLines + let fileLinesList = lines contents + let lineCount = length fileLinesList + let fileLines = listArray (1, lineCount) fileLinesList let groups = groupWith lineNo comments - mapM_ (\x -> do - let lineNum = lineNo (head x) + mapM_ (\commentsForLine -> do + let lineNum = fromIntegral $ lineNo (head commentsForLine) let line = if lineNum < 1 || lineNum > lineCount then "" - else fileLines !! fromIntegral (lineNum - 1) + else fileLines ! fromIntegral lineNum putStrLn "" putStrLn $ color "message" $ "In " ++ fileName ++" line " ++ show lineNum ++ ":" putStrLn (color "source" line) - mapM_ (\c -> putStrLn (color (severityText c) $ cuteIndent c)) x + mapM_ (\c -> putStrLn (color (severityText c) $ cuteIndent c)) commentsForLine putStrLn "" + showFixedString color commentsForLine (fromIntegral lineNum) fileLines ) groups +-- Pick out only the lines necessary to show a fix in action +sliceFile :: Fix -> Array Int String -> (Fix, Array Int String) +sliceFile fix lines = + (mapPositions adjust fix, sliceLines lines) + where + (minLine, maxLine) = + foldl (\(mm, mx) pos -> ((min mm $ fromIntegral $ posLine pos), (max mx $ fromIntegral $ posLine pos))) + (maxBound, minBound) $ + concatMap (\x -> [repStartPos x, repEndPos x]) $ fixReplacements fix + sliceLines :: Array Int String -> Array Int String + sliceLines = ixmap (1, maxLine - minLine + 1) (\x -> x + minLine - 1) + adjust pos = + pos { + posLine = posLine pos - (fromIntegral minLine) + 1 + } + +showFixedString :: ColorFunc -> [PositionedComment] -> Int -> Array Int String -> IO () +showFixedString color comments lineNum fileLines = + let line = fileLines ! fromIntegral lineNum in + case mapMaybe pcFix comments of + [] -> return () + fixes -> do + -- Folding automatically removes overlap + let mergedFix = fold fixes + -- We show the complete, associated fixes, whether or not it includes this + -- and/or other unrelated lines. + let (excerptFix, excerpt) = sliceFile mergedFix fileLines + -- in the spirit of error prone + putStrLn $ color "message" "Did you mean: " + putStrLn $ unlines $ applyFix excerptFix excerpt + cuteIndent :: PositionedComment -> String cuteIndent comment = replicate (fromIntegral $ colNo comment - 1) ' ' ++ @@ -145,14 +186,9 @@ code num = "SC" ++ show num +getColorFunc :: ColorOption -> IO ColorFunc getColorFunc 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 + useColor <- shouldOutputColor colorOption return $ if useColor then colorComment else const id where colorComment level comment = diff --git a/src/ShellCheck/Interface.hs b/src/ShellCheck/Interface.hs --- a/src/ShellCheck/Interface.hs +++ b/src/ShellCheck/Interface.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -17,14 +17,15 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . -} +{-# LANGUAGE DeriveGeneric, DeriveAnyClass #-} module ShellCheck.Interface ( SystemInterface(..) - , CheckSpec(csFilename, csScript, csCheckSourced, csExcludedWarnings, csShellTypeOverride, csMinSeverity) + , CheckSpec(csFilename, csScript, csCheckSourced, csIncludedWarnings, csExcludedWarnings, csShellTypeOverride, csMinSeverity, csIgnoreRC, csOptionalChecks) , CheckResult(crFilename, crComments) - , ParseSpec(psFilename, psScript, psCheckSourced, psShellTypeOverride) + , ParseSpec(psFilename, psScript, psCheckSourced, psIgnoreRC, psShellTypeOverride) , ParseResult(prComments, prTokenPositions, prRoot) - , AnalysisSpec(asScript, asShellType, asExecutionMode, asCheckSourced) + , AnalysisSpec(asScript, asShellType, asFallbackShell, asExecutionMode, asCheckSourced, asTokenPositions, asOptionalChecks) , AnalysisResult(arComments) , FormatterOptions(foColorOption, foWikiLinkCount) , Shell(Ksh, Sh, Bash, Dash) @@ -34,9 +35,9 @@ , Severity(ErrorC, WarningC, InfoC, StyleC) , Position(posFile, posLine, posColumn) , Comment(cSeverity, cCode, cMessage) - , PositionedComment(pcStartPos , pcEndPos , pcComment) + , PositionedComment(pcStartPos , pcEndPos , pcComment, pcFix) , ColorOption(ColorAuto, ColorAlways, ColorNever) - , TokenComment(tcId, tcComment) + , TokenComment(tcId, tcComment, tcFix) , emptyCheckResult , newParseResult , newAnalysisSpec @@ -45,20 +46,43 @@ , newPosition , newTokenComment , mockedSystemInterface + , mockRcFile , newParseSpec , emptyCheckSpec , newPositionedComment , newComment + , Fix(fixReplacements) + , newFix + , InsertionPoint(InsertBefore, InsertAfter) + , Replacement(repStartPos, repEndPos, repString, repPrecedence, repInsertionPoint) + , newReplacement + , CheckDescription(cdName, cdDescription, cdPositive, cdNegative) + , newCheckDescription ) where import ShellCheck.AST + +import Control.DeepSeq import Control.Monad.Identity +import Data.List +import Data.Monoid +import Data.Ord +import Data.Semigroup +import GHC.Generics (Generic) import qualified Data.Map as Map -newtype SystemInterface m = SystemInterface { +data SystemInterface m = SystemInterface { -- Read a file by filename, or return an error - siReadFile :: String -> m (Either ErrorMessage String) + siReadFile :: String -> m (Either ErrorMessage String), + -- Given: + -- the current script, + -- a list of source-path annotations in effect, + -- and a sourced file, + -- find the sourced file + siFindSource :: String -> [String] -> String -> m FilePath, + -- Get the configuration file (name, contents) for a filename + siGetConfig :: String -> m (Maybe (FilePath, String)) } -- ShellCheck input and output @@ -66,9 +90,12 @@ csFilename :: String, csScript :: String, csCheckSourced :: Bool, + csIgnoreRC :: Bool, csExcludedWarnings :: [Integer], + csIncludedWarnings :: Maybe [Integer], csShellTypeOverride :: Maybe Shell, - csMinSeverity :: Severity + csMinSeverity :: Severity, + csOptionalChecks :: [String] } deriving (Show, Eq) data CheckResult = CheckResult { @@ -87,9 +114,12 @@ csFilename = "", csScript = "", csCheckSourced = False, + csIgnoreRC = False, csExcludedWarnings = [], + csIncludedWarnings = Nothing, csShellTypeOverride = Nothing, - csMinSeverity = StyleC + csMinSeverity = StyleC, + csOptionalChecks = [] } newParseSpec :: ParseSpec @@ -97,6 +127,7 @@ psFilename = "", psScript = "", psCheckSourced = False, + psIgnoreRC = False, psShellTypeOverride = Nothing } @@ -105,6 +136,7 @@ psFilename :: String, psScript :: String, psCheckSourced :: Bool, + psIgnoreRC :: Bool, psShellTypeOverride :: Maybe Shell } deriving (Show, Eq) @@ -125,15 +157,21 @@ data AnalysisSpec = AnalysisSpec { asScript :: Token, asShellType :: Maybe Shell, + asFallbackShell :: Maybe Shell, asExecutionMode :: ExecutionMode, - asCheckSourced :: Bool + asCheckSourced :: Bool, + asOptionalChecks :: [String], + asTokenPositions :: Map.Map Id (Position, Position) } newAnalysisSpec token = AnalysisSpec { asScript = token, asShellType = Nothing, + asFallbackShell = Nothing, asExecutionMode = Executed, - asCheckSourced = False + asCheckSourced = False, + asOptionalChecks = [], + asTokenPositions = Map.empty } newtype AnalysisResult = AnalysisResult { @@ -155,7 +193,20 @@ foWikiLinkCount = 3 } +data CheckDescription = CheckDescription { + cdName :: String, + cdDescription :: String, + cdPositive :: String, + cdNegative :: String + } +newCheckDescription = CheckDescription { + cdName = "", + cdDescription = "", + cdPositive = "", + cdNegative = "" + } + -- Supporting data types data Shell = Ksh | Sh | Bash | Dash deriving (Show, Eq) data ExecutionMode = Executed | Sourced deriving (Show, Eq) @@ -163,12 +214,13 @@ type ErrorMessage = String type Code = Integer -data Severity = ErrorC | WarningC | InfoC | StyleC deriving (Show, Eq, Ord) +data Severity = ErrorC | WarningC | InfoC | StyleC + deriving (Show, Eq, Ord, Generic, NFData) data Position = Position { posFile :: String, -- Filename posLine :: Integer, -- 1 based source line posColumn :: Integer -- 1 based source column, where tabs are 8 -} deriving (Show, Eq) +} deriving (Show, Eq, Generic, NFData, Ord) newPosition :: Position newPosition = Position { @@ -181,7 +233,7 @@ cSeverity :: Severity, cCode :: Code, cMessage :: String -} deriving (Show, Eq) +} deriving (Show, Eq, Generic, NFData) newComment :: Comment newComment = Comment { @@ -190,27 +242,64 @@ cMessage = "" } +-- only support single line for now +data Replacement = Replacement { + repStartPos :: Position, + repEndPos :: Position, + repString :: String, + -- Order in which the replacements should happen: highest precedence first. + repPrecedence :: Int, + -- Whether to insert immediately before or immediately after the specified region. + repInsertionPoint :: InsertionPoint +} deriving (Show, Eq, Generic, NFData) + +data InsertionPoint = InsertBefore | InsertAfter + deriving (Show, Eq, Generic, NFData) + +instance Ord Replacement where + compare r1 r2 = (repStartPos r1) `compare` (repStartPos r2) + +newReplacement = Replacement { + repStartPos = newPosition, + repEndPos = newPosition, + repString = "", + repPrecedence = 1, + repInsertionPoint = InsertAfter +} + +data Fix = Fix { + fixReplacements :: [Replacement] +} deriving (Show, Eq, Generic, NFData) + +newFix = Fix { + fixReplacements = [] +} + data PositionedComment = PositionedComment { pcStartPos :: Position, pcEndPos :: Position, - pcComment :: Comment -} deriving (Show, Eq) + pcComment :: Comment, + pcFix :: Maybe Fix +} deriving (Show, Eq, Generic, NFData) newPositionedComment :: PositionedComment newPositionedComment = PositionedComment { pcStartPos = newPosition, pcEndPos = newPosition, - pcComment = newComment + pcComment = newComment, + pcFix = Nothing } data TokenComment = TokenComment { tcId :: Id, - tcComment :: Comment -} deriving (Show, Eq) + tcComment :: Comment, + tcFix :: Maybe Fix +} deriving (Show, Eq, Generic, NFData) newTokenComment = TokenComment { tcId = Id 0, - tcComment = newComment + tcComment = newComment, + tcFix = Nothing } data ColorOption = @@ -222,11 +311,18 @@ -- For testing mockedSystemInterface :: [(String, String)] -> SystemInterface Identity mockedSystemInterface files = SystemInterface { - siReadFile = rf + siReadFile = rf, + siFindSource = fs, + siGetConfig = const $ return Nothing } where rf file = case filter ((== file) . fst) files of [] -> return $ Left "File not included in mock." [(_, contents)] -> return $ Right contents + 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-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -69,7 +69,7 @@ functionChars = variableChars <|> oneOf ":+?-./^@" -- Chars to allow in functions using the 'function' keyword extendedFunctionChars = functionChars <|> oneOf "[]*=!" -specialVariable = oneOf "@*#?-$!" +specialVariable = oneOf (concat specialVariables) paramSubSpecialChars = oneOf "/:+-=%" quotableChars = "|&;<>()\\ '\t\n\r\xA0" ++ doubleQuotableChars quotable = almostSpace <|> oneOf quotableChars @@ -113,6 +113,7 @@ allspacingOrFail = do s <- allspacing when (null s) $ fail "Expected whitespace" + return s readUnicodeQuote = do start <- startSpan @@ -137,7 +138,6 @@ return ' ' --------- Message/position annotation on top of user state -data Note = Note Id Severity Code String deriving (Show, Eq) data ParseNote = ParseNote SourcePos SourcePos Severity Code String deriving (Show, Eq) data Context = ContextName SourcePos String @@ -165,10 +165,6 @@ } codeForParseNote (ParseNote _ _ _ code _) = code -noteToParseNote map (Note id severity code message) = - ParseNote pos pos severity code message - where - pos = fromJust $ Map.lookup id map getLastId = lastId <$> getState @@ -263,6 +259,15 @@ disabling' (DisableComment n) = code == n disabling' _ = False +getCurrentAnnotations includeSource = + concatMap get . takeWhile (not . isBoundary) <$> getCurrentContexts + where + get (ContextAnnotation list) = list + get _ = [] + isBoundary (ContextSource _) = not includeSource + isBoundary _ = False + + shouldFollow file = do context <- getCurrentContexts if any isThisFile context @@ -306,6 +311,8 @@ data Environment m = Environment { systemInterface :: SystemInterface m, checkSourced :: Bool, + ignoreRC :: Bool, + currentFilename :: String, shellTypeOverride :: Maybe Shell } @@ -949,9 +956,12 @@ readAnnotation = called "shellcheck directive" $ do try readAnnotationPrefix many1 linewhitespace + readAnnotationWithoutPrefix + +readAnnotationWithoutPrefix = do values <- many1 readKey optional readAnyComment - void linefeed <|> do + void linefeed <|> eof <|> do parseNote ErrorC 1125 "Invalid key=value pair? Ignoring the rest of this directive starting here." many (noneOf "\n") void linefeed <|> eof @@ -960,7 +970,7 @@ where readKey = do keyPos <- getPosition - key <- many1 letter + key <- many1 (letter <|> char '-') char '=' <|> fail "Expected '=' after directive key" annotations <- case key of "disable" -> readCode `sepBy` char ',' @@ -970,10 +980,18 @@ int <- many1 digit return $ DisableComment (read int) + "enable" -> readName `sepBy` char ',' + where + readName = EnableComment <$> many1 (letter <|> char '-') + "source" -> do filename <- many1 $ noneOf " \n" return [SourceOverride filename] + "source-path" -> do + dirname <- many1 $ noneOf " \n" + return [SourcePath dirname] + "shell" -> do pos <- getPosition shell <- many1 $ noneOf " \n" @@ -1200,7 +1218,7 @@ suggestForgotClosingQuote startPos endPos "backtick expansion" -- Result positions may be off due to escapes - result <- subParse subStart subParser (unEscape subString) + result <- subParse subStart (tryWithErrors subParser <|> return []) (unEscape subString) return $ T_Backticked id result where unEscape [] = [] @@ -1506,10 +1524,10 @@ readNormalDollar = do ensureDollar - readDollarExp <|> readDollarDoubleQuote <|> readDollarSingleQuote <|> readDollarLonely + readDollarExp <|> readDollarDoubleQuote <|> readDollarSingleQuote <|> readDollarLonely False readDoubleQuotedDollar = do ensureDollar - readDollarExp <|> readDollarLonely + readDollarExp <|> readDollarLonely True prop_readDollarExpression1 = isOk readDollarExpression "$(((1) && 3))" @@ -1611,7 +1629,7 @@ word <- readDollarBracedWord char '}' id <- endSpan start - return $ T_DollarBraced id word + return $ T_DollarBraced id True word prop_readDollarExpansion1= isOk readDollarExpansion "$(echo foo; ls\n)" prop_readDollarExpansion2= isOk readDollarExpansion "$( )" @@ -1638,7 +1656,7 @@ let singleCharred p = do value <- wrapString ((:[]) <$> p) id <- endSpan start - return $ (T_DollarBraced id value) + return $ (T_DollarBraced id False value) let positional = do value <- singleCharred digit @@ -1651,7 +1669,7 @@ let regular = do value <- wrapString readVariableName id <- endSpan start - return (T_DollarBraced id value) `attempting` do + return (T_DollarBraced id False value) `attempting` do lookAhead $ char '[' parseNoteAt pos ErrorC 1087 "Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet)." @@ -1671,13 +1689,33 @@ rest <- many variableChars return (f:rest) -readDollarLonely = do + +prop_readDollarLonely1 = isWarning readNormalWord "\"$\"var" +prop_readDollarLonely2 = isWarning readNormalWord "\"$\"\"var\"" +prop_readDollarLonely3 = isOk readNormalWord "\"$\"$var" +prop_readDollarLonely4 = isOk readNormalWord "\"$\"*" +prop_readDollarLonely5 = isOk readNormalWord "$\"str\"" +readDollarLonely quoted = do start <- startSpan char '$' id <- endSpan start - n <- lookAhead (anyChar <|> (eof >> return '_')) + when quoted $ do + isHack <- quoteForEscape + when isHack $ + parseProblemAtId id StyleC 1135 + "Prefer escape over ending quote to make $ literal. Instead of \"It costs $\"5, use \"It costs \\$5\"." return $ T_Literal id "$" + where + quoteForEscape = option False $ try . lookAhead $ do + char '"' + -- Check for "foo $""bar" + optional $ char '"' + c <- anyVar + -- Don't trigger on [[ x == "$"* ]] or "$"$pattern + return $ c `notElem` "*$" + anyVar = variableStart <|> digit <|> specialVariable + prop_readHereDoc = isOk readScript "cat << foo\nlol\ncow\nfoo" prop_readHereDoc2 = isNotOk readScript "cat <<- EOF\n cow\n EOF" prop_readHereDoc3 = isOk readScript "cat << foo\n$\"\nfoo" @@ -2056,7 +2094,8 @@ readSource :: Monad m => Token -> SCParser m Token -readSource t@(T_Redirecting _ _ (T_SimpleCommand cmdId _ (cmd:file:_))) = do +readSource t@(T_Redirecting _ _ (T_SimpleCommand cmdId _ (cmd:file':rest'))) = do + let file = getFile file' rest' override <- getSourceOverride let literalFile = do name <- override `mplus` getLiteralString file @@ -2072,15 +2111,21 @@ proceed <- shouldFollow filename if not proceed then do + -- FIXME: This actually gets squashed without -a parseNoteAtId (getId file) InfoC 1093 "This file appears to be recursively sourced. Ignoring." return t else do sys <- Mr.asks systemInterface - input <- + (input, resolvedFile) <- if filename == "/dev/null" -- always allow /dev/null - then return (Right "") - else system $ siReadFile sys filename + then return (Right "", filename) + else do + currentScript <- Mr.asks currentFilename + paths <- mapMaybe getSourcePath <$> getCurrentAnnotations True + resolved <- system $ siFindSource sys currentScript paths filename + contents <- system $ siReadFile sys resolved + return (contents, resolved) case input of Left err -> do parseNoteAtId (getId file) InfoC 1091 $ @@ -2091,7 +2136,7 @@ id2 <- getNewIdFor cmdId let included = do - src <- subRead filename script + src <- subRead resolvedFile script return $ T_SourceCommand id1 t (T_Include id2 src) let failed = do @@ -2101,10 +2146,22 @@ included <|> failed where + getFile :: Token -> [Token] -> Token + getFile file (next:rest) = + case getLiteralString file of + Just "--" -> next + x -> file + getFile file _ = file + + getSourcePath t = + case t of + SourcePath x -> Just x + _ -> Nothing + subRead name script = withContext (ContextSource name) $ inSeparateContext $ - subParse (initialPos name) readScript script + subParse (initialPos name) (readScriptFile True) script readSource t = return t @@ -2331,6 +2388,17 @@ id <- endSpan start return $ T_BraceGroup id list +prop_readBatsTest = isOk readBatsTest "@test 'can parse' {\n true\n}" +readBatsTest = called "bats @test" $ do + start <- startSpan + try $ string "@test" + spacing + name <- readNormalWord + spacing + test <- readBraceGroup + id <- endSpan start + return $ T_BatsTest id name test + prop_readWhileClause = isOk readWhileClause "while [[ -e foo ]]; do sleep 1; done" readWhileClause = called "while loop" $ do start <- startSpan @@ -2590,6 +2658,7 @@ readForClause, readSelectClause, readCaseClause, + readBatsTest, readFunctionDefinition ] spacing @@ -2696,7 +2765,7 @@ variable <- readVariableName when lenient $ optional (readNormalDollar >> parseNoteAt pos ErrorC - 1067 "For indirection, use (associative) arrays or 'read \"var$n\" <<< \"value\"'") + 1067 "For indirection, use arrays, declare \"var$n=value\", or (for sh) read/eval.") indices <- many readArrayIndex hasLeftSpace <- fmap (not . null) spacing pos <- getPosition @@ -2715,9 +2784,10 @@ when (hasLeftSpace || hasRightSpace) $ parseNoteAt pos ErrorC 1068 $ "Don't put spaces around the " - ++ if op == Append - then "+= when appending." - else "= in assignments." + ++ (if op == Append + then "+= when appending" + else "= in assignments") + ++ " (or quote to make it literal)." value <- readArray <|> readNormalWord spacing return $ T_Assignment id op variable indices value @@ -2735,11 +2805,12 @@ string "=" >> return Assign ] - readEmptyLiteral = do - start <- startSpan - id <- endSpan start - return $ T_Literal id "" +readEmptyLiteral = do + start <- startSpan + id <- endSpan start + return $ T_Literal id "" + readArrayIndex = do start <- startSpan char '[' @@ -2886,12 +2957,14 @@ prop_readShebang6 = isWarning readShebang " # Copyright \n!#/bin/bash" prop_readShebang7 = isNotOk readShebang "# Copyright \nfoo\n#!/bin/bash" readShebang = do + start <- startSpan anyShebang <|> try readMissingBang <|> withHeader many linewhitespace str <- many $ noneOf "\r\n" + id <- endSpan start optional carriageReturn optional linefeed - return str + return $ T_Literal id str where anyShebang = choice $ map try [ readCorrect, @@ -2967,22 +3040,72 @@ try (lookAhead p) action -prop_readScript1 = isOk readScriptFile "#!/bin/bash\necho hello world\n" -prop_readScript2 = isWarning readScriptFile "#!/bin/bash\r\necho hello world\n" -prop_readScript3 = isWarning readScriptFile "#!/bin/bash\necho hello\xA0world" -prop_readScript4 = isWarning readScriptFile "#!/usr/bin/perl\nfoo=(" -prop_readScript5 = isOk readScriptFile "#!/bin/bash\n#This is an empty script\n\n" -readScriptFile = do + +readConfigFile :: Monad m => FilePath -> SCParser m [Annotation] +readConfigFile filename = do + shouldIgnore <- Mr.asks ignoreRC + if shouldIgnore then return [] else read' filename + where + read' filename = do + sys <- Mr.asks systemInterface + contents <- system $ siGetConfig sys filename + case contents of + Nothing -> return [] + Just (file, str) -> readConfig file str + + readConfig filename contents = do + result <- lift $ runParserT readConfigKVs initialUserState filename contents + case result of + Right result -> + return result + + Left err -> do + parseProblem ErrorC 1134 $ errorFor filename err + return [] + + errorFor filename err = + let line = "line " ++ (show . sourceLine $ errorPos err) + suggestion = getStringFromParsec $ errorMessages err + in + "Failed to process " ++ filename ++ ", " ++ line ++ ": " + ++ suggestion + +prop_readConfigKVs1 = isOk readConfigKVs "disable=1234" +prop_readConfigKVs2 = isOk readConfigKVs "# Comment\ndisable=1234 # Comment\n" +prop_readConfigKVs3 = isOk readConfigKVs "" +prop_readConfigKVs4 = isOk readConfigKVs "\n\n\n\n\t \n" +prop_readConfigKVs5 = isOk readConfigKVs "# shellcheck accepts annotation-like comments in rc files\ndisable=1234" +readConfigKVs = do + anySpacingOrComment + annotations <- many (readAnnotationWithoutPrefix <* anySpacingOrComment) + eof + return $ concat annotations +anySpacingOrComment = + many (void allspacingOrFail <|> void readAnyComment) + +prop_readScript1 = isOk readScript "#!/bin/bash\necho hello world\n" +prop_readScript2 = isWarning readScript "#!/bin/bash\r\necho hello world\n" +prop_readScript3 = isWarning readScript "#!/bin/bash\necho hello\xA0world" +prop_readScript4 = isWarning readScript "#!/usr/bin/perl\nfoo=(" +prop_readScript5 = isOk readScript "#!/bin/bash\n#This is an empty script\n\n" +readScriptFile sourced = do start <- startSpan pos <- getPosition optional $ do readUtf8Bom parseProblem ErrorC 1082 "This file has a UTF-8 BOM. Remove it with: LC_CTYPE=C sed '1s/^...//' < yourscript ." - sb <- option "" readShebang + shebang <- readShebang <|> readEmptyLiteral + let (T_Literal _ shebangString) = shebang allspacing annotationStart <- startSpan - annotations <- readAnnotations + fileAnnotations <- readAnnotations + rcAnnotations <- if sourced + then return [] + else do + filename <- Mr.asks currentFilename + readConfigFile filename + let annotations = fileAnnotations ++ rcAnnotations annotationId <- endSpan annotationStart let shellAnnotationSpecified = any (\x -> case x of ShellOverride {} -> True; _ -> False) annotations @@ -2990,19 +3113,19 @@ let ignoreShebang = shellAnnotationSpecified || shellFlagSpecified unless ignoreShebang $ - verifyShebang pos (getShell sb) - if ignoreShebang || isValidShell (getShell sb) /= Just False + verifyShebang pos (getShell shebangString) + if ignoreShebang || isValidShell (getShell shebangString) /= Just False then do commands <- withAnnotations annotations readCompoundListOrEmpty id <- endSpan start verifyEof let script = T_Annotation annotationId annotations $ - T_Script id sb commands + T_Script id shebang commands reparseIndices script else do many anyChar id <- endSpan start - return $ T_Script id sb [] + return $ T_Script id shebang [] where basename s = reverse . takeWhile (/= '/') . reverse $ s @@ -3019,7 +3142,7 @@ case isValidShell s of Just True -> return () Just False -> parseProblemAt pos ErrorC 1071 "ShellCheck only supports sh/bash/dash/ksh scripts. Sorry!" - Nothing -> parseProblemAt pos InfoC 1008 "This shebang was unrecognized. Note that ShellCheck only handles sh/bash/dash/ksh." + Nothing -> parseProblemAt pos ErrorC 1008 "This shebang was unrecognized. ShellCheck only supports sh/bash/dash/ksh. Add a 'shell' directive to specify." isValidShell s = let good = s == "" || any (`isPrefixOf` s) goodShells @@ -3036,6 +3159,7 @@ "ash", "dash", "bash", + "bats", "ksh" ] badShells = [ @@ -3051,7 +3175,7 @@ readUtf8Bom = called "Byte Order Mark" $ string "\xFEFF" -readScript = readScriptFile +readScript = readScriptFile False -- Interactively run a specific parser in ghci: -- debugParse readSimpleCommand "echo 'hello world'" @@ -3086,6 +3210,8 @@ Environment { systemInterface = (mockedSystemInterface []), checkSourced = False, + currentFilename = "myscript", + ignoreRC = False, shellTypeOverride = Nothing } @@ -3261,6 +3387,8 @@ env = Environment { systemInterface = sys, checkSourced = psCheckSourced spec, + currentFilename = psFilename spec, + ignoreRC = psIgnoreRC spec, shellTypeOverride = psShellTypeOverride spec } diff --git a/src/ShellCheck/Regex.hs b/src/ShellCheck/Regex.hs --- a/src/ShellCheck/Regex.hs +++ b/src/ShellCheck/Regex.hs @@ -1,5 +1,5 @@ {- - Copyright 2012-2015 Vidar Holen + Copyright 2012-2019 Vidar Holen This file is part of ShellCheck. https://www.shellcheck.net @@ -30,7 +30,7 @@ -- Precompile the regex mkRegex :: String -> Regex mkRegex str = - let make :: RegexMaker Regex CompOption ExecOption String => String -> Regex + let make :: String -> Regex make = makeRegex in make str diff --git a/test/shellcheck.hs b/test/shellcheck.hs --- a/test/shellcheck.hs +++ b/test/shellcheck.hs @@ -2,22 +2,28 @@ import Control.Monad import System.Exit -import qualified ShellCheck.Checker import qualified ShellCheck.Analytics import qualified ShellCheck.AnalyzerLib -import qualified ShellCheck.Parser +import qualified ShellCheck.Checker import qualified ShellCheck.Checks.Commands +import qualified ShellCheck.Checks.Custom import qualified ShellCheck.Checks.ShellSupport +import qualified ShellCheck.Fixer +import qualified ShellCheck.Formatter.Diff +import qualified ShellCheck.Parser main = do putStrLn "Running ShellCheck tests..." results <- sequence [ - ShellCheck.Checker.runTests, - ShellCheck.Checks.Commands.runTests, - ShellCheck.Checks.ShellSupport.runTests, - ShellCheck.Analytics.runTests, - ShellCheck.AnalyzerLib.runTests, - ShellCheck.Parser.runTests + ShellCheck.Analytics.runTests + ,ShellCheck.AnalyzerLib.runTests + ,ShellCheck.Checker.runTests + ,ShellCheck.Checks.Commands.runTests + ,ShellCheck.Checks.Custom.runTests + ,ShellCheck.Checks.ShellSupport.runTests + ,ShellCheck.Fixer.runTests + ,ShellCheck.Formatter.Diff.runTests + ,ShellCheck.Parser.runTests ] if and results then exitSuccess