Add new rule - Don't use process substitution to avoid masking return values

[ggjulio]

For new checks and feature suggestions

• https://www.shellcheck.net/ (i.e. the latest commit) currently gives no useful warnings about this
• I searched through https://github.com/koalaman/shellcheck/issues and didn't find anything related

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

set -euo pipefail
readarray -t my_array < <(some_command_failing | sort -u)
echo passed

Here's what shellcheck currently says:

No errors

Here's what I wanted or expected to see:

Process substitution should be usually avoided to avoid masking return values at least when errexit is used.

It's a similar issue than when using a subshell in a variable declaration:

Declare and assign separately to avoid masking return values. (shellcheck SC2155)

IMHO it would make sense to create a new rule :

Don't use process substitution to avoid masking return values. (shellcheck SCxxxx)

source: http://mywiki.wooledge.org/BashFAQ/105

Bad example:

readarray -t my_array < <(some_command | sort -u)

Good example:

output=$(some_command | sort -u)
readarray -t my_array <<< "${output}"

Originally posted by @ggjulio in #3084 (comment)

[wileyhy]

set -e is crazy pants. set -e "will drive you to drink," as the saying goes, but process substitutions are useful. They are easier than while / read loops, for one. The exit code of a command is often less valuable than the data it pipes to Stdout. Is that data what the script's author was expecting it to be, so that `awk '$2 ~ /f..b.r/ { print "whatever" }' can behave as planned? Shell has limitations. Does a file exist? Is a variable populated? Does some input or output match some regexp? There are straightforward examples like `dnf install foo` where their exit codes are well documented and make logical sense. But there are also commands like `ping` that can produce x3 useful exit codes. Whether 0 = success and 1 = fail isn't standardized. (See arithmetic evaluation in bash.) There isn't any standard, regarding exit codes, that coders of command line programs are required to follow. Many POSIX compliant scripts use non-Posix commands (eg, sudo). How do you define "fail," in the context of CLI programs? What if your idea of "fail" is something different from what the author or maintainer of the (any given) program considers to be definable as "fail." Tasks usually involve some form of statefulness. Something can be in some particular "form" or "state" or "condition" at any given time. A job might require that the statefulness of some particular thing in the system ('is package management up to date?) be changed from one state to another state; from its current state to some future state. How can one positively know the state of anything? Data. `firewall-cmd --state`. A program can communicate in various ways, but exit codes offer less raw information for humans than human language does. Exit codes aren't standardized, and any number 0 can somewhat easily be mistaken for any other number. It's much more difficult, in comparison, to mistake one verbal output of `getenforce` from any other, for example (and `getenforce` has x3 valid outputs). Therefore exit codes are less reliable, and reliable keeps the lights on. "I yield back the remainder of my time." ;-P Wiley

…

On Tue, Dec 3, 2024, 01:40 Julio ***@***.***> wrote: Process substitution should be usually avoided to avoid masking return values at least when errexit is used. It's a similar issue than when using a subshell in a variable declaration: Declare and assign separately to avoid masking return values. (shellcheck SC2155 <https://www.shellcheck.net/wiki/SC2155>) IMHO it would make sense to create a new rule : Don't use process substitution to avoid masking return values. (shellcheck SCxxxx) image.png (view on web) <https://github.com/user-attachments/assets/81c9b316-32d2-4509-bdb5-55c5172a4b88> image.png (view on web) <https://github.com/user-attachments/assets/c526cdfa-94d1-44a7-af02-beee34c61946> source: http://mywiki.wooledge.org/BashFAQ/105 Bad example: readarray -t my_array < <(some_command | sort -u) Good example: output=$(some_command | sort -u) readarray -t my_array <<< "${output}" *Originally posted by @ggjulio <https://github.com/ggjulio> in #3084 (comment) <#3084 (comment)>* — Reply to this email directly, view it on GitHub <#3094>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F253WYGHE6YLM6XN2SD2DV4A3AVCNFSM6AAAAABS5LD3TKVHI2DSMVQWIX3LMV43ASLTON2WKOZSG4YTINBYGQZDENI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[ggjulio]

@wileyhy This an opinion that not everyone share. I cited a source that is against errexit as example but it's not the topic of this issue.

Some shellcheck rules are enabled/disabled based on whether errexit is on/off, so there is no point of debating about it.

To balance the original source cited :

• http://redsymbol.net/articles/unofficial-bash-strict-mode/
• https://blog.cloudflare.com/pipefail-how-a-missing-shell-option-slowed-cloudflare-down/
• basically any popular OSS repos usually uses errexit (ci/cd scripts, container entrypoints, any critical scripts that must fail if something goes wrong)

Please don't flood this issue with a second response. Either open a different issue, or use GH discussion / Reddit.

[wileyhy]

Sometimes people use Shellcheck as a tutorial program. (I think it's probably quite common to have done so, but how do you measure something like that?) As such, the default rule set matters because it shapes how learners of shell conceive of "common best practices." So, you see, there very much is a "point." Is "it" something that everyone should do? Limiting use of procsubs would be about enabling a greater observability of exit codes in order to more efficiently use errexit. Did I get that right? Was there anything I said incorrectly? "Introduce a rule that disfavors all use of procsubs," which really are just kind of pedestrian (I don't recall ever having seen a debate about 'for or against procsubs'), "in favor of more efficiently using errexit," a widely and sometimes hotly contested feature of bash, POSIX shell and numerous other shells. Examples of such debates are readily available. One can distill that idea down to "get rid of something that isn't broken in favor of something that isn't not broken." Is that really something that everyone should do? I think the answer to this particular question should be "no." Why disfavour procsubs in order to fix errexit? Why not just fix errexit? Wiley

…

On Sat, Dec 7, 2024, 00:32 Julio ***@***.***> wrote: @wileyhy <https://github.com/wileyhy> This an opinion that not everyone share. I cited a source that is against errexit as example but it's not the topic of this issue. Shellcheck enable/disable rules based on whether errexit is on/off, so there is absolutely no point of debating about it. To balance the original source I cited : - redsymbol.net/articles/unofficial-bash-strict-mode/ - https://blog.cloudflare.com/pipefail-how-a-missing-shell-option-slowed-cloudflare-down/ - basically any popular OSS repos (scripts used in ci/cd, container entrypoints, any critical scripts that must fail if something goes wrong) Please don't flood this issue with a second response. Either open a different issue, or use GH discussion / Reddit. — Reply to this email directly, view it on GitHub <#3094 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUF2F2ZYON2NCB5ZQ6P3STL2EKXAFAVCNFSM6AAAAABS5LD3TKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRVGAZDSMBSGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>