koalaman / shellcheck

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

Do not emit `SC2010`/`SC2012` for plain `ls | ...` #2900

Open mohd-akram opened 10 months ago

mohd-akram commented 10 months ago

For bugs

Using a plain ls | grep and/or ls dir/ | grep (with the trailing slash) is safe and predictable. Per POSIX, they would always output one entry per line since the output is piped (i.e. not output to a terminal). Both forms cannot confuse a directory with a file (as in ls dir | grep).

The relevant bit from POSIX:

The default format shall be to list one entry per line to standard output; the exceptions are to terminals or when one of the -C, -m, or -x options is specified.

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

#!/bin/sh
ls | grep
ls dir/ | grep
ls | cat

Here's what shellcheck currently says:

^-- [SC2010](https://www.shellcheck.net/wiki/SC2010) (warning): Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
^-- [SC2012](https://www.shellcheck.net/wiki/SC2012) (info): Use find instead of ls to better handle non-alphanumeric filenames.

The warning also seems to be misleading. I don't believe ls has any issues with non-alphanumeric filenames when not outputting to a terminal (only newlines, and many things would break in that case, including find which is suggested). The warning should be changed to check for arguments not terminated with a slash, or at the very least allow a plain ls which is as safe as printf '%s\n' *.

Here's what I wanted or expected to see:

No warning.