Proposal: Change linter to allow globals and disallow mutation of global variables

[leighmcculloch]

Proposal

Change linter to allow globals and disallow mutation of global variables.

Why

This linter's goal is to reduce side-effects that arise out of global (package level) variables. This is challenging because there are multiple common patterns using package level variables. Many are already exceptions, such as sentinel error values, regular expressions, then there's also a handful of other cases like #33 that aren't currently handled.

Side-effects arise out of the use of globals because any function in a package can mutate an unexported package variable, and any function anywhere in an application can mutate an exported package variable. When any functions mutates these values it can have an effect on other code in the application.

Preventing mutations of package level variables would have the same result of preventing side-effects, since globals that are not mutated do not cause side-effects.

This change would allow globals to be used as pseudo-constants in the many patterns that Go developers use them as such.

This issue is a request for comment from anyone using this linter.

How

It's unclear to me if there's a reasonable way to detect mutations of package level variables.

The simple case of reassignment is, I think, probably easy. We'd need to look for statements that assign to a value where that value being assigned to is a package level value.

However, there's the difficult case of what if someone passes a reference to a package level value to a function. Or what if the type of the package variable is a struct with functions that mutate itself.

If anyone has ideas, please post it to this issue.

[leonklingele]

imo, this should be part of curioswitch/go-reassign#18.

[leighmcculloch]

Interesting, I didn't know about that linter. It looks swish.

imo, this should be part of curioswitch/go-reassign#18.

Maybe. It's unclear to me if that would be scope creep for that linter, but I don't know what their plans are. My understanding of the linters goal is to prevent outside mutation of package level variables that are intended to be publicly accessible by not publicly mutable, and essentially add an access modifier to the package level variables that Go doesn't have as a first level feature.

I think the goal of this issue would be to prevent any mutability of package level variables by anyone, even internals, and the goal is quite different: The goal is to reduce side-effects.

So the goals of both, assuming I'm understanding the goals of go-reassign correctly – which I might not be, are I think different.