RFC: Should SC2312 warn on testing pipelines?

[hseg]

For bugs

• Rule Id (if any, e.g. SC1000): SC2312
• My shellcheck version (shellcheck --version or "online"): 0.10.0 and online
• [x] The rule's wiki page does not already cover this (e.g. https://shellcheck.net/wiki/SC2086)
• [x] I tried on https://www.shellcheck.net/ and verified that this is still a problem on the latest commit

For new checks and feature suggestions

• [x] https://www.shellcheck.net/ (i.e. the latest commit) currently gives no useful warnings about this
• [x] 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:

#!/bin/bash
# shellcheck enable=all

if false | grep hello; then echo failure; fi

Here's what shellcheck currently says:

Line 4:
if false | grep hello; then echo failure; fi
   ^-- SC2312 (info): Consider invoking this command separately to avoid masking its return value (or use '|| true' to ignore).

Here's what I wanted or expected to see:

Either nothing (after all, the fact that I'm testing the result of the pipeline indicates I am capturing the return value, albeit only of the last command), or a tailored warning indicating that the test as written will make me blind to which command failed.

Judging by the wiki page, the intent of the warning is more to flag that command substitutions can fail and leave null strings in their place. If this is the intent, then the wording of the warning doesn't communicate it efectively.

[hseg]

In fact, this fires on any usage of a pipeline:

#!/bin/bash
# shellcheck enable=all

echo "hello world" | grep hello | grep world

I wouldn't consider the "corrected" script to be idiomatic bash, especially considering PIPESTATUS exists. The only case I would consider perhaps leaving intact is

f() { echo "hello world" | grep hello | grep world; }

where the user has left the return code to implicitly be that of the final command, which might be surprising -- perhaps warn to set -o pipefail instead? Though I'm not sure how to make that setting scope only to the function, and am not sure we want to advise users to use it globally.