syntax: simplify assignments that don't need to be quoted

[whatisaphone]

When using the simplify flag (-s), here are some more patterns that could be simplified. All of these are removing double quotes.

# Before
foo="${bar}"
# After
foo=${bar}

# Before
foo="$(date)"
# After
foo=$(date)

# Before
case "${foo}" in
    'a b') echo works ;;
esac
# After
case ${foo} in
    'a b') echo works ;;
esac

I tested in this bash version:

GNU bash, version 4.4.20(1)-release (x86_64-pc-linux-gnu)

I don't think this screws things up for other shells, but I guess you can never be 100% sure with shell, can you

[mvdan]

While you are correct in theory (both of those cases don't do word splitting), I think it could throw off lots of users who don't remember that, and make consistency harder to accomplish.

For example:

foo="$(xxx)" cmd "$(yyy)" # perhaps a bit unnecessary, but more consistent
foo=$(xxx) cmd "$(yyy)"

Also, some other tools like shellcheck encourage using quoting for practically all uses of parameter expansions and command substitutions, so the two tools might end up trying to fight each other.

[whatisaphone]

Good points.

I did check these cases for compatibility with shellcheck before opening the issue. (And I'm not sure if any other shell linters exist besides IDE-specific ones)

I just suggested this for consistency, since I noticed shfmt -s removes quotes in some other situations. Example:

foo='a b'
[[ -z "${foo}" ]]

I guess nothing is perfect and you have to draw the line somewhere. Feel free to close if you want, or do whatever you think is best!

[mvdan]

I forget exactly all the things that -s does. I should check again :)

[nkakouros]

Given that quoting all parameter expansions is quite popular as a best practice (that's what I have gathered anyway), would it be too much to extract the removal of quotes into a separate flag? --simplify would still have plenty of useful stuff to do:

https://github.com/mvdan/sh/blob/5146d3eb581ae7f0b4f55a59d0c642fa5cea88d2/syntax/simplify.go#L11-L19

that you don't miss out on if you don't want unquoted expansions.

[mvdan]

We only remove one instance of double quoting, [[ "$var" == str ]], and I think that one shouldn't be controversial. [[ already has its own parse and evaluation mode with tokens like , and (, and we format with spaces around most expressions, so it should be fairly obvious that [[ $var == str ]] is not a bug. So I don't see a reason to remove it from -s, @nkakouros.

Even though the same technically applies to foo=${bar}, as assignments get parsed and evaluated in a special way, I'm on the fence about it. I've seen quoting in assignments quite often, and it can help with consistency, like I showed above. So it feels like we would be annoying or confusing the user more often than not. The other week I even forgot whether foo=$bar was a bug or not, and I implemented it! :)

I'm going to close this as "working as intended" for now. Note that I would happily have others write programs on top of our syntax package, just like shfmt is - then you can write transformations which are as aggressive or customizable as you'd like. Mine definitely lean towards being conservative and easy to maintain.