Bash - Flag constants as read-only

[MrChocolatine]

Global constants

In your coding guidelines:

coding-guidelines/bash.md

Lines 58 to 62 in 5c5931d

	* Constant variables use `SCREAMING_SNAKE_CASE` names.
	This means variables that are assigned once and then never modified, and that only depend on
	other constant variables. The exception to "never modified" is where multiple statements are
	used to compute the content of the "constant" (see `COMPILE_FLAGS` in example at the end).

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:

coding-guidelines/bash.md

Lines 94 to 104 in 5c5931d

	Use `local` for variables inside functions unless they need to be global for some reason
	(They most likely should not be). This stops execution of functions from polluting the global
	namespace. Do this both for constants and other variables.
	```bash
	function cowsay {
	local COW_PREFIX="The cow says"
	local message="$1"
	echo "$COW_PREFIX: $message"
	}
	```

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 👍 .