Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SC2086 should not complain about usage within set #1194

Closed
1 of 3 tasks
smoser opened this issue Apr 27, 2018 · 3 comments
Closed
1 of 3 tasks

SC2086 should not complain about usage within set #1194

smoser opened this issue Apr 27, 2018 · 3 comments

Comments

@smoser
Copy link

smoser commented Apr 27, 2018

It seems that SC2086 should not complain about usage with 'set'. In most cases if someone has unquoted strings in set -- $VAR then that was by design. Perhaps there could be another warning code for use in set -- specifically.

For bugs

  • Rule Id: SC2086
  • My shellcheck version: 0.4.6 and online
  • I tried on shellcheck.net and verified that this is still a problem on the latest commit

For new checks and feature suggestions

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

#!/bin/sh
foo="foo bar zee"
set -- $foo
echo "there were $# tokens in $foo"

Here's what shellcheck currently says:

$ shellcheck myscript
 
Line 3:
set -- $foo
       ^-- SC2086: Double quote to prevent globbing and word splitting.

Here's what I wanted or expected to see:

I'd like to not have to disable the check on that line and still get the generally-useful SC2086 globally.

@orbea
Copy link

orbea commented Apr 27, 2018

You can avoid the warning with.

eval "set -- $foo"

However while this is safe in your example extra care should be taken to make sure you always control the variables when eval is used.

@koalaman
Copy link
Owner

I'm not sure this is a general issue with set. I'd argue for a warning even in this case because this doesn't seem intentional:

there were 182 tokens in foo * zee

There are also other reasons why you'd want such a warning:

var="hello world"
set -- --msgbox $var 6 40
dialog "$@"

If splitting/globbing is a generally an issue, but you've identified a specific case where it's not, then that's exactly what disable statements are for.

@smoser
Copy link
Author

smoser commented Apr 30, 2018

I had suggested "Perhaps there could be another warning code for use in set -- specifically.".
As @orbea said, you could use eval but that gets tricky anyway.

I'd much prefer just adding a warning specifically to 'set' that I could disable globally compared to in-line disable statements on each use.

smoser added a commit to canonical/cloud-init that referenced this issue May 8, 2018
This fixes warnings reported by shellcheck at 0.4.6.
The complaints that we are ignoring globally (top of the file) are:
 2015: Note that A && B || C is not if-then-else. C may run if A is true.
 2039: In POSIX sh, 'local' is undefined.
 2162: read without -r will mangle backslashes.
 2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

Most of the complaints were just noise, but a few unused variables
were reported and fixed.

Related shellcheck issues opened:
 - koalaman/shellcheck#1191
 - koalaman/shellcheck#1192
 - koalaman/shellcheck#1193
 - koalaman/shellcheck#1194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants