anordal / shellharden

The corrective bash syntax highlighter
Mozilla Public License 2.0
4.63k stars 130 forks source link

Allow unquoted arguments to local et al #30

Closed pbrisbin closed 4 years ago

pbrisbin commented 4 years ago

I've grown accustom to not quoting the right hand side of variable assignments,

var=$othervar
var=$(subshell)
var=value

This is perfectly safe in all POSIX shells (AFAIK). Reference.

Shellharden adds quotes to all of these. Do you consider this a bug?

anordal commented 4 years ago

You must be an early adopter. Try upgrading.

This feature (57214: Allow unquoted rvalues) was part of the v4.1 release.

I define a bug as a deviation from documentation, so this was a missing feature.

pbrisbin commented 4 years ago

Ah, I apologize, I'm not sure what I was looking at. I ran into this in the context of Restyled, where it restyled some extra quotes on me and claimed it was shellharden that did it. I just checked directly and (of course), you're right:

% docker run -it --rm restyled/restyler-shellharden:v4.1.1-2 sh
/code # shellharden '' <<'EOM'
> foo=$bar
> baz=bat
> bip=im\ bop
> EOM
foo=$bar
baz=bat
bip=im\ bop
/code #

I'm not sure what was going on in that PR, but I'll just have to investigate some other cause next time I see it. Sorry for the noise.

pbrisbin commented 4 years ago

Oh wait, I found it! I think it may still be a bug:

/code # shellharden '' <<'EOM'
> local foo=$1
> EOM
local foo="$1"
/code # shellharden --version
4.1.1

The local seems to be throwing it off.

anordal commented 4 years ago

Thank you!

What's going on here is that local is a command. It is not (exactly the same as) an assignment, neigher syntactically (consider multiple arguments) nor when it comes to errexit's influence on command substitutions.

Thus why I recommend to separate the declaration from the assignment, at least with command substitutions.

But I must admit that local foo=$1 is legit, useful, and worth supporting.

pbrisbin commented 4 years ago

Thus why I recommend to separate the declaration from the assignment, at least with command substitutions.

That's exactly what ShellCheck says to do as well, but only with command substitution -- so I tend to assign right in the declaration otherwise.

Thanks for re-opening!