SC2317 should only trigger once per unreachable code path

[dimo414]

For bugs

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

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

#!/bin/bash
foo() { bar; baz; bang; biff; bow; }
exit 0

Here's what shellcheck currently says:

$ shellcheck myscript
 
Line 2:
foo() { bar; baz; bang; biff; bow; }
        ^-- SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).
             ^-- SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).
                  ^-- SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).
                        ^-- SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).
                              ^-- SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).

Here's what I wanted or expected to see:

It's debatable whether this should trigger at all, but clearly there's no reason for this finding to trigger on every command in an unreachable code path. This generates a lot of noise and makes it hard to tell which findings are actionable.

In this case foo is what's actually not being reached, not (just) the individual commands within foo. So it would be even better to flag a case like this as "foo appears unused" rather than "the contents of foo appear unreachable".

[oliv3r]

It's not even correct, is it? It's not an unreachable function, it's an unused function. Which is fine, isn't it?

e.g. if we have a template, that always has a 'pr_info', 'pr_error' function, and in a script, it doesn't have one of those, is that an error? a warning? gcc treats unreachable functions as a warning, this is true, but then we also have the __maybe_unused.

I'm all for telling the user of unreachable codepaths (you did something not so smart, this code can not ever be executed), vs 'you have an unused function'. One is (potentially) an error, the other at the most a warning.

I'd be in favor in splitting these two cases, and probably not even care about unused functions, but at least unused functions (or supporting the '__may_be_unused' flag).