Bash - Flag constants as read-only

[MrChocolatine]

trafficstars

Global constants

In your coding guidelines:

https://github.com/mullvad/coding-guidelines/blob/5c5931de7a7b0efde46a3fc91d3ec62ec10a58a4/bash.md?plain=1#L58-L62

May I suggest to also declare these as readonly for more strictness?

The above example would become:

readonly SCREAMING_SNAKE_CASE="something"

Your usual:

SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

would become:

readonly SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

Local/scoped constants

On a very similar note, for variables scoped to their function with local:

https://github.com/mullvad/coding-guidelines/blob/5c5931de7a7b0efde46a3fc91d3ec62ec10a58a4/bash.md?plain=1#L94-L104

For those that should be constants, additionally to having them written in uppercase you could also declare them as read-only with local -r:

function cowsay { 
    local -r COW_PREFIX="The cow says"
    local message="$1" 
    echo "$COW_PREFIX: $message" 
} 

[faern]

Looks good to me! We'll incorporate this in our bash coding guidelines one way or another. Thanks.

[MrChocolatine]

Looks like I was making an update at the same time you commented @faern , I added a section on how to declare local variables as read-only too.

[MrChocolatine]

@faern I would be happy to help you incorporate these changes and update your existing Bash scripts, as it would only be refactors.

Let me know if that's something you would be okay with or if you need to talk more internally before doing any change 👍 .

[faern]

Thank you. But I don't think we want too much churn in our bash scripts. We have plenty of scripts where we don't follow our own guidelines perfectly. So I expect it to be a can of worms. I want to open the lid to that can slowly.

I think we first want to figure out exactly what our guidelines should say. There is a small problem with just doing what you wrote exactly. shellcheck prefers:

VARIABLE=$(...)
readonly VARIABLE

Because otherwise the exit code of the subshell is not handled properly. And I don't think we want to bloat all our scripts with extra lines just because of this. In the end, readonly is nice-to-have. But it's far from important, and readability matters. So we need to figure out when exactly we want to enforce readonly.

[MrChocolatine]

I totally understand 👍 .