[Request] Always use curly braces for variables

[techhazard]

I really love shellharden so far, since that allows me to quickly rewrite scripts that I wrote too hastily :slightly_smiling_face: . But when I ran it on a script I put a lot of effort in, it suggested to remove the braces ({, }), in places that do not strictly need them, which is something I prefer to do.

A variable in bash is like a hand grenade – take off its quotes, and it starts ticking.

For the same reason I prefer to always use the braces. Since I do not know how the line will change in the future.

Would it be possible to make shellharden do this? Perhaps as a configuration option?

A simple example which would have been prevented by always using braces:

my_file='abc.txt'
cp "$my_file"{,_copy}
ls "$my_file"

my_file='abc.txt'
cp "$my_file"{,_copy}
ls "$my_file_copy"  # error, unset variable

[anordal]

Thank you for the suggestion and the example. Relaxing this rule is something I have pondered, but never been completely sure about.

The relevant section of the howto:

Now onto string interpolation, where braces are actually useful:

Bad (concatenation): $var1"more string content"$var2
Good (concatentation): "$var1""more string content""$var2"
Good (interpolation): "${var1}more string content${var2}"

Shellharden currently adds and removes braces on an as-needed basis: In the bad example, var1 becomes interpolated with braces, but braces are not accepted on var2 even in the good (interpolation) case, since they are never needed at the end of a string. The latter requirement may well be lifted.

The purpose of the current rule is to rewrite code like ${var1} ${var2} ${var3}, where it seems the author thought braces were some kind of substitute for quoting, and the same code after someone else has made the minimum effort to pass shellcheck "${var1}" "${var2}" "${var3}", where braces and quoting compete for verbosity. That, besides quoting in general, was what I was fed up with (part of the original problem statement) and the minimum bar for what Shellharden had to accomplish.

I have thought about various heuristics for allowing curly braces when actually used in string interpolation, but as you point out, a concatenation with something that comes after is also relevant.

[techhazard]

Thanks for the reply :slightly_smiling_face:, I can see how this already fixes a lot of issues.

Do you know what you want to do with the always-on braces?

[wizard-28]

Would it be possible to make shellharden do this? Perhaps as a configuration option?

I would also prefer this to be a config option and/or a flag passed to the command.

[nkuttler]

Personally, I prefer consistency, that's why I try to always use curly braces. There's also the minor benefit that diffs can be easier to read when they don't include a curly brace change just because something close by was modified.

[anordal]

Consistency is to me more a reason against relaxing the ban on unnecessary braces: Needing braces to delimit variables is not the norm: They are only needed in string processing, and strictly speaking not there either:

# Interpolation
"${var1}bla ${var2}"bla

# Concatenation
"$var1"'bla '"$var2"bla

But correct is correct, and Shellharden is supposed to accept that unless it's bad for a good reason. What I'm considering is to permit unnecessary braces specifically in the interpolation case above, for example if there is more strinng content within the quotes. The rule must be obvious, humanly predictable and right – it would be unpopular to change later.

But if we are talking about adding a mode where braces are always allowed or even obligatory: To add to my main argument that it competes for verbosity and thereby distracts cargo cult programmers who like to worry about wrong things at the cost of the right things: Such a coding style is unfortunately unergonomic to everyone with an AltGr key on their keyboard:

That's AltGr+7. The problem is not the mere 11cm stretch, but that it's in no way reachable at a normal hand and thumb angle. I have seen disappointingly little innovation in keyboard layouts in this area.

[nkuttler]

Code formatters are the epitome of cargo cult programming. 🤷

[hron84]

@anordal start using US keyboard layout, the distance between the shift and curly braces are much shorter. :slightly_smiling_face:

On other hand: I also vote on optionally relaxing this rule for the sake of the consistency on the code. Despite how Bash handles the variables, another aspect on writing a good code is make it easy to read. In my practice, the code is more readable if the variables consistently distinguishable. In the current context, variables fenced by curly braces makes a consistent shape, while the basic $foobar variable camuflages into the surrounding text. Sure, if you are a developer you can have a syntax-highlighed, rich-tooled editor, but as a sysadmin or devops it's not always a case. Sometimes I read bash scripts via a simple cat output, or even worse, pasted into an email, and boys, that's quite hard to interpret. Curly braces help me a lot to understand the script faster.

[meleu]

My apologies for the notifications you guys are receiving because of this post...

I just would like to say that I would also appreciate if there was an option to make shellharden put curly brances in the variables (pretty please :pleading_face::pray:).

[anordal]

Do you know what you want to do with the always-on braces?

Yes. No always-on braces. Let's allow unnecessary braces in interpolations (when there is more content in the string):

" ${ok}"
"${ok} "
"${ok}${ok}"
"$not_considered_an_interpolation"_but_a_concatenation
"$not_considered_an_interpolation"{_but_two,_concatenations}

I hope this is an improvement, although it won't consider @techhazard's example an interpolation.

The concern addressed by unnecessary braces is brittleness. A strictly lesser concern than correctness itself, but perfectly legitimises unnecessary things. This relates to consistency: All necessary braces (on regular variable expansions) can be identified as interpolations.

In terms of where to draw the line, I think this one is the simplest and most reasonable one. It has to be humanly predictable.

A reason to stop here is again consistency: Most variable expansions aren't interpolations. And they shouldn't: The noble thing to do for a shellscript (or any glue code) is to pass variables cleanly.

Code formatters, cargo cult

I would say that the human is the ultimate judge in subjective disputes. Which is why Shellharden is not that kind of formatter. It shall only care about objective things. Cargo cult is defined not by the what, but the why. As such, unnecessary braces can't themselves be called that, but they do obscure the importance of quoting. When they get used seemingly at the expense of quoting, that's when it crosses a reasonable threshold for cargo cult detection and it becomes an objective reason to discourage them.

start using US keyboard layout

It's not about me. The fact that the AltGr key is unergonomically placed in most of the world – nobody has made an ergonomic keyboard – to me discourages that coding style, which speaks against enforcing it, even as an option.

I will allow unnecessary braces in interpolations in version 4.3.0.

[AlexSkrypnyk]

In terms of where to draw the line, I think this one is the simplest and most reasonable one. It has to be humanly predictable.

I do not get this approach @anordal

This is your personal opinion on how the scripts should be written. This opinion should not affect the tool's functionality.

People are asking for a feature. Why not implement it as a flag and let people decide themselves what they want?

[citosid]

I'm sorry to revive this @anordal . Looking at this part of the commit: https://github.com/anordal/shellharden/commit/8e2ed6dfb796c86628c245d9d5cf3c7bddb5e936#diff-8d98843b7ac9e2735c99b61fb37acd50949099336fbedabc69dca5fe0b5a0b08R142

In my team we have the rule that every variable needs to be quoted and surrounded by braces. Not everyone is very experienced with bash and is not a language we use too often, however we feel is important still have linters and rules that make sure our code is always the same.

However, using this is causing problems due to it behaving differently (we completely understand the reasoning behind this and why it works as it does, not arguing about it).

Is there any way to tell it to never remove braces even if it considers them redundant?

[AlexSkrypnyk]

@citosid https://github.com/AlexSkrypnyk/shellvar