Associative array key with hyphen breaks in v3.6.0

[DannyBen]

This is a new bug that did not exist in 3.5.1.

Test code

#!/usr/bin/env bash

if [[ -n "${args[--no-cache]:-}" ]]; then
  echo "ok"
fi

Actual output

➜ shfmt -d -i 2 test.sh
--- test.sh.orig
+++ test.sh
@@ -1,5 +1,5 @@
 #!/usr/bin/env bash

-if [[ -n "${args[--no-cache]:-}" ]]; then
+if [[ -n "${args[--no - cache]:-}" ]]; then
   echo "ok"
 fi

Expected output

Empty, no change.

[mvdan]

I don't think I've ever had a regression reported against a new release so quickly :)

The change looks correct; the key isn't quoted, so we assume it is an arithmetic expression, hence the spacing around the - operator. See the "caveats" section in the README. The way to tell the parser that your key is a string is to quote it, as otherwise it's ambiguous.

I'm not sure I can reproduce the regression either; the previous release results in the same output:

$ shfmt -version
v3.5.1
$ cat f.sh
#!/usr/bin/env bash

if [[ -n "${args[--no-cache]:-}" ]]; then
  echo "ok"
fi
$ shfmt -d f.sh
--- f.sh.orig
+++ f.sh
@@ -1,5 +1,5 @@
 #!/usr/bin/env bash

-if [[ -n "${args[--no-cache]:-}" ]]; then
-  echo "ok"
+if [[ -n "${args[--no - cache]:-}" ]]; then
+   echo "ok"
 fi

[DannyBen]

Weird, but I also see the same issue now with 3.5.1. Maybe I was wrong about it working differently. Its just that shfmt did not report anything on my scripts other than the case comment indentation issue I reported.

I still think this is a bug, not a caveat. This script passes shellcheck, which means in my book that this is a valid way of providing associative key. Can't the rule that checks for hyphens be modified to avoid a pattern that indicates it is an array?

That said - I understand if it cannot be fixed. I will adjust my script generator to quote these.

Feel free to close this issue if it is unfixable.

[mvdan]

I still think this is a bug, not a caveat.

Depends on how you look at it, I guess :) Bash as a language is ambiguous in multiple ways. That's generally possible to support in the parser, as it can re-try parsing in a different way if its first guess was wrong (https://github.com/mvdan/sh/issues/686). However, in cases like yours, the printer (formatter) doesn't have this luxury. The parser accepted an arithmetic expression, so as far as the printer is concerned, it needs to be properly formatted with spaces.

This script passes shellcheck, which means in my book that this is a valid way of providing associative key.

I'm not sure how shellcheck works, but I imagine your case is doable for any tool that juts inspects the code. Per the above, the parser is not the problem, it's the formatter.

Can't the rule that checks for hyphens be modified to avoid a pattern that indicates it is an array?

I'm sorry, I'm not sure I follow. The parser can't know if args is an indexed or associative array, and arrays in bash are indexed by default, so that's how we parse index expressions - as arithmetic expressions. The only way for the parser to know that the key is a string is to quote it. I realise that that's unfortunate, but that's simply a product of how the language is (frustratingly) ambiguous :)

[DannyBen]

Understood. I will adjust my code accordingly, with the objective of all the code generated by bashly to be shfmt compliant (https://github.com/DannyBen/bashly/issues/313).

Thanks for the release, I will close this issue then.