koalaman / shellcheck

ShellCheck, a static analysis tool for shell scripts
https://www.shellcheck.net
GNU General Public License v3.0
36.25k stars 1.77k forks source link

Invalid SC2016 for xargs sh -c '$PWD' #1894

Open tioteath opened 4 years ago

tioteath commented 4 years ago

For bugs

For new checks and feature suggestions

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

#!/bin/sh

fswatch . --recursive --utc-time --one-per-batch --insensitive --allow-overflow --print0 --monitor=poll_monitor | xargs -0 -I{} --no-run-if-empty /bin/sh -c 'export ACTUAL_PWD=${PWD}'

Here's what shellcheck currently says:

Line 3  SC2016: Expressions don't expand in single quotes, use double quotes for that.

Here's what I wanted or expected to see:

Expressions get expanded in the fork of sh, hence single quotes.

Akianonymus commented 4 years ago

+1

pmenzel commented 2 years ago

Same issue with bash -c 'some bash commands'

dimo414 commented 2 years ago

IMO this finding is correct, and should be suppressed if no variable expansion is intended. Callers could just as easily want eager variable expansion, and single-quotes + a suppression makes it clear the author intended to lazily evaluate the variables in the subprocess' environment.

There is an allowlist for commands that often expect single-quoted variables, but I worry xargs would be too heavy-handed a command to include in that list.

nwithers-ecr commented 2 years ago

Here is something similar that I am doing

xargs -I{} bash -c 'k3d kubeconfig get {} > $HOME/.kube/config.d/k3d-{}.yaml'

but rather than not displaying SC2016 (we can always turn it off), it seems to me these expressions should be reporting SC2086