Spurious SC2086 with multiple files (0.9.0 regression)
For bugs
- Rule Id (if any, e.g. SC1000): SC2086
- My shellcheck version (
shellcheck --versionor "online"): 0.9.0 - [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
Here's a snippet or screenshot that shows the problem:
main.sh:
#!/bin/bash
source b.sh
b.sh:
function fun() {
local frame=0 str
while str=$(caller $frame) ; do
echo "$str"
done
}
exit 1
exit
Here's what shellcheck currently says:
$ shellcheck -x -a ./main.sh
In b.sh line 3:
while str=$(caller $frame) ; do
^----^ SC2086 (info): Double quote to prevent globbing and word splitting.
Did you mean:
while str=$(caller "$frame") ; do
For more information:
https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
Here's what I wanted or expected to see:
No warning, as with a single file, and as with ShellCheck 0.8.x
I get SC2086 for $frame both while checking the b.sh file directly and while included. And that seems to be expected, if you want to silence it you should add a directive about it.
I get SC2086 for $frame both while checking the b.sh file directly and while included.
There are some suspicious differences between the two. Parsing the file directly produces a different set of diagnostics. However, that is a separate problem, and not the one reported here.
The example above is a reduction from a larger script, reduced in a way that preserved the exact set of diagnostics.
And that seems to be expected
What makes you say that?
Please pay attention to the following:
framecan never have any values that would necessitate quoting.- ShellCheck can generally observe when variables can never have any values that would necessitate quoting, and does not emit SC2086 in those cases.
- This is a regression compared to the behavior of ShellCheck 0.8.1.
frame can never have any values that would necessitate quoting.
never in the current context. If that context changes it can however and it's easy to miss that part.
Having a recommendation to always add quotes is a nice way of avoiding that and as far as I can remember is the real world behind this decision.
the other warnings are about reachability and those seems valid as the function is not executed. the extra exit after the first exit is a clear example.
That is a valid point of view, but it doesn't change that there is almost certainly a bug here, at least by nature of inconsistency, and the bug is a regression.
This looks like a bug in shellcheck 0.8.0, fixed in 0.9.0.
No, there is definitely a bug here. Please note the details of the test case.
This file produces no warnings:
#!/bin/bash
function fun() {
local frame=0 str
while str=$(caller $frame) ; do
echo "$str"
done
}
If this was actually about quoting $frame, then this would also produce warnings. But it (correctly) doesn't.
Things start to get wonky once you start adding multiple files or adding exit statements. This is what this bug is actually about.
Worth noting that #2851 is very similar, possibly even having the same root cause. It should be easier to agree that the diagnostic emitted there is wrong.
According to the changelog SC2086 should be more accurate in 0.9.0 and this seem to be a proof of the opposite.
- SC2086: Now uses DFA to make more accurate predictions about values
Given that there are room for a PR.
What ever was the case in 0.8.0 I have no memory of. And as stated I am in the camp of quoting everything everywhere =)